Make AES GCM more consistent cross platform (#189)

Co-authored-by: sbiscigl <[email protected]>
Co-authored-by: Michael Graeb <[email protected]>

.github/workflows/ci.yml

@@ -129,6 +129,15 @@ jobs:
         python -c "from urllib.request import urlretrieve; urlretrieve('${{ env.BUILDER_HOST }}/${{ env.BUILDER_SOURCE }}/${{ env.BUILDER_VERSION }}/builder.pyz?run=${{ env.RUN }}', 'builder.pyz')"
         python builder.pyz build -p ${{ env.PACKAGE_NAME }}
 
+  windows-debug:
+    runs-on: windows-2022 # latest
+    steps:
+    - name: Build ${{ env.PACKAGE_NAME }} + consumers
+      run: |
+        python -c "from urllib.request import urlretrieve; urlretrieve('${{ env.BUILDER_HOST }}/${{ env.BUILDER_SOURCE }}/${{ env.BUILDER_VERSION }}/builder.pyz?run=${{ env.RUN }}', 'builder.pyz')"
+        python builder.pyz build -p ${{ env.PACKAGE_NAME }} --config Debug --variant=just-tests
+
+
   windows-vc14:
     runs-on: windows-2019 # windows-2019 is last env with Visual Studio 2015 (v14.0)
     strategy:

builder.json

@@ -90,6 +90,11 @@
         },
         "no-tests": {
             "!test_steps": []
+        },
+        "just-tests": {
+            "!test_steps": [
+                "test"
+            ]
         }
     },
     "test_steps": [

include/aws/cal/private/symmetric_cipher_priv.h

@@ -34,9 +34,9 @@ struct aws_symmetric_cipher {
     /**
      deprecated for use, only for backwards compat.
      Use state to represent current state of cipher.
-     good represented if the ciphter was initialized
+     good represented if the cipher was initialized
      without any errors, ready to process input,
-     and not finialized yet. This corresponds to
+     and not finalized yet. This corresponds to
      the state AWS_SYMMETRIC_CIPHER_READY.
     */
     bool good;

include/aws/cal/symmetric_cipher.h

@@ -26,8 +26,7 @@ typedef struct aws_symmetric_cipher *(aws_aes_ctr_256_new_fn)(struct aws_allocat
 typedef struct aws_symmetric_cipher *(aws_aes_gcm_256_new_fn)(struct aws_allocator *allocator,
                                                               const struct aws_byte_cursor *key,
                                                               const struct aws_byte_cursor *iv,
-                                                              const struct aws_byte_cursor *aad,
-                                                              const struct aws_byte_cursor *decryption_tag);
+                                                              const struct aws_byte_cursor *aad);
 
 typedef struct aws_symmetric_cipher *(aws_aes_keywrap_256_new_fn)(struct aws_allocator *allocator,
                                                                   const struct aws_byte_cursor *key);
@@ -88,24 +87,23 @@ AWS_CAL_API struct aws_symmetric_cipher *aws_aes_ctr_256_new(
  *
  * respectively.
  *
- * If they are set, that key and iv will be copied internally and used by the cipher.
- *
- * If tag and aad are set they will be copied internally and used by the cipher.
- * decryption_tag would most likely be used for a decrypt operation to detect tampering or corruption.
- * The Tag for the most recent encrypt operation will be available in:
+ * If aad is set it will be copied and applied to the cipher.
  *
- * aws_symmetric_cipher_get_tag()
+ * If they are set, that key and iv will be copied internally and used by the cipher.
  *
- * If aad is set it will be copied and applied to the cipher.
+ * For decryption purposes tag can be provided via aws_symmetric_cipher_set_tag method.
+ * Note: for decrypt operations, tag must be provided before first decrypt is called.
+ * (this is a windows bcrypt limitations, but for consistency sake same limitation is extended to other platforms)
+ * Tag generated during encryption can be retrieved using aws_symmetric_cipher_get_tag method
+ * after finalize is called.
  *
  * Returns NULL on failure. You can check aws_last_error() to get the error code indicating the failure cause.
  */
 AWS_CAL_API struct aws_symmetric_cipher *aws_aes_gcm_256_new(
     struct aws_allocator *allocator,
     const struct aws_byte_cursor *key,
     const struct aws_byte_cursor *iv,
-    const struct aws_byte_cursor *aad,
-    const struct aws_byte_cursor *decryption_tag);
+    const struct aws_byte_cursor *aad);
 
 /**
  * Creates an instance of AES Keywrap with 256-bit key.
@@ -191,6 +189,12 @@ AWS_CAL_API int aws_symmetric_cipher_finalize_decryption(struct aws_symmetric_ci
  * Resets the cipher state for starting a new encrypt or decrypt operation. Note encrypt/decrypt cannot be mixed on the
  * same cipher without a call to reset in between them. However, this leaves the key, iv etc... materials setup for
  * immediate reuse.
+ * Note: GCM tag is not preserved between operations. If you intend to do encrypt followed directly by decrypt, make
+ * sure to make a copy of tag before reseting the cipher and pass that copy for decryption.
+ *
+ * Warning: In most cases it's a really bad idea to reset a cipher and perform another operation using that cipher.
+ * Key and IV should not be reused for different operations. Instead of reseting the cipher, destroy the cipher
+ * and create new one with a new key/iv pair. Use reset at your own risk, and only after careful consideration.
  *
  * returns AWS_OP_SUCCESS on success. Call aws_last_error() to determine the failure cause if it returns
  * AWS_OP_ERR;
@@ -208,6 +212,11 @@ AWS_CAL_API int aws_symmetric_cipher_reset(struct aws_symmetric_cipher *cipher);
  */
 AWS_CAL_API struct aws_byte_cursor aws_symmetric_cipher_get_tag(const struct aws_symmetric_cipher *cipher);
 
+/**
+ * Sets the GMAC tag on the cipher. Does nothing for ciphers that do not support tag.
+ */
+AWS_CAL_API void aws_symmetric_cipher_set_tag(struct aws_symmetric_cipher *cipher, struct aws_byte_cursor tag);
+
 /**
  * Gets the original initialization vector as a cursor.
  * The memory in this cursor is unsafe as it refers to the internal buffer.
@@ -241,7 +250,7 @@ AWS_CAL_API bool aws_symmetric_cipher_is_good(const struct aws_symmetric_cipher
 
 /**
  * Retuns the current state of the cipher. Ther state of the cipher can be ready for use, finalized, or has encountered
- * an error. if the cipher is in a finished or eror state, it must be reset before further use.
+ * an error. if the cipher is in a finished or error state, it must be reset before further use.
  */
 AWS_CAL_API enum aws_symmetric_cipher_state aws_symmetric_cipher_get_state(const struct aws_symmetric_cipher *cipher);
 

source/darwin/commoncrypto_aes.c

@@ -360,6 +360,14 @@ struct aws_symmetric_cipher *aws_aes_ctr_256_new_impl(
     return &cc_cipher->cipher_base;
 }
 
+static int s_gcm_decrypt(struct aws_symmetric_cipher *cipher, struct aws_byte_cursor input, struct aws_byte_buf *out) {
+    if (cipher->tag.buffer == NULL) {
+        return aws_raise_error(AWS_ERROR_INVALID_ARGUMENT);
+    }
+
+    return s_decrypt(cipher, input, out);
+}
+
 #ifdef SUPPORT_AES_GCM_VIA_SPI
 
 /*
@@ -428,7 +436,7 @@ static int s_finalize_gcm_decryption(struct aws_symmetric_cipher *cipher, struct
     struct cc_aes_cipher *cc_cipher = cipher->impl;
 
     size_t tag_length = AWS_AES_256_CIPHER_BLOCK_SIZE;
-    CCStatus status = s_cc_crypto_gcm_finalize(cc_cipher->encryptor_handle, cipher->tag.buffer, tag_length);
+    CCStatus status = s_cc_crypto_gcm_finalize(cc_cipher->decryptor_handle, cipher->tag.buffer, tag_length);
     if (status != kCCSuccess) {
         cipher->state = AWS_SYMMETRIC_CIPHER_ERROR;
         return aws_raise_error(AWS_ERROR_INVALID_ARGUMENT);
@@ -441,8 +449,7 @@ static int s_initialize_gcm_cipher_materials(
     struct cc_aes_cipher *cc_cipher,
     const struct aws_byte_cursor *key,
     const struct aws_byte_cursor *iv,
-    const struct aws_byte_cursor *aad,
-    const struct aws_byte_cursor *tag) {
+    const struct aws_byte_cursor *aad) {
     if (!cc_cipher->cipher_base.key.len) {
         if (key) {
             aws_byte_buf_init_copy_from_cursor(&cc_cipher->cipher_base.key, cc_cipher->cipher_base.allocator, *key);
@@ -471,10 +478,6 @@ static int s_initialize_gcm_cipher_materials(
         aws_byte_buf_init_copy_from_cursor(&cc_cipher->cipher_base.aad, cc_cipher->cipher_base.allocator, *aad);
     }
 
-    if (tag && tag->len) {
-        aws_byte_buf_init_copy_from_cursor(&cc_cipher->cipher_base.tag, cc_cipher->cipher_base.allocator, *tag);
-    }
-
     CCCryptorStatus status = CCCryptorCreateWithMode(
         kCCEncrypt,
         kCCModeGCM,
@@ -548,9 +551,10 @@ static int s_gcm_reset(struct aws_symmetric_cipher *cipher) {
     struct cc_aes_cipher *cc_cipher = cipher->impl;
 
     int ret_val = s_reset(cipher);
+    aws_byte_buf_clean_up_secure(&cc_cipher->cipher_base.tag);
 
     if (ret_val == AWS_OP_SUCCESS) {
-        ret_val = s_initialize_gcm_cipher_materials(cc_cipher, NULL, NULL, NULL, NULL);
+        ret_val = s_initialize_gcm_cipher_materials(cc_cipher, NULL, NULL, NULL);
     }
 
     return ret_val;
@@ -559,7 +563,7 @@ static int s_gcm_reset(struct aws_symmetric_cipher *cipher) {
 static struct aws_symmetric_cipher_vtable s_aes_gcm_vtable = {
     .finalize_decryption = s_finalize_gcm_decryption,
     .finalize_encryption = s_finalize_gcm_encryption,
-    .decrypt = s_decrypt,
+    .decrypt = s_gcm_decrypt,
     .encrypt = s_encrypt,
     .provider = "CommonCrypto",
     .alg_name = "AES-GCM 256",
@@ -571,15 +575,14 @@ struct aws_symmetric_cipher *aws_aes_gcm_256_new_impl(
     struct aws_allocator *allocator,
     const struct aws_byte_cursor *key,
     const struct aws_byte_cursor *iv,
-    const struct aws_byte_cursor *aad,
-    const struct aws_byte_cursor *tag) {
+    const struct aws_byte_cursor *aad) {
     struct cc_aes_cipher *cc_cipher = aws_mem_calloc(allocator, 1, sizeof(struct cc_aes_cipher));
     cc_cipher->cipher_base.allocator = allocator;
     cc_cipher->cipher_base.block_size = AWS_AES_256_CIPHER_BLOCK_SIZE;
     cc_cipher->cipher_base.impl = cc_cipher;
     cc_cipher->cipher_base.vtable = &s_aes_gcm_vtable;
 
-    if (s_initialize_gcm_cipher_materials(cc_cipher, key, iv, aad, tag) != AWS_OP_SUCCESS) {
+    if (s_initialize_gcm_cipher_materials(cc_cipher, key, iv, aad) != AWS_OP_SUCCESS) {
         s_destroy(&cc_cipher->cipher_base);
         return NULL;
     }
@@ -596,14 +599,12 @@ struct aws_symmetric_cipher *aws_aes_gcm_256_new_impl(
     struct aws_allocator *allocator,
     const struct aws_byte_cursor *key,
     const struct aws_byte_cursor *iv,
-    const struct aws_byte_cursor *aad,
-    const struct aws_byte_cursor *tag) {
+    const struct aws_byte_cursor *aad) {
 
     (void)allocator;
     (void)key;
     (void)iv;
     (void)aad;
-    (void)tag;
     aws_raise_error(AWS_ERROR_PLATFORM_NOT_SUPPORTED);
     return NULL;
 }

source/symmetric_cipher.c

@@ -22,8 +22,7 @@ extern struct aws_symmetric_cipher *aws_aes_gcm_256_new_impl(
     struct aws_allocator *allocator,
     const struct aws_byte_cursor *key,
     const struct aws_byte_cursor *iv,
-    const struct aws_byte_cursor *aad,
-    const struct aws_byte_cursor *decryption_tag);
+    const struct aws_byte_cursor *aad);
 
 extern struct aws_symmetric_cipher *aws_aes_keywrap_256_new_impl(
     struct aws_allocator *allocator,
@@ -54,13 +53,11 @@ struct aws_symmetric_cipher *aws_aes_gcm_256_new_impl(
     struct aws_allocator *allocator,
     const struct aws_byte_cursor *key,
     const struct aws_byte_cursor *iv,
-    const struct aws_byte_cursor *aad,
-    const struct aws_byte_cursor *decryption_tag) {
+    const struct aws_byte_cursor *aad) {
     (void)allocator;
     (void)key;
     (void)iv;
     (void)aad;
-    (void)decryption_tag;
     abort();
 }
 
@@ -127,13 +124,12 @@ struct aws_symmetric_cipher *aws_aes_gcm_256_new(
     struct aws_allocator *allocator,
     const struct aws_byte_cursor *key,
     const struct aws_byte_cursor *iv,
-    const struct aws_byte_cursor *aad,
-    const struct aws_byte_cursor *decryption_tag) {
+    const struct aws_byte_cursor *aad) {
     if (s_validate_key_materials(key, AWS_AES_256_KEY_BYTE_LEN, iv, AWS_AES_256_CIPHER_BLOCK_SIZE - sizeof(uint32_t)) !=
         AWS_OP_SUCCESS) {
         return NULL;
     }
-    return s_aes_gcm_new_fn(allocator, key, iv, aad, decryption_tag);
+    return s_aes_gcm_new_fn(allocator, key, iv, aad);
 }
 
 struct aws_symmetric_cipher *aws_aes_keywrap_256_new(
@@ -223,6 +219,12 @@ struct aws_byte_cursor aws_symmetric_cipher_get_tag(const struct aws_symmetric_c
     return aws_byte_cursor_from_buf(&cipher->tag);
 }
 
+void aws_symmetric_cipher_set_tag(struct aws_symmetric_cipher *cipher, struct aws_byte_cursor tag) {
+    AWS_PRECONDITION(aws_byte_cursor_is_valid(&tag));
+    aws_byte_buf_clean_up_secure(&cipher->tag);
+    aws_byte_buf_init_copy_from_cursor(&cipher->tag, cipher->allocator, tag);
+}
+
 struct aws_byte_cursor aws_symmetric_cipher_get_initialization_vector(const struct aws_symmetric_cipher *cipher) {
     return aws_byte_cursor_from_buf(&cipher->iv);
 }

source/unix/openssl_aes.c

@@ -123,9 +123,7 @@ static void s_destroy(struct aws_symmetric_cipher *cipher) {
     aws_byte_buf_clean_up_secure(&cipher->key);
     aws_byte_buf_clean_up_secure(&cipher->iv);
 
-    if (cipher->tag.buffer) {
-        aws_byte_buf_clean_up_secure(&cipher->tag);
-    }
+    aws_byte_buf_clean_up_secure(&cipher->tag);
 
     if (cipher->aad.buffer) {
         aws_byte_buf_clean_up_secure(&cipher->aad);
@@ -326,28 +324,51 @@ struct aws_symmetric_cipher *aws_aes_ctr_256_new_impl(
     return NULL;
 }
 
+static int s_gcm_decrypt(struct aws_symmetric_cipher *cipher, struct aws_byte_cursor input, struct aws_byte_buf *out) {
+    if (cipher->tag.buffer == NULL) {
+        return aws_raise_error(AWS_ERROR_INVALID_ARGUMENT);
+    }
+
+    return s_decrypt(cipher, input, out);
+}
+
 static int s_finalize_gcm_encryption(struct aws_symmetric_cipher *cipher, struct aws_byte_buf *out) {
     struct openssl_aes_cipher *openssl_cipher = cipher->impl;
 
+    if (cipher->tag.buffer == NULL) {
+        aws_byte_buf_init(&cipher->tag, cipher->allocator, AWS_AES_256_CIPHER_BLOCK_SIZE);
+    }
+
     int ret_val = s_finalize_encryption(cipher, out);
 
     if (ret_val == AWS_OP_SUCCESS) {
-        if (!cipher->tag.len) {
-            if (!EVP_CIPHER_CTX_ctrl(
-                    openssl_cipher->encryptor_ctx,
-                    EVP_CTRL_GCM_GET_TAG,
-                    (int)cipher->tag.capacity,
-                    cipher->tag.buffer)) {
-                cipher->state = AWS_SYMMETRIC_CIPHER_ERROR;
-                return aws_raise_error(AWS_ERROR_INVALID_ARGUMENT);
-            }
-            cipher->tag.len = AWS_AES_256_CIPHER_BLOCK_SIZE;
+        if (!EVP_CIPHER_CTX_ctrl(
+                openssl_cipher->encryptor_ctx, EVP_CTRL_GCM_GET_TAG, (int)cipher->tag.capacity, cipher->tag.buffer)) {
+            cipher->state = AWS_SYMMETRIC_CIPHER_ERROR;
+            return aws_raise_error(AWS_ERROR_INVALID_ARGUMENT);
         }
+        cipher->tag.len = AWS_AES_256_CIPHER_BLOCK_SIZE;
     }
 
     return ret_val;
 }
 
+static int s_finalize_gcm_decryption(struct aws_symmetric_cipher *cipher, struct aws_byte_buf *out) {
+    struct openssl_aes_cipher *openssl_cipher = cipher->impl;
+
+    if (openssl_cipher->cipher_base.tag.len) {
+        if (!EVP_CIPHER_CTX_ctrl(
+                openssl_cipher->decryptor_ctx,
+                EVP_CTRL_GCM_SET_TAG,
+                (int)openssl_cipher->cipher_base.tag.len,
+                openssl_cipher->cipher_base.tag.buffer)) {
+            return aws_raise_error(AWS_ERROR_INVALID_ARGUMENT);
+        }
+    }
+
+    return s_finalize_decryption(cipher, out);
+}
+
 static int s_init_gcm_cipher_materials(struct aws_symmetric_cipher *cipher) {
     struct openssl_aes_cipher *openssl_cipher = cipher->impl;
 
@@ -388,15 +409,7 @@ static int s_init_gcm_cipher_materials(struct aws_symmetric_cipher *cipher) {
         }
     }
 
-    if (openssl_cipher->cipher_base.tag.len) {
-        if (!EVP_CIPHER_CTX_ctrl(
-                openssl_cipher->decryptor_ctx,
-                EVP_CTRL_GCM_SET_TAG,
-                (int)openssl_cipher->cipher_base.tag.len,
-                openssl_cipher->cipher_base.tag.buffer)) {
-            return aws_raise_error(AWS_ERROR_INVALID_ARGUMENT);
-        }
-    }
+    aws_byte_buf_clean_up_secure(&openssl_cipher->cipher_base.tag);
 
     return AWS_OP_SUCCESS;
 }
@@ -416,18 +429,17 @@ static struct aws_symmetric_cipher_vtable s_gcm_vtable = {
     .provider = "OpenSSL Compatible LibCrypto",
     .destroy = s_destroy,
     .reset = s_reset_gcm_cipher_materials,
-    .decrypt = s_decrypt,
+    .decrypt = s_gcm_decrypt,
     .encrypt = s_encrypt,
-    .finalize_decryption = s_finalize_decryption,
+    .finalize_decryption = s_finalize_gcm_decryption,
     .finalize_encryption = s_finalize_gcm_encryption,
 };
 
 struct aws_symmetric_cipher *aws_aes_gcm_256_new_impl(
     struct aws_allocator *allocator,
     const struct aws_byte_cursor *key,
     const struct aws_byte_cursor *iv,
-    const struct aws_byte_cursor *aad,
-    const struct aws_byte_cursor *decryption_tag) {
+    const struct aws_byte_cursor *aad) {
 
     struct openssl_aes_cipher *cipher = aws_mem_calloc(allocator, 1, sizeof(struct openssl_aes_cipher));
     cipher->cipher_base.allocator = allocator;
@@ -465,14 +477,6 @@ struct aws_symmetric_cipher *aws_aes_gcm_256_new_impl(
         aws_byte_buf_init_copy_from_cursor(&cipher->cipher_base.aad, allocator, *aad);
     }
 
-    /* Set tag for the decryptor to use.*/
-    if (decryption_tag) {
-        aws_byte_buf_init_copy_from_cursor(&cipher->cipher_base.tag, allocator, *decryption_tag);
-    } else {
-        /* we'll need this later when we grab the tag during encryption time. */
-        aws_byte_buf_init(&cipher->cipher_base.tag, allocator, AWS_AES_256_CIPHER_BLOCK_SIZE);
-    }
-
     /* Initialize the cipher contexts with the specified key and IV. */
     if (s_init_gcm_cipher_materials(&cipher->cipher_base)) {
         goto error;