From b9cbaf8f10da417e4939f7e1e9b2d10aa1120181 Mon Sep 17 00:00:00 2001 From: Ethan Buchman Date: Thu, 4 Jan 2018 13:52:41 -0500 Subject: [PATCH] priv-val: fix timestamp for signing things that only differ by timestamp --- node/node.go | 4 +- types/priv_validator.go | 120 +++++++++++++++++++++++++---------- types/priv_validator_test.go | 60 ++++++++++++++++++ 3 files changed, 148 insertions(+), 36 deletions(-) diff --git a/node/node.go b/node/node.go index f922d8321..fde8e1e0a 100644 --- a/node/node.go +++ b/node/node.go @@ -185,9 +185,9 @@ func NewNode(config *cfg.Config, // Log whether this node is a validator or an observer if state.Validators.HasAddress(privValidator.GetAddress()) { - consensusLogger.Info("This node is a validator") + consensusLogger.Info("This node is a validator", "addr", privValidator.GetAddress(), "pubKey", privValidator.GetPubKey()) } else { - consensusLogger.Info("This node is not a validator") + consensusLogger.Info("This node is not a validator", "addr", privValidator.GetAddress(), "pubKey", privValidator.GetPubKey()) } // Make MempoolReactor diff --git a/types/priv_validator.go b/types/priv_validator.go index 31c65eeb4..3577049e7 100644 --- a/types/priv_validator.go +++ b/types/priv_validator.go @@ -17,10 +17,10 @@ import ( // TODO: type ? const ( - stepNone = 0 // Used to distinguish the initial state - stepPropose = 1 - stepPrevote = 2 - stepPrecommit = 3 + stepNone int8 = 0 // Used to distinguish the initial state + stepPropose int8 = 1 + stepPrevote int8 = 2 + stepPrecommit int8 = 3 ) func voteToStep(vote *Vote) int8 { @@ -199,12 +199,9 @@ func (privVal *PrivValidatorFS) Reset() { func (privVal *PrivValidatorFS) SignVote(chainID string, vote *Vote) error { privVal.mtx.Lock() defer privVal.mtx.Unlock() - signature, err := privVal.signBytesHRS(vote.Height, vote.Round, voteToStep(vote), - SignBytes(chainID, vote), checkVotesOnlyDifferByTimestamp) - if err != nil { + if err := privVal.signVote(chainID, vote); err != nil { return errors.New(cmn.Fmt("Error signing vote: %v", err)) } - vote.Signature = signature return nil } @@ -213,12 +210,9 @@ func (privVal *PrivValidatorFS) SignVote(chainID string, vote *Vote) error { func (privVal *PrivValidatorFS) SignProposal(chainID string, proposal *Proposal) error { privVal.mtx.Lock() defer privVal.mtx.Unlock() - signature, err := privVal.signBytesHRS(proposal.Height, proposal.Round, stepPropose, - SignBytes(chainID, proposal), checkProposalsOnlyDifferByTimestamp) - if err != nil { + if err := privVal.signProposal(chainID, proposal); err != nil { return fmt.Errorf("Error signing proposal: %v", err) } - proposal.Signature = signature return nil } @@ -250,36 +244,82 @@ func (privVal *PrivValidatorFS) checkHRS(height int64, round int, step int8) (bo return false, nil } -// signBytesHRS signs the given signBytes if the height/round/step (HRS) are -// greater than the latest state. If the HRS are equal and the only thing changed is the timestamp, -// it returns the privValidator.LastSignature. Else it returns an error. -func (privVal *PrivValidatorFS) signBytesHRS(height int64, round int, step int8, - signBytes []byte, checkFn checkOnlyDifferByTimestamp) (crypto.Signature, error) { - sig := crypto.Signature{} +// signVote checks if the vote is good to sign and sets the vote signature. +// It may need to set the timestamp as well if the vote is otherwise the same as +// a previously signed vote (ie. we crashed after signing but before the vote hit the WAL). +func (privVal *PrivValidatorFS) signVote(chainID string, vote *Vote) error { + height, round, step := vote.Height, vote.Round, voteToStep(vote) + signBytes := SignBytes(chainID, vote) sameHRS, err := privVal.checkHRS(height, round, step) if err != nil { - return sig, err + return err } // We might crash before writing to the wal, - // causing us to try to re-sign for the same HRS + // causing us to try to re-sign for the same HRS. + // If signbytes are the same, use the last signature. + // If they only differ by timestamp, use last timestamp and signature + // Otherwise, return error if sameHRS { - // if they're the same or only differ by timestamp, - // return the LastSignature. Otherwise, error - if bytes.Equal(signBytes, privVal.LastSignBytes) || - checkFn(privVal.LastSignBytes, signBytes) { - return privVal.LastSignature, nil + if bytes.Equal(signBytes, privVal.LastSignBytes) { + vote.Signature = privVal.LastSignature + } else if timestamp, ok := checkVotesOnlyDifferByTimestamp(privVal.LastSignBytes, signBytes); ok { + vote.Timestamp = timestamp + vote.Signature = privVal.LastSignature + } else { + err = fmt.Errorf("Conflicting data") } - return sig, fmt.Errorf("Conflicting data") + return err } - sig, err = privVal.Sign(signBytes) + // It passed the checks. Sign the vote + sig, err := privVal.Sign(signBytes) if err != nil { - return sig, err + return err } privVal.saveSigned(height, round, step, signBytes, sig) - return sig, nil + vote.Signature = sig + return nil +} + +// signProposal checks if the proposal is good to sign and sets the proposal signature. +// It may need to set the timestamp as well if the proposal is otherwise the same as +// a previously signed proposal ie. we crashed after signing but before the proposal hit the WAL). +func (privVal *PrivValidatorFS) signProposal(chainID string, proposal *Proposal) error { + height, round, step := proposal.Height, proposal.Round, stepPropose + signBytes := SignBytes(chainID, proposal) + + sameHRS, err := privVal.checkHRS(height, round, step) + if err != nil { + return err + } + + // We might crash before writing to the wal, + // causing us to try to re-sign for the same HRS. + // If signbytes are the same, use the last signature. + // If they only differ by timestamp, use last timestamp and signature + // Otherwise, return error + if sameHRS { + if bytes.Equal(signBytes, privVal.LastSignBytes) { + proposal.Signature = privVal.LastSignature + } else if timestamp, ok := checkProposalsOnlyDifferByTimestamp(privVal.LastSignBytes, signBytes); ok { + proposal.Timestamp = timestamp + proposal.Signature = privVal.LastSignature + } else { + err = fmt.Errorf("Conflicting data") + } + return err + } + + // It passed the checks. Sign the proposal + sig, err := privVal.Sign(signBytes) + if err != nil { + return err + } + privVal.saveSigned(height, round, step, signBytes, sig) + proposal.Signature = sig + return nil } // Persist height/round/step and signature @@ -331,8 +371,9 @@ func (pvs PrivValidatorsByAddress) Swap(i, j int) { type checkOnlyDifferByTimestamp func([]byte, []byte) bool -// returns true if the only difference in the votes is their timestamp -func checkVotesOnlyDifferByTimestamp(lastSignBytes, newSignBytes []byte) bool { +// returns the timestamp from the lastSignBytes. +// returns true if the only difference in the votes is their timestamp. +func checkVotesOnlyDifferByTimestamp(lastSignBytes, newSignBytes []byte) (time.Time, bool) { var lastVote, newVote CanonicalJSONOnceVote if err := json.Unmarshal(lastSignBytes, &lastVote); err != nil { panic(fmt.Sprintf("LastSignBytes cannot be unmarshalled into vote: %v", err)) @@ -341,6 +382,11 @@ func checkVotesOnlyDifferByTimestamp(lastSignBytes, newSignBytes []byte) bool { panic(fmt.Sprintf("signBytes cannot be unmarshalled into vote: %v", err)) } + lastTime, err := time.Parse(timeFormat, lastVote.Vote.Timestamp) + if err != nil { + panic(err) + } + // set the times to the same value and check equality now := CanonicalTime(time.Now()) lastVote.Vote.Timestamp = now @@ -348,11 +394,12 @@ func checkVotesOnlyDifferByTimestamp(lastSignBytes, newSignBytes []byte) bool { lastVoteBytes, _ := json.Marshal(lastVote) newVoteBytes, _ := json.Marshal(newVote) - return bytes.Equal(newVoteBytes, lastVoteBytes) + return lastTime, bytes.Equal(newVoteBytes, lastVoteBytes) } +// returns the timestamp from the lastSignBytes. // returns true if the only difference in the proposals is their timestamp -func checkProposalsOnlyDifferByTimestamp(lastSignBytes, newSignBytes []byte) bool { +func checkProposalsOnlyDifferByTimestamp(lastSignBytes, newSignBytes []byte) (time.Time, bool) { var lastProposal, newProposal CanonicalJSONOnceProposal if err := json.Unmarshal(lastSignBytes, &lastProposal); err != nil { panic(fmt.Sprintf("LastSignBytes cannot be unmarshalled into proposal: %v", err)) @@ -361,6 +408,11 @@ func checkProposalsOnlyDifferByTimestamp(lastSignBytes, newSignBytes []byte) boo panic(fmt.Sprintf("signBytes cannot be unmarshalled into proposal: %v", err)) } + lastTime, err := time.Parse(timeFormat, lastProposal.Proposal.Timestamp) + if err != nil { + panic(err) + } + // set the times to the same value and check equality now := CanonicalTime(time.Now()) lastProposal.Proposal.Timestamp = now @@ -368,5 +420,5 @@ func checkProposalsOnlyDifferByTimestamp(lastSignBytes, newSignBytes []byte) boo lastProposalBytes, _ := json.Marshal(lastProposal) newProposalBytes, _ := json.Marshal(newProposal) - return bytes.Equal(newProposalBytes, lastProposalBytes) + return lastTime, bytes.Equal(newProposalBytes, lastProposalBytes) } diff --git a/types/priv_validator_test.go b/types/priv_validator_test.go index 2fefee606..dd0ebff71 100644 --- a/types/priv_validator_test.go +++ b/types/priv_validator_test.go @@ -173,6 +173,58 @@ func TestSignProposal(t *testing.T) { assert.Equal(sig, proposal.Signature) } +func TestDifferByTimestamp(t *testing.T) { + _, tempFilePath := cmn.Tempfile("priv_validator_") + privVal := GenPrivValidatorFS(tempFilePath) + + block1 := PartSetHeader{5, []byte{1, 2, 3}} + height, round := int64(10), 1 + chainID := "mychainid" + + // test proposal + { + proposal := newProposal(height, round, block1) + err := privVal.SignProposal(chainID, proposal) + assert.NoError(t, err, "expected no error signing proposal") + signBytes := SignBytes(chainID, proposal) + sig := proposal.Signature + timeStamp := clipToMS(proposal.Timestamp) + + // manipulate the timestamp. should get changed back + proposal.Timestamp = proposal.Timestamp.Add(time.Millisecond) + proposal.Signature = crypto.Signature{} + err = privVal.SignProposal("mychainid", proposal) + assert.NoError(t, err, "expected no error on signing same proposal") + + assert.Equal(t, timeStamp, proposal.Timestamp) + assert.Equal(t, signBytes, SignBytes(chainID, proposal)) + assert.Equal(t, sig, proposal.Signature) + } + + // test vote + { + voteType := VoteTypePrevote + blockID := BlockID{[]byte{1, 2, 3}, PartSetHeader{}} + vote := newVote(privVal.Address, 0, height, round, voteType, blockID) + err := privVal.SignVote("mychainid", vote) + assert.NoError(t, err, "expected no error signing vote") + + signBytes := SignBytes(chainID, vote) + sig := vote.Signature + timeStamp := clipToMS(vote.Timestamp) + + // manipulate the timestamp. should get changed back + vote.Timestamp = vote.Timestamp.Add(time.Millisecond) + vote.Signature = crypto.Signature{} + err = privVal.SignVote("mychainid", vote) + assert.NoError(t, err, "expected no error on signing same vote") + + assert.Equal(t, timeStamp, vote.Timestamp) + assert.Equal(t, signBytes, SignBytes(chainID, vote)) + assert.Equal(t, sig, vote.Signature) + } +} + func newVote(addr data.Bytes, idx int, height int64, round int, typ byte, blockID BlockID) *Vote { return &Vote{ ValidatorAddress: addr, @@ -190,5 +242,13 @@ func newProposal(height int64, round int, partsHeader PartSetHeader) *Proposal { Height: height, Round: round, BlockPartsHeader: partsHeader, + Timestamp: time.Now().UTC(), } } + +func clipToMS(t time.Time) time.Time { + nano := t.UnixNano() + million := int64(1000000) + nano = (nano / million) * million + return time.Unix(0, nano).UTC() +}