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

fix: backport patch to sync exposed crypto #16822

Merged
merged 4 commits into from Feb 12, 2019

Conversation

3 participants
@codebytere
Copy link
Member

codebytere commented Feb 7, 2019

Description of Change

Closes #16812.

Backports https://boringssl-review.googlesource.com/c/boringssl/+/33984 to our patch set in order to correctly enumerate and instantiate ciphers. Also adds a test to ensure that DES-EDE-CBC actually works.

cc @nornagon

Checklist

Release Notes

Notes: Added a patch to fix incorrect enumeration and instantiation of Node.js ciphers in the Crypto module.

@codebytere codebytere requested a review from electron/reviewers as a code owner Feb 7, 2019

@nornagon

This comment has been minimized.

Copy link
Contributor

nornagon commented Feb 7, 2019

FYI, that patch may change a little still before it gets merged upstream so we may need to update this.

Show resolved Hide resolved spec/node-spec.js Outdated
@codebytere

This comment has been minimized.

Copy link
Member Author

codebytere commented Feb 12, 2019

@nornagon @ryzokuken all set, ptal when you can 🙇‍♀️

@codebytere codebytere changed the title [wip] fix: backport patch to sync exposed crypto fix: backport patch to sync exposed crypto Feb 12, 2019

@ryzokuken
Copy link
Member

ryzokuken left a comment

🤦‍♂️ LGTM. Thanks.

@codebytere

This comment has been minimized.

Copy link
Member Author

codebytere commented Feb 12, 2019

Flake on linux tests that is unrelated! Merging.

@codebytere codebytere merged commit cfba599 into master Feb 12, 2019

7 of 8 checks passed

appveyor: win-x64-debug Waiting for AppVeyor build to complete
Details
Semantic Pull Request ready to be squashed
Details
appveyor: win-ia32-debug AppVeyor build succeeded
Details
appveyor: win-ia32-testing AppVeyor build succeeded
Details
appveyor: win-ia32-testing-pr AppVeyor build succeeded
Details
appveyor: win-x64-testing AppVeyor build succeeded
Details
appveyor: win-x64-testing-pr AppVeyor build succeeded
Details
release-notes Release notes found

@codebytere codebytere deleted the crypto-sort-patch branch Feb 12, 2019

@release-clerk

This comment has been minimized.

Copy link

release-clerk bot commented Feb 12, 2019

Release Notes Persisted

Some ciphers that were listed in crypto.getCiphers() were not instantiable with crypto.createCipheriv(). Now they are.

@trop

This comment has been minimized.

Copy link
Contributor

trop bot commented Feb 12, 2019

I have automatically backported this PR to "5-0-x", please check out #16909

@trop trop bot added in-flight/5-0-x and removed target/5-0-x labels Feb 12, 2019

@codebytere

This comment has been minimized.

Copy link
Member Author

codebytere commented Feb 12, 2019

/trop run backport-to 4-0-x

@trop

This comment has been minimized.

Copy link
Contributor

trop bot commented Feb 12, 2019

The backport process for this PR has been manually initiated,
sending your 1's and 0's to "4-0-x" here we go! :D

@trop

This comment has been minimized.

Copy link
Contributor

trop bot commented Feb 12, 2019

I have automatically backported this PR to "4-0-x", please check out #16912

@trop trop bot added in-flight/4-0-x and removed in-flight/5-0-x labels Feb 12, 2019

@sofianguy sofianguy added this to 5.0.0-beta.3 in 5.0.x Feb 18, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment