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

bls12-381: Add edge case signature #549

Merged
merged 2 commits into from
Sep 23, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 30 additions & 0 deletions pairing/bls12381/bls12381_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -810,3 +810,33 @@ func BLSBenchmark(b *testing.B, curveOption string) {
}
})
}

// This signature fails to validate when using kilic@7536ae1cb938a818cffabc34c4f9a7335b1c7f9a or latter
func TestSignatureEdgeCase(t *testing.T) {
// G2 pubkey
publicBytes := []byte{0x83, 0xcf, 0xf, 0x28, 0x96, 0xad, 0xee, 0x7e, 0xb8, 0xb5, 0xf0, 0x1f, 0xca, 0xd3, 0x91, 0x22, 0x12, 0xc4, 0x37, 0xe0, 0x7, 0x3e, 0x91, 0x1f, 0xb9, 0x0, 0x22, 0xd3, 0xe7, 0x60, 0x18, 0x3c, 0x8c, 0x4b, 0x45, 0xb, 0x6a, 0xa, 0x6c, 0x3a, 0xc6, 0xa5, 0x77, 0x6a, 0x2d, 0x10, 0x64, 0x51, 0xd, 0x1f, 0xec, 0x75, 0x8c, 0x92, 0x1c, 0xc2, 0x2b, 0xe, 0x17, 0xe6, 0x3a, 0xaf, 0x4b, 0xcb, 0x5e, 0xd6, 0x63, 0x4, 0xde, 0x9c, 0xf8, 0x9, 0xbd, 0x27, 0x4c, 0xa7, 0x3b, 0xab, 0x4a, 0xf5, 0xa6, 0xe9, 0xc7, 0x6a, 0x4b, 0xc0, 0x9e, 0x76, 0xea, 0xe8, 0x99, 0x1e, 0xf5, 0xec, 0xe4, 0x5a}
message := []byte{0xa1, 0xc6, 0xbe, 0xc3, 0xb9, 0xa6, 0xf0, 0x98, 0x9d, 0x4d, 0x80, 0x2d, 0xbf, 0xe2, 0xb9, 0xb, 0x49, 0x5f, 0xa1, 0x74, 0x2b, 0x58, 0x99, 0x63, 0x45, 0x1e, 0xeb, 0xa9, 0xb1, 0x87, 0xb8, 0x15}
// G1 signature
sig := []byte{0x95, 0x89, 0x0, 0x9b, 0x47, 0xbf, 0xd9, 0xe3, 0x65, 0x10, 0x6b, 0x11, 0xa3, 0x42, 0xfe, 0x50, 0x75, 0xeb, 0x44, 0x5, 0xb0, 0x2b, 0x80, 0xe8, 0x93, 0x42, 0x69, 0x86, 0xcf, 0xb6, 0x0, 0x77, 0x99, 0x8e, 0x3b, 0x47, 0x99, 0x68, 0x86, 0xe0, 0x35, 0xca, 0x1c, 0xde, 0x5f, 0xd9, 0x62, 0x89}

var suites = []struct {
name string
suite pairing.Suite
}{
{"kilic", kilic.NewBLS12381Suite()},
{"circl", circl.NewSuiteBLS12381()},
}
for _, s := range suites {
suite := s.suite
t.Run(s.name, func(t *testing.T) {
schemeOnG1 := bls.NewSchemeOnG1(suite)
pubkey := suite.G2().Point()
err := pubkey.UnmarshalBinary(publicBytes)
require.NoError(t, err)

err = schemeOnG1.Verify(pubkey, message, sig)
require.NoError(t, err)
})

}
}
4 changes: 3 additions & 1 deletion pairing/bls12381/circl/g1.go
Original file line number Diff line number Diff line change
Expand Up @@ -97,5 +97,7 @@ func (p *G1Elt) Mul(s kyber.Scalar, q kyber.Point) kyber.Point {

func (p *G1Elt) IsInCorrectGroup() bool { return p.inner.IsOnG1() }

func (p *G1Elt) Hash(msg []byte) kyber.Point { p.inner.Hash(msg, nil); return p }
var domainG1 = []byte("BLS_SIG_BLS12381G1_XMD:SHA-256_SSWU_RO_NUL_")
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if the DST for the curves shouldn't be the ones as per RFC9380, and the BLS signature DST shouldn't be specified in the BLS and BDN packages rather.
Cc @K1li4nL as well.

Copy link
Contributor

@K1li4nL K1li4nL Sep 18, 2024

Choose a reason for hiding this comment

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

Ideally yes, this would allow us to include the RFC test vectors easily for these curves

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was to match the behaviour of circl to kilic. If there is anything I can do here, please don't hesitate to let me know.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

With the current interface (I guess that is the change you discussed in the other thread), there is no way for the BLS package to specify the DST, otherwise I would have done it there.


func (p *G1Elt) Hash(msg []byte) kyber.Point { p.inner.Hash(msg, domainG1); return p }
func (p *G1Elt) Hash2(msg, dst []byte) kyber.Point { p.inner.Hash(msg, dst); return p }
4 changes: 3 additions & 1 deletion pairing/bls12381/circl/g2.go
Original file line number Diff line number Diff line change
Expand Up @@ -97,5 +97,7 @@ func (p *G2Elt) Mul(s kyber.Scalar, q kyber.Point) kyber.Point {

func (p *G2Elt) IsInCorrectGroup() bool { return p.inner.IsOnG2() }

func (p *G2Elt) Hash(msg []byte) kyber.Point { p.inner.Hash(msg, nil); return p }
var domainG2 = []byte("BLS_SIG_BLS12381G2_XMD:SHA-256_SSWU_RO_NUL_")

func (p *G2Elt) Hash(msg []byte) kyber.Point { p.inner.Hash(msg, domainG2); return p }
func (p *G2Elt) Hash2(msg, dst []byte) kyber.Point { p.inner.Hash(msg, dst); return p }
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if we really need Hash2 still in V4 if we make the Hash take a DST arg by default. Cc @K1li4nL

Copy link
Contributor

Choose a reason for hiding this comment

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

Unless there are good reasons to have this flexibility I suppose we can get rid of Hash2() and have a default DST yes

2 changes: 1 addition & 1 deletion sign/bls/bls.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ func (s *scheme) Verify(X kyber.Point, msg, sig []byte) error {
HM := hashable.Hash(msg)
sigPoint := s.sigGroup.Point()
if err := sigPoint.UnmarshalBinary(sig); err != nil {
return err
return fmt.Errorf("bls: unmarshalling signature point: %w", err)
}
if !s.pairing(X, HM, sigPoint) {
return errors.New("bls: invalid signature")
Expand Down
Loading