Proper state management for AES GCM and CCM (microsoft#112)

mamckee · web-flow · commit c4aac63a63e4 · 2025-03-26T18:33:28.000-07:00
* Only reset IV for new IV length. Track iv and tag state in ccm

* Update tagSet after ccm encrypt

* Free rsa private exponent in import

* Secure malloc and free primes and private exponent
diff --git a/ScosslCommon/inc/scossl_aes_aead.h b/ScosslCommon/inc/scossl_aes_aead.h
@@ -88,10 +88,12 @@ typedef struct
     SCOSSL_CCM_STAGE ccmStage;
     BYTE iv[SCOSSL_CCM_MAX_IV_LENGTH];
     SIZE_T ivlen;
+    INT32 ivSet;
     SYMCRYPT_CCM_STATE state;
     SYMCRYPT_AES_EXPANDED_KEY key;
     BYTE tag[EVP_CCM_TLS_TAG_LEN];
     SIZE_T taglen;
+    INT32 tagSet;
     UINT64 cbData;
     BYTE tlsAad[EVP_AEAD_TLS1_AAD_LEN];
     INT32 tlsAadSet;
diff --git a/ScosslCommon/src/scossl_aes_aead.c b/ScosslCommon/src/scossl_aes_aead.c
@@ -287,12 +287,15 @@ SCOSSL_STATUS scossl_aes_gcm_set_iv_len(SCOSSL_CIPHER_GCM_CTX *ctx, size_t ivlen
         return SCOSSL_FAILURE;
     }
 
-    ctx->ivlen = ivlen;
-
-    if (ctx->iv != NULL)
+    if (ivlen != ctx->ivlen)
     {
-        OPENSSL_free(ctx->iv);
-        ctx->iv = NULL;
+        ctx->ivlen = ivlen;
+    
+        if (ctx->iv != NULL)
+        {
+            OPENSSL_free(ctx->iv);
+            ctx->iv = NULL;
+        }
     }
 
     return SCOSSL_SUCCESS;
@@ -429,6 +432,8 @@ void scossl_aes_ccm_init_ctx(SCOSSL_CIPHER_CCM_CTX *ctx,
         memcpy(ctx->iv, iv, ctx->ivlen);
     }
     ctx->taglen = SCOSSL_CCM_MAX_TAG_LENGTH;
+    ctx->ivSet = 0;
+    ctx->tagSet = 0;
     ctx->tlsAadSet = 0;
 }
 
