Browse Source

crypto: API modifications (#5236)

## Description

This PR aims to make the crypto.PubKey interface more intuitive. 

Changes: 

- `VerfiyBytes` -> `VerifySignature`

Before `Bytes()` was amino encoded, now since it is the byte representation should we get rid of it entirely?

EDIT: decided to keep `Bytes()` as it is useful if you are using the interface instead of the concrete key

Closes: #XXX
pull/5240/head
Marko 4 years ago
committed by GitHub
parent
commit
9e98c74e3c
No known key found for this signature in database GPG Key ID: 4AEE18F83AFDEB23
21 changed files with 44 additions and 41 deletions
  1. +1
    -0
      CHANGELOG_PENDING.md
  2. +3
    -1
      consensus/state.go
  3. +1
    -1
      crypto/crypto.go
  4. +1
    -1
      crypto/ed25519/ed25519.go
  5. +2
    -2
      crypto/ed25519/ed25519_test.go
  6. +1
    -1
      crypto/internal/benchmarking/bench.go
  7. +1
    -1
      crypto/secp256k1/secp256k1_cgo.go
  8. +1
    -1
      crypto/secp256k1/secp256k1_nocgo.go
  9. +2
    -2
      crypto/secp256k1/secp256k1_nocgo_test.go
  10. +2
    -2
      crypto/secp256k1/secp256k1_test.go
  11. +1
    -1
      crypto/sr25519/pubkey.go
  12. +3
    -3
      crypto/sr25519/sr25519_test.go
  13. +1
    -1
      p2p/conn/secret_connection.go
  14. +1
    -1
      rpc/client/evidence_test.go
  15. +2
    -2
      tools/tm-signer-harness/internal/test_harness.go
  16. +6
    -6
      types/evidence.go
  17. +3
    -3
      types/proposal_test.go
  18. +6
    -6
      types/protobuf_test.go
  19. +3
    -3
      types/validator_set.go
  20. +1
    -1
      types/vote.go
  21. +2
    -2
      types/vote_test.go

+ 1
- 0
CHANGELOG_PENDING.md View File

@ -15,6 +15,7 @@ Friendly reminder, we have a [bug bounty program](https://hackerone.com/tendermi
- [crypto] [\#5214] Change `GenPrivKeySecp256k1` to `GenPrivKeyFromSecret` to be consistent with other keys - [crypto] [\#5214] Change `GenPrivKeySecp256k1` to `GenPrivKeyFromSecret` to be consistent with other keys
- [state] [\#5191](https://github.com/tendermint/tendermint/pull/5191) Add `State.InitialHeight` field to record initial block height, must be `1` (not `0`) to start from 1 (@erikgrinaker) - [state] [\#5191](https://github.com/tendermint/tendermint/pull/5191) Add `State.InitialHeight` field to record initial block height, must be `1` (not `0`) to start from 1 (@erikgrinaker)
- [state] `LoadStateFromDBOrGenesisFile()` and `LoadStateFromDBOrGenesisDoc()` no longer saves the state in the database if not found, the genesis state is simply returned (@erikgrinaker) - [state] `LoadStateFromDBOrGenesisFile()` and `LoadStateFromDBOrGenesisDoc()` no longer saves the state in the database if not found, the genesis state is simply returned (@erikgrinaker)
- [crypto] \#5236 `VerifyBytes` is now `VerifySignature` on the `crypto.PubKey` interface.
### FEATURES: ### FEATURES:


+ 3
- 1
consensus/state.go View File

@ -1708,7 +1708,9 @@ func (cs *State) defaultSetProposal(proposal *types.Proposal) error {
p := proposal.ToProto() p := proposal.ToProto()
// Verify signature // Verify signature
if !cs.Validators.GetProposer().PubKey.VerifyBytes(types.ProposalSignBytes(cs.state.ChainID, p), proposal.Signature) {
if !cs.Validators.GetProposer().PubKey.VerifySignature(
types.ProposalSignBytes(cs.state.ChainID, p), proposal.Signature,
) {
return ErrInvalidProposalSignature return ErrInvalidProposalSignature
} }


+ 1
- 1
crypto/crypto.go View File

@ -22,7 +22,7 @@ func AddressHash(bz []byte) Address {
type PubKey interface { type PubKey interface {
Address() Address Address() Address
Bytes() []byte Bytes() []byte
VerifyBytes(msg []byte, sig []byte) bool
VerifySignature(msg []byte, sig []byte) bool
Equals(PubKey) bool Equals(PubKey) bool
Type() string Type() string
} }


+ 1
- 1
crypto/ed25519/ed25519.go View File

@ -145,7 +145,7 @@ func (pubKey PubKey) Bytes() []byte {
return []byte(pubKey) return []byte(pubKey)
} }
func (pubKey PubKey) VerifyBytes(msg []byte, sig []byte) bool {
func (pubKey PubKey) VerifySignature(msg []byte, sig []byte) bool {
// make sure we use the same algorithm to sign // make sure we use the same algorithm to sign
if len(sig) != SignatureSize { if len(sig) != SignatureSize {
return false return false


+ 2
- 2
crypto/ed25519/ed25519_test.go View File

@ -20,11 +20,11 @@ func TestSignAndValidateEd25519(t *testing.T) {
require.Nil(t, err) require.Nil(t, err)
// Test the signature // Test the signature
assert.True(t, pubKey.VerifyBytes(msg, sig))
assert.True(t, pubKey.VerifySignature(msg, sig))
// Mutate the signature, just one bit. // Mutate the signature, just one bit.
// TODO: Replace this with a much better fuzzer, tendermint/ed25519/issues/10 // TODO: Replace this with a much better fuzzer, tendermint/ed25519/issues/10
sig[7] ^= byte(0x01) sig[7] ^= byte(0x01)
assert.False(t, pubKey.VerifyBytes(msg, sig))
assert.False(t, pubKey.VerifySignature(msg, sig))
} }

+ 1
- 1
crypto/internal/benchmarking/bench.go View File

@ -57,7 +57,7 @@ func BenchmarkVerification(b *testing.B, priv crypto.PrivKey) {
} }
b.ResetTimer() b.ResetTimer()
for i := 0; i < b.N; i++ { for i := 0; i < b.N; i++ {
pub.VerifyBytes(message, signature)
pub.VerifySignature(message, signature)
} }
} }


+ 1
- 1
crypto/secp256k1/secp256k1_cgo.go View File

@ -18,6 +18,6 @@ func (privKey PrivKey) Sign(msg []byte) ([]byte, error) {
return rs, nil return rs, nil
} }
func (pubKey PubKey) VerifyBytes(msg []byte, sig []byte) bool {
func (pubKey PubKey) VerifySignature(msg []byte, sig []byte) bool {
return secp256k1.VerifySignature(pubKey[:], crypto.Sha256(msg), sig) return secp256k1.VerifySignature(pubKey[:], crypto.Sha256(msg), sig)
} }

+ 1
- 1
crypto/secp256k1/secp256k1_nocgo.go View File

@ -30,7 +30,7 @@ func (privKey PrivKey) Sign(msg []byte) ([]byte, error) {
// VerifyBytes verifies a signature of the form R || S. // VerifyBytes verifies a signature of the form R || S.
// It rejects signatures which are not in lower-S form. // It rejects signatures which are not in lower-S form.
func (pubKey PubKey) VerifyBytes(msg []byte, sigStr []byte) bool {
func (pubKey PubKey) VerifySignature(msg []byte, sigStr []byte) bool {
if len(sigStr) != 64 { if len(sigStr) != 64 {
return false return false
} }


+ 2
- 2
crypto/secp256k1/secp256k1_nocgo_test.go View File

@ -22,14 +22,14 @@ func TestSignatureVerificationAndRejectUpperS(t *testing.T) {
require.False(t, sig.S.Cmp(secp256k1halfN) > 0) require.False(t, sig.S.Cmp(secp256k1halfN) > 0)
pub := priv.PubKey() pub := priv.PubKey()
require.True(t, pub.VerifyBytes(msg, sigStr))
require.True(t, pub.VerifySignature(msg, sigStr))
// malleate: // malleate:
sig.S.Sub(secp256k1.S256().CurveParams.N, sig.S) sig.S.Sub(secp256k1.S256().CurveParams.N, sig.S)
require.True(t, sig.S.Cmp(secp256k1halfN) > 0) require.True(t, sig.S.Cmp(secp256k1halfN) > 0)
malSigStr := serializeSig(sig) malSigStr := serializeSig(sig)
require.False(t, pub.VerifyBytes(msg, malSigStr),
require.False(t, pub.VerifySignature(msg, malSigStr),
"VerifyBytes incorrect with malleated & invalid S. sig=%v, key=%v", "VerifyBytes incorrect with malleated & invalid S. sig=%v, key=%v",
sig, sig,
priv, priv,


+ 2
- 2
crypto/secp256k1/secp256k1_test.go View File

@ -55,12 +55,12 @@ func TestSignAndValidateSecp256k1(t *testing.T) {
sig, err := privKey.Sign(msg) sig, err := privKey.Sign(msg)
require.Nil(t, err) require.Nil(t, err)
assert.True(t, pubKey.VerifyBytes(msg, sig))
assert.True(t, pubKey.VerifySignature(msg, sig))
// Mutate the signature, just one bit. // Mutate the signature, just one bit.
sig[3] ^= byte(0x01) sig[3] ^= byte(0x01)
assert.False(t, pubKey.VerifyBytes(msg, sig))
assert.False(t, pubKey.VerifySignature(msg, sig))
} }
// This test is intended to justify the removal of calls to the underlying library // This test is intended to justify the removal of calls to the underlying library


+ 1
- 1
crypto/sr25519/pubkey.go View File

@ -31,7 +31,7 @@ func (pubKey PubKey) Bytes() []byte {
return []byte(pubKey) return []byte(pubKey)
} }
func (pubKey PubKey) VerifyBytes(msg []byte, sig []byte) bool {
func (pubKey PubKey) VerifySignature(msg []byte, sig []byte) bool {
// make sure we use the same algorithm to sign // make sure we use the same algorithm to sign
if len(sig) != SignatureSize { if len(sig) != SignatureSize {
return false return false


+ 3
- 3
crypto/sr25519/sr25519_test.go View File

@ -20,12 +20,12 @@ func TestSignAndValidateSr25519(t *testing.T) {
require.Nil(t, err) require.Nil(t, err)
// Test the signature // Test the signature
assert.True(t, pubKey.VerifyBytes(msg, sig))
assert.True(t, pubKey.VerifyBytes(msg, sig))
assert.True(t, pubKey.VerifySignature(msg, sig))
assert.True(t, pubKey.VerifySignature(msg, sig))
// Mutate the signature, just one bit. // Mutate the signature, just one bit.
// TODO: Replace this with a much better fuzzer, tendermint/ed25519/issues/10 // TODO: Replace this with a much better fuzzer, tendermint/ed25519/issues/10
sig[7] ^= byte(0x01) sig[7] ^= byte(0x01)
assert.False(t, pubKey.VerifyBytes(msg, sig))
assert.False(t, pubKey.VerifySignature(msg, sig))
} }

+ 1
- 1
p2p/conn/secret_connection.go View File

@ -170,7 +170,7 @@ func MakeSecretConnection(conn io.ReadWriteCloser, locPrivKey crypto.PrivKey) (*
if _, ok := remPubKey.(ed25519.PubKey); !ok { if _, ok := remPubKey.(ed25519.PubKey); !ok {
return nil, fmt.Errorf("expected ed25519 pubkey, got %T", remPubKey) return nil, fmt.Errorf("expected ed25519 pubkey, got %T", remPubKey)
} }
if !remPubKey.VerifyBytes(challenge[:], remSignature) {
if !remPubKey.VerifySignature(challenge[:], remSignature) {
return nil, errors.New("challenge verification failed") return nil, errors.New("challenge verification failed")
} }


+ 1
- 1
rpc/client/evidence_test.go View File

@ -145,7 +145,7 @@ func TestBroadcastEvidence_DuplicateVoteEvidence(t *testing.T) {
pk, err := cryptoenc.PubKeyFromProto(v.PubKey) pk, err := cryptoenc.PubKeyFromProto(v.PubKey)
require.NoError(t, err) require.NoError(t, err)
require.EqualValues(t, rawpub, pk.Bytes(), "Stored PubKey not equal with expected, value %v", string(qres.Value))
require.EqualValues(t, rawpub, pk, "Stored PubKey not equal with expected, value %v", string(qres.Value))
require.Equal(t, int64(9), v.Power, "Stored Power not equal with expected, value %v", string(qres.Value)) require.Equal(t, int64(9), v.Power, "Stored Power not equal with expected, value %v", string(qres.Value))
for _, fake := range fakes { for _, fake := range fakes {


+ 2
- 2
tools/tm-signer-harness/internal/test_harness.go View File

@ -247,7 +247,7 @@ func (th *TestHarness) TestSignProposal() error {
return err return err
} }
// now validate the signature on the proposal // now validate the signature on the proposal
if sck.VerifyBytes(propBytes, prop.Signature) {
if sck.VerifySignature(propBytes, prop.Signature) {
th.logger.Info("Successfully validated proposal signature") th.logger.Info("Successfully validated proposal signature")
} else { } else {
th.logger.Error("FAILED: Proposal signature validation failed") th.logger.Error("FAILED: Proposal signature validation failed")
@ -298,7 +298,7 @@ func (th *TestHarness) TestSignVote() error {
} }
// now validate the signature on the proposal // now validate the signature on the proposal
if sck.VerifyBytes(voteBytes, vote.Signature) {
if sck.VerifySignature(voteBytes, vote.Signature) {
th.logger.Info("Successfully validated vote signature", "type", voteType) th.logger.Info("Successfully validated vote signature", "type", voteType)
} else { } else {
th.logger.Error("FAILED: Vote signature validation failed", "type", voteType) th.logger.Error("FAILED: Vote signature validation failed", "type", voteType)


+ 6
- 6
types/evidence.go View File

@ -290,10 +290,10 @@ func (dve *DuplicateVoteEvidence) Verify(chainID string, pubKey crypto.PubKey) e
va := dve.VoteA.ToProto() va := dve.VoteA.ToProto()
vb := dve.VoteB.ToProto() vb := dve.VoteB.ToProto()
// Signatures must be valid // Signatures must be valid
if !pubKey.VerifyBytes(VoteSignBytes(chainID, va), dve.VoteA.Signature) {
if !pubKey.VerifySignature(VoteSignBytes(chainID, va), dve.VoteA.Signature) {
return fmt.Errorf("verifying VoteA: %w", ErrVoteInvalidSignature) return fmt.Errorf("verifying VoteA: %w", ErrVoteInvalidSignature)
} }
if !pubKey.VerifyBytes(VoteSignBytes(chainID, vb), dve.VoteB.Signature) {
if !pubKey.VerifySignature(VoteSignBytes(chainID, vb), dve.VoteB.Signature) {
return fmt.Errorf("verifying VoteB: %w", ErrVoteInvalidSignature) return fmt.Errorf("verifying VoteB: %w", ErrVoteInvalidSignature)
} }
@ -724,7 +724,7 @@ func (e *LunaticValidatorEvidence) Verify(chainID string, pubKey crypto.PubKey)
} }
v := e.Vote.ToProto() v := e.Vote.ToProto()
if !pubKey.VerifyBytes(VoteSignBytes(chainID, v), e.Vote.Signature) {
if !pubKey.VerifySignature(VoteSignBytes(chainID, v), e.Vote.Signature) {
return errors.New("invalid signature") return errors.New("invalid signature")
} }
@ -948,10 +948,10 @@ func (e *PotentialAmnesiaEvidence) Verify(chainID string, pubKey crypto.PubKey)
vb := e.VoteB.ToProto() vb := e.VoteB.ToProto()
// Signatures must be valid // Signatures must be valid
if !pubKey.VerifyBytes(VoteSignBytes(chainID, va), e.VoteA.Signature) {
if !pubKey.VerifySignature(VoteSignBytes(chainID, va), e.VoteA.Signature) {
return fmt.Errorf("verifying VoteA: %w", ErrVoteInvalidSignature) return fmt.Errorf("verifying VoteA: %w", ErrVoteInvalidSignature)
} }
if !pubKey.VerifyBytes(VoteSignBytes(chainID, vb), e.VoteB.Signature) {
if !pubKey.VerifySignature(VoteSignBytes(chainID, vb), e.VoteB.Signature) {
return fmt.Errorf("verifying VoteB: %w", ErrVoteInvalidSignature) return fmt.Errorf("verifying VoteB: %w", ErrVoteInvalidSignature)
} }
@ -1145,7 +1145,7 @@ func (e *ProofOfLockChange) ValidateVotes(valSet *ValidatorSet, chainID string)
if bytes.Equal(validator.Address, vote.ValidatorAddress) { if bytes.Equal(validator.Address, vote.ValidatorAddress) {
exists = true exists = true
v := vote.ToProto() v := vote.ToProto()
if !validator.PubKey.VerifyBytes(VoteSignBytes(chainID, v), vote.Signature) {
if !validator.PubKey.VerifySignature(VoteSignBytes(chainID, v), vote.Signature) {
return fmt.Errorf("cannot verify vote (from validator: %d) against signature: %v", return fmt.Errorf("cannot verify vote (from validator: %d) against signature: %v",
vote.ValidatorIndex, vote.Signature) vote.ValidatorIndex, vote.Signature)
} }


+ 3
- 3
types/proposal_test.go View File

@ -71,7 +71,7 @@ func TestProposalVerifySignature(t *testing.T) {
prop.Signature = p.Signature prop.Signature = p.Signature
// verify the same proposal // verify the same proposal
valid := pubKey.VerifyBytes(signBytes, prop.Signature)
valid := pubKey.VerifySignature(signBytes, prop.Signature)
require.True(t, valid) require.True(t, valid)
// serialize, deserialize and verify again.... // serialize, deserialize and verify again....
@ -90,7 +90,7 @@ func TestProposalVerifySignature(t *testing.T) {
// verify the transmitted proposal // verify the transmitted proposal
newSignBytes := ProposalSignBytes("test_chain_id", pb) newSignBytes := ProposalSignBytes("test_chain_id", pb)
require.Equal(t, string(signBytes), string(newSignBytes)) require.Equal(t, string(signBytes), string(newSignBytes))
valid = pubKey.VerifyBytes(newSignBytes, np.Signature)
valid = pubKey.VerifySignature(newSignBytes, np.Signature)
require.True(t, valid) require.True(t, valid)
} }
@ -118,7 +118,7 @@ func BenchmarkProposalVerifySignature(b *testing.B) {
require.NoError(b, err) require.NoError(b, err)
for i := 0; i < b.N; i++ { for i := 0; i < b.N; i++ {
pubKey.VerifyBytes(ProposalSignBytes("test_chain_id", pbp), testProposal.Signature)
pubKey.VerifySignature(ProposalSignBytes("test_chain_id", pbp), testProposal.Signature)
} }
} }


+ 6
- 6
types/protobuf_test.go View File

@ -84,12 +84,12 @@ func TestABCIEvidence(t *testing.T) {
type pubKeyEddie struct{} type pubKeyEddie struct{}
func (pubKeyEddie) Address() Address { return []byte{} }
func (pubKeyEddie) Bytes() []byte { return []byte{} }
func (pubKeyEddie) VerifyBytes(msg []byte, sig []byte) bool { return false }
func (pubKeyEddie) Equals(crypto.PubKey) bool { return false }
func (pubKeyEddie) String() string { return "" }
func (pubKeyEddie) Type() string { return "pubKeyEddie" }
func (pubKeyEddie) Address() Address { return []byte{} }
func (pubKeyEddie) Bytes() []byte { return []byte{} }
func (pubKeyEddie) VerifySignature(msg []byte, sig []byte) bool { return false }
func (pubKeyEddie) Equals(crypto.PubKey) bool { return false }
func (pubKeyEddie) String() string { return "" }
func (pubKeyEddie) Type() string { return "pubKeyEddie" }
func TestABCIValidatorFromPubKeyAndPower(t *testing.T) { func TestABCIValidatorFromPubKeyAndPower(t *testing.T) {
pubkey := ed25519.GenPrivKey().PubKey() pubkey := ed25519.GenPrivKey().PubKey()


+ 3
- 3
types/validator_set.go View File

@ -688,7 +688,7 @@ func (vals *ValidatorSet) VerifyCommit(chainID string, blockID BlockID,
// Validate signature. // Validate signature.
voteSignBytes := commit.VoteSignBytes(chainID, int32(idx)) voteSignBytes := commit.VoteSignBytes(chainID, int32(idx))
if !val.PubKey.VerifyBytes(voteSignBytes, commitSig.Signature) {
if !val.PubKey.VerifySignature(voteSignBytes, commitSig.Signature) {
return fmt.Errorf("wrong signature (#%d): %X", idx, commitSig.Signature) return fmt.Errorf("wrong signature (#%d): %X", idx, commitSig.Signature)
} }
// Good! // Good!
@ -746,7 +746,7 @@ func (vals *ValidatorSet) VerifyCommitLight(chainID string, blockID BlockID,
// Validate signature. // Validate signature.
voteSignBytes := commit.VoteSignBytes(chainID, int32(idx)) voteSignBytes := commit.VoteSignBytes(chainID, int32(idx))
if !val.PubKey.VerifyBytes(voteSignBytes, commitSig.Signature) {
if !val.PubKey.VerifySignature(voteSignBytes, commitSig.Signature) {
return fmt.Errorf("wrong signature (#%d): %X", idx, commitSig.Signature) return fmt.Errorf("wrong signature (#%d): %X", idx, commitSig.Signature)
} }
@ -807,7 +807,7 @@ func (vals *ValidatorSet) VerifyCommitLightTrusting(chainID string, commit *Comm
// Validate signature. // Validate signature.
voteSignBytes := commit.VoteSignBytes(chainID, int32(idx)) voteSignBytes := commit.VoteSignBytes(chainID, int32(idx))
if !val.PubKey.VerifyBytes(voteSignBytes, commitSig.Signature) {
if !val.PubKey.VerifySignature(voteSignBytes, commitSig.Signature) {
return fmt.Errorf("wrong signature (#%d): %X", idx, commitSig.Signature) return fmt.Errorf("wrong signature (#%d): %X", idx, commitSig.Signature)
} }


+ 1
- 1
types/vote.go View File

@ -149,7 +149,7 @@ func (vote *Vote) Verify(chainID string, pubKey crypto.PubKey) error {
return ErrVoteInvalidValidatorAddress return ErrVoteInvalidValidatorAddress
} }
v := vote.ToProto() v := vote.ToProto()
if !pubKey.VerifyBytes(VoteSignBytes(chainID, v), vote.Signature) {
if !pubKey.VerifySignature(VoteSignBytes(chainID, v), vote.Signature) {
return ErrVoteInvalidSignature return ErrVoteInvalidSignature
} }
return nil return nil


+ 2
- 2
types/vote_test.go View File

@ -161,7 +161,7 @@ func TestVoteVerifySignature(t *testing.T) {
require.NoError(t, err) require.NoError(t, err)
// verify the same vote // verify the same vote
valid := pubkey.VerifyBytes(VoteSignBytes("test_chain_id", v), v.Signature)
valid := pubkey.VerifySignature(VoteSignBytes("test_chain_id", v), v.Signature)
require.True(t, valid) require.True(t, valid)
// serialize, deserialize and verify again.... // serialize, deserialize and verify again....
@ -174,7 +174,7 @@ func TestVoteVerifySignature(t *testing.T) {
// verify the transmitted vote // verify the transmitted vote
newSignBytes := VoteSignBytes("test_chain_id", precommit) newSignBytes := VoteSignBytes("test_chain_id", precommit)
require.Equal(t, string(signBytes), string(newSignBytes)) require.Equal(t, string(signBytes), string(newSignBytes))
valid = pubkey.VerifyBytes(newSignBytes, precommit.Signature)
valid = pubkey.VerifySignature(newSignBytes, precommit.Signature)
require.True(t, valid) require.True(t, valid)
} }


Loading…
Cancel
Save