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 — [v10.x backport] tls: add min/max protocol version options by sam-github · Pull Request #24979 · nodejs/node

Conversation

@sam-github

@sam-github sam-github commented Dec 12, 2018

Copy link
Copy Markdown
Contributor

Backport of a backport... prep work for openssl 1.1.1 to land on v10.x

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.

Backport-PR-URL: #24676
PR-URL: #24405
Reviewed-By: Refael Ackermann [email protected]
Reviewed-By: Rod Vagg [email protected]

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@sam-github
sam-github requested review from rvagg and shigeki December 12, 2018 04:37
@nodejs-github-bot

nodejs-github-bot commented Dec 12, 2018

Copy link
Copy Markdown
Collaborator

@sam-github build started: https://ci.nodejs.org/blue/organizations/jenkins/node-test-pull-request-lite-pipeline/detail/node-test-pull-request-lite-pipeline/1907/pipeline
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 lib / src Issues and PRs involving general changes in the lib/ or src/ directories. v10.x labels Dec 12, 2018
@sam-github

sam-github commented Dec 12, 2018

Copy link
Copy Markdown
Contributor Author

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

@nodejs/lts @nodejs/crypto
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@sam-github sam-github mentioned this pull request Dec 12, 2018
OpenSSL 1.1.1 Support TODO (pls help) #18770
Closed
@rvagg rvagg added crypto Issues and PRs related to the crypto subsystem. semver-minor PRs that contain new features and should be released in the next minor version. labels Dec 18, 2018
rvagg
rvagg approved these changes Dec 18, 2018
View reviewed changes

@rvagg rvagg 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.

👌 sweet, I don't recall the rules but I think it needs to be live in 11.x for a release or two before going in. Also tagged as semver-minor which may conflict with plans for 10.x, but this really needs to happen because we're running out of time on 1.1.1.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@MylesBorins

MylesBorins commented Dec 21, 2018

Copy link
Copy Markdown
Contributor

Our next semver minor isn't planned until march... should we do it earlier?
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@sam-github

sam-github commented Dec 23, 2018

Copy link
Copy Markdown
Contributor Author

no hurry, it's a pre-req for openssl-1.1.1, but can land in the same semver-minor as the openssl update
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@MylesBorins
MylesBorins force-pushed the v10.x-staging branch from 17f4208 to d04c3c2 Compare December 26, 2018 16:20
@Trott Trott mentioned this pull request Dec 29, 2018
TLS maxVersion not working #25267
Closed
@sam-github
sam-github force-pushed the backport-tls-min-max-to-10.x branch from 4d4c651 to 0b06887 Compare January 22, 2019 22:19
@MylesBorins
MylesBorins force-pushed the v10.x-staging branch from c1ee936 to c6cffad Compare January 29, 2019 05:06
@codebytere
codebytere force-pushed the v10.x-staging branch 3 times, most recently from 2d6e145 to 7840f71 Compare January 29, 2019 18:12
@BridgeAR BridgeAR mentioned this pull request Feb 20, 2019
v10.15.3 proposal #26063
Merged
@BethGriggs
BethGriggs force-pushed the v10.x-staging branch from 13f9356 to 5711238 Compare February 20, 2019 16:06
@sam-github
sam-github force-pushed the backport-tls-min-max-to-10.x branch from 0b06887 to d06c259 Compare February 22, 2019 23:52
sam-github added 2 commits February 22, 2019 15:52
@sam-github
doc: fix REPLACEME for tls min/max protocol option
c62283c 
Fill in correct pr-url: value in the YAML changelog that was missing
from f512f5e. The stanza was also sorted in the wrong order, most
recent is supposed to be in the beginning of the changes, not the end.

PR-URL: nodejs#24759
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
@sam-github
tls: add min/max protocol version options
a8a0889 
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.

Backport-PR-URL: nodejs#24676
PR-URL: nodejs#24405
Reviewed-By: Refael Ackermann <[email protected]>
Reviewed-By: Rod Vagg <[email protected]>
@sam-github
sam-github force-pushed the backport-tls-min-max-to-10.x branch from d06c259 to a8a0889 Compare February 22, 2019 23:52
@sam-github

sam-github commented Feb 23, 2019

Copy link
Copy Markdown
Contributor Author

Closed in favour of #26270
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@sam-github sam-github closed this Feb 23, 2019
@sam-github sam-github mentioned this pull request Feb 23, 2019
[v10.x backport] Update openssl1.1.1a #26270
Closed
4 tasks done
@sam-github
sam-github deleted the backport-tls-min-max-to-10.x branch March 20, 2019 15:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@shigeki shigeki Awaiting requested review from shigeki
1 more reviewer
@rvagg rvagg rvagg 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

crypto Issues and PRs related to the crypto subsystem. lib / src Issues and PRs involving general changes in the lib/ or src/ directories. semver-minor PRs that contain new features and should be released in the next minor version.

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.

4 participants

@sam-github @nodejs-github-bot @MylesBorins @rvagg
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.