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.

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: bad assertion in crypto functions that use BN_bn2bin_padded #17220

Merged
merged 2 commits into from Mar 7, 2019

Conversation

nornagon
Copy link
Member

@nornagon nornagon commented Mar 4, 2019

in particular, this picks up electron/node#70a78f07b, which fixes an issue with incorrect usage of the BN_bn2bin_padded API in boringssl. This also adds tests to make sure those functions are fixed.

Checklist

Release Notes

Notes: Fixed an assertion when calling ECDH.getPrivateKey(), diffieHellman.generateKeys() or diffieHellman.get*().

in particular, this picks up electron/node#70a78f07b, which fixes an issue with incorrect usage of the BN_bn2bin_padded API in boringssl
@nornagon nornagon requested a review from deepak1556 March 4, 2019 19:41
@electron-cation electron-cation bot added the new-pr 🌱 PR opened in the last 24 hours label Mar 4, 2019
@nornagon
Copy link
Member Author

nornagon commented Mar 4, 2019

This will need a manual backport to 4 and 5.

@nornagon nornagon changed the title chore: roll node fix: crash in crypto functions that use BN_bn2bin_padded Mar 4, 2019
@nornagon nornagon changed the title fix: crash in crypto functions that use BN_bn2bin_padded fix: assertion in crypto functions that use BN_bn2bin_padded Mar 4, 2019
@nornagon nornagon changed the title fix: assertion in crypto functions that use BN_bn2bin_padded fix: bad assertion in crypto functions that use BN_bn2bin_padded Mar 4, 2019
@nornagon nornagon mentioned this pull request Mar 4, 2019
6 tasks
@electron-cation electron-cation bot removed the new-pr 🌱 PR opened in the last 24 hours label Mar 4, 2019
Copy link
Member

@deepak1556 deepak1556 left a comment

Choose a reason for hiding this comment

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

Nice catch, thanks!

@electron-cation electron-cation bot added the new-pr 🌱 PR opened in the last 24 hours label Mar 4, 2019
@codebytere
Copy link
Member

codebytere commented Mar 4, 2019

one error seems to be:

type.toLowerCase

🤔

and the other seems to be

Error: Failed to get ECDH private key

@deepak1556
Copy link
Member

@nornagon any reason this needs backport to 4.x ? AFAIK the changes on electron/node only made it to electron 5.x branch.

@nornagon
Copy link
Member Author

nornagon commented Mar 4, 2019

ack, you're right, no need for 4.

@electron-cation electron-cation bot removed the new-pr 🌱 PR opened in the last 24 hours label Mar 5, 2019
@deepak1556 deepak1556 merged commit b575631 into master Mar 7, 2019
@release-clerk
Copy link

release-clerk bot commented Mar 7, 2019

Release Notes Persisted

Fixed an assertion when calling ECDH.getPrivateKey(), diffieHellman.generateKeys() or diffieHellman.get*().

@trop
Copy link
Contributor

trop bot commented Mar 7, 2019

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

@deepak1556 deepak1556 deleted the roll-node branch March 7, 2019 02:00
@sofianguy sofianguy added this to Fixed in 5.0.0-beta.6 in 5.0.x Mar 20, 2019
kiku-jw pushed a commit to kiku-jw/electron that referenced this pull request May 16, 2019
…ctron#17220)

* chore: roll node

in particular, this picks up electron/node#70a78f07b, which fixes an issue with incorrect usage of the BN_bn2bin_padded API in boringssl

* fix tests
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
5.0.x
Fixed in 5.0.0-beta.6
Development

Successfully merging this pull request may close these issues.

None yet

3 participants