tls.connect Error: self signed certificate when connect to imap.gmail.com:933 #28167

CoNETProject · 2019-06-11T08:05:47Z

Version: v12.0.0 ~ v12.4.0 (Old ~ v11.15.0 have no same issue )
Platform: MacOS, Debian, Windows
Subsystem: no
Module (and version) (if relevant): tls.connect(options[, callback])

Error: self signed certificate

When?
looks happened connect to imap.gmail.com:933 only.

imap.mail.me.com have no same issue.

//	This only happen at v12.0.0 ~ v12.4.0 (Old version ~ v11.15.0 have no same issue )

const socket = tls.connect (993, 'imap.gmail.com', () => {
	...
)
socket.on('error', err => {
	// Will ERROR!
	// Error: self signed certificate
	...
})

//	Apple mail have no same problem
const socket = tls.connect (993, 'imap.mail.me.com', () => {
	...
)
socket.on('error', err => {
	...
})

I checked tls connect with openssl command to imap.gmail.com at same OS, looks Good. It have not Man-in-the-middle attack OS. The code have no problem when I back NodeJS to v11.

addaleax · 2019-06-11T09:11:34Z

@nodejs/crypto

bnoordhuis · 2019-06-11T10:17:20Z

The issue is that Node.js (no longer?) sends a SNI record by default. You can work around it like this:

const options = {
  port: 993,
  host: 'imap.gmail.com',
  servername: 'imap.gmail.com',  // SNI
};
tls.connect(options, () => { /* ... */ });

It might have been caused by an openssl upgrade because I can reproduce with out/Release/openssl-cli, that also won't validate unless you pass -servername imap.gmail.com.

(GMail helpfully sends back a certificate with Issuer: OU = "No SNI provided; please fix your client.", CN = invalid2.invalid ^^)

CoNETProject · 2019-06-11T14:46:53Z

Thank you.
Looks have no problem now via use both host and servername.

silverwind · 2019-06-11T16:54:25Z

I can reproduce with out/Release/openssl-cli

Sending SNI on the openssl CLI was always opt-in via -servername, so that's probably not related.

sam-github · 2019-06-11T18:02:06Z

This might be a bug. 11.x and 12.x have the same OpenSSL version, but different behaviour, so it looks like something changed in node, not in openssl:

w/core % cat imap.gmail.js
console.log(process.versions.openssl);
require('tls').connect(993, 'imap.gmail.com', function() {
  console.log('CONNECT');
  this.end();
});
w/core % nvm run 11 imap.gmail.js
Running node v11.15.0 (npm v6.7.0)
1.1.1b
CONNECT
w/core % nvm run 12 imap.gmail.js
Running node v12.4.0 (npm v6.9.0)
1.1.1b
events.js:177
      throw er; // Unhandled 'error' event
      ^

Error: self signed certificate
    at TLSSocket.onConnectSecure (_tls_wrap.js:1317:34)
    at TLSSocket.emit (events.js:200:13)
    at TLSSocket._finishInit (_tls_wrap.js:792:8)
    at TLSWrap.ssl.onhandshakedone (_tls_wrap.js:606:12)
Emitted 'error' event at:
    at emitErrorNT (internal/streams/destroy.js:91:8)
    at emitErrorAndCloseNT (internal/streams/destroy.js:59:3)
    at processTicksAndRejections (internal/process/task_queues.js:84:9) {
  code: 'DEPTH_ZERO_SELF_SIGNED_CERT'
}

sam-github · 2019-06-11T18:10:07Z

I'm bisecting, maybe I can pinpoint a specific change.

bnoordhuis · 2019-06-11T18:18:33Z

Sending SNI on the openssl CLI was always opt-in via -servername, so that's probably not related.

The reason I mention it is that the stock openssl s_client on my system works without -servername.

silverwind · 2019-06-11T18:45:39Z

openssl s_client on my system works without -servername

We're offtopic but it's because it was changed in 1.1.1 as per this changelog entry:

s_client will now send the Server Name Indication (SNI) extension by
default unless the new "-noservername" option is used. The server name is
based on the host provided to the "-connect" option unless overridden by
using "-servername".

sam-github · 2019-06-12T16:57:30Z

bisect pointed to 42dbaed, my commit to add TLS1.3 support, and indeed it works with TLS1.2:

core/lts (master $% u=) % ./out/Release/node --tls-max-v1.3 ../imap.gmail.js
1.1.1b
events.js:177
      throw er; // Unhandled 'error' event
      ^

Error: self signed certificate
    at TLSSocket.onConnectSecure (_tls_wrap.js:1317:34)
    at TLSSocket.emit (events.js:200:13)
    at TLSSocket._finishInit (_tls_wrap.js:792:8)
    at TLSWrap.ssl.onhandshakedone (_tls_wrap.js:606:12)
Emitted 'error' event at:
    at emitErrorNT (internal/streams/destroy.js:91:8)
    at emitErrorAndCloseNT (internal/streams/destroy.js:59:3)
    at processTicksAndRejections (internal/process/task_queues.js:74:11) {
  code: 'DEPTH_ZERO_SELF_SIGNED_CERT'
}
core/lts (master $% u=) % ./out/Release/node --tls-max-v1.2 ../imap.gmail.js 
1.1.1b
CONNECT

However, running with --trace-tls I see something strange: SNI is not sent for TLS1.2 or TLS1.3 (by default), but only for TLS1.3 is the server returning the bogus self-signed cert with Subject = Issuer = "No SNI provided; please fix your client.", CN = invalid2.invalid. If I add SNI, it does fix the TLS1.3 connect, but why? Is the SNI extension required for TLS1.3? Well, no, but it is by google's servers: openssl/openssl#5362 (comment)

So, this is confirmed as a gmail bug.

Possibly, tls.connect() should default servername: to host: (in the case that host is a name and not an IP address) as http.request() does, but that would be a breaking change.

Thoughts?

More discussion: https://mta.openssl.org/pipermail/openssl-project/2018-April/000623.html

bnoordhuis · 2019-06-13T09:32:11Z

I suppose GMail is technically in the right because the spec says that a server MAY require SNI and imap.gmail.com does....

...although it's "interesting" that it doesn't actually care what you set the servername to: it happily accepts -servername panopticon.bigbro. Can I suggest we add a special case for imap.gmail.com and send that servername? ^^

We could turn on SNI by default although that leaks the servername because the SNI payload is unencrypted (and it'll be a while before ESNI is a thing.)

silverwind · 2019-06-13T11:57:55Z

I guess we have to default to sending a SNI (and mimic what browsers do). It is indeed a data leak, but I guess more services will work with it present than without.

BTW, I don't expect ESNI in the current draft form to get any widespread adoption because of it's complexity.

sam-github · 2019-06-13T15:27:41Z

@silverwind We already mimic what browsers do by defaulting to sending SNI for https (see Agent.prototype.addRequest), so it is only direct use of tls that doesn't set servername. I'll PR a default use of servername to tls.connect (unless someone else beats me to it).

bnoordhuis · 2019-07-03T16:05:28Z

@sam-github Did you open that PR and if so, can you link to it?

sam-github · 2019-07-03T21:03:46Z

@bnoordhuis nope, still on my todo list

edmorley · 2019-07-30T15:31:57Z

Is the SNI extension required for TLS1.3? Well, no, but it is by google's servers: openssl/openssl#5362 (comment)

Hi! Just to add some clarification here - RFC 8446 section 9.2 says:

Additionally, all implementations MUST support the use of the
"server_name" extension with applications capable of using it.
Servers MAY require clients to send a valid "server_name" extension.
Servers requiring this extension SHOULD respond to a ClientHello
lacking a "server_name" extension by terminating the connection with
a "missing_extension" alert.

ie: The "MAY" only refers to GMail choosing to be strict about this - clients are covered by the "MUST" - so Node's tls.connect is currently not spec-compliant.

ronag · 2020-06-08T21:03:12Z

@sam-github

I'll PR a default use of servername to tls.connect (unless someone else beats me to it).

What would be the default for servername? The hostname if a name and not an IP?

ronag · 2020-06-12T09:29:47Z

@mildsunrise

mildsunrise · 2020-06-12T11:09:08Z

I'm not familiar with https but servername is calculated here and yes, we derive it from the Host header and don't set it for IPs.

TBH I only recently found tls doesn't set SNI by default, I can agree it's not intuitive but it seems to be conventional. Maybe we could mention it more explicitely in the docs?

Add a note warning users that when using tls.connect(), the `servername` option must be set explicitely to enable SNI, otherwise the connection could fail. Fixes: nodejs#28167

mildsunrise · 2020-06-12T11:31:09Z

I propose to add the warning and close this (but regardless, I'm not against changing the default if we decide to do so)

mildsunrise · 2020-06-12T11:36:44Z

@edmorley sorry, I hadn't seen your comment...

clients are covered by the "MUST" - so Node's tls.connect is currently not spec-compliant.

But that "MUST" only requires implementations to support the use of SNI. We do support it, we just don't enable it by default, right?

edmorley · 2020-06-12T11:53:17Z

@mildsunrise Hi! Yeah I suppose that's technically correct, however I feel like having the default tls.connect API behaviour be non-spec-compliant wouldn't match user expectations?

Perhaps the best option is to add a warning to the docs for now, and then in the next Node major make it the default for TLS 1.3 connections only?

ronag · 2020-06-12T11:59:16Z

make it the default for TLS 1.3 connections only?

Sorry to bump in. This is the part I'm missing. Make what exactly the default, i.e. what is "it"?

mildsunrise · 2020-06-12T12:05:14Z

Current behaviour doesn't match user expectations, that's for sure (but it is spec-compliant imo, since it's the application (i.e. the user) who's in charge of enabling it)

I understand you want to enable SNI by default for TLS 1.3 connections?

edmorley · 2020-06-12T12:28:23Z

but it is spec-compliant imo

I'm not saying that tls.connect is not spec compliant per se, but that the "out of the box defaults" make it easy to accidentally write a non-spec-compliant client.

This is the part I'm missing. Make what exactly the default, i.e. what is "it"?

To hopefully clarify:

tls.connect does not enable SNI by default (presumably a combination of "because it never has since SNI is newish" + the privacy reasons mentioned above)
for TLS 1.2 and older, this is not normally a problem, however the TLS 1.3 spec says that clients MUST enable SNI
whilst the end user is ultimately responsible for writing a client to correctly meet the spec, and tls.connect is ultimately a pretty low-level API, from an API user-experience point of view it still seems best to have the tls.connect defaults match the requirements of the spec
ie instead of documenting that users should enable SNI (by setting servername to the same value as host) manually, tls.connect could do so by default iff host is not an IP and the protocol is TLS 1.3

Anyway happy to let others make the judgement call on this :-)

mildsunrise · 2020-06-12T12:42:48Z

Oh, TLSv1.3 mandates SNI? If you're referring to the paragraph where it says «In the absence of an application profile standard specifying otherwise, a TLS-compliant application MUST implement» like you said it's the application who's responsible of following spec (but yes, I agree that this is a valid reason for us to enable SNI by default).

If we want to enable SNI by default, I'd vote we do it on all connections... doing it only for TLSv1.3 could be confusing, IMHO.

Edit: The potential for breakage is small by itself, but as @bnoordhuis said we'll be suddenly leaking data and this could break applications that run behind an SNI-based firewall, for instance.

edmorley · 2020-06-12T12:49:14Z

I'd vote we do it on all connections... doing it only for TLSv1.3 could be confusing, IMHO.

I agree, I only mentioned that as a way to mitigate the privacy concerns for pre -TLSv1.3 use-cases, but (a) it would be confusing, (b) in those cases users could always disable it explicitly if desired I suppose.

bnoordhuis · 2020-06-13T08:38:09Z

RFC 8446 lists SNI in the mandatory-to-implement extensions section but nowhere does it seem to say TLSv1.3 clients MUST use SNI. SHOULD yes, MUST no.

https://tools.ietf.org/html/rfc8446#section-4.4.2.2:

   -  The "server_name" [RFC6066] and "certificate_authorities"
      extensions are used to guide certificate selection.  As servers
      MAY require the presence of the "server_name" extension, clients
      SHOULD send this extension, when applicable.

https://tools.ietf.org/html/rfc8446#section-9.2:

   Additionally, all implementations MUST support the use of the
   "server_name" extension with applications capable of using it.
   Servers MAY require clients to send a valid "server_name" extension.
   Servers requiring this extension SHOULD respond to a ClientHello
   lacking a "server_name" extension by terminating the connection with
   a "missing_extension" alert.

(server_name = SNI)

mildsunrise · 2020-06-22T07:45:56Z

Warning added! On whether to enable SNI by default, I'd recommend to open a PR or separate issue to continue discussion there

Add a note warning users that when using tls.connect(), the `servername` option must be set explicitely to enable SNI, otherwise the connection could fail. PR-URL: #33855 Fixes: #28167 Co-authored-by: Denys Otrishko <shishugi@gmail.com> Reviewed-By: Robert Nagy <ronagy@icloud.com> Reviewed-By: Denys Otrishko <shishugi@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de>

nodejs/node#28167

CoNETProject mentioned this issue Jun 11, 2019

tls.connect Error: self signed certificate when connect to imap.gmail.com:933 nodejs/help#1974

Closed

addaleax added the tls Issues and PRs related to the tls subsystem. label Jun 11, 2019

bnoordhuis added the confirmed-bug Issues with confirmed bugs. label Jun 11, 2019

sam-github removed the confirmed-bug Issues with confirmed bugs. label Jun 12, 2019

ile mentioned this issue Oct 27, 2019

Connection error: Error: self signed certificate mscdex/node-imap#705

Closed

danbulant mentioned this issue Dec 31, 2019

missing SNI header mscdex/node-imap#792

Open

broofa mentioned this issue Feb 19, 2020

Error: self signed certificate but only in Electron mscdex/node-imap#773

Closed

MarcoCatalan mentioned this issue Apr 8, 2020

Self signed certificate emailjs/emailjs-imap-client#233

Open

ronag mentioned this issue Jun 8, 2020

Https request to google returns DEPTH_ZERO_SELF_SIGNED_CERT nodejs/undici#217

Closed

mildsunrise mentioned this issue Jun 12, 2020

doc: warn that tls.connect() doesn't set SNI #33855

Closed

mildsunrise closed this as completed in b8ea471 Jun 22, 2020

rafhtl mentioned this issue Jul 3, 2020

for gmail you need to rafhtl/email-mirror#1

Closed

izwerg mentioned this issue Aug 5, 2020

Possible bug (?) chadxz/imap-simple#80

Closed

ojintoad mentioned this issue Aug 21, 2020

DO-5053 allow additional properties on connection NGPVAN/amqp.channel#10

Merged

iamrc90 mentioned this issue May 10, 2021

servername param added for Servername Indication ditesh/node-poplib#37

Open

hkjeffchan pushed a commit to hkjeffchan/node-pop3 that referenced this issue Jan 14, 2022

allow SNI in tls connection

9a6c0b9

nodejs/node#28167

hkjeffchan mentioned this issue Jan 14, 2022

allow SNI in tls connection node-pop3/node-pop3#24

Merged

brettz9 pushed a commit to node-pop3/node-pop3 that referenced this issue Jan 14, 2022

allow SNI in tls connection

d1f5644

nodejs/node#28167

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

tls.connect Error: self signed certificate when connect to imap.gmail.com:933 #28167

tls.connect Error: self signed certificate when connect to imap.gmail.com:933 #28167

CoNETProject commented Jun 11, 2019 •

edited

Loading

addaleax commented Jun 11, 2019

bnoordhuis commented Jun 11, 2019

CoNETProject commented Jun 11, 2019

silverwind commented Jun 11, 2019

sam-github commented Jun 11, 2019

sam-github commented Jun 11, 2019

bnoordhuis commented Jun 11, 2019

silverwind commented Jun 11, 2019 •

edited

Loading

sam-github commented Jun 12, 2019 •

edited

Loading

bnoordhuis commented Jun 13, 2019

silverwind commented Jun 13, 2019

sam-github commented Jun 13, 2019

bnoordhuis commented Jul 3, 2019

sam-github commented Jul 3, 2019

edmorley commented Jul 30, 2019

ronag commented Jun 8, 2020

ronag commented Jun 12, 2020

mildsunrise commented Jun 12, 2020 •

edited

Loading

mildsunrise commented Jun 12, 2020

mildsunrise commented Jun 12, 2020

edmorley commented Jun 12, 2020

ronag commented Jun 12, 2020 •

edited

Loading

mildsunrise commented Jun 12, 2020

edmorley commented Jun 12, 2020 •

edited

Loading

mildsunrise commented Jun 12, 2020 •

edited

Loading

edmorley commented Jun 12, 2020

bnoordhuis commented Jun 13, 2020

mildsunrise commented Jun 22, 2020

tls.connect Error: self signed certificate when connect to imap.gmail.com:933 #28167

tls.connect Error: self signed certificate when connect to imap.gmail.com:933 #28167

Comments

CoNETProject commented Jun 11, 2019 • edited Loading

addaleax commented Jun 11, 2019

bnoordhuis commented Jun 11, 2019

CoNETProject commented Jun 11, 2019

silverwind commented Jun 11, 2019

sam-github commented Jun 11, 2019

sam-github commented Jun 11, 2019

bnoordhuis commented Jun 11, 2019

silverwind commented Jun 11, 2019 • edited Loading

sam-github commented Jun 12, 2019 • edited Loading

bnoordhuis commented Jun 13, 2019

silverwind commented Jun 13, 2019

sam-github commented Jun 13, 2019

bnoordhuis commented Jul 3, 2019

sam-github commented Jul 3, 2019

edmorley commented Jul 30, 2019

ronag commented Jun 8, 2020

ronag commented Jun 12, 2020

mildsunrise commented Jun 12, 2020 • edited Loading

mildsunrise commented Jun 12, 2020

mildsunrise commented Jun 12, 2020

edmorley commented Jun 12, 2020

ronag commented Jun 12, 2020 • edited Loading

mildsunrise commented Jun 12, 2020

edmorley commented Jun 12, 2020 • edited Loading

mildsunrise commented Jun 12, 2020 • edited Loading

edmorley commented Jun 12, 2020

bnoordhuis commented Jun 13, 2020

mildsunrise commented Jun 22, 2020

CoNETProject commented Jun 11, 2019 •

edited

Loading

silverwind commented Jun 11, 2019 •

edited

Loading

sam-github commented Jun 12, 2019 •

edited

Loading

mildsunrise commented Jun 12, 2020 •

edited

Loading

ronag commented Jun 12, 2020 •

edited

Loading

edmorley commented Jun 12, 2020 •

edited

Loading

mildsunrise commented Jun 12, 2020 •

edited

Loading