Skip to content

Root certificate is prioritized over SNICallback context in HTTPS Server #54235

Description

@Ethan-Arrowood

Version

22.5.1

Platform

Darwin Ethan 23.4.0 Darwin Kernel Version 23.4.0: Wed Feb 21 21:44:54 PST 2024; root:xnu-10063.101.15~2/RELEASE_ARM64_T6030 arm64

(Reproducible on other Node.js versions and platforms as well)

Subsystem

tls, https

What steps will reproduce the bug?

Edit: I've added a test that reproduces the issue here: #54251

Old Example Repro

Note: The actual common names (CN) have been replaced with fake strings. To run this yourself you will need to provide your own cert/key pairs (1 generated locally using openssl, 1 or 2 generated from Let's Encrypt).

import https from "node:https";
import fs from "node:fs";
import tls from "node:tls";
import events from "node:events";
import dns from "node:dns";

// Generated locally using `openssl` (CN=test.cloud.com)
let test = {
	cert: fs.readFileSync("certs/test.cloud.com.crt"),
	key: fs.readFileSync("certs/test.cloud.com.key"),
};

// Generated by Let's Encrypt (CN=foo.cloud.com)
let foo = {
	cert: fs.readFileSync("certs/foo.cloud.com/cert.pem"),
	key: fs.readFileSync("certs/foo.cloud.com/key.pem"),
};

// Generated by Let's Encrypt (CN=bar.cloud.com)
let bar = {
	cert: fs.readFileSync("certs/bar.cloud.com/cert.pem"),
	key: fs.readFileSync("certs/bar.cloud.com/key.pem"),
};

let contexts = new Map([
	['test.cloud.com', tls.createSecureContext(test)],
	['foo.cloud.com', tls.createSecureContext(foo)],
	['bar.cloud.com', tls.createSecureContext(bar)],
])

let server = https.createServer({
        // Set the HTTPS Server Default Cert and Key
	cert: foo.cert,
	key: foo.key,
        // SNICallback that returns key/cert pair 
	SNICallback: (servername, cb) => {
		cb(null, contexts.get(servername))
	}
}, (req, res) => {
	console.log(`Server processing request from ${req.socket.servername}`);
	res.writeHead(200);
	res.end(`Hello, World!`);
});

// You can also do this using `/etc/hosts` i.e.:
// `127.0.0.1 foo.cloud.com bar.cloud.com test.cloud.com`
https.globalAgent = new https.Agent({
	lookup: (hostname, options, cb) => {
		if (hostname === "foo.cloud.com" || hostname === "bar.cloud.com" || hostname === "test.cloud.com")
			hostname = "localhost";
		return dns.lookup(hostname, options, cb);
	},
});

server.listen(8443);

await events.once(server, 'listening');

console.log('Server is ready');

function makeRequest (hostname) {
	console.log(`\nRequesting https://${hostname}`);

	return new Promise((resolve, reject) => {
		https.get(`https://${hostname}:8443`, { rejectUnauthorized: false }, (res) => {
			let cert = res.socket.getPeerCertificate();
	
			console.log('Certificate subject', cert.subject);
			console.log('Certificate issuer', cert.issuer);
			console.log('Certificate subjectaltname', cert.subjectaltname);
	
			res.on('data', (chunk) => {
				console.log(`Response message: ${chunk.toString('utf-8')}`);
				resolve();
			});
	
			res.on("error", (e) => {
				console.log(`Response Error: ${e.message}`);
				reject(e);
			});
		}).on("error", (e) => {
			console.log(`https.get Error: ${e.message}`);
			reject(e);
		});
	});
}

await makeRequest('test.cloud.com')
await makeRequest('foo.cloud.com')
await makeRequest('bar.cloud.com')
await makeRequest('127.0.0.1') // Also need to support non SNI requests

server.close();

Output:

❯ node repro.mjs
Server is ready

Requesting https://test.cloud.com
Server processing request from test.cloud.com
Certificate subject { CN: 'foo.cloud.com' }  # <- Expect this to be test.cloud.com
Certificate issuer {
  C: 'US',
  O: "(STAGING) Let's Encrypt",
  CN: '(STAGING) Pseudo Plum E5'
}
Certificate subjectaltname DNS:foo.cloud.com # <- Expect this to be test.cloud.com
Response message: Hello, World!

Requesting https://foo.cloud.com
Server processing request from foo.cloud.com
Certificate subject { CN: 'foo.cloud.com' } # <- this is correct
Certificate issuer {
  C: 'US',
  O: "(STAGING) Let's Encrypt",
  CN: '(STAGING) Pseudo Plum E5'
}
Certificate subjectaltname DNS:foo.cloud.com # <- this is correct
Response message: Hello, World!

Requesting https://bar.cloud.com
Server processing request from bar.cloud.com
Certificate subject { CN: 'bar.cloud.com' } # <- this is correct
Certificate issuer {
  C: 'US',
  O: "(STAGING) Let's Encrypt",
  CN: '(STAGING) False Fennel E6'
}
Certificate subjectaltname DNS:bar.cloud.com # <- this is correct
Response message: Hello, World!

Requesting https://127.0.0.1
Server processing request from false # <- no req.socket.servername is expected
Certificate subject { CN: 'foo.cloud.com' } # <- this is correct
Certificate issuer {
  C: 'US',
  O: "(STAGING) Let's Encrypt",
  CN: '(STAGING) Pseudo Plum E5'
}
Certificate subjectaltname DNS:foo.cloud.com # <- this is correct
Response message: Hello, World!

How often does it reproduce? Is there a required condition?

This bug only seems to happen when the root cert/key pair is from Let's Encrypt. We ran into this issue with a Digicert first, but were able to reproduce using a locally generated certificate too.

I've further narrowed down the conditions. It seems that certain certificates (those with better CA or maybe created using an ext) will be prioritized over others without those things. It is not directly related to a certain provider. The reproduction demonstrates that the test/fixtures/keys/ca5-cert.pem is prioritized over test/fixtures/keys/agent1-cert.pem.

What is the expected behavior? Why is that the expected behavior?

We expect the correct certificate to be used every time.

Additional information

Other issues/prs I reviewed:

I also asked Claude AI about this to see if it could help me along and it replied with some interesting points:

1. Certificate Priority:
Some HTTPS implementations give priority to certain types of certificates. Let's Encrypt certificates are widely trusted and might be given precedence over locally generated ones.
2. SNI Implementation:
The SNI (Server Name Indication) callback is typically used to select the appropriate certificate after the initial TLS handshake has begun. If the server has already selected a default certificate (in this case, the Let's Encrypt one), it might not switch to the one returned by the SNI callback.
3. Certificate Chain:
Let's Encrypt certificates come with a full certificate chain that establishes trust up to a root CA. Locally generated certificates might not have this, potentially influencing the server's decision.
4. TLS/SSL Library Behavior:
The underlying TLS/SSL library (like OpenSSL) might have specific behaviors or optimizations that prefer certain certificate types or structures.

Maybe that'll help folks who understand SNI better 🤷‍♂️

Activity

  1. added
    tlsIssues and PRs related to the tls subsystem.
    httpsIssues and PRs related to the https subsystem.
    opensslIssues and PRs related to the OpenSSL dependency.
    on Aug 6, 2024
  2. Ethan-Arrowood commented on Aug 7, 2024

    @Ethan-Arrowood
    ContributorAuthor

    Edit: this issue doesn't seem strictly related to Let's Encrypt. I was able to create a locally generated cert that is prioritized. I think this highlights that this is maybe not a bug but a poorly documented feature of OpenSSL? If anyone knows anything about certificate prioritization, more information would be greatly appreciated. I'm working on a Node.js regression test too that hopefully demonstrates it.

  3. changed the title [-]Let's Encrypt based root certificate is prioritized over SNICallback context in HTTPS Server[/-] [+]Root certificate is prioritized over SNICallback context in HTTPS Server[/+] on Aug 7, 2024
  4. Ethan-Arrowood commented on Aug 12, 2024

    @Ethan-Arrowood
    ContributorAuthor

    @nodejs/http @nodejs/net any ideas?

  5. Ethan-Arrowood commented on Aug 15, 2024

    @Ethan-Arrowood
    ContributorAuthor

    I did a test using uWebSockets and this doesn't seem reproducible there:

    const app = uWS.SSLApp({
      key_file_name: "example.com-key.pem",
      cert_file_name: 'example.com.pem',
    }).addServerName('test.cloud.com', {
      key_file_name: 'test.cloud.com.key',
      cert_file_name: 'test.cloud.com.crt'
    }).get('/*', (res, req) => {
      res.end('Hello World!');
    }).listen(port, (token) => {
      if (token) {
        console.log('Listening to port ' + port);
      } else {
        console.log('Failed to listen to port ' + port);
      }
    });

    Requests to this app will get the correct certificates back regardless of the certificate content.

    I think this helps point the issue back towards Node.js and its usage of OpenSSL.

  6. Ethan-Arrowood commented on Aug 19, 2024

    @Ethan-Arrowood
    ContributorAuthor

    I did a debug step-through using the test I wrote in #54251 here is what I found:

    Starting off with the SNICallback flow:

    • node/lib/_tls_wrap.js

      Lines 214 to 230 in e4f61de

      owner._SNICallback(servername, (err, context) => {
      if (once)
      return owner.destroy(new ERR_MULTIPLE_CALLBACK());
      once = true;
      if (err)
      return owner.destroy(err);
      if (owner._handle === null)
      return owner.destroy(new ERR_SOCKET_CLOSED());
      // TODO(indutny): eventually disallow raw `SecureContext`
      if (context)
      owner._handle.sni_context = context.context || context;
      requestOCSP(owner, info);
      });
      This is where the SNICallback is actually executed, on line 227, the context returned by the callback is assigned to the sni_context of the socket
    • Following that, the requestOCSP() method is called with the updated socket + info as {servername: '[agent1.com](http://agent1.com/)', OCSPRequest: false}
    • Stepping into that method we hit the very first conditional (
      if (!info.OCSPRequest || !socket.server)
      ) which enters into requestOCSPDone(socket)
    • This small function (
      socket._handle.certCbDone();
      ) calls into a native method certCbDone() defined here: https://github.com/nodejs/node/blob/main/src/crypto/crypto_tls.cc#L1553

    Stepping through TLSWrap::CertCBDone:

    • There are two properties on TLSWrap::SecureContext, sni_context_ and sc_ and these are two separate certificates.
    • sc_ is the default certificate for that server, meanwhile sni_context_ is in fact the correct certificate for the given servername (see screenshot for proof).
    Screenshot 2024-08-16 at 2 44 15 PM - The next important call is to [UseSNIContext](https://github.com/nodejs/node/blob/e4f61de14f8cfb83f1ce0ad1597b86278cd5f5f1/src/crypto/crypto_common.cc#L118-L131) called from here: https://github.com/nodejs/node/blob/e4f61de14f8cfb83f1ce0ad1597b86278cd5f5f1/src/crypto/crypto_tls.cc#L1573 - That function is what is doing the actual certificate resolution for the request. It makes calls to SSL and as far as I can tell it is up to SSL's resolution strategy for which certificate in a chain gets used. This is the unexpected behavior I'm struggling to understand as either "intended behavior" or a "bug" of `SNICallback`/`addContext`
  7. Ethan-Arrowood commented on Aug 19, 2024

    @Ethan-Arrowood
    ContributorAuthor

    I am considering a fix to UseSNIContext. I'm digging into the SSL calls now to understand if maybe theres a better way to call them for this.

  8. Ethan-Arrowood commented on Aug 21, 2024

    @Ethan-Arrowood
    ContributorAuthor

    @indutny do you happen to know whats going on here? I'm still trying to determine if there is a better way to implement UseSNIContext or not. This is my first time working with OpenSSL directly.

  9. Ethan-Arrowood commented on Aug 21, 2024

    @Ethan-Arrowood
    ContributorAuthor

    I guess a code change may not be necessary here. As far as I can tell this is about the OpenSSL Verification configuration (https://docs.openssl.org/3.0/man1/openssl-verification-options/).

    I think I could try using these options to get the certs to play nice.

    I'd still appreciate someone with more OpenSSL experience to weigh in here whether this is a bug or not.

    Thank you!

  10. Ethan-Arrowood commented on Jun 4, 2025

    @Ethan-Arrowood
    ContributorAuthor

    I know this has been stagnant for awhile, but why was repro-exists removed? I have a regression test case here proving my issue: #54251

  11. mcollina commented on Jun 4, 2025

    @mcollina
    SponsorMember

    @Ethan-Arrowood I think the best solution would be for you to send a PR.

  12. Ethan-Arrowood commented on Jun 4, 2025

    @Ethan-Arrowood
    ContributorAuthor

    Yes of course, given the test case in #54251, at the time I tried some solutions - particularly trying to configure some of these options: https://docs.openssl.org/3.0/man1/openssl-verification-options/#description, but I couldn't get anything to satisfy the test case. Hence my last request for someone with more OpenSSL experience to take a look

  13. mikuso commented on Jul 22, 2026

    @mikuso

    Hi @Ethan-Arrowood , I think I have encountered this same bug.

    In my experiments, the bug seems to occur when the certificate/key type in the SNICallback context differs from the certificate/key type in the default context. (i.e. an SNICallback returning a context with EC keys is not compatible with a default context that uses RSA keys (and vice versa)). The context returned in the SNICallback will only take effect if its key type is the same as the key type used in the default context, otherwise the default is used instead. (I think this is what you mean by "prioritized".)

    I have tested this with various pairings of EC and RSA certs, and the bug only (and always) occurs for me when the SNICallback returns a context using a key type different from the default.

    In your test case, I can see that the certificate for ca5 uses an EC key, and agent1 uses an RSA key - so this is consistent with the behaviour exhibited in your test case too.

    In the case of dual-certificate contexts (i.e. where the context is created using both EC and RSA keys/certs (for compatibility with clients that only support one or the other)) - the default context works fine, and selects the appropriate keypair according to the client's supported ciphers and signature algorithms, but the context returned from SNICallback doesn't do this correctly, exhibiting the following quirk:

    • If the SNICallback context is constructed with the EC key/cert added first and the RSA key/cert added second, then:
      • If the client only supports RSA ciphers, then the server will use the SNI context's RSA key/cert correctly.
      • If the client only supports EC ciphers, then the server will default to using the EC key/cert of the default context.
    • Likewise, if the SNICallback context is constructed in the opposite order (RSA first, EC second), then:
      • If the client only supports RSA ciphers, then the server will default to using the RSA key/cert of the default context.
      • If the client only supports EC ciphers, then the server will use the SNI context's EC key/cert correctly.
  14. mikuso commented on Jul 22, 2026

    @mikuso

    I have gone back to see when this bug was first introduced (or to see if it has always been this way).

    I can confirm that this bug first appeared in nodejs v12.0.0.
    It does not occur in v11.15.0.

    This major version also coincides with the upgrade to OpenSSL 1.1.1b, which may be relevant.

  15. mikuso commented on Jul 22, 2026

    @mikuso

    Making a bit further progress...

    I have discovered that this bug is related to the version of TLS being used.

    If the https server is created with the option maxVersion: 'TLSv1.2', then the bug does not occur. (You can retry your test case with this option and you will see that it now passes).

    The bug only occurs when the client uses TLSv1.3.

    This explains why I didn't see the bug in nodejs v11.15.0, as tls.DEFAULT_MAX_VERSION was TLSv1.2 until nodejs v12.0.0 when TLSv1.3 support was added and it became the new default.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    confirmed-bugIssues and PRs for confirmed bugs.httpsIssues and PRs related to the https subsystem.opensslIssues and PRs related to the OpenSSL dependency.tlsIssues and PRs related to the tls subsystem.

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions