address PR feedback

awslabs · May 8, 2024 · 019a62e · 019a62e
1 parent 8e18c62
commit 019a62e
Show file tree

Hide file tree

Showing 7 changed files with 120 additions and 107 deletions.
diff --git a/include/aws/cal/private/symmetric_cipher_priv.h b/include/aws/cal/private/symmetric_cipher_priv.h
@@ -31,8 +31,14 @@ struct aws_symmetric_cipher {
     struct aws_byte_buf tag;
     size_t block_size;
     size_t key_length_bits;
-    // deprecated for use, only for backwards compat
-    // use state to represent current state of cipher.
+    /**
+     deprecated for use, only for backwards compat.
+     Use state to represent current state of cipher.
+     good represented if the ciphter was initialized
+     without any errors, ready to process input,
+     and not finialized yet. This corresponds to
+     the state AWS_SYMMETRIC_CIPHER_READY.
+    */
     bool good;
     enum aws_symmetric_cipher_state state;
     void *impl;

diff --git a/include/aws/cal/symmetric_cipher.h b/include/aws/cal/symmetric_cipher.h
@@ -35,7 +35,11 @@ typedef struct aws_symmetric_cipher *(aws_aes_gcm_256_new_fn)(
 typedef struct aws_symmetric_cipher *(
     aws_aes_keywrap_256_new_fn)(struct aws_allocator *allocator, const struct aws_byte_cursor *key);
 
-enum aws_symmetric_cipher_state { AWS_CIPHER_READY, AWS_CIPHER_FINALIZED, AWS_CIPHER_ERROR };
+enum aws_symmetric_cipher_state {
+    AWS_SYMMETRIC_CIPHER_READY,
+    AWS_SYMMETRIC_CIPHER_FINALIZED,
+    AWS_SYMMETRIC_CIPHER_ERROR,
+};
 
 AWS_EXTERN_C_BEGIN
 
@@ -238,13 +242,12 @@ AWS_CAL_API struct aws_byte_cursor aws_symmetric_cipher_get_key(const struct aws
  */
 AWS_CAL_API bool aws_symmetric_cipher_is_good(const struct aws_symmetric_cipher *cipher);
 
-/**
- * Retuns the current state of the cipher. If the cipher is finished that means it must
- * be re-set to be used again, but its most recent result succeeded successfully.
- */
-AWS_CAL_API enum aws_symmetric_cipher_state aws_symmetric_cipher_get_state(const struct aws_symmetric_cipher *cipher);
-
 AWS_EXTERN_C_END
 AWS_POP_SANE_WARNING_LEVEL
 
+/**
+ * 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.
+ */
+AWS_CAL_API enum aws_symmetric_cipher_state aws_symmetric_cipher_get_state(const struct aws_symmetric_cipher *cipher);
 #endif /* AWS_CAL_SYMMETRIC_CIPHER_H */
diff --git a/source/darwin/commoncrypto_aes.c b/source/darwin/commoncrypto_aes.c
@@ -46,7 +46,7 @@ static int s_encrypt(struct aws_symmetric_cipher *cipher, struct aws_byte_cursor
         cc_cipher->encryptor_handle, input.ptr, input.len, out->buffer + out->len, available_write_space, &len_written);
 
     if (status != kCCSuccess) {
-        cipher->state = AWS_CIPHER_ERROR;
+        cipher->state = AWS_SYMMETRIC_CIPHER_ERROR;
         return aws_raise_error(AWS_ERROR_INVALID_ARGUMENT);
     }
 
@@ -70,7 +70,7 @@ static int s_decrypt(struct aws_symmetric_cipher *cipher, struct aws_byte_cursor
         cc_cipher->decryptor_handle, input.ptr, input.len, out->buffer + out->len, available_write_space, &len_written);
 
     if (status != kCCSuccess) {
-        cipher->state = AWS_CIPHER_ERROR;
+        cipher->state = AWS_SYMMETRIC_CIPHER_ERROR;
         return aws_raise_error(AWS_ERROR_INVALID_ARGUMENT);
     }
 
@@ -95,7 +95,7 @@ static int s_finalize_encryption(struct aws_symmetric_cipher *cipher, struct aws
         CCCryptorFinal(cc_cipher->encryptor_handle, out->buffer + out->len, available_write_space, &len_written);
 
     if (status != kCCSuccess) {
-        cipher->state = AWS_CIPHER_ERROR;
+        cipher->state = AWS_SYMMETRIC_CIPHER_ERROR;
         return aws_raise_error(AWS_ERROR_INVALID_ARGUMENT);
     }
 
@@ -120,7 +120,7 @@ static int s_finalize_decryption(struct aws_symmetric_cipher *cipher, struct aws
         CCCryptorFinal(cc_cipher->decryptor_handle, out->buffer + out->len, available_write_space, &len_written);
 
     if (status != kCCSuccess) {
-        cipher->state = AWS_CIPHER_ERROR;
+        cipher->state = AWS_SYMMETRIC_CIPHER_ERROR;
         return aws_raise_error(AWS_ERROR_INVALID_ARGUMENT);
     }
 
@@ -254,7 +254,7 @@ struct aws_symmetric_cipher *aws_aes_cbc_256_new_impl(
         return NULL;
     }
 
-    cc_cipher->cipher_base.state = AWS_CIPHER_READY;
+    cc_cipher->cipher_base.state = AWS_SYMMETRIC_CIPHER_READY;
     cc_cipher->cipher_base.key_length_bits = AWS_AES_256_KEY_BIT_LEN;
 
     return &cc_cipher->cipher_base;
@@ -354,7 +354,7 @@ struct aws_symmetric_cipher *aws_aes_ctr_256_new_impl(
         return NULL;
     }
 
-    cc_cipher->cipher_base.state = AWS_CIPHER_READY;
+    cc_cipher->cipher_base.state = AWS_SYMMETRIC_CIPHER_READY;
     cc_cipher->cipher_base.key_length_bits = AWS_AES_256_KEY_BIT_LEN;
 
     return &cc_cipher->cipher_base;
@@ -414,7 +414,7 @@ static int s_finalize_gcm_encryption(struct aws_symmetric_cipher *cipher, struct
     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);
     if (status != kCCSuccess) {
-        cipher->state = AWS_CIPHER_ERROR;
+        cipher->state = AWS_SYMMETRIC_CIPHER_ERROR;
         return aws_raise_error(AWS_ERROR_INVALID_ARGUMENT);
     }
 
@@ -430,7 +430,7 @@ static int s_finalize_gcm_decryption(struct aws_symmetric_cipher *cipher, struct
     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);
     if (status != kCCSuccess) {
-        cipher->state = AWS_CIPHER_ERROR;
+        cipher->state = AWS_SYMMETRIC_CIPHER_ERROR;
         return aws_raise_error(AWS_ERROR_INVALID_ARGUMENT);
     }
 
@@ -584,7 +584,7 @@ struct aws_symmetric_cipher *aws_aes_gcm_256_new_impl(
         return NULL;
     }
 
-    cc_cipher->cipher_base.state = AWS_CIPHER_READY;
+    cc_cipher->cipher_base.state = AWS_SYMMETRIC_CIPHER_READY;
     cc_cipher->cipher_base.key_length_bits = AWS_AES_256_KEY_BIT_LEN;
 
     return &cc_cipher->cipher_base;
@@ -622,7 +622,7 @@ static int s_finalize_keywrap_encryption(struct aws_symmetric_cipher *cipher, st
     struct cc_aes_cipher *cc_cipher = cipher->impl;
 
     if (cc_cipher->working_buffer.len == 0) {
-        cipher->state = AWS_CIPHER_ERROR;
+        cipher->state = AWS_SYMMETRIC_CIPHER_ERROR;
         return aws_raise_error(AWS_ERROR_INVALID_STATE);
     }
 
@@ -644,7 +644,7 @@ static int s_finalize_keywrap_encryption(struct aws_symmetric_cipher *cipher, st
         &output_buffer_len);
 
     if (status != kCCSuccess) {
-        cipher->state = AWS_CIPHER_ERROR;
+        cipher->state = AWS_SYMMETRIC_CIPHER_ERROR;
         return aws_raise_error(AWS_ERROR_INVALID_STATE);
     }
 
@@ -657,7 +657,7 @@ static int s_finalize_keywrap_decryption(struct aws_symmetric_cipher *cipher, st
     struct cc_aes_cipher *cc_cipher = cipher->impl;
 
     if (cc_cipher->working_buffer.len == 0) {
-        cipher->state = AWS_CIPHER_ERROR;
+        cipher->state = AWS_SYMMETRIC_CIPHER_ERROR;
         return aws_raise_error(AWS_ERROR_INVALID_STATE);
     }
 
@@ -679,7 +679,7 @@ static int s_finalize_keywrap_decryption(struct aws_symmetric_cipher *cipher, st
         &output_buffer_len);
 
     if (status != kCCSuccess) {
-        cipher->state = AWS_CIPHER_ERROR;
+        cipher->state = AWS_SYMMETRIC_CIPHER_ERROR;
         return aws_raise_error(AWS_ERROR_INVALID_STATE);
     }
 
@@ -716,7 +716,7 @@ struct aws_symmetric_cipher *aws_aes_keywrap_256_new_impl(
     }
 
     aws_byte_buf_init(&cc_cipher->working_buffer, allocator, (AWS_AES_256_CIPHER_BLOCK_SIZE * 2) + 8);
-    cc_cipher->cipher_base.state = AWS_CIPHER_READY;
+    cc_cipher->cipher_base.state = AWS_SYMMETRIC_CIPHER_READY;
     cc_cipher->cipher_base.key_length_bits = AWS_AES_256_KEY_BIT_LEN;
 
     return &cc_cipher->cipher_base;

diff --git a/source/symmetric_cipher.c b/source/symmetric_cipher.c
@@ -162,7 +162,7 @@ int aws_symmetric_cipher_encrypt(
         return AWS_OP_ERR;
     }
 
-    if (cipher->state == AWS_CIPHER_READY) {
+    if (cipher->state == AWS_SYMMETRIC_CIPHER_READY) {
         return cipher->vtable->encrypt(cipher, to_encrypt, out);
     }
 
@@ -180,27 +180,31 @@ int aws_symmetric_cipher_decrypt(
         return AWS_OP_ERR;
     }
 
-    if (cipher->state == AWS_CIPHER_READY) {
+    if (cipher->state == AWS_SYMMETRIC_CIPHER_READY) {
         return cipher->vtable->decrypt(cipher, to_decrypt, out);
     }
 
     return aws_raise_error(AWS_ERROR_INVALID_STATE);
 }
 
 int aws_symmetric_cipher_finalize_encryption(struct aws_symmetric_cipher *cipher, struct aws_byte_buf *out) {
-    if (cipher->state == AWS_CIPHER_READY) {
+    if (cipher->state == AWS_SYMMETRIC_CIPHER_READY) {
         int ret_val = cipher->vtable->finalize_encryption(cipher, out);
-        cipher->state = AWS_CIPHER_FINALIZED;
+        if (cipher->state != AWS_SYMMETRIC_CIPHER_ERROR) {
+            cipher->state = AWS_SYMMETRIC_CIPHER_FINALIZED;
+        }
         return ret_val;
     }
 
     return aws_raise_error(AWS_ERROR_INVALID_STATE);
 }
 
 int aws_symmetric_cipher_finalize_decryption(struct aws_symmetric_cipher *cipher, struct aws_byte_buf *out) {
-    if (cipher->state == AWS_CIPHER_READY) {
+    if (cipher->state == AWS_SYMMETRIC_CIPHER_READY) {
         int ret_val = cipher->vtable->finalize_decryption(cipher, out);
-        cipher->state = AWS_CIPHER_FINALIZED;
+        if (cipher->state != AWS_SYMMETRIC_CIPHER_ERROR) {
+            cipher->state = AWS_SYMMETRIC_CIPHER_FINALIZED;
+        }
         return ret_val;
     }
     return aws_raise_error(AWS_ERROR_INVALID_STATE);
@@ -209,7 +213,7 @@ int aws_symmetric_cipher_finalize_decryption(struct aws_symmetric_cipher *cipher
 int aws_symmetric_cipher_reset(struct aws_symmetric_cipher *cipher) {
     int ret_val = cipher->vtable->reset(cipher);
     if (ret_val == AWS_OP_SUCCESS) {
-        cipher->state = AWS_CIPHER_READY;
+        cipher->state = AWS_SYMMETRIC_CIPHER_READY;
     }
 
     return ret_val;
@@ -228,7 +232,7 @@ struct aws_byte_cursor aws_symmetric_cipher_get_key(const struct aws_symmetric_c
 }
 
 bool aws_symmetric_cipher_is_good(const struct aws_symmetric_cipher *cipher) {
-    return cipher->state == AWS_CIPHER_READY;
+    return cipher->state == AWS_SYMMETRIC_CIPHER_READY;
 }
 
 enum aws_symmetric_cipher_state aws_symmetric_cipher_get_state(const struct aws_symmetric_cipher *cipher) {

diff --git a/source/unix/openssl_aes.c b/source/unix/openssl_aes.c
@@ -41,7 +41,7 @@ static int s_encrypt(struct aws_symmetric_cipher *cipher, struct aws_byte_cursor
     int len_written = (int)(available_write_space);
     if (!EVP_EncryptUpdate(
             openssl_cipher->encryptor_ctx, out->buffer + out->len, &len_written, input.ptr, (int)input.len)) {
-        cipher->state = AWS_CIPHER_ERROR;
+        cipher->state = AWS_SYMMETRIC_CIPHER_ERROR;
         return aws_raise_error(AWS_ERROR_INVALID_ARGUMENT);
     }
 
@@ -60,7 +60,7 @@ static int s_finalize_encryption(struct aws_symmetric_cipher *cipher, struct aws
 
     int len_written = (int)(out->capacity - out->len);
     if (!EVP_EncryptFinal_ex(openssl_cipher->encryptor_ctx, out->buffer + out->len, &len_written)) {
-        cipher->state = AWS_CIPHER_ERROR;
+        cipher->state = AWS_SYMMETRIC_CIPHER_ERROR;
         return aws_raise_error(AWS_ERROR_INVALID_ARGUMENT);
     }
 
@@ -82,7 +82,7 @@ static int s_decrypt(struct aws_symmetric_cipher *cipher, struct aws_byte_cursor
     int len_written = (int)available_write_space;
     if (!EVP_DecryptUpdate(
             openssl_cipher->decryptor_ctx, out->buffer + out->len, &len_written, input.ptr, (int)input.len)) {
-        cipher->state = AWS_CIPHER_ERROR;
+        cipher->state = AWS_SYMMETRIC_CIPHER_ERROR;
         return aws_raise_error(AWS_ERROR_INVALID_ARGUMENT);
     }
 
@@ -101,7 +101,7 @@ static int s_finalize_decryption(struct aws_symmetric_cipher *cipher, struct aws
 
     int len_written = (int)out->capacity - out->len;
     if (!EVP_DecryptFinal_ex(openssl_cipher->decryptor_ctx, out->buffer + out->len, &len_written)) {
-        cipher->state = AWS_CIPHER_ERROR;
+        cipher->state = AWS_SYMMETRIC_CIPHER_ERROR;
         return aws_raise_error(AWS_ERROR_INVALID_ARGUMENT);
     }
 
@@ -142,7 +142,7 @@ static int s_clear_reusable_state(struct aws_symmetric_cipher *cipher) {
     EVP_CIPHER_CTX_cleanup(openssl_cipher->encryptor_ctx);
     EVP_CIPHER_CTX_cleanup(openssl_cipher->decryptor_ctx);
     aws_byte_buf_secure_zero(&openssl_cipher->working_buffer);
-    cipher->state = AWS_CIPHER_READY;
+    cipher->state = AWS_SYMMETRIC_CIPHER_READY;
     return AWS_OP_SUCCESS;
 }
 
@@ -227,7 +227,7 @@ struct aws_symmetric_cipher *aws_aes_cbc_256_new_impl(
         goto error;
     }
 
-    cipher->cipher_base.state = AWS_CIPHER_READY;
+    cipher->cipher_base.state = AWS_SYMMETRIC_CIPHER_READY;
     return &cipher->cipher_base;
 
 error:
@@ -318,7 +318,7 @@ struct aws_symmetric_cipher *aws_aes_ctr_256_new_impl(
         goto error;
     }
 
-    cipher->cipher_base.state = AWS_CIPHER_READY;
+    cipher->cipher_base.state = AWS_SYMMETRIC_CIPHER_READY;
     return &cipher->cipher_base;
 
 error:
@@ -338,7 +338,7 @@ static int s_finalize_gcm_encryption(struct aws_symmetric_cipher *cipher, struct
                     EVP_CTRL_GCM_GET_TAG,
                     (int)cipher->tag.capacity,
                     cipher->tag.buffer)) {
-                cipher->state = AWS_CIPHER_ERROR;
+                cipher->state = AWS_SYMMETRIC_CIPHER_ERROR;
                 return aws_raise_error(AWS_ERROR_INVALID_ARGUMENT);
             }
             cipher->tag.len = AWS_AES_256_CIPHER_BLOCK_SIZE;
@@ -478,7 +478,7 @@ struct aws_symmetric_cipher *aws_aes_gcm_256_new_impl(
         goto error;
     }
 
-    cipher->cipher_base.state = AWS_CIPHER_READY;
+    cipher->cipher_base.state = AWS_SYMMETRIC_CIPHER_READY;
     return &cipher->cipher_base;
 
 error:
@@ -504,7 +504,7 @@ static int s_key_wrap_finalize_encryption(struct aws_symmetric_cipher *cipher, s
     struct openssl_aes_cipher *openssl_cipher = cipher->impl;
 
     if (openssl_cipher->working_buffer.len < MIN_CEK_LENGTH_BYTES) {
-        cipher->state = AWS_CIPHER_ERROR;
+        cipher->state = AWS_SYMMETRIC_CIPHER_ERROR;
         return aws_raise_error(AWS_ERROR_INVALID_STATE);
     }
 
@@ -547,7 +547,7 @@ static int s_key_wrap_finalize_encryption(struct aws_symmetric_cipher *cipher, s
             /* encrypt the concatenated A and R[I] and store it in B */
             if (!EVP_EncryptUpdate(
                     openssl_cipher->encryptor_ctx, b.buffer, &b_out_len, temp_input.buffer, (int)temp_input.capacity)) {
-                cipher->state = AWS_CIPHER_ERROR;
+                cipher->state = AWS_SYMMETRIC_CIPHER_ERROR;
                 return aws_raise_error(AWS_ERROR_INVALID_ARGUMENT);
             }
 
@@ -572,7 +572,7 @@ static int s_key_wrap_finalize_decryption(struct aws_symmetric_cipher *cipher, s
     struct openssl_aes_cipher *openssl_cipher = cipher->impl;
 
     if (openssl_cipher->working_buffer.len < MIN_CEK_LENGTH_BYTES + KEYWRAP_BLOCK_SIZE) {
-        cipher->state = AWS_CIPHER_ERROR;
+        cipher->state = AWS_SYMMETRIC_CIPHER_ERROR;
         return aws_raise_error(AWS_ERROR_INVALID_STATE);
     }
 
@@ -619,7 +619,7 @@ static int s_key_wrap_finalize_decryption(struct aws_symmetric_cipher *cipher, s
             /* Decrypt the concatenated buffer */
             if (!EVP_DecryptUpdate(
                     openssl_cipher->decryptor_ctx, b.buffer, &b_out_len, temp_input.buffer, (int)temp_input.capacity)) {
-                cipher->state = AWS_CIPHER_ERROR;
+                cipher->state = AWS_SYMMETRIC_CIPHER_ERROR;
                 return aws_raise_error(AWS_ERROR_INVALID_ARGUMENT);
             }
 
@@ -637,7 +637,7 @@ static int s_key_wrap_finalize_decryption(struct aws_symmetric_cipher *cipher, s
     /* here we perform the integrity check to make sure A == 0xA6A6A6A6A6A6A6A6 */
     for (size_t i = 0; i < KEYWRAP_BLOCK_SIZE; ++i) {
         if (a[i] != INTEGRITY_VALUE) {
-            cipher->state = AWS_CIPHER_ERROR;
+            cipher->state = AWS_SYMMETRIC_CIPHER_ERROR;
             return aws_raise_error(AWS_ERROR_CAL_SIGNATURE_VALIDATION_FAILED);
         }
     }
@@ -653,7 +653,7 @@ static int s_init_keywrap_cipher_materials(struct aws_symmetric_cipher *cipher)
           EVP_CIPHER_CTX_set_padding(openssl_cipher->encryptor_ctx, 0)) ||
         !(EVP_DecryptInit_ex(openssl_cipher->decryptor_ctx, EVP_aes_256_ecb(), NULL, cipher->key.buffer, NULL) &&
           EVP_CIPHER_CTX_set_padding(openssl_cipher->decryptor_ctx, 0))) {
-        cipher->state = AWS_CIPHER_ERROR;
+        cipher->state = AWS_SYMMETRIC_CIPHER_ERROR;
         return aws_raise_error(AWS_ERROR_INVALID_ARGUMENT);
     }
 
@@ -713,7 +713,7 @@ struct aws_symmetric_cipher *aws_aes_keywrap_256_new_impl(
         goto error;
     }
 
-    cipher->cipher_base.state = AWS_CIPHER_READY;
+    cipher->cipher_base.state = AWS_SYMMETRIC_CIPHER_READY;
     return &cipher->cipher_base;
 
 error: