commit ec476ef0441f32fbcab558127412461617516336
author Adam Langley <alangley@gmail.com> Fri Jan 14 13:22:59 2022 -0800
committer Boringssl LUCI CQ <boringssl-scoped@luci-project-accounts.iam.gserviceaccount.com> Fri Jan 14 22:09:00 2022 +0000
tree 3ade35840011aed3a6bbefb04c775dedbd84b8d2
parent f7975703204c738144990efb8406eee0668fbf2e

Zero out the values from the integrity check.

140-3 says

> the zeroisation of protected and unprotected SSPs
> shall be performed in the following scenarios:
>   ...
>   For temporary value(s) generated during the integrity test of the
>   module’s software or firmware upon completion of the integrity test.

(IG 9.7.B)

Change-Id: I911f294860bf33b13b2c997fc633c9bda777fc48
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/50945
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>

crypto/fipsmodule/bcm.c

diff --git a/crypto/fipsmodule/bcm.c b/crypto/fipsmodule/bcm.c
index 3a1ad15..639235e 100644
--- a/crypto/fipsmodule/bcm.c
+++ b/crypto/fipsmodule/bcm.c
@@ -233,7 +233,7 @@
     fprintf(stderr, "HMAC failed.\n");
     goto err;
   }
-  HMAC_CTX_cleanup(&hmac_ctx);
+  HMAC_CTX_cleanse(&hmac_ctx); // FIPS 140-3, AS05.10.
 
   const uint8_t *expected = BORINGSSL_bcm_text_hash;
 
@@ -241,6 +241,8 @@
     goto err;
   }
 
+  OPENSSL_cleanse(result, sizeof(result)); // FIPS 140-3, AS05.10.
+
   if (!boringssl_fips_self_test(BORINGSSL_bcm_text_hash, sizeof(result))) {
     goto err;
   }

crypto/fipsmodule/digest/digest.c

diff --git a/crypto/fipsmodule/digest/digest.c b/crypto/fipsmodule/digest/digest.c
index 059d72c..cb723d6 100644
--- a/crypto/fipsmodule/digest/digest.c
+++ b/crypto/fipsmodule/digest/digest.c
@@ -106,6 +106,11 @@
   return 1;
 }
 
+void EVP_MD_CTX_cleanse(EVP_MD_CTX *ctx) {
+  OPENSSL_cleanse(ctx->md_data, ctx->digest->ctx_size);
+  EVP_MD_CTX_cleanup(ctx);
+}
+
 void EVP_MD_CTX_free(EVP_MD_CTX *ctx) {
   if (!ctx) {
     return;

crypto/fipsmodule/hmac/hmac.c

diff --git a/crypto/fipsmodule/hmac/hmac.c b/crypto/fipsmodule/hmac/hmac.c
index fb57bf2..d37cec8 100644
--- a/crypto/fipsmodule/hmac/hmac.c
+++ b/crypto/fipsmodule/hmac/hmac.c
@@ -102,6 +102,13 @@
   OPENSSL_cleanse(ctx, sizeof(HMAC_CTX));
 }
 
+void HMAC_CTX_cleanse(HMAC_CTX *ctx) {
+  EVP_MD_CTX_cleanse(&ctx->i_ctx);
+  EVP_MD_CTX_cleanse(&ctx->o_ctx);
+  EVP_MD_CTX_cleanse(&ctx->md_ctx);
+  OPENSSL_cleanse(ctx, sizeof(HMAC_CTX));
+}
+
 void HMAC_CTX_free(HMAC_CTX *ctx) {
   if (ctx == NULL) {
     return;

include/openssl/digest.h

diff --git a/include/openssl/digest.h b/include/openssl/digest.h
index fa76168..6e88999 100644
--- a/include/openssl/digest.h
+++ b/include/openssl/digest.h
@@ -117,6 +117,13 @@
 // freshly initialised state. It does not free |ctx| itself. It returns one.
 OPENSSL_EXPORT int EVP_MD_CTX_cleanup(EVP_MD_CTX *ctx);
 
+// EVP_MD_CTX_cleanse zeros the digest state in |ctx| and then performs the
+// actions of |EVP_MD_CTX_cleanup|. Note that some |EVP_MD_CTX| objects contain
+// more than just a digest (e.g. those resulting from |EVP_DigestSignInit|) but
+// this function does not zero out more than just the digest state even in that
+// case.
+OPENSSL_EXPORT void EVP_MD_CTX_cleanse(EVP_MD_CTX *ctx);
+
 // EVP_MD_CTX_free calls |EVP_MD_CTX_cleanup| and then frees |ctx| itself.
 OPENSSL_EXPORT void EVP_MD_CTX_free(EVP_MD_CTX *ctx);
 

include/openssl/hmac.h

diff --git a/include/openssl/hmac.h b/include/openssl/hmac.h
index b5d1e42..56b0802 100644
--- a/include/openssl/hmac.h
+++ b/include/openssl/hmac.h
@@ -98,6 +98,10 @@
 // HMAC_CTX_cleanup frees data owned by |ctx|. It does not free |ctx| itself.
 OPENSSL_EXPORT void HMAC_CTX_cleanup(HMAC_CTX *ctx);
 
+// HMAC_CTX_cleanse zeros the digest state from |ctx| and then performs the
+// actions of |HMAC_CTX_cleanup|.
+OPENSSL_EXPORT void HMAC_CTX_cleanse(HMAC_CTX *ctx);
+
 // HMAC_CTX_free calls |HMAC_CTX_cleanup| and then frees |ctx| itself.
 OPENSSL_EXPORT void HMAC_CTX_free(HMAC_CTX *ctx);