Skip to content

crypto: use compatible ecdh function - #39054

Closed
codebytere wants to merge 1 commit into
nodejs:masterfrom
codebytere:use-boringssl-compat-ecdh
Closed

crypto: use compatible ecdh function#39054
codebytere wants to merge 1 commit into
nodejs:masterfrom
codebytere:use-boringssl-compat-ecdh

Conversation

@codebytere

Copy link
Copy Markdown
Member

Refs electron/electron#27688.

This PR switches ECDH::SetPrivateKey to use a compatible ECDH function - the previously used one is a no-op in BoringSSL. Also change an algorithm in the associated test suite to one supported by BoringSSL so we can run more smoke tests.

@github-actionsgithub-actionsBot added c++ Issues and PRs that require attention from people who are familiar with C++. crypto Issues and PRs related to the crypto subsystem. needs-ci PRs that need a full CI run. labels Jun 16, 2021
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@addaleax

Copy link
Copy Markdown
Member

Maybe it also makes sense to add a comment here so that nobody accidentally undoes this?

@codebytere
codebytereforce-pushed the use-boringssl-compat-ecdh branch from 365d4b6 to 001c143CompareJune 16, 2021 15:37
@codebyterecodebytere removed the needs-ci PRs that need a full CI run. label Jun 17, 2021
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Comment threadsrc/crypto/crypto_ec.cc Outdated
@codebytere
codebytereforce-pushed the use-boringssl-compat-ecdh branch from 001c143 to e0b0d4bCompareJune 29, 2021 13:06
@codebytere
codebytere requested a review from tniessenJune 29, 2021 13:07

@tniessentniessen left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thank you, I think that's much cleaner 😃

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@codebytere

Copy link
Copy Markdown
MemberAuthor

Landed in 5830449

@codebytere
codebytere deleted the use-boringssl-compat-ecdh branch June 30, 2021 08:14
codebytere added a commit that referenced this pull request Jun 30, 2021
PR-URL: #39054
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
targos pushed a commit that referenced this pull request Jul 11, 2021
PR-URL: #39054
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
@targostargos mentioned this pull request Jul 13, 2021
codebytere added a commit to electron/electron that referenced this pull request Jul 14, 2021
jkleinsc added a commit to electron/electron that referenced this pull request Jul 15, 2021
* chore: bump node in DEPS to v16.4.2
* chore: update patches
* ci: run main and remote woa tests separately
* chore: bump node in DEPS to v16.5.0
* build: restore libplatform headers in distribution
nodejs/node#39288
* build: pass directory instead of list of files to js2c.py
nodejs/node#39069
* chore: various BoringSSL/OpenSSL upstreams
- nodejs/node#39136
- nodejs/node#39138
- nodejs/node#39054
* test: move debugger test case to parallel
nodejs/node#39300
* chore: fixup patch indices
* build: pass directory instead of list of files to js2c.py
nodejs/node#39069
Co-authored-by: electron-roller[bot] <84116207+electron-roller[bot]@users.noreply.github.com>
Co-authored-by: PatchUp <73610968+patchup[bot]@users.noreply.github.com>
Co-authored-by: John Kleinschmidt <jkleinsc@electronjs.org>
Co-authored-by: Shelley Vohr <shelley.vohr@gmail.com>
BlackHole1 pushed a commit to BlackHole1/electron that referenced this pull request Aug 30, 2021
* chore: bump node in DEPS to v16.4.2
* chore: update patches
* ci: run main and remote woa tests separately
* chore: bump node in DEPS to v16.5.0
* build: restore libplatform headers in distribution
nodejs/node#39288
* build: pass directory instead of list of files to js2c.py
nodejs/node#39069
* chore: various BoringSSL/OpenSSL upstreams
- nodejs/node#39136
- nodejs/node#39138
- nodejs/node#39054
* test: move debugger test case to parallel
nodejs/node#39300
* chore: fixup patch indices
* build: pass directory instead of list of files to js2c.py
nodejs/node#39069
Co-authored-by: electron-roller[bot] <84116207+electron-roller[bot]@users.noreply.github.com>
Co-authored-by: PatchUp <73610968+patchup[bot]@users.noreply.github.com>
Co-authored-by: John Kleinschmidt <jkleinsc@electronjs.org>
Co-authored-by: Shelley Vohr <shelley.vohr@gmail.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

c++Issues and PRs that require attention from people who are familiar with C++.cryptoIssues and PRs related to the crypto subsystem.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants

@codebytere@nodejs-github-bot@addaleax@fhinkel@tniessen@targos