GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — Update openssl1.1.1b by sam-github · Pull Request #26327 · nodejs/node

Conversation

@sam-github

@sam-github sam-github commented Feb 26, 2019

Copy link
Copy Markdown
Contributor
Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

bricss reacted with thumbs up emoji bricss reacted with heart emoji bricss reacted with eyes emoji
All reactions
  • 👍 1 reaction
  • ❤️ 1 reaction
  • 👀 1 reaction
sam-github and others added 5 commits February 26, 2019 11:28
@sam-github
Revert "deps: remove OpenSSL git and travis configuration"
5b59fb8
This reverts commit f395a4a.

As of openssl-1.1.1b, the dot-files are no longer distributed, so this
is no longer necessary.
@sam-github
deps: upgrade openssl sources to 1.1.1b
8c936cf
This updates all sources in deps/openssl/openssl with openssl-1.1.1b.
@shigeki @sam-github
deps: add s390 asm rules for OpenSSL-1.1.1
c095c55
This is a floating patch against OpenSSL-1.1.1 to generate asm files
with Makefile rules.

PR-URL: nodejs#25381
Reviewed-By: Daniel Bevenius <[email protected]>
Reviewed-By: Shigeki Ohtsu <[email protected]>
@sam-github
deps: update archs files for OpenSSL-1.1.1b
679294f
`cd deps/openssl/config; make` updates all archs dependant files.
@sam-github
deps: openssl-1.1.1b no longer packages .gitignore
960257a
Since its not packaged, we don't have to delete it, and the Makefile
and update can become a (tiny) bit simpler.
@sam-github
sam-github requested a review from shigeki February 26, 2019 21:18
@nodejs-github-bot

nodejs-github-bot commented Feb 26, 2019

Copy link
Copy Markdown
Collaborator

@sam-github sadly an error occured when I tried to trigger a build :(

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@nodejs-github-bot nodejs-github-bot added the openssl Issues and PRs related to the OpenSSL dependency. label Feb 26, 2019
This was referenced Feb 27, 2019
OpenSSL 1.1.1 Support TODO (pls help) #18770
Closed
TLS1.3 support #26209
Closed
@sam-github

sam-github commented Feb 28, 2019

Copy link
Copy Markdown
Contributor Author

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@sam-github

sam-github commented Feb 28, 2019

Copy link
Copy Markdown
Contributor Author

Looks like everything passed, except custom-suites-freestyle, aka test-worker, which failed to git clone.

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

gireeshpunathil
gireeshpunathil approved these changes Mar 1, 2019
View reviewed changes
@sam-github

sam-github commented Mar 1, 2019

Copy link
Copy Markdown
Contributor Author

@nodejs/collaborators please take a look - routine OpenSSL patch update, nothing hard to review, and CI is green

I'd like this to hit 10.x on top of #26270 (so from the point of view of users of 10.x it would go straight to 1.1.1b skipping 1.1.1a), and I'm trying to figure out if that's possible . It would have to be in 11.x for a couple weeks until it can land in 10.x. It can't make the 10.16.0-rc of Mar 12th, but it could make the 10.16.0 release date of April 2nd if there is an 11.x in the next couple weeks.

@nodejs/release, the @nodejs/lts release schedule for 10.x is documented on https://github.com/nodejs/Release/wiki (but not 8.x or 6.x), is the 11.x schedule anywhere?

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@sam-github

sam-github commented Mar 1, 2019

Copy link
Copy Markdown
Contributor Author

@mhdawson points out that review-by-file has over 600 file diffs, so questioned my characterization of "not hard to review" :-)

