From c5cc3c8d3fbb4636922864105e98162012ae0162 Mon Sep 17 00:00:00 2001 From: Yawning Angel <3646968+Yawning@users.noreply.github.com> Date: Sat, 26 Jun 2021 16:53:30 +0000 Subject: [PATCH] crypto: Use a different library for ed25519/sr25519 (#6526) At Oasis we have spend some time writing a new Ed25519/X25519/sr25519 implementation called curve25519-voi. This PR switches the import from ed25519consensus/go-schnorrkel, which should lead to performance gains on most systems. Summary of changes: * curve25519-voi is now used for Ed25519 operations, following the existing ZIP-215 semantics. * curve25519-voi's public key cache is enabled (hardcoded size of 4096 entries, should be tuned, see the code comment) to accelerate repeated Ed25519 verification with the same public key(s). * (BREAKING) curve25519-voi is now used for sr25519 operations. This is a breaking change as the current sr25519 support does something decidedly non-standard when going from a MiniSecretKey to a SecretKey and or PublicKey (The expansion routine is called twice). While I believe the new behavior (that expands once and only once) to be more "correct", this changes the semantics as implemented. * curve25519-voi is now used for merlin since the included STROBE implementation produces much less garbage on the heap. Side issues fixed: * The version of go-schnorrkel that is currently imported by tendermint has a badly broken batch verification implementation. Upstream has fixed the issue after I reported it, so the version should be bumped in the interim. Open design questions/issues: * As noted, the public key cache size should be tuned. It is currently backed by a trivial thread-safe LRU cache, which is not scan-resistant, but replacing it with something better is a matter of implementing an interface. * As far as I can tell, the only reason why serial verification on batch failure is necessary is to provide more detailed error messages (that are only used in some unit tests). If you trust the batch verification to be consistent with serial verification then the fallback can be eliminated entirely (the BatchVerifier provided by the new library supports an option that omits the fallback if this is chosen as the way forward). * curve25519-voi's sr25519 support could use more optimization and more eyes on the code. The algorithm unfortunately is woefully under-specified, and the implementation was done primarily because I got really sad when I actually looked at go-schnorrkel, and we do not use the algorithm at this time. --- CHANGELOG_PENDING.md | 3 + crypto/batch/batch.go | 11 ++ crypto/crypto.go | 8 +- crypto/ed25519/bench_test.go | 29 +++- crypto/ed25519/ed25519.go | 57 +++++-- crypto/ed25519/ed25519_test.go | 3 +- crypto/sr25519/batch.go | 40 ++--- crypto/sr25519/bench_test.go | 39 +++-- crypto/sr25519/encoding.go | 12 +- crypto/sr25519/privkey.go | 147 ++++++++++++------ crypto/sr25519/pubkey.go | 69 ++++---- crypto/sr25519/sr25519_test.go | 48 +++++- go.mod | 6 +- go.sum | 22 +-- .../p2p/conn/evil_secret_connection_test.go | 6 +- internal/p2p/conn/secret_connection.go | 16 +- privval/secret_connection.go | 16 +- types/validation.go | 114 +++++++------- 18 files changed, 387 insertions(+), 259 deletions(-) diff --git a/CHANGELOG_PENDING.md b/CHANGELOG_PENDING.md index fc1c38169..327a79a19 100644 --- a/CHANGELOG_PENDING.md +++ b/CHANGELOG_PENDING.md @@ -62,6 +62,7 @@ Friendly reminder: We have a [bug bounty program](https://hackerone.com/tendermi - [mempool] \#6466 The original mempool reactor has been versioned as `v0` and moved to a sub-package under the root `mempool` package. Some core types have been kept in the `mempool` package such as `TxCache` and it's implementations, the `Mempool` interface itself and `TxInfo`. (@alexanderbez) + - [crypto/sr25519] \#6526 Do not re-execute the Ed25519-style key derivation step when doing signing and verification. The derivation is now done once and only once. This breaks `sr25519.GenPrivKeyFromSecret` output compatibility. (@Yawning) - Blockchain Protocol @@ -95,6 +96,8 @@ Friendly reminder: We have a [bug bounty program](https://hackerone.com/tendermi - [statesync] \#6566 Allow state sync fetchers and request timeout to be configurable. (@alexanderbez) - [types] \#6478 Add `block_id` to `newblock` event (@jeebster) - [crypto/ed25519] \#5632 Adopt zip215 `ed25519` verification. (@marbar3778) +- [crypto/ed25519] \#6526 Use [curve25519-voi](https://github.com/oasisprotocol/curve25519-voi) for `ed25519` signing and verification. (@Yawning) +- [crypto/sr25519] \#6526 Use [curve25519-voi](https://github.com/oasisprotocol/curve25519-voi) for `sr25519` signing and verification. (@Yawning) - [privval] \#5603 Add `--key` to `init`, `gen_validator`, `testnet` & `unsafe_reset_priv_validator` for use in generating `secp256k1` keys. - [privval] \#5725 Add gRPC support to private validator. - [privval] \#5876 `tendermint show-validator` will query the remote signer if gRPC is being used (@marbar3778) diff --git a/crypto/batch/batch.go b/crypto/batch/batch.go index e53ceb319..dbd11373c 100644 --- a/crypto/batch/batch.go +++ b/crypto/batch/batch.go @@ -20,3 +20,14 @@ func CreateBatchVerifier(pk crypto.PubKey) (crypto.BatchVerifier, bool) { // case where the key does not support batch verification return nil, false } + +// SupportsBatchVerifier checks if a key type implements the batch verifier +// interface. +func SupportsBatchVerifier(pk crypto.PubKey) bool { + switch pk.Type() { + case ed25519.KeyType, sr25519.KeyType: + return true + } + + return false +} diff --git a/crypto/crypto.go b/crypto/crypto.go index 5d68b578b..8d44b82f5 100644 --- a/crypto/crypto.go +++ b/crypto/crypto.go @@ -46,7 +46,9 @@ type Symmetric interface { type BatchVerifier interface { // Add appends an entry into the BatchVerifier. Add(key PubKey, message, signature []byte) error - // Verify verifies all the entries in the BatchVerifier. - // If the verification fails it is unknown which entry failed and each entry will need to be verified individually. - Verify() bool + // Verify verifies all the entries in the BatchVerifier, and returns + // if every signature in the batch is valid, and a vector of bools + // indicating the verification status of each signature (in the order + // that signatures were added to the batch). + Verify() (bool, []bool) } diff --git a/crypto/ed25519/bench_test.go b/crypto/ed25519/bench_test.go index ca48aa294..e57cd393f 100644 --- a/crypto/ed25519/bench_test.go +++ b/crypto/ed25519/bench_test.go @@ -28,22 +28,37 @@ func BenchmarkVerification(b *testing.B) { } func BenchmarkVerifyBatch(b *testing.B) { + msg := []byte("BatchVerifyTest") + for _, sigsCount := range []int{1, 8, 64, 1024} { sigsCount := sigsCount b.Run(fmt.Sprintf("sig-count-%d", sigsCount), func(b *testing.B) { - b.ReportAllocs() - v := NewBatchVerifier() + // Pre-generate all of the keys, and signatures, but do not + // benchmark key-generation and signing. + pubs := make([]crypto.PubKey, 0, sigsCount) + sigs := make([][]byte, 0, sigsCount) for i := 0; i < sigsCount; i++ { priv := GenPrivKey() - pub := priv.PubKey() - msg := []byte("BatchVerifyTest") sig, _ := priv.Sign(msg) - err := v.Add(pub, msg, sig) - require.NoError(b, err) + pubs = append(pubs, priv.PubKey().(PubKey)) + sigs = append(sigs, sig) } + b.ResetTimer() + + b.ReportAllocs() // NOTE: dividing by n so that metrics are per-signature for i := 0; i < b.N/sigsCount; i++ { - if !v.Verify() { + // The benchmark could just benchmark the Verify() + // routine, but there is non-trivial overhead associated + // with BatchVerifier.Add(), which should be included + // in the benchmark. + v := NewBatchVerifier() + for i := 0; i < sigsCount; i++ { + err := v.Add(pubs[i], msg, sigs[i]) + require.NoError(b, err) + } + + if ok, _ := v.Verify(); !ok { b.Fatal("signature set failed batch verification") } } diff --git a/crypto/ed25519/ed25519.go b/crypto/ed25519/ed25519.go index 5b936618c..3ac7f6d07 100644 --- a/crypto/ed25519/ed25519.go +++ b/crypto/ed25519/ed25519.go @@ -2,13 +2,13 @@ package ed25519 import ( "bytes" - "crypto/ed25519" "crypto/subtle" "errors" "fmt" "io" - "github.com/hdevalence/ed25519consensus" + "github.com/oasisprotocol/curve25519-voi/primitives/ed25519" + "github.com/oasisprotocol/curve25519-voi/primitives/ed25519/extra/cache" "github.com/tendermint/tendermint/crypto" "github.com/tendermint/tendermint/crypto/tmhash" @@ -17,7 +17,18 @@ import ( //------------------------------------- -var _ crypto.PrivKey = PrivKey{} +var ( + _ crypto.PrivKey = PrivKey{} + + // curve25519-voi's Ed25519 implementation supports configurable + // verification behavior, and tendermint uses the ZIP-215 verification + // semantics. + verifyOptions = &ed25519.Options{ + Verify: ed25519.VerifyOptionsZIP_215, + } + + cachingVerifier = cache.NewVerifier(cache.NewLRUCache(cacheSize)) +) const ( PrivKeyName = "tendermint/PrivKeyEd25519" @@ -34,6 +45,14 @@ const ( SeedSize = 32 KeyType = "ed25519" + + // cacheSize is the number of public keys that will be cached in + // an expanded format for repeated signature verification. + // + // TODO/perf: Either this should exclude single verification, or be + // tuned to `> validatorSize + maxTxnsPerBlock` to avoid cache + // thrashing. + cacheSize = 4096 ) func init() { @@ -107,14 +126,12 @@ func GenPrivKey() PrivKey { // genPrivKey generates a new ed25519 private key using the provided reader. func genPrivKey(rand io.Reader) PrivKey { - seed := make([]byte, SeedSize) - - _, err := io.ReadFull(rand, seed) + _, priv, err := ed25519.GenerateKey(rand) if err != nil { panic(err) } - return PrivKey(ed25519.NewKeyFromSeed(seed)) + return PrivKey(priv) } // GenPrivKeyFromSecret hashes the secret with SHA2, and uses @@ -153,7 +170,7 @@ func (pubKey PubKey) VerifySignature(msg []byte, sig []byte) bool { return false } - return ed25519consensus.Verify(ed25519.PublicKey(pubKey), msg, sig) + return cachingVerifier.VerifyWithOptions(ed25519.PublicKey(pubKey), msg, sig, verifyOptions) } func (pubKey PubKey) String() string { @@ -175,30 +192,36 @@ func (pubKey PubKey) Equals(other crypto.PubKey) bool { var _ crypto.BatchVerifier = &BatchVerifier{} // BatchVerifier implements batch verification for ed25519. -// https://github.com/hdevalence/ed25519consensus is used for batch verification type BatchVerifier struct { - ed25519consensus.BatchVerifier + *ed25519.BatchVerifier } func NewBatchVerifier() crypto.BatchVerifier { - return &BatchVerifier{ed25519consensus.NewBatchVerifier()} + return &BatchVerifier{ed25519.NewBatchVerifier()} } func (b *BatchVerifier) Add(key crypto.PubKey, msg, signature []byte) error { - if l := len(key.Bytes()); l != PubKeySize { + pkEd, ok := key.(PubKey) + if !ok { + return fmt.Errorf("pubkey is not Ed25519") + } + + pkBytes := pkEd.Bytes() + + if l := len(pkBytes); l != PubKeySize { return fmt.Errorf("pubkey size is incorrect; expected: %d, got %d", PubKeySize, l) } - // check that the signature is the correct length & the last byte is set correctly - if len(signature) != SignatureSize || signature[63]&224 != 0 { + // check that the signature is the correct length + if len(signature) != SignatureSize { return errors.New("invalid signature") } - b.BatchVerifier.Add(ed25519.PublicKey(key.Bytes()), msg, signature) + cachingVerifier.AddWithOptions(b.BatchVerifier, ed25519.PublicKey(pkBytes), msg, signature, verifyOptions) return nil } -func (b *BatchVerifier) Verify() bool { - return b.BatchVerifier.Verify() +func (b *BatchVerifier) Verify() (bool, []bool) { + return b.BatchVerifier.Verify(crypto.CReader()) } diff --git a/crypto/ed25519/ed25519_test.go b/crypto/ed25519/ed25519_test.go index 6a139e9e2..63c781a3b 100644 --- a/crypto/ed25519/ed25519_test.go +++ b/crypto/ed25519/ed25519_test.go @@ -50,5 +50,6 @@ func TestBatchSafe(t *testing.T) { require.NoError(t, err) } - require.True(t, v.Verify()) + ok, _ := v.Verify() + require.True(t, ok) } diff --git a/crypto/sr25519/batch.go b/crypto/sr25519/batch.go index 9f8665668..462728598 100644 --- a/crypto/sr25519/batch.go +++ b/crypto/sr25519/batch.go @@ -3,40 +3,44 @@ package sr25519 import ( "fmt" - schnorrkel "github.com/ChainSafe/go-schnorrkel" + "github.com/oasisprotocol/curve25519-voi/primitives/sr25519" "github.com/tendermint/tendermint/crypto" ) -var _ crypto.BatchVerifier = BatchVerifier{} +var _ crypto.BatchVerifier = &BatchVerifier{} // BatchVerifier implements batch verification for sr25519. -// https://github.com/ChainSafe/go-schnorrkel is used for batch verification type BatchVerifier struct { - *schnorrkel.BatchVerifier + *sr25519.BatchVerifier } func NewBatchVerifier() crypto.BatchVerifier { - return BatchVerifier{schnorrkel.NewBatchVerifier()} + return &BatchVerifier{sr25519.NewBatchVerifier()} } -func (b BatchVerifier) Add(key crypto.PubKey, msg, sig []byte) error { - var sig64 [SignatureSize]byte - copy(sig64[:], sig) - signature := new(schnorrkel.Signature) - err := signature.Decode(sig64) - if err != nil { - return fmt.Errorf("unable to decode signature: %w", err) +func (b *BatchVerifier) Add(key crypto.PubKey, msg, signature []byte) error { + pk, ok := key.(PubKey) + if !ok { + return fmt.Errorf("sr25519: pubkey is not sr25519") } - signingContext := schnorrkel.NewSigningContext([]byte{}, msg) + var srpk sr25519.PublicKey + if err := srpk.UnmarshalBinary(pk); err != nil { + return fmt.Errorf("sr25519: invalid public key: %w", err) + } + + var sig sr25519.Signature + if err := sig.UnmarshalBinary(signature); err != nil { + return fmt.Errorf("sr25519: unable to decode signature: %w", err) + } - var pk [PubKeySize]byte - copy(pk[:], key.Bytes()) + st := signingCtx.NewTranscriptBytes(msg) + b.BatchVerifier.Add(&srpk, st, &sig) - return b.BatchVerifier.Add(signingContext, signature, schnorrkel.NewPublicKey(pk)) + return nil } -func (b BatchVerifier) Verify() bool { - return b.BatchVerifier.Verify() +func (b *BatchVerifier) Verify() (bool, []bool) { + return b.BatchVerifier.Verify(crypto.CReader()) } diff --git a/crypto/sr25519/bench_test.go b/crypto/sr25519/bench_test.go index 6c3b4e21d..559bd0576 100644 --- a/crypto/sr25519/bench_test.go +++ b/crypto/sr25519/bench_test.go @@ -28,22 +28,37 @@ func BenchmarkVerification(b *testing.B) { } func BenchmarkVerifyBatch(b *testing.B) { - for _, n := range []int{1, 8, 64, 1024} { - n := n - b.Run(fmt.Sprintf("sig-count-%d", n), func(b *testing.B) { - b.ReportAllocs() - v := NewBatchVerifier() - for i := 0; i < n; i++ { + msg := []byte("BatchVerifyTest") + + for _, sigsCount := range []int{1, 8, 64, 1024} { + sigsCount := sigsCount + b.Run(fmt.Sprintf("sig-count-%d", sigsCount), func(b *testing.B) { + // Pre-generate all of the keys, and signatures, but do not + // benchmark key-generation and signing. + pubs := make([]crypto.PubKey, 0, sigsCount) + sigs := make([][]byte, 0, sigsCount) + for i := 0; i < sigsCount; i++ { priv := GenPrivKey() - pub := priv.PubKey() - msg := []byte("BatchVerifyTest") sig, _ := priv.Sign(msg) - err := v.Add(pub, msg, sig) - require.NoError(b, err) + pubs = append(pubs, priv.PubKey().(PubKey)) + sigs = append(sigs, sig) } + b.ResetTimer() + + b.ReportAllocs() // NOTE: dividing by n so that metrics are per-signature - for i := 0; i < b.N/n; i++ { - if !v.Verify() { + for i := 0; i < b.N/sigsCount; i++ { + // The benchmark could just benchmark the Verify() + // routine, but there is non-trivial overhead associated + // with BatchVerifier.Add(), which should be included + // in the benchmark. + v := NewBatchVerifier() + for i := 0; i < sigsCount; i++ { + err := v.Add(pubs[i], msg, sigs[i]) + require.NoError(b, err) + } + + if ok, _ := v.Verify(); !ok { b.Fatal("signature set failed batch verification") } } diff --git a/crypto/sr25519/encoding.go b/crypto/sr25519/encoding.go index 41570b5d0..c0a8a7925 100644 --- a/crypto/sr25519/encoding.go +++ b/crypto/sr25519/encoding.go @@ -1,23 +1,13 @@ package sr25519 -import ( - "github.com/tendermint/tendermint/crypto" - tmjson "github.com/tendermint/tendermint/libs/json" -) - -var _ crypto.PrivKey = PrivKey{} +import tmjson "github.com/tendermint/tendermint/libs/json" const ( PrivKeyName = "tendermint/PrivKeySr25519" PubKeyName = "tendermint/PubKeySr25519" - - // SignatureSize is the size of an Edwards25519 signature. Namely the size of a compressed - // Sr25519 point, and a field element. Both of which are 32 bytes. - SignatureSize = 64 ) func init() { - tmjson.RegisterType(PubKey{}, PubKeyName) tmjson.RegisterType(PrivKey{}, PrivKeyName) } diff --git a/crypto/sr25519/privkey.go b/crypto/sr25519/privkey.go index f85c7af56..f628ca1aa 100644 --- a/crypto/sr25519/privkey.go +++ b/crypto/sr25519/privkey.go @@ -1,70 +1,84 @@ package sr25519 import ( - "crypto/subtle" + "encoding/json" "fmt" "io" + "github.com/oasisprotocol/curve25519-voi/primitives/sr25519" + "github.com/tendermint/tendermint/crypto" +) + +var ( + _ crypto.PrivKey = PrivKey{} - schnorrkel "github.com/ChainSafe/go-schnorrkel" + signingCtx = sr25519.NewSigningContext([]byte{}) ) -// PrivKeySize is the number of bytes in an Sr25519 private key. -const PrivKeySize = 32 +const ( + // PrivKeySize is the size of a sr25519 signature in bytes. + PrivKeySize = sr25519.MiniSecretKeySize -// PrivKeySr25519 implements crypto.PrivKey. -type PrivKey []byte + KeyType = "sr25519" +) -// Bytes returns the byte representation of the PrivKey. +// PrivKey implements crypto.PrivKey. +type PrivKey struct { + msk sr25519.MiniSecretKey + kp *sr25519.KeyPair +} + +// Bytes returns the byte-encoded PrivKey. func (privKey PrivKey) Bytes() []byte { - return []byte(privKey) + if privKey.kp == nil { + return nil + } + return privKey.msk[:] } // Sign produces a signature on the provided message. func (privKey PrivKey) Sign(msg []byte) ([]byte, error) { - var p [PrivKeySize]byte - copy(p[:], privKey) - miniSecretKey, err := schnorrkel.NewMiniSecretKeyFromRaw(p) - if err != nil { - return []byte{}, err + if privKey.kp == nil { + return nil, fmt.Errorf("sr25519: uninitialized private key") } - secretKey := miniSecretKey.ExpandEd25519() - signingContext := schnorrkel.NewSigningContext([]byte{}, msg) + st := signingCtx.NewTranscriptBytes(msg) + + sig, err := privKey.kp.Sign(crypto.CReader(), st) + if err != nil { + return nil, fmt.Errorf("sr25519: failed to sign message: %w", err) + } - sig, err := secretKey.Sign(signingContext) + sigBytes, err := sig.MarshalBinary() if err != nil { - return []byte{}, err + return nil, fmt.Errorf("sr25519: failed to serialize signature: %w", err) } - sigBytes := sig.Encode() - return sigBytes[:], nil + return sigBytes, nil } // PubKey gets the corresponding public key from the private key. +// +// Panics if the private key is not initialized. func (privKey PrivKey) PubKey() crypto.PubKey { - var p [PrivKeySize]byte - copy(p[:], privKey) - miniSecretKey, err := schnorrkel.NewMiniSecretKeyFromRaw(p) - if err != nil { - panic(fmt.Sprintf("Invalid private key: %v", err)) + if privKey.kp == nil { + panic("sr25519: uninitialized private key") } - secretKey := miniSecretKey.ExpandEd25519() - pubkey, err := secretKey.Public() + b, err := privKey.kp.PublicKey().MarshalBinary() if err != nil { - panic(fmt.Sprintf("Could not generate public key: %v", err)) + panic("sr25519: failed to serialize public key: " + err.Error()) } - key := pubkey.Encode() - return PubKey(key[:]) + + return PubKey(b) } // Equals - you probably don't need to use this. // Runs in constant time based on length of the keys. func (privKey PrivKey) Equals(other crypto.PrivKey) bool { - if otherEd, ok := other.(PrivKey); ok { - return subtle.ConstantTimeCompare(privKey[:], otherEd[:]) == 1 + if otherSr, ok := other.(PrivKey); ok { + return privKey.msk.Equal(&otherSr.msk) } return false } @@ -73,6 +87,44 @@ func (privKey PrivKey) Type() string { return KeyType } +func (privKey PrivKey) MarshalJSON() ([]byte, error) { + var b []byte + + // Handle uninitialized private keys gracefully. + if privKey.kp != nil { + b = privKey.Bytes() + } + + return json.Marshal(b) +} + +func (privKey *PrivKey) UnmarshalJSON(data []byte) error { + for i := range privKey.msk { + privKey.msk[i] = 0 + } + privKey.kp = nil + + var b []byte + if err := json.Unmarshal(data, &b); err != nil { + return fmt.Errorf("sr25519: failed to deserialize JSON: %w", err) + } + if len(b) == 0 { + return nil + } + + msk, err := sr25519.NewMiniSecretKeyFromBytes(b) + if err != nil { + return err + } + + sk := msk.ExpandEd25519() + + privKey.msk = *msk + privKey.kp = sk.KeyPair() + + return nil +} + // GenPrivKey generates a new sr25519 private key. // It uses OS randomness in conjunction with the current global random seed // in tendermint/libs/common to generate the private key. @@ -80,20 +132,18 @@ func GenPrivKey() PrivKey { return genPrivKey(crypto.CReader()) } -// genPrivKey generates a new sr25519 private key using the provided reader. -func genPrivKey(rand io.Reader) PrivKey { - var seed [64]byte - - out := make([]byte, 64) - _, err := io.ReadFull(rand, out) +func genPrivKey(rng io.Reader) PrivKey { + msk, err := sr25519.GenerateMiniSecretKey(rng) if err != nil { - panic(err) + panic("sr25519: failed to generate MiniSecretKey: " + err.Error()) } - copy(seed[:], out) + sk := msk.ExpandEd25519() - key := schnorrkel.NewMiniSecretKey(seed).ExpandEd25519().Encode() - return key[:] + return PrivKey{ + msk: *msk, + kp: sk.KeyPair(), + } } // GenPrivKeyFromSecret hashes the secret with SHA2, and uses @@ -102,9 +152,14 @@ func genPrivKey(rand io.Reader) PrivKey { // if it's derived from user input. func GenPrivKeyFromSecret(secret []byte) PrivKey { seed := crypto.Sha256(secret) // Not Ripemd160 because we want 32 bytes. - var bz [PrivKeySize]byte - copy(bz[:], seed) - privKey, _ := schnorrkel.NewMiniSecretKeyFromRaw(bz) - key := privKey.ExpandEd25519().Encode() - return key[:] + + var privKey PrivKey + if err := privKey.msk.UnmarshalBinary(seed); err != nil { + panic("sr25519: failed to deserialize MiniSecretKey: " + err.Error()) + } + + sk := privKey.msk.ExpandEd25519() + privKey.kp = sk.KeyPair() + + return privKey } diff --git a/crypto/sr25519/pubkey.go b/crypto/sr25519/pubkey.go index 8983a6d65..7e701dd1f 100644 --- a/crypto/sr25519/pubkey.go +++ b/crypto/sr25519/pubkey.go @@ -4,74 +4,65 @@ import ( "bytes" "fmt" + "github.com/oasisprotocol/curve25519-voi/primitives/sr25519" + "github.com/tendermint/tendermint/crypto" "github.com/tendermint/tendermint/crypto/tmhash" - - schnorrkel "github.com/ChainSafe/go-schnorrkel" ) var _ crypto.PubKey = PubKey{} -// PubKeySize is the number of bytes in an Sr25519 public key. const ( - PubKeySize = 32 - KeyType = "sr25519" + // PubKeySize is the size of a sr25519 public key in bytes. + PubKeySize = sr25519.PublicKeySize + + // SignatureSize is the size of a sr25519 signature in bytes. + SignatureSize = sr25519.SignatureSize ) -// PubKeySr25519 implements crypto.PubKey for the Sr25519 signature scheme. +// PubKey implements crypto.PubKey. type PubKey []byte // Address is the SHA256-20 of the raw pubkey bytes. func (pubKey PubKey) Address() crypto.Address { - return crypto.Address(tmhash.SumTruncated(pubKey[:])) + if len(pubKey) != PubKeySize { + panic("pubkey is incorrect size") + } + return crypto.Address(tmhash.SumTruncated(pubKey)) } -// Bytes returns the byte representation of the PubKey. +// Bytes returns the PubKey byte format. func (pubKey PubKey) Bytes() []byte { return []byte(pubKey) } -func (pubKey PubKey) VerifySignature(msg []byte, sig []byte) bool { - // make sure we use the same algorithm to sign - if len(sig) != SignatureSize { - return false - } - var sig64 [SignatureSize]byte - copy(sig64[:], sig) - - publicKey := &(schnorrkel.PublicKey{}) - var p [PubKeySize]byte - copy(p[:], pubKey) - err := publicKey.Decode(p) - if err != nil { - return false +func (pubKey PubKey) Equals(other crypto.PubKey) bool { + if otherSr, ok := other.(PubKey); ok { + return bytes.Equal(pubKey[:], otherSr[:]) } - signingContext := schnorrkel.NewSigningContext([]byte{}, msg) + return false +} - signature := &(schnorrkel.Signature{}) - err = signature.Decode(sig64) - if err != nil { +func (pubKey PubKey) VerifySignature(msg []byte, sigBytes []byte) bool { + var srpk sr25519.PublicKey + if err := srpk.UnmarshalBinary(pubKey); err != nil { return false } - return publicKey.Verify(signature, signingContext) -} - -func (pubKey PubKey) String() string { - return fmt.Sprintf("PubKeySr25519{%X}", []byte(pubKey)) -} - -// Equals - checks that two public keys are the same time -// Runs in constant time based on length of the keys. -func (pubKey PubKey) Equals(other crypto.PubKey) bool { - if otherEd, ok := other.(PubKey); ok { - return bytes.Equal(pubKey[:], otherEd[:]) + var sig sr25519.Signature + if err := sig.UnmarshalBinary(sigBytes); err != nil { + return false } - return false + + st := signingCtx.NewTranscriptBytes(msg) + return srpk.Verify(st, &sig) } func (pubKey PubKey) Type() string { return KeyType +} +func (pubKey PubKey) String() string { + return fmt.Sprintf("PubKeySr25519{%X}", []byte(pubKey)) } diff --git a/crypto/sr25519/sr25519_test.go b/crypto/sr25519/sr25519_test.go index 60c7a2999..de5c125f4 100644 --- a/crypto/sr25519/sr25519_test.go +++ b/crypto/sr25519/sr25519_test.go @@ -1,6 +1,8 @@ package sr25519_test import ( + "encoding/base64" + "encoding/json" "testing" "github.com/stretchr/testify/assert" @@ -11,7 +13,6 @@ import ( ) func TestSignAndValidateSr25519(t *testing.T) { - privKey := sr25519.GenPrivKey() pubKey := privKey.PubKey() @@ -32,6 +33,7 @@ func TestSignAndValidateSr25519(t *testing.T) { func TestBatchSafe(t *testing.T) { v := sr25519.NewBatchVerifier() + vFail := sr25519.NewBatchVerifier() for i := 0; i <= 38; i++ { priv := sr25519.GenPrivKey() pub := priv.PubKey() @@ -48,9 +50,49 @@ func TestBatchSafe(t *testing.T) { err = v.Add(pub, msg, sig) require.NoError(t, err) + + switch i % 2 { + case 0: + err = vFail.Add(pub, msg, sig) + case 1: + msg[2] ^= byte(0x01) + err = vFail.Add(pub, msg, sig) + } + require.NoError(t, err) } - if !v.Verify() { - t.Error("failed batch verification") + ok, valid := v.Verify() + require.True(t, ok, "failed batch verification") + for i, ok := range valid { + require.Truef(t, ok, "sig[%d] should be marked valid", i) } + + ok, valid = vFail.Verify() + require.False(t, ok, "succeeded batch verification (invalid batch)") + for i, ok := range valid { + expected := (i % 2) == 0 + require.Equalf(t, expected, ok, "sig[%d] should be %v", i, expected) + } +} + +func TestJSON(t *testing.T) { + privKey := sr25519.GenPrivKey() + + t.Run("PrivKey", func(t *testing.T) { + b, err := json.Marshal(privKey) + require.NoError(t, err) + + // b should be the base64 encoded MiniSecretKey, enclosed by doublequotes. + b64 := base64.StdEncoding.EncodeToString(privKey.Bytes()) + b64 = "\"" + b64 + "\"" + require.Equal(t, []byte(b64), b) + + var privKey2 sr25519.PrivKey + err = json.Unmarshal(b, &privKey2) + require.NoError(t, err) + require.Len(t, privKey2.Bytes(), sr25519.PrivKeySize) + require.EqualValues(t, privKey.Bytes(), privKey2.Bytes()) + }) + + // PubKeys are just []byte, so there is no special handling. } diff --git a/go.mod b/go.mod index bbee413b5..de57b78a5 100644 --- a/go.mod +++ b/go.mod @@ -4,7 +4,6 @@ go 1.15 require ( github.com/BurntSushi/toml v0.3.1 - github.com/ChainSafe/go-schnorrkel v0.0.0-20210222182958-bd440c890782 github.com/Masterminds/squirrel v1.5.0 github.com/Workiva/go-datastructures v1.0.53 github.com/adlio/schema v1.1.13 @@ -19,12 +18,11 @@ require ( github.com/gorilla/websocket v1.4.2 github.com/grpc-ecosystem/go-grpc-middleware v1.3.0 github.com/grpc-ecosystem/go-grpc-prometheus v1.2.0 - github.com/gtank/merlin v0.1.1 - github.com/hdevalence/ed25519consensus v0.0.0-20210204194344-59a8610d2b87 github.com/lib/pq v1.10.2 github.com/libp2p/go-buffer-pool v0.0.2 github.com/minio/highwayhash v1.0.2 github.com/niemeyer/pretty v0.0.0-20200227124842-a10e7caefd8e // indirect + github.com/oasisprotocol/curve25519-voi v0.0.0-20210609091139-0a56a4bca00b github.com/ory/dockertest v3.3.5+incompatible github.com/pkg/errors v0.9.1 github.com/prometheus/client_golang v1.11.0 @@ -37,7 +35,7 @@ require ( github.com/spf13/viper v1.8.0 github.com/stretchr/testify v1.7.0 github.com/tendermint/tm-db v0.6.4 - golang.org/x/crypto v0.0.0-20201117144127-c1f2f97bffc9 + golang.org/x/crypto v0.0.0-20201221181555-eec23a3978ad golang.org/x/net v0.0.0-20210405180319-a5a99cb37ef4 google.golang.org/grpc v1.38.0 gopkg.in/check.v1 v1.0.0-20200902074654-038fdea0a05b // indirect diff --git a/go.sum b/go.sum index 25db57b46..785b487b0 100644 --- a/go.sum +++ b/go.sum @@ -37,15 +37,11 @@ cloud.google.com/go/storage v1.6.0/go.mod h1:N7U0C8pVQ/+NIKOBQyamJIeKQKkZ+mxpohl cloud.google.com/go/storage v1.8.0/go.mod h1:Wv1Oy7z6Yz3DshWRJFhqM/UCfaWIRTdp0RXyy7KQOVs= cloud.google.com/go/storage v1.10.0/go.mod h1:FLPqc6j+Ki4BU591ie1oL6qBQGu2Bl/tZ9ullr3+Kg0= dmitri.shuralyov.com/gpu/mtl v0.0.0-20190408044501-666a987793e9/go.mod h1:H6x//7gZCb22OMCxBHrMx7a5I7Hp++hsVxbQ4BYO7hU= -filippo.io/edwards25519 v1.0.0-beta.2 h1:/BZRNzm8N4K4eWfK28dL4yescorxtO7YG1yun8fy+pI= -filippo.io/edwards25519 v1.0.0-beta.2/go.mod h1:X+pm78QAUPtFLi1z9PYIlS/bdDnvbCOGKtZ+ACWEf7o= github.com/Azure/go-ansiterm v0.0.0-20170929234023-d6e3b3328b78 h1:w+iIsaOQNcT7OZ575w+acHgRric5iCyQh+xv+KJ4HB8= github.com/Azure/go-ansiterm v0.0.0-20170929234023-d6e3b3328b78/go.mod h1:LmzpDX56iTiv29bbRTIsUNlaFfuhWRQBWjQdVyAevI8= github.com/BurntSushi/toml v0.3.1 h1:WXkYYl6Yr3qBf1K79EBnL4mak0OimBfB0XUf9Vl28OQ= github.com/BurntSushi/toml v0.3.1/go.mod h1:xHWCNGjB5oqiDr8zfno3MHue2Ht5sIBksp03qcyfWMU= github.com/BurntSushi/xgb v0.0.0-20160522181843-27f122750802/go.mod h1:IVnqGOEym/WlBOVXweHU+Q+/VP0lqqI8lqeDx9IjBqo= -github.com/ChainSafe/go-schnorrkel v0.0.0-20210222182958-bd440c890782 h1:lwmjzta2Xu+3rPVY/VeNQj2xfNkyih4CwyRxYg3cpRQ= -github.com/ChainSafe/go-schnorrkel v0.0.0-20210222182958-bd440c890782/go.mod h1:URdX5+vg25ts3aCh8H5IFZybJYKWhJHYMTnf+ULtoC4= github.com/DataDog/zstd v1.4.1 h1:3oxKN3wbHibqx897utPC2LTQU4J+IHWWJO+glkAkpFM= github.com/DataDog/zstd v1.4.1/go.mod h1:1jcaCB/ufaK+sKp1NBhlGmpz41jOoPQ35bpF36t7BBo= github.com/Knetic/govaluate v3.0.1-0.20171022003610-9aa49832a739+incompatible/go.mod h1:r7JcOSlj0wfOMncg0iLm8Leh48TZaKVeNIfJntJ2wa0= @@ -135,8 +131,6 @@ github.com/coreos/go-systemd v0.0.0-20190321100706-95778dfbb74e/go.mod h1:F5haX7 github.com/coreos/go-systemd/v22 v22.3.2/go.mod h1:Y58oyj3AT4RCenI/lSvhwexgC+NSVTIJ3seZv2GcEnc= github.com/coreos/pkg v0.0.0-20160727233714-3ac0863d7acf/go.mod h1:E3G3o1h8I7cfcXa63jLwjI0eiQQMgzzUDFVpN/nH/eA= github.com/coreos/pkg v0.0.0-20180928190104-399ea9e2e55f/go.mod h1:E3G3o1h8I7cfcXa63jLwjI0eiQQMgzzUDFVpN/nH/eA= -github.com/cosmos/go-bip39 v0.0.0-20180819234021-555e2067c45d h1:49RLWk1j44Xu4fjHb6JFYmeUnDORVwHNkDxaQ0ctCVU= -github.com/cosmos/go-bip39 v0.0.0-20180819234021-555e2067c45d/go.mod h1:tSxLoYXyBmiFeKpvmq4dzayMdCjCnu8uqmCysIGBT2Y= github.com/cpuguy83/go-md2man v1.0.10/go.mod h1:SmD6nW6nTyfqj6ABTjUi3V3JVMnlJmwcJI5acqYI6dE= github.com/cpuguy83/go-md2man/v2 v2.0.0-20190314233015-f79a8a8ca69d/go.mod h1:maD7wRr/U5Z6m/iR4s+kqSMx2CaBsrgA7czyZG/E6dU= github.com/cpuguy83/go-md2man/v2 v2.0.0/go.mod h1:maD7wRr/U5Z6m/iR4s+kqSMx2CaBsrgA7czyZG/E6dU= @@ -301,11 +295,6 @@ github.com/grpc-ecosystem/go-grpc-prometheus v1.2.0/go.mod h1:8NvIoxWQoOIhqOTXgf github.com/grpc-ecosystem/grpc-gateway v1.9.0/go.mod h1:vNeuVxBJEsws4ogUvrchl83t/GYV9WGTSLVdBhOQFDY= github.com/grpc-ecosystem/grpc-gateway v1.9.5/go.mod h1:vNeuVxBJEsws4ogUvrchl83t/GYV9WGTSLVdBhOQFDY= github.com/grpc-ecosystem/grpc-gateway v1.16.0/go.mod h1:BDjrQk3hbvj6Nolgz8mAMFbcEtjT1g+wF4CSlocrBnw= -github.com/gtank/merlin v0.1.1-0.20191105220539-8318aed1a79f/go.mod h1:T86dnYJhcGOh5BjZFCJWTDeTK7XW8uE+E21Cy/bIQ+s= -github.com/gtank/merlin v0.1.1 h1:eQ90iG7K9pOhtereWsmyRJ6RAwcP4tHTDBHXNg+u5is= -github.com/gtank/merlin v0.1.1/go.mod h1:T86dnYJhcGOh5BjZFCJWTDeTK7XW8uE+E21Cy/bIQ+s= -github.com/gtank/ristretto255 v0.1.2 h1:JEqUCPA1NvLq5DwYtuzigd7ss8fwbYay9fi4/5uMzcc= -github.com/gtank/ristretto255 v0.1.2/go.mod h1:Ph5OpO6c7xKUGROZfWVLiJf9icMDwUeIvY4OmlYW69o= github.com/hashicorp/consul/api v1.1.0/go.mod h1:VmuI/Lkw1nC05EYQWNKwWGbkg+FbDBtguAZLlVdkD9Q= github.com/hashicorp/consul/api v1.3.0/go.mod h1:MmDNSzIMUjNpY/mQ398R4bk2FnqQLoPndWW5VkKPlCE= github.com/hashicorp/consul/sdk v0.1.1/go.mod h1:VKf9jXwCTEY1QZP2MOLRhb5i/I/ssyNV1vwHyQBF0x8= @@ -330,8 +319,6 @@ github.com/hashicorp/logutils v1.0.0/go.mod h1:QIAnNjmIWmVIIkWDTG1z5v++HQmx9WQRO github.com/hashicorp/mdns v1.0.0/go.mod h1:tL+uN++7HEJ6SQLQ2/p+z2pH24WQKWjBPkE0mNTz8vQ= github.com/hashicorp/memberlist v0.1.3/go.mod h1:ajVTdAv/9Im8oMAAj5G31PhhMCZJV2pPBoIllUwCN7I= github.com/hashicorp/serf v0.8.2/go.mod h1:6hOLApaqBFA1NXqRQAsxw9QxuDEvNxSQRwA/JwenrHc= -github.com/hdevalence/ed25519consensus v0.0.0-20210204194344-59a8610d2b87 h1:uUjLpLt6bVvZ72SQc/B4dXcPBw4Vgd7soowdRl52qEM= -github.com/hdevalence/ed25519consensus v0.0.0-20210204194344-59a8610d2b87/go.mod h1:XGsKKeXxeRr95aEOgipvluMPlgjr7dGlk9ZTWOjcUcg= github.com/hpcloud/tail v1.0.0/go.mod h1:ab1qPbhIpdTxEkNHXyeSf5vhxWSCs/tWer42PpOxQnU= github.com/hudl/fargo v1.3.0/go.mod h1:y3CKSmjA+wD2gak7sUSXTAoopbhU08POFhmITJgmKTg= github.com/ianlancetaylor/demangle v0.0.0-20181102032728-5e5cf60278f6/go.mod h1:aSSvb/t6k1mPoxDqO4vJh6VOCGPwU4O0C2/Eqndh1Sc= @@ -394,8 +381,6 @@ github.com/mattn/go-runewidth v0.0.2/go.mod h1:LwmH8dsx7+W8Uxz3IHJYH5QSwggIsqBzp github.com/matttproud/golang_protobuf_extensions v1.0.1 h1:4hp9jkHxhMHkqkrB3Ix0jegS5sx/RkqARlsWZ6pIwiU= github.com/matttproud/golang_protobuf_extensions v1.0.1/go.mod h1:D8He9yQNgCq6Z5Ld7szi9bcBfOoFv/3dc6xSMkL2PC0= github.com/miekg/dns v1.0.14/go.mod h1:W1PPwlIAgtquWBMBEV9nkV9Cazfe8ScdGz/Lj7v3Nrg= -github.com/mimoo/StrobeGo v0.0.0-20181016162300-f8f6d4d2b643 h1:hLDRPB66XQT/8+wG9WsDpiCvZf1yKO7sz7scAjSlBa0= -github.com/mimoo/StrobeGo v0.0.0-20181016162300-f8f6d4d2b643/go.mod h1:43+3pMjjKimDBf5Kr4ZFNGbLql1zKkbImw+fZbw3geM= github.com/minio/highwayhash v1.0.2 h1:Aak5U0nElisjDCfPSG79Tgzkn2gl66NxOMspRrKnA/g= github.com/minio/highwayhash v1.0.2/go.mod h1:BQskDq+xkJ12lmlUUi7U0M5Swg3EWR+dLTk+kldvVxY= github.com/mitchellh/cli v1.0.0/go.mod h1:hNIlj7HEI86fIcpObd7a0FcrxTWetlwJDGcceTlRvqc= @@ -425,6 +410,8 @@ github.com/niemeyer/pretty v0.0.0-20200227124842-a10e7caefd8e h1:fD57ERR4JtEqsWb github.com/niemeyer/pretty v0.0.0-20200227124842-a10e7caefd8e/go.mod h1:zD1mROLANZcx1PVRCS0qkT7pwLkGfwJo4zjcN/Tysno= github.com/nxadm/tail v1.4.4 h1:DQuhQpB1tVlglWS2hLQ5OV6B5r8aGxSrPc5Qo6uTN78= github.com/nxadm/tail v1.4.4/go.mod h1:kenIhsEOeOJmVchQTgglprH7qJGnHDVpk1VPCcaMI8A= +github.com/oasisprotocol/curve25519-voi v0.0.0-20210609091139-0a56a4bca00b h1:MKwruh+HeCSKWphkxuzvRzU4QzDkg7yiPkDVV0cDFgI= +github.com/oasisprotocol/curve25519-voi v0.0.0-20210609091139-0a56a4bca00b/go.mod h1:TLJifjWF6eotcfzDjKZsDqWJ+73Uvj/N85MvVyrvynM= github.com/oklog/oklog v0.3.2/go.mod h1:FCV+B7mhrz4o+ueLpx+KqkyXRGMWOYEvfiXtdGtbWGs= github.com/oklog/run v1.0.0/go.mod h1:dlhp/R75TPv97u0XWUtDeV/lRKWPKSdTuV0TZvrmrQA= github.com/oklog/ulid v1.3.1/go.mod h1:CirwcVhetQ6Lv90oh/F+FBtV6XMibvdAFo93nm5qn4U= @@ -638,12 +625,11 @@ golang.org/x/crypto v0.0.0-20190605123033-f99c8df09eb5/go.mod h1:yigFU9vqHzYiE8U golang.org/x/crypto v0.0.0-20190701094942-4def268fd1a4/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI= golang.org/x/crypto v0.0.0-20190820162420-60c769a6c586/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI= golang.org/x/crypto v0.0.0-20191011191535-87dc89f01550/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI= -golang.org/x/crypto v0.0.0-20191206172530-e9b2fee46413/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto= golang.org/x/crypto v0.0.0-20200115085410-6d4e4cb37c7d/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto= golang.org/x/crypto v0.0.0-20200510223506-06a226fb4e37/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto= golang.org/x/crypto v0.0.0-20200622213623-75b288015ac9/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto= -golang.org/x/crypto v0.0.0-20201117144127-c1f2f97bffc9 h1:phUcVbl53swtrUN8kQEXFhUxPlIlWyBfKmidCu7P95o= -golang.org/x/crypto v0.0.0-20201117144127-c1f2f97bffc9/go.mod h1:jdWPYTVW3xRLrWPugEBEK3UY2ZEsg3UU495nc5E+M+I= +golang.org/x/crypto v0.0.0-20201221181555-eec23a3978ad h1:DN0cp81fZ3njFcrLCytUHRSUkqBjfTo4Tx9RJTWs0EY= +golang.org/x/crypto v0.0.0-20201221181555-eec23a3978ad/go.mod h1:jdWPYTVW3xRLrWPugEBEK3UY2ZEsg3UU495nc5E+M+I= golang.org/x/exp v0.0.0-20190121172915-509febef88a4/go.mod h1:CJ0aWSM057203Lf6IL+f9T1iT9GByDxfZKAQTCR3kQA= golang.org/x/exp v0.0.0-20190306152737-a1d7652674e8/go.mod h1:CJ0aWSM057203Lf6IL+f9T1iT9GByDxfZKAQTCR3kQA= golang.org/x/exp v0.0.0-20190510132918-efd6b22b2522/go.mod h1:ZjyILWgesfNpC6sMxTJOJm9Kp84zZh5NQWvqDGG3Qr8= diff --git a/internal/p2p/conn/evil_secret_connection_test.go b/internal/p2p/conn/evil_secret_connection_test.go index ff12270c0..6d8b7cbf7 100644 --- a/internal/p2p/conn/evil_secret_connection_test.go +++ b/internal/p2p/conn/evil_secret_connection_test.go @@ -7,7 +7,7 @@ import ( "testing" gogotypes "github.com/gogo/protobuf/types" - "github.com/gtank/merlin" + "github.com/oasisprotocol/curve25519-voi/primitives/merlin" "github.com/stretchr/testify/assert" "golang.org/x/crypto/chacha20poly1305" @@ -208,9 +208,7 @@ func (c *evilConn) signChallenge() []byte { const challengeSize = 32 var challenge [challengeSize]byte - challengeSlice := transcript.ExtractBytes(labelSecretConnectionMac, challengeSize) - - copy(challenge[:], challengeSlice[0:challengeSize]) + transcript.ExtractBytes(challenge[:], labelSecretConnectionMac) sendAead, err := chacha20poly1305.New(sendSecret[:]) if err != nil { diff --git a/internal/p2p/conn/secret_connection.go b/internal/p2p/conn/secret_connection.go index c1017db1a..2f0d269d6 100644 --- a/internal/p2p/conn/secret_connection.go +++ b/internal/p2p/conn/secret_connection.go @@ -14,8 +14,8 @@ import ( "time" gogotypes "github.com/gogo/protobuf/types" - "github.com/gtank/merlin" pool "github.com/libp2p/go-buffer-pool" + "github.com/oasisprotocol/curve25519-voi/primitives/merlin" "golang.org/x/crypto/chacha20poly1305" "golang.org/x/crypto/curve25519" "golang.org/x/crypto/hkdf" @@ -38,16 +38,16 @@ const ( aeadSizeOverhead = 16 // overhead of poly 1305 authentication tag aeadKeySize = chacha20poly1305.KeySize aeadNonceSize = chacha20poly1305.NonceSize + + labelEphemeralLowerPublicKey = "EPHEMERAL_LOWER_PUBLIC_KEY" + labelEphemeralUpperPublicKey = "EPHEMERAL_UPPER_PUBLIC_KEY" + labelDHSecret = "DH_SECRET" + labelSecretConnectionMac = "SECRET_CONNECTION_MAC" ) var ( ErrSmallOrderRemotePubKey = errors.New("detected low order point from remote peer") - labelEphemeralLowerPublicKey = []byte("EPHEMERAL_LOWER_PUBLIC_KEY") - labelEphemeralUpperPublicKey = []byte("EPHEMERAL_UPPER_PUBLIC_KEY") - labelDHSecret = []byte("DH_SECRET") - labelSecretConnectionMac = []byte("SECRET_CONNECTION_MAC") - secretConnKeyAndChallengeGen = []byte("TENDERMINT_SECRET_CONNECTION_KEY_AND_CHALLENGE_GEN") ) @@ -132,9 +132,7 @@ func MakeSecretConnection(conn io.ReadWriteCloser, locPrivKey crypto.PrivKey) (* const challengeSize = 32 var challenge [challengeSize]byte - challengeSlice := transcript.ExtractBytes(labelSecretConnectionMac, challengeSize) - - copy(challenge[:], challengeSlice[0:challengeSize]) + transcript.ExtractBytes(challenge[:], labelSecretConnectionMac) sendAead, err := chacha20poly1305.New(sendSecret[:]) if err != nil { diff --git a/privval/secret_connection.go b/privval/secret_connection.go index 99264de90..8847f91db 100644 --- a/privval/secret_connection.go +++ b/privval/secret_connection.go @@ -14,8 +14,8 @@ import ( "time" gogotypes "github.com/gogo/protobuf/types" - "github.com/gtank/merlin" pool "github.com/libp2p/go-buffer-pool" + "github.com/oasisprotocol/curve25519-voi/primitives/merlin" "golang.org/x/crypto/chacha20poly1305" "golang.org/x/crypto/curve25519" "golang.org/x/crypto/hkdf" @@ -42,16 +42,16 @@ const ( aeadSizeOverhead = 16 // overhead of poly 1305 authentication tag aeadKeySize = chacha20poly1305.KeySize aeadNonceSize = chacha20poly1305.NonceSize + + labelEphemeralLowerPublicKey = "EPHEMERAL_LOWER_PUBLIC_KEY" + labelEphemeralUpperPublicKey = "EPHEMERAL_UPPER_PUBLIC_KEY" + labelDHSecret = "DH_SECRET" + labelSecretConnectionMac = "SECRET_CONNECTION_MAC" ) var ( ErrSmallOrderRemotePubKey = errors.New("detected low order point from remote peer") - labelEphemeralLowerPublicKey = []byte("EPHEMERAL_LOWER_PUBLIC_KEY") - labelEphemeralUpperPublicKey = []byte("EPHEMERAL_UPPER_PUBLIC_KEY") - labelDHSecret = []byte("DH_SECRET") - labelSecretConnectionMac = []byte("SECRET_CONNECTION_MAC") - secretConnKeyAndChallengeGen = []byte("TENDERMINT_SECRET_CONNECTION_KEY_AND_CHALLENGE_GEN") ) @@ -136,9 +136,7 @@ func MakeSecretConnection(conn io.ReadWriteCloser, locPrivKey crypto.PrivKey) (* const challengeSize = 32 var challenge [challengeSize]byte - challengeSlice := transcript.ExtractBytes(labelSecretConnectionMac, challengeSize) - - copy(challenge[:], challengeSlice[0:challengeSize]) + transcript.ExtractBytes(challenge[:], labelSecretConnectionMac) sendAead, err := chacha20poly1305.New(sendSecret[:]) if err != nil { diff --git a/types/validation.go b/types/validation.go index ce91711a6..1bf0265db 100644 --- a/types/validation.go +++ b/types/validation.go @@ -9,6 +9,12 @@ import ( tmmath "github.com/tendermint/tendermint/libs/math" ) +const batchVerifyThreshold = 2 + +func shouldBatchVerify(vals *ValidatorSet, commit *Commit) bool { + return len(commit.Signatures) >= batchVerifyThreshold && batch.SupportsBatchVerifier(vals.GetProposer().PubKey) +} + // VerifyCommit verifies +2/3 of the set had signed the given commit. // // It checks all the signatures! While it's safe to exit as soon as we have @@ -18,7 +24,6 @@ import ( // with a bonus for including more than +2/3 of the signatures. func VerifyCommit(chainID string, vals *ValidatorSet, blockID BlockID, height int64, commit *Commit) error { - // run a basic validation of the arguments if err := verifyBasicValsAndCommit(vals, commit, height, blockID); err != nil { return err @@ -35,18 +40,14 @@ func VerifyCommit(chainID string, vals *ValidatorSet, blockID BlockID, count := func(c CommitSig) bool { return c.ForBlock() } // attempt to batch verify - cacheSignBytes, success, err := tryVerifyCommitBatch( - chainID, vals, commit, votingPowerNeeded, ignore, count, true, true) - if err != nil { - return err - } - if success { - return nil + if shouldBatchVerify(vals, commit) { + return verifyCommitBatch(chainID, vals, commit, + votingPowerNeeded, ignore, count, true, true) } // if verification failed or is not supported then fallback to single verification return verifyCommitSingle(chainID, vals, commit, votingPowerNeeded, - cacheSignBytes, ignore, count, true, true) + ignore, count, true, true) } // LIGHT CLIENT VERIFICATION METHODS @@ -57,7 +58,6 @@ func VerifyCommit(chainID string, vals *ValidatorSet, blockID BlockID, // signatures. func VerifyCommitLight(chainID string, vals *ValidatorSet, blockID BlockID, height int64, commit *Commit) error { - // run a basic validation of the arguments if err := verifyBasicValsAndCommit(vals, commit, height, blockID); err != nil { return err @@ -73,19 +73,14 @@ func VerifyCommitLight(chainID string, vals *ValidatorSet, blockID BlockID, count := func(c CommitSig) bool { return true } // attempt to batch verify - cacheSignBytes, success, err := tryVerifyCommitBatch( - chainID, vals, commit, votingPowerNeeded, ignore, count, false, true) - if err != nil { - return err - } - if success { - return nil + if shouldBatchVerify(vals, commit) { + return verifyCommitBatch(chainID, vals, commit, + votingPowerNeeded, ignore, count, false, true) } // if verification failed or is not supported then fallback to single verification return verifyCommitSingle(chainID, vals, commit, votingPowerNeeded, - cacheSignBytes, ignore, count, false, true) - + ignore, count, false, true) } // VerifyCommitLightTrusting verifies that trustLevel of the validator set signed @@ -124,18 +119,14 @@ func VerifyCommitLightTrusting(chainID string, vals *ValidatorSet, commit *Commi // attempt to batch verify commit. As the validator set doesn't necessarily // correspond with the validator set that signed the block we need to look // up by address rather than index. - cacheSignBytes, success, err := tryVerifyCommitBatch( - chainID, vals, commit, votingPowerNeeded, ignore, count, false, false) - if err != nil { - return err - } - if success { - return nil + if shouldBatchVerify(vals, commit) { + return verifyCommitBatch(chainID, vals, commit, + votingPowerNeeded, ignore, count, false, false) } // attempt with single verification return verifyCommitSingle(chainID, vals, commit, votingPowerNeeded, - cacheSignBytes, ignore, count, false, false) + ignore, count, false, false) } // ValidateHash returns an error if the hash is not empty, but its @@ -152,12 +143,13 @@ func ValidateHash(h []byte) error { // Batch verification -// tryVerifyCommitBatch attempts to batch verify. If it is not supported or -// verification fails it returns false. If there is an error in the signatures -// or the way that they are counted an error is returned. A cache of all the -// commits in byte form is returned in case it needs to be used again for single -// verification -func tryVerifyCommitBatch( +// verifyCommitBatch batch verifies commits. This routine is equivalent +// to verifyCommitSingle in behavior, just faster iff every signature in the +// batch is valid. +// +// Note: The caller is responsible for checking to see if this routine is +// usable via `shouldVerifyBatch(vals, commit)`. +func verifyCommitBatch( chainID string, vals *ValidatorSet, commit *Commit, @@ -166,21 +158,20 @@ func tryVerifyCommitBatch( countSig func(CommitSig) bool, countAllSignatures bool, lookUpByIndex bool, -) (map[string][]byte, bool, error) { +) error { var ( val *Validator valIdx int32 seenVals = make(map[int32]int, len(commit.Signatures)) + batchSigIdxs = make([]int, 0, len(commit.Signatures)) talliedVotingPower int64 = 0 - // we keep a cache of the signed bytes to make it quicker to verify - // individually if we need to - cacheSignBytes = make(map[string][]byte, len(commit.Signatures)) ) // attempt to create a batch verifier bv, ok := batch.CreateBatchVerifier(vals.GetProposer().PubKey) - // check if batch verification is supported - if !ok || len(commit.Signatures) < 2 { - return cacheSignBytes, false, nil + // re-check if batch verification is supported + if !ok || len(commit.Signatures) < batchVerifyThreshold { + // This should *NEVER* happen. + return fmt.Errorf("unsupported signature algorithm or insufficient signatures for batch verification") } for idx, commitSig := range commit.Signatures { @@ -206,20 +197,19 @@ func tryVerifyCommitBatch( // that the same validator doesn't commit twice if firstIndex, ok := seenVals[valIdx]; ok { secondIndex := idx - return cacheSignBytes, false, fmt.Errorf("double vote from %v (%d and %d)", val, firstIndex, secondIndex) + return fmt.Errorf("double vote from %v (%d and %d)", val, firstIndex, secondIndex) } seenVals[valIdx] = idx } // Validate signature. voteSignBytes := commit.VoteSignBytes(chainID, int32(idx)) - // cache the signBytes in case batch verification fails - cacheSignBytes[string(val.PubKey.Bytes())] = voteSignBytes // add the key, sig and message to the verifier if err := bv.Add(val.PubKey, voteSignBytes, commitSig.Signature); err != nil { - return cacheSignBytes, false, err + return err } + batchSigIdxs = append(batchSigIdxs, idx) // If this signature counts then add the voting power of the validator // to the tally @@ -237,18 +227,32 @@ func tryVerifyCommitBatch( // ensure that we have batched together enough signatures to exceed the // voting power needed else there is no need to even verify if got, needed := talliedVotingPower, votingPowerNeeded; got <= needed { - return cacheSignBytes, false, ErrNotEnoughVotingPowerSigned{Got: got, Needed: needed} + return ErrNotEnoughVotingPowerSigned{Got: got, Needed: needed} } - // attempt to verify the batch. If this fails, fall back to single - // verification - if bv.Verify() { + // attempt to verify the batch. + ok, validSigs := bv.Verify() + if ok { // success - return cacheSignBytes, true, nil + return nil + } + + // one or more of the signatures is invalid, find and return the first + // invalid signature. + for i, ok := range validSigs { + if !ok { + // go back from the batch index to the commit.Signatures index + idx := batchSigIdxs[i] + sig := commit.Signatures[idx] + return fmt.Errorf("wrong signature (#%d): %X", idx, sig) + } } - // verification failed - return cacheSignBytes, false, nil + // execution reaching here is a bug, and one of the following has + // happened: + // * non-zero tallied voting power, empty batch (impossible?) + // * bv.Verify() returned `false, []bool{true, ..., true}` (BUG) + return fmt.Errorf("BUG: batch verification failed with no invalid signatures") } // Single Verification @@ -263,7 +267,6 @@ func verifyCommitSingle( vals *ValidatorSet, commit *Commit, votingPowerNeeded int64, - cachedVals map[string][]byte, ignoreSig func(CommitSig) bool, countSig func(CommitSig) bool, countAllSignatures bool, @@ -303,12 +306,7 @@ func verifyCommitSingle( seenVals[valIdx] = idx } - // Check if we have the validator in the cache - if cachedVote, ok := cachedVals[string(val.PubKey.Bytes())]; !ok { - voteSignBytes = commit.VoteSignBytes(chainID, int32(idx)) - } else { - voteSignBytes = cachedVote - } + voteSignBytes = commit.VoteSignBytes(chainID, int32(idx)) if !val.PubKey.VerifySignature(voteSignBytes, commitSig.Signature) { return fmt.Errorf("wrong signature (#%d): %X", idx, commitSig.Signature)