Skip to content

Commit 195f679

Browse files
nils91targos
nils91
authored and
targos
committed
crypto: don't crash with some selfsigned certs
Refs: #37757 Refs: #37889 PR-URL: #37990Fixes: #37757 Reviewed-By: James M Snell <jasnell@gmail.com>
1 parent 14afb39 commit 195f679

7 files changed

Lines changed: 141 additions & 0 deletions

File tree

‎src/crypto/crypto_common.cc‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -480,8 +480,16 @@ MaybeLocal<Object> GetLastIssuedCert(
480480
return MaybeLocal<Object>();
481481
issuer_chain = ca_info;
482482

483+
// Take the value of cert->get() before the call to cert->reset()
484+
// in order to compare it to ca after and provide a way to exit this loop
485+
// in case it gets stuck.
486+
X509* value_before_reset = cert->get();
487+
483488
// Delete previous cert and continue aggregating issuers.
484489
cert->reset(ca);
490+
491+
if (value_before_reset == ca)
492+
break;
485493
}
486494
return MaybeLocal<Object>(issuer_chain);
487495
}

‎test/fixtures/keys/selfsigned-no-keycertsign/README.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
# Self-signed certificate without keyCertSign bit
2+
The self-signed certificate ([cert.pem](./cert.pem)) and the key ([key.pem](./key.pem)) in this folder are used by the test [test-https-selfsigned-no-keycertsign-no-crash](../../../parallel/test-https-selfsigned-no-keycertsign-no-crash.js). The config ([cert.conf](./cert.conf)) and the file used to generate key and certificate in this folder ([https-renew-cert.sh](./https_renew_cert.sh)) are not used by the test but for reference.

‎test/fixtures/keys/selfsigned-no-keycertsign/cert.conf‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
[req]
2+
distinguished_name = req_distinguished_name
3+
req_extensions = v3_req
4+
prompt = no
5+
6+
[req_distinguished_name]
7+
C = DE
8+
CN = localhost
9+
10+
[v3_req]
11+
keyUsage = digitalSignature, keyEncipherment
12+
extendedKeyUsage = serverAuth
13+
subjectAltName = @alt_names
14+
[alt_names]
15+
DNS.1 = 127.0.0.1
16+
DNS.2 = localhost
17+
IP.1 = 127.0.0.1

‎test/fixtures/keys/selfsigned-no-keycertsign/cert.pem‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
-----BEGIN CERTIFICATE-----
2+
MIIC9jCCAd6gAwIBAgIJANHflGRpZM1IMA0GCSqGSIb3DQEBCwUAMBQxEjAQBgNV
3+
BAMMCWxvY2FsaG9zdDAeFw0yMTAzMTUwOTEzMjdaFw0yMjAzMTUwOTEzMjdaMBQx
4+
EjAQBgNVBAMMCWxvY2FsaG9zdDCCASIwDQYJKoZIhvcNAQEBBQADggEPADCCAQoC
5+
ggEBANMt6TLw9gIxucRgZBn8owavEIMAddxMTjkHiR7jGfaBrvvVTB8ymsIizw/Q
6+
KTANmu2r3EOqeR9Ht25KZFKxOKCDMd3aKHht38HInXIF6CQe8c5P0xsVKZAWkell
7+
8ohL05EsFpcrJODIdHfaovODrtX8w1WexqDsUoPQdEk7pISJ2HhmXzpf7QmV00Ux
8+
8J+64v2pTg8/C9VgpSgxE4oXlfJEqdSIAzGDT+VX96GWXTh7QqLjiQ9T96QHUJEn
9+
Bx0Sr4rO9mY2lOQG408QuCLR/ng2J+lYx+03SC8Lq7lrtt4M06Ffr8TQRgpDAjkU
10+
0YitbuysD5XgtCeFq0Fi3v1z700CAwEAAaNLMEkwCwYDVR0PBAQDAgWgMBMGA1Ud
11+
JQQMMAoGCCsGAQUFBwMBMCUGA1UdEQQeMByCCTEyNy4wLjAuMYIJbG9jYWxob3N0
12+
hwR/AAABMA0GCSqGSIb3DQEBCwUAA4IBAQDAUCt/8Le2EO0ONOkQYUcPmSut6Siz
13+
UIQrJ8Lwfs0fb+Zk9ElNGLwYTzooKDgzK8cLQ8g8F2WkolBEPXDsy1Ab+e66WkJH
14+
NH/zAgEyG6cXXRNc+ObM5KbjY0YuDGiajKcndknuuCB+onlC1Pv5oFUSNa3/06+S
15+
sziFloGbg5S0AHT6lYnwZSM6G7Pre8mcRNRxL6Yw1FOOUpQZKPd7juy4GBRlCucn
16+
wmp/Fl0wIBDs91Vprig2TO+U6GvtqJ3n/RKXUz1ykUKETtRneSkqa6hFYjwRzawd
17+
ANpjy/orrVkqXriAbI/1xvBMInWdcMpXNeiOkxQeQdy8TLBk0ZViSJnf
18+
-----END CERTIFICATE-----

