From 1900abea1cfe0f02ea272e59aa497f20d59d59f3 Mon Sep 17 00:00:00 2001 From: Andrew Hopkins Date: Mon, 13 Jan 2025 10:52:36 -0800 Subject: [PATCH 1/2] Ensure service indicator is incremented only once, update RSA and ED25519 to ensure the state is locked --- crypto/curve25519_extra/curve25519_extra.c | 2 +- crypto/fipsmodule/curve25519/curve25519.c | 2 ++ crypto/fipsmodule/rsa/rsa_impl.c | 2 ++ include/openssl/service_indicator.h | 2 +- 4 files changed, 6 insertions(+), 2 deletions(-) diff --git a/crypto/curve25519_extra/curve25519_extra.c b/crypto/curve25519_extra/curve25519_extra.c index c7c1f7a2cc..4ec619e5e9 100644 --- a/crypto/curve25519_extra/curve25519_extra.c +++ b/crypto/curve25519_extra/curve25519_extra.c @@ -80,9 +80,9 @@ int ED25519ph_sign_digest(uint8_t out_sig[ED25519_SIGNATURE_LEN], const uint8_t *context, size_t context_len) { FIPS_service_indicator_lock_state(); boringssl_ensure_hasheddsa_self_test(); - FIPS_service_indicator_unlock_state(); int res = ED25519ph_sign_digest_no_self_test(out_sig, digest, private_key, context, context_len); + FIPS_service_indicator_unlock_state(); if (res) { FIPS_service_indicator_update_state(); } diff --git a/crypto/fipsmodule/curve25519/curve25519.c b/crypto/fipsmodule/curve25519/curve25519.c index de468abc17..3b428cea9d 100644 --- a/crypto/fipsmodule/curve25519/curve25519.c +++ b/crypto/fipsmodule/curve25519/curve25519.c @@ -126,6 +126,7 @@ static void ed25519_keypair_pct(uint8_t public_key[ED25519_PUBLIC_KEY_LEN], void ED25519_keypair(uint8_t out_public_key[ED25519_PUBLIC_KEY_LEN], uint8_t out_private_key[ED25519_PRIVATE_KEY_LEN]) { + FIPS_service_indicator_lock_state(); boringssl_ensure_eddsa_self_test(); SET_DIT_AUTO_RESET; @@ -141,6 +142,7 @@ void ED25519_keypair(uint8_t out_public_key[ED25519_PUBLIC_KEY_LEN], ed25519_keypair_pct(out_public_key, out_private_key); + FIPS_service_indicator_unlock_state(); FIPS_service_indicator_update_state(); } diff --git a/crypto/fipsmodule/rsa/rsa_impl.c b/crypto/fipsmodule/rsa/rsa_impl.c index d5d6660fed..f3e98fc4b8 100644 --- a/crypto/fipsmodule/rsa/rsa_impl.c +++ b/crypto/fipsmodule/rsa/rsa_impl.c @@ -1278,9 +1278,11 @@ int RSA_generate_key_fips(RSA *rsa, int bits, BN_GENCB *cb) { } BIGNUM *e = BN_new(); + FIPS_service_indicator_lock_state(); int ret = e != NULL && BN_set_word(e, RSA_F4) && RSA_generate_key_ex_maybe_fips(rsa, bits, e, cb, /*check_fips=*/1); + FIPS_service_indicator_unlock_state(); BN_free(e); if(ret) { // Approved key size check step is already done at start of function. diff --git a/include/openssl/service_indicator.h b/include/openssl/service_indicator.h index f1160eb8e1..39ab6ea19b 100644 --- a/include/openssl/service_indicator.h +++ b/include/openssl/service_indicator.h @@ -50,7 +50,7 @@ enum FIPSStatus { int before = FIPS_service_indicator_before_call(); \ func; \ int after = FIPS_service_indicator_after_call(); \ - if (before != after) { \ + if (before + 1 == after) { \ (approved) = AWSLC_APPROVED; \ } \ } \ From 80ecc1df980ad29f67c798f95696dbc5ed8f2965 Mon Sep 17 00:00:00 2001 From: Andrew Hopkins Date: Fri, 24 Jan 2025 13:51:32 -0800 Subject: [PATCH 2/2] Use a debug macro instead --- crypto/fipsmodule/curve25519/curve25519.c | 2 ++ include/openssl/service_indicator.h | 8 +++++++- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/crypto/fipsmodule/curve25519/curve25519.c b/crypto/fipsmodule/curve25519/curve25519.c index 3b428cea9d..16249bd1a2 100644 --- a/crypto/fipsmodule/curve25519/curve25519.c +++ b/crypto/fipsmodule/curve25519/curve25519.c @@ -126,6 +126,8 @@ static void ed25519_keypair_pct(uint8_t public_key[ED25519_PUBLIC_KEY_LEN], void ED25519_keypair(uint8_t out_public_key[ED25519_PUBLIC_KEY_LEN], uint8_t out_private_key[ED25519_PRIVATE_KEY_LEN]) { + // We have to avoid the self tests and digest function in ed25519_keypair_pct + // from updating the service indicator. FIPS_service_indicator_lock_state(); boringssl_ensure_eddsa_self_test(); SET_DIT_AUTO_RESET; diff --git a/include/openssl/service_indicator.h b/include/openssl/service_indicator.h index 39ab6ea19b..ba2f4c449d 100644 --- a/include/openssl/service_indicator.h +++ b/include/openssl/service_indicator.h @@ -44,13 +44,19 @@ enum FIPSStatus { // |AWSLC_NOT_APPROVED| accordingly to the approved state of the service ran. // It is highly recommended that users of the service indicator use this macro // when interacting with the service indicator. +// +// This macro tests before != after to handle potential uint64_t rollover in +// long-running applications that use the release build of AWS-LC. Debug builds +// use an assert before + 1 == after to ensure in testing the service indicator +// is operating as expected. #define CALL_SERVICE_AND_CHECK_APPROVED(approved, func) \ do { \ (approved) = AWSLC_NOT_APPROVED; \ int before = FIPS_service_indicator_before_call(); \ func; \ int after = FIPS_service_indicator_after_call(); \ - if (before + 1 == after) { \ + if (before != after) { \ + assert(before + 1 == after); \ (approved) = AWSLC_APPROVED; \ } \ } \