From d5ff89d23b1588f5e0b344e7a749f40d5018ce08 Mon Sep 17 00:00:00 2001
From: Sean McGrail <mcgrails@amazon.com>
Date: Wed, 16 Oct 2024 23:21:57 +0000
Subject: [PATCH] 800-131Ar1: length of the key-derivation key shall be at
 least 112 bits.

---
 crypto/fipsmodule/kdf/kbkdf.c                    |  2 +-
 crypto/fipsmodule/service_indicator/internal.h   |  4 ++--
 .../service_indicator/service_indicator.c        |  7 +++++--
 .../service_indicator/service_indicator_test.cc  | 16 ++++++++++++++--
 4 files changed, 22 insertions(+), 7 deletions(-)

diff --git a/crypto/fipsmodule/kdf/kbkdf.c b/crypto/fipsmodule/kdf/kbkdf.c
index d33d6ffae2..8d074d2b70 100644
--- a/crypto/fipsmodule/kdf/kbkdf.c
+++ b/crypto/fipsmodule/kdf/kbkdf.c
@@ -104,7 +104,7 @@ int KBKDF_ctr_hmac(uint8_t *out_key, size_t out_len, const EVP_MD *digest,
   HMAC_CTX_free(hmac_ctx);
   FIPS_service_indicator_unlock_state();
   if (ret) {
-    KBKDF_ctr_hmac_verify_service_indicator(digest);
+    KBKDF_ctr_hmac_verify_service_indicator(digest, secret_len);
   }
   return ret;
 }
diff --git a/crypto/fipsmodule/service_indicator/internal.h b/crypto/fipsmodule/service_indicator/internal.h
index 0f8881132b..58fc1dd0b0 100644
--- a/crypto/fipsmodule/service_indicator/internal.h
+++ b/crypto/fipsmodule/service_indicator/internal.h
@@ -55,7 +55,7 @@ void TLSKDF_verify_service_indicator(const EVP_MD *dgst, const char *label,
                                      size_t label_len);
 void SSKDF_digest_verify_service_indicator(const EVP_MD *dgst);
 void SSKDF_hmac_verify_service_indicator(const EVP_MD *dgst);
-void KBKDF_ctr_hmac_verify_service_indicator(const EVP_MD *dgst);
+void KBKDF_ctr_hmac_verify_service_indicator(const EVP_MD *dgst, size_t secret_len);
 void EVP_PKEY_encapsulate_verify_service_indicator(const EVP_PKEY_CTX* ctx);
 void EVP_PKEY_decapsulate_verify_service_indicator(const EVP_PKEY_CTX* ctx);
 
@@ -127,7 +127,7 @@ OPENSSL_INLINE void SSKDF_digest_verify_service_indicator(
 OPENSSL_INLINE void SSKDF_hmac_verify_service_indicator(
     OPENSSL_UNUSED const EVP_MD *dgst) {}
 
-OPENSSL_INLINE void KBKDF_ctr_hmac_verify_service_indicator(OPENSSL_UNUSED const EVP_MD *dgst) {}
+OPENSSL_INLINE void KBKDF_ctr_hmac_verify_service_indicator(OPENSSL_UNUSED const EVP_MD *dgst, size_t secret_len) {}
 
 OPENSSL_INLINE void EVP_PKEY_encapsulate_verify_service_indicator(OPENSSL_UNUSED const EVP_PKEY_CTX* ctx) {}
 
diff --git a/crypto/fipsmodule/service_indicator/service_indicator.c b/crypto/fipsmodule/service_indicator/service_indicator.c
index d034134f9f..94a85ea21e 100644
--- a/crypto/fipsmodule/service_indicator/service_indicator.c
+++ b/crypto/fipsmodule/service_indicator/service_indicator.c
@@ -609,7 +609,7 @@ void SSKDF_hmac_verify_service_indicator(const EVP_MD *dgst) {
 //
 // Sourced from NIST SP 800-108r1-upd1 Section 3:  Pseudorandom Function (PRF)
 // https://doi.org/10.6028/NIST.SP.800-108r1-upd1
-void KBKDF_ctr_hmac_verify_service_indicator(const EVP_MD *dgst) {
+void KBKDF_ctr_hmac_verify_service_indicator(const EVP_MD *dgst, size_t secret_len) {
   switch (dgst->type) {
     case NID_sha1:
     case NID_sha224:
@@ -618,7 +618,10 @@ void KBKDF_ctr_hmac_verify_service_indicator(const EVP_MD *dgst) {
     case NID_sha512:
     case NID_sha512_224:
     case NID_sha512_256:
-      FIPS_service_indicator_update_state();
+      // SP 800-131Ar1, Section 8: "The length of the key-derivation key shall be at least 112 bits.” 
+      if (secret_len >= 14) {
+        FIPS_service_indicator_update_state();
+      }
       break;
     default:
       break;
diff --git a/crypto/fipsmodule/service_indicator/service_indicator_test.cc b/crypto/fipsmodule/service_indicator/service_indicator_test.cc
index e0a77e3ec4..cff1a08776 100644
--- a/crypto/fipsmodule/service_indicator/service_indicator_test.cc
+++ b/crypto/fipsmodule/service_indicator/service_indicator_test.cc
@@ -4785,8 +4785,10 @@ INSTANTIATE_TEST_SUITE_P(All, KBKDFCtrHmacIndicatorTest,
 TEST_P(KBKDFCtrHmacIndicatorTest, KBKDF) {
   const KBKDFCtrHmacTestVector &vector = GetParam();
 
-  const uint8_t secret[20] = {'A', 'W', 'S', '-', 'L', 'C', ' ', 'K', 'B', 'K',
-                              'D', 'F', '-', 'C', 'T', 'R', ' ', 'K', 'E', 'Y'};
+  // 14 bytes (112 bits) is the minimum key-derivation key that would be
+  // approved if using an approved algorithm;
+  const uint8_t secret[14] = {'K', 'B', 'K', 'D', 'F', '-', 'C',
+                              'T', 'R', ' ', ' ', 'K', 'E', 'Y'};
   const uint8_t info[16] = {'A', 'W', 'S', '-', 'L', 'C', ' ', 'K',
                             'B', 'K', 'D', 'F', '-', 'C', 'T', 'R'};
   uint8_t output[16] = {0};
@@ -4798,6 +4800,16 @@ TEST_P(KBKDFCtrHmacIndicatorTest, KBKDF) {
                     &output[0], sizeof(output), vector.md(), &secret[0],
                     sizeof(secret), &info[0], sizeof(info))));
   ASSERT_EQ(vector.expectation, approved);
+
+  // Pass a secret key (key-derivation key) size of 13 bytes (104 bits)
+  // regardless of algorithm, this is less then 112 bits minimum
+  // required by SP 800-131Ar1, Section 8. So indicator should reflect
+  // this being unapproved.
+  CALL_SERVICE_AND_CHECK_APPROVED(
+      approved,
+      ASSERT_TRUE(KBKDF_ctr_hmac(&output[0], sizeof(output), vector.md(),
+                                 &secret[0], 13, &info[0], sizeof(info))));
+  ASSERT_EQ(AWSLC_NOT_APPROVED, approved);
 }
 
 TEST(ServiceIndicatorTest, ML_KEM) {