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

v11.15.0 proposal #27314

Merged
merged 15 commits into from
Apr 30, 2019
Merged

v11.15.0 proposal #27314

merged 15 commits into from
Apr 30, 2019

Conversation

codebytere
Copy link
Member

Notable changes

  • deps: add s390 asm rules for OpenSSL-1.1.1 (Shigeki Ohtsu) #19794
  • src: add .code and SSL specific error properties (Sam Roberts) #25093
  • tls:
    • add --tls-min-v1.2 CLI switch (Sam Roberts) #26951
    • supported shared openssl 1.1.0 (Sam Roberts) #26951
    • revert default max toTLSv1.2 (Sam Roberts) #26951
    • revert change to invalid protocol error type (Sam Roberts) #26951
    • support TLSv1.3 (Sam Roberts) #26209
    • add code for ERR_TLS_INVALID_PROTOCOL_METHOD (Sam Roberts) #24729

Commits

  • [8db791d0fe] - deps: update archs files for OpenSSL-1.1.1b (Sam Roberts) #26327
  • [1c98b720b1] - (SEMVER-MINOR) deps: add s390 asm rules for OpenSSL-1.1.1 (Shigeki Ohtsu) #19794
  • [d8cc478ae9] - deps: upgrade openssl sources to 1.1.1b (Sam Roberts) #26327
  • [fa6f0f1644] - doc: describe tls.DEFAULT_MIN_VERSION/_MAX_VERSION (Sam Roberts) #26821
  • [8b5d350a35] - (SEMVER-MINOR) src: add .code and SSL specific error properties (Sam Roberts) #25093
  • [bf2c283555] - (SEMVER-MINOR) tls: add --tls-min-v1.2 CLI switch (Sam Roberts) #26951
  • [7aeca270f6] - (SEMVER-MINOR) tls: supported shared openssl 1.1.0 (Sam Roberts) #26951
  • [d2666e6ded] - tls: add debugging to native TLS code (Anna Henningsen) #26843
  • [225417b849] - tls: add CHECK for impossible condition (AnnaHenningsen) #26843
  • [109c097797] - (SEMVER-MINOR) tls: revert default max toTLSv1.2 (Sam Roberts) #26951
  • [7393e37af1] - (SEMVER-MINOR) tls: support TLSv1.3 (Sam Roberts) #26209
  • [8e14859459] - (SEMVER-MINOR) tls: revert change to invalid protocol error type (Sam Roberts) #26951
  • [00688b6042] - (SEMVER-MINOR) tls: add code for ERR_TLS_INVALID_PROTOCOL_METHOD (Sam Roberts) #24729

sam-github and others added 13 commits April 15, 2019 13:29
Add an error code property to invalid `secureProtocol` method
exceptions.

Backport-PR-URL: #26951
PR-URL: #24729
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
In #24729, the error was changed to
be a TypeError, which is the standard type for this kind of error.
However, it was Error in 11.x and earlier, so revert that single aspect,
so the backport can be semver-minor.

PR-URL: #26951
Reviewed-By: Rod Vagg <rod@vagg.org>
Reviewed-By: Beth Griggs <Bethany.Griggs@uk.ibm.com>
SSL errors have a long structured message, but lacked the standard .code
property which can be used for stable comparisons. Add a `code`
property, as well as the 3 string components of an SSL error: `reason`,
`library`, and `function`.

Backport-PR-URL: #26951
PR-URL: #25093
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
This updates all sources in deps/openssl/openssl with openssl-1.1.1b.

Backport-PR-URL: #26951
PR-URL: #26327
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
This is a floating patch against OpenSSL-1.1.1 to generate asm files
with Makefile rules.

Backport-PR-URL: #26951
PR-URL: #26327
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>

Original:

Fixes: #4270
PR-URL: #19794
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rod Vagg <rod@vagg.org>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
`cd deps/openssl/config; make` updates all archs dependant files.

Backport-PR-URL: #26951
PR-URL: #26327
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
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 #25831 for more
information.

Backport-PR-URL: #26951
PR-URL: #26209
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rod Vagg <rod@vagg.org>
TLSv1.3 is still supported when explicitly configured, but it is not the
default.

PR-URL: #26951
Reviewed-By: Rod Vagg <rod@vagg.org>
Reviewed-By: Beth Griggs <Bethany.Griggs@uk.ibm.com>
Backport-PR-URL: #26951
PR-URL: #26843
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Minwoo Jung <minwoo@nodesource.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Backport-PR-URL: #26951
PR-URL: #26843
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Minwoo Jung <minwoo@nodesource.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Backport-PR-URL: #26951
PR-URL: #26821
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
PR-URL: #26951
Reviewed-By: Rod Vagg <rod@vagg.org>
Reviewed-By: Beth Griggs <Bethany.Griggs@uk.ibm.com>
For 11.x, the default minimum is TLSv1, so it needs a CLI switch to
change the default to the more secure minimum of TLSv1.2.

PR-URL: #26951
Reviewed-By: Rod Vagg <rod@vagg.org>
Reviewed-By: Beth Griggs <Bethany.Griggs@uk.ibm.com>
@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot nodejs-github-bot added lib / src Issues and PRs related to general changes in the lib or src directory. v11.x labels Apr 19, 2019
@codebytere codebytere force-pushed the v11.15.0-proposal branch 2 times, most recently from 3669215 to 171465f Compare April 19, 2019 18:20
@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Apr 19, 2019

