GeezerVison is showing github.com — crypto: add guards and adjust tests for BoringSSL · nodejs/node@b10653a
File treeExpand file treeCollapse file tree Open diff view settings Some content is hiddenLarge Commits have some content hidden by default. Use the searchbox below for content that may be hidden. Dismiss bannerExpand file treeCollapse file tree Open diff view settings Collapse file Copy file name to clipboardExpand all lines: deps/ncrypto/ncrypto.cc +10-1Lines changed: 10 additions & 1 deletion | Original file line number | Diff line number | Diff line change |
|---|
@@ -147,7 +147,12 @@ DataPointer DataPointer::SecureAlloc(size_t len) { | 147 | 147 | #ifndef OPENSSL_IS_BORINGSSL | 148 | 148 | auto ptr = OPENSSL_secure_zalloc(len); | 149 | 149 | 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); | 151 | 156 | #else | 152 | 157 | // BoringSSL does not implement the OPENSSL_secure_zalloc API. | 153 | 158 | auto ptr = OPENSSL_malloc(len); | @@ -3103,9 +3108,13 @@ const Cipher Cipher::AES_256_GCM = Cipher::FromNid(NID_aes_256_gcm); | 3103 | 3108 | const Cipher Cipher::AES_128_KW = Cipher::FromNid(NID_id_aes128_wrap); | 3104 | 3109 | const Cipher Cipher::AES_192_KW = Cipher::FromNid(NID_id_aes192_wrap); | 3105 | 3110 | const Cipher Cipher::AES_256_KW = Cipher::FromNid(NID_id_aes256_wrap); | | 3111 | + | | 3112 | +#ifndef OPENSSL_IS_BORINGSSL | 3106 | 3113 | const Cipher Cipher::AES_128_OCB = Cipher::FromNid(NID_aes_128_ocb); | 3107 | 3114 | const Cipher Cipher::AES_192_OCB = Cipher::FromNid(NID_aes_192_ocb); | 3108 | 3115 | const Cipher Cipher::AES_256_OCB = Cipher::FromNid(NID_aes_256_ocb); | | 3116 | +#endif | | 3117 | + | 3109 | 3118 | const Cipher Cipher::CHACHA20_POLY1305 = Cipher::FromNid(NID_chacha20_poly1305); | 3110 | 3119 |
| 3111 | 3120 | bool Cipher::isGcmMode() const { | |
Collapse file Copy file name to clipboardExpand all lines: deps/ncrypto/ncrypto.h +6-3Lines changed: 6 additions & 3 deletions | Original file line number | Diff line number | Diff line change |
|---|
@@ -309,9 +309,12 @@ class Cipher final { | 309 | 309 | #else | 310 | 310 | static constexpr size_t MAX_AUTH_TAG_LENGTH = 16; | 311 | 311 | #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) | 315 | 318 |
| 316 | 319 | Cipher() = default; | 317 | 320 | Cipher(const EVP_CIPHER* cipher) : cipher_(cipher) {} | |
Collapse file Copy file name to clipboardExpand all lines: src/crypto/crypto_cipher.cc +2Lines changed: 2 additions & 0 deletions | Original file line number | Diff line number | Diff line change |
|---|
@@ -447,8 +447,10 @@ bool CipherBase::InitAuthenticated(const char* cipher_type, | 447 | 447 | // Other modes (CCM, OCB) require an explicit tag length. | 448 | 448 | if (ctx_.isGcmMode()) { | 449 | 449 | auth_tag_len = EVP_GCM_TLS_TAG_LEN; | | 450 | +#ifdef EVP_CHACHAPOLY_TLS_TAG_LEN | 450 | 451 | } else if (ctx_.isChaCha20Poly1305()) { | 451 | 452 | auth_tag_len = EVP_CHACHAPOLY_TLS_TAG_LEN; | | 453 | +#endif | 452 | 454 | } else { | 453 | 455 | THROW_ERR_CRYPTO_INVALID_AUTH_TAG( | 454 | 456 | env(), "authTagLength required for %s", cipher_type); | |
Collapse file Copy file name to clipboardExpand all lines: src/crypto/crypto_context.cc +5Lines changed: 5 additions & 0 deletions | Original file line number | Diff line number | Diff line change |
|---|
@@ -1928,8 +1928,13 @@ void SecureContext::SetDHParam(const FunctionCallbackInfo<Value>& args) { | 1928 | 1928 | // true to this function instead of the original string. Any other string | 1929 | 1929 | // value will be interpreted as custom DH parameters below. | 1930 | 1930 | if (args[0]->IsTrue()) { | | 1931 | +#ifdef SSL_CTX_set_dh_auto | 1931 | 1932 | CHECK(SSL_CTX_set_dh_auto(sc->ctx_.get(), true)); | 1932 | 1933 | return; | | 1934 | +#else | | 1935 | + return THROW_ERR_CRYPTO_UNSUPPORTED_OPERATION( | | 1936 | + env, "Automatic DH parameter selection is not supported"); | | 1937 | +#endif | 1933 | 1938 | } | 1934 | 1939 |
| 1935 | 1940 | DHPointer dh; | |
Collapse file Copy file name to clipboardExpand all lines: src/crypto/crypto_dh.cc +4-2Lines changed: 4 additions & 2 deletions | Original file line number | Diff line number | Diff line change |
|---|
@@ -309,15 +309,17 @@ void ComputeSecret(const FunctionCallbackInfo<Value>& args) { | 309 | 309 | BignumPointer key(key_buf.data(), key_buf.size()); | 310 | 310 |
| 311 | 311 | switch (dh.checkPublicKey(key)) { | 312 | | - case DHPointer::CheckPublicKeyResult::INVALID: | 313 | | - // Fall-through | 314 | 312 | case DHPointer::CheckPublicKeyResult::CHECK_FAILED: | 315 | 313 | return THROW_ERR_CRYPTO_INVALID_KEYTYPE(env, | 316 | 314 | "Unspecified validation error"); | | 315 | +#ifndef OPENSSL_IS_BORINGSSL | 317 | 316 | case DHPointer::CheckPublicKeyResult::TOO_SMALL: | 318 | 317 | return THROW_ERR_CRYPTO_INVALID_KEYLEN(env, "Supplied key is too small"); | 319 | 318 | case DHPointer::CheckPublicKeyResult::TOO_LARGE: | 320 | 319 | 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"); | 321 | 323 | case DHPointer::CheckPublicKeyResult::NONE: | 322 | 324 | break; | 323 | 325 | } | |
Collapse file Copy file name to clipboardExpand all lines: src/crypto/crypto_rsa.cc +12Lines changed: 12 additions & 0 deletions | Original file line number | Diff line number | Diff line change |
|---|
@@ -135,6 +135,18 @@ Maybe<void> RsaKeyGenTraits::AdditionalConfig( | 135 | 135 | params->params.modulus_bits = args[*offset + 1].As<Uint32>()->Value(); | 136 | 136 | params->params.exponent = args[*offset + 2].As<Uint32>()->Value(); | 137 | 137 |
| | 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 | + | 138 | 150 | *offset += 3; | 139 | 151 |
| 140 | 152 | if (params->params.variant == kKeyVariantRSA_PSS) { | |
Collapse file Copy file name to clipboardExpand all lines: src/crypto/crypto_util.cc +46-39Lines changed: 46 additions & 39 deletions | Original file line number | Diff line number | Diff line change |
|---|
@@ -553,44 +553,51 @@ Maybe<void> Decorate(Environment* env, | 553 | 553 | c = ToUpper(c); | 554 | 554 | } | 555 | 555 |
| 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) | 594 | 601 |
| 595 | 602 | #define V(name) case ERR_LIB_##name: lib = #name "_"; break; | 596 | 603 | const char* lib = ""; | @@ -600,6 +607,7 @@ Maybe<void> Decorate(Environment* env, | 600 | 607 | } | 601 | 608 | #undef V | 602 | 609 | #undef OSSL_ERROR_CODES_MAP | | 610 | +#undef OSSL_ERROR_CODES_MAP_OPENSSL_ONLY | 603 | 611 | // Don't generate codes like "ERR_OSSL_SSL_". | 604 | 612 | if (lib && strcmp(lib, "SSL_") == 0) | 605 | 613 | prefix = ""; | @@ -728,7 +736,6 @@ void SecureBuffer(const FunctionCallbackInfo<Value>& args) { | 728 | 736 | uint32_t len = args[0].As<Uint32>()->Value(); | 729 | 737 |
| 730 | 738 | auto data = DataPointer::SecureAlloc(len); | 731 | | - CHECK(data.isSecure()); | 732 | 739 | if (!data) { | 733 | 740 | return THROW_ERR_OPERATION_FAILED(env, "Allocation failed"); | 734 | 741 | } | |
Collapse file 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 number | Diff line number | Diff line change |
|---|
@@ -102,17 +102,19 @@ if (!process.features.openssl_is_boringssl) { | 102 | 102 | // ECDSA w/ ieee-p1363 signature encoding | 103 | 103 | test('ec_secp256k1_public.pem', 'ec_secp256k1_private.pem', 'sha384', false, | 104 | 104 | { dsaEncoding: 'ieee-p1363' }); | 105 | | -} | 106 | 105 |
| 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' }); | 112 | 111 |
| 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 | +} | 116 | 118 |
| 117 | 119 | // Test Parallel Execution w/ KeyObject is threadsafe in openssl3 | 118 | 120 | { | |
Collapse file Copy file name to clipboardExpand all lines: test/parallel/test-crypto-authenticated.js +30-17Lines changed: 30 additions & 17 deletions | Original file line number | Diff line number | Diff line change |
|---|
@@ -626,22 +626,25 @@ for (const test of TEST_CASES) { | 626 | 626 |
| 627 | 627 | { | 628 | 628 | // 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 }; | 633 | 635 |
| 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 | + } | 642 | 645 | } | 643 | 646 |
| 644 | | -{ | | 647 | +if (!process.features.openssl_is_boringssl) { | 645 | 648 | const key = Buffer.alloc(32); | 646 | 649 | const iv = Buffer.alloc(12); | 647 | 650 |
| @@ -653,11 +656,13 @@ for (const test of TEST_CASES) { | 653 | 656 | message: errMessages.authTagLength | 654 | 657 | }); | 655 | 658 | } | | 659 | +} else { | | 660 | + common.printSkipMessage('Skipping unsupported chacha20-poly1305 test'); | 656 | 661 | } | 657 | 662 |
| 658 | 663 | // ChaCha20-Poly1305 should respect the authTagLength option and should not | 659 | 664 | // require the authentication tag before calls to update() during decryption. | 660 | | -{ | | 665 | +if (!process.features.openssl_is_boringssl) { | 661 | 666 | const key = Buffer.alloc(32); | 662 | 667 | const iv = Buffer.alloc(12); | 663 | 668 |
| @@ -697,6 +702,8 @@ for (const test of TEST_CASES) { | 697 | 702 | } | 698 | 703 | } | 699 | 704 | } | | 705 | +} else { | | 706 | + common.printSkipMessage('Skipping unsupported chacha20-poly1305 test'); | 700 | 707 | } | 701 | 708 |
| 702 | 709 | // ChaCha20-Poly1305 should default to an authTagLength of 16. When encrypting, | @@ -706,7 +713,7 @@ for (const test of TEST_CASES) { | 706 | 713 | // shorter tags as long as their length was valid according to NIST SP 800-38D. | 707 | 714 | // For ChaCha20-Poly1305, we intentionally deviate from that because there are | 708 | 715 | // no recommended or approved authentication tag lengths below 16 bytes. | 709 | | -{ | | 716 | +if (!process.features.openssl_is_boringssl) { | 710 | 717 | const rfcTestCases = TEST_CASES.filter(({ algo, tampered }) => { | 711 | 718 | return algo === 'chacha20-poly1305' && tampered === false; | 712 | 719 | }); | @@ -740,10 +747,12 @@ for (const test of TEST_CASES) { | 740 | 747 |
| 741 | 748 | assert.strictEqual(plaintext.toString('hex'), testCase.plain); | 742 | 749 | } | | 750 | +} else { | | 751 | + common.printSkipMessage('Skipping unsupported chacha20-poly1305 test'); | 743 | 752 | } | 744 | 753 |
| 745 | 754 | // https://github.com/nodejs/node/issues/45874 | 746 | | -{ | | 755 | +if (!process.features.openssl_is_boringssl) { | 747 | 756 | const rfcTestCases = TEST_CASES.filter(({ algo, tampered }) => { | 748 | 757 | return algo === 'chacha20-poly1305' && tampered === false; | 749 | 758 | }); | @@ -771,10 +780,12 @@ for (const test of TEST_CASES) { | 771 | 780 | assert.throws(() => { | 772 | 781 | decipher.final(); | 773 | 782 | }, /Unsupported state or unable to authenticate data/); | | 783 | +} else { | | 784 | + common.printSkipMessage('Skipping unsupported chacha20-poly1305 test'); | 774 | 785 | } | 775 | 786 |
| 776 | 787 | // Refs: https://github.com/nodejs/node/issues/62342 | 777 | | -{ | | 788 | +if (ciphers.includes('aes-128-ccm')) { | 778 | 789 | const key = crypto.randomBytes(16); | 779 | 790 | const nonce = crypto.randomBytes(13); | 780 | 791 |
| @@ -794,4 +805,6 @@ for (const test of TEST_CASES) { | 794 | 805 | decipher.setAAD(Buffer.alloc(0), { plaintextLength: 0 }); | 795 | 806 | decipher.update(new DataView(new ArrayBuffer(0))); | 796 | 807 | decipher.final(); | | 808 | +} else { | | 809 | + common.printSkipMessage('Skipping unsupported aes-128-ccm test'); | 797 | 810 | } |
Collapse file Copy file name to clipboardExpand all lines: test/parallel/test-crypto-cipheriv-decipheriv.js +4Lines changed: 4 additions & 0 deletions | Original file line number | Diff line number | Diff line change |
|---|
@@ -62,6 +62,10 @@ function testCipher2(key, iv) { | 62 | 62 |
| 63 | 63 |
| 64 | 64 | function testCipher3(key, iv) { | | 65 | + if (!crypto.getCiphers().includes('id-aes128-wrap')) { | | 66 | + common.printSkipMessage(`unsupported id-aes128-wrap test`); | | 67 | + return; | | 68 | + } | 65 | 69 | // Test encryption and decryption with explicit key and iv. | 66 | 70 | // AES Key Wrap test vector comes from RFC3394 | 67 | 71 | const plaintext = Buffer.from('00112233445566778899AABBCCDDEEFF', 'hex'); | |
|