Skip to content

Commit 6a95668

Browse files
panvasxa
panva
authored and
sxa
committed
crypto: add guards and adjust tests for BoringSSL
Signed-off-by: Filip Skokan <panva.ip@gmail.com> PR-URL: #62883 Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
1 parent 3d621c3 commit 6a95668

52 files changed

Lines changed: 504 additions & 315 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎deps/ncrypto/ncrypto.cc‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -147,7 +147,12 @@ DataPointer DataPointer::SecureAlloc(size_t len) {
147147
#ifndef OPENSSL_IS_BORINGSSL
148148
auto ptr = OPENSSL_secure_zalloc(len);
149149
if (ptr == nullptr) return {};
150-
return DataPointer(ptr, len, true);
150+
// OPENSSL_secure_zalloc transparently falls back to a regular allocation
151+
// when the secure heap is not initialized or is exhausted. Reflect the
152+
// actual provenance of the pointer so that reset() routes to the correct
153+
// free function (OPENSSL_secure_clear_free vs. OPENSSL_clear_free) and
154+
// callers of isSecure() get a truthful answer.
155+
return DataPointer(ptr, len, CRYPTO_secure_allocated(ptr) == 1);
151156
#else
152157
// BoringSSL does not implement the OPENSSL_secure_zalloc API.
153158
auto ptr = OPENSSL_malloc(len);
@@ -3103,9 +3108,13 @@ const Cipher Cipher::AES_256_GCM = Cipher::FromNid(NID_aes_256_gcm);
31033108
const Cipher Cipher::AES_128_KW = Cipher::FromNid(NID_id_aes128_wrap);
31043109
const Cipher Cipher::AES_192_KW = Cipher::FromNid(NID_id_aes192_wrap);
31053110
const Cipher Cipher::AES_256_KW = Cipher::FromNid(NID_id_aes256_wrap);
3111+
3112+
#ifndef OPENSSL_IS_BORINGSSL
31063113
const Cipher Cipher::AES_128_OCB = Cipher::FromNid(NID_aes_128_ocb);
31073114
const Cipher Cipher::AES_192_OCB = Cipher::FromNid(NID_aes_192_ocb);
31083115
const Cipher Cipher::AES_256_OCB = Cipher::FromNid(NID_aes_256_ocb);
3116+
#endif
3117+
31093118
const Cipher Cipher::CHACHA20_POLY1305 = Cipher::FromNid(NID_chacha20_poly1305);
31103119

31113120
bool Cipher::isGcmMode() const {

‎deps/ncrypto/ncrypto.h‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -309,9 +309,12 @@ class Cipher final {
309309
#else
310310
static constexpr size_t MAX_AUTH_TAG_LENGTH = 16;
311311
#endif
312-
static_assert(EVP_GCM_TLS_TAG_LEN <= MAX_AUTH_TAG_LENGTH &&
313-
EVP_CCM_TLS_TAG_LEN <= MAX_AUTH_TAG_LENGTH &&
314-
EVP_CHACHAPOLY_TLS_TAG_LEN <= MAX_AUTH_TAG_LENGTH);
312+
static_assert(EVP_GCM_TLS_TAG_LEN <= MAX_AUTH_TAG_LENGTH
313+
#ifndef OPENSSL_IS_BORINGSSL
314+
&& EVP_CCM_TLS_TAG_LEN <= MAX_AUTH_TAG_LENGTH &&
315+
EVP_CHACHAPOLY_TLS_TAG_LEN <= MAX_AUTH_TAG_LENGTH
316+
#endif
317+
); // NOLINT(whitespace/parens)
315318

316319
Cipher() = default;
317320
Cipher(const EVP_CIPHER* cipher) : cipher_(cipher) {}

‎src/crypto/crypto_cipher.cc‎

Lines changed: 31 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -432,61 +432,46 @@ bool CipherBase::InitAuthenticated(const char* cipher_type,
432432
return false;
433433
}
434434

435-
if (ctx_.isGcmMode()) {
436-
if (auth_tag_len != kNoAuthTagLength) {
437-
if (!Cipher::IsValidGCMTagLength(auth_tag_len)) {
438-
THROW_ERR_CRYPTO_INVALID_AUTH_TAG(
439-
env(),
440-
"Invalid authentication tag length: %u",
441-
auth_tag_len);
442-
return false;
443-
}
444-
445-
// Remember the given authentication tag length for later.
446-
auth_tag_len_ = auth_tag_len;
447-
}
448-
} else {
449-
if (auth_tag_len == kNoAuthTagLength) {
450-
// We treat ChaCha20-Poly1305 specially. Like GCM, the authentication tag
451-
// length defaults to 16 bytes when encrypting. Unlike GCM, the
452-
// authentication tag length also defaults to 16 bytes when decrypting,
453-
// whereas GCM would accept any valid authentication tag length.
454-
if (ctx_.isChaCha20Poly1305()) {
455-
auth_tag_len = EVP_CHACHAPOLY_TLS_TAG_LEN;
456-
} else {
457-
THROW_ERR_CRYPTO_INVALID_AUTH_TAG(
458-
env(), "authTagLength required for %s", cipher_type);
459-
return false;
460-
}
461-
}
462-
435+
if (ctx_.isCcmMode()) {
463436
// TODO(tniessen) Support CCM decryption in FIPS mode
464-
465-
if (ctx_.isCcmMode() && kind_ == kDecipher && ncrypto::isFipsEnabled()) {
466-
THROW_ERR_CRYPTO_UNSUPPORTED_OPERATION(env(),
467-
"CCM encryption not supported in FIPS mode");
437+
if (kind_ == kDecipher && ncrypto::isFipsEnabled()) {
438+
THROW_ERR_CRYPTO_UNSUPPORTED_OPERATION(
439+
env(), "CCM encryption not supported in FIPS mode");
468440
return false;
469441
}
470442

471-
// Tell OpenSSL about the desired length.
472-
if (!ctx_.setAeadTagLength(auth_tag_len)) {
443+
// Restrict the message length to min(INT_MAX, 2^(8*(15-iv_len))-1) bytes.
444+
CHECK(iv_len >= 7 && iv_len <= 13);
445+
max_message_size_ = INT_MAX;
446+
if (iv_len == 12) max_message_size_ = 16777215;
447+
if (iv_len == 13) max_message_size_ = 65535;
448+
}
449+
450+
if (auth_tag_len == kNoAuthTagLength) {
451+
// GCM accepts any valid authentication tag length when decrypting without
452+
// an explicit authTagLength. This remains deprecated, but supported.
453+
if (ctx_.isGcmMode()) {
454+
return true;
455+
#ifdef EVP_CHACHAPOLY_TLS_TAG_LEN
456+
} else if (ctx_.isChaCha20Poly1305()) {
457+
auth_tag_len = EVP_CHACHAPOLY_TLS_TAG_LEN;
458+
#endif
459+
} else {
473460
THROW_ERR_CRYPTO_INVALID_AUTH_TAG(
474-
env(), "Invalid authentication tag length: %u", auth_tag_len);
461+
env(), "authTagLength required for %s", cipher_type);
475462
return false;
476463
}
477-
478-
// Remember the given authentication tag length for later.
479-
auth_tag_len_ = auth_tag_len;
480-
481-
if (ctx_.isCcmMode()) {
482-
// Restrict the message length to min(INT_MAX, 2^(8*(15-iv_len))-1) bytes.
483-
CHECK(iv_len >= 7 && iv_len <= 13);
484-
max_message_size_ = INT_MAX;
485-
if (iv_len == 12) max_message_size_ = 16777215;
486-
if (iv_len == 13) max_message_size_ = 65535;
487-
}
464+
} else if ((ctx_.isGcmMode() && !Cipher::IsValidGCMTagLength(auth_tag_len)) ||
465+
(!ctx_.isGcmMode() && !ctx_.setAeadTagLength(auth_tag_len))) {
466+
// GCM authentication tag lengths are restricted according to NIST 800-38d,
467+
// page 9. For other modes, we rely on OpenSSL to validate the length.
468+
THROW_ERR_CRYPTO_INVALID_AUTH_TAG(
469+
env(), "Invalid authentication tag length: %u", auth_tag_len);
470+
return false;
488471
}
489472

473+
// Remember the given authentication tag length for later.
474+
auth_tag_len_ = auth_tag_len;
490475
return true;
491476
}
492477

‎src/crypto/crypto_context.cc‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1915,8 +1915,13 @@ void SecureContext::SetDHParam(const FunctionCallbackInfo<Value>& args) {
19151915
// true to this function instead of the original string. Any other string
19161916
// value will be interpreted as custom DH parameters below.
19171917
if (args[0]->IsTrue()) {
1918+
#ifdef SSL_CTX_set_dh_auto
19181919
CHECK(SSL_CTX_set_dh_auto(sc->ctx_.get(), true));
19191920
return;
1921+
#else
1922+
return THROW_ERR_CRYPTO_UNSUPPORTED_OPERATION(
1923+
env, "Automatic DH parameter selection is not supported");
1924+
#endif
19201925
}
19211926

19221927
DHPointer dh;

‎src/crypto/crypto_dh.cc‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -309,15 +309,17 @@ void ComputeSecret(const FunctionCallbackInfo<Value>& args) {
309309
BignumPointer key(key_buf.data(), key_buf.size());
310310

311311
switch (dh.checkPublicKey(key)) {
312-
case DHPointer::CheckPublicKeyResult::INVALID:
313-
// Fall-through
314312
case DHPointer::CheckPublicKeyResult::CHECK_FAILED:
315313
return THROW_ERR_CRYPTO_INVALID_KEYTYPE(env,
316314
"Unspecified validation error");
315+
#ifndef OPENSSL_IS_BORINGSSL
317316
case DHPointer::CheckPublicKeyResult::TOO_SMALL:
318317
return THROW_ERR_CRYPTO_INVALID_KEYLEN(env, "Supplied key is too small");
319318
case DHPointer::CheckPublicKeyResult::TOO_LARGE:
320319
return THROW_ERR_CRYPTO_INVALID_KEYLEN(env, "Supplied key is too large");
320+
#endif
321+
case DHPointer::CheckPublicKeyResult::INVALID:
322+
return THROW_ERR_CRYPTO_INVALID_KEYTYPE(env, "Supplied key is invalid");
321323
case DHPointer::CheckPublicKeyResult::NONE:
322324
break;
323325
}

‎src/crypto/crypto_rsa.cc‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -135,6 +135,18 @@ Maybe<void> RsaKeyGenTraits::AdditionalConfig(
135135
params->params.modulus_bits = args[*offset + 1].As<Uint32>()->Value();
136136
params->params.exponent = args[*offset + 2].As<Uint32>()->Value();
137137

138+
#ifdef OPENSSL_IS_BORINGSSL
139+
// BoringSSL hangs indefinitely generating an RSA key with e=1, and for
140+
// other invalid exponents (e=0, even values) reports the misleading error
141+
// RSA_R_TOO_MANY_ITERATIONS only after running the full keygen loop. Reject
142+
// those up-front with a clear error. The constraint here (odd integer >= 3)
143+
// matches BoringSSL's own rsa_check_public_key validation.
144+
if (params->params.exponent < 3 || (params->params.exponent & 1) == 0) {
145+
THROW_ERR_OUT_OF_RANGE(env, "publicExponent is invalid");
146+
return Nothing<void>();
147+
}
148+
#endif
149+
138150
*offset += 3;
139151

140152
if (params->params.variant == kKeyVariantRSA_PSS) {

‎src/crypto/crypto_util.cc‎

Lines changed: 46 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -514,44 +514,51 @@ Maybe<void> Decorate(Environment* env,
514514
c = ToUpper(c);
515515
}
516516

517-
#define OSSL_ERROR_CODES_MAP(V) \
518-
V(SYS) \
519-
V(BN) \
520-
V(RSA) \
521-
V(DH) \
522-
V(EVP) \
523-
V(BUF) \
524-
V(OBJ) \
525-
V(PEM) \
526-
V(DSA) \
527-
V(X509) \
528-
V(ASN1) \
529-
V(CONF) \
530-
V(CRYPTO) \
531-
V(EC) \
532-
V(SSL) \
533-
V(BIO) \
534-
V(PKCS7) \
535-
V(X509V3) \
536-
V(PKCS12) \
537-
V(RAND) \
538-
V(DSO) \
539-
V(ENGINE) \
540-
V(OCSP) \
541-
V(UI) \
542-
V(COMP) \
543-
V(ECDSA) \
544-
V(ECDH) \
545-
V(OSSL_STORE) \
546-
V(FIPS) \
547-
V(CMS) \
548-
V(TS) \
549-
V(HMAC) \
550-
V(CT) \
551-
V(ASYNC) \
552-
V(KDF) \
553-
V(SM2) \
554-
V(USER) \
517+
#ifdef OPENSSL_IS_BORINGSSL
518+
#define OSSL_ERROR_CODES_MAP_OPENSSL_ONLY(V)
519+
#else
520+
#define OSSL_ERROR_CODES_MAP_OPENSSL_ONLY(V) \
521+
V(PKCS12) \
522+
V(DSO) \
523+
V(OSSL_STORE) \
524+
V(FIPS) \
525+
V(TS) \
526+
V(CT) \
527+
V(ASYNC) \
528+
V(KDF) \
529+
V(SM2)
530+
#endif
531