tls: fix SNICallback certificate selection

Signed-off-by: Matteo Collina <hello@matteocollina.com>
PR-URL: #64700
Reviewed-By: Daniel Lemire <daniel@lemire.me>
Reviewed-By: Tim Perry <pimterry@gmail.com>

Author: mcollina

deps/ncrypto/ncrypto.cc

@@ -4083,7 +4083,13 @@ bool SSLPointer::setSniContext(const SSLCtxPointer& ctx) const {
 EVP_PKEY* pkey = SSL_CTX_get0_privatekey(ctx.get());
 STACK_OF(X509) * chain;
 int err = SSL_CTX_get0_chain_certs(ctx.get(), &chain);
-if (err == 1) err = SSL_use_certificate(get(), x509);
+if (err == 1) {
+// SSL_use_certificate replaces only the certificate matching the key
+// type. Clear all existing certificates so credentials from the default
+// context cannot be selected for a different key type.
+SSL_certs_clear(get());
+ err = SSL_use_certificate(get(), x509);
+ }
 if (err == 1) err = SSL_use_PrivateKey(get(), pkey);
 if (err == 1 && chain != nullptr) err = SSL_set1_chain(get(), chain);
 return err == 1;

test/parallel/test-https-snicallback-override.js

@@ -0,0 +1,71 @@
+'use strict';
+
+constcommon=require('../common');
+if(!common.hasCrypto)
+common.skip('missing crypto');
+
+constassert=require('assert');
+const{ X509Certificate }=require('crypto');
+consthttps=require('https');
+consttls=require('tls');
+constfixtures=require('../common/fixtures');
+
+constdefaultCredentials={
+cert: fixtures.readKey('ca5-cert.pem'),
+key: fixtures.readKey('ca5-key.pem'),
+};
+constsniCredentials={
+cert: fixtures.readKey('agent1-cert.pem'),
+key: fixtures.readKey('agent1-key.pem'),
+};
+
+constdefaultCertificate=newX509Certificate(defaultCredentials.cert);
+constsniCertificate=newX509Certificate(sniCredentials.cert);
+constsniContext=tls.createSecureContext(sniCredentials);
+
+functionrequest(port,servername,expectedCertificate){
+returnnewPromise((resolve,reject)=>{
+constreq=https.get({
+host: '127.0.0.1',
+ port,
+ servername,
+rejectUnauthorized: false,
+agent: false,
+},common.mustCall((response)=>{
+try{
+constcertificate=response.socket.getPeerX509Certificate();
+assert.strictEqual(certificate.fingerprint256,
+expectedCertificate.fingerprint256);
+}catch(err){
+reject(err);
+return;
+}
+
+response.resume();
+response.once('end',resolve);
+response.once('error',reject);
+}));
+req.once('error',reject);
+});
+}
+
+constserver=https.createServer({
+cert: defaultCredentials.cert,
+key: defaultCredentials.key,
+SNICallback: common.mustCall((servername,callback)=>{
+assert.strictEqual(servername,'agent1.com');
+callback(null,sniContext);
+},1),
+},(_request,response)=>{
+response.end('ok');
+});
+
+server.listen(0,common.mustCall(async()=>{
+try{
+const{ port }=server.address();
+awaitrequest(port,undefined,defaultCertificate);
+awaitrequest(port,'agent1.com',sniCertificate);
+}finally{
+server.close(common.mustCall());
+}
+}));