FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

deps: update openssl config by shigeki · Pull Request #5630 · nodejs/node · GitHub

/ node Public

deps: update openssl config - #5630

Closed
shigeki wants to merge 1 commit into
nodejs:masterfrom
shigeki:update_openssl_config
ConversationCommits (1)ChecksFiles changed
Closed

deps: update openssl config#5630
shigeki wants to merge 1 commit into
nodejs:masterfrom
shigeki:update_openssl_config

Conversation

shigeki commented Mar 9, 2016

Copy link
Copy Markdown
Contributor

Pull Request check-list

  • Does make -j8 test (UNIX) or vcbuild test nosign (Windows) pass with
    this change (including linting)?
  • Is the commit message formatted according to [CONTRIBUTING.md][0]?
  • [CI tests] If this change fixes a bug (or a performance problem), is a regression
    test (or a benchmark) included?
  • Is a documentation update included (if this change modifies
    existing APIs, or introduces new ones)?

Affected core subsystem(s)

deps: openssl

Description of change

deps/openssl/config were not updated in the last upgrading. Two new defines of OPENSSL_NO_SSL2 and OPENSSL_NO_WEAK_SSL_CIPHERS are defined in opensslconf.h.

This disables several ciphers included in EXPORT and LOW. They are also disabled in the default cipher list but I think that applying this to LTS should be discussed in LTS issue. I will open it.

CI is running on https://ci.nodejs.org/job/node-test-commit/2502/ but something Jenkins error was on MacOS. I will try it again.

R: @bnoordhuis

deps: update openssl config
ca8adf0 
OPENSSL_NO_SSL2 and OPENSSL_NO_WEAK_SSL_CIPHERS are defined in
opensslconf.h

shigeki commented Mar 9, 2016

Copy link
Copy Markdown
Contributor Author

New CI is https://ci.nodejs.org/job/node-test-pull-request/1890/

Fishrock123 commented Mar 9, 2016

Copy link
Copy Markdown
Contributor

s/udpate/update/ :)

Fishrock123 added the openssl Issues and PRs related to the OpenSSL dependency. label Mar 9, 2016
shigeki mentioned this pull request Mar 9, 2016
Closed
shigeki force-pushed the update_openssl_config branch from a12afa6 to d76834d Compare March 9, 2016 17:14
shigeki changed the title deps: udpate openssl config deps: update openssl config Mar 9, 2016

shigeki commented Mar 9, 2016

Copy link
Copy Markdown
Contributor Author

@Fishrock123 Oops. Fixed typo. Thanks.

shigeki force-pushed the update_openssl_config branch from d76834d to ca8adf0 Compare March 10, 2016 01:18

shigeki commented Mar 10, 2016

Copy link
Copy Markdown
Contributor Author

CI in https://ci.nodejs.org/job/node-test-commit/2505/ is all green.

shigeki commented Mar 10, 2016

Copy link
Copy Markdown
Contributor Author

Strictly speaking, this fix corresponds to semver-major. As discussed in LTS on nodejs/Release#85, I would like to apply this to 5.x.

jasnell commented Mar 11, 2016

Copy link
Copy Markdown
Member

LGTM for master... Let's add it to the lts-agenda for Monday to discuss.

shigeki commented Mar 15, 2016

Copy link
Copy Markdown
Contributor Author

@indutny Could you review this PR?

indutny commented Mar 15, 2016

Copy link
Copy Markdown
Member

LGTM, if CI is green

shigeki commented Mar 15, 2016

Copy link
Copy Markdown
Contributor Author

@indutny Thanks. CI was submitted in https://ci.nodejs.org/job/node-test-pull-request/1927/.

shigeki pushed a commit that referenced this pull request Mar 15, 2016
deps: update openssl config
668fb17 
OPENSSL_NO_SSL2 and OPENSSL_NO_WEAK_SSL_CIPHERS are defined in
opensslconf.h

Fixes: nodejs/Release#85
PR-URL: #5630
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Fedor Indutny <fedor@indutny.com>

shigeki commented Mar 15, 2016

Copy link
Copy Markdown
Contributor Author

CI is all green. Landed in 668fb17. Thanks.

shigeki closed this Mar 15, 2016

jasnell commented Mar 15, 2016

Copy link
Copy Markdown
Member

@shigeki ... This was backported to v0.12 and v0.10 in #5712 correct?

Also, removed the lts-agenda label on this since it was decided via the github conversation to go ahead and land this in the lts branches.

shigeki commented Mar 15, 2016

Copy link
Copy Markdown
Contributor Author

@jasnell #5712 was the fix for openssl-1.0.1 and it has already landed to v0.12-staging and v0.10-staging branches. This PR is the fix for openssl-1.0.2 and it need to be backported from master to 5.x and 4.x. I would like you to take care of them.

jasnell commented Mar 15, 2016

Copy link
Copy Markdown
Member

Sounds good! :-)

evanlucas pushed a commit that referenced this pull request Mar 15, 2016
deps: update openssl config
2e000d1 
OPENSSL_NO_SSL2 and OPENSSL_NO_WEAK_SSL_CIPHERS are defined in
opensslconf.h

Fixes: nodejs/Release#85
PR-URL: #5630
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Fedor Indutny <fedor@indutny.com>
evanlucas mentioned this pull request Mar 15, 2016
Merged
rvagg pushed a commit that referenced this pull request Mar 16, 2016
deps: update openssl config
29510aa 
OPENSSL_NO_SSL2 and OPENSSL_NO_WEAK_SSL_CIPHERS are defined in
opensslconf.h

Fixes: nodejs/Release#85
PR-URL: #5630
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Fedor Indutny <fedor@indutny.com>

MylesBorins commented Mar 17, 2016

Copy link
Copy Markdown
Contributor

@shigeki is this pressing enough to roll into the next v4 release asap or should it have a bit of time to live on v5 first?

shigeki commented Mar 18, 2016

Copy link
Copy Markdown
Contributor Author

@thealphanerd This fix is a kind of regular procedures in upgrading openssl and has just a small risk. So it's already enough to roll to v4.x.

MylesBorins pushed a commit that referenced this pull request Mar 21, 2016
deps: update openssl config
d8a5a31 
OPENSSL_NO_SSL2 and OPENSSL_NO_WEAK_SSL_CIPHERS are defined in
opensslconf.h

Fixes: nodejs/Release#85
PR-URL: #5630
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Fedor Indutny <fedor@indutny.com>
MylesBorins pushed a commit that referenced this pull request Mar 21, 2016
deps: update openssl config
19ae308 
OPENSSL_NO_SSL2 and OPENSSL_NO_WEAK_SSL_CIPHERS are defined in
opensslconf.h

Fixes: nodejs/Release#85
PR-URL: #5630
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Fedor Indutny <fedor@indutny.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

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.

5 participants

Back | FazBrowse Home | New Git URL