‎test/fixtures/keys/selfsigned-no-keycertsign/https_renew_cert.sh‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
#!/bin/bash
2+
openssl genrsa -out rsa.pem 2048
3+
openssl rsa -in rsa.pem -out key.pem
4+
openssl req -sha256 -new -key key.pem -out csr.pem -subj "/CN=localhost"
5+
openssl x509 -req -extfile cert.conf -extensions v3_req -days 365 -in csr.pem -signkey key.pem -out cert.pem
6+

‎test/fixtures/keys/selfsigned-no-keycertsign/key.pem‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
-----BEGIN RSA PRIVATE KEY-----
2+
MIIEogIBAAKCAQEA0y3pMvD2AjG5xGBkGfyjBq8QgwB13ExOOQeJHuMZ9oGu+9VM
3+
HzKawiLPD9ApMA2a7avcQ6p5H0e3bkpkUrE4oIMx3dooeG3fwcidcgXoJB7xzk/T
4+
GxUpkBaR6WXyiEvTkSwWlysk4Mh0d9qi84Ou1fzDVZ7GoOxSg9B0STukhInYeGZf
5+
Ol/tCZXTRTHwn7ri/alODz8L1WClKDETiheV8kSp1IgDMYNP5Vf3oZZdOHtCouOJ
6+
D1P3pAdQkScHHRKvis72ZjaU5AbjTxC4ItH+eDYn6VjH7TdILwuruWu23gzToV+v
7+
xNBGCkMCORTRiK1u7KwPleC0J4WrQWLe/XPvTQIDAQABAoIBAFIlWMIVE0z1NNLb
8+
v/SP3oaaEK00v6QLFp5+fOtD4fSOq5eQeATmtWZxDeSTz4G+uRZctNipdmYhiovf
9+
ajj0cReXEQ3Ab9+wtcp2lDAndg6e7uaXDIJLcBh5fxawLnCwNkMRSFRTVwwNTajV
10+
pm9dOORKZ11l3tP4OXzG2IUoKy3Wj/1SKLL4zrdHi7802+L/GstK6/BGma+NFrFz
11+
U6yNqpvuzv7BH7w9G3nSz7u+8SjcY22Vs6q69GAQG3yf356cYCJhV7QIJXU0/VAF
12+
GFx5UDwlsOT2NhoOd/b9Q9RexKDl+qDupXQo0YFOObHIjHs8UGLOZkBtv4apCarA
13+
6u+BOwECgYEA9GbrP/5SfmN8xvF2XVjqjk9IUcvWAuTM4Bxav72e6aR9IOdye9vi
14+
+GhwM6qON+LOnMVNhUKJ0+R/jjLy6Jq+00uKU65Q79x7lCBVSDDXWacV0IFIoAOp
15+
P4LkykjRZyzpIvjK5HGL1JYqZi89im93uuOiyMjoFS2syU+19b83UUECgYEA3TNk
16+
JVGWYLMcD3uVTe2e/yZSsX+0+QL8hm3bUSOIJ/mIe2dqCXb6MK0ndMS0aCLGtDSt
17+
wGTWwuc4rFattHYEI8Iro+tshgQs9bLM037hmiCrZvmcQsgt+3FNuYv4oCGp5U85
18+
mWYF5SVUYRyv8M9aZoKTjc8meR0Wv3ZGGC9iDw0CgYA0XKyAPGO+MmB0Wx1J6Jfw
19+
P2o2JB7I5e5DAbArrluSoSwx1YSApt6c6/tGBn+L16r+iYMPTu8ql6UAeUfzr9u8
20+
d02+mfU7Ppi3Zqn+2n/49ERHNLuzlLU5JzkPYcSDf2q/lGAby3vy4u1YkTx1IWac
21+
gtLIg8q9ZtjDFLHeYcZfQQKBgCCOpdjQT1/gPOsSd4FGzjYjv9wcPdjA1cY7eSJS
22+
JoIruijfqb3G40Ay3DHVmfAR3kk7z68XqHx7Z94Fy/9Zt3ZD6ARybEC1cKChNoCS
23+
lkYHNPMtHhC+QfZWUOhUb72x9r2nkYTAfXGisu6wOD0rZ9TatzkSGkmNPIHluJ9q
24+
qfYpAoGAPJiBBdSt7DC9ZZraQGMEHfRkE5CxEIRbIHJ9+U3Z7LTQT6MJ1y3VfcGs
25+
PetHcWtbU0Cl8blShaSwpxyCI01x3tUPw/b7tXMan/ImzjUgRe7kQXh2sf39V3b/
26+
fvzKXWBvOvc1lgG0pFgI/2xtGQQGTe74MzX5xFgw6eadRUnJeKI=
27+
-----END RSA PRIVATE KEY-----