@nodejs nodejs deleted a comment from nodejs-github-bot Apr 19, 2019
@BridgeAR
Copy link
Member

There's still one open backport: #27259. I am not sure if we want to have a small last release or if we want to include everything else as well that landed in the meanwhile @nodejs/releasers?

@rvagg
Copy link
Member

rvagg commented Apr 22, 2019

The odd "Current" lines have always had at least 8 months support, so I'd hope that it's not being entirely dropped on the floor at the 6 month mark.

@MylesBorins
Copy link
Contributor

@rvagg my recollection was that after 6 months current releases moved to maintenance mode. Below are all releases that were done for current release after 6 months, of the 5 releases only 1 of them seems to have been a non maintenance release.

For 9.x there was only a single release after April:

For 7.x there were two:

For 5.x there were two:

@codebytere
Copy link
Member Author

Should i go ahead with what's here or get that last backport in?

@rvagg
Copy link
Member

rvagg commented Apr 23, 2019

@MylesBorins yeah, that's fine and I think that's how we've framed it in the past, our support graphic has grey bits for those extra 2 months but our support table just has an EOL for June.

My point is, that we shouldn't be dropping 11.x on the floor just because 12 is out. And, if someone wants to continue pushing releases out beyond security releases then I don't think there's a reason to stop that for the next couple of months. Maybe it'd even be a good way for someone new to releases to have learn the ropes? Of course if there's nobody that can be bothered then there's not much we can do about that either!

@BethGriggs
Copy link
Member

@codebytere I think it'd be nice to get the last backport in this release

@Fishrock123
Copy link
Contributor

It'd be nice to still get this in, imo.

The differences to the original patch are the replacement of
`i::IsIdentifier...()` with `unicode_cache_.IsIdentifier...()`,
because the former is not available on Node.js v11.x, as well
as the omitted `no_gc` argument for `GetFlatContent()`.

Original commit message:

    Assume flat string when checking CompileFunctionInContext arguments.

    R=jkummerow@chromium.org

    Change-Id: I54c6137a3c6e14d4102188f154aa7216e7414dbc
    Reviewed-on: https://chromium-review.googlesource.com/c/1388533
    Reviewed-by: Jakob Kummerow <jkummerow@chromium.org>
    Commit-Queue: Yang Guo <yangguo@chromium.org>
    Cr-Commit-Position: refs/heads/master@{#58562}

Refs: v8/v8@61f4c22
Fixes: #27256

PR-URL: #27259
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Shelley Vohr <codebytere@gmail.com>
@codebytere codebytere force-pushed the v11.15.0-proposal branch 2 times, most recently from be5d7eb to 9a7702f Compare April 29, 2019 04:22
@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Apr 29, 2019

@nodejs nodejs deleted a comment from nodejs-github-bot Apr 29, 2019
Notable changes:

* deps: add s390 asm rules for OpenSSL-1.1.1 (Shigeki Ohtsu) [#19794](#19794)
* src: add .code and SSL specific error properties (Sam Roberts) [#25093](#25093)
* tls:
  * add --tls-min-v1.2 CLI switch (Sam Roberts) [#26951](#26951)
  * supported shared openssl 1.1.0 (Sam Roberts) [#26951](#26951)
  * revert default max toTLSv1.2 (Sam Roberts) [#26951](#26951)
  * revert change to invalid protocol error type (Sam Roberts) [#26951](#26951)
  * support TLSv1.3 (Sam Roberts) [#26209](#26209)
  * add code for ERR\_TLS\_INVALID\_PROTOCOL\_METHOD (Sam Roberts) [#24729](#24729)

PR-URL: #27314
@codebytere
Copy link
Member Author

codebytere commented Apr 30, 2019

Release: https://ci-release.nodejs.org/job/iojs+release/4415/

@codebytere codebytere merged commit e65a904 into v11.x Apr 30, 2019
codebytere added a commit that referenced this pull request Apr 30, 2019
codebytere added a commit that referenced this pull request Apr 30, 2019
Notable changes:

* deps: add s390 asm rules for OpenSSL-1.1.1 (Shigeki Ohtsu) [#19794](#19794)
* src: add .code and SSL specific error properties (Sam Roberts) [#25093](#25093)
* tls:
  * add --tls-min-v1.2 CLI switch (Sam Roberts) [#26951](#26951)
  * supported shared openssl 1.1.0 (Sam Roberts) [#26951](#26951)
  * revert default max toTLSv1.2 (Sam Roberts) [#26951](#26951)
  * revert change to invalid protocol error type (Sam Roberts) [#26951](#26951)
  * support TLSv1.3 (Sam Roberts) [#26209](#26209)
  * add code for ERR\_TLS\_INVALID\_PROTOCOL\_METHOD (Sam Roberts) [#24729](#24729)

PR-URL: #27314
codebytere added a commit to nodejs/nodejs.org that referenced this pull request Apr 30, 2019
codebytere added a commit to nodejs/nodejs.org that referenced this pull request Apr 30, 2019
@codebytere codebytere deleted the v11.15.0-proposal branch April 30, 2019 18:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lib / src Issues and PRs related to general changes in the lib or src directory.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants