Repository: node
Issue: Root certificate is prioritized over SNICallback context in HTTPS Server
Labels: tls,https,openssl

### Version

22.5.1

### Platform

```text
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: https://github.com/nodejs/node/pull/54251

<details>
<summary>Old Example Repro</summary>

> 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).

```js
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!
```
</details>

### 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:
- https://github.com/nodejs/node/issues/4464
- https://github.com/nodejs/node/pull/23644
- https://github.com/nodejs/node/pull/23811
- https://github.com/nodejs/node/pull/36072
- https://stackoverflow.com/questions/29604697/i-cannot-use-snicallback

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 🤷‍♂️