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

autoSelectFamily: true breaks TLS #46679

Closed
addaleax opened this issue Feb 16, 2023 · 5 comments · Fixed by #46587
Closed

autoSelectFamily: true breaks TLS #46679

addaleax opened this issue Feb 16, 2023 · 5 comments · Fixed by #46587
Labels
confirmed-bug Issues with confirmed bugs. net Issues and PRs related to the net subsystem. tls Issues and PRs related to the tls subsystem.

Comments

@addaleax
Copy link
Member

Version

v19.6.0

Platform

x64 macOS 13.2.1; x64 Ubuntu 22.04

Subsystem

tls

What steps will reproduce the bug?

const socket = tls.connect({ host: 'google.com', port: 443, servername: 'google.com', autoSelectFamily: true }); 
socket.on('secureConnect', () => (console.log('secure connect'), socket.end()))

Results in Error: self-signed certificate with autoSelectFamily: true, works with autoSelectFamily: false.

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

Consistently.

What is the expected behavior?

Same as autoSelectFamily: false / same as when specifying IP addresses directly (with servername set).

What do you see instead?

TLS certificate validation errors.

Additional information

No response

@addaleax addaleax added tls Issues and PRs related to the tls subsystem. net Issues and PRs related to the net subsystem. labels Feb 16, 2023
@targos
Copy link
Member

targos commented Feb 16, 2023

Also note that specifying family: 4 or family: 6 both work without error.

@bnoordhuis
Copy link
Member

With { autoSelectFamily: true }, SSL_get_servername(TLSEXT_NAMETYPE_host_name) returns NULL.

That is, openssl thinks there's no SNI record and therefore this line returns false instead of 'google.com':

this.servername = this._handle.getServername();

I noticed TLSSocket.prototype._init() gets called when autoSelectFamily is set. There's also an extra call to Socket.prototype._read() that wasn't there before. This block seems responsible.

node/lib/net.js

Lines 1547 to 1550 in a2bbe5f

if (self[kWrapConnectedHandle]) {
self[kWrapConnectedHandle](handle);
initSocketHandle(self); // This is called again to initialize the TLSWrap
}

I suspect this line (possibly other init stuff as well) needs to be reapplied in TLSSocket.prototype._init(), otherwise node "forgets" relevant TLS context state:

tlssock.setServername(options.servername);

An interesting question is if { autoSelectFamily: true, secureContext: context } works and if not, what to do about it.

@mcollina
Copy link
Member

cc @ShogunPanda

@ShogunPanda
Copy link
Contributor

Thanks! I'll take a look on Mon when I'll tackle all related problems forever.

@tniessen tniessen added the confirmed-bug Issues with confirmed bugs. label Feb 19, 2023
@ShogunPanda
Copy link
Contributor

ShogunPanda commented Feb 21, 2023

I found the issue. This will be fixed once #46587 is merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
confirmed-bug Issues with confirmed bugs. net Issues and PRs related to the net subsystem. tls Issues and PRs related to the tls subsystem.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants