From 284871a2904b421624e21630226626f057b4fb1c Mon Sep 17 00:00:00 2001 From: Piotr Dyraga Date: Fri, 5 Jan 2024 17:18:44 +0100 Subject: [PATCH 1/6] Draft implementation of BIP-340 signature verification This is a draft implementation. The remaining BIP-340 test vectors need to be added, TODOs need to be addressed, and one failing test needs to be fixed. The VerifySignature verifies the provided BIP-340 signature for the message against the group public key. The function returns true and nil error when the signature is valid. The function returns false and an error when the signature is invalid. The error provides a detailed explanation on why the signature verification failed. VerifySignature implements Verify(pk, m, sig) function as defined in BIP-340. One important design decision is to accept *Signature and *Point into Verify function instead of bytes, as in the prototype. This has the implication. To ensure consistency and that we'll not return positive verification result for (x,y) that is not on the curve, we need to verify y coordinate even though BIP-340 verification is not really interested in it. I think this is acceptable given the complexity of byte operations is hidden behind a ciphersuite and inside the protocol code, we'll operate on known domain objects. --- frost/bip340.go | 115 ++++++++++++++++++++++++++++++++ frost/bip340_test.go | 99 +++++++++++++++++++++++++++ frost/ciphersuite.go | 11 +++ internal/testutils/testutils.go | 13 ++++ 4 files changed, 238 insertions(+) diff --git a/frost/bip340.go b/frost/bip340.go index 17888a2..815ceac 100644 --- a/frost/bip340.go +++ b/frost/bip340.go @@ -2,6 +2,7 @@ package frost import ( "crypto/sha256" + "fmt" "math/big" "github.com/ethereum/go-ethereum/crypto/secp256k1" @@ -213,6 +214,120 @@ func (b *Bip340Ciphersuite) EncodePoint(point *Point) []byte { return xbs } +// VerifySignature verifies the provided [BIP-340] signature for the message +// against the group public key. The function returns true and nil error when +// the signature is valid. The function returns false and an error when the +// signature is invalid. The error provides a detailed explanation on why the +// signature verification failed. +// +// VerifySignature implements Verify(pk, m, sig) function as defined in [BIP-340]. +func (b *Bip340Ciphersuite) VerifySignature( + signature *Signature, + publicKey *Point, + message []byte, +) (bool, error) { + // Not required by [BIP-340] but performed to ensure input data consistency. + // We do not want to return true if Y is an invalid coordinate. + /* + // TODO: check if not nil coordinates + + if !b.curve.IsOnCurve(signature.R.X, signature.R.Y) { + return false, fmt.Errorf("signature.R is not on the curve") + } + if !b.curve.IsOnCurve(publicKey.X, publicKey.Y) { + return false, fmt.Errorf("publicKey is not on the curve") + } + */ + + // Let P = lift_x(int(pk)); fail if that fails. + pk := new(big.Int).SetBytes(b.EncodePoint(publicKey)) + P, err := b.liftX(pk) + if err != nil { + return false, fmt.Errorf("liftX failed: [%v]", err) + } + + // Let r = int(sig[0:32]); fail if r ≥ p. + r := signature.R.X // int(sig[0:32]) + if r.Cmp(b.curve.P) != -1 { + return false, fmt.Errorf("r >= P") + } + + // Let s = int(sig[32:64]); fail if s ≥ n. + s := signature.Z // int(sig[32:64]) + if s.Cmp(b.curve.N) != -1 { + return false, fmt.Errorf("s >= N") + } + + // Let e = int(hashBIP0340/challenge(bytes(r) || bytes(P) || m)) mod n. + eHash := b.H2( + b.EncodePoint(signature.R), + b.EncodePoint(P), + message) + e := new(big.Int).Mod(eHash, b.curve.N) + + // Let R = s⋅G - e⋅P. + R := b.curve.EcSub( + b.curve.EcBaseMul(s), + b.curve.EcMul(P, e), + ) + + // Fail if is_infinite(R) + if !b.curve.IsOnCurve(R.X, R.Y) { + return false, fmt.Errorf("point R is infinite") + } + + // Fail if not has_even_y(R). + if R.Y.Bit(0) != 0 { + return false, fmt.Errorf("coordinate R.y is not even") + } + + // Fail if x(R) != r. + if R.X.Cmp(r) != 0 { + return false, fmt.Errorf("coordinate R.x != r") + } + + // Return success if no failure occurred before reaching this point. + return true, nil +} + +// liftX function implements lift_x(x) function as defined in [BIP-340]. +func (b *Bip340Ciphersuite) liftX(x *big.Int) (*Point, error) { + // From [BIP-340] specification section: + // + // The function lift_x(x), where x is a 256-bit unsigned integer, returns + // the point P for which x(P) = x[10] and has_even_y(P), or fails if x is + // greater than p-1 or no such point exists. + + // Fail if x ≥ p. + p := b.curve.P + if x.Cmp(p) != -1 { + return nil, fmt.Errorf("value of x exceeds field size") + } + + // Let c = x^3 + 7 mod p. + c := new(big.Int).Exp(x, big.NewInt(3), p) + c.Add(c, big.NewInt(7)) + c.Mod(c, p) + + // Let y = c^[(p+1)/4] mod p. + e := new(big.Int).Add(p, big.NewInt(1)) + e.Div(e, big.NewInt(4)) + y := new(big.Int).Exp(c, e, p) + + // Fail if c ≠ y^2 mod p. + y2 := new(big.Int).Exp(y, big.NewInt(2), p) + if c.Cmp(y2) != 0 { + return nil, fmt.Errorf("no curve point matching x") + } + + // Return the unique point P such that x(P) = x and y(P) = y if y mod 2 = 0 + // or y(P) = p-y otherwise. + if y.Bit(0) != 0 { + y.Sub(p, y) + } + return &Point{x, y}, nil +} + // concat performs a concatenation of byte slices without the modification of // the slices passed as parameters. A brand new slice instance is always // returned from the function. diff --git a/frost/bip340_test.go b/frost/bip340_test.go index 86bfb66..1025a9b 100644 --- a/frost/bip340_test.go +++ b/frost/bip340_test.go @@ -2,6 +2,8 @@ package frost import ( "bytes" + "encoding/hex" + "fmt" "math/big" "testing" @@ -407,6 +409,103 @@ func TestBip340CiphersuiteHash(t *testing.T) { } } +func TestVerifySignature(t *testing.T) { + tests := []struct { + signature string + publicKeyX string + message string + isValid bool + expectedErr string + }{ + { + signature: "E907831F80848D1069A5371B402410364BDF1C5F8307B0084C55F1CE2DCA821525F66A4A85EA8B71E482A74F382D2CE5EBEEE8FDB2172F477DF4900D310536C0", + publicKeyX: "F9308A019258C31049344F85F89D5229B531C845836F99B08601F113BCE036F9", + message: "0000000000000000000000000000000000000000000000000000000000000000", + isValid: true, + }, + { + signature: "6896BD60EEAE296DB48A229FF71DFE071BDE413E6D43F917DC8DCF8C78DE33418906D11AC976ABCCB20B091292BFF4EA897EFCB639EA871CFA95F6DE339E4B0A", + publicKeyX: "DFF1D77F2A671C5F36183726DB2341BE58FEAE1DA2DECED843240F7B502BA659", + message: "243F6A8885A308D313198A2E03707344A4093822299F31D0082EFA98EC4E6C89", + isValid: true, + }, + { + signature: "5831AAEED7B44BB74E5EAB94BA9D4294C49BCF2A60728D8B4C200F50DD313C1BAB745879A5AD954A72C45A91C3A51D3C7ADEA98D82F8481E0E1E03674A6F3FB7", + publicKeyX: "DD308AFEC5777E13121FA72B9CC1B7CC0139715309B086C960E18FD969774EB8", + message: "7E2D58D8B3BCDF1ABADEC7829054F90DDA9805AAB56C77333024B9D0A508B75C", + isValid: true, + }, + { + signature: "7EB0509757E246F19449885651611CB965ECC1A187DD51B64FDA1EDC9637D5EC97582B9CB13DB3933705B32BA982AF5AF25FD78881EBB32771FC5922EFC66EA3", + publicKeyX: "25D1DFF95105F5253C4022F628A996AD3A0D95FBF21D468A1B33F8C160D8F517", + message: "FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF", + isValid: true, + }, + { + signature: "00000000000000000000003B78CE563F89A0ED9414F5AA28AD0D96D6795F9C6376AFB1548AF603B3EB45C9F8207DEE1060CB71C04E80F593060B07D28308D7F4", + publicKeyX: "D69C3509BB99E412E68B0FE8544E72837DFA30746D8BE2AA65975F29D22DC7B9", + message: "4DF3C3F68FCC83B27E9D42C90431A72499F17875C81A599B566C9889B9696703", + isValid: true, + }, + // TODO: add the remaining BIP-340 test vectors + // https://github.com/bitcoin/bips/blob/master/bip-0340/test-vectors.csv + } + + for i, test := range tests { + t.Run(fmt.Sprintf("test case %v", i), func(t *testing.T) { + sigBytes, err := hex.DecodeString(test.signature) + if err != nil { + t.Fatal(err) + } + + pubKeyXBytes, err := hex.DecodeString(test.publicKeyX) + if err != nil { + t.Fatal(err) + } + + msg, err := hex.DecodeString(test.message) + if err != nil { + t.Fatal(err) + } + + signature := &Signature{ + R: &Point{ + X: new(big.Int).SetBytes(sigBytes[0:32]), + Y: nil, // TODO: fix it + }, + Z: new(big.Int).SetBytes(sigBytes[32:64]), + } + + pubKey := &Point{ + X: new(big.Int).SetBytes(pubKeyXBytes), + Y: nil, // TODO: fix it + } + + ciphersuite = NewBip340Ciphersuite() + res, err := ciphersuite.VerifySignature(signature, pubKey, msg) + + testutils.AssertBoolsEqual( + t, + "signature verification result", + test.isValid, + res, + ) + + if !test.isValid { + if err == nil { + t.Fatal("expected not-nil error") + } + testutils.AssertStringsEqual( + t, + "signature verification error message", + test.expectedErr, + err.Error(), + ) + } + }) + } +} + func TestConcat(t *testing.T) { tests := map[string]struct { expected []byte diff --git a/frost/ciphersuite.go b/frost/ciphersuite.go index 6d878e4..db3426a 100644 --- a/frost/ciphersuite.go +++ b/frost/ciphersuite.go @@ -25,6 +25,17 @@ type Ciphersuite interface { // serialization, always reflecting the given ciphersuite's specification // requirements. EncodePoint(point *Point) []byte + + // VerifySignature verifies the provided signature for the message against + // the group public key. The function returns true and nil error when the + // signature is valid. The function returns false and an error when the + // signature is valid. The error provides a detailed explanation on why + // the signature verification failed. + VerifySignature( + signature *Signature, + publicKey *Point, + message []byte, + ) (bool, error) } // Hashing interface abstracts out hash functions implementations specific to the diff --git a/internal/testutils/testutils.go b/internal/testutils/testutils.go index 5cafd82..1c29bc1 100644 --- a/internal/testutils/testutils.go +++ b/internal/testutils/testutils.go @@ -76,6 +76,19 @@ func AssertStringsEqual(t *testing.T, description string, expected string, actua } } +// AssertBoolsEqual checks if two booleans are equal. If not, it reports a test +// failure. +func AssertBoolsEqual(t *testing.T, description string, expected bool, actual bool) { + if expected != actual { + t.Errorf( + "unexpected %s\nexpected: %v\nactual: %v\n", + description, + expected, + actual, + ) + } +} + func testBytesEqual(expectedBytes []byte, actualBytes []byte) error { minLen := len(expectedBytes) diffCount := 0 From 4fe4459e52ec15d35a6ea4ae11af65f7be31087f Mon Sep 17 00:00:00 2001 From: Piotr Dyraga Date: Mon, 8 Jan 2024 13:16:46 +0100 Subject: [PATCH 2/6] Improved BIP340 sig verification in ciphersuite Added missing test vectors with one test vector failing - this requires further investigation. Explained why we accept public key's Y coordinate in parameters as opposed to what is proposed in BIP-340. tl;dr; support for other ciphersuites and in FROST it does not matter where we strip Y coordinate information: after aggregate and before the verification or inside the verification. --- frost/bip340.go | 47 ++++++++++++---- frost/bip340_test.go | 130 ++++++++++++++++++++++++++++++++++++++++--- 2 files changed, 159 insertions(+), 18 deletions(-) diff --git a/frost/bip340.go b/frost/bip340.go index 815ceac..254b19d 100644 --- a/frost/bip340.go +++ b/frost/bip340.go @@ -220,24 +220,49 @@ func (b *Bip340Ciphersuite) EncodePoint(point *Point) []byte { // signature is invalid. The error provides a detailed explanation on why the // signature verification failed. // -// VerifySignature implements Verify(pk, m, sig) function as defined in [BIP-340]. +// VerifySignature implements Verify(pk, m, sig) function defined in [BIP-340]. func (b *Bip340Ciphersuite) VerifySignature( signature *Signature, publicKey *Point, message []byte, ) (bool, error) { + // This function accepts the public key as an elliptic curve point and + // signature as a structure outputted as defined in [FROST] aggregate + // function. This is not precisely how [BIP-340] defines the verification + // function signature. From [BIP-340]: + // + // "Note that the correctness of verification relies on the fact that lift_x + // always returns a point with an even Y coordinate. A hypothetical + // verification algorithm that treats points as public keys, and takes the + // point P directly as input would fail any time a point with odd Y is used. + // While it is possible to correct for this by negating points with odd Y + // coordinate before further processing, this would result in a scheme where + // every (message, signature) pair is valid for two public keys (a type of + // malleability that exists for ECDSA as well, but we don't wish to retain). + // We avoid these problems by treating just the X coordinate as public key." + // + // In our specific case, we define FROST ciphersuite that will operate on + // the same types as the [FROST] algorithm. This is a requirement to make + // the ciphersuite used generic for [FROST]. + // + // Accepting the public key as a point and signature as a struct outputted + // from the aggregate function makes the code easier to follow as no + // conversions have to be made before the verification. Also, from our + // specific perspective, it does not make a difference where the conversion + // is made and where we strip the public key's Y coordinate information: + // between the aggregation and before the verification or inside the + // verification. For a more generic case where we would validate [BIP-340] + // signatures from Bitcoin chain, it would make more sense to strip Y + // coordinate before calling this function. + // Not required by [BIP-340] but performed to ensure input data consistency. // We do not want to return true if Y is an invalid coordinate. - /* - // TODO: check if not nil coordinates - - if !b.curve.IsOnCurve(signature.R.X, signature.R.Y) { - return false, fmt.Errorf("signature.R is not on the curve") - } - if !b.curve.IsOnCurve(publicKey.X, publicKey.Y) { - return false, fmt.Errorf("publicKey is not on the curve") - } - */ + if !b.curve.IsOnCurve(publicKey.X, publicKey.Y) { + return false, fmt.Errorf("point publicKey is infinite") + } + if publicKey.X.Cmp(b.curve.P) == 1 { + return false, fmt.Errorf("point publicKey exceeds field size") + } // Let P = lift_x(int(pk)); fail if that fails. pk := new(big.Int).SetBytes(b.EncodePoint(publicKey)) diff --git a/frost/bip340_test.go b/frost/bip340_test.go index 1025a9b..0b1c9b4 100644 --- a/frost/bip340_test.go +++ b/frost/bip340_test.go @@ -417,6 +417,7 @@ func TestVerifySignature(t *testing.T) { isValid bool expectedErr string }{ + // official [BIP-340] test vectors: https://github.com/bitcoin/bips/blob/master/bip-0340/test-vectors.csv { signature: "E907831F80848D1069A5371B402410364BDF1C5F8307B0084C55F1CE2DCA821525F66A4A85EA8B71E482A74F382D2CE5EBEEE8FDB2172F477DF4900D310536C0", publicKeyX: "F9308A019258C31049344F85F89D5229B531C845836F99B08601F113BCE036F9", @@ -447,12 +448,124 @@ func TestVerifySignature(t *testing.T) { message: "4DF3C3F68FCC83B27E9D42C90431A72499F17875C81A599B566C9889B9696703", isValid: true, }, - // TODO: add the remaining BIP-340 test vectors - // https://github.com/bitcoin/bips/blob/master/bip-0340/test-vectors.csv + { + signature: "6CFF5C3BA86C69EA4B7376F31A9BCB4F74C1976089B2D9963DA2E5543E17776969E89B4C5564D00349106B8497785DD7D1D713A8AE82B32FA79D5F7FC407D39B", + publicKeyX: "EEFDEA4CDB677750A420FEE807EACF21EB9898AE79B9768766E4FAA04A2D4A34", + message: "243F6A8885A308D313198A2E03707344A4093822299F31D0082EFA98EC4E6C89", + isValid: false, + expectedErr: "point publicKey is infinite", + }, + + { + signature: "FFF97BD5755EEEA420453A14355235D382F6472F8568A18B2F057A14602975563CC27944640AC607CD107AE10923D9EF7A73C643E166BE5EBEAFA34B1AC553E2", + publicKeyX: "DFF1D77F2A671C5F36183726DB2341BE58FEAE1DA2DECED843240F7B502BA659", + message: "243F6A8885A308D313198A2E03707344A4093822299F31D0082EFA98EC4E6C89", + isValid: false, + expectedErr: "coordinate R.y is not even", + }, + { + signature: "1FA62E331EDBC21C394792D2AB1100A7B432B013DF3F6FF4F99FCB33E0E1515F28890B3EDB6E7189B630448B515CE4F8622A954CFE545735AAEA5134FCCDB2BD", + publicKeyX: "DFF1D77F2A671C5F36183726DB2341BE58FEAE1DA2DECED843240F7B502BA659", + message: "243F6A8885A308D313198A2E03707344A4093822299F31D0082EFA98EC4E6C89", + isValid: false, + expectedErr: "coordinate R.y is not even", + }, + + { + signature: "6CFF5C3BA86C69EA4B7376F31A9BCB4F74C1976089B2D9963DA2E5543E177769961764B3AA9B2FFCB6EF947B6887A226E8D7C93E00C5ED0C1834FF0D0C2E6DA6", + publicKeyX: "DFF1D77F2A671C5F36183726DB2341BE58FEAE1DA2DECED843240F7B502BA659", + message: "243F6A8885A308D313198A2E03707344A4093822299F31D0082EFA98EC4E6C89", + isValid: false, + expectedErr: "coordinate R.x != r", + }, + { + signature: "0000000000000000000000000000000000000000000000000000000000000000123DDA8328AF9C23A94C1FEECFD123BA4FB73476F0D594DCB65C6425BD186051", + publicKeyX: "DFF1D77F2A671C5F36183726DB2341BE58FEAE1DA2DECED843240F7B502BA659", + message: "243F6A8885A308D313198A2E03707344A4093822299F31D0082EFA98EC4E6C89", + isValid: false, + expectedErr: "point R is infinite", + }, + { + signature: "00000000000000000000000000000000000000000000000000000000000000017615FBAF5AE28864013C099742DEADB4DBA87F11AC6754F93780D5A1837CF197", + publicKeyX: "DFF1D77F2A671C5F36183726DB2341BE58FEAE1DA2DECED843240F7B502BA659", + message: "243F6A8885A308D313198A2E03707344A4093822299F31D0082EFA98EC4E6C89", + isValid: false, + expectedErr: "point R is infinite", + }, + { + signature: "4A298DACAE57395A15D0795DDBFD1DCB564DA82B0F269BC70A74F8220429BA1D69E89B4C5564D00349106B8497785DD7D1D713A8AE82B32FA79D5F7FC407D39B", + publicKeyX: "DFF1D77F2A671C5F36183726DB2341BE58FEAE1DA2DECED843240F7B502BA659", + message: "243F6A8885A308D313198A2E03707344A4093822299F31D0082EFA98EC4E6C89", + isValid: false, + expectedErr: "point R is infinite", + }, + { + signature: "FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFEFFFFFC2F69E89B4C5564D00349106B8497785DD7D1D713A8AE82B32FA79D5F7FC407D39B", + publicKeyX: "DFF1D77F2A671C5F36183726DB2341BE58FEAE1DA2DECED843240F7B502BA659", + message: "243F6A8885A308D313198A2E03707344A4093822299F31D0082EFA98EC4E6C89", + isValid: false, + expectedErr: "r >= P", + }, + { + signature: "6CFF5C3BA86C69EA4B7376F31A9BCB4F74C1976089B2D9963DA2E5543E177769FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFEBAAEDCE6AF48A03BBFD25E8CD0364141", + publicKeyX: "DFF1D77F2A671C5F36183726DB2341BE58FEAE1DA2DECED843240F7B502BA659", + message: "243F6A8885A308D313198A2E03707344A4093822299F31D0082EFA98EC4E6C89", + isValid: false, + expectedErr: "s >= N", + }, + { + signature: "6CFF5C3BA86C69EA4B7376F31A9BCB4F74C1976089B2D9963DA2E5543E17776969E89B4C5564D00349106B8497785DD7D1D713A8AE82B32FA79D5F7FC407D39B", + publicKeyX: "FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFEFFFFFC30", + message: "243F6A8885A308D313198A2E03707344A4093822299F31D0082EFA98EC4E6C89", + isValid: false, + expectedErr: "point publicKey exceeds field size", + }, + { + signature: "71535DB165ECD9FBBC046E5FFAEA61186BB6AD436732FCCC25291A55895464CF6069CE26BF03466228F19A3A62DB8A649F2D560FAC652827D1AF0574E427AB63", + publicKeyX: "778CAA53B4393AC467774D09497A87224BF9FAB6F6E68B23086497324D6FD117", + message: "", + isValid: true, + }, + { + signature: "08A20A0AFEF64124649232E0693C583AB1B9934AE63B4C3511F3AE1134C6A303EA3173BFEA6683BD101FA5AA5DBC1996FE7CACFC5A577D33EC14564CEC2BACBF", + publicKeyX: "778CAA53B4393AC467774D09497A87224BF9FAB6F6E68B23086497324D6FD117", + message: "11", + isValid: true, + }, + { + signature: "5130F39A4059B43BC7CAC09A19ECE52B5D8699D1A71E3C52DA9AFDB6B50AC370C4A482B77BF960F8681540E25B6771ECE1E5A37FD80E5A51897C5566A97EA5A5", + publicKeyX: "778CAA53B4393AC467774D09497A87224BF9FAB6F6E68B23086497324D6FD117", + message: "0102030405060708090A0B0C0D0E0F1011", + isValid: true, + }, + { + signature: "403B12B0D8555A344175EA7EC746566303321E5DBFA8BE6F091635163ECA79A8585ED3E3170807E7C03B720FC54C7B23897FCBA0E9D0B4A06894CFD249F22367", + publicKeyX: "778CAA53B4393AC467774D09497A87224BF9FAB6F6E68B23086497324D6FD117", + message: "99999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999", + isValid: true, + }, } for i, test := range tests { t.Run(fmt.Sprintf("test case %v", i), func(t *testing.T) { + ciphersuite = NewBip340Ciphersuite() + + calculateY := func(x *big.Int) *big.Int { + x3 := new(big.Int).Mul(x, x) //x² + x3.Mul(x3, x) //x³ + x3.Add(x3, ciphersuite.curve.B) //x³+B + x3.Mod(x3, ciphersuite.curve.P) //(x³+B)%P + y := new(big.Int).ModSqrt(x3, ciphersuite.curve.P) + + // x is not on the curve; this is a negative test case for + // which we can't calculate y + if y == nil { + return big.NewInt(2) // even + } + + return y + } + sigBytes, err := hex.DecodeString(test.signature) if err != nil { t.Fatal(err) @@ -468,20 +581,23 @@ func TestVerifySignature(t *testing.T) { t.Fatal(err) } + rX := new(big.Int).SetBytes(sigBytes[0:32]) + rY := calculateY(rX) signature := &Signature{ R: &Point{ - X: new(big.Int).SetBytes(sigBytes[0:32]), - Y: nil, // TODO: fix it + X: rX, + Y: rY, }, Z: new(big.Int).SetBytes(sigBytes[32:64]), } + pubKeyX := new(big.Int).SetBytes(pubKeyXBytes) + pubKeyY := calculateY(pubKeyX) pubKey := &Point{ - X: new(big.Int).SetBytes(pubKeyXBytes), - Y: nil, // TODO: fix it + X: pubKeyX, + Y: pubKeyY, } - ciphersuite = NewBip340Ciphersuite() res, err := ciphersuite.VerifySignature(signature, pubKey, msg) testutils.AssertBoolsEqual( From d50ca09e6031e5a718f35e8740428b2cdbab8b13 Mon Sep 17 00:00:00 2001 From: Piotr Dyraga Date: Mon, 8 Jan 2024 13:19:35 +0100 Subject: [PATCH 3/6] EncodePoint function moved to Hashing interface EncodePoint is important for hashing, especially H2. It is not important for the signature verification calls (in our current model) or for the curve operations. --- frost/ciphersuite.go | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/frost/ciphersuite.go b/frost/ciphersuite.go index db3426a..8d22396 100644 --- a/frost/ciphersuite.go +++ b/frost/ciphersuite.go @@ -14,18 +14,6 @@ type Ciphersuite interface { Hashing Curve() Curve - // EncodePoint encodes the given elliptic curve point to a byte slice in - // a way that is *specific* to the given ciphersuite needs. This is - // especially important when calculating a signature challenge in [FROST]. - // - // This function may yield a different result than SerializePoint function - // from the Curve interface. While the SerializePoint result should be - // considered an internal serialization that may be optimized for speed or - // data consistency, the EncodePoint result should be considered an external - // serialization, always reflecting the given ciphersuite's specification - // requirements. - EncodePoint(point *Point) []byte - // VerifySignature verifies the provided signature for the message against // the group public key. The function returns true and nil error when the // signature is valid. The function returns false and an error when the @@ -55,6 +43,18 @@ type Hashing interface { H3(m []byte, ms ...[]byte) *big.Int H4(m []byte) []byte H5(m []byte) []byte + + // EncodePoint encodes the given elliptic curve point to a byte slice in + // a way that is *specific* to the given ciphersuite needs. This is + // especially important when calculating a signature challenge in [FROST]. + // + // This function may yield a different result than SerializePoint function + // from the Curve interface. While the SerializePoint result should be + // considered an internal serialization that may be optimized for speed or + // data consistency, the EncodePoint result should be considered an external + // serialization, always reflecting the given ciphersuite's specification + // requirements. + EncodePoint(point *Point) []byte } // Curve interface abstracts out the particular elliptic curve implementation From 929f34ddccefd1a357804b1dfa2004db4ee4b4c4 Mon Sep 17 00:00:00 2001 From: Piotr Dyraga Date: Mon, 8 Jan 2024 16:36:00 +0100 Subject: [PATCH 4/6] Fixed the expected error message in BIP340 verification test This test was previously failing. The signature verification algorithm in BIP-340 does not check that the 32 public key bytes in the signature match a valid point on the curve. This check is effectively implicit in the algorithm later, because G and P are known valid curve points, so the point R that is calculated is also a valid point. Thus, if r is not a valid point's X-coordinate, R.X != r. --- frost/bip340_test.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/frost/bip340_test.go b/frost/bip340_test.go index 0b1c9b4..062bc3c 100644 --- a/frost/bip340_test.go +++ b/frost/bip340_test.go @@ -455,7 +455,6 @@ func TestVerifySignature(t *testing.T) { isValid: false, expectedErr: "point publicKey is infinite", }, - { signature: "FFF97BD5755EEEA420453A14355235D382F6472F8568A18B2F057A14602975563CC27944640AC607CD107AE10923D9EF7A73C643E166BE5EBEAFA34B1AC553E2", publicKeyX: "DFF1D77F2A671C5F36183726DB2341BE58FEAE1DA2DECED843240F7B502BA659", @@ -497,7 +496,7 @@ func TestVerifySignature(t *testing.T) { publicKeyX: "DFF1D77F2A671C5F36183726DB2341BE58FEAE1DA2DECED843240F7B502BA659", message: "243F6A8885A308D313198A2E03707344A4093822299F31D0082EFA98EC4E6C89", isValid: false, - expectedErr: "point R is infinite", + expectedErr: "coordinate R.x != r", }, { signature: "FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFEFFFFFC2F69E89B4C5564D00349106B8497785DD7D1D713A8AE82B32FA79D5F7FC407D39B", From e5e8ca78eb960d8458559562aa74be0bcffdbc3d Mon Sep 17 00:00:00 2001 From: Piotr Dyraga Date: Mon, 8 Jan 2024 16:37:53 +0100 Subject: [PATCH 5/6] Simplified BIP-340 verification error messages --- frost/bip340.go | 10 +++++----- frost/bip340_test.go | 16 ++++++++-------- 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/frost/bip340.go b/frost/bip340.go index 254b19d..77b15cd 100644 --- a/frost/bip340.go +++ b/frost/bip340.go @@ -258,10 +258,10 @@ func (b *Bip340Ciphersuite) VerifySignature( // Not required by [BIP-340] but performed to ensure input data consistency. // We do not want to return true if Y is an invalid coordinate. if !b.curve.IsOnCurve(publicKey.X, publicKey.Y) { - return false, fmt.Errorf("point publicKey is infinite") + return false, fmt.Errorf("publicKey is infinite") } if publicKey.X.Cmp(b.curve.P) == 1 { - return false, fmt.Errorf("point publicKey exceeds field size") + return false, fmt.Errorf("publicKey exceeds field size") } // Let P = lift_x(int(pk)); fail if that fails. @@ -298,17 +298,17 @@ func (b *Bip340Ciphersuite) VerifySignature( // Fail if is_infinite(R) if !b.curve.IsOnCurve(R.X, R.Y) { - return false, fmt.Errorf("point R is infinite") + return false, fmt.Errorf("R is infinite") } // Fail if not has_even_y(R). if R.Y.Bit(0) != 0 { - return false, fmt.Errorf("coordinate R.y is not even") + return false, fmt.Errorf("R.y is not even") } // Fail if x(R) != r. if R.X.Cmp(r) != 0 { - return false, fmt.Errorf("coordinate R.x != r") + return false, fmt.Errorf("R.x != r") } // Return success if no failure occurred before reaching this point. diff --git a/frost/bip340_test.go b/frost/bip340_test.go index 062bc3c..dd18be9 100644 --- a/frost/bip340_test.go +++ b/frost/bip340_test.go @@ -453,21 +453,21 @@ func TestVerifySignature(t *testing.T) { publicKeyX: "EEFDEA4CDB677750A420FEE807EACF21EB9898AE79B9768766E4FAA04A2D4A34", message: "243F6A8885A308D313198A2E03707344A4093822299F31D0082EFA98EC4E6C89", isValid: false, - expectedErr: "point publicKey is infinite", + expectedErr: "publicKey is infinite", }, { signature: "FFF97BD5755EEEA420453A14355235D382F6472F8568A18B2F057A14602975563CC27944640AC607CD107AE10923D9EF7A73C643E166BE5EBEAFA34B1AC553E2", publicKeyX: "DFF1D77F2A671C5F36183726DB2341BE58FEAE1DA2DECED843240F7B502BA659", message: "243F6A8885A308D313198A2E03707344A4093822299F31D0082EFA98EC4E6C89", isValid: false, - expectedErr: "coordinate R.y is not even", + expectedErr: "R.y is not even", }, { signature: "1FA62E331EDBC21C394792D2AB1100A7B432B013DF3F6FF4F99FCB33E0E1515F28890B3EDB6E7189B630448B515CE4F8622A954CFE545735AAEA5134FCCDB2BD", publicKeyX: "DFF1D77F2A671C5F36183726DB2341BE58FEAE1DA2DECED843240F7B502BA659", message: "243F6A8885A308D313198A2E03707344A4093822299F31D0082EFA98EC4E6C89", isValid: false, - expectedErr: "coordinate R.y is not even", + expectedErr: "R.y is not even", }, { @@ -475,28 +475,28 @@ func TestVerifySignature(t *testing.T) { publicKeyX: "DFF1D77F2A671C5F36183726DB2341BE58FEAE1DA2DECED843240F7B502BA659", message: "243F6A8885A308D313198A2E03707344A4093822299F31D0082EFA98EC4E6C89", isValid: false, - expectedErr: "coordinate R.x != r", + expectedErr: "R.x != r", }, { signature: "0000000000000000000000000000000000000000000000000000000000000000123DDA8328AF9C23A94C1FEECFD123BA4FB73476F0D594DCB65C6425BD186051", publicKeyX: "DFF1D77F2A671C5F36183726DB2341BE58FEAE1DA2DECED843240F7B502BA659", message: "243F6A8885A308D313198A2E03707344A4093822299F31D0082EFA98EC4E6C89", isValid: false, - expectedErr: "point R is infinite", + expectedErr: "R is infinite", }, { signature: "00000000000000000000000000000000000000000000000000000000000000017615FBAF5AE28864013C099742DEADB4DBA87F11AC6754F93780D5A1837CF197", publicKeyX: "DFF1D77F2A671C5F36183726DB2341BE58FEAE1DA2DECED843240F7B502BA659", message: "243F6A8885A308D313198A2E03707344A4093822299F31D0082EFA98EC4E6C89", isValid: false, - expectedErr: "point R is infinite", + expectedErr: "R is infinite", }, { signature: "4A298DACAE57395A15D0795DDBFD1DCB564DA82B0F269BC70A74F8220429BA1D69E89B4C5564D00349106B8497785DD7D1D713A8AE82B32FA79D5F7FC407D39B", publicKeyX: "DFF1D77F2A671C5F36183726DB2341BE58FEAE1DA2DECED843240F7B502BA659", message: "243F6A8885A308D313198A2E03707344A4093822299F31D0082EFA98EC4E6C89", isValid: false, - expectedErr: "coordinate R.x != r", + expectedErr: "R.x != r", }, { signature: "FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFEFFFFFC2F69E89B4C5564D00349106B8497785DD7D1D713A8AE82B32FA79D5F7FC407D39B", @@ -517,7 +517,7 @@ func TestVerifySignature(t *testing.T) { publicKeyX: "FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFEFFFFFC30", message: "243F6A8885A308D313198A2E03707344A4093822299F31D0082EFA98EC4E6C89", isValid: false, - expectedErr: "point publicKey exceeds field size", + expectedErr: "publicKey exceeds field size", }, { signature: "71535DB165ECD9FBBC046E5FFAEA61186BB6AD436732FCCC25291A55895464CF6069CE26BF03466228F19A3A62DB8A649F2D560FAC652827D1AF0574E427AB63", From e16596e5635a28cff33218546d818ee211d98cff Mon Sep 17 00:00:00 2001 From: Piotr Dyraga Date: Mon, 8 Jan 2024 16:42:50 +0100 Subject: [PATCH 6/6] EcSub for Bip340 should keep the Y coordinate in the field order This does not make any difference to the result but it is preferable to stay in the field order for the clarity's sake. --- frost/bip340.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frost/bip340.go b/frost/bip340.go index 77b15cd..9a36153 100644 --- a/frost/bip340.go +++ b/frost/bip340.go @@ -55,7 +55,7 @@ func (bc *Bip340Curve) EcAdd(a *Point, b *Point) *Point { // EcSub returns the subtraction of two elliptic curve points. func (bc *Bip340Curve) EcSub(a *Point, b *Point) *Point { - bNeg := &Point{b.X, new(big.Int).Neg(b.Y)} + bNeg := &Point{b.X, new(big.Int).Sub(bc.Params().P, b.Y)} return bc.EcAdd(a, bNeg) }