TLS1.3 support

[sam-github]

This introduces TLS1.3 support and makes it the default max protocol,
but also supports CLI/NODE_OPTIONS switches to disable it if necessary.

TLS1.3 is a major update to the TLS protocol, with many security
enhancements. It should be preferred over TLS1.2 whenever possible.

TLS1.3 is different enough that even though the OpenSSL APIs are
technically API/ABI compatible, that when TLS1.3 is negotiated, the
timing of protocol records and of callbacks broke assumptions hard-coded
into the 'tls' module.

This change introduces no API incompatibilities when TLS1.2 is
negotiated. It is the intention that it be backported to current and LTS
release lines with the default maximum TLS protocol reset to 'TLSv1.2'.
This will allow users of those lines to explicitly enable TLS1.3 if they
want.

API incompatibilities between TLS1.2 and TLS1.3 are:

- Variations of `conn.write('data'); conn.destroy()` have undefined
behaviour according to the streams API. They may or may not send the
'data', and may or may not cause a ERR_STREAM_DESTROYED error to be
emitted. This has always been true, but conditions under which the write
suceeds is slightly but observably different when TLS1.3 is negotiated
vs when TLS1.2 or below is negotiated.

- If TLS1.3 is negotiated, and a server calls `conn.end()` in its
'secureConnection' listener without any data being written, the client
will not receive session tickets (no 'session' events will be emitted,
and `conn.getSession()` will never return a resumable session).

- The return value of `conn.getSession()` API may not return a resumable
session if called right after the handshake. The effect will be that
clients using the legacy `getSession()` API will resume sessions if
TLS1.2 is negotiated, but will do full handshakes if TLS1.3 is
negotiated.  See https://github.com/nodejs/node/pull/25831 for more
information.

Checklist

• make -j4 test (UNIX), or vcbuild test (Windows) passes
• tests and/or benchmarks are included
• documentation is changed or added
• commit message follows commit guidelines

[nodejs-github-bot]

@sam-github build started: https://ci.nodejs.org/blue/organizations/jenkins/node-test-pull-request-lite-pipeline/detail/node-test-pull-request-lite-pipeline/2668/pipeline

[sam-github]

@bnoordhuis note that I renamed the CLI options you introduced in #23814 that set the min TLS version, because I added options to limit the max, and I wanted whether it was setting the max or min to be clear. The original options haven't landed anywhere because they came with a change to disable TLS1.0 and 1.1, so that rename shouldn't affect anyone, though I'll have to backport part of #23814 with this PR if/when it gets backported

[sam-github]

ci: https://ci.nodejs.org/job/node-test-pull-request/20944/

[sam-github]

Is this OK? I rewrote this test from scratch, but if wanted, I can put the old copyright back in.

[sam-github]

@nodejs/TSC or @jasnell (who was involved in restoring incorrectly removed copyrights early in the io.js days, IIRC)

[sam-github]

@nodejs/tsc

[rvagg]

this is fine

[sam-github]

@nodejs/crypto PTAL

A note on the test approach: I tried to make as few changes to the tests as possible, since its supposed to be API almost-compatible. When I had to make significant changes to a test, I often wrapped it, so that the test is run with both TLS1.2 and TLS1.3 as the default (the tests usually negotiate the default max).

Since I want to backport this to release lines where TLS1.2 is the default max, I hacked the default max to be TLSv1.2, and ran make test locally. It passes.

In theory, the entire test suite could be run with TLS1.2 and 1.3 (and 1.0, and...), but that seemed excessive. If anyone has concerns about specific tests, I can add coverage for them.

[sam-github]

I added lts-watch-10.x, because I'd like it considered for 10.x (after being in 11.x for sufficient time).

I added backport-requested-11.x because I'd like it landed in 11.x, and it can't as-is because of the default.

@nodejs/release @nodejs/lts please adjust if this isn't your expectation.

[jasnell]

First glance This looks good. Will have to go back over in detail before signing off, however. Thank you for working on this

[sam-github]

ci: https://ci.nodejs.org/job/node-test-pull-request/21044/

[sam-github]

last one didn't even start, try again:

ci: https://ci.nodejs.org/job/node-test-pull-request/21070

[jasnell]

This feels like a complete hack but I'm not sure there's a better way unless we expose two different options for setting the cipher suites.

[sam-github]

I couldn't think of a better way, and I think two options would be awful. This code may be odd, but its only a couple lines long, and its very robust. We could go the BoringSSL way, and not even allow the TLS1.3 suites to be configurable, but that seems like not such a great idea, either. IoT devs would not be happy.

[gauravmahto]

@sam-github @BethGriggs
Will, this be backported to v10.x?

[sam-github]

I took a shot at it, https://github.com/sam-github/node/commits/tls1.3-v10.x, but it has so many deps that weren't backported it just exploded in difficulty, and I gave up. It isn't a priority for me, not now at least, and you are literally the only person who I have ever heard express any interest in TLS1.3 on 10.x...

Maybe someone else who feels the drive will have to step and do the backport. @gauravmahto , perhaps you?

[gauravmahto]

@sam-github I would love to. But I'll definitely need the guidance related to where to start and through the process. Last time I tried building on my machine (Windows), it kinda worked but several tests started to fail. Maybe this is the perfect time for me to give it another shot. :)

[gauravmahto]

@sam-github I built and ran the tests. Looks like everything worked just fine. :)
Still, I would need proper guidance. Looking forward to it.

[sam-github]

I don't have a lot to offer outside of https://github.com/nodejs/node/blob/master/doc/guides/contributing/pull-requests.md#the-process-of-making-changes and https://github.com/nodejs/node/blob/master/COLLABORATOR_GUIDE.md#how-are-lts-branches-managed

Feel free to PR a branch containing whatever you think needs doing to v10.x-staging. Make sure it tests locally, and once it passes on travis, ask someone to start a full CI and see if it passes.