Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix node crashing due to endless loop #37757 #37990

Closed
wants to merge 14 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions src/crypto/crypto_common.cc
Original file line number Diff line number Diff line change
Expand Up @@ -480,8 +480,16 @@ MaybeLocal<Object> GetLastIssuedCert(
return MaybeLocal<Object>();
issuer_chain = ca_info;

// Take the value of cert->get() before the call to cert->reset()
// in order to compare it to ca after and provide a way to exit this loop
// in case it gets stuck.
X509* value_before_reset = cert->get();

// Delete previous cert and continue aggregating issuers.
cert->reset(ca);

if (value_before_reset == ca)
break;
}
return MaybeLocal<Object>(issuer_chain);
}
Expand Down
2 changes: 2 additions & 0 deletions test/fixtures/keys/selfsigned-no-keycertsign/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
# Self-signed certificate without keyCertSign bit
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.
17 changes: 17 additions & 0 deletions test/fixtures/keys/selfsigned-no-keycertsign/cert.conf
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
[req]
distinguished_name = req_distinguished_name
req_extensions = v3_req
prompt = no

[req_distinguished_name]
C = DE
CN = localhost

[v3_req]
keyUsage = digitalSignature, keyEncipherment
extendedKeyUsage = serverAuth
subjectAltName = @alt_names
[alt_names]
DNS.1 = 127.0.0.1
DNS.2 = localhost
IP.1 = 127.0.0.1
18 changes: 18 additions & 0 deletions test/fixtures/keys/selfsigned-no-keycertsign/cert.pem
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
-----BEGIN CERTIFICATE-----
MIIC9jCCAd6gAwIBAgIJANHflGRpZM1IMA0GCSqGSIb3DQEBCwUAMBQxEjAQBgNV
BAMMCWxvY2FsaG9zdDAeFw0yMTAzMTUwOTEzMjdaFw0yMjAzMTUwOTEzMjdaMBQx
EjAQBgNVBAMMCWxvY2FsaG9zdDCCASIwDQYJKoZIhvcNAQEBBQADggEPADCCAQoC
ggEBANMt6TLw9gIxucRgZBn8owavEIMAddxMTjkHiR7jGfaBrvvVTB8ymsIizw/Q
KTANmu2r3EOqeR9Ht25KZFKxOKCDMd3aKHht38HInXIF6CQe8c5P0xsVKZAWkell
8ohL05EsFpcrJODIdHfaovODrtX8w1WexqDsUoPQdEk7pISJ2HhmXzpf7QmV00Ux
8J+64v2pTg8/C9VgpSgxE4oXlfJEqdSIAzGDT+VX96GWXTh7QqLjiQ9T96QHUJEn
Bx0Sr4rO9mY2lOQG408QuCLR/ng2J+lYx+03SC8Lq7lrtt4M06Ffr8TQRgpDAjkU
0YitbuysD5XgtCeFq0Fi3v1z700CAwEAAaNLMEkwCwYDVR0PBAQDAgWgMBMGA1Ud
JQQMMAoGCCsGAQUFBwMBMCUGA1UdEQQeMByCCTEyNy4wLjAuMYIJbG9jYWxob3N0
hwR/AAABMA0GCSqGSIb3DQEBCwUAA4IBAQDAUCt/8Le2EO0ONOkQYUcPmSut6Siz
UIQrJ8Lwfs0fb+Zk9ElNGLwYTzooKDgzK8cLQ8g8F2WkolBEPXDsy1Ab+e66WkJH
NH/zAgEyG6cXXRNc+ObM5KbjY0YuDGiajKcndknuuCB+onlC1Pv5oFUSNa3/06+S
sziFloGbg5S0AHT6lYnwZSM6G7Pre8mcRNRxL6Yw1FOOUpQZKPd7juy4GBRlCucn
wmp/Fl0wIBDs91Vprig2TO+U6GvtqJ3n/RKXUz1ykUKETtRneSkqa6hFYjwRzawd
ANpjy/orrVkqXriAbI/1xvBMInWdcMpXNeiOkxQeQdy8TLBk0ZViSJnf
-----END CERTIFICATE-----
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
#!/bin/bash
openssl genrsa -out rsa.pem 2048
openssl rsa -in rsa.pem -out key.pem
openssl req -sha256 -new -key key.pem -out csr.pem -subj "/CN=localhost"
openssl x509 -req -extfile cert.conf -extensions v3_req -days 365 -in csr.pem -signkey key.pem -out cert.pem

