Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ensure service indicator is incremented only once, update RSA and ED25519 to ensure the state is locked #2112

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
+13 −1
Open

Ensure service indicator is incremented only once, update RSA and ED25519 to ensure the state is locked #2112

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
cc1fe6e
Ensure service indicator is incremented only once, update RSA and ED2…
andrewhop Jan 13, 2025
354fc6f
Use a debug macro instead
andrewhop Jan 24, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion crypto/curve25519_extra/curve25519_extra.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here I thought I got it right in all of the places :)

if (res) {
FIPS_service_indicator_update_state();
}
Expand Down
4 changes: 4 additions & 0 deletions crypto/fipsmodule/curve25519/curve25519.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,9 @@ 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();
andrewhop marked this conversation as resolved.
Show resolved Hide resolved
boringssl_ensure_eddsa_self_test();
SET_DIT_AUTO_RESET;

Expand All @@ -141,6 +144,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();
}

Expand Down
2 changes: 2 additions & 0 deletions crypto/fipsmodule/rsa/rsa_impl.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
6 changes: 6 additions & 0 deletions include/openssl/service_indicator.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 != after) { \
assert(before + 1 == after); \
(approved) = AWSLC_APPROVED; \
} \
} \
Expand Down
Loading