Skip to content

Navigation Menu

Sign in
Appearance settings

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

Provide feedback

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

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 195f679

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

File tree

Expand file treeCollapse file tree

7 files changed

+141
-0
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

7 files changed

+141
-0
lines changed
Open diff view settings
Collapse file

‎src/crypto/crypto_common.cc‎

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

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

Copy file name to clipboard
+2Lines changed: 2 additions & 0 deletions
  • Display the source diff
  • Display the rich diff
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.
Collapse file

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

Copy file name to clipboard
+17Lines 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
Collapse file

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

Copy file name to clipboard
+18Lines 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-----
Collapse file

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

Copy file name to clipboard
+6Lines 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+
Collapse file

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

Copy file name to clipboard
+27Lines 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-----
Collapse file

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

Copy file name to clipboard
+63Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
'use strict';
2+
const common = require('../common');
3+
const fixtures = 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+
const crypto = 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+
const minOpenSSL = 269488271;
23+
24+
if (crypto.constants.OPENSSL_VERSION_NUMBER < minOpenSSL)
25+
common.skip('OpenSSL < 1.1.1h');
26+
27+
const https = require('https');
28+
const path = require('path');
29+
30+
const key =
31+
fixtures.readKey(path.join('selfsigned-no-keycertsign', 'key.pem'));
32+
33+
const cert =
34+
fixtures.readKey(path.join('selfsigned-no-keycertsign', 'cert.pem'));
35+
36+
const serverOptions = {
37+
key: key,
38+
cert: cert
39+
};
40+
41+
// Start the server
42+
const httpsServer = 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+
const clientOptions = {
52+
hostname: '127.0.0.1',
53+
port: httpsServer.address().port,
54+
ca: cert
55+
};
56+
// Try to connect
57+
const req = 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 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.