Commit 137ff67

Browse files
panvaaduh95
authored andcommitted
crypto: use available BoringSSL APIs
Use BoringSSL's current RSA and DH validation results instead of maintaining backend-specific prechecks and collapsing key errors. Report negotiated TLS groups and the documented zero security level through BoringSSL's compatibility APIs. Signed-off-by: Filip Skokan <panva.ip@gmail.com> PR-URL: #65423 Backport-PR-URL: #65483 Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Tim Perry <pimterry@gmail.com>
1 parent 0e87576 commit 137ff67

10 files changed

Lines changed: 66 additions & 89 deletions

File tree

‎deps/ncrypto/ncrypto.cc‎

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2252,14 +2252,11 @@ DHPointer::CheckPublicKeyResult DHPointer::checkPublicKey(
22522252
if (DH_check_pub_key(dh_.get(), pub_key.get(), &codes) != 1) {
22532253
return DHPointer::CheckPublicKeyResult::CHECK_FAILED;
22542254
}
2255-
#ifndef OPENSSL_IS_BORINGSSL
2256-
// Boringssl does not define DH_CHECK_PUBKEY_TOO_SMALL or TOO_LARGE
22572255
if (codes & DH_CHECK_PUBKEY_TOO_SMALL) {
22582256
return DHPointer::CheckPublicKeyResult::TOO_SMALL;
22592257
} elseif (codes & DH_CHECK_PUBKEY_TOO_LARGE) {
22602258
return DHPointer::CheckPublicKeyResult::TOO_LARGE;
22612259
}
2262-
#endif
22632260
if (codes != 0) {
22642261
return DHPointer::CheckPublicKeyResult::INVALID;
22652262
}
@@ -4309,6 +4306,13 @@ std::optional<std::string_view> SSLPointer::getNegotiatedGroup() const {
43094306
constchar* group = SSL_get0_group_name(get());
43104307
if (group == nullptr) return std::nullopt;
43114308
return group;
4309+
#elif defined(OPENSSL_IS_BORINGSSL)
4310+
if (!ssl_) return std::nullopt;
4311+
constint nid = SSL_get_negotiated_group(get());
4312+
if (nid == NID_undef) return std::nullopt;
4313+
constchar* group = OBJ_nid2sn(nid);
4314+
if (group == nullptr) return std::nullopt;
4315+
return group;
43124316
#else
43134317
return std::nullopt;
43144318
#endif
@@ -4333,19 +4337,17 @@ std::optional<std::string_view> SSLPointer::getCipherVersion() const {
43334337
}
43344338

43354339
std::optional<int> SSLPointer::getSecurityLevel() {
4336-
#ifndef OPENSSL_IS_BORINGSSL
43374340
auto ctx = SSLCtxPointer::New();
43384341
if (!ctx) return std::nullopt;
43394342

4343+
#ifdef OPENSSL_IS_BORINGSSL
4344+
returnSSL_CTX_get_security_level(ctx.get());
4345+
#else
43404346
auto ssl = SSLPointer::New(ctx);
43414347
if (!ssl) return std::nullopt;
43424348

43434349
returnSSL_get_security_level(ssl);
4344-
#else
4345-
// OPENSSL_TLS_SECURITY_LEVEL is not defined in BoringSSL
4346-
// so assume it is the default OPENSSL_TLS_SECURITY_LEVEL value.
4347-
return1;
4348-
#endif// OPENSSL_IS_BORINGSSL
4350+
#endif
43494351
}
43504352

43514353
SSLCtxPointer::SSLCtxPointer(SSL_CTX* ctx) : ctx_(ctx) {}

‎deps/ncrypto/ncrypto.h‎

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1231,9 +1231,9 @@ class DHPointer final {
12311231
UNABLE_TO_CHECK_GENERATOR = 0x04,
12321232
NOT_SUITABLE_GENERATOR = 0x08,
12331233
Q_NOT_PRIME = 0x10,
1234-
#ifndef OPENSSL_IS_BORINGSSL
1235-
// Boringssl does not define the DH_CHECK_INVALID_[Q or J]_VALUE
12361234
INVALID_Q = 0x20,
1235+
#ifndef OPENSSL_IS_BORINGSSL
1236+
// BoringSSL does not define DH_CHECK_INVALID_J_VALUE.
12371237
INVALID_J = 0x40,
12381238
MODULUS_TOO_SMALL = 0x80,
12391239
MODULUS_TOO_LARGE = 0x100,
@@ -1244,14 +1244,9 @@ class DHPointer final {
12441244

12451245
enumclassCheckPublicKeyResult {
12461246
NONE,
1247-
#ifndef OPENSSL_IS_BORINGSSL
1248-
// Boringssl does not define DH_R_CHECK_PUBKEY_TOO_SMALL or TOO_LARGE
1249-
TOO_SMALL = DH_R_CHECK_PUBKEY_TOO_SMALL,
1250-
TOO_LARGE = DH_R_CHECK_PUBKEY_TOO_LARGE,
1251-
INVALID = DH_R_CHECK_PUBKEY_INVALID,
1252-
#else
1253-
INVALID = DH_R_INVALID_PUBKEY,
1254-
#endif
1247+
TOO_SMALL,
1248+
TOO_LARGE,
1249+
INVALID,
12551250
CHECK_FAILED = 512,
12561251
};
12571252
// Check to see if the given public key is suitable for this DH instance.

‎src/crypto/crypto_dh.cc‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -319,12 +319,10 @@ void ComputeSecret(const FunctionCallbackInfo<Value>& args) {
319319
case DHPointer::CheckPublicKeyResult::CHECK_FAILED:
320320
returnTHROW_ERR_CRYPTO_INVALID_KEYTYPE(env,
321321
"Unspecified validation error");
322-
#ifndef OPENSSL_IS_BORINGSSL
323322
case DHPointer::CheckPublicKeyResult::TOO_SMALL:
324323
returnTHROW_ERR_CRYPTO_INVALID_KEYLEN(env, "Supplied key is too small");
325324
case DHPointer::CheckPublicKeyResult::TOO_LARGE:
326325
returnTHROW_ERR_CRYPTO_INVALID_KEYLEN(env, "Supplied key is too large");
327-
#endif
328326
case DHPointer::CheckPublicKeyResult::INVALID:
329327
returnTHROW_ERR_CRYPTO_INVALID_KEYTYPE(env, "Supplied key is invalid");
330328
case DHPointer::CheckPublicKeyResult::NONE:

‎src/crypto/crypto_rsa.cc‎

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

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

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

‎test/common/boringssl.js‎

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -137,12 +137,9 @@ function testRenegotiationUnsupported() {
137137
}
138138

139139
/**
140-
* OpenSSL exposes the negotiated ephemeral key type, name, and size for TLS
141-
* clients. With BoringSSL the same ECDHE TLS 1.2 handshake succeeds, but
142-
* getEphemeralKeyInfo() returns null on the server side and an object whose
143-
* fields are undefined on the client side.
140+
* BoringSSL exposes the negotiated TLS group but not the ephemeral key size.
144141
*/
145-
functiontestEphemeralKeyInfoUnsupported(){
142+
functiontestEphemeralKeyInfo(){
146143
constserver=tls.createServer({
147144
key: fixtures.readKey('agent2-key.pem'),
148145
cert: fixtures.readKey('agent2-cert.pem'),
@@ -161,8 +158,8 @@ function testEphemeralKeyInfoUnsupported() {
161158
maxVersion: 'TLSv1.2',
162159
},common.mustCall(()=>{
163160
assert.deepStrictEqual(client.getEphemeralKeyInfo(),{
164-
type: undefined,
165-
name: undefined,
161+
type: 'TLSGroup',
162+
name: 'prime256v1',
166163
size: undefined,
167164
});
168165
server.close();
@@ -337,7 +334,7 @@ module.exports = {
337334
assertMultiKeyUnsupported,
338335
assertNoCipherMatch,
339336
assertOpenSSLSecurityLevelsUnsupported,
340-
testEphemeralKeyInfoUnsupported,
337+
testEphemeralKeyInfo,
341338
testLegacyProtocolUnsupported,
342339
testMultiPfxSelectionDifference,
343340
testPskTls13Unsupported,

‎test/parallel/test-crypto-dh-curves.js‎

Lines changed: 25 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -35,33 +35,33 @@ if (!process.features.openssl_is_boringssl) {
3535
assert.strictEqual(
3636
crypto.createDiffieHellman(notSafePrime,Buffer.from([2])).verifyError,
3737
DH_CHECK_P_NOT_SAFE_PRIME);
38-
39-
constgroup=crypto.getDiffieHellman('modp14');
40-
constalice=crypto.createDiffieHellman(
41-
group.getPrime(),group.getGenerator());
42-
alice.generateKeys();
43-
constgroupPrime=BigInt(`0x${group.getPrime('hex')}`);
44-
assert.throws(
45-
()=>alice.computeSecret(Buffer.from([1])),
46-
{
47-
code: 'ERR_CRYPTO_INVALID_KEYLEN',
48-
message: 'Supplied key is too small'
49-
});
50-
assert.throws(
51-
()=>alice.computeSecret(group.getPrime()),
52-
{
53-
code: 'ERR_CRYPTO_INVALID_KEYLEN',
54-
message: 'Supplied key is too large'
55-
});
56-
assert.throws(
57-
()=>alice.computeSecret(
58-
Buffer.from((groupPrime-1n).toString(16),'hex')),
59-
{
60-
code: 'ERR_CRYPTO_INVALID_KEYLEN',
61-
message: 'Supplied key is too large'
62-
});
6338
}
6439

40+
constgroup=crypto.getDiffieHellman('modp14');
41+
constalice=crypto.createDiffieHellman(
42+
group.getPrime(),group.getGenerator());
43+
alice.generateKeys();
44+
constgroupPrime=BigInt(`0x${group.getPrime('hex')}`);
45+
assert.throws(
46+
()=>alice.computeSecret(Buffer.from([1])),
47+
{
48+
code: 'ERR_CRYPTO_INVALID_KEYLEN',
49+
message: 'Supplied key is too small'
50+
});
51+
assert.throws(
52+
()=>alice.computeSecret(group.getPrime()),
53+
{
54+
code: 'ERR_CRYPTO_INVALID_KEYLEN',
55+
message: 'Supplied key is too large'
56+
});
57+
assert.throws(
58+
()=>alice.computeSecret(
59+
Buffer.from((groupPrime-1n).toString(16),'hex')),
60+
{
61+
code: 'ERR_CRYPTO_INVALID_KEYLEN',
62+
message: 'Supplied key is too large'
63+
});
64+
6565
// Confirm DH_check() results are exposed for optional examination.
6666
constbad_dh=process.features.openssl_is_boringssl ?
6767
crypto.createDiffieHellman('abcd','hex',0) :

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

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -93,9 +93,7 @@ const {
9393
{
9494
assert.throws(()=>{
9595
dh3.computeSecret('');
96-
},{message: process.features.openssl_is_boringssl ?
97-
'Supplied key is invalid' :
98-
'Supplied key is too small'});
96+
},{message: 'Supplied key is too small'});
9997
}
10098
}
10199

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

Lines changed: 13 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -376,25 +376,20 @@ const isBoringSSL = process.features.openssl_is_boringssl;
376376
}
377377

378378
// Test invalid exponents. (caught by OpenSSL)
379+
letinvalidExponentError=/badevalue/;
380+
if(isBoringSSL){
381+
invalidExponentError=/BAD_E_VALUE/;
382+
}elseif(hasOpenSSL3){
383+
invalidExponentError=/exponent/;
384+
}
379385
for(constpublicExponentof[1,1+0x10001]){
380-
if(isBoringSSL){
381-
assert.throws(()=>generateKeyPair('rsa',{
382-
modulusLength: 4096,
383-
publicExponent
384-
},common.mustNotCall()),{
385-
name: 'RangeError',
386-
code: 'ERR_OUT_OF_RANGE',
387-
message: 'publicExponent is invalid',
388-
});
389-
}else{
390-
generateKeyPair('rsa',{
391-
modulusLength: 4096,
392-
publicExponent
393-
},common.mustCall((err)=>{
394-
assert.strictEqual(err.name,'Error');
395-
assert.match(err.message,hasOpenSSL3 ? /exponent/ : /badevalue/);
396-
}));
397-
}
386+
generateKeyPair('rsa',{
387+
modulusLength: 4096,
388+
publicExponent
389+
},common.mustCall((err)=>{
390+
assert.strictEqual(err.name,'Error');
391+
assert.match(err.message,invalidExponentError);
392+
}));
398393
}
399394
}
400395

‎test/parallel/test-crypto-sec-level.js‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,4 +15,8 @@ const assert = require('assert');
1515
// This test simply validates that we can get some value for the secLevel
1616
// when needed by tests.
1717
constsecLevel=require('internal/crypto/util').getOpenSSLSecLevel();
18-
assert.ok(secLevel>=0&&secLevel<=5);
18+
if(process.features.openssl_is_boringssl){
19+
assert.strictEqual(secLevel,0);
20+
}else{
21+
assert.ok(secLevel>=0&&secLevel<=5);
22+
}

‎test/parallel/test-tls-client-getephemeralkeyinfo.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ if (!common.hasCrypto)
44
common.skip('missing crypto');
55

66
if(process.features.openssl_is_boringssl){
7-
require('../common/boringssl').testEphemeralKeyInfoUnsupported();
7+
require('../common/boringssl').testEphemeralKeyInfo();
88
return;
99
}
1010

0 commit comments

Comments
 (0)
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Skip to content

Commit 137ff67

Browse files
panvaaduh95
authored andcommitted
crypto: use available BoringSSL APIs
Use BoringSSL's current RSA and DH validation results instead of maintaining backend-specific prechecks and collapsing key errors. Report negotiated TLS groups and the documented zero security level through BoringSSL's compatibility APIs. Signed-off-by: Filip Skokan <panva.ip@gmail.com> PR-URL: #65423 Backport-PR-URL: #65483 Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Tim Perry <pimterry@gmail.com>
1 parent 0e87576 commit 137ff67

10 files changed

Lines changed: 66 additions & 89 deletions

File tree

‎deps/ncrypto/ncrypto.cc‎

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2252,14 +2252,11 @@ DHPointer::CheckPublicKeyResult DHPointer::checkPublicKey(
22522252
if (DH_check_pub_key(dh_.get(), pub_key.get(), &codes) != 1) {
22532253
return DHPointer::CheckPublicKeyResult::CHECK_FAILED;
22542254
}
2255-
#ifndef OPENSSL_IS_BORINGSSL
2256-
// Boringssl does not define DH_CHECK_PUBKEY_TOO_SMALL or TOO_LARGE
22572255
if (codes & DH_CHECK_PUBKEY_TOO_SMALL) {
22582256
return DHPointer::CheckPublicKeyResult::TOO_SMALL;
22592257
} elseif (codes & DH_CHECK_PUBKEY_TOO_LARGE) {
22602258
return DHPointer::CheckPublicKeyResult::TOO_LARGE;
22612259
}
2262-
#endif
22632260
if (codes != 0) {
22642261
return DHPointer::CheckPublicKeyResult::INVALID;
22652262
}
@@ -4309,6 +4306,13 @@ std::optional<std::string_view> SSLPointer::getNegotiatedGroup() const {
43094306
constchar* group = SSL_get0_group_name(get());
43104307
if (group == nullptr) return std::nullopt;
43114308
return group;
4309+
#elif defined(OPENSSL_IS_BORINGSSL)
4310+
if (!ssl_) return std::nullopt;
4311+
constint nid = SSL_get_negotiated_group(get());
4312+
if (nid == NID_undef) return std::nullopt;
4313+
constchar* group = OBJ_nid2sn(nid);
4314+
if (group == nullptr) return std::nullopt;
4315+
return group;
43124316
#else
43134317
return std::nullopt;
43144318
#endif
@@ -4333,19 +4337,17 @@ std::optional<std::string_view> SSLPointer::getCipherVersion() const {
43334337
}
43344338

43354339
std::optional<int> SSLPointer::getSecurityLevel() {
4336-
#ifndef OPENSSL_IS_BORINGSSL
43374340
auto ctx = SSLCtxPointer::New();
43384341
if (!ctx) return std::nullopt;
43394342

4343+
#ifdef OPENSSL_IS_BORINGSSL
4344+
returnSSL_CTX_get_security_level(ctx.get());
4345+
#else
43404346
auto ssl = SSLPointer::New(ctx);
43414347
if (!ssl) return std::nullopt;
43424348

43434349
returnSSL_get_security_level(ssl);
4344-
#else
4345-
// OPENSSL_TLS_SECURITY_LEVEL is not defined in BoringSSL
4346-
// so assume it is the default OPENSSL_TLS_SECURITY_LEVEL value.
4347-
return1;
4348-
#endif// OPENSSL_IS_BORINGSSL
4350+
#endif
43494351
}
43504352

43514353
SSLCtxPointer::SSLCtxPointer(SSL_CTX* ctx) : ctx_(ctx) {}

‎deps/ncrypto/ncrypto.h‎

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1231,9 +1231,9 @@ class DHPointer final {
12311231
UNABLE_TO_CHECK_GENERATOR = 0x04,
12321232
NOT_SUITABLE_GENERATOR = 0x08,
12331233
Q_NOT_PRIME = 0x10,
1234-
#ifndef OPENSSL_IS_BORINGSSL
1235-
// Boringssl does not define the DH_CHECK_INVALID_[Q or J]_VALUE
12361234
INVALID_Q = 0x20,
1235+
#ifndef OPENSSL_IS_BORINGSSL
1236+
// BoringSSL does not define DH_CHECK_INVALID_J_VALUE.
12371237
INVALID_J = 0x40,
12381238
MODULUS_TOO_SMALL = 0x80,
12391239
MODULUS_TOO_LARGE = 0x100,
@@ -1244,14 +1244,9 @@ class DHPointer final {
12441244

12451245
enumclassCheckPublicKeyResult {
12461246
NONE,
1247-
#ifndef OPENSSL_IS_BORINGSSL
1248-
// Boringssl does not define DH_R_CHECK_PUBKEY_TOO_SMALL or TOO_LARGE
1249-
TOO_SMALL = DH_R_CHECK_PUBKEY_TOO_SMALL,
1250-
TOO_LARGE = DH_R_CHECK_PUBKEY_TOO_LARGE,
1251-
INVALID = DH_R_CHECK_PUBKEY_INVALID,
1252-
#else
1253-
INVALID = DH_R_INVALID_PUBKEY,
1254-
#endif
1247+
TOO_SMALL,
1248+
TOO_LARGE,
1249+
INVALID,
12551250
CHECK_FAILED = 512,
12561251
};
12571252
// Check to see if the given public key is suitable for this DH instance.

‎src/crypto/crypto_dh.cc‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -319,12 +319,10 @@ void ComputeSecret(const FunctionCallbackInfo<Value>& args) {
319319
case DHPointer::CheckPublicKeyResult::CHECK_FAILED:
320320
returnTHROW_ERR_CRYPTO_INVALID_KEYTYPE(env,
321321
"Unspecified validation error");
322-
#ifndef OPENSSL_IS_BORINGSSL
323322
case DHPointer::CheckPublicKeyResult::TOO_SMALL:
324323
returnTHROW_ERR_CRYPTO_INVALID_KEYLEN(env, "Supplied key is too small");
325324
case DHPointer::CheckPublicKeyResult::TOO_LARGE:
326325
returnTHROW_ERR_CRYPTO_INVALID_KEYLEN(env, "Supplied key is too large");
327-
#endif
328326
case DHPointer::CheckPublicKeyResult::INVALID:
329327
returnTHROW_ERR_CRYPTO_INVALID_KEYTYPE(env, "Supplied key is invalid");
330328
case DHPointer::CheckPublicKeyResult::NONE:

‎src/crypto/crypto_rsa.cc‎

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

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

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

‎test/common/boringssl.js‎

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -137,12 +137,9 @@ function testRenegotiationUnsupported() {
137137
}
138138

139139
/**
140-
* OpenSSL exposes the negotiated ephemeral key type, name, and size for TLS
141-
* clients. With BoringSSL the same ECDHE TLS 1.2 handshake succeeds, but
142-
* getEphemeralKeyInfo() returns null on the server side and an object whose
143-
* fields are undefined on the client side.
140+
* BoringSSL exposes the negotiated TLS group but not the ephemeral key size.
144141
*/
145-
functiontestEphemeralKeyInfoUnsupported(){
142+
functiontestEphemeralKeyInfo(){
146143
constserver=tls.createServer({
147144
key: fixtures.readKey('agent2-key.pem'),
148145
cert: fixtures.readKey('agent2-cert.pem'),
@@ -161,8 +158,8 @@ function testEphemeralKeyInfoUnsupported() {
161158
maxVersion: 'TLSv1.2',
162159
},common.mustCall(()=>{
163160
assert.deepStrictEqual(client.getEphemeralKeyInfo(),{
164-
type: undefined,
165-
name: undefined,
161+
type: 'TLSGroup',
162+
name: 'prime256v1',
166163
size: undefined,
167164
});
168165
server.close();
@@ -337,7 +334,7 @@ module.exports = {
337334
assertMultiKeyUnsupported,
338335
assertNoCipherMatch,
339336
assertOpenSSLSecurityLevelsUnsupported,
340-
testEphemeralKeyInfoUnsupported,
337+
testEphemeralKeyInfo,
341338
testLegacyProtocolUnsupported,
342339
testMultiPfxSelectionDifference,
343340
testPskTls13Unsupported,

‎test/parallel/test-crypto-dh-curves.js‎

Lines changed: 25 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -35,33 +35,33 @@ if (!process.features.openssl_is_boringssl) {
3535
assert.strictEqual(
3636
crypto.createDiffieHellman(notSafePrime,Buffer.from([2])).verifyError,
3737
DH_CHECK_P_NOT_SAFE_PRIME);
38-
39-
constgroup=crypto.getDiffieHellman('modp14');
40-
constalice=crypto.createDiffieHellman(
41-
group.getPrime(),group.getGenerator());
42-
alice.generateKeys();
43-
constgroupPrime=BigInt(`0x${group.getPrime('hex')}`);
44-
assert.throws(
45-
()=>alice.computeSecret(Buffer.from([1])),
46-
{
47-
code: 'ERR_CRYPTO_INVALID_KEYLEN',
48-
message: 'Supplied key is too small'
49-
});
50-
assert.throws(
51-
()=>alice.computeSecret(group.getPrime()),
52-
{
53-
code: 'ERR_CRYPTO_INVALID_KEYLEN',
54-
message: 'Supplied key is too large'
55-
});
56-
assert.throws(
57-
()=>alice.computeSecret(
58-
Buffer.from((groupPrime-1n).toString(16),'hex')),
59-
{
60-
code: 'ERR_CRYPTO_INVALID_KEYLEN',
61-
message: 'Supplied key is too large'
62-
});
6338
}
6439

40+
constgroup=crypto.getDiffieHellman('modp14');
41+
constalice=crypto.createDiffieHellman(
42+
group.getPrime(),group.getGenerator());
43+
alice.generateKeys();
44+
constgroupPrime=BigInt(`0x${group.getPrime('hex')}`);
45+
assert.throws(
46+
()=>alice.computeSecret(Buffer.from([1])),
47+
{
48+
code: 'ERR_CRYPTO_INVALID_KEYLEN',
49+
message: 'Supplied key is too small'
50+
});
51+
assert.throws(
52+
()=>alice.computeSecret(group.getPrime()),
53+
{
54+
code: 'ERR_CRYPTO_INVALID_KEYLEN',
55+
message: 'Supplied key is too large'
56+
});
57+
assert.throws(
58+
()=>alice.computeSecret(
59+
Buffer.from((groupPrime-1n).toString(16),'hex')),
60+
{
61+
code: 'ERR_CRYPTO_INVALID_KEYLEN',
62+
message: 'Supplied key is too large'
63+
});
64+
6565
// Confirm DH_check() results are exposed for optional examination.
6666
constbad_dh=process.features.openssl_is_boringssl ?
6767
crypto.createDiffieHellman('abcd','hex',0) :

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

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -93,9 +93,7 @@ const {
9393
{
9494
assert.throws(()=>{
9595
dh3.computeSecret('');
96-
},{message: process.features.openssl_is_boringssl ?
97-
'Supplied key is invalid' :
98-
'Supplied key is too small'});
96+
},{message: 'Supplied key is too small'});
9997
}
10098
}
10199

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

Lines changed: 13 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -376,25 +376,20 @@ const isBoringSSL = process.features.openssl_is_boringssl;
376376
}
377377

378378
// Test invalid exponents. (caught by OpenSSL)
379+
letinvalidExponentError=/badevalue/;
380+
if(isBoringSSL){
381+
invalidExponentError=/BAD_E_VALUE/;
382+
}elseif(hasOpenSSL3){
383+
invalidExponentError=/exponent/;
384+
}
379385
for(constpublicExponentof[1,1+0x10001]){
380-
if(isBoringSSL){
381-
assert.throws(()=>generateKeyPair('rsa',{
382-
modulusLength: 4096,
383-
publicExponent
384-
},common.mustNotCall()),{
385-
name: 'RangeError',
386-
code: 'ERR_OUT_OF_RANGE',
387-
message: 'publicExponent is invalid',
388-
});
389-
}else{
390-
generateKeyPair('rsa',{
391-
modulusLength: 4096,
392-
publicExponent
393-
},common.mustCall((err)=>{
394-
assert.strictEqual(err.name,'Error');
395-
assert.match(err.message,hasOpenSSL3 ? /exponent/ : /badevalue/);
396-
}));
397-
}
386+
generateKeyPair('rsa',{
387+
modulusLength: 4096,
388+
publicExponent
389+
},common.mustCall((err)=>{
390+
assert.strictEqual(err.name,'Error');
391+
assert.match(err.message,invalidExponentError);
392+
}));
398393
}
399394
}
400395

‎test/parallel/test-crypto-sec-level.js‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,4 +15,8 @@ const assert = require('assert');
1515
// This test simply validates that we can get some value for the secLevel
1616
// when needed by tests.
1717
constsecLevel=require('internal/crypto/util').getOpenSSLSecLevel();
18-
assert.ok(secLevel>=0&&secLevel<=5);
18+
if(process.features.openssl_is_boringssl){
19+
assert.strictEqual(secLevel,0);
20+
}else{
21+
assert.ok(secLevel>=0&&secLevel<=5);
22+
}

‎test/parallel/test-tls-client-getephemeralkeyinfo.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ if (!common.hasCrypto)
44
common.skip('missing crypto');
55

66
if(process.features.openssl_is_boringssl){
7-
require('../common/boringssl').testEphemeralKeyInfoUnsupported();
7+
require('../common/boringssl').testEphemeralKeyInfo();
88
return;
99
}
1010

0 commit comments

Comments
 (0)
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix"); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Commit 137ff67

Browse files
panvaaduh95
authored andcommitted
crypto: use available BoringSSL APIs
Use BoringSSL's current RSA and DH validation results instead of maintaining backend-specific prechecks and collapsing key errors. Report negotiated TLS groups and the documented zero security level through BoringSSL's compatibility APIs. Signed-off-by: Filip Skokan <panva.ip@gmail.com> PR-URL: #65423 Backport-PR-URL: #65483 Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Tim Perry <pimterry@gmail.com>
1 parent 0e87576 commit 137ff67

10 files changed

Lines changed: 66 additions & 89 deletions

File tree

‎deps/ncrypto/ncrypto.cc‎

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2252,14 +2252,11 @@ DHPointer::CheckPublicKeyResult DHPointer::checkPublicKey(
22522252
if (DH_check_pub_key(dh_.get(), pub_key.get(), &codes) != 1) {
22532253
return DHPointer::CheckPublicKeyResult::CHECK_FAILED;
22542254
}
2255-
#ifndef OPENSSL_IS_BORINGSSL
2256-
// Boringssl does not define DH_CHECK_PUBKEY_TOO_SMALL or TOO_LARGE
22572255
if (codes & DH_CHECK_PUBKEY_TOO_SMALL) {
22582256
return DHPointer::CheckPublicKeyResult::TOO_SMALL;
22592257
} elseif (codes & DH_CHECK_PUBKEY_TOO_LARGE) {
22602258
return DHPointer::CheckPublicKeyResult::TOO_LARGE;
22612259
}
2262-
#endif
22632260
if (codes != 0) {
22642261
return DHPointer::CheckPublicKeyResult::INVALID;
22652262
}
@@ -4309,6 +4306,13 @@ std::optional<std::string_view> SSLPointer::getNegotiatedGroup() const {
43094306
constchar* group = SSL_get0_group_name(get());
43104307
if (group == nullptr) return std::nullopt;
43114308
return group;
4309+
#elif defined(OPENSSL_IS_BORINGSSL)
4310+
if (!ssl_) return std::nullopt;
4311+
constint nid = SSL_get_negotiated_group(get());
4312+
if (nid == NID_undef) return std::nullopt;
4313+
constchar* group = OBJ_nid2sn(nid);
4314+
if (group == nullptr) return std::nullopt;
4315+
return group;
43124316
#else
43134317
return std::nullopt;
43144318
#endif
@@ -4333,19 +4337,17 @@ std::optional<std::string_view> SSLPointer::getCipherVersion() const {
43334337
}
43344338

43354339
std::optional<int> SSLPointer::getSecurityLevel() {
4336-
#ifndef OPENSSL_IS_BORINGSSL
43374340
auto ctx = SSLCtxPointer::New();
43384341
if (!ctx) return std::nullopt;
43394342

4343+
#ifdef OPENSSL_IS_BORINGSSL
4344+
returnSSL_CTX_get_security_level(ctx.get());
4345+
#else
43404346
auto ssl = SSLPointer::New(ctx);
43414347
if (!ssl) return std::nullopt;
43424348

43434349
returnSSL_get_security_level(ssl);
4344-
#else
4345-
// OPENSSL_TLS_SECURITY_LEVEL is not defined in BoringSSL
4346-
// so assume it is the default OPENSSL_TLS_SECURITY_LEVEL value.
4347-
return1;
4348-
#endif// OPENSSL_IS_BORINGSSL
4350+
#endif
43494351
}
43504352

43514353
SSLCtxPointer::SSLCtxPointer(SSL_CTX* ctx) : ctx_(ctx) {}

‎deps/ncrypto/ncrypto.h‎

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1231,9 +1231,9 @@ class DHPointer final {
12311231
UNABLE_TO_CHECK_GENERATOR = 0x04,
12321232
NOT_SUITABLE_GENERATOR = 0x08,
12331233
Q_NOT_PRIME = 0x10,
1234-
#ifndef OPENSSL_IS_BORINGSSL
1235-
// Boringssl does not define the DH_CHECK_INVALID_[Q or J]_VALUE
12361234
INVALID_Q = 0x20,
1235+
#ifndef OPENSSL_IS_BORINGSSL
1236+
// BoringSSL does not define DH_CHECK_INVALID_J_VALUE.
12371237
INVALID_J = 0x40,
12381238
MODULUS_TOO_SMALL = 0x80,
12391239
MODULUS_TOO_LARGE = 0x100,
@@ -1244,14 +1244,9 @@ class DHPointer final {
12441244

12451245
enumclassCheckPublicKeyResult {
12461246
NONE,
1247-
#ifndef OPENSSL_IS_BORINGSSL
1248-
// Boringssl does not define DH_R_CHECK_PUBKEY_TOO_SMALL or TOO_LARGE
1249-
TOO_SMALL = DH_R_CHECK_PUBKEY_TOO_SMALL,
1250-
TOO_LARGE = DH_R_CHECK_PUBKEY_TOO_LARGE,
1251-
INVALID = DH_R_CHECK_PUBKEY_INVALID,
1252-
#else
1253-
INVALID = DH_R_INVALID_PUBKEY,
1254-
#endif
1247+
TOO_SMALL,
1248+
TOO_LARGE,
1249+
INVALID,
12551250
CHECK_FAILED = 512,
12561251
};
12571252
// Check to see if the given public key is suitable for this DH instance.

‎src/crypto/crypto_dh.cc‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -319,12 +319,10 @@ void ComputeSecret(const FunctionCallbackInfo<Value>& args) {
319319
case DHPointer::CheckPublicKeyResult::CHECK_FAILED:
320320
returnTHROW_ERR_CRYPTO_INVALID_KEYTYPE(env,
321321
"Unspecified validation error");
322-
#ifndef OPENSSL_IS_BORINGSSL
323322
case DHPointer::CheckPublicKeyResult::TOO_SMALL:
324323
returnTHROW_ERR_CRYPTO_INVALID_KEYLEN(env, "Supplied key is too small");
325324
case DHPointer::CheckPublicKeyResult::TOO_LARGE:
326325
returnTHROW_ERR_CRYPTO_INVALID_KEYLEN(env, "Supplied key is too large");
327-
#endif
328326
case DHPointer::CheckPublicKeyResult::INVALID:
329327
returnTHROW_ERR_CRYPTO_INVALID_KEYTYPE(env, "Supplied key is invalid");
330328
case DHPointer::CheckPublicKeyResult::NONE:

‎src/crypto/crypto_rsa.cc‎

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

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

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

‎test/common/boringssl.js‎

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -137,12 +137,9 @@ function testRenegotiationUnsupported() {
137137
}
138138

139139
/**
140-
* OpenSSL exposes the negotiated ephemeral key type, name, and size for TLS
141-
* clients. With BoringSSL the same ECDHE TLS 1.2 handshake succeeds, but
142-
* getEphemeralKeyInfo() returns null on the server side and an object whose
143-
* fields are undefined on the client side.
140+
* BoringSSL exposes the negotiated TLS group but not the ephemeral key size.
144141
*/
145-
functiontestEphemeralKeyInfoUnsupported(){
142+
functiontestEphemeralKeyInfo(){
146143
constserver=tls.createServer({
147144
key: fixtures.readKey('agent2-key.pem'),
148145
cert: fixtures.readKey('agent2-cert.pem'),
@@ -161,8 +158,8 @@ function testEphemeralKeyInfoUnsupported() {
161158
maxVersion: 'TLSv1.2',
162159
},common.mustCall(()=>{
163160
assert.deepStrictEqual(client.getEphemeralKeyInfo(),{
164-
type: undefined,
165-
name: undefined,
161+
type: 'TLSGroup',
162+
name: 'prime256v1',
166163
size: undefined,
167164
});
168165
server.close();
@@ -337,7 +334,7 @@ module.exports = {
337334
assertMultiKeyUnsupported,
338335
assertNoCipherMatch,
339336
assertOpenSSLSecurityLevelsUnsupported,
340-
testEphemeralKeyInfoUnsupported,
337+
testEphemeralKeyInfo,
341338
testLegacyProtocolUnsupported,
342339
testMultiPfxSelectionDifference,
343340
testPskTls13Unsupported,

‎test/parallel/test-crypto-dh-curves.js‎

Lines changed: 25 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -35,33 +35,33 @@ if (!process.features.openssl_is_boringssl) {
3535
assert.strictEqual(
3636
crypto.createDiffieHellman(notSafePrime,Buffer.from([2])).verifyError,
3737
DH_CHECK_P_NOT_SAFE_PRIME);
38-
39-
constgroup=crypto.getDiffieHellman('modp14');
40-
constalice=crypto.createDiffieHellman(
41-
group.getPrime(),group.getGenerator());
42-
alice.generateKeys();
43-
constgroupPrime=BigInt(`0x${group.getPrime('hex')}`);
44-
assert.throws(
45-
()=>alice.computeSecret(Buffer.from([1])),
46-
{
47-
code: 'ERR_CRYPTO_INVALID_KEYLEN',
48-
message: 'Supplied key is too small'
49-
});
50-
assert.throws(
51-
()=>alice.computeSecret(group.getPrime()),
52-
{
53-
code: 'ERR_CRYPTO_INVALID_KEYLEN',
54-
message: 'Supplied key is too large'
55-
});
56-
assert.throws(
57-
()=>alice.computeSecret(
58-
Buffer.from((groupPrime-1n).toString(16),'hex')),
59-
{
60-
code: 'ERR_CRYPTO_INVALID_KEYLEN',
61-
message: 'Supplied key is too large'
62-
});
6338
}
6439

40+
constgroup=crypto.getDiffieHellman('modp14');
41+
constalice=crypto.createDiffieHellman(
42+
group.getPrime(),group.getGenerator());
43+
alice.generateKeys();
44+
constgroupPrime=BigInt(`0x${group.getPrime('hex')}`);
45+
assert.throws(
46+
()=>alice.computeSecret(Buffer.from([1])),
47+
{
48+
code: 'ERR_CRYPTO_INVALID_KEYLEN',
49+
message: 'Supplied key is too small'
50+
});
51+
assert.throws(
52+
()=>alice.computeSecret(group.getPrime()),
53+
{
54+
code: 'ERR_CRYPTO_INVALID_KEYLEN',
55+
message: 'Supplied key is too large'
56+
});
57+
assert.throws(
58+
()=>alice.computeSecret(
59+
Buffer.from((groupPrime-1n).toString(16),'hex')),
60+
{
61+
code: 'ERR_CRYPTO_INVALID_KEYLEN',
62+
message: 'Supplied key is too large'
63+
});
64+
6565
// Confirm DH_check() results are exposed for optional examination.
6666
constbad_dh=process.features.openssl_is_boringssl ?
6767
crypto.createDiffieHellman('abcd','hex',0) :

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

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -93,9 +93,7 @@ const {
9393
{
9494
assert.throws(()=>{
9595
dh3.computeSecret('');
96-
},{message: process.features.openssl_is_boringssl ?
97-
'Supplied key is invalid' :
98-
'Supplied key is too small'});
96+
},{message: 'Supplied key is too small'});
9997
}
10098
}
10199

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

Lines changed: 13 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -376,25 +376,20 @@ const isBoringSSL = process.features.openssl_is_boringssl;
376376
}
377377

378378
// Test invalid exponents. (caught by OpenSSL)
379+
letinvalidExponentError=/badevalue/;
380+
if(isBoringSSL){
381+
invalidExponentError=/BAD_E_VALUE/;
382+
}elseif(hasOpenSSL3){
383+
invalidExponentError=/exponent/;
384+
}
379385
for(constpublicExponentof[1,1+0x10001]){
380-
if(isBoringSSL){
381-
assert.throws(()=>generateKeyPair('rsa',{
382-
modulusLength: 4096,
383-
publicExponent
384-
},common.mustNotCall()),{
385-
name: 'RangeError',
386-
code: 'ERR_OUT_OF_RANGE',
387-
message: 'publicExponent is invalid',
388-
});
389-
}else{
390-
generateKeyPair('rsa',{
391-
modulusLength: 4096,
392-
publicExponent
393-
},common.mustCall((err)=>{
394-
assert.strictEqual(err.name,'Error');
395-
assert.match(err.message,hasOpenSSL3 ? /exponent/ : /badevalue/);
396-
}));
397-
}
386+
generateKeyPair('rsa',{
387+
modulusLength: 4096,
388+
publicExponent
389+
},common.mustCall((err)=>{
390+
assert.strictEqual(err.name,'Error');
391+
assert.match(err.message,invalidExponentError);
392+
}));
398393
}
399394
}
400395

‎test/parallel/test-crypto-sec-level.js‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,4 +15,8 @@ const assert = require('assert');
1515
// This test simply validates that we can get some value for the secLevel
1616
// when needed by tests.
1717
constsecLevel=require('internal/crypto/util').getOpenSSLSecLevel();
18-
assert.ok(secLevel>=0&&secLevel<=5);
18+
if(process.features.openssl_is_boringssl){
19+
assert.strictEqual(secLevel,0);
20+
}else{
21+
assert.ok(secLevel>=0&&secLevel<=5);
22+
}

‎test/parallel/test-tls-client-getephemeralkeyinfo.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ if (!common.hasCrypto)
44
common.skip('missing crypto');
55

66
if(process.features.openssl_is_boringssl){
7-
require('../common/boringssl').testEphemeralKeyInfoUnsupported();
7+
require('../common/boringssl').testEphemeralKeyInfo();
88
return;
99
}
1010

0 commit comments

Comments
 (0)
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms"); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Commit 137ff67

Browse files
panvaaduh95
authored andcommitted
crypto: use available BoringSSL APIs
Use BoringSSL's current RSA and DH validation results instead of maintaining backend-specific prechecks and collapsing key errors. Report negotiated TLS groups and the documented zero security level through BoringSSL's compatibility APIs. Signed-off-by: Filip Skokan <panva.ip@gmail.com> PR-URL: #65423 Backport-PR-URL: #65483 Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Tim Perry <pimterry@gmail.com>
1 parent 0e87576 commit 137ff67

10 files changed

Lines changed: 66 additions & 89 deletions

File tree

‎deps/ncrypto/ncrypto.cc‎

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2252,14 +2252,11 @@ DHPointer::CheckPublicKeyResult DHPointer::checkPublicKey(
22522252
if (DH_check_pub_key(dh_.get(), pub_key.get(), &codes) != 1) {
22532253
return DHPointer::CheckPublicKeyResult::CHECK_FAILED;
22542254
}
2255-
#ifndef OPENSSL_IS_BORINGSSL
2256-
// Boringssl does not define DH_CHECK_PUBKEY_TOO_SMALL or TOO_LARGE
22572255
if (codes & DH_CHECK_PUBKEY_TOO_SMALL) {
22582256
return DHPointer::CheckPublicKeyResult::TOO_SMALL;
22592257
} elseif (codes & DH_CHECK_PUBKEY_TOO_LARGE) {
22602258
return DHPointer::CheckPublicKeyResult::TOO_LARGE;
22612259
}
2262-
#endif
22632260
if (codes != 0) {
22642261
return DHPointer::CheckPublicKeyResult::INVALID;
22652262
}
@@ -4309,6 +4306,13 @@ std::optional<std::string_view> SSLPointer::getNegotiatedGroup() const {
43094306
constchar* group = SSL_get0_group_name(get());
43104307
if (group == nullptr) return std::nullopt;
43114308
return group;
4309+
#elif defined(OPENSSL_IS_BORINGSSL)
4310+
if (!ssl_) return std::nullopt;
4311+
constint nid = SSL_get_negotiated_group(get());
4312+
if (nid == NID_undef) return std::nullopt;
4313+
constchar* group = OBJ_nid2sn(nid);
4314+
if (group == nullptr) return std::nullopt;
4315+
return group;
43124316
#else
43134317
return std::nullopt;
43144318
#endif
@@ -4333,19 +4337,17 @@ std::optional<std::string_view> SSLPointer::getCipherVersion() const {
43334337
}
43344338

43354339
std::optional<int> SSLPointer::getSecurityLevel() {
4336-
#ifndef OPENSSL_IS_BORINGSSL
43374340
auto ctx = SSLCtxPointer::New();
43384341
if (!ctx) return std::nullopt;
43394342

4343+
#ifdef OPENSSL_IS_BORINGSSL
4344+
returnSSL_CTX_get_security_level(ctx.get());
4345+
#else
43404346
auto ssl = SSLPointer::New(ctx);
43414347
if (!ssl) return std::nullopt;
43424348

43434349
returnSSL_get_security_level(ssl);
4344-
#else
4345-
// OPENSSL_TLS_SECURITY_LEVEL is not defined in BoringSSL
4346-
// so assume it is the default OPENSSL_TLS_SECURITY_LEVEL value.
4347-
return1;
4348-
#endif// OPENSSL_IS_BORINGSSL
4350+
#endif
43494351
}
43504352

43514353
SSLCtxPointer::SSLCtxPointer(SSL_CTX* ctx) : ctx_(ctx) {}

‎deps/ncrypto/ncrypto.h‎

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1231,9 +1231,9 @@ class DHPointer final {
12311231
UNABLE_TO_CHECK_GENERATOR = 0x04,
12321232
NOT_SUITABLE_GENERATOR = 0x08,
12331233
Q_NOT_PRIME = 0x10,
1234-
#ifndef OPENSSL_IS_BORINGSSL
1235-
// Boringssl does not define the DH_CHECK_INVALID_[Q or J]_VALUE
12361234
INVALID_Q = 0x20,
1235+
#ifndef OPENSSL_IS_BORINGSSL
1236+
// BoringSSL does not define DH_CHECK_INVALID_J_VALUE.
12371237
INVALID_J = 0x40,
12381238
MODULUS_TOO_SMALL = 0x80,
12391239
MODULUS_TOO_LARGE = 0x100,
@@ -1244,14 +1244,9 @@ class DHPointer final {
12441244

12451245
enumclassCheckPublicKeyResult {
12461246
NONE,
1247-
#ifndef OPENSSL_IS_BORINGSSL
1248-
// Boringssl does not define DH_R_CHECK_PUBKEY_TOO_SMALL or TOO_LARGE
1249-
TOO_SMALL = DH_R_CHECK_PUBKEY_TOO_SMALL,
1250-
TOO_LARGE = DH_R_CHECK_PUBKEY_TOO_LARGE,
1251-
INVALID = DH_R_CHECK_PUBKEY_INVALID,
1252-
#else
1253-
INVALID = DH_R_INVALID_PUBKEY,
1254-
#endif
1247+
TOO_SMALL,
1248+
TOO_LARGE,
1249+
INVALID,
12551250
CHECK_FAILED = 512,
12561251
};
12571252
// Check to see if the given public key is suitable for this DH instance.

‎src/crypto/crypto_dh.cc‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -319,12 +319,10 @@ void ComputeSecret(const FunctionCallbackInfo<Value>& args) {
319319
case DHPointer::CheckPublicKeyResult::CHECK_FAILED:
320320
returnTHROW_ERR_CRYPTO_INVALID_KEYTYPE(env,
321321
"Unspecified validation error");
322-
#ifndef OPENSSL_IS_BORINGSSL
323322
case DHPointer::CheckPublicKeyResult::TOO_SMALL:
324323
returnTHROW_ERR_CRYPTO_INVALID_KEYLEN(env, "Supplied key is too small");
325324
case DHPointer::CheckPublicKeyResult::TOO_LARGE:
326325
returnTHROW_ERR_CRYPTO_INVALID_KEYLEN(env, "Supplied key is too large");
327-
#endif
328326
case DHPointer::CheckPublicKeyResult::INVALID:
329327
returnTHROW_ERR_CRYPTO_INVALID_KEYTYPE(env, "Supplied key is invalid");
330328
case DHPointer::CheckPublicKeyResult::NONE:

‎src/crypto/crypto_rsa.cc‎

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

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

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

‎test/common/boringssl.js‎

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -137,12 +137,9 @@ function testRenegotiationUnsupported() {
137137
}
138138

139139
/**
140-
* OpenSSL exposes the negotiated ephemeral key type, name, and size for TLS
141-
* clients. With BoringSSL the same ECDHE TLS 1.2 handshake succeeds, but
142-
* getEphemeralKeyInfo() returns null on the server side and an object whose
143-
* fields are undefined on the client side.
140+
* BoringSSL exposes the negotiated TLS group but not the ephemeral key size.
144141
*/
145-
functiontestEphemeralKeyInfoUnsupported(){
142+
functiontestEphemeralKeyInfo(){
146143
constserver=tls.createServer({
147144
key: fixtures.readKey('agent2-key.pem'),
148145
cert: fixtures.readKey('agent2-cert.pem'),
@@ -161,8 +158,8 @@ function testEphemeralKeyInfoUnsupported() {
161158
maxVersion: 'TLSv1.2',
162159
},common.mustCall(()=>{
163160
assert.deepStrictEqual(client.getEphemeralKeyInfo(),{
164-
type: undefined,
165-
name: undefined,
161+
type: 'TLSGroup',
162+
name: 'prime256v1',
166163
size: undefined,
167164
});
168165
server.close();
@@ -337,7 +334,7 @@ module.exports = {
337334
assertMultiKeyUnsupported,
338335
assertNoCipherMatch,
339336
assertOpenSSLSecurityLevelsUnsupported,
340-
testEphemeralKeyInfoUnsupported,
337+
testEphemeralKeyInfo,
341338
testLegacyProtocolUnsupported,
342339
testMultiPfxSelectionDifference,
343340
testPskTls13Unsupported,

‎test/parallel/test-crypto-dh-curves.js‎

Lines changed: 25 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -35,33 +35,33 @@ if (!process.features.openssl_is_boringssl) {
3535
assert.strictEqual(
3636
crypto.createDiffieHellman(notSafePrime,Buffer.from([2])).verifyError,
3737
DH_CHECK_P_NOT_SAFE_PRIME);
38-
39-
constgroup=crypto.getDiffieHellman('modp14');
40-
constalice=crypto.createDiffieHellman(
41-
group.getPrime(),group.getGenerator());
42-
alice.generateKeys();
43-
constgroupPrime=BigInt(`0x${group.getPrime('hex')}`);
44-
assert.throws(
45-
()=>alice.computeSecret(Buffer.from([1])),
46-
{
47-
code: 'ERR_CRYPTO_INVALID_KEYLEN',
48-
message: 'Supplied key is too small'
49-
});
50-
assert.throws(
51-
()=>alice.computeSecret(group.getPrime()),
52-
{
53-
code: 'ERR_CRYPTO_INVALID_KEYLEN',
54-
message: 'Supplied key is too large'
55-
});
56-
assert.throws(
57-
()=>alice.computeSecret(
58-
Buffer.from((groupPrime-1n).toString(16),'hex')),
59-
{
60-
code: 'ERR_CRYPTO_INVALID_KEYLEN',
61-
message: 'Supplied key is too large'
62-
});
6338
}
6439

40+
constgroup=crypto.getDiffieHellman('modp14');
41+
constalice=crypto.createDiffieHellman(
42+
group.getPrime(),group.getGenerator());
43+
alice.generateKeys();
44+
constgroupPrime=BigInt(`0x${group.getPrime('hex')}`);
45+
assert.throws(
46+
()=>alice.computeSecret(Buffer.from([1])),
47+
{
48+
code: 'ERR_CRYPTO_INVALID_KEYLEN',
49+
message: 'Supplied key is too small'
50+
});
51+
assert.throws(
52+
()=>alice.computeSecret(group.getPrime()),
53+
{
54+
code: 'ERR_CRYPTO_INVALID_KEYLEN',
55+
message: 'Supplied key is too large'
56+
});
57+
assert.throws(
58+
()=>alice.computeSecret(
59+
Buffer.from((groupPrime-1n).toString(16),'hex')),
60+
{
61+
code: 'ERR_CRYPTO_INVALID_KEYLEN',
62+
message: 'Supplied key is too large'
63+
});
64+
6565
// Confirm DH_check() results are exposed for optional examination.
6666
constbad_dh=process.features.openssl_is_boringssl ?
6767
crypto.createDiffieHellman('abcd','hex',0) :

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

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -93,9 +93,7 @@ const {
9393
{
9494
assert.throws(()=>{
9595
dh3.computeSecret('');
96-
},{message: process.features.openssl_is_boringssl ?
97-
'Supplied key is invalid' :
98-
'Supplied key is too small'});
96+
},{message: 'Supplied key is too small'});
9997
}
10098
}
10199

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

Lines changed: 13 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -376,25 +376,20 @@ const isBoringSSL = process.features.openssl_is_boringssl;
376376
}
377377

378378
// Test invalid exponents. (caught by OpenSSL)
379+
letinvalidExponentError=/badevalue/;
380+
if(isBoringSSL){
381+
invalidExponentError=/BAD_E_VALUE/;
382+
}elseif(hasOpenSSL3){
383+
invalidExponentError=/exponent/;
384+
}
379385
for(constpublicExponentof[1,1+0x10001]){
380-
if(isBoringSSL){
381-
assert.throws(()=>generateKeyPair('rsa',{
382-
modulusLength: 4096,
383-
publicExponent
384-
},common.mustNotCall()),{
385-
name: 'RangeError',
386-
code: 'ERR_OUT_OF_RANGE',
387-
message: 'publicExponent is invalid',
388-
});
389-
}else{
390-
generateKeyPair('rsa',{
391-
modulusLength: 4096,
392-
publicExponent
393-
},common.mustCall((err)=>{
394-
assert.strictEqual(err.name,'Error');
395-
assert.match(err.message,hasOpenSSL3 ? /exponent/ : /badevalue/);
396-
}));
397-
}
386+
generateKeyPair('rsa',{
387+
modulusLength: 4096,
388+
publicExponent
389+
},common.mustCall((err)=>{
390+
assert.strictEqual(err.name,'Error');
391+
assert.match(err.message,invalidExponentError);
392+
}));
398393
}
399394
}
400395

‎test/parallel/test-crypto-sec-level.js‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,4 +15,8 @@ const assert = require('assert');
1515
// This test simply validates that we can get some value for the secLevel
1616
// when needed by tests.
1717
constsecLevel=require('internal/crypto/util').getOpenSSLSecLevel();
18-
assert.ok(secLevel>=0&&secLevel<=5);
18+
if(process.features.openssl_is_boringssl){
19+
assert.strictEqual(secLevel,0);
20+
}else{
21+
assert.ok(secLevel>=0&&secLevel<=5);
22+
}

‎test/parallel/test-tls-client-getephemeralkeyinfo.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ if (!common.hasCrypto)
44
common.skip('missing crypto');
55

66
if(process.features.openssl_is_boringssl){
7-
require('../common/boringssl').testEphemeralKeyInfoUnsupported();
7+
require('../common/boringssl').testEphemeralKeyInfo();
88
return;
99
}
1010

0 commit comments

Comments
 (0)
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links"); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
Skip to content

Commit 137ff67

Browse files
panvaaduh95
authored andcommitted
crypto: use available BoringSSL APIs
Use BoringSSL's current RSA and DH validation results instead of maintaining backend-specific prechecks and collapsing key errors. Report negotiated TLS groups and the documented zero security level through BoringSSL's compatibility APIs. Signed-off-by: Filip Skokan <panva.ip@gmail.com> PR-URL: #65423 Backport-PR-URL: #65483 Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Tim Perry <pimterry@gmail.com>
1 parent 0e87576 commit 137ff67

10 files changed

Lines changed: 66 additions & 89 deletions

File tree

‎deps/ncrypto/ncrypto.cc‎

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2252,14 +2252,11 @@ DHPointer::CheckPublicKeyResult DHPointer::checkPublicKey(
22522252
if (DH_check_pub_key(dh_.get(), pub_key.get(), &codes) != 1) {
22532253
return DHPointer::CheckPublicKeyResult::CHECK_FAILED;
22542254
}
2255-
#ifndef OPENSSL_IS_BORINGSSL
2256-
// Boringssl does not define DH_CHECK_PUBKEY_TOO_SMALL or TOO_LARGE
22572255
if (codes & DH_CHECK_PUBKEY_TOO_SMALL) {
22582256
return DHPointer::CheckPublicKeyResult::TOO_SMALL;
22592257
} elseif (codes & DH_CHECK_PUBKEY_TOO_LARGE) {
22602258
return DHPointer::CheckPublicKeyResult::TOO_LARGE;
22612259
}
2262-
#endif
22632260
if (codes != 0) {
22642261
return DHPointer::CheckPublicKeyResult::INVALID;
22652262
}
@@ -4309,6 +4306,13 @@ std::optional<std::string_view> SSLPointer::getNegotiatedGroup() const {
43094306
constchar* group = SSL_get0_group_name(get());
43104307
if (group == nullptr) return std::nullopt;
43114308
return group;
4309+
#elif defined(OPENSSL_IS_BORINGSSL)
4310+
if (!ssl_) return std::nullopt;
4311+
constint nid = SSL_get_negotiated_group(get());
4312+
if (nid == NID_undef) return std::nullopt;
4313+
constchar* group = OBJ_nid2sn(nid);
4314+
if (group == nullptr) return std::nullopt;
4315+
return group;
43124316
#else
43134317
return std::nullopt;
43144318
#endif
@@ -4333,19 +4337,17 @@ std::optional<std::string_view> SSLPointer::getCipherVersion() const {
43334337
}
43344338

43354339
std::optional<int> SSLPointer::getSecurityLevel() {
4336-
#ifndef OPENSSL_IS_BORINGSSL
43374340
auto ctx = SSLCtxPointer::New();
43384341
if (!ctx) return std::nullopt;
43394342

4343+
#ifdef OPENSSL_IS_BORINGSSL
4344+
returnSSL_CTX_get_security_level(ctx.get());
4345+
#else
43404346
auto ssl = SSLPointer::New(ctx);
43414347
if (!ssl) return std::nullopt;
43424348

43434349
returnSSL_get_security_level(ssl);
4344-
#else
4345-
// OPENSSL_TLS_SECURITY_LEVEL is not defined in BoringSSL
4346-
// so assume it is the default OPENSSL_TLS_SECURITY_LEVEL value.
4347-
return1;
4348-
#endif// OPENSSL_IS_BORINGSSL
4350+
#endif
43494351
}
43504352

43514353
SSLCtxPointer::SSLCtxPointer(SSL_CTX* ctx) : ctx_(ctx) {}

‎deps/ncrypto/ncrypto.h‎

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1231,9 +1231,9 @@ class DHPointer final {
12311231
UNABLE_TO_CHECK_GENERATOR = 0x04,
12321232
NOT_SUITABLE_GENERATOR = 0x08,
12331233
Q_NOT_PRIME = 0x10,
1234-
#ifndef OPENSSL_IS_BORINGSSL
1235-
// Boringssl does not define the DH_CHECK_INVALID_[Q or J]_VALUE
12361234
INVALID_Q = 0x20,
1235+
#ifndef OPENSSL_IS_BORINGSSL
1236+
// BoringSSL does not define DH_CHECK_INVALID_J_VALUE.
12371237
INVALID_J = 0x40,
12381238
MODULUS_TOO_SMALL = 0x80,
12391239
MODULUS_TOO_LARGE = 0x100,
@@ -1244,14 +1244,9 @@ class DHPointer final {
12441244

12451245
enumclassCheckPublicKeyResult {
12461246
NONE,
1247-
#ifndef OPENSSL_IS_BORINGSSL
1248-
// Boringssl does not define DH_R_CHECK_PUBKEY_TOO_SMALL or TOO_LARGE
1249-
TOO_SMALL = DH_R_CHECK_PUBKEY_TOO_SMALL,
1250-
TOO_LARGE = DH_R_CHECK_PUBKEY_TOO_LARGE,
1251-
INVALID = DH_R_CHECK_PUBKEY_INVALID,
1252-
#else
1253-
INVALID = DH_R_INVALID_PUBKEY,
1254-
#endif
1247+
TOO_SMALL,
1248+
TOO_LARGE,
1249+
INVALID,
12551250
CHECK_FAILED = 512,
12561251
};
12571252
// Check to see if the given public key is suitable for this DH instance.

‎src/crypto/crypto_dh.cc‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -319,12 +319,10 @@ void ComputeSecret(const FunctionCallbackInfo<Value>& args) {
319319
case DHPointer::CheckPublicKeyResult::CHECK_FAILED:
320320
returnTHROW_ERR_CRYPTO_INVALID_KEYTYPE(env,
321321
"Unspecified validation error");
322-
#ifndef OPENSSL_IS_BORINGSSL
323322
case DHPointer::CheckPublicKeyResult::TOO_SMALL:
324323
returnTHROW_ERR_CRYPTO_INVALID_KEYLEN(env, "Supplied key is too small");
325324
case DHPointer::CheckPublicKeyResult::TOO_LARGE:
326325
returnTHROW_ERR_CRYPTO_INVALID_KEYLEN(env, "Supplied key is too large");
327-
#endif
328326
case DHPointer::CheckPublicKeyResult::INVALID:
329327
returnTHROW_ERR_CRYPTO_INVALID_KEYTYPE(env, "Supplied key is invalid");
330328
case DHPointer::CheckPublicKeyResult::NONE:

‎src/crypto/crypto_rsa.cc‎

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

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

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

‎test/common/boringssl.js‎

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -137,12 +137,9 @@ function testRenegotiationUnsupported() {
137137
}
138138

139139
/**
140-
* OpenSSL exposes the negotiated ephemeral key type, name, and size for TLS
141-
* clients. With BoringSSL the same ECDHE TLS 1.2 handshake succeeds, but
142-
* getEphemeralKeyInfo() returns null on the server side and an object whose
143-
* fields are undefined on the client side.
140+
* BoringSSL exposes the negotiated TLS group but not the ephemeral key size.
144141
*/
145-
functiontestEphemeralKeyInfoUnsupported(){
142+
functiontestEphemeralKeyInfo(){
146143
constserver=tls.createServer({
147144
key: fixtures.readKey('agent2-key.pem'),
148145
cert: fixtures.readKey('agent2-cert.pem'),
@@ -161,8 +158,8 @@ function testEphemeralKeyInfoUnsupported() {
161158
maxVersion: 'TLSv1.2',
162159
},common.mustCall(()=>{
163160
assert.deepStrictEqual(client.getEphemeralKeyInfo(),{
164-
type: undefined,
165-
name: undefined,
161+
type: 'TLSGroup',
162+
name: 'prime256v1',
166163
size: undefined,
167164
});
168165
server.close();
@@ -337,7 +334,7 @@ module.exports = {
337334
assertMultiKeyUnsupported,
338335
assertNoCipherMatch,
339336
assertOpenSSLSecurityLevelsUnsupported,
340-
testEphemeralKeyInfoUnsupported,
337+
testEphemeralKeyInfo,
341338
testLegacyProtocolUnsupported,
342339
testMultiPfxSelectionDifference,
343340
testPskTls13Unsupported,

‎test/parallel/test-crypto-dh-curves.js‎

Lines changed: 25 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -35,33 +35,33 @@ if (!process.features.openssl_is_boringssl) {
3535
assert.strictEqual(
3636
crypto.createDiffieHellman(notSafePrime,Buffer.from([2])).verifyError,
3737
DH_CHECK_P_NOT_SAFE_PRIME);
38-
39-
constgroup=crypto.getDiffieHellman('modp14');
40-
constalice=crypto.createDiffieHellman(
41-
group.getPrime(),group.getGenerator());
42-
alice.generateKeys();
43-
constgroupPrime=BigInt(`0x${group.getPrime('hex')}`);
44-
assert.throws(
45-
()=>alice.computeSecret(Buffer.from([1])),
46-
{
47-
code: 'ERR_CRYPTO_INVALID_KEYLEN',
48-
message: 'Supplied key is too small'
49-
});
50-
assert.throws(
51-
()=>alice.computeSecret(group.getPrime()),
52-
{
53-
code: 'ERR_CRYPTO_INVALID_KEYLEN',
54-
message: 'Supplied key is too large'
55-
});
56-
assert.throws(
57-
()=>alice.computeSecret(
58-
Buffer.from((groupPrime-1n).toString(16),'hex')),
59-
{
60-
code: 'ERR_CRYPTO_INVALID_KEYLEN',
61-
message: 'Supplied key is too large'
62-
});
6338
}
6439

40+
constgroup=crypto.getDiffieHellman('modp14');
41+
constalice=crypto.createDiffieHellman(
42+
group.getPrime(),group.getGenerator());
43+
alice.generateKeys();
44+
constgroupPrime=BigInt(`0x${group.getPrime('hex')}`);
45+
assert.throws(
46+
()=>alice.computeSecret(Buffer.from([1])),
47+
{
48+
code: 'ERR_CRYPTO_INVALID_KEYLEN',
49+
message: 'Supplied key is too small'
50+
});
51+
assert.throws(
52+
()=>alice.computeSecret(group.getPrime()),
53+
{
54+
code: 'ERR_CRYPTO_INVALID_KEYLEN',
55+
message: 'Supplied key is too large'
56+
});
57+
assert.throws(
58+
()=>alice.computeSecret(
59+
Buffer.from((groupPrime-1n).toString(16),'hex')),
60+
{
61+
code: 'ERR_CRYPTO_INVALID_KEYLEN',
62+
message: 'Supplied key is too large'
63+
});
64+
6565
// Confirm DH_check() results are exposed for optional examination.
6666
constbad_dh=process.features.openssl_is_boringssl ?
6767
crypto.createDiffieHellman('abcd','hex',0) :

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

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -93,9 +93,7 @@ const {
9393
{
9494
assert.throws(()=>{
9595
dh3.computeSecret('');
96-
},{message: process.features.openssl_is_boringssl ?
97-
'Supplied key is invalid' :
98-
'Supplied key is too small'});
96+
},{message: 'Supplied key is too small'});
9997
}
10098
}
10199

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

Lines changed: 13 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -376,25 +376,20 @@ const isBoringSSL = process.features.openssl_is_boringssl;
376376
}
377377

378378
// Test invalid exponents. (caught by OpenSSL)
379+
letinvalidExponentError=/badevalue/;
380+
if(isBoringSSL){
381+
invalidExponentError=/BAD_E_VALUE/;
382+
}elseif(hasOpenSSL3){
383+
invalidExponentError=/exponent/;
384+
}
379385
for(constpublicExponentof[1,1+0x10001]){
380-
if(isBoringSSL){
381-
assert.throws(()=>generateKeyPair('rsa',{
382-
modulusLength: 4096,
383-
publicExponent
384-
},common.mustNotCall()),{
385-
name: 'RangeError',
386-
code: 'ERR_OUT_OF_RANGE',
387-
message: 'publicExponent is invalid',
388-
});
389-
}else{
390-
generateKeyPair('rsa',{
391-
modulusLength: 4096,
392-
publicExponent
393-
},common.mustCall((err)=>{
394-
assert.strictEqual(err.name,'Error');
395-
assert.match(err.message,hasOpenSSL3 ? /exponent/ : /badevalue/);
396-
}));
397-
}
386+
generateKeyPair('rsa',{
387+
modulusLength: 4096,
388+
publicExponent
389+
},common.mustCall((err)=>{
390+
assert.strictEqual(err.name,'Error');
391+
assert.match(err.message,invalidExponentError);
392+
}));
398393
}
399394
}
400395

‎test/parallel/test-crypto-sec-level.js‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,4 +15,8 @@ const assert = require('assert');
1515
// This test simply validates that we can get some value for the secLevel
1616
// when needed by tests.
1717
constsecLevel=require('internal/crypto/util').getOpenSSLSecLevel();
18-
assert.ok(secLevel>=0&&secLevel<=5);
18+
if(process.features.openssl_is_boringssl){
19+
assert.strictEqual(secLevel,0);
20+
}else{
21+
assert.ok(secLevel>=0&&secLevel<=5);
22+
}

‎test/parallel/test-tls-client-getephemeralkeyinfo.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ if (!common.hasCrypto)
44
common.skip('missing crypto');
55

66
if(process.features.openssl_is_boringssl){
7-
require('../common/boringssl').testEphemeralKeyInfoUnsupported();
7+
require('../common/boringssl').testEphemeralKeyInfo();
88
return;
99
}
1010

0 commit comments

Comments
 (0)
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default"); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Commit 137ff67

Browse files
panvaaduh95
authored andcommitted
crypto: use available BoringSSL APIs
Use BoringSSL's current RSA and DH validation results instead of maintaining backend-specific prechecks and collapsing key errors. Report negotiated TLS groups and the documented zero security level through BoringSSL's compatibility APIs. Signed-off-by: Filip Skokan <panva.ip@gmail.com> PR-URL: #65423 Backport-PR-URL: #65483 Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Tim Perry <pimterry@gmail.com>
1 parent 0e87576 commit 137ff67

10 files changed

Lines changed: 66 additions & 89 deletions

File tree

‎deps/ncrypto/ncrypto.cc‎

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2252,14 +2252,11 @@ DHPointer::CheckPublicKeyResult DHPointer::checkPublicKey(
22522252
if (DH_check_pub_key(dh_.get(), pub_key.get(), &codes) != 1) {
22532253
return DHPointer::CheckPublicKeyResult::CHECK_FAILED;
22542254
}
2255-
#ifndef OPENSSL_IS_BORINGSSL
2256-
// Boringssl does not define DH_CHECK_PUBKEY_TOO_SMALL or TOO_LARGE
22572255
if (codes & DH_CHECK_PUBKEY_TOO_SMALL) {
22582256
return DHPointer::CheckPublicKeyResult::TOO_SMALL;
22592257
} elseif (codes & DH_CHECK_PUBKEY_TOO_LARGE) {
22602258
return DHPointer::CheckPublicKeyResult::TOO_LARGE;
22612259
}
2262-
#endif
22632260
if (codes != 0) {
22642261
return DHPointer::CheckPublicKeyResult::INVALID;
22652262
}
@@ -4309,6 +4306,13 @@ std::optional<std::string_view> SSLPointer::getNegotiatedGroup() const {
43094306
constchar* group = SSL_get0_group_name(get());
43104307
if (group == nullptr) return std::nullopt;
43114308
return group;
4309+
#elif defined(OPENSSL_IS_BORINGSSL)
4310+
if (!ssl_) return std::nullopt;
4311+
constint nid = SSL_get_negotiated_group(get());
4312+
if (nid == NID_undef) return std::nullopt;
4313+
constchar* group = OBJ_nid2sn(nid);
4314+
if (group == nullptr) return std::nullopt;
4315+
return group;
43124316
#else
43134317
return std::nullopt;
43144318
#endif
@@ -4333,19 +4337,17 @@ std::optional<std::string_view> SSLPointer::getCipherVersion() const {
43334337
}
43344338

43354339
std::optional<int> SSLPointer::getSecurityLevel() {
4336-
#ifndef OPENSSL_IS_BORINGSSL
43374340
auto ctx = SSLCtxPointer::New();
43384341
if (!ctx) return std::nullopt;
43394342

4343+
#ifdef OPENSSL_IS_BORINGSSL
4344+
returnSSL_CTX_get_security_level(ctx.get());
4345+
#else
43404346
auto ssl = SSLPointer::New(ctx);
43414347
if (!ssl) return std::nullopt;
43424348

43434349
returnSSL_get_security_level(ssl);
4344-
#else
4345-
// OPENSSL_TLS_SECURITY_LEVEL is not defined in BoringSSL
4346-
// so assume it is the default OPENSSL_TLS_SECURITY_LEVEL value.
4347-
return1;
4348-
#endif// OPENSSL_IS_BORINGSSL
4350+
#endif
43494351
}
43504352

43514353
SSLCtxPointer::SSLCtxPointer(SSL_CTX* ctx) : ctx_(ctx) {}

‎deps/ncrypto/ncrypto.h‎

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1231,9 +1231,9 @@ class DHPointer final {
12311231
UNABLE_TO_CHECK_GENERATOR = 0x04,
12321232
NOT_SUITABLE_GENERATOR = 0x08,
12331233
Q_NOT_PRIME = 0x10,
1234-
#ifndef OPENSSL_IS_BORINGSSL
1235-
// Boringssl does not define the DH_CHECK_INVALID_[Q or J]_VALUE
12361234
INVALID_Q = 0x20,
1235+
#ifndef OPENSSL_IS_BORINGSSL
1236+
// BoringSSL does not define DH_CHECK_INVALID_J_VALUE.
12371237
INVALID_J = 0x40,
12381238
MODULUS_TOO_SMALL = 0x80,
12391239
MODULUS_TOO_LARGE = 0x100,
@@ -1244,14 +1244,9 @@ class DHPointer final {
12441244

12451245
enumclassCheckPublicKeyResult {
12461246
NONE,
1247-
#ifndef OPENSSL_IS_BORINGSSL
1248-
// Boringssl does not define DH_R_CHECK_PUBKEY_TOO_SMALL or TOO_LARGE
1249-
TOO_SMALL = DH_R_CHECK_PUBKEY_TOO_SMALL,
1250-
TOO_LARGE = DH_R_CHECK_PUBKEY_TOO_LARGE,
1251-
INVALID = DH_R_CHECK_PUBKEY_INVALID,
1252-
#else
1253-
INVALID = DH_R_INVALID_PUBKEY,
1254-
#endif
1247+
TOO_SMALL,
1248+
TOO_LARGE,
1249+
INVALID,
12551250
CHECK_FAILED = 512,
12561251
};
12571252
// Check to see if the given public key is suitable for this DH instance.

‎src/crypto/crypto_dh.cc‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -319,12 +319,10 @@ void ComputeSecret(const FunctionCallbackInfo<Value>& args) {
319319
case DHPointer::CheckPublicKeyResult::CHECK_FAILED:
320320
returnTHROW_ERR_CRYPTO_INVALID_KEYTYPE(env,
321321
"Unspecified validation error");
322-
#ifndef OPENSSL_IS_BORINGSSL
323322
case DHPointer::CheckPublicKeyResult::TOO_SMALL:
324323
returnTHROW_ERR_CRYPTO_INVALID_KEYLEN(env, "Supplied key is too small");
325324
case DHPointer::CheckPublicKeyResult::TOO_LARGE:
326325
returnTHROW_ERR_CRYPTO_INVALID_KEYLEN(env, "Supplied key is too large");
327-
#endif
328326
case DHPointer::CheckPublicKeyResult::INVALID:
329327
returnTHROW_ERR_CRYPTO_INVALID_KEYTYPE(env, "Supplied key is invalid");
330328
case DHPointer::CheckPublicKeyResult::NONE:

‎src/crypto/crypto_rsa.cc‎

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

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

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

‎test/common/boringssl.js‎

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -137,12 +137,9 @@ function testRenegotiationUnsupported() {
137137
}
138138

139139
/**
140-
* OpenSSL exposes the negotiated ephemeral key type, name, and size for TLS
141-
* clients. With BoringSSL the same ECDHE TLS 1.2 handshake succeeds, but
142-
* getEphemeralKeyInfo() returns null on the server side and an object whose
143-
* fields are undefined on the client side.
140+
* BoringSSL exposes the negotiated TLS group but not the ephemeral key size.
144141
*/
145-
functiontestEphemeralKeyInfoUnsupported(){
142+
functiontestEphemeralKeyInfo(){
146143
constserver=tls.createServer({
147144
key: fixtures.readKey('agent2-key.pem'),
148145
cert: fixtures.readKey('agent2-cert.pem'),
@@ -161,8 +158,8 @@ function testEphemeralKeyInfoUnsupported() {
161158
maxVersion: 'TLSv1.2',
162159
},common.mustCall(()=>{
163160
assert.deepStrictEqual(client.getEphemeralKeyInfo(),{
164-
type: undefined,
165-
name: undefined,
161+
type: 'TLSGroup',
162+
name: 'prime256v1',
166163
size: undefined,
167164
});
168165
server.close();
@@ -337,7 +334,7 @@ module.exports = {
337334
assertMultiKeyUnsupported,
338335
assertNoCipherMatch,
339336
assertOpenSSLSecurityLevelsUnsupported,
340-
testEphemeralKeyInfoUnsupported,
337+
testEphemeralKeyInfo,
341338
testLegacyProtocolUnsupported,
342339
testMultiPfxSelectionDifference,
343340
testPskTls13Unsupported,

‎test/parallel/test-crypto-dh-curves.js‎

Lines changed: 25 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -35,33 +35,33 @@ if (!process.features.openssl_is_boringssl) {
3535
assert.strictEqual(
3636
crypto.createDiffieHellman(notSafePrime,Buffer.from([2])).verifyError,
3737
DH_CHECK_P_NOT_SAFE_PRIME);
38-
39-
constgroup=crypto.getDiffieHellman('modp14');
40-
constalice=crypto.createDiffieHellman(
41-
group.getPrime(),group.getGenerator());
42-
alice.generateKeys();
43-
constgroupPrime=BigInt(`0x${group.getPrime('hex')}`);
44-
assert.throws(
45-
()=>alice.computeSecret(Buffer.from([1])),
46-
{
47-
code: 'ERR_CRYPTO_INVALID_KEYLEN',
48-
message: 'Supplied key is too small'
49-
});
50-
assert.throws(
51-
()=>alice.computeSecret(group.getPrime()),
52-
{
53-
code: 'ERR_CRYPTO_INVALID_KEYLEN',
54-
message: 'Supplied key is too large'
55-
});
56-
assert.throws(
57-
()=>alice.computeSecret(
58-
Buffer.from((groupPrime-1n).toString(16),'hex')),
59-
{
60-
code: 'ERR_CRYPTO_INVALID_KEYLEN',
61-
message: 'Supplied key is too large'
62-
});
6338
}
6439

40+
constgroup=crypto.getDiffieHellman('modp14');
41+
constalice=crypto.createDiffieHellman(
42+
group.getPrime(),group.getGenerator());
43+
alice.generateKeys();
44+
constgroupPrime=BigInt(`0x${group.getPrime('hex')}`);
45+
assert.throws(
46+
()=>alice.computeSecret(Buffer.from([1])),
47+
{
48+
code: 'ERR_CRYPTO_INVALID_KEYLEN',
49+
message: 'Supplied key is too small'
50+
});
51+
assert.throws(
52+
()=>alice.computeSecret(group.getPrime()),
53+
{
54+
code: 'ERR_CRYPTO_INVALID_KEYLEN',
55+
message: 'Supplied key is too large'
56+
});
57+
assert.throws(
58+
()=>alice.computeSecret(
59+
Buffer.from((groupPrime-1n).toString(16),'hex')),
60+
{
61+
code: 'ERR_CRYPTO_INVALID_KEYLEN',
62+
message: 'Supplied key is too large'
63+
});
64+
6565
// Confirm DH_check() results are exposed for optional examination.
6666
constbad_dh=process.features.openssl_is_boringssl ?
6767
crypto.createDiffieHellman('abcd','hex',0) :

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

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -93,9 +93,7 @@ const {
9393
{
9494
assert.throws(()=>{
9595
dh3.computeSecret('');
96-
},{message: process.features.openssl_is_boringssl ?
97-
'Supplied key is invalid' :
98-
'Supplied key is too small'});
96+
},{message: 'Supplied key is too small'});
9997
}
10098
}
10199

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

Lines changed: 13 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -376,25 +376,20 @@ const isBoringSSL = process.features.openssl_is_boringssl;
376376
}
377377

378378
// Test invalid exponents. (caught by OpenSSL)
379+
letinvalidExponentError=/badevalue/;
380+
if(isBoringSSL){
381+
invalidExponentError=/BAD_E_VALUE/;
382+
}elseif(hasOpenSSL3){
383+
invalidExponentError=/exponent/;
384+
}
379385
for(constpublicExponentof[1,1+0x10001]){
380-
if(isBoringSSL){
381-
assert.throws(()=>generateKeyPair('rsa',{
382-
modulusLength: 4096,
383-
publicExponent
384-
},common.mustNotCall()),{
385-
name: 'RangeError',
386-
code: 'ERR_OUT_OF_RANGE',
387-
message: 'publicExponent is invalid',
388-
});
389-
}else{
390-
generateKeyPair('rsa',{
391-
modulusLength: 4096,
392-
publicExponent
393-
},common.mustCall((err)=>{
394-
assert.strictEqual(err.name,'Error');
395-
assert.match(err.message,hasOpenSSL3 ? /exponent/ : /badevalue/);
396-
}));
397-
}
386+
generateKeyPair('rsa',{
387+
modulusLength: 4096,
388+
publicExponent
389+
},common.mustCall((err)=>{
390+
assert.strictEqual(err.name,'Error');
391+
assert.match(err.message,invalidExponentError);
392+
}));
398393
}
399394
}
400395

‎test/parallel/test-crypto-sec-level.js‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,4 +15,8 @@ const assert = require('assert');
1515
// This test simply validates that we can get some value for the secLevel
1616
// when needed by tests.
1717
constsecLevel=require('internal/crypto/util').getOpenSSLSecLevel();
18-
assert.ok(secLevel>=0&&secLevel<=5);
18+
if(process.features.openssl_is_boringssl){
19+
assert.strictEqual(secLevel,0);
20+
}else{
21+
assert.ok(secLevel>=0&&secLevel<=5);
22+
}

‎test/parallel/test-tls-client-getephemeralkeyinfo.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ if (!common.hasCrypto)
44
common.skip('missing crypto');
55

66
if(process.features.openssl_is_boringssl){
7-
require('../common/boringssl').testEphemeralKeyInfoUnsupported();
7+
require('../common/boringssl').testEphemeralKeyInfo();
88
return;
99
}
1010

0 commit comments

Comments
 (0)
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers"); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Commit 137ff67

Browse files
panvaaduh95
authored andcommitted
crypto: use available BoringSSL APIs
Use BoringSSL's current RSA and DH validation results instead of maintaining backend-specific prechecks and collapsing key errors. Report negotiated TLS groups and the documented zero security level through BoringSSL's compatibility APIs. Signed-off-by: Filip Skokan <panva.ip@gmail.com> PR-URL: #65423 Backport-PR-URL: #65483 Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Tim Perry <pimterry@gmail.com>
1 parent 0e87576 commit 137ff67

10 files changed

Lines changed: 66 additions & 89 deletions

File tree

‎deps/ncrypto/ncrypto.cc‎

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2252,14 +2252,11 @@ DHPointer::CheckPublicKeyResult DHPointer::checkPublicKey(
22522252
if (DH_check_pub_key(dh_.get(), pub_key.get(), &codes) != 1) {
22532253
return DHPointer::CheckPublicKeyResult::CHECK_FAILED;
22542254
}
2255-
#ifndef OPENSSL_IS_BORINGSSL
2256-
// Boringssl does not define DH_CHECK_PUBKEY_TOO_SMALL or TOO_LARGE
22572255
if (codes & DH_CHECK_PUBKEY_TOO_SMALL) {
22582256
return DHPointer::CheckPublicKeyResult::TOO_SMALL;
22592257
} elseif (codes & DH_CHECK_PUBKEY_TOO_LARGE) {
22602258
return DHPointer::CheckPublicKeyResult::TOO_LARGE;
22612259
}
2262-
#endif
22632260
if (codes != 0) {
22642261
return DHPointer::CheckPublicKeyResult::INVALID;
22652262
}
@@ -4309,6 +4306,13 @@ std::optional<std::string_view> SSLPointer::getNegotiatedGroup() const {
43094306
constchar* group = SSL_get0_group_name(get());
43104307
if (group == nullptr) return std::nullopt;
43114308
return group;
4309+
#elif defined(OPENSSL_IS_BORINGSSL)
4310+
if (!ssl_) return std::nullopt;
4311+
constint nid = SSL_get_negotiated_group(get());
4312+
if (nid == NID_undef) return std::nullopt;
4313+
constchar* group = OBJ_nid2sn(nid);
4314+
if (group == nullptr) return std::nullopt;
4315+
return group;
43124316
#else
43134317
return std::nullopt;
43144318
#endif
@@ -4333,19 +4337,17 @@ std::optional<std::string_view> SSLPointer::getCipherVersion() const {
43334337
}
43344338

43354339
std::optional<int> SSLPointer::getSecurityLevel() {
4336-
#ifndef OPENSSL_IS_BORINGSSL
43374340
auto ctx = SSLCtxPointer::New();
43384341
if (!ctx) return std::nullopt;
43394342

4343+
#ifdef OPENSSL_IS_BORINGSSL
4344+
returnSSL_CTX_get_security_level(ctx.get());
4345+
#else
43404346
auto ssl = SSLPointer::New(ctx);
43414347
if (!ssl) return std::nullopt;
43424348

43434349
returnSSL_get_security_level(ssl);
4344-
#else
4345-
// OPENSSL_TLS_SECURITY_LEVEL is not defined in BoringSSL
4346-
// so assume it is the default OPENSSL_TLS_SECURITY_LEVEL value.
4347-
return1;
4348-
#endif// OPENSSL_IS_BORINGSSL
4350+
#endif
43494351
}
43504352

43514353
SSLCtxPointer::SSLCtxPointer(SSL_CTX* ctx) : ctx_(ctx) {}

‎deps/ncrypto/ncrypto.h‎

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1231,9 +1231,9 @@ class DHPointer final {
12311231
UNABLE_TO_CHECK_GENERATOR = 0x04,
12321232
NOT_SUITABLE_GENERATOR = 0x08,
12331233
Q_NOT_PRIME = 0x10,
1234-
#ifndef OPENSSL_IS_BORINGSSL
1235-
// Boringssl does not define the DH_CHECK_INVALID_[Q or J]_VALUE
12361234
INVALID_Q = 0x20,
1235+
#ifndef OPENSSL_IS_BORINGSSL
1236+
// BoringSSL does not define DH_CHECK_INVALID_J_VALUE.
12371237
INVALID_J = 0x40,
12381238
MODULUS_TOO_SMALL = 0x80,
12391239
MODULUS_TOO_LARGE = 0x100,
@@ -1244,14 +1244,9 @@ class DHPointer final {
12441244

12451245
enumclassCheckPublicKeyResult {
12461246
NONE,
1247-
#ifndef OPENSSL_IS_BORINGSSL
1248-
// Boringssl does not define DH_R_CHECK_PUBKEY_TOO_SMALL or TOO_LARGE
1249-
TOO_SMALL = DH_R_CHECK_PUBKEY_TOO_SMALL,
1250-
TOO_LARGE = DH_R_CHECK_PUBKEY_TOO_LARGE,
1251-
INVALID = DH_R_CHECK_PUBKEY_INVALID,
1252-
#else
1253-
INVALID = DH_R_INVALID_PUBKEY,
1254-
#endif
1247+
TOO_SMALL,
1248+
TOO_LARGE,
1249+
INVALID,
12551250
CHECK_FAILED = 512,
12561251
};
12571252
// Check to see if the given public key is suitable for this DH instance.

‎src/crypto/crypto_dh.cc‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -319,12 +319,10 @@ void ComputeSecret(const FunctionCallbackInfo<Value>& args) {
319319
case DHPointer::CheckPublicKeyResult::CHECK_FAILED:
320320
returnTHROW_ERR_CRYPTO_INVALID_KEYTYPE(env,
321321
"Unspecified validation error");
322-
#ifndef OPENSSL_IS_BORINGSSL
323322
case DHPointer::CheckPublicKeyResult::TOO_SMALL:
324323
returnTHROW_ERR_CRYPTO_INVALID_KEYLEN(env, "Supplied key is too small");
325324
case DHPointer::CheckPublicKeyResult::TOO_LARGE:
326325
returnTHROW_ERR_CRYPTO_INVALID_KEYLEN(env, "Supplied key is too large");
327-
#endif
328326
case DHPointer::CheckPublicKeyResult::INVALID:
329327
returnTHROW_ERR_CRYPTO_INVALID_KEYTYPE(env, "Supplied key is invalid");
330328
case DHPointer::CheckPublicKeyResult::NONE:

‎src/crypto/crypto_rsa.cc‎

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

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

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

‎test/common/boringssl.js‎

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -137,12 +137,9 @@ function testRenegotiationUnsupported() {
137137
}
138138

139139
/**
140-
* OpenSSL exposes the negotiated ephemeral key type, name, and size for TLS
141-
* clients. With BoringSSL the same ECDHE TLS 1.2 handshake succeeds, but
142-
* getEphemeralKeyInfo() returns null on the server side and an object whose
143-
* fields are undefined on the client side.
140+
* BoringSSL exposes the negotiated TLS group but not the ephemeral key size.
144141
*/
145-
functiontestEphemeralKeyInfoUnsupported(){
142+
functiontestEphemeralKeyInfo(){
146143
constserver=tls.createServer({
147144
key: fixtures.readKey('agent2-key.pem'),
148145
cert: fixtures.readKey('agent2-cert.pem'),
@@ -161,8 +158,8 @@ function testEphemeralKeyInfoUnsupported() {
161158
maxVersion: 'TLSv1.2',
162159
},common.mustCall(()=>{
163160
assert.deepStrictEqual(client.getEphemeralKeyInfo(),{
164-
type: undefined,
165-
name: undefined,
161+
type: 'TLSGroup',
162+
name: 'prime256v1',
166163
size: undefined,
167164
});
168165
server.close();
@@ -337,7 +334,7 @@ module.exports = {
337334
assertMultiKeyUnsupported,
338335
assertNoCipherMatch,
339336
assertOpenSSLSecurityLevelsUnsupported,
340-
testEphemeralKeyInfoUnsupported,
337+
testEphemeralKeyInfo,
341338
testLegacyProtocolUnsupported,
342339
testMultiPfxSelectionDifference,
343340
testPskTls13Unsupported,

‎test/parallel/test-crypto-dh-curves.js‎

Lines changed: 25 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -35,33 +35,33 @@ if (!process.features.openssl_is_boringssl) {
3535
assert.strictEqual(
3636
crypto.createDiffieHellman(notSafePrime,Buffer.from([2])).verifyError,
3737
DH_CHECK_P_NOT_SAFE_PRIME);
38-
39-
constgroup=crypto.getDiffieHellman('modp14');
40-
constalice=crypto.createDiffieHellman(
41-
group.getPrime(),group.getGenerator());
42-
alice.generateKeys();
43-
constgroupPrime=BigInt(`0x${group.getPrime('hex')}`);
44-
assert.throws(
45-
()=>alice.computeSecret(Buffer.from([1])),
46-
{
47-
code: 'ERR_CRYPTO_INVALID_KEYLEN',
48-
message: 'Supplied key is too small'
49-
});
50-
assert.throws(
51-
()=>alice.computeSecret(group.getPrime()),
52-
{
53-
code: 'ERR_CRYPTO_INVALID_KEYLEN',
54-
message: 'Supplied key is too large'
55-
});
56-
assert.throws(
57-
()=>alice.computeSecret(
58-
Buffer.from((groupPrime-1n).toString(16),'hex')),
59-
{
60-
code: 'ERR_CRYPTO_INVALID_KEYLEN',
61-
message: 'Supplied key is too large'
62-
});
6338
}
6439

40+
constgroup=crypto.getDiffieHellman('modp14');
41+
constalice=crypto.createDiffieHellman(
42+
group.getPrime(),group.getGenerator());
43+
alice.generateKeys();
44+
constgroupPrime=BigInt(`0x${group.getPrime('hex')}`);
45+
assert.throws(
46+
()=>alice.computeSecret(Buffer.from([1])),
47+
{
48+
code: 'ERR_CRYPTO_INVALID_KEYLEN',
49+
message: 'Supplied key is too small'
50+
});
51+
assert.throws(
52+
()=>alice.computeSecret(group.getPrime()),
53+
{
54+
code: 'ERR_CRYPTO_INVALID_KEYLEN',
55+
message: 'Supplied key is too large'
56+
});
57+
assert.throws(
58+
()=>alice.computeSecret(
59+
Buffer.from((groupPrime-1n).toString(16),'hex')),
60+
{
61+
code: 'ERR_CRYPTO_INVALID_KEYLEN',
62+
message: 'Supplied key is too large'
63+
});
64+
6565
// Confirm DH_check() results are exposed for optional examination.
6666
constbad_dh=process.features.openssl_is_boringssl ?
6767
crypto.createDiffieHellman('abcd','hex',0) :

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

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -93,9 +93,7 @@ const {
9393
{
9494
assert.throws(()=>{
9595
dh3.computeSecret('');
96-
},{message: process.features.openssl_is_boringssl ?
97-
'Supplied key is invalid' :
98-
'Supplied key is too small'});
96+
},{message: 'Supplied key is too small'});
9997
}
10098
}
10199

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

Lines changed: 13 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -376,25 +376,20 @@ const isBoringSSL = process.features.openssl_is_boringssl;
376376
}
377377

378378
// Test invalid exponents. (caught by OpenSSL)
379+
letinvalidExponentError=/badevalue/;
380+
if(isBoringSSL){
381+
invalidExponentError=/BAD_E_VALUE/;
382+
}elseif(hasOpenSSL3){
383+
invalidExponentError=/exponent/;
384+
}
379385
for(constpublicExponentof[1,1+0x10001]){
380-
if(isBoringSSL){
381-
assert.throws(()=>generateKeyPair('rsa',{
382-
modulusLength: 4096,
383-
publicExponent
384-
},common.mustNotCall()),{
385-
name: 'RangeError',
386-
code: 'ERR_OUT_OF_RANGE',
387-
message: 'publicExponent is invalid',
388-
});
389-
}else{
390-
generateKeyPair('rsa',{
391-
modulusLength: 4096,
392-
publicExponent
393-
},common.mustCall((err)=>{
394-
assert.strictEqual(err.name,'Error');
395-
assert.match(err.message,hasOpenSSL3 ? /exponent/ : /badevalue/);
396-
}));
397-
}
386+
generateKeyPair('rsa',{
387+
modulusLength: 4096,
388+
publicExponent
389+
},common.mustCall((err)=>{
390+
assert.strictEqual(err.name,'Error');
391+
assert.match(err.message,invalidExponentError);
392+
}));
398393
}
399394
}
400395

‎test/parallel/test-crypto-sec-level.js‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,4 +15,8 @@ const assert = require('assert');
1515
// This test simply validates that we can get some value for the secLevel
1616
// when needed by tests.
1717
constsecLevel=require('internal/crypto/util').getOpenSSLSecLevel();
18-
assert.ok(secLevel>=0&&secLevel<=5);
18+
if(process.features.openssl_is_boringssl){
19+
assert.strictEqual(secLevel,0);
20+
}else{
21+
assert.ok(secLevel>=0&&secLevel<=5);
22+
}

‎test/parallel/test-tls-client-getephemeralkeyinfo.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ if (!common.hasCrypto)
44
common.skip('missing crypto');
55

66
if(process.features.openssl_is_boringssl){
7-
require('../common/boringssl').testEphemeralKeyInfoUnsupported();
7+
require('../common/boringssl').testEphemeralKeyInfo();
88
return;
99
}
1010

0 commit comments

Comments
 (0)
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode"); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
Skip to content

Commit 137ff67

Browse files
panvaaduh95
authored andcommitted
crypto: use available BoringSSL APIs
Use BoringSSL's current RSA and DH validation results instead of maintaining backend-specific prechecks and collapsing key errors. Report negotiated TLS groups and the documented zero security level through BoringSSL's compatibility APIs. Signed-off-by: Filip Skokan <panva.ip@gmail.com> PR-URL: #65423 Backport-PR-URL: #65483 Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Tim Perry <pimterry@gmail.com>
1 parent 0e87576 commit 137ff67

10 files changed

Lines changed: 66 additions & 89 deletions

File tree

‎deps/ncrypto/ncrypto.cc‎

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2252,14 +2252,11 @@ DHPointer::CheckPublicKeyResult DHPointer::checkPublicKey(
22522252
if (DH_check_pub_key(dh_.get(), pub_key.get(), &codes) != 1) {
22532253
return DHPointer::CheckPublicKeyResult::CHECK_FAILED;
22542254
}
2255-
#ifndef OPENSSL_IS_BORINGSSL
2256-
// Boringssl does not define DH_CHECK_PUBKEY_TOO_SMALL or TOO_LARGE
22572255
if (codes & DH_CHECK_PUBKEY_TOO_SMALL) {
22582256
return DHPointer::CheckPublicKeyResult::TOO_SMALL;
22592257
} elseif (codes & DH_CHECK_PUBKEY_TOO_LARGE) {
22602258
return DHPointer::CheckPublicKeyResult::TOO_LARGE;
22612259
}
2262-
#endif
22632260
if (codes != 0) {
22642261
return DHPointer::CheckPublicKeyResult::INVALID;
22652262
}
@@ -4309,6 +4306,13 @@ std::optional<std::string_view> SSLPointer::getNegotiatedGroup() const {
43094306
constchar* group = SSL_get0_group_name(get());
43104307
if (group == nullptr) return std::nullopt;
43114308
return group;
4309+
#elif defined(OPENSSL_IS_BORINGSSL)
4310+
if (!ssl_) return std::nullopt;
4311+
constint nid = SSL_get_negotiated_group(get());
4312+
if (nid == NID_undef) return std::nullopt;
4313+
constchar* group = OBJ_nid2sn(nid);
4314+
if (group == nullptr) return std::nullopt;
4315+
return group;
43124316
#else
43134317
return std::nullopt;
43144318
#endif
@@ -4333,19 +4337,17 @@ std::optional<std::string_view> SSLPointer::getCipherVersion() const {
43334337
}
43344338

43354339
std::optional<int> SSLPointer::getSecurityLevel() {
4336-
#ifndef OPENSSL_IS_BORINGSSL
43374340
auto ctx = SSLCtxPointer::New();
43384341
if (!ctx) return std::nullopt;
43394342

4343+
#ifdef OPENSSL_IS_BORINGSSL
4344+
returnSSL_CTX_get_security_level(ctx.get());
4345+
#else
43404346
auto ssl = SSLPointer::New(ctx);
43414347
if (!ssl) return std::nullopt;
43424348

43434349
returnSSL_get_security_level(ssl);
4344-
#else
4345-
// OPENSSL_TLS_SECURITY_LEVEL is not defined in BoringSSL
4346-
// so assume it is the default OPENSSL_TLS_SECURITY_LEVEL value.
4347-
return1;
4348-
#endif// OPENSSL_IS_BORINGSSL
4350+
#endif
43494351
}
43504352

43514353
SSLCtxPointer::SSLCtxPointer(SSL_CTX* ctx) : ctx_(ctx) {}

‎deps/ncrypto/ncrypto.h‎

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1231,9 +1231,9 @@ class DHPointer final {
12311231
UNABLE_TO_CHECK_GENERATOR = 0x04,
12321232
NOT_SUITABLE_GENERATOR = 0x08,
12331233
Q_NOT_PRIME = 0x10,
1234-
#ifndef OPENSSL_IS_BORINGSSL
1235-
// Boringssl does not define the DH_CHECK_INVALID_[Q or J]_VALUE
12361234
INVALID_Q = 0x20,
1235+
#ifndef OPENSSL_IS_BORINGSSL
1236+
// BoringSSL does not define DH_CHECK_INVALID_J_VALUE.
12371237
INVALID_J = 0x40,
12381238
MODULUS_TOO_SMALL = 0x80,
12391239
MODULUS_TOO_LARGE = 0x100,
@@ -1244,14 +1244,9 @@ class DHPointer final {
12441244

12451245
enumclassCheckPublicKeyResult {
12461246
NONE,
1247-
#ifndef OPENSSL_IS_BORINGSSL
1248-
// Boringssl does not define DH_R_CHECK_PUBKEY_TOO_SMALL or TOO_LARGE
1249-
TOO_SMALL = DH_R_CHECK_PUBKEY_TOO_SMALL,
1250-
TOO_LARGE = DH_R_CHECK_PUBKEY_TOO_LARGE,
1251-
INVALID = DH_R_CHECK_PUBKEY_INVALID,
1252-
#else
1253-
INVALID = DH_R_INVALID_PUBKEY,
1254-
#endif
1247+
TOO_SMALL,
1248+
TOO_LARGE,
1249+
INVALID,
12551250
CHECK_FAILED = 512,
12561251
};
12571252
// Check to see if the given public key is suitable for this DH instance.

‎src/crypto/crypto_dh.cc‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -319,12 +319,10 @@ void ComputeSecret(const FunctionCallbackInfo<Value>& args) {
319319
case DHPointer::CheckPublicKeyResult::CHECK_FAILED:
320320
returnTHROW_ERR_CRYPTO_INVALID_KEYTYPE(env,
321321
"Unspecified validation error");
322-
#ifndef OPENSSL_IS_BORINGSSL
323322
case DHPointer::CheckPublicKeyResult::TOO_SMALL:
324323
returnTHROW_ERR_CRYPTO_INVALID_KEYLEN(env, "Supplied key is too small");
325324
case DHPointer::CheckPublicKeyResult::TOO_LARGE:
326325
returnTHROW_ERR_CRYPTO_INVALID_KEYLEN(env, "Supplied key is too large");
327-
#endif
328326
case DHPointer::CheckPublicKeyResult::INVALID:
329327
returnTHROW_ERR_CRYPTO_INVALID_KEYTYPE(env, "Supplied key is invalid");
330328
case DHPointer::CheckPublicKeyResult::NONE:

‎src/crypto/crypto_rsa.cc‎

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

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

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

‎test/common/boringssl.js‎

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -137,12 +137,9 @@ function testRenegotiationUnsupported() {
137137
}
138138

139139
/**
140-
* OpenSSL exposes the negotiated ephemeral key type, name, and size for TLS
141-
* clients. With BoringSSL the same ECDHE TLS 1.2 handshake succeeds, but
142-
* getEphemeralKeyInfo() returns null on the server side and an object whose
143-
* fields are undefined on the client side.
140+
* BoringSSL exposes the negotiated TLS group but not the ephemeral key size.
144141
*/
145-
functiontestEphemeralKeyInfoUnsupported(){
142+
functiontestEphemeralKeyInfo(){
146143
constserver=tls.createServer({
147144
key: fixtures.readKey('agent2-key.pem'),
148145
cert: fixtures.readKey('agent2-cert.pem'),
@@ -161,8 +158,8 @@ function testEphemeralKeyInfoUnsupported() {
161158
maxVersion: 'TLSv1.2',
162159
},common.mustCall(()=>{
163160
assert.deepStrictEqual(client.getEphemeralKeyInfo(),{
164-
type: undefined,
165-
name: undefined,
161+
type: 'TLSGroup',
162+
name: 'prime256v1',
166163
size: undefined,
167164
});
168165
server.close();
@@ -337,7 +334,7 @@ module.exports = {
337334
assertMultiKeyUnsupported,
338335
assertNoCipherMatch,
339336
assertOpenSSLSecurityLevelsUnsupported,
340-
testEphemeralKeyInfoUnsupported,
337+
testEphemeralKeyInfo,
341338
testLegacyProtocolUnsupported,
342339
testMultiPfxSelectionDifference,
343340
testPskTls13Unsupported,

‎test/parallel/test-crypto-dh-curves.js‎

Lines changed: 25 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -35,33 +35,33 @@ if (!process.features.openssl_is_boringssl) {
3535
assert.strictEqual(
3636
crypto.createDiffieHellman(notSafePrime,Buffer.from([2])).verifyError,
3737
DH_CHECK_P_NOT_SAFE_PRIME);
38-
39-
constgroup=crypto.getDiffieHellman('modp14');
40-
constalice=crypto.createDiffieHellman(
41-
group.getPrime(),group.getGenerator());
42-
alice.generateKeys();
43-
constgroupPrime=BigInt(`0x${group.getPrime('hex')}`);
44-
assert.throws(
45-
()=>alice.computeSecret(Buffer.from([1])),
46-
{
47-
code: 'ERR_CRYPTO_INVALID_KEYLEN',
48-
message: 'Supplied key is too small'
49-
});
50-
assert.throws(
51-
()=>alice.computeSecret(group.getPrime()),
52-
{
53-
code: 'ERR_CRYPTO_INVALID_KEYLEN',
54-
message: 'Supplied key is too large'
55-
});
56-
assert.throws(
57-
()=>alice.computeSecret(
58-
Buffer.from((groupPrime-1n).toString(16),'hex')),
59-
{
60-
code: 'ERR_CRYPTO_INVALID_KEYLEN',
61-
message: 'Supplied key is too large'
62-
});
6338
}
6439

40+
constgroup=crypto.getDiffieHellman('modp14');
41+
constalice=crypto.createDiffieHellman(
42+
group.getPrime(),group.getGenerator());
43+
alice.generateKeys();
44+
constgroupPrime=BigInt(`0x${group.getPrime('hex')}`);
45+
assert.throws(
46+
()=>alice.computeSecret(Buffer.from([1])),
47+
{
48+
code: 'ERR_CRYPTO_INVALID_KEYLEN',
49+
message: 'Supplied key is too small'
50+
});
51+
assert.throws(
52+
()=>alice.computeSecret(group.getPrime()),
53+
{
54+
code: 'ERR_CRYPTO_INVALID_KEYLEN',
55+
message: 'Supplied key is too large'
56+
});
57+
assert.throws(
58+
()=>alice.computeSecret(
59+
Buffer.from((groupPrime-1n).toString(16),'hex')),
60+
{
61+
code: 'ERR_CRYPTO_INVALID_KEYLEN',
62+
message: 'Supplied key is too large'
63+
});
64+
6565
// Confirm DH_check() results are exposed for optional examination.
6666
constbad_dh=process.features.openssl_is_boringssl ?
6767
crypto.createDiffieHellman('abcd','hex',0) :

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

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -93,9 +93,7 @@ const {
9393
{
9494
assert.throws(()=>{
9595
dh3.computeSecret('');
96-
},{message: process.features.openssl_is_boringssl ?
97-
'Supplied key is invalid' :
98-
'Supplied key is too small'});
96+
},{message: 'Supplied key is too small'});
9997
}
10098
}
10199

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

Lines changed: 13 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -376,25 +376,20 @@ const isBoringSSL = process.features.openssl_is_boringssl;
376376
}
377377

378378
// Test invalid exponents. (caught by OpenSSL)
379+
letinvalidExponentError=/badevalue/;
380+
if(isBoringSSL){
381+
invalidExponentError=/BAD_E_VALUE/;
382+
}elseif(hasOpenSSL3){
383+
invalidExponentError=/exponent/;
384+
}
379385
for(constpublicExponentof[1,1+0x10001]){
380-
if(isBoringSSL){
381-
assert.throws(()=>generateKeyPair('rsa',{
382-
modulusLength: 4096,
383-
publicExponent
384-
},common.mustNotCall()),{
385-
name: 'RangeError',
386-
code: 'ERR_OUT_OF_RANGE',
387-
message: 'publicExponent is invalid',
388-
});
389-
}else{
390-
generateKeyPair('rsa',{
391-
modulusLength: 4096,
392-
publicExponent
393-
},common.mustCall((err)=>{
394-
assert.strictEqual(err.name,'Error');
395-
assert.match(err.message,hasOpenSSL3 ? /exponent/ : /badevalue/);
396-
}));
397-
}
386+
generateKeyPair('rsa',{
387+
modulusLength: 4096,
388+
publicExponent
389+
},common.mustCall((err)=>{
390+
assert.strictEqual(err.name,'Error');
391+
assert.match(err.message,invalidExponentError);
392+
}));
398393
}
399394
}
400395

‎test/parallel/test-crypto-sec-level.js‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,4 +15,8 @@ const assert = require('assert');
1515
// This test simply validates that we can get some value for the secLevel
1616
// when needed by tests.
1717
constsecLevel=require('internal/crypto/util').getOpenSSLSecLevel();
18-
assert.ok(secLevel>=0&&secLevel<=5);
18+
if(process.features.openssl_is_boringssl){
19+
assert.strictEqual(secLevel,0);
20+
}else{
21+
assert.ok(secLevel>=0&&secLevel<=5);
22+
}

‎test/parallel/test-tls-client-getephemeralkeyinfo.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ if (!common.hasCrypto)
44
common.skip('missing crypto');
55

66
if(process.features.openssl_is_boringssl){
7-
require('../common/boringssl').testEphemeralKeyInfoUnsupported();
7+
require('../common/boringssl').testEphemeralKeyInfo();
88
return;
99
}
1010

0 commit comments

Comments
 (0)