Remove BLS signatures aggregate #540
Conversation
K1li4nL
force-pushed
the
clean-up-bls
branch
3 times, most recently
from
0b262c6
to
fbbb53b
Compare
There was a problem hiding this comment.
Overalls LGTM, but this could benefit from @AnomalRoil 's review.
|
Turning this back into draft, I didn't properly ran the benchmarks... It seems that our bdn implementation assumes kyber.Scalar to be mod.Int, making it incompatible to use with circl_bls12381 |
K1li4nL
force-pushed
the
clean-up-bls
branch
from
9b67b52
to
4092ac2
Compare
sign/bls/bls.go
Outdated
| // new version of the protocol should be used to make sure a signature | ||
| // When using aggregated signatures, this version is vulnerable to rogue | ||
| // public-key attack. | ||
| // The new version of the protocol should be used to make sure a signature |
There was a problem hiding this comment.
| // The new version of the protocol should be used to make sure a signature | |
| // The `sign/bdn` package should be used to make sure a signature |
| // such that src[0] goes into dst[len-1] and vice versa. | ||
| // dst and src may be the same slice but otherwise must not overlap. | ||
| func reverse(dst, src []byte) []byte { | ||
| if dst == nil { |
There was a problem hiding this comment.
What if len(dst) != len(src) ? Either explain behaviour in doc, or enforce len(dst) == len(src)?
There was a problem hiding this comment.
good point, I enforced the equality
sign/bdn/bdn.go
Outdated
| l := len(dst) | ||
| for i, j := 0, l-1; i < (l+1)/2; { | ||
| dst[i], dst[j] = src[j], src[i] | ||
| i++ | ||
| j-- | ||
| } |
There was a problem hiding this comment.
| l := len(dst) | |
| for i, j := 0, l-1; i < (l+1)/2; { | |
| dst[i], dst[j] = src[j], src[i] | |
| i++ | |
| j-- | |
| } | |
| for left, right := 0, len(dst)-1; left < right; left, right = left+1, right-1 { | |
| dst[left], dst[right] = src[right], src[left] | |
| } |
sign/bdn/bdn.go
Outdated
| b, err := mod.NewIntBytes(out[i*16:(i+1)*16], modulus128, kyber.LittleEndian).MarshalBinary() | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
| if g.Scalar().ByteOrder() == kyber.BigEndian { | ||
| reverse(b, b) | ||
| } | ||
|
|
||
| coefs[i] = g.Scalar() | ||
| coefs[i].SetBytes(b) |
There was a problem hiding this comment.
g.Scalar is of size 32 whereas the coefs in bdn are meant to be of size 128 bits, e.g. 16.
I think that's explaining the issues you've had so far.
Also why not just:
| b, err := mod.NewIntBytes(out[i*16:(i+1)*16], modulus128, kyber.LittleEndian).MarshalBinary() | |
| if err != nil { | |
| return nil, err | |
| } | |
| if g.Scalar().ByteOrder() == kyber.BigEndian { | |
| reverse(b, b) | |
| } | |
| coefs[i] = g.Scalar() | |
| coefs[i].SetBytes(b) | |
| b, err := mod.NewIntBytes(out[i*16:(i+1)*16], modulus128, g.Scalar().ByteOrder()).MarshalBinary() | |
| if err != nil { | |
| return nil, err | |
| } | |
| coefs[i] = b |
There was a problem hiding this comment.
Although it looks cleaner, this is a breaking change, these bytes in out were always interpreted in little-endian mod 2^128 with no regard to the endianness of the suite.
About the 32bytes, yes exactly, when we assumed the suite would use mod/int as scalars we had 16bytes because the modulus of our mod/int scalar was defined as 2^128:
b, err := mod.NewIntBytes(out[i*16:(i+1)*16], modulus128, kyber.LittleEndian)
Now that we are agnostic to the scalar implementation underneath, we can't define the modulus that way (unless we touch the interface) hence we get the imprecise 32. I am not sure about the best way to solve that.
K1li4nL
force-pushed
the
clean-up-bls
branch
from
4092ac2
to
071ab17
Compare
|
As the BLS signature aggregate scheme is vulnerable to rogue public-key attack, this pr removes the "aggregate" part of the code and use BDN where BLS aggregates were used.