G2hash #428
G2hash #428
Conversation
Thanks to David Jao for hints on how to implement this
|
Wow - great. I think I need more crypto support to better understand what's happening here ;) @nikkolasg @Daeinar @bford - care to chime in and have a look if that is OK? |
| @@ -29,6 +29,10 @@ func (e *gfP) Set(f *gfP) { | |||
| e[3] = f[3] | |||
| } | |||
|
|
|||
| func (e *gfP) Equals(f *gfP) bool { | |||
There was a problem hiding this comment.
This is not constant time, is that ok? pointG1 and pointG2 are careful to be constant time:
func (p *pointG2) Equal(q kyber.Point) bool {
x, _ := p.MarshalBinary()
y, _ := q.MarshalBinary()
return subtle.ConstantTimeCompare(x, y) == 1
}
Likewise for the other Equals as well.
There was a problem hiding this comment.
Ian writes: Note that Hash() is not constant time (either the existing G1 implementation or our new G2 one), but fair enough that Equals may as well be. I've fixed it in both gfP and gfP2 (in the latest commit 78796c8).
pairing/bn256/point.go
Outdated
| gfP2{*newGFp(0), *newGFp(1)}} | ||
|
|
||
| // Multiply by the cofactor to get a point in G2 | ||
| cofactor := bigFromBase10("65000549695646603732796438742359905743080310161342220753873227084684201343597") |
There was a problem hiding this comment.
Maybe this belongs in constants.go?
There was a problem hiding this comment.
Ian writes: Makes sense; done (in the latest commit 77e41db).
| y := &gfP2{} | ||
| one := &gfP2{*newGFp(0), *newGFp(1)} | ||
|
|
||
| for { |
There was a problem hiding this comment.
This same algorithm is already implemented in hashToPoint(), on line 226. Can you explain why this second implementation is needed for g2?
hashToPoint says, "ideally we want to do this using gfP, but gfP doesn't have a ModSqrt function". In this PR you are adding a Sort to gfp2. I don't understand why g1 and g2 need different approaches to the same problem.
There was a problem hiding this comment.
By the way: I am not advocating that things should be all perfectly cleaned up and every single line of duplicate code removed. And the behaviour of hash to a G1 point must not be changed, or else the existing BLS signatures would all stop validating.
So if the explanation is, "we knew about hashToPoint and we know we can't touch it, but for X reason G2 needs the same thing but slightly different", then that's ok with me.
There was a problem hiding this comment.
Ian writes:
The core of the hash algorithm is indeed (intentionally) the same for G1 and G2, but there are a few differences:
- They operate over different fields (gfP vs gfP2) and groups (G1 vs the twist group that contains G2), of course
- The G2 version has to multiply by the G2 cofactor at the end
- The initial x coordinate (in gfP for G1 and in gfP2 for G2) is a hash of the input data, but twice as much hash output has to be produced for G2, since gfP2 elements are twice as large
So I suppose it's possible that someone could implement Sqrt for gfP, and then make common interfaces for both the fields and the groups that unify the differences in the core of the algorithm, but still initialize the initial x coordinate differently, and multiply by the cofactor only in the G2 version? That would certainly clean up the existing G1Hash() function that has to convert everything to BigInt and back because of a lack of Sqrt in gfP, but I don't think that should be part of this PR.
| @@ -435,6 +435,64 @@ func (p *pointG2) String() string { | |||
| return "bn256.G2:" + p.g.String() | |||
| } | |||
|
|
|||
| func (p *pointG2) Hash(m []byte) kyber.Point { | |||
| // Hash the data to get the "real" and "imaginary" parts of the | |||
There was a problem hiding this comment.
Is this an existing standard that you are implementing, in which case, please mention it. Or are you syncing up to an existing implementation someplace else, in which case please reference it and add test vectors to the unit test that come from the other implementation.
Or is this your own invention?
It all looks reasonable to me, just trying to understand the context a bit more.
There was a problem hiding this comment.
Ian writes: I do not know of another implementation or standard for hashing to G2 on this curve. As above, the choice of algorithm follows closely the existing hash-to-G1, but the choice of exactly how to hash the input data to the initial x coordinate is somewhat abritrary. That's why the unit tests that check explicit output values are marked as regression tests: they're to test future implementations against this implementation, not this implementation against some external implementation or standard.
|
I will be on vacation for 2 weeks starting Monday. My colleagues @gnarula and @ineiti will keep working with you on this PR. Thanks again for your contribution. Speaking of contribution, if @AkshayaMani is the author of some of these commits, she should also sign a CLAI and send it in. Ask Prof Goldberg for where to send it. |
|
Hello guys. How does this PR relate to the internet draft for the same thing: https://datatracker.ietf.org/doc/draft-irtf-cfrg-hash-to-curve/ I found this while looking at cloudflare/bn256#17 |
|
Ian writes: That internet draft specifies a different (and constant-time!) algorithm for hash-to-curve in both G1 and G2. Our PR adapts the existing (not constant-time) hash-to-G1 algorithm to the G2 curve. As you noted earlier, if you change the existing hash-to-G1 algorithm, existing signatures would stop validating, so you'd need to be careful about migrating to the internet draft version of hash-to-curve. |
|
Ian Goldberg seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
|
Hello and thank you for your work, unfortunately we won't merge, implement hashing following RFC9380 |
Implemented hashing to G2 (pairing/bn256/point.go) and a helper function that computes the square root of an element in gfP2 (pairing/bn256/gfp2.go).
Added the following tests: (i) a test to verify the implementation of hashing to G2, (ii) a regression test for hashing to G2, and (iii) a test for checking the bilinearity of pairings computed with the points output by G1's and G2's Hash functions.