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:
- Renegotiation is not supported by TLS1.3 protocol, attempts to call
`.renegotiate()` will always fail.
- Compiling against a system OpenSSL lower than 1.1.1 is no longer
supported (OpenSSL-1.1.0 used to be supported with configure flags).
- 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.
PR-URL: https://github.com/nodejs/node/pull/26209
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rod Vagg <rod@vagg.org>
OpenSSL 1.0.0 returned incorrect version information. OpenSSL 1.1.0
fixed this, but returning the correct information broke our tests, so
was considered semver-major. Because of this, the version was hard-coded
to the OpenSSL 1.0.0 (incorrect) string in 5fe81c8aff.
This is ancient history, start returning the correct cipher version.
PR-URL: https://github.com/nodejs/node/pull/26625
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Since 4697e1b0d7, it is no longer
necessary to use `v8::External`s to pass `StreamBase` instances
to native functions.
PR-URL: https://github.com/nodejs/node/pull/26510
Refs: https://github.com/nodejs/node/pull/25142
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
In `_tls_wrap.js` while calling `socket.connect` the `localPort` was
missing, restore it.
PR-URL: https://github.com/nodejs/node/pull/24554
Fixes: https://github.com/nodejs/node/issues/24543
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
This updates a lot of comments.
PR-URL: https://github.com/nodejs/node/pull/26223
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: Anto Aravinth <anto.aravinth.cse@gmail.com>
In the initial version of this test there were two zero-length writes to
force tls state to cycle. The second is not necessary, at least not now,
but the first was. The renegotiate() API should ensure that packet
exchange takes place, not its users, so move the zero-length write into
tls.
See: https://github.com/nodejs/node/pull/14239
See: https://github.com/nodejs/node/commit/b1909d3a70f9
PR-URL: https://github.com/nodejs/node/pull/25997
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Allow undefined as a callback, but do not allow null.
PR-URL: https://github.com/nodejs/node/pull/25929
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Minwoo Jung <minwoo@nodesource.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
`tls` shadows the global `tls` require, and isn't indicative of the
arument type.
PR-URL: https://github.com/nodejs/node/pull/25861
Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Don't throw on invalid property access if options is not provided, and
ensure callback is a function.
PR-URL: https://github.com/nodejs/node/pull/25876
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
OpenSSL has supported async notification of sessions and tickets since
1.1.0 using SSL_CTX_sess_set_new_cb(), for all versions of TLS. Using
the async API is optional for TLS1.2 and below, but for TLS1.3 it will
be mandatory. Future-proof applications should start to use async
notification immediately. In the future, for TLS1.3, applications that
don't use the async API will silently, but gracefully, fail to resume
sessions and instead do a full handshake.
See: https://wiki.openssl.org/index.php/TLS1.3#Sessions
PR-URL: https://github.com/nodejs/node/pull/25831
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com>
Renamed some internal C++ methods and properties for consistency, and
commented SSL I/O.
- Rename waiting_new_session_ after is_waiting_new_session(), instead of
using reverse naming (new_session_wait_), and change "waiting" to
"awaiting".
- Make TLSWrap::ClearIn() return void, the value is never used.
- Fix a getTicketKeys() cut-n-paste error. Since it doesn't use the
arguments, remove them from the js wrapper.
- Remove call of setTicketKeys(getTicketKeys()), its a no-op.
PR-URL: https://github.com/nodejs/node/pull/25713
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
If specified, and only when a socket is created internally, the option
will make `socket.setTimeout()` to be called on the created socket with
the given timeout.
This is consistent with the `timeout` option of `net.connect()` and
prevents the `timeout` option of the `https.Agent` from being ignored
when a socket is created.
PR-URL: https://github.com/nodejs/node/pull/25517
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
Reviewed-By: Roman Reiss <me@silverwind.io>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Its confusing to call a js class with a handle a "Wrap", usually it's
the C++ handle that is called a Wrap (tcp_wrap, tls_wrap, ...). Its
derived from Socket, and makes a JS stream look like a Socket, so call
it that. Also, remove use of lib/_stream_wrap.js so it can be deprecated
some time.
PR-URL: https://github.com/nodejs/node/pull/25153
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
session ID was named session in C++ and key in JS, Name them after what
they are, as the 'newSession' event docs do.
PR-URL: https://github.com/nodejs/node/pull/25153
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Don't use "socket" to describe two different objects in the same
function.
PR-URL: https://github.com/nodejs/node/pull/25153
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
"wrapped" argument is the caller's "socket", not its "wrap", and its
referred to as "socket" in the comments, so call it that.
PR-URL: https://github.com/nodejs/node/pull/25153
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
This removes all internal calls to the deprecated `_extends()`
function. It is slower than `Object.assign()` and the object spread
notation since V8 6.8 and using the spread notation often also
results in shorter code.
PR-URL: https://github.com/nodejs/node/pull/25105
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Use the object spread notation instead of using Object.assign.
It is not only easier to read it is also faster as of V8 6.8.
PR-URL: https://github.com/nodejs/node/pull/25104
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Yuta Hiroto <hello@hiroppy.me>
Reviewed-By: Minwoo Jung <minwoo@nodesource.com>
Reviewed-By: Denys Otrishko <shishugi@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
The existing secureProtocol option only allows setting the allowed
protocol to a specific version, or setting it to "all supported
versions". It also used obscure strings based on OpenSSL C API
functions. Directly setting the min or max is easier to use and explain.
PR-URL: https://github.com/nodejs/node/pull/24405
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Rod Vagg <rod@vagg.org>
This function was undocumented and only used in one place
throughout the codebase, plus a test.
PR-URL: https://github.com/nodejs/node/pull/23820
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
onConnectEnd(), which is called by TLSSocket, has a guard to
prevent being called multiple times, but it does not prevent the
OpenSSL error handler from being called, leading to multiple
error events. This commit adds that piece of missing logic.
PR-URL: https://github.com/nodejs/node/pull/23636
Fixes: https://github.com/nodejs/node/issues/23631
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Wyatt Preul <wpreul@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
internalBinding is used so often that it should just automatically be
available for usage in internals.
PR-URL: https://github.com/nodejs/node/pull/23025
Refs: https://github.com/nodejs/node/commit/2a9eb31
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Similar approach is used for `TLSWrap`, where C++ handle methods are
mapped one-to-one in JS.
PR-URL: https://github.com/nodejs/node/pull/22142
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
This commit makes the tls_wrap builtin an internal builtin, and
changes usage of the builtin from using process.binding('tls_wrap')
to use internalBinding instead.
Refs: https://github.com/nodejs/node/issues/22160
PR-URL: https://github.com/nodejs/node/pull/22429
Refs: https://github.com/nodejs/node/issues/22160
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
This commit makes the crypto builtin an internal builtin, and
changes usage of the builtin from using process.binding('crypto')
to use internalBinding instead.
Refs: https://github.com/nodejs/node/issues/22160
PR-URL: https://github.com/nodejs/node/pull/22426
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Pulls out a common argument validator to `internal/validators`
PR-URL: https://github.com/nodejs/node/pull/22101
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Instead of somtimes using an `owner` string to link from a
native handle object to the corresponding JS object, standardize
on a single symbol that fulfills this role.
PR-URL: https://github.com/nodejs/node/pull/22002
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Jon Moss <me@jonathanmoss.me>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Minwoo Jung <minwoo@nodesource.com>
Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com>
Reviewed-By: Ali Ijaz Sheikh <ofrobots@google.com>
Remove compile-time and run-time conditionals for features that
OpenSSL 1.0.0 and 1.0.1 didn't support: ALPN, OCSP and/or SNI.
They are no longer necessary since our baseline is OpenSSL 1.0.2.
PR-URL: https://github.com/nodejs/node/pull/21094
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
If a value is passed for SNICallback and it is not a function,
createServer() will now throw.
PR-URL: https://github.com/nodejs/node/pull/20969
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Ujjwal Sharma <usharma1998@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
This commit removes the var sharedCreds which is just reassigned to
this._sharedCreds in the following line.
PR-URL: https://github.com/nodejs/node/pull/20491
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Re-arrange and cleanup the flow of the onhandshakestart to be
more clear and less repetitive. Exit early in the case of a
first ever handshake for a given connection.
PR-URL: https://github.com/nodejs/node/pull/20466
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
This improves the error handling for a couple cases where the
received value would not have been handled so far or where the name
is wrong etc.
PR-URL: https://github.com/nodejs/node/pull/19445
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
This makes a effort to make sure all of these errors will actually
also show the received input.
On top of that it refactors a few tests for better maintainability.
It will also change the returned type to always be a simple typeof
instead of special handling null.
PR-URL: https://github.com/nodejs/node/pull/19445
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
This adds a rule that forbids the use of native Error constructors in
the `lib` directory. This is to encourage use of the `internal/errors`
mechanism. The rule is disabled for errors that are not created with
the `internal/errors` module but are still assigned an error code.
PR-URL: https://github.com/nodejs/node/pull/19373
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
The error emitted when a connection is closed before the
TLS handshake completes seemed rather unspefic by just saying
`socket hang up`.
Use a more verbose message, that also indicates that this is
a purely client-side error, and remove a misleading comment.
PR-URL: https://github.com/nodejs/node/pull/18989
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