It should be reviewed by looking at https://github.com/nodejs/node/pull/26327/commits:

  • Revert "deps: remove OpenSSL git and travis configuration" and deps: openssl-1.1.1b no longer packages .gitignore are trivial updates to the upgrade script, openssl no longer package some dot-files (yay!), so we no longer have to delete them
  • deps: upgrade openssl sources to 1.1.1b: result of https://github.com/nodejs/node/blob/master/deps/openssl/config/README.md#1-obtain-and-extract-new-openssl-sources, it is a simple untar of openssl1.1.1b replacing the existing openssl1.1.1a (huge, but doesn't need file-by-file review). I'm going to PR (seperately) a change to the commit message in the upgrade instructions to it is crystal clear what the commit contains.
  • deps: add s390 asm rules for OpenSSL-1.1.1 : result of https://github.com/nodejs/node/blob/master/deps/openssl/config/README.md#2-apply-a-floating-patch, it is a cherry pick of a commit that is already on the master, it doesn't need re-review but it needs reapplication because the files it changed got reset to their original state when update to the upstream the 1.1.1b source
  • deps: update archs files for OpenSSL-1.1.1b: result of https://github.com/nodejs/node/blob/master/deps/openssl/config/README.md#3-execute-make-in-depsopensslconfig-directory, as the commit message says, while huge, this is a mechanical commit of the cd deps/openssl/config; make. No file-by-file review necessary.

And that's it.

bricss reacted with eyes emoji
All reactions
  • 👀 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@sam-github sam-github mentioned this pull request Mar 1, 2019
deps: update OpenSSL upgrade process #26378
Closed
4 tasks
mhdawson
mhdawson approved these changes Mar 1, 2019
View reviewed changes

@mhdawson mhdawson left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

RSLGTM

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@sam-github

sam-github commented Mar 2, 2019

Copy link
Copy Markdown
Contributor Author

CI is green

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@sam-github

sam-github commented Mar 4, 2019

Copy link
Copy Markdown
Contributor Author

Green in CI, two approvals, 6 days open... I'll land it tomorrow if no objections, so @nodejs/crypto last chance to review.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

bnoordhuis
bnoordhuis approved these changes Mar 5, 2019
View reviewed changes
Comment thread deps/openssl/openssl/CHANGES
of a post handshake message exchange (although the messages themselves are
still signalled). This could break some applications that were expecting
the old signals. However without this KeyUpdate is not usable for many
applications.

