Skip to content

Commit 277f68f

Browse files
panvapanva
committed
crypto: add guards and adjust tests for BoringSSL
Signed-off-by: Filip Skokan <panva.ip@gmail.com>
1 parent c29a34c commit 277f68f

52 files changed

Lines changed: 448 additions & 166 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: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3103,9 +3103,13 @@ const Cipher Cipher::AES_256_GCM = Cipher::FromNid(NID_aes_256_gcm);
31033103
const Cipher Cipher::AES_128_KW = Cipher::FromNid(NID_id_aes128_wrap);
31043104
const Cipher Cipher::AES_192_KW = Cipher::FromNid(NID_id_aes192_wrap);
31053105
const Cipher Cipher::AES_256_KW = Cipher::FromNid(NID_id_aes256_wrap);
3106+
3107+
#ifndef OPENSSL_IS_BORINGSSL
31063108
const Cipher Cipher::AES_128_OCB = Cipher::FromNid(NID_aes_128_ocb);
31073109
const Cipher Cipher::AES_192_OCB = Cipher::FromNid(NID_aes_192_ocb);
31083110
const Cipher Cipher::AES_256_OCB = Cipher::FromNid(NID_aes_256_ocb);
3111+
#endif
3112+
31093113
const Cipher Cipher::CHACHA20_POLY1305 = Cipher::FromNid(NID_chacha20_poly1305);
31103114

31113115
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+
);
315318

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

‎src/crypto/crypto_cipher.cc‎

Lines 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);

‎src/crypto/crypto_context.cc‎

Lines 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;

‎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 & 38 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 = "";

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

Lines 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
{

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

Lines 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
}

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

Lines 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)