Skip to content

crypto/ec/ecp_nistp*.c: sanitize for undefined/implmentation-specific behaviour. #4974

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

crypto/ec/ecp_nistp*.c: sanitize for undefined/implmentation-specific behaviour. #4974

dot-asm wants to merge 1 commit into from

Conversation

@dot-asm
Copy link
Contributor

@dot-asm dot-asm commented Dec 23, 2017

Addressed #4970.

@dot-asm dot-asm mentioned this pull request Dec 23, 2017
Closed
kroeckx
kroeckx reviewed Dec 23, 2017
View reviewed changes
crypto/ec/ecp_nistp256.c
* all zeros if the high word of tmp[3] if 0
*/
low = (u64)tmp[3];
mask = 0 - (low >> 63);
Copy link
Member

@kroeckx kroeckx Dec 23, 2017

Choose a reason for hiding this comment

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

I'm wondering if it's useful to have a 64 bit version of constant_time_msb()

Copy link
Contributor Author

@dot-asm dot-asm Dec 23, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

There is one, but only in master. While this should go to multiple branches. Another reason I'm reluctant to use it is because I [for one] value original expression. I mean it would like mixing accents, you know...

@dot-asm dot-asm added branch: 1.0.2 Applies to OpenSSL_1_0_2-stable branch (EOL) 1.1.0 branch: master Applies to master branch labels Dec 23, 2017
@kroeckx kroeckx added the approval: done This pull request has the required number of approvals label Dec 23, 2017
levitte pushed a commit that referenced this pull request Dec 27, 2017
ec/ecp_nistp*.c: sanitize for undefined/implmentation-specific behavi…
8af7e94 
…our.

Reviewed-by: Kurt Roeckx <kurt@roeckx.be>
(Merged from #4974)
levitte pushed a commit that referenced this pull request Dec 27, 2017
ec/ecp_nistp*.c: sanitize for undefined/implmentation-specific behavi…
24f3a34 
…our.

Reviewed-by: Kurt Roeckx <kurt@roeckx.be>
(Merged from #4974)

(cherry picked from commit 8af7e94)
levitte pushed a commit that referenced this pull request Dec 27, 2017
ec/ecp_nistp*.c: sanitize for undefined/implmentation-specific behavi…
58cc296 
…our.

Reviewed-by: Kurt Roeckx <kurt@roeckx.be>
(Merged from #4974)

(cherry picked from commit 8af7e94)
@dot-asm dot-asm closed this Dec 27, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@kroeckx kroeckx kroeckx approved these changes

Assignees

No one assigned

Labels

approval: done This pull request has the required number of approvals branch: master Applies to master branch branch: 1.0.2 Applies to OpenSSL_1_0_2-stable branch (EOL)

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@dot-asm @kroeckx