diff --git a/crypto/fipsmodule/aes/aes_test.cc b/crypto/fipsmodule/aes/aes_test.cc index a38bf9c..dd3b374 100644 --- a/crypto/fipsmodule/aes/aes_test.cc +++ b/crypto/fipsmodule/aes/aes_test.cc
@@ -289,7 +289,7 @@ } if (bsaes_capable()) { - vpaes_set_encrypt_key(kKey, bits, &key); + ASSERT_EQ(vpaes_set_encrypt_key(kKey, bits, &key), 0); CHECK_ABI(vpaes_encrypt_key_to_bsaes, &key, &key); for (size_t blocks : block_counts) { SCOPED_TRACE(blocks); @@ -298,7 +298,7 @@ } } - vpaes_set_decrypt_key(kKey, bits, &key); + ASSERT_EQ(vpaes_set_decrypt_key(kKey, bits, &key), 0); CHECK_ABI(vpaes_decrypt_key_to_bsaes, &key, &key); for (size_t blocks : block_counts) { SCOPED_TRACE(blocks); @@ -308,7 +308,7 @@ } if (vpaes_capable()) { - CHECK_ABI(vpaes_set_encrypt_key, kKey, bits, &key); + ASSERT_EQ(CHECK_ABI(vpaes_set_encrypt_key, kKey, bits, &key), 0); CHECK_ABI(vpaes_encrypt, block, block, &key); for (size_t blocks : block_counts) { SCOPED_TRACE(blocks); @@ -321,7 +321,7 @@ #endif } - CHECK_ABI(vpaes_set_decrypt_key, kKey, bits, &key); + ASSERT_EQ(CHECK_ABI(vpaes_set_decrypt_key, kKey, bits, &key), 0); CHECK_ABI(vpaes_decrypt, block, block, &key); #if defined(VPAES_CBC) for (size_t blocks : block_counts) { @@ -333,7 +333,7 @@ } if (hwaes_capable()) { - CHECK_ABI(aes_hw_set_encrypt_key, kKey, bits, &key); + ASSERT_EQ(CHECK_ABI(aes_hw_set_encrypt_key, kKey, bits, &key), 0); CHECK_ABI(aes_hw_encrypt, block, block, &key); for (size_t blocks : block_counts) { SCOPED_TRACE(blocks); @@ -346,7 +346,7 @@ #endif } - CHECK_ABI(aes_hw_set_decrypt_key, kKey, bits, &key); + ASSERT_EQ(CHECK_ABI(aes_hw_set_decrypt_key, kKey, bits, &key), 0); CHECK_ABI(aes_hw_decrypt, block, block, &key); for (size_t blocks : block_counts) { SCOPED_TRACE(blocks);
diff --git a/crypto/fipsmodule/rand/rand.c b/crypto/fipsmodule/rand/rand.c index bf0f486..cb1ee7d 100644 --- a/crypto/fipsmodule/rand/rand.c +++ b/crypto/fipsmodule/rand/rand.c
@@ -242,12 +242,13 @@ CRYPTO_STATIC_MUTEX_unlock_write(entropy_buffer_lock_bss_get()); } -// rand_get_seed fills |seed| with entropy and sets -// |*out_want_additional_input| to one if that entropy came directly from the -// CPU and zero otherwise. +// rand_get_seed fills |seed| with entropy. In some cases, it will additionally +// fill |additional_input| with entropy to supplement |seed|. It sets +// |*out_additional_input_len| to the number of extra bytes. static void rand_get_seed(struct rand_thread_state *state, uint8_t seed[CTR_DRBG_ENTROPY_LEN], - int *out_want_additional_input) { + uint8_t additional_input[CTR_DRBG_ENTROPY_LEN], + size_t *out_additional_input_len) { uint8_t entropy_bytes[sizeof(state->last_block) + CTR_DRBG_ENTROPY_LEN * BORINGSSL_FIPS_OVERREAD]; uint8_t *entropy = entropy_bytes; @@ -259,7 +260,8 @@ entropy_len -= sizeof(state->last_block); } - get_seed_entropy(entropy, entropy_len, out_want_additional_input); + int want_additional_input; + get_seed_entropy(entropy, entropy_len, &want_additional_input); if (!state->last_block_valid) { OPENSSL_memcpy(state->last_block, entropy, sizeof(state->last_block)); @@ -295,20 +297,30 @@ seed[j] ^= entropy[CTR_DRBG_ENTROPY_LEN * i + j]; } } + + // If we used something other than system entropy then also + // opportunistically read from the system. This avoids solely relying on the + // hardware once the entropy pool has been initialized. + *out_additional_input_len = 0; + if (want_additional_input && + CRYPTO_sysrand_if_available(additional_input, CTR_DRBG_ENTROPY_LEN)) { + *out_additional_input_len = CTR_DRBG_ENTROPY_LEN; + } } #else -// rand_get_seed fills |seed| with entropy and sets -// |*out_want_additional_input| to one if that entropy came directly from the -// CPU and zero otherwise. +// rand_get_seed fills |seed| with entropy. In some cases, it will additionally +// fill |additional_input| with entropy to supplement |seed|. It sets +// |*out_additional_input_len| to the number of extra bytes. static void rand_get_seed(struct rand_thread_state *state, uint8_t seed[CTR_DRBG_ENTROPY_LEN], - int *out_want_additional_input) { + uint8_t additional_input[CTR_DRBG_ENTROPY_LEN], + size_t *out_additional_input_len) { // If not in FIPS mode, we don't overread from the system entropy source and // we don't depend only on the hardware RDRAND. CRYPTO_sysrand_for_seed(seed, CTR_DRBG_ENTROPY_LEN); - *out_want_additional_input = 0; + *out_additional_input_len = 0; } #endif @@ -367,20 +379,9 @@ state->last_block_valid = 0; uint8_t seed[CTR_DRBG_ENTROPY_LEN]; - int want_additional_input; - rand_get_seed(state, seed, &want_additional_input); - uint8_t personalization[CTR_DRBG_ENTROPY_LEN] = {0}; size_t personalization_len = 0; -#if defined(OPENSSL_URANDOM) - // If we used something other than system entropy then also - // opportunistically read from the system. This avoids solely relying on the - // hardware once the entropy pool has been initialized. - if (want_additional_input && - CRYPTO_sysrand_if_available(personalization, sizeof(personalization))) { - personalization_len = sizeof(personalization); - } -#endif + rand_get_seed(state, seed, personalization, &personalization_len); if (!CTR_DRBG_init(&state->drbg, seed, personalization, personalization_len)) { @@ -407,8 +408,10 @@ if (state->calls >= kReseedInterval || state->fork_generation != fork_generation) { uint8_t seed[CTR_DRBG_ENTROPY_LEN]; - int want_additional_input; - rand_get_seed(state, seed, &want_additional_input); + uint8_t reseed_additional_data[CTR_DRBG_ENTROPY_LEN] = {0}; + size_t reseed_additional_data_len = 0; + rand_get_seed(state, seed, reseed_additional_data, + &reseed_additional_data_len); #if defined(BORINGSSL_FIPS) // Take a read lock around accesses to |state->drbg|. This is needed to // avoid returning bad entropy if we race with @@ -420,7 +423,8 @@ // kernel, syscalls made with |syscall| did not abort the transaction. CRYPTO_STATIC_MUTEX_lock_read(state_clear_all_lock_bss_get()); #endif - if (!CTR_DRBG_reseed(&state->drbg, seed, NULL, 0)) { + if (!CTR_DRBG_reseed(&state->drbg, seed, reseed_additional_data, + reseed_additional_data_len)) { abort(); } state->calls = 0; @@ -469,3 +473,10 @@ int RAND_pseudo_bytes(uint8_t *buf, size_t len) { return RAND_bytes(buf, len); } + +void RAND_get_system_entropy_for_custom_prng(uint8_t *buf, size_t len) { + if (len > 256) { + abort(); + } + CRYPTO_sysrand_for_seed(buf, len); +}
diff --git a/crypto/fipsmodule/rand/urandom.c b/crypto/fipsmodule/rand/urandom.c index 11f1b5b..77dad74 100644 --- a/crypto/fipsmodule/rand/urandom.c +++ b/crypto/fipsmodule/rand/urandom.c
@@ -62,14 +62,11 @@ #include <sys/random.h> #endif -#if defined(OPENSSL_FREEBSD) -#define URANDOM_BLOCKS_FOR_ENTROPY -#if __FreeBSD__ >= 12 +#if defined(OPENSSL_FREEBSD) && __FreeBSD__ >= 12 // getrandom is supported in FreeBSD 12 and up. #define FREEBSD_GETRANDOM #include <sys/random.h> #endif -#endif #include <openssl/thread.h> #include <openssl/mem.h> @@ -190,8 +187,14 @@ return; #endif - // Android FIPS builds must support getrandom. -#if defined(BORINGSSL_FIPS) && defined(OPENSSL_ANDROID) + // FIPS builds must support getrandom. + // + // Historically, only Android FIPS builds required getrandom, while Linux FIPS + // builds had a /dev/urandom fallback which used RNDGETENTCNT as a poor + // approximation for getrandom's blocking behavior. This is now removed, but + // avoid making assumptions on this removal until March 2023, in case it needs + // to be restored. This comment can be deleted after March 2023. +#if defined(BORINGSSL_FIPS) perror("getrandom not found"); abort(); #endif @@ -255,29 +258,6 @@ #endif // USE_NR_getrandom return; } - -#if defined(BORINGSSL_FIPS) && !defined(URANDOM_BLOCKS_FOR_ENTROPY) - // In FIPS mode on platforms where urandom doesn't block at startup, we ensure - // that the kernel has sufficient entropy before continuing. This is - // automatically handled by getrandom, which requires that the entropy pool - // has been initialised, but for urandom we have to poll. - for (;;) { - int entropy_bits; - if (ioctl(fd, RNDGETENTCNT, &entropy_bits)) { - fprintf(stderr, - "RNDGETENTCNT on /dev/urandom failed. We cannot continue in this " - "case when in FIPS mode.\n"); - abort(); - } - - static const int kBitsNeeded = 256; - if (entropy_bits >= kBitsNeeded) { - break; - } - - usleep(250000); - } -#endif // BORINGSSL_FIPS && !URANDOM_BLOCKS_FOR_ENTROPY } // fill_with_entropy writes |len| bytes of entropy into |out|. It returns one
diff --git a/crypto/fipsmodule/rand/urandom_test.cc b/crypto/fipsmodule/rand/urandom_test.cc index dabaf39..13ac610 100644 --- a/crypto/fipsmodule/rand/urandom_test.cc +++ b/crypto/fipsmodule/rand/urandom_test.cc
@@ -45,7 +45,6 @@ kGetRandom, kOpen, kUrandomRead, - kUrandomIoctl, kAbort, }; @@ -77,11 +76,6 @@ return e; } - static Event UrandomIoctl() { - Event e(Syscall::kUrandomIoctl); - return e; - } - static Event Abort() { Event e(Syscall::kAbort); return e; @@ -103,9 +97,6 @@ snprintf(buf, sizeof(buf), "read(urandom_fd, _, %zu)", length); break; - case Syscall::kUrandomIoctl: - return "ioctl(urandom_fd, RNDGETENTCNT, _)"; - case Syscall::kAbort: return "abort()"; } @@ -139,14 +130,11 @@ static const unsigned NO_URANDOM = 2; // getrandom always returns |EAGAIN| if given |GRNG_NONBLOCK|. static const unsigned GETRANDOM_NOT_READY = 4; -// The ioctl on urandom returns only 255 bits of entropy the first time that -// it's called. -static const unsigned URANDOM_NOT_READY = 8; // getrandom gives |EINVAL| unless |NO_GETRANDOM| is set. -static const unsigned GETRANDOM_ERROR = 16; +static const unsigned GETRANDOM_ERROR = 8; // Reading from /dev/urandom gives |EINVAL|. -static const unsigned URANDOM_ERROR = 32; -static const unsigned NEXT_FLAG = 64; +static const unsigned URANDOM_ERROR = 16; +static const unsigned NEXT_FLAG = 32; // GetTrace runs |thunk| in a forked process and observes the resulting system // calls using ptrace. It simulates a variety of failures based on the contents @@ -204,8 +192,6 @@ const auto syscall_number = regs.orig_rax; bool is_opening_urandom = false; - bool is_urandom_ioctl = false; - uintptr_t ioctl_output_addr = 0; // inject_error is zero to indicate that the system call should run // normally. Otherwise it's, e.g. -EINVAL, to indicate that the system call // should not run and that error should be injected on return. @@ -251,16 +237,6 @@ } break; } - - case __NR_ioctl: { - const int ioctl_fd = regs.rdi; - if (urandom_fd >= 0 && ioctl_fd == urandom_fd && - regs.rsi == RNDGETENTCNT) { - out_trace->push_back(Event::UrandomIoctl()); - is_urandom_ioctl = true; - ioctl_output_addr = regs.rdx; - } - } } if (inject_error) { @@ -294,33 +270,6 @@ } else if (is_opening_urandom) { ASSERT_EQ(0, ptrace(PTRACE_GETREGS, child_pid, nullptr, ®s)); urandom_fd = regs.rax; - } else if (is_urandom_ioctl) { - // The result is the number of bits of entropy that the kernel currently - // believes that it has. urandom.c waits until 256 bits are ready. - int result = 256; - - // If we are simulating urandom not being ready then we have the ioctl - // indicate one too few bits of entropy the first time it's queried. - if (flags & URANDOM_NOT_READY) { - result--; - flags &= ~URANDOM_NOT_READY; - } - - // ptrace always works with ill-defined "words", which appear to be 64-bit - // on x86-64. Since the ioctl result is a 32-bit int, do a - // read-modify-write to inject the answer. - const uintptr_t aligned_addr = ioctl_output_addr & ~7; - const uintptr_t offset = ioctl_output_addr - aligned_addr; - union { - uint64_t word; - uint8_t bytes[8]; - } u; - u.word = ptrace(PTRACE_PEEKDATA, child_pid, - reinterpret_cast<void *>(aligned_addr), nullptr); - memcpy(&u.bytes[offset], &result, sizeof(result)); - ASSERT_EQ(0, ptrace(PTRACE_POKEDATA, child_pid, - reinterpret_cast<void *>(aligned_addr), - reinterpret_cast<void *>(u.word))); } } } @@ -347,7 +296,6 @@ #endif std::vector<Event> ret; - bool urandom_probed = false; bool getrandom_ready = false; // Probe for getrandom support @@ -356,32 +304,19 @@ std::function<bool(bool, size_t)> sysrand; if (flags & NO_GETRANDOM) { + if (is_fips) { + // FIPS builds require getrandom. + ret.push_back(Event::Abort()); + return ret; + } + ret.push_back(Event::Open("/dev/urandom")); if (flags & NO_URANDOM) { ret.push_back(Event::Abort()); return ret; } - wait_for_entropy = [&ret, &urandom_probed, flags] { - if (!is_fips || urandom_probed) { - return; - } - - // Probe urandom for entropy. - ret.push_back(Event::UrandomIoctl()); - if (flags & URANDOM_NOT_READY) { - // If the first attempt doesn't report enough entropy, probe - // repeatedly until it does, which will happen with the second attempt. - ret.push_back(Event::UrandomIoctl()); - } - - urandom_probed = true; - }; - - sysrand = [&ret, &wait_for_entropy, flags](bool block, size_t len) { - if (block) { - wait_for_entropy(); - } + sysrand = [&ret, flags](bool block, size_t len) { ret.push_back(Event::UrandomRead(len)); if (flags & URANDOM_ERROR) { ret.push_back(Event::Abort()); @@ -457,11 +392,6 @@ } break; - case Event::Syscall::kUrandomIoctl: - ADD_FAILURE() << "Urandom polling found with RDRAND: " - << ToString(events); - break; - default: break; } @@ -483,7 +413,6 @@ TRACE_FLAG(NO_GETRANDOM); TRACE_FLAG(NO_URANDOM); TRACE_FLAG(GETRANDOM_NOT_READY); - TRACE_FLAG(URANDOM_NOT_READY); TRACE_FLAG(GETRANDOM_ERROR); TRACE_FLAG(URANDOM_ERROR);
diff --git a/crypto/fipsmodule/rsa/padding.c b/crypto/fipsmodule/rsa/padding.c index 605647a..0dafed4 100644 --- a/crypto/fipsmodule/rsa/padding.c +++ b/crypto/fipsmodule/rsa/padding.c
@@ -489,29 +489,23 @@ int RSA_verify_PKCS1_PSS_mgf1(const RSA *rsa, const uint8_t *mHash, const EVP_MD *Hash, const EVP_MD *mgf1Hash, const uint8_t *EM, int sLen) { - int i; - int ret = 0; - int maskedDBLen, MSBits, emLen; - size_t hLen; - const uint8_t *H; - uint8_t *DB = NULL; - EVP_MD_CTX ctx; - uint8_t H_[EVP_MAX_MD_SIZE]; - EVP_MD_CTX_init(&ctx); - if (mgf1Hash == NULL) { mgf1Hash = Hash; } - hLen = EVP_MD_size(Hash); + int ret = 0; + uint8_t *DB = NULL; + EVP_MD_CTX ctx; + EVP_MD_CTX_init(&ctx); FIPS_service_indicator_lock_state(); // Negative sLen has special meanings: // -1 sLen == hLen // -2 salt length is autorecovered from signature // -N reserved + size_t hLen = EVP_MD_size(Hash); if (sLen == -1) { - sLen = hLen; + sLen = (int)hLen; } else if (sLen == -2) { sLen = -2; } else if (sLen < -2) { @@ -519,8 +513,8 @@ goto err; } - MSBits = (BN_num_bits(rsa->n) - 1) & 0x7; - emLen = RSA_size(rsa); + unsigned MSBits = (BN_num_bits(rsa->n) - 1) & 0x7; + size_t emLen = RSA_size(rsa); if (EM[0] & (0xFF << MSBits)) { OPENSSL_PUT_ERROR(RSA, RSA_R_FIRST_OCTET_INVALID); goto err; @@ -529,8 +523,9 @@ EM++; emLen--; } - if (emLen < (int)hLen + 2 || emLen < ((int)hLen + sLen + 2)) { - // sLen can be small negative + // |sLen| may be -2 for the non-standard salt length recovery mode. + if (emLen < hLen + 2 || + (sLen >= 0 && emLen < hLen + (size_t)sLen + 2)) { OPENSSL_PUT_ERROR(RSA, RSA_R_DATA_TOO_LARGE); goto err; } @@ -538,8 +533,8 @@ OPENSSL_PUT_ERROR(RSA, RSA_R_LAST_OCTET_INVALID); goto err; } - maskedDBLen = emLen - hLen - 1; - H = EM + maskedDBLen; + size_t maskedDBLen = emLen - hLen - 1; + const uint8_t *H = EM + maskedDBLen; DB = OPENSSL_malloc(maskedDBLen); if (!DB) { OPENSSL_PUT_ERROR(RSA, ERR_R_MALLOC_FAILURE); @@ -548,42 +543,49 @@ if (!PKCS1_MGF1(DB, maskedDBLen, H, hLen, mgf1Hash)) { goto err; } - for (i = 0; i < maskedDBLen; i++) { + for (size_t i = 0; i < maskedDBLen; i++) { DB[i] ^= EM[i]; } if (MSBits) { DB[0] &= 0xFF >> (8 - MSBits); } - for (i = 0; DB[i] == 0 && i < (maskedDBLen - 1); i++) { + // This step differs slightly from EMSA-PSS-VERIFY (RFC 8017) step 10 because + // it accepts a non-standard salt recovery flow. DB should be some number of + // zeros, a one, then the salt. + size_t salt_start; + for (salt_start = 0; DB[salt_start] == 0 && salt_start < maskedDBLen - 1; + salt_start++) { ; } - if (DB[i++] != 0x1) { + if (DB[salt_start] != 0x1) { OPENSSL_PUT_ERROR(RSA, RSA_R_SLEN_RECOVERY_FAILED); goto err; } - if (sLen >= 0 && (maskedDBLen - i) != sLen) { + salt_start++; + // If a salt length was specified, check it matches. + if (sLen >= 0 && maskedDBLen - salt_start != (size_t)sLen) { OPENSSL_PUT_ERROR(RSA, RSA_R_SLEN_CHECK_FAILED); goto err; } + uint8_t H_[EVP_MAX_MD_SIZE]; if (!EVP_DigestInit_ex(&ctx, Hash, NULL) || !EVP_DigestUpdate(&ctx, kPSSZeroes, sizeof(kPSSZeroes)) || !EVP_DigestUpdate(&ctx, mHash, hLen) || - !EVP_DigestUpdate(&ctx, DB + i, maskedDBLen - i) || + !EVP_DigestUpdate(&ctx, DB + salt_start, maskedDBLen - salt_start) || !EVP_DigestFinal_ex(&ctx, H_, NULL)) { goto err; } - if (OPENSSL_memcmp(H_, H, hLen)) { + if (OPENSSL_memcmp(H_, H, hLen) != 0) { OPENSSL_PUT_ERROR(RSA, RSA_R_BAD_SIGNATURE); - ret = 0; - } else { - ret = 1; + goto err; } + ret = 1; + err: OPENSSL_free(DB); EVP_MD_CTX_cleanup(&ctx); FIPS_service_indicator_unlock_state(); - return ret; }
diff --git a/include/openssl/rand.h b/include/openssl/rand.h index bd41f9e..586274d 100644 --- a/include/openssl/rand.h +++ b/include/openssl/rand.h
@@ -25,9 +25,20 @@ // Random number generation. -// RAND_bytes writes |len| bytes of random data to |buf| and returns one. +// RAND_bytes writes |len| bytes of random data to |buf| and returns one. In the +// event that sufficient random data can not be obtained, |abort| is called. OPENSSL_EXPORT int RAND_bytes(uint8_t *buf, size_t len); +// RAND_get_system_entropy_for_custom_prng writes |len| bytes of random data +// from a system entropy source to |buf|. The maximum length of entropy which +// may be requested is 256 bytes. If more than 256 bytes of data is requested, +// or if sufficient random data can not be obtained, |abort| is called. +// |RAND_bytes| should normally be used instead of this function. This function +// should only be used for seed values or where |malloc| should not be called +// from BoringSSL. This function is not FIPS compliant. +OPENSSL_EXPORT void RAND_get_system_entropy_for_custom_prng(uint8_t *buf, + size_t len); + // RAND_cleanup frees any resources used by the RNG. This is not safe if other // threads might still be calling |RAND_bytes|. OPENSSL_EXPORT void RAND_cleanup(void);