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.

Sign up for GitHub

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

Jump to bottom

crypto: guard with OPENSSL_NO_GOST #30050

Closed
wants to merge 1 commit into from
Closed

crypto: guard with OPENSSL_NO_GOST #30050

wants to merge 1 commit into from

Conversation

codebytere
Copy link
Member

@codebytere codebytere commented Oct 21, 2019 •
edited
Loading

This PR wraps references to Gost-based NIDs inside GetSharedSigalgs with OPENSSL_NO_GOST. This helps prevent compilation failures in Electron owing to BoringSSL incompatibilities as well as reduces the size of Node's shipped binary by removing some potentially unnecessary code.

cc @tniessen

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

@codebytere codebytere requested a review from tniessen October 21, 2019 16:24
@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. crypto Issues and PRs related to the crypto subsystem. labels Oct 21, 2019
tniessen
tniessen approved these changes Oct 21, 2019
View reviewed changes
src/node_crypto.cc Outdated Show resolved Hide resolved
@nodejs-github-bot
Copy link
Collaborator

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

@danbev danbev added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Oct 23, 2019
danbev pushed a commit that referenced this pull request Oct 24, 2019
@codebytere @danbev
crypto: guard with OPENSSL_NO_GOST
a228e22 
PR-URL: #30050
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: David Carlier <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
@danbev
Copy link
Contributor

danbev commented Oct 24, 2019

Landed in a228e22.

@danbev danbev closed this Oct 24, 2019
targos pushed a commit that referenced this pull request Oct 24, 2019
@codebytere @targos
crypto: guard with OPENSSL_NO_GOST
8b75aab 
PR-URL: #30050
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: David Carlier <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
@targos targos mentioned this pull request Nov 5, 2019
Merged
targos pushed a commit that referenced this pull request Nov 8, 2019
@codebytere @targos
crypto: guard with OPENSSL_NO_GOST
d48eb86 
PR-URL: #30050
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: David Carlier <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
targos pushed a commit that referenced this pull request Nov 10, 2019
@codebytere @targos
crypto: guard with OPENSSL_NO_GOST
73cd1dd 
PR-URL: #30050
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: David Carlier <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
@targos targos mentioned this pull request Nov 10, 2019
Merged
targos pushed a commit that referenced this pull request Nov 11, 2019
@codebytere @targos
crypto: guard with OPENSSL_NO_GOST
b7bdfd3 
PR-URL: #30050
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: David Carlier <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@richardlau richardlau richardlau approved these changes

@devnexen devnexen devnexen approved these changes

@tniessen tniessen tniessen approved these changes

@bnoordhuis bnoordhuis bnoordhuis approved these changes

Assignees
No one assigned
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. c++ Issues and PRs that require attention from people who are familiar with C++. crypto Issues and PRs related to the crypto subsystem.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@codebytere @nodejs-github-bot @danbev @bnoordhuis @tniessen @devnexen @richardlau