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

sike mitigation with test cases #336

Merged
merged 1 commit into from
Jun 6, 2022

Conversation

zhdllwyc
Copy link
Contributor

@zhdllwyc zhdllwyc commented Jun 2, 2022

No description provided.

Copy link
Contributor

@armfazh armfazh left a comment

Choose a reason for hiding this comment

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

some minor changes needed

dh/sidh/internal/templates/core.gotemp Outdated Show resolved Hide resolved
dh/sidh/internal/templates/curve.gotemp Outdated Show resolved Hide resolved
dh/sidh/internal/templates/curve.gotemp Outdated Show resolved Hide resolved
dh/sidh/internal/templates/curve_test.gotemp Outdated Show resolved Hide resolved
dh/sidh/internal/templates/curve_test.gotemp Outdated Show resolved Hide resolved
dh/sidh/internal/templates/curve_test.gotemp Show resolved Hide resolved
@armfazh armfazh requested a review from bwesterb June 2, 2022 22:49
@armfazh armfazh added the fix-A-bug code that fix a bug label Jun 2, 2022
Copy link
Member

@bwesterb bwesterb left a comment

Choose a reason for hiding this comment

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

Thanks @zhdllwyc, this is great.

dh/sidh/internal/p434/core.go Show resolved Hide resolved
dh/sidh/internal/p434/cov.txt Outdated Show resolved Hide resolved
dh/sidh/sidh.go Outdated Show resolved Hide resolved
dh/sidh/internal/templates/core.gotemp Outdated Show resolved Hide resolved
dh/sidh/internal/templates/core.gotemp Show resolved Hide resolved
// Compute e_3 = log3(2^(nbits+1))
var e3 uint32
e3_float := float64(int(nbits)+1)/math.Log2(3)
e3 = uint32(e3_float)
Copy link
Member

Choose a reason for hiding this comment

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

Maybe nicer to have e3 as a constant in SidhParams instead of recomputing.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Having e3 as a parameter requires me to change the params.go
I want to put changes in the test files as much as possible.
Do you think I should touch the params.go?

Copy link
Contributor

Choose a reason for hiding this comment

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

e3 is definitely a constant, however, its calculation is simple and used once.
We may revisit later moving this value to the params files.

dh/sidh/internal/templates/curve.gotemp Show resolved Hide resolved
dh/sidh/internal/templates/curve_test.gotemp Outdated Show resolved Hide resolved
dh/sidh/internal/templates/curve_test.gotemp Show resolved Hide resolved
dh/sidh/internal/templates/curve_test.gotemp Show resolved Hide resolved
@zhdllwyc zhdllwyc force-pushed the sike-mitigation branch 2 times, most recently from 9ca8a3e to 005359c Compare June 3, 2022 21:04
Copy link
Contributor

@armfazh armfazh left a comment

Choose a reason for hiding this comment

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

two minor changes needed so far

dh/sidh/internal/templates/curve_test.gotemp Outdated Show resolved Hide resolved
dh/sidh/internal/templates/curve_test.gotemp Outdated Show resolved Hide resolved
@armfazh armfazh requested a review from bwesterb June 3, 2022 22:12
@@ -260,7 +261,7 @@ func DeriveSecretA(ss, prv []byte, pub3Pt *[3]Fp2) {
}

// Establishing shared keys in in 3-torsion group
func DeriveSecretB(ss, prv []byte, pub3Pt *[3]Fp2) {
func DeriveSecretB(ss, prv []byte, pub3Pt *[3]Fp2) {
Copy link
Member

Choose a reason for hiding this comment

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

Run go fmt

@armfazh armfazh merged commit 10923e8 into cloudflare:master Jun 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fix-A-bug code that fix a bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants