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

Use constant time P256 implementation #1328

Merged
merged 6 commits into from Aug 5, 2019

Conversation

@gdbelvin
Copy link
Collaborator

commented Jul 31, 2019

Shoutout to @Bren2010 for identifying this.

The VRF construction requires multiplying an adversary-chosen point by a fixed secret scalar, which can of course introduce timing attacks.

This PR switches from the pure Go, variable-time curve implementation to Go's constant time, cpu optimized implementation for a 30x speedup.

go test ./core/crypto/vrf/p256 -bench=.
goos: darwin
goarch: amd64
pkg: github.com/google/keytransparency/core/crypto/vrf/p256
Before BenchmarkEvaluate/1_goroutines-12         	     300	   5494696 ns/op
After  BenchmarkEvaluate/1_goroutines-12         	   10000	    189376 ns/op

Depends on #1330

gdbelvin added some commits May 17, 2019

Test paralleism
pkg: github.com/google/keytransparency/core/crypto/vrf/p256
BenchmarkEvaluate/1_goroutines-12         	     300	   5538849 ns/op
BenchmarkEvaluate/2_goroutines-12         	     500	   2931862 ns/op
BenchmarkEvaluate/4_goroutines-12         	    1000	   1625216 ns/op
BenchmarkEvaluate/8_goroutines-12         	    1000	   1333662 ns/op
BenchmarkEvaluate/16_goroutines-12        	    1000	   1447403 ns/op
BenchmarkEvaluate/32_goroutines-12        	    1000	   1526478 ns/op
BenchmarkEvaluate/64_goroutines-12        	    2000	   1569843 ns/op
BenchmarkEvaluate/128_goroutines-12       	   10000	   1817793 ns/op

@gdbelvin gdbelvin requested a review from thaidn as a code owner Jul 31, 2019

@googlebot googlebot added the cla: yes label Jul 31, 2019

@codecov

This comment has been minimized.

Copy link

commented Jul 31, 2019

Codecov Report

Merging #1328 into master will decrease coverage by 0.02%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1328      +/-   ##
==========================================
- Coverage   30.31%   30.28%   -0.03%     
==========================================
  Files          48       48              
  Lines        3866     3866              
==========================================
- Hits         1172     1171       -1     
- Misses       2512     2513       +1     
  Partials      182      182
Impacted Files Coverage Δ
core/crypto/vrf/p256/p256.go 82.22% <100%> (ø) ⬆️
core/client/client.go 28.28% <0%> (-0.66%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e593c9e...f55fcfe. Read the comment docs.

@thaidn

thaidn approved these changes Aug 2, 2019

Merge branch 'master' into vrf
* master:
  Include LogRootRequest in Request Protos (#1329)

@gdbelvin gdbelvin force-pushed the gdbelvin:vrf branch from 483f8e4 to 78992a9 Aug 5, 2019

@dep dep bot added the dependent label Aug 5, 2019

gdbelvin added some commits Aug 5, 2019

@gdbelvin gdbelvin merged commit aac75b1 into google:master Aug 5, 2019

5 checks passed

GolangCI No issues found!
Details
Travis CI - Pull Request Build Passed
Details
cla/google All necessary CLAs are signed
codecov/patch 100% of diff hit (target 30.31%)
Details
codecov/project Absolute coverage decreased by -0.02% but relative coverage increased by +69.68% compared to e593c9e
Details

@gdbelvin gdbelvin deleted the gdbelvin:vrf branch Aug 5, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.