Make |gcm128_context| memcpy-safe.

This removes the confusion about whether |gcm128_context| copies the
key (it didn't) or whether the caller is responsible for keeping the
key alive for the lifetime of the |gcm128_context| (it was).

Change-Id: Ia0ad0a8223e664381fbbfb56570b2545f51cad9f
Reviewed-on: https://boringssl-review.googlesource.com/6053
Reviewed-by: Adam Langley <alangley@gmail.com>

3 files changed, 21 insertions, 20 deletions

diff --git a/crypto/modes/gcm.c b/crypto/modes/gcm.c
index 34e5dcf8..c934fbd6 100644
--- a/crypto/modes/gcm.c
+++ b/crypto/modes/gcm.c
@@ -426,7 +426,6 @@ void CRYPTO_gcm128_init(GCM128_CONTEXT *ctx, const void *key,
 
   memset(ctx, 0, sizeof(*ctx));
   ctx->block = block;
-  ctx->key = key;
 
   (*block)(ctx->H.c, ctx->H.c, key);
 
@@ -492,7 +491,8 @@ void CRYPTO_gcm128_init(GCM128_CONTEXT *ctx, const void *key,
 #endif
 }
 
-void CRYPTO_gcm128_setiv(GCM128_CONTEXT *ctx, const uint8_t *iv, size_t len) {
+void CRYPTO_gcm128_setiv(GCM128_CONTEXT *ctx, const void *key,
+                         const uint8_t *iv, size_t len) {
   const union {
     long one;
     char little;
@@ -560,7 +560,7 @@ void CRYPTO_gcm128_setiv(GCM128_CONTEXT *ctx, const uint8_t *iv, size_t len) {
     }
   }
 
-  (*ctx->block)(ctx->Yi.c, ctx->EK0.c, ctx->key);
+  (*ctx->block)(ctx->Yi.c, ctx->EK0.c, key);
   ++ctr;
   if (is_endian.little) {
     PUTU32(ctx->Yi.c + 12, ctr);
@@ -633,8 +633,9 @@ int CRYPTO_gcm128_aad(GCM128_CONTEXT *ctx, const uint8_t *aad, size_t len) {
   return 1;
 }
 
-int CRYPTO_gcm128_encrypt(GCM128_CONTEXT *ctx, const unsigned char *in,
-                          unsigned char *out, size_t len) {
+int CRYPTO_gcm128_encrypt(GCM128_CONTEXT *ctx, const void *key,
+                          const unsigned char *in, unsigned char *out,
+                          size_t len) {
   const union {
     long one;
     char little;
@@ -643,7 +644,6 @@ int CRYPTO_gcm128_encrypt(GCM128_CONTEXT *ctx, const unsigned char *in,
   size_t i;
   uint64_t mlen = ctx->len.u[1];
   block128_f block = ctx->block;
-  const void *key = ctx->key;
 #ifdef GCM_FUNCREF_4BIT
   void (*gcm_gmult_p)(uint64_t Xi[2], const u128 Htable[16]) = ctx->gmult;
 #ifdef GHASH
@@ -793,8 +793,9 @@ int CRYPTO_gcm128_encrypt(GCM128_CONTEXT *ctx, const unsigned char *in,
   return 1;
 }
 
-int CRYPTO_gcm128_decrypt(GCM128_CONTEXT *ctx, const unsigned char *in,
-                          unsigned char *out, size_t len) {
+int CRYPTO_gcm128_decrypt(GCM128_CONTEXT *ctx, const void *key,
+                          const unsigned char *in, unsigned char *out,
+                          size_t len) {
   const union {
     long one;
     char little;
@@ -803,7 +804,6 @@ int CRYPTO_gcm128_decrypt(GCM128_CONTEXT *ctx, const unsigned char *in,
   size_t i;
   uint64_t mlen = ctx->len.u[1];
   block128_f block = ctx->block;
-  const void *key = ctx->key;
 #ifdef GCM_FUNCREF_4BIT
   void (*gcm_gmult_p)(uint64_t Xi[2], const u128 Htable[16]) = ctx->gmult;
 #ifdef GHASH
@@ -960,15 +960,15 @@ int CRYPTO_gcm128_decrypt(GCM128_CONTEXT *ctx, const unsigned char *in,
   return 1;
 }
 
-int CRYPTO_gcm128_encrypt_ctr32(GCM128_CONTEXT *ctx, const uint8_t *in,
-                                uint8_t *out, size_t len, ctr128_f stream) {
+int CRYPTO_gcm128_encrypt_ctr32(GCM128_CONTEXT *ctx, const void *key,
+                                const uint8_t *in, uint8_t *out, size_t len,
+                                ctr128_f stream) {
   const union {
     long one;
     char little;
   } is_endian = {1};
   unsigned int n, ctr;
   uint64_t mlen = ctx->len.u[1];
-  const void *key = ctx->key;
 #ifdef GCM_FUNCREF_4BIT
   void (*gcm_gmult_p)(uint64_t Xi[2], const u128 Htable[16]) = ctx->gmult;
 #ifdef GHASH
@@ -1069,8 +1069,8 @@ int CRYPTO_gcm128_encrypt_ctr32(GCM128_CONTEXT *ctx, const uint8_t *in,
   return 1;
 }
 
-int CRYPTO_gcm128_decrypt_ctr32(GCM128_CONTEXT *ctx, const uint8_t *in,
-                                uint8_t *out, size_t len,
+int CRYPTO_gcm128_decrypt_ctr32(GCM128_CONTEXT *ctx, const void *key,
+                                const uint8_t *in, uint8_t *out, size_t len,
                                 ctr128_f stream) {
   const union {
     long one;
@@ -1078,7 +1078,6 @@ int CRYPTO_gcm128_decrypt_ctr32(GCM128_CONTEXT *ctx, const uint8_t *in,
   } is_endian = {1};
   unsigned int n, ctr;
   uint64_t mlen = ctx->len.u[1];
-  const void *key = ctx->key;
 #ifdef GCM_FUNCREF_4BIT
   void (*gcm_gmult_p)(uint64_t Xi[2], const u128 Htable[16]) = ctx->gmult;
 #ifdef GHASH
diff --git a/crypto/modes/gcm_test.c b/crypto/modes/gcm_test.c
index 89ed792d..fec46de2 100644
--- a/crypto/modes/gcm_test.c
+++ b/crypto/modes/gcm_test.c
@@ -346,13 +346,13 @@ static int run_test_case(unsigned test_num, const struct test_case *test) {
   }
 
   CRYPTO_gcm128_init(&ctx, &aes_key, (block128_f) AES_encrypt);
-  CRYPTO_gcm128_setiv(&ctx, nonce, nonce_len);
+  CRYPTO_gcm128_setiv(&ctx, &aes_key, nonce, nonce_len);
   memset(out, 0, plaintext_len);
   if (additional_data) {
     CRYPTO_gcm128_aad(&ctx, additional_data, additional_data_len);
   }
   if (plaintext) {
-    CRYPTO_gcm128_encrypt(&ctx, plaintext, out, plaintext_len);
+    CRYPTO_gcm128_encrypt(&ctx, &aes_key, plaintext, out, plaintext_len);
   }
   if (!CRYPTO_gcm128_finish(&ctx, tag, tag_len) ||
       (ciphertext && memcmp(out, ciphertext, plaintext_len) != 0)) {
@@ -362,13 +362,13 @@ static int run_test_case(unsigned test_num, const struct test_case *test) {
     goto out;
   }
 
-  CRYPTO_gcm128_setiv(&ctx, nonce, nonce_len);
+  CRYPTO_gcm128_setiv(&ctx, &aes_key, nonce, nonce_len);
   memset(out, 0, plaintext_len);
   if (additional_data) {
     CRYPTO_gcm128_aad(&ctx, additional_data, additional_data_len);
   }
   if (ciphertext) {
-    CRYPTO_gcm128_decrypt(&ctx, ciphertext, out, plaintext_len);
+    CRYPTO_gcm128_decrypt(&ctx, &aes_key, ciphertext, out, plaintext_len);
   }
   if (!CRYPTO_gcm128_finish(&ctx, tag, tag_len)) {
     fprintf(stderr, "%u: decrypt failed.\n", test_num);
diff --git a/crypto/modes/internal.h b/crypto/modes/internal.h
index 0c2200fb..6d881ecd 100644
--- a/crypto/modes/internal.h
+++ b/crypto/modes/internal.h
@@ -152,6 +152,9 @@ __inline uint32_t _bswap4(uint32_t val) {
 /* GCM definitions */
 typedef struct { uint64_t hi,lo; } u128;
 
+/* This differs from upstream's |gcm128_context| in that it does not have the
+ * |key| pointer, in order to make it |memcpy|-friendly. Rather the key is
+ * passed into each call that needs it. */
 struct gcm128_context {
   /* Following 6 names follow names in GCM specification */
   union {
@@ -170,7 +173,6 @@ struct gcm128_context {
 
   unsigned int mres, ares;
   block128_f block;
-  const void *key;
 };
 
 struct ccm128_context {