‎test/parallel/test-https-selfsigned-no-keycertsign-no-crash.js‎

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
'use strict';
2+
constcommon=require('../common');
3+
constfixtures=require('../common/fixtures');
4+
5+
// This test starts an https server and tries
6+
// to connect to it using a self-signed certificate.
7+
// This certificate´s keyUsage does not include the keyCertSign
8+
// bit, which used to crash node. The test ensures node
9+
// will not crash. Key and certificate are from #37889.
10+
// Note: This test assumes that the connection will succeed.
11+
12+
if(!common.hasCrypto)
13+
common.skip('missing crypto');
14+
15+
constcrypto=require('crypto');
16+
17+
// See #37990 for details on why this is problematic with FIPS.
18+
if(process.config.variables.openssl_is_fips)
19+
common.skip('Skipping as test uses non-fips compliant EC curve');
20+
21+
// This test will fail for OpenSSL < 1.1.1h
22+
constminOpenSSL=269488271;
23+
24+
if(crypto.constants.OPENSSL_VERSION_NUMBER<minOpenSSL)
25+
common.skip('OpenSSL < 1.1.1h');
26+
27+
consthttps=require('https');
28+
constpath=require('path');
29+
30+
constkey=
31+
fixtures.readKey(path.join('selfsigned-no-keycertsign','key.pem'));
32+
33+
constcert=
34+
fixtures.readKey(path.join('selfsigned-no-keycertsign','cert.pem'));
35+
36+
constserverOptions={
37+
key: key,
38+
cert: cert
39+
};
40+
41+
// Start the server
42+
consthttpsServer=https.createServer(serverOptions,(req,res)=>{
43+
res.writeHead(200);
44+
res.end('hello world\n');
45+
});
46+
httpsServer.listen(0);
47+
48+
httpsServer.on('listening',()=>{
49+
// Once the server started listening, built the client config
50+
// with the server´s used port
51+
constclientOptions={
52+
hostname: '127.0.0.1',
53+
port: httpsServer.address().port,
54+
ca: cert
55+
};
56+
// Try to connect
57+
constreq=https.request(clientOptions,common.mustCall((res)=>{
58+
httpsServer.close();
59+
}));
60+
61+
req.on('error',common.mustNotCall());
62+
req.end();
63+
});

0 commit comments

Comments
 (0)