@bnoordhuis bnoordhuis Mar 5, 2019

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FWIW, this is openssl/openssl@37857e9. That change looks safe to me in the sense of no observable change with TLS <= 1.2 (and TLSv1.3 isn't an issue for us just yet.) Agree/disagree?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

@sam-github sam-github Mar 5, 2019

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not just safe for TSL1.3, necessary. I'm floating this on #26209 and can drop it from there once 1.1.1b is merged.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@BridgeAR BridgeAR added the author ready PRs with CI started, the required approvals, and no outstanding review comments. label Mar 5, 2019
@sam-github

sam-github commented Mar 5, 2019

Copy link
Copy Markdown
Contributor Author

Landed in fe71629...b348ae7

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@sam-github sam-github closed this Mar 5, 2019
sam-github added a commit that referenced this pull request Mar 5, 2019
@sam-github
Revert "deps: remove OpenSSL git and travis configuration"
cbb7836
This reverts commit f395a4a.

As of openssl-1.1.1b, the dot-files are no longer distributed, so this
is no longer necessary.

PR-URL: #26327
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
sam-github added a commit that referenced this pull request Mar 5, 2019
@sam-github
deps: upgrade openssl sources to 1.1.1b
86c87e6
This updates all sources in deps/openssl/openssl with openssl-1.1.1b.

PR-URL: #26327
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
sam-github pushed a commit that referenced this pull request Mar 5, 2019
@shigeki @sam-github
deps: add s390 asm rules for OpenSSL-1.1.1
6df7bd6
This is a floating patch against OpenSSL-1.1.1 to generate asm files
with Makefile rules.

PR-URL: #26327
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>

Original:

Fixes: #4270
PR-URL: #19794
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Rod Vagg <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
sam-github added a commit that referenced this pull request Mar 5, 2019
@sam-github
deps: update archs files for OpenSSL-1.1.1b
c8d9ea4
`cd deps/openssl/config; make` updates all archs dependant files.

PR-URL: #26327
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
sam-github added a commit that referenced this pull request Mar 5, 2019
@sam-github
deps: openssl-1.1.1b no longer packages .gitignore
b348ae7
Since its not packaged, we don't have to delete it, and the Makefile
and update can become a (tiny) bit simpler.

PR-URL: #26327
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
@sam-github
sam-github deleted the update_openssl1.1.1b branch March 5, 2019 16:55
This was referenced Apr 23, 2019
2019-04-23 Version 12.0.0 (Current) @BethGriggs beevelop/docker-android-nodejs#302
Closed
2019-04-23 Version 12.0.0 (Current) @BethGriggs beevelop/docker-nodejs#275
Closed
2019-04-23 Version 12.0.0 (Current) @BethGriggs beevelop/docker-nodejs#276
Closed
2019-04-23 Version 12.0.0 (Current) @BethGriggs beevelop/docker-android-nodejs#303
Closed
2019-04-23 Version 12.0.0 (Current) @BethGriggs beevelop/docker-nodejs#277
Closed
2019-04-23 Version 12.0.0 (Current) @BethGriggs beevelop/docker-android-nodejs#304
Closed
2019-04-23 Version 12.0.0 (Current) @BethGriggs beevelop/docker-nodejs#278
Closed
2019-04-23 Version 12.0.0 (Current) @BethGriggs beevelop/docker-android-nodejs#305
Closed
2019-04-23 Version 12.0.0 (Current) @BethGriggs beevelop/docker-nodejs#279
Closed
2019-04-23 Version 12.0.0 (Current) @BethGriggs beevelop/docker-android-nodejs#306
Closed
This was referenced Apr 25, 2019
[v10.x backport]: Update OpenSSL 1.1.1b #27419
Closed
[v10.x backport] TLS1.3 (and dependent PRs) #27432
Closed
sam-github added a commit to sam-github/node that referenced this pull request Apr 29, 2019
@sam-github
deps: update OpenSSL upgrade process
a868ebe
Particularly, ensure that the commit messages are self-explanatory so
that reviewers can understand that the large commits are the result of a
simple repeatable process. This should make them easier to review.

See: nodejs#26327 (comment)

PR-URL: nodejs#26378
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
sam-github added a commit to sam-github/node that referenced this pull request Apr 29, 2019
@sam-github
deps: upgrade openssl sources to 1.1.1b
c80bff3
This updates all sources in deps/openssl/openssl with openssl-1.1.1b.

PR-URL: nodejs#26327
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
@BethGriggs BethGriggs mentioned this pull request May 16, 2019
v10.16.0 proposal #27514
Merged
This was referenced May 29, 2019
2019-05-28 Version 10.16.0 'Dubnium' (LTS) @BethGriggs beevelop/docker-android-nodejs#315
Closed
2019-05-28 Version 10.16.0 'Dubnium' (LTS) @BethGriggs beevelop/docker-android-nodejs#316
Closed
2019-05-28 Version 10.16.0 'Dubnium' (LTS) @BethGriggs beevelop/docker-android-nodejs#317
Closed
2019-05-28 Version 10.16.0 'Dubnium' (LTS) @BethGriggs beevelop/docker-android-nodejs#318
Closed
2019-05-28 Version 10.16.0 'Dubnium' (LTS) @BethGriggs beevelop/docker-android-nodejs#319
Closed
@watson watson mentioned this pull request Oct 14, 2019
Upgrade to Node 12.x elastic/kibana#47968
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@bnoordhuis bnoordhuis bnoordhuis approved these changes
@gireeshpunathil gireeshpunathil gireeshpunathil approved these changes
@shigeki shigeki Awaiting requested review from shigeki
+1 more reviewer
@mhdawson mhdawson mhdawson approved these changes
Reviewers whose approvals may not affect merge requirements

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

author ready PRs with CI started, the required approvals, and no outstanding review comments. openssl Issues and PRs related to the OpenSSL dependency.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

7 participants

@sam-github @nodejs-github-bot @bnoordhuis @gireeshpunathil @mhdawson @BridgeAR @shigeki
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.