Skip to content

crypto: move typechecking for timingSafeEqual into C++#34141

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
Closed
addaleax wants to merge 1 commit into from
Closed

crypto: move typechecking for timingSafeEqual into C++ #34141

addaleax wants to merge 1 commit into from

Conversation

@addaleax
Copy link
Member

@addaleaxaddaleax commented Jun 30, 2020•
edited
Loading

This makes the function more robust against V8 inlining.

Fixes: #34073

@nodejs/crypto

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • commit message follows commit guidelines

@nodejs-github-botnodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. lib / src Issues and PRs related to general changes in the lib or src directory. labels Jun 30, 2020
@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Jun 30, 2020•
edited by addaleax
Loading

CI: https://ci.nodejs.org/job/node-test-pull-request/32165/
Pummel CI: https://ci.nodejs.org/job/node-test-commit-custom-suites-freestyle/15157/

@addaleaxaddaleax added crypto Issues and PRs related to the crypto subsystem. and removed lib / src Issues and PRs related to general changes in the lib or src directory. labels Jun 30, 2020
@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Jun 30, 2020

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

ryzokuken
ryzokuken approved these changes Jul 1, 2020
View reviewed changes
Copy link
Contributor

@ryzokukenryzokuken left a comment

Choose a reason for hiding this comment

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

Thanks!

bnoordhuis
bnoordhuis approved these changes Jul 1, 2020
View reviewed changes
Copy link
Member

@bnoordhuisbnoordhuis left a comment

Choose a reason for hiding this comment

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

Small typo in the commit log: s/functionn/function/

@addaleax
crypto: move typechecking for timingSafeEqual into C++
6ff1747
This makes the function more robust against V8 inlining. Fixes: nodejs#34073
@addaleaxaddaleax added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Jul 1, 2020
@nodejs-github-bot

This comment has been minimized.

Sign in to view
@addaleaxaddaleaxforce-pushed the timingsafe-c++-typechecks branch from 8cd88c3 to 6ff1747CompareJuly 1, 2020 11:50
@nodejs-github-bot

This comment has been minimized.

Sign in to view
This was referenced Jul 2, 2020
Open
Open
@nodejs-github-bot

This comment has been minimized.

Sign in to view
@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Jul 3, 2020

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

jasnell pushed a commit that referenced this pull request Jul 3, 2020
@addaleax@jasnell
crypto: move typechecking for timingSafeEqual into C++
1d7be32
This makes the function more robust against V8 inlining. Fixes: #34073 PR-URL: #34141 Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Ujjwal Sharma <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Tobias Nießen <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Zeyu Yang <[email protected]>
@jasnell
Copy link
Member

jasnell commented Jul 3, 2020

Landed in 1d7be32

@jasnelljasnell closed this Jul 3, 2020
@github-actionsgithub-actionsbot mentioned this pull request Jul 4, 2020
Open
32 tasks
This was referenced Jul 5, 2020
Open
Open
Open
Open
This was referenced Jul 9, 2020
Open
Open
Open
Open
MylesBorins pushed a commit that referenced this pull request Jul 14, 2020
@addaleax@MylesBorins
crypto: move typechecking for timingSafeEqual into C++
6488cb3
This makes the function more robust against V8 inlining. Fixes: #34073 PR-URL: #34141 Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Ujjwal Sharma <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Tobias Nießen <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Zeyu Yang <[email protected]>
@MylesBorinsMylesBorins mentioned this pull request Jul 14, 2020
Merged
MylesBorins pushed a commit that referenced this pull request Jul 16, 2020
@addaleax@MylesBorins
crypto: move typechecking for timingSafeEqual into C++
cfd4c80
This makes the function more robust against V8 inlining. Fixes: #34073 PR-URL: #34141 Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Ujjwal Sharma <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Tobias Nießen <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Zeyu Yang <[email protected]>
addaleax added a commit that referenced this pull request Sep 22, 2020
@addaleax
crypto: move typechecking for timingSafeEqual into C++
bf4e778
This makes the function more robust against V8 inlining. Fixes: #34073 PR-URL: #34141 Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Ujjwal Sharma <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Tobias Nießen <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Zeyu Yang <[email protected]>
@codebyterecodebytere mentioned this pull request Sep 28, 2020
Merged
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@bnoordhuisbnoordhuisbnoordhuis approved these changes

@TrottTrottTrott approved these changes

@tniessentniessentniessen approved these changes

@richardlaurichardlaurichardlau approved these changes

@himself65himself65himself65 approved these changes

+1 more reviewer

@ryzokukenryzokukenryzokuken approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

author readyPRs 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++.cryptoIssues and PRs related to the crypto subsystem.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

pummel/test-crypto-timing-safe-equal-benchmarks failing

9 participants

@addaleax@nodejs-github-bot@jasnell@bnoordhuis@Trott@tniessen@richardlau@ryzokuken@himself65