Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit b10653a

Browse filesBrowse files
panvaaduh95
panva
authored and
aduh95
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 19b7adf commit b10653a
Copy full SHA for b10653a

53 files changed

+473-168Lines changed: 473 additions & 168 deletions

File tree

Expand file treeCollapse file tree
Open diff view settings
Filter options

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.
Dismiss banner
Expand file treeCollapse file tree
Open diff view settings
Collapse file

‎deps/ncrypto/ncrypto.cc‎

Copy file name to clipboardExpand all lines: deps/ncrypto/ncrypto.cc
+10-1Lines 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 {
Collapse file

‎deps/ncrypto/ncrypto.h‎

Copy file name to clipboardExpand all lines: deps/ncrypto/ncrypto.h
+6-3Lines 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) {}
Collapse file

‎src/crypto/crypto_cipher.cc‎

Copy file name to clipboardExpand all lines: src/crypto/crypto_cipher.cc
+2Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -447,8 +447,10 @@ bool CipherBase::InitAuthenticated(const char* cipher_type,
447447
// Other modes (CCM, OCB) require an explicit tag length.
448448
if (ctx_.isGcmMode()) {
449449
auth_tag_len = EVP_GCM_TLS_TAG_LEN;
450+
#ifdef EVP_CHACHAPOLY_TLS_TAG_LEN
450451
} else if (ctx_.isChaCha20Poly1305()) {
451452
auth_tag_len = EVP_CHACHAPOLY_TLS_TAG_LEN;
453+
#endif
452454
} else {
453455
THROW_ERR_CRYPTO_INVALID_AUTH_TAG(
454456
env(), "authTagLength required for %s", cipher_type);
Collapse file

‎src/crypto/crypto_context.cc‎

Copy file name to clipboardExpand all lines: src/crypto/crypto_context.cc
+5Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1928,8 +1928,13 @@ void SecureContext::SetDHParam(const FunctionCallbackInfo<Value>& args) {
19281928
// true to this function instead of the original string. Any other string
19291929
// value will be interpreted as custom DH parameters below.
19301930
if (args[0]->IsTrue()) {
1931+
#ifdef SSL_CTX_set_dh_auto
19311932
CHECK(SSL_CTX_set_dh_auto(sc->ctx_.get(), true));
19321933
return;
1934+
#else
1935+
return THROW_ERR_CRYPTO_UNSUPPORTED_OPERATION(
1936+
env, "Automatic DH parameter selection is not supported");
1937+
#endif
19331938
}
19341939

19351940
DHPointer dh;
Collapse file

‎src/crypto/crypto_dh.cc‎

Copy file name to clipboardExpand all lines: src/crypto/crypto_dh.cc
+4-2Lines 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
}
Collapse file

‎src/crypto/crypto_rsa.cc‎

Copy file name to clipboardExpand all lines: src/crypto/crypto_rsa.cc
+12Lines 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) {
Collapse file

‎src/crypto/crypto_util.cc‎

Copy file name to clipboardExpand all lines: src/crypto/crypto_util.cc
+46-39Lines changed: 46 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -553,44 +553,51 @@ Maybe<void> Decorate(Environment* env,
553553
c = ToUpper(c);
554554
}
555555

556-
#define OSSL_ERROR_CODES_MAP(V) \
557-
V(SYS) \
558-
V(BN) \
559-
V(RSA) \
560-
V(DH) \
561-
V(EVP) \
562-
V(BUF) \
563-
V(OBJ) \
564-
V(PEM) \
565-
V(DSA) \
566-
V(X509) \
567-
V(ASN1) \
568-
V(CONF) \
569-
V(CRYPTO) \
570-
V(EC) \
571-
V(SSL) \
572-
V(BIO) \
573-
V(PKCS7) \
574-
V(X509V3) \
575-
V(PKCS12) \
576-
V(RAND) \
577-
V(DSO) \
578-
V(ENGINE) \
579-
V(OCSP) \
580-
V(UI) \
581-
V(COMP) \
582-
V(ECDSA) \
583-
V(ECDH) \
584-
V(OSSL_STORE) \
585-
V(FIPS) \
586-
V(CMS) \
587-
V(TS) \
588-
V(HMAC) \
589-
V(CT) \
590-
V(ASYNC) \
591-
V(KDF) \
592-
V(SM2) \
593-
V(USER) \
556+
#ifdef OPENSSL_IS_BORINGSSL
557+
#define OSSL_ERROR_CODES_MAP_OPENSSL_ONLY(V)
558+
#else
559+
#define OSSL_ERROR_CODES_MAP_OPENSSL_ONLY(V) \
560+
V(PKCS12) \
561+
V(DSO) \
562+
V(OSSL_STORE) \
563+
V(FIPS) \
564+
V(TS) \
565+
V(CT) \
566+
V(ASYNC) \
567+
V(KDF) \
568+
V(SM2)
569+
#endif
570+
571+
#define OSSL_ERROR_CODES_MAP(V) \
572+
V(SYS) \
573+
V(BN) \
574+
V(RSA) \
575+
V(DH) \
576+
V(EVP) \
577+
V(BUF) \
578+
V(OBJ) \
579+
V(PEM) \
580+
V(DSA) \
581+
V(X509) \
582+
V(ASN1) \
583+
V(CONF) \
584+
V(CRYPTO) \
585+
V(EC) \
586+
V(SSL) \
587+
V(BIO) \
588+
V(PKCS7) \
589+
V(X509V3) \
590+
V(RAND) \
591+
V(ENGINE) \
592+
V(OCSP) \
593+
V(UI) \
594+
V(COMP) \
595+
V(ECDSA) \
596+
V(ECDH) \
597+
V(CMS) \
598+
V(HMAC) \
599+
V(USER) \
600+
OSSL_ERROR_CODES_MAP_OPENSSL_ONLY(V)
594601

595602
#define V(name) case ERR_LIB_##name: lib = #name "_"; break;
596603
const char* lib = "";
@@ -600,6 +607,7 @@ Maybe<void> Decorate(Environment* env,
600607
}
601608
#undef V
602609
#undef OSSL_ERROR_CODES_MAP
610+
#undef OSSL_ERROR_CODES_MAP_OPENSSL_ONLY
603611
// Don't generate codes like "ERR_OSSL_SSL_".
604612
if (lib && strcmp(lib, "SSL_") == 0)
605613
prefix = "";
@@ -728,7 +736,6 @@ void SecureBuffer(const FunctionCallbackInfo<Value>& args) {
728736
uint32_t len = args[0].As<Uint32>()->Value();
729737

730738
auto data = DataPointer::SecureAlloc(len);
731-
CHECK(data.isSecure());
732739
if (!data) {
733740
return THROW_ERR_OPERATION_FAILED(env, "Allocation failed");
734741
}
Collapse file

‎test/parallel/test-crypto-async-sign-verify.js‎

Copy file name to clipboardExpand all lines: test/parallel/test-crypto-async-sign-verify.js
+11-9Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -102,17 +102,19 @@ if (!process.features.openssl_is_boringssl) {
102102
// ECDSA w/ ieee-p1363 signature encoding
103103
test('ec_secp256k1_public.pem', 'ec_secp256k1_private.pem', 'sha384', false,
104104
{ dsaEncoding: 'ieee-p1363' });
105-
}
106105

107-
// DSA w/ der signature encoding
108-
test('dsa_public.pem', 'dsa_private.pem', 'sha256',
109-
false);
110-
test('dsa_public.pem', 'dsa_private.pem', 'sha256',
111-
false, { dsaEncoding: 'der' });
106+
// DSA w/ der signature encoding
107+
test('dsa_public.pem', 'dsa_private.pem', 'sha256',
108+
false);
109+
test('dsa_public.pem', 'dsa_private.pem', 'sha256',
110+
false, { dsaEncoding: 'der' });
112111

113-
// DSA w/ ieee-p1363 signature encoding
114-
test('dsa_public.pem', 'dsa_private.pem', 'sha256', false,
115-
{ dsaEncoding: 'ieee-p1363' });
112+
// DSA w/ ieee-p1363 signature encoding
113+
test('dsa_public.pem', 'dsa_private.pem', 'sha256', false,
114+
{ dsaEncoding: 'ieee-p1363' });
115+
} else {
116+
common.printSkipMessage('Skipping unsupported ed448/secp256k1/dsa test cases');
117+
}
116118

117119
// Test Parallel Execution w/ KeyObject is threadsafe in openssl3
118120
{
Collapse file

‎test/parallel/test-crypto-authenticated.js‎

Copy file name to clipboardExpand all lines: test/parallel/test-crypto-authenticated.js
+30-17Lines changed: 30 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -626,22 +626,25 @@ for (const test of TEST_CASES) {
626626

627627
{
628628
// CCM cipher without data should not crash, see https://github.com/nodejs/node/issues/38035.
629-
const algo = 'aes-128-ccm';
630-
const key = Buffer.alloc(16);
631-
const iv = Buffer.alloc(12);
632-
const opts = { authTagLength: 10 };
629+
if (!ciphers.includes('aes-128-ccm')) {
630+
common.printSkipMessage(`unsupported aes-128-ccm test`);
631+
} else {
632+
const key = Buffer.alloc(16);
633+
const iv = Buffer.alloc(12);
634+
const opts = { authTagLength: 10 };
633635

634-
const cipher = crypto.createCipheriv(algo, key, iv, opts);
635-
assert.throws(() => {
636-
cipher.final();
637-
}, hasOpenSSL3 ? {
638-
code: 'ERR_OSSL_TAG_NOT_SET'
639-
} : {
640-
message: /Unsupported state/
641-
});
636+
const cipher = crypto.createCipheriv('aes-128-ccm', key, iv, opts);
637+
assert.throws(() => {
638+
cipher.final();
639+
}, hasOpenSSL3 ? {
640+
code: 'ERR_OSSL_TAG_NOT_SET'
641+
} : {
642+
message: /Unsupported state/
643+
});
644+
}
642645
}
643646

644-
{
647+
if (!process.features.openssl_is_boringssl) {
645648
const key = Buffer.alloc(32);
646649
const iv = Buffer.alloc(12);
647650

@@ -653,11 +656,13 @@ for (const test of TEST_CASES) {
653656
message: errMessages.authTagLength
654657
});
655658
}
659+
} else {
660+
common.printSkipMessage('Skipping unsupported chacha20-poly1305 test');
656661
}
657662

658663
// ChaCha20-Poly1305 should respect the authTagLength option and should not
659664
// require the authentication tag before calls to update() during decryption.
660-
{
665+
if (!process.features.openssl_is_boringssl) {
661666
const key = Buffer.alloc(32);
662667
const iv = Buffer.alloc(12);
663668

@@ -697,6 +702,8 @@ for (const test of TEST_CASES) {
697702
}
698703
}
699704
}
705+
} else {
706+
common.printSkipMessage('Skipping unsupported chacha20-poly1305 test');
700707
}
701708

702709
// ChaCha20-Poly1305 should default to an authTagLength of 16. When encrypting,
@@ -706,7 +713,7 @@ for (const test of TEST_CASES) {
706713
// shorter tags as long as their length was valid according to NIST SP 800-38D.
707714
// For ChaCha20-Poly1305, we intentionally deviate from that because there are
708715
// no recommended or approved authentication tag lengths below 16 bytes.
709-
{
716+
if (!process.features.openssl_is_boringssl) {
710717
const rfcTestCases = TEST_CASES.filter(({ algo, tampered }) => {
711718
return algo === 'chacha20-poly1305' && tampered === false;
712719
});
@@ -740,10 +747,12 @@ for (const test of TEST_CASES) {
740747

741748
assert.strictEqual(plaintext.toString('hex'), testCase.plain);
742749
}
750+
} else {
751+
common.printSkipMessage('Skipping unsupported chacha20-poly1305 test');
743752
}
744753

745754
// https://github.com/nodejs/node/issues/45874
746-
{
755+
if (!process.features.openssl_is_boringssl) {
747756
const rfcTestCases = TEST_CASES.filter(({ algo, tampered }) => {
748757
return algo === 'chacha20-poly1305' && tampered === false;
749758
});
@@ -771,10 +780,12 @@ for (const test of TEST_CASES) {
771780
assert.throws(() => {
772781
decipher.final();
773782
}, /Unsupported state or unable to authenticate data/);
783+
} else {
784+
common.printSkipMessage('Skipping unsupported chacha20-poly1305 test');
774785
}
775786

776787
// Refs: https://github.com/nodejs/node/issues/62342
777-
{
788+
if (ciphers.includes('aes-128-ccm')) {
778789
const key = crypto.randomBytes(16);
779790
const nonce = crypto.randomBytes(13);
780791

@@ -794,4 +805,6 @@ for (const test of TEST_CASES) {
794805
decipher.setAAD(Buffer.alloc(0), { plaintextLength: 0 });
795806
decipher.update(new DataView(new ArrayBuffer(0)));
796807
decipher.final();
808+
} else {
809+
common.printSkipMessage('Skipping unsupported aes-128-ccm test');
797810
}
Collapse file

‎test/parallel/test-crypto-cipheriv-decipheriv.js‎

Copy file name to clipboardExpand all lines: test/parallel/test-crypto-cipheriv-decipheriv.js
+4Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,10 @@ function testCipher2(key, iv) {
6262

6363

6464
function testCipher3(key, iv) {
65+
if (!crypto.getCiphers().includes('id-aes128-wrap')) {
66+
common.printSkipMessage(`unsupported id-aes128-wrap test`);
67+
return;
68+
}
6569
// Test encryption and decryption with explicit key and iv.
6670
// AES Key Wrap test vector comes from RFC3394
6771
const plaintext = Buffer.from('00112233445566778899AABBCCDDEEFF', 'hex');

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.