@@ -450,6 +455,7 @@ SCOSSL_STATUS scossl_aes_ccm_init_key(SCOSSL_CIPHER_CCM_CTX *ctx,
 
         ctx->ivlen = ivlen;
         memcpy(ctx->iv, iv, ctx->ivlen);
+        ctx->ivSet = 1;
     }
     if (key)
     {
@@ -618,6 +624,13 @@ SCOSSL_STATUS scossl_aes_ccm_cipher(SCOSSL_CIPHER_CCM_CTX *ctx, INT32 encrypt,
 
     if (ctx->ccmStage == SCOSSL_CCM_STAGE_SET_CBDATA)
     {
+        if (!ctx->ivSet)
+        {
+            SCOSSL_LOG_ERROR(SCOSSL_ERR_F_AES_CCM_CIPHER, ERR_R_PASSED_INVALID_ARGUMENT,
+                "No IV provided to CCM");
+            return SCOSSL_FAILURE;
+        }
+
         if (out == NULL)
         {
             // Auth Data Passed in
@@ -653,10 +666,17 @@ SCOSSL_STATUS scossl_aes_ccm_cipher(SCOSSL_CIPHER_CCM_CTX *ctx, INT32 encrypt,
                 SymCryptCcmEncryptPart(&ctx->state, in, out, inl);
             }
             SymCryptCcmEncryptFinal(&ctx->state, ctx->tag, ctx->taglen);
+            ctx->tagSet = 1;
             ctx->ccmStage = SCOSSL_CCM_STAGE_COMPLETE;
         }
         else
         {
+            if (!ctx->tagSet)
+            {
+                SCOSSL_LOG_ERROR(SCOSSL_ERR_F_AES_CCM_CIPHER, ERR_R_PASSED_INVALID_ARGUMENT,
+                    "No tag provided to CCM Decrypt");
+                return SCOSSL_FAILURE;
+            }
             // Decryption
             if (in != NULL)
             {
@@ -680,7 +700,7 @@ SCOSSL_STATUS scossl_aes_ccm_get_aead_tag(SCOSSL_CIPHER_CCM_CTX *ctx, INT32 encr
                                           unsigned char *tag, size_t taglen)
 {
     if ((taglen & 1) || taglen < SCOSSL_CCM_MIN_TAG_LENGTH || taglen > SCOSSL_CCM_MAX_TAG_LENGTH ||
-        taglen > ctx->taglen || !encrypt)
+        taglen > ctx->taglen || !encrypt || !ctx->tagSet)
     {
         return SCOSSL_FAILURE;
     }
@@ -702,6 +722,7 @@ SCOSSL_STATUS scossl_aes_ccm_set_aead_tag(SCOSSL_CIPHER_CCM_CTX *ctx, INT32 encr
         memcpy(ctx->tag, tag, taglen);
     }
     ctx->taglen = taglen;
+    ctx->tagSet = 1;
 
     return SCOSSL_SUCCESS;
 }
@@ -717,7 +738,12 @@ SCOSSL_STATUS scossl_aes_ccm_set_iv_len(SCOSSL_CIPHER_CCM_CTX *ctx, size_t ivlen
         return SCOSSL_FAILURE;
     }
 
-    ctx->ivlen = ivlen;
+    if (ctx->ivlen != ivlen)
+    {
+        ctx->ivlen = ivlen;
+        ctx->ivSet = 0;
+    }
+
     return SCOSSL_SUCCESS;
 }
 
@@ -751,6 +777,8 @@ SCOSSL_STATUS scossl_aes_ccm_set_iv_fixed(SCOSSL_CIPHER_CCM_CTX *ctx, INT32 encr
         return SCOSSL_FAILURE;
     }
 
+    ctx->ivSet = 1;
+
     return SCOSSL_SUCCESS;
 }
 
diff --git a/SymCryptProvider/src/keymgmt/p_scossl_rsa_keymgmt.c b/SymCryptProvider/src/keymgmt/p_scossl_rsa_keymgmt.c
@@ -1064,8 +1064,8 @@ static BOOL p_scossl_rsa_keymgmt_match(_In_ SCOSSL_PROV_RSA_KEY_CTX *keyCtx1, _I
 cleanup:
     OPENSSL_free(pbModulus1);
     OPENSSL_free(pbModulus2);
-    OPENSSL_secure_free(pbPrivateExponent1);
-    OPENSSL_secure_free(pbPrivateExponent2);
+    OPENSSL_secure_clear_free(pbPrivateExponent1, cbModulus);
+    OPENSSL_secure_clear_free(pbPrivateExponent2, cbModulus);
 
     return ret;
 }
@@ -1114,7 +1114,7 @@ static SCOSSL_STATUS p_scossl_rsa_keymgmt_import(_Inout_ SCOSSL_PROV_RSA_KEY_CTX
         {
             cbModulus = p->data_size;
 
-            pbModulus = OPENSSL_zalloc(cbModulus);
+            pbModulus = OPENSSL_malloc(cbModulus);
             if (pbModulus == NULL)
             {
                 ERR_raise(ERR_LIB_PROV, ERR_R_MALLOC_FAILURE);
@@ -1141,7 +1141,7 @@ static SCOSSL_STATUS p_scossl_rsa_keymgmt_import(_Inout_ SCOSSL_PROV_RSA_KEY_CTX
             {
                 pcbPrimes[0] = p->data_size;
 
-                ppbPrimes[0] = OPENSSL_zalloc(pcbPrimes[0]);
+                ppbPrimes[0] = OPENSSL_secure_malloc(pcbPrimes[0]);
                 if (ppbPrimes[0] == NULL)
                 {
                     ERR_raise(ERR_LIB_PROV, ERR_R_MALLOC_FAILURE);
@@ -1160,7 +1160,7 @@ static SCOSSL_STATUS p_scossl_rsa_keymgmt_import(_Inout_ SCOSSL_PROV_RSA_KEY_CTX
             {
                 pcbPrimes[1] = p->data_size;
 
-                ppbPrimes[1] = OPENSSL_zalloc(pcbPrimes[1]);
+                ppbPrimes[1] = OPENSSL_secure_malloc(pcbPrimes[1]);
                 if(ppbPrimes[1] == NULL)
                 {
                     ERR_raise(ERR_LIB_PROV, ERR_R_MALLOC_FAILURE);
@@ -1182,7 +1182,7 @@ static SCOSSL_STATUS p_scossl_rsa_keymgmt_import(_Inout_ SCOSSL_PROV_RSA_KEY_CTX
             {
                 cbPrivateExponent = p->data_size;
 
-                pbPrivateExponent = OPENSSL_zalloc(cbPrivateExponent);
+                pbPrivateExponent = OPENSSL_secure_malloc(cbPrivateExponent);
                 if(pbPrivateExponent == NULL)
                 {
                     ERR_raise(ERR_LIB_PROV, ERR_R_MALLOC_FAILURE);
@@ -1271,9 +1271,10 @@ static SCOSSL_STATUS p_scossl_rsa_keymgmt_import(_Inout_ SCOSSL_PROV_RSA_KEY_CTX
     ret = SCOSSL_SUCCESS;
 
 cleanup:
+    OPENSSL_secure_clear_free(pbPrivateExponent, cbModulus);
+    OPENSSL_secure_clear_free(ppbPrimes[0], pcbPrimes[0]);
+    OPENSSL_secure_clear_free(ppbPrimes[1], pcbPrimes[1]);
     OPENSSL_free(pbModulus);
-    OPENSSL_free(ppbPrimes[0]);
-    OPENSSL_free(ppbPrimes[1]);
     BN_free(bn);
 
     return ret;

Original file line number	Diff line number	Diff line change
`@@ -287,12 +287,15 @@ SCOSSL_STATUS scossl_aes_gcm_set_iv_len(SCOSSL_CIPHER_GCM_CTX *ctx, size_t ivlen`
`287`	`287`	`return SCOSSL_FAILURE;`
`288`	`288`	`}`
`289`	`289`
`290`		`- ctx->ivlen = ivlen;`
`291`		`-`
`292`		`- if (ctx->iv != NULL)`
	`290`	`+ if (ivlen != ctx->ivlen)`
`293`	`291`	`{`
`294`		`- OPENSSL_free(ctx->iv);`
`295`		`- ctx->iv = NULL;`
	`292`	`+ ctx->ivlen = ivlen;`
	`293`	`+`
	`294`	`+ if (ctx->iv != NULL)`
	`295`	`+ {`
	`296`	`+ OPENSSL_free(ctx->iv);`
	`297`	`+ ctx->iv = NULL;`
	`298`	`+ }`
`296`	`299`	`}`
`297`	`300`
`298`	`301`	`return SCOSSL_SUCCESS;`
`@@ -429,6 +432,8 @@ void scossl_aes_ccm_init_ctx(SCOSSL_CIPHER_CCM_CTX *ctx,`
`429`	`432`	`memcpy(ctx->iv, iv, ctx->ivlen);`
`430`	`433`	`}`
`431`	`434`	`ctx->taglen = SCOSSL_CCM_MAX_TAG_LENGTH;`
	`435`	`+ ctx->ivSet = 0;`
	`436`	`+ ctx->tagSet = 0;`
`432`	`437`	`ctx->tlsAadSet = 0;`
`433`	`438`	`}`
`434`	`439`
`@@ -450,6 +455,7 @@ SCOSSL_STATUS scossl_aes_ccm_init_key(SCOSSL_CIPHER_CCM_CTX *ctx,`
`450`	`455`
`451`	`456`	`ctx->ivlen = ivlen;`
`452`	`457`	`memcpy(ctx->iv, iv, ctx->ivlen);`
	`458`	`+ ctx->ivSet = 1;`
`453`	`459`	`}`
`454`	`460`	`if (key)`
`455`	`461`	`{`
`@@ -618,6 +624,13 @@ SCOSSL_STATUS scossl_aes_ccm_cipher(SCOSSL_CIPHER_CCM_CTX *ctx, INT32 encrypt,`
`618`	`624`
`619`	`625`	`if (ctx->ccmStage == SCOSSL_CCM_STAGE_SET_CBDATA)`
`620`	`626`	`{`
	`627`	`+ if (!ctx->ivSet)`
	`628`	`+ {`
	`629`	`+ SCOSSL_LOG_ERROR(SCOSSL_ERR_F_AES_CCM_CIPHER, ERR_R_PASSED_INVALID_ARGUMENT,`
	`630`	`+ "No IV provided to CCM");`
	`631`	`+ return SCOSSL_FAILURE;`
	`632`	`+ }`
	`633`	`+`
`621`	`634`	`if (out == NULL)`
`622`	`635`	`{`
`623`	`636`	`// Auth Data Passed in`
`@@ -653,10 +666,17 @@ SCOSSL_STATUS scossl_aes_ccm_cipher(SCOSSL_CIPHER_CCM_CTX *ctx, INT32 encrypt,`
`653`	`666`	`SymCryptCcmEncryptPart(&ctx->state, in, out, inl);`
`654`	`667`	`}`
`655`	`668`	`SymCryptCcmEncryptFinal(&ctx->state, ctx->tag, ctx->taglen);`
	`669`	`+ ctx->tagSet = 1;`
`656`	`670`	`ctx->ccmStage = SCOSSL_CCM_STAGE_COMPLETE;`
`657`	`671`	`}`
`658`	`672`	`else`
`659`	`673`	`{`
	`674`	`+ if (!ctx->tagSet)`
	`675`	`+ {`
	`676`	`+ SCOSSL_LOG_ERROR(SCOSSL_ERR_F_AES_CCM_CIPHER, ERR_R_PASSED_INVALID_ARGUMENT,`
	`677`	`+ "No tag provided to CCM Decrypt");`
	`678`	`+ return SCOSSL_FAILURE;`
	`679`	`+ }`
`660`	`680`	`// Decryption`
`661`	`681`	`if (in != NULL)`
`662`	`682`	`{`
`@@ -680,7 +700,7 @@ SCOSSL_STATUS scossl_aes_ccm_get_aead_tag(SCOSSL_CIPHER_CCM_CTX *ctx, INT32 encr`
`680`	`700`	`unsigned char *tag, size_t taglen)`
`681`	`701`	`{`
`682`	`702`	`if ((taglen & 1) \|\| taglen < SCOSSL_CCM_MIN_TAG_LENGTH \|\| taglen > SCOSSL_CCM_MAX_TAG_LENGTH \|\|`
`683`		`- taglen > ctx->taglen \|\| !encrypt)`
	`703`	`+ taglen > ctx->taglen \|\| !encrypt \|\| !ctx->tagSet)`
`684`	`704`	`{`
`685`	`705`	`return SCOSSL_FAILURE;`
`686`	`706`	`}`
`@@ -702,6 +722,7 @@ SCOSSL_STATUS scossl_aes_ccm_set_aead_tag(SCOSSL_CIPHER_CCM_CTX *ctx, INT32 encr`
`702`	`722`	`memcpy(ctx->tag, tag, taglen);`
`703`	`723`	`}`
`704`	`724`	`ctx->taglen = taglen;`
	`725`	`+ ctx->tagSet = 1;`
`705`	`726`
`706`	`727`	`return SCOSSL_SUCCESS;`
`707`	`728`	`}`
`@@ -717,7 +738,12 @@ SCOSSL_STATUS scossl_aes_ccm_set_iv_len(SCOSSL_CIPHER_CCM_CTX *ctx, size_t ivlen`
`717`	`738`	`return SCOSSL_FAILURE;`
`718`	`739`	`}`
`719`	`740`
`720`		`- ctx->ivlen = ivlen;`
	`741`	`+ if (ctx->ivlen != ivlen)`
	`742`	`+ {`
	`743`	`+ ctx->ivlen = ivlen;`
	`744`	`+ ctx->ivSet = 0;`
	`745`	`+ }`
	`746`	`+`
`721`	`747`	`return SCOSSL_SUCCESS;`
`722`	`748`	`}`
`723`	`749`
`@@ -751,6 +777,8 @@ SCOSSL_STATUS scossl_aes_ccm_set_iv_fixed(SCOSSL_CIPHER_CCM_CTX *ctx, INT32 encr`
`751`	`777`	`return SCOSSL_FAILURE;`
`752`	`778`	`}`
`753`	`779`
	`780`	`+ ctx->ivSet = 1;`
	`781`	`+`
`754`	`782`	`return SCOSSL_SUCCESS;`
`755`	`783`	`}`
`756`	`784`
Original file line number	Diff line number	Diff line change
`@@ -1064,8 +1064,8 @@ static BOOL p_scossl_rsa_keymgmt_match(_In_ SCOSSL_PROV_RSA_KEY_CTX *keyCtx1, _I`
`1064`	`1064`	`cleanup:`
`1065`	`1065`	`OPENSSL_free(pbModulus1);`
`1066`	`1066`	`OPENSSL_free(pbModulus2);`
`1067`		`- OPENSSL_secure_free(pbPrivateExponent1);`
`1068`		`- OPENSSL_secure_free(pbPrivateExponent2);`
	`1067`	`+ OPENSSL_secure_clear_free(pbPrivateExponent1, cbModulus);`
	`1068`	`+ OPENSSL_secure_clear_free(pbPrivateExponent2, cbModulus);`
`1069`	`1069`
`1070`	`1070`	`return ret;`
`1071`	`1071`	`}`
`@@ -1114,7 +1114,7 @@ static SCOSSL_STATUS p_scossl_rsa_keymgmt_import(_Inout_ SCOSSL_PROV_RSA_KEY_CTX`
`1114`	`1114`	`{`
`1115`	`1115`	`cbModulus = p->data_size;`
`1116`	`1116`
`1117`		`- pbModulus = OPENSSL_zalloc(cbModulus);`
	`1117`	`+ pbModulus = OPENSSL_malloc(cbModulus);`
`1118`	`1118`	`if (pbModulus == NULL)`
`1119`	`1119`	`{`
`1120`	`1120`	`ERR_raise(ERR_LIB_PROV, ERR_R_MALLOC_FAILURE);`
`@@ -1141,7 +1141,7 @@ static SCOSSL_STATUS p_scossl_rsa_keymgmt_import(_Inout_ SCOSSL_PROV_RSA_KEY_CTX`
`1141`	`1141`	`{`
`1142`	`1142`	`pcbPrimes[0] = p->data_size;`
`1143`	`1143`
`1144`		`- ppbPrimes[0] = OPENSSL_zalloc(pcbPrimes[0]);`
	`1144`	`+ ppbPrimes[0] = OPENSSL_secure_malloc(pcbPrimes[0]);`
`1145`	`1145`	`if (ppbPrimes[0] == NULL)`
`1146`	`1146`	`{`
`1147`	`1147`	`ERR_raise(ERR_LIB_PROV, ERR_R_MALLOC_FAILURE);`
`@@ -1160,7 +1160,7 @@ static SCOSSL_STATUS p_scossl_rsa_keymgmt_import(_Inout_ SCOSSL_PROV_RSA_KEY_CTX`
`1160`	`1160`	`{`
`1161`	`1161`	`pcbPrimes[1] = p->data_size;`
`1162`	`1162`
`1163`		`- ppbPrimes[1] = OPENSSL_zalloc(pcbPrimes[1]);`
	`1163`	`+ ppbPrimes[1] = OPENSSL_secure_malloc(pcbPrimes[1]);`
`1164`	`1164`	`if(ppbPrimes[1] == NULL)`
`1165`	`1165`	`{`
`1166`	`1166`	`ERR_raise(ERR_LIB_PROV, ERR_R_MALLOC_FAILURE);`
`@@ -1182,7 +1182,7 @@ static SCOSSL_STATUS p_scossl_rsa_keymgmt_import(_Inout_ SCOSSL_PROV_RSA_KEY_CTX`
`1182`	`1182`	`{`
`1183`	`1183`	`cbPrivateExponent = p->data_size;`
`1184`	`1184`
`1185`		`- pbPrivateExponent = OPENSSL_zalloc(cbPrivateExponent);`
	`1185`	`+ pbPrivateExponent = OPENSSL_secure_malloc(cbPrivateExponent);`
`1186`	`1186`	`if(pbPrivateExponent == NULL)`
`1187`	`1187`	`{`
`1188`	`1188`	`ERR_raise(ERR_LIB_PROV, ERR_R_MALLOC_FAILURE);`
`@@ -1271,9 +1271,10 @@ static SCOSSL_STATUS p_scossl_rsa_keymgmt_import(_Inout_ SCOSSL_PROV_RSA_KEY_CTX`
`1271`	`1271`	`ret = SCOSSL_SUCCESS;`
`1272`	`1272`
`1273`	`1273`	`cleanup:`
	`1274`	`+ OPENSSL_secure_clear_free(pbPrivateExponent, cbModulus);`
	`1275`	`+ OPENSSL_secure_clear_free(ppbPrimes[0], pcbPrimes[0]);`
	`1276`	`+ OPENSSL_secure_clear_free(ppbPrimes[1], pcbPrimes[1]);`
`1274`	`1277`	`OPENSSL_free(pbModulus);`
`1275`		`- OPENSSL_free(ppbPrimes[0]);`
`1276`		`- OPENSSL_free(ppbPrimes[1]);`
`1277`	`1278`	`BN_free(bn);`
`1278`	`1279`
`1279`	`1280`	`return ret;`