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

tls: fix reference to certificate object subjectAltName in checkServerIdentity function #42470

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
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
2 changes: 1 addition & 1 deletion doc/api/errors.md
Original file line number Diff line number Diff line change
Expand Up @@ -2564,7 +2564,7 @@ what the test was doing when the failure occurred.
### `ERR_TLS_CERT_ALTNAME_FORMAT`

This error is thrown by `checkServerIdentity` if a user-supplied
`subjectaltname` property violates encoding rules. Certificate objects produced
`subjectAltName` property violates encoding rules. Certificate objects produced
by Node.js itself always comply with encoding rules and will never cause
this error.

Expand Down
4 changes: 2 additions & 2 deletions doc/api/tls.md
Original file line number Diff line number Diff line change
Expand Up @@ -1203,7 +1203,7 @@ certificate.
It is returned as a `:` separated hexadecimal string. Example:
`'2A:7A:C2:DD:...'`.
* `ext_key_usage` {Array} (Optional) The extended key usage, a set of OIDs.
* `subjectaltname` {string} (Optional) A string containing concatenated names
* `subjectAltName` {string} (Optional) A string containing concatenated names
for the subject, an alternative to the `subject` names.
* `infoAccess` {Array} (Optional) An array describing the AuthorityInfoAccess,
used with OCSP.
Expand Down Expand Up @@ -1248,7 +1248,7 @@ Example certificate:
L: 'Salford',
O: 'COMODO CA Limited',
CN: 'COMODO RSA Domain Validation Secure Server CA' },
subjectaltname: 'DNS:*.nodejs.org, DNS:nodejs.org',
subjectAltName: 'DNS:*.nodejs.org, DNS:nodejs.org',
infoAccess:
{ 'CA Issuers - URI':
[ 'http://crt.comodoca.com/COMODORSADomainValidationSecureServerCA.crt' ],
Expand Down
2 changes: 1 addition & 1 deletion lib/tls.js
Original file line number Diff line number Diff line change
Expand Up @@ -271,7 +271,7 @@ function splitEscapedAltNames(altNames) {

exports.checkServerIdentity = function checkServerIdentity(hostname, cert) {
const subject = cert.subject;
const altNames = cert.subjectaltname;
const altNames = cert.subjectAltName;
const dnsNames = [];
const ips = [];

Expand Down
2 changes: 1 addition & 1 deletion src/env.h
Original file line number Diff line number Diff line change
Expand Up @@ -419,7 +419,7 @@ constexpr size_t kFsStatsBufferLength =
V(stream_average_duration_string, "streamAverageDuration") \
V(stream_count_string, "streamCount") \
V(subject_string, "subject") \
V(subjectaltname_string, "subjectaltname") \
V(subjectaltname_string, "subjectAltName") \
V(syscall_string, "syscall") \
V(target_string, "target") \
V(thread_id_string, "threadId") \
Expand Down
2 changes: 1 addition & 1 deletion test/parallel/test-https-foafssl.js
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ const server = https.createServer(options, common.mustCall(function(req, res) {

cert = req.connection.getPeerCertificate();

assert.strictEqual(cert.subjectaltname, webIdUrl);
assert.strictEqual(cert.subjectAltName, webIdUrl);
assert.strictEqual(cert.exponent, exponent);
assert.strictEqual(cert.modulus, modulus);
res.writeHead(200, { 'content-type': 'text/plain' });
Expand Down
2 changes: 1 addition & 1 deletion test/parallel/test-tls-0-dns-altname.js
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ const server = tls.createServer({
rejectUnauthorized: false
}, common.mustCall(() => {
const cert = c.getPeerCertificate();
assert.strictEqual(cert.subjectaltname,
assert.strictEqual(cert.subjectAltName,
'DNS:"good.example.org\\u0000.evil.example.com", ' +
'DNS:just-another.example.com, ' +
'IP Address:8.8.8.8, ' +
Expand Down
56 changes: 28 additions & 28 deletions test/parallel/test-tls-check-server-identity.js
Original file line number Diff line number Diff line change
Expand Up @@ -77,30 +77,30 @@ const tests = [
// IP address is valid but it seems so suspect that we currently reject it.
{
host: '8.8.8.8',
cert: { subject: { CN: '8.8.8.8' }, subjectaltname: 'DNS:8.8.8.8' },
cert: { subject: { CN: '8.8.8.8' }, subjectAltName: 'DNS:8.8.8.8' },
error: 'IP: 8.8.8.8 is not in the cert\'s list: '
},

// Likewise for "URI:" Subject Alternative Names.
// See also https://github.com/nodejs/node/issues/8108.
{
host: '8.8.8.8',
cert: { subject: { CN: '8.8.8.8' }, subjectaltname: 'URI:http://8.8.8.8/' },
cert: { subject: { CN: '8.8.8.8' }, subjectAltName: 'URI:http://8.8.8.8/' },
error: 'IP: 8.8.8.8 is not in the cert\'s list: '
},

// An "IP Address:" Subject Alternative Name however is acceptable.
{
host: '8.8.8.8',
cert: { subject: { CN: '8.8.8.8' }, subjectaltname: 'IP Address:8.8.8.8' }
cert: { subject: { CN: '8.8.8.8' }, subjectAltName: 'IP Address:8.8.8.8' }
},

// But not when it's a CIDR.
{
host: '8.8.8.8',
cert: {
subject: { CN: '8.8.8.8' },
subjectaltname: 'IP Address:8.8.8.0/24'
subjectAltName: 'IP Address:8.8.8.0/24'
},
error: 'IP: 8.8.8.8 is not in the cert\'s list: '
},
Expand All @@ -119,7 +119,7 @@ const tests = [
},
{ host: 'b.a.com',
cert: {
subjectaltname: 'DNS:omg.com',
subjectAltName: 'DNS:omg.com',
subject: { CN: '*.a.com' },
},
error: 'Host: b.a.com. is not in the cert\'s altnames: ' +
Expand All @@ -140,14 +140,14 @@ const tests = [
// Empty Subject w/DNS name
{
host: 'a.com', cert: {
subjectaltname: 'DNS:a.com',
subjectAltName: 'DNS:a.com',
}
},

// Empty Subject w/URI name
{
host: 'a.b.a.com', cert: {
subjectaltname: 'URI:http://a.b.a.com/',
subjectAltName: 'URI:http://a.b.a.com/',
},
error: 'Cert does not contain a DNS name'
},
Expand All @@ -162,93 +162,93 @@ const tests = [
// DNS names and CN
{
host: 'a.com', cert: {
subjectaltname: 'DNS:*',
subjectAltName: 'DNS:*',
subject: { CN: 'b.com' }
},
error: 'Host: a.com. is not in the cert\'s altnames: ' +
'DNS:*'
},
{
host: 'a.com', cert: {
subjectaltname: 'DNS:*.com',
subjectAltName: 'DNS:*.com',
subject: { CN: 'b.com' }
},
error: 'Host: a.com. is not in the cert\'s altnames: ' +
'DNS:*.com'
},
{
host: 'a.co.uk', cert: {
subjectaltname: 'DNS:*.co.uk',
subjectAltName: 'DNS:*.co.uk',
subject: { CN: 'b.com' }
}
},
{
host: 'a.com', cert: {
subjectaltname: 'DNS:*.a.com',
subjectAltName: 'DNS:*.a.com',
subject: { CN: 'a.com' }
},
error: 'Host: a.com. is not in the cert\'s altnames: ' +
'DNS:*.a.com'
},
{
host: 'a.com', cert: {
subjectaltname: 'DNS:*.a.com',
subjectAltName: 'DNS:*.a.com',
subject: { CN: 'b.com' }
},
error: 'Host: a.com. is not in the cert\'s altnames: ' +
'DNS:*.a.com'
},
{
host: 'a.com', cert: {
subjectaltname: 'DNS:a.com',
subjectAltName: 'DNS:a.com',
subject: { CN: 'b.com' }
}
},
{
host: 'a.com', cert: {
subjectaltname: 'DNS:A.COM',
subjectAltName: 'DNS:A.COM',
subject: { CN: 'b.com' }
}
},

// DNS names
{
host: 'a.com', cert: {
subjectaltname: 'DNS:*.a.com',
subjectAltName: 'DNS:*.a.com',
subject: {}
},
error: 'Host: a.com. is not in the cert\'s altnames: ' +
'DNS:*.a.com'
},
{
host: 'b.a.com', cert: {
subjectaltname: 'DNS:*.a.com',
subjectAltName: 'DNS:*.a.com',
subject: {}
}
},
{
host: 'c.b.a.com', cert: {
subjectaltname: 'DNS:*.a.com',
subjectAltName: 'DNS:*.a.com',
subject: {}
},
error: 'Host: c.b.a.com. is not in the cert\'s altnames: ' +
'DNS:*.a.com'
},
{
host: 'b.a.com', cert: {
subjectaltname: 'DNS:*b.a.com',
subjectAltName: 'DNS:*b.a.com',
subject: {}
}
},
{
host: 'a-cb.a.com', cert: {
subjectaltname: 'DNS:*b.a.com',
subjectAltName: 'DNS:*b.a.com',
subject: {}
}
},
{
host: 'a.b.a.com', cert: {
subjectaltname: 'DNS:*b.a.com',
subjectAltName: 'DNS:*b.a.com',
subject: {}
},
error: 'Host: a.b.a.com. is not in the cert\'s altnames: ' +
Expand All @@ -257,57 +257,57 @@ const tests = [
// Multiple DNS names
{
host: 'a.b.a.com', cert: {
subjectaltname: 'DNS:*b.a.com, DNS:a.b.a.com',
subjectAltName: 'DNS:*b.a.com, DNS:a.b.a.com',
subject: {}
}
},
// URI names
{
host: 'a.b.a.com', cert: {
subjectaltname: 'URI:http://a.b.a.com/',
subjectAltName: 'URI:http://a.b.a.com/',
subject: {}
},
error: 'Cert does not contain a DNS name'
},
{
host: 'a.b.a.com', cert: {
subjectaltname: 'URI:http://*.b.a.com/',
subjectAltName: 'URI:http://*.b.a.com/',
subject: {}
},
error: 'Cert does not contain a DNS name'
},
// IP addresses
{
host: 'a.b.a.com', cert: {
subjectaltname: 'IP Address:127.0.0.1',
subjectAltName: 'IP Address:127.0.0.1',
subject: {}
},
error: 'Cert does not contain a DNS name'
},
{
host: '127.0.0.1', cert: {
subjectaltname: 'IP Address:127.0.0.1',
subjectAltName: 'IP Address:127.0.0.1',
subject: {}
}
},
{
host: '127.0.0.2', cert: {
subjectaltname: 'IP Address:127.0.0.1',
subjectAltName: 'IP Address:127.0.0.1',
subject: {}
},
error: 'IP: 127.0.0.2 is not in the cert\'s list: ' +
'127.0.0.1'
},
{
host: '127.0.0.1', cert: {
subjectaltname: 'DNS:a.com',
subjectAltName: 'DNS:a.com',
subject: {}
},
error: 'IP: 127.0.0.1 is not in the cert\'s list: '
},
{
host: 'localhost', cert: {
subjectaltname: 'DNS:a.com',
subjectAltName: 'DNS:a.com',
subject: { CN: 'localhost' }
},
error: 'Host: localhost. is not in the cert\'s altnames: ' +
Expand Down
10 changes: 5 additions & 5 deletions test/parallel/test-x509-escaping.js
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ const { hasOpenSSL3 } = common;
assert.strictEqual(cert.subjectAltName, expectedSANs[i]);

// Test that the certificate obtained by checkServerIdentity has the correct
// subjectaltname property.
// subjectAltName property.
const server = tls.createServer({
key: serverKey,
cert: pem,
Expand All @@ -131,7 +131,7 @@ const { hasOpenSSL3 } = common;
servername: 'example.com',
checkServerIdentity: (hostname, peerCert) => {
assert.strictEqual(hostname, 'example.com');
assert.strictEqual(peerCert.subjectaltname, expectedSANs[i]);
assert.strictEqual(peerCert.subjectAltName, expectedSANs[i]);
},
}, common.mustCall());
}));
Expand Down Expand Up @@ -224,7 +224,7 @@ const { hasOpenSSL3 } = common;
`${expected.text}${hasOpenSSL3 ? '' : '\n'}`);

// Test that the certificate obtained by checkServerIdentity has the correct
// subjectaltname property.
// subjectAltName property.
const server = tls.createServer({
key: serverKey,
cert: pem,
Expand Down Expand Up @@ -400,7 +400,7 @@ const { hasOpenSSL3 } = common;
// alternative name strings (that do not follow escaping rules).
assert.throws(() => {
tls.checkServerIdentity('example.com', {
subjectaltname: `DNS:${invalidStringLiteral}`,
subjectAltName: `DNS:${invalidStringLiteral}`,
});
}, {
code: 'ERR_TLS_CERT_ALTNAME_FORMAT',
Expand All @@ -422,7 +422,7 @@ const { hasOpenSSL3 } = common;
assert.strictEqual(san.split(', ')[1], `DNS:${hostname}`);

// The new implementation should parse the string correctly.
const err = tls.checkServerIdentity(hostname, { subjectaltname: san });
const err = tls.checkServerIdentity(hostname, { subjectAltName: san });
assert(err);
assert.strictEqual(err.code, 'ERR_TLS_CERT_ALTNAME_INVALID');
assert.strictEqual(err.message, 'Hostname/IP does not match certificate\'s ' +
Expand Down