27 changes: 27 additions & 0 deletions test/fixtures/keys/selfsigned-no-keycertsign/key.pem
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
-----BEGIN RSA PRIVATE KEY-----
MIIEogIBAAKCAQEA0y3pMvD2AjG5xGBkGfyjBq8QgwB13ExOOQeJHuMZ9oGu+9VM
HzKawiLPD9ApMA2a7avcQ6p5H0e3bkpkUrE4oIMx3dooeG3fwcidcgXoJB7xzk/T
GxUpkBaR6WXyiEvTkSwWlysk4Mh0d9qi84Ou1fzDVZ7GoOxSg9B0STukhInYeGZf
Ol/tCZXTRTHwn7ri/alODz8L1WClKDETiheV8kSp1IgDMYNP5Vf3oZZdOHtCouOJ
D1P3pAdQkScHHRKvis72ZjaU5AbjTxC4ItH+eDYn6VjH7TdILwuruWu23gzToV+v
xNBGCkMCORTRiK1u7KwPleC0J4WrQWLe/XPvTQIDAQABAoIBAFIlWMIVE0z1NNLb
v/SP3oaaEK00v6QLFp5+fOtD4fSOq5eQeATmtWZxDeSTz4G+uRZctNipdmYhiovf
ajj0cReXEQ3Ab9+wtcp2lDAndg6e7uaXDIJLcBh5fxawLnCwNkMRSFRTVwwNTajV
pm9dOORKZ11l3tP4OXzG2IUoKy3Wj/1SKLL4zrdHi7802+L/GstK6/BGma+NFrFz
U6yNqpvuzv7BH7w9G3nSz7u+8SjcY22Vs6q69GAQG3yf356cYCJhV7QIJXU0/VAF
GFx5UDwlsOT2NhoOd/b9Q9RexKDl+qDupXQo0YFOObHIjHs8UGLOZkBtv4apCarA
6u+BOwECgYEA9GbrP/5SfmN8xvF2XVjqjk9IUcvWAuTM4Bxav72e6aR9IOdye9vi
+GhwM6qON+LOnMVNhUKJ0+R/jjLy6Jq+00uKU65Q79x7lCBVSDDXWacV0IFIoAOp
P4LkykjRZyzpIvjK5HGL1JYqZi89im93uuOiyMjoFS2syU+19b83UUECgYEA3TNk
JVGWYLMcD3uVTe2e/yZSsX+0+QL8hm3bUSOIJ/mIe2dqCXb6MK0ndMS0aCLGtDSt
wGTWwuc4rFattHYEI8Iro+tshgQs9bLM037hmiCrZvmcQsgt+3FNuYv4oCGp5U85
mWYF5SVUYRyv8M9aZoKTjc8meR0Wv3ZGGC9iDw0CgYA0XKyAPGO+MmB0Wx1J6Jfw
P2o2JB7I5e5DAbArrluSoSwx1YSApt6c6/tGBn+L16r+iYMPTu8ql6UAeUfzr9u8
d02+mfU7Ppi3Zqn+2n/49ERHNLuzlLU5JzkPYcSDf2q/lGAby3vy4u1YkTx1IWac
gtLIg8q9ZtjDFLHeYcZfQQKBgCCOpdjQT1/gPOsSd4FGzjYjv9wcPdjA1cY7eSJS
JoIruijfqb3G40Ay3DHVmfAR3kk7z68XqHx7Z94Fy/9Zt3ZD6ARybEC1cKChNoCS
lkYHNPMtHhC+QfZWUOhUb72x9r2nkYTAfXGisu6wOD0rZ9TatzkSGkmNPIHluJ9q
qfYpAoGAPJiBBdSt7DC9ZZraQGMEHfRkE5CxEIRbIHJ9+U3Z7LTQT6MJ1y3VfcGs
PetHcWtbU0Cl8blShaSwpxyCI01x3tUPw/b7tXMan/ImzjUgRe7kQXh2sf39V3b/
fvzKXWBvOvc1lgG0pFgI/2xtGQQGTe74MzX5xFgw6eadRUnJeKI=
-----END RSA PRIVATE KEY-----
63 changes: 63 additions & 0 deletions test/parallel/test-https-selfsigned-no-keycertsign-no-crash.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
'use strict';
const common = require('../common');
const fixtures = require('../common/fixtures');

// This test starts an https server and tries
// to connect to it using a self-signed certificate.
// This certificate´s keyUsage does not include the keyCertSign
// bit, which used to crash node. The test ensures node
// will not crash. Key and certificate are from #37889.
// Note: This test assumes that the connection will succeed.

if (!common.hasCrypto)
common.skip('missing crypto');

const crypto = require('crypto');

// See #37990 for details on why this is problematic with FIPS.
if (process.config.variables.openssl_is_fips)
common.skip('Skipping as test uses non-fips compliant EC curve');

// This test will fail for OpenSSL < 1.1.1h
const minOpenSSL = 269488271;

if (crypto.constants.OPENSSL_VERSION_NUMBER < minOpenSSL)
common.skip('OpenSSL < 1.1.1h');

const https = require('https');
const path = require('path');

const key =
fixtures.readKey(path.join('selfsigned-no-keycertsign', 'key.pem'));

const cert =
fixtures.readKey(path.join('selfsigned-no-keycertsign', 'cert.pem'));

const serverOptions = {
key: key,
cert: cert
};

// Start the server
const httpsServer = https.createServer(serverOptions, (req, res) => {
res.writeHead(200);
res.end('hello world\n');
});
httpsServer.listen(0);

httpsServer.on('listening', () => {
// Once the server started listening, built the client config
// with the server´s used port
const clientOptions = {
hostname: '127.0.0.1',
port: httpsServer.address().port,
ca: cert
};
// Try to connect
const req = https.request(clientOptions, common.mustCall((res) => {
httpsServer.close();
}));

req.on('error', common.mustNotCall());
req.end();
});