diff --git a/consensus/msgs_test.go b/consensus/msgs_test.go index bc82992560b..f374bf1ed1c 100644 --- a/consensus/msgs_test.go +++ b/consensus/msgs_test.go @@ -351,10 +351,20 @@ func TestConsMsgsVectors(t *testing.T) { BlockID: bi, } vpb := v.ToProto() + + // with canonical vote extension & _NO_ non-rp extension v.Extension = []byte("extension") - // TODO bernd: extend test to cover v.NonRpExtension vextPb := v.ToProto() + // with non-rp extension & _NO_ canonical vote extension + v.Extension = []byte{} + v.NonRpExtension = []byte("0x01") + + // with canonical vote extension & non-rp extension + v.Extension = []byte("extension") + vextPbNonRp := v.ToProto() + vextAllPb := v.ToProto() + testCases := []struct { testName string cMsg proto.Message @@ -392,6 +402,18 @@ func TestConsMsgsVectors(t *testing.T) { {"Vote_with_ext", &cmtcons.Message{Sum: &cmtcons.Message_Vote{ Vote: &cmtcons.Vote{Vote: vextPb}}}, "327b0a790802100122480a206164645f6d6f72655f6578636c616d6174696f6e5f6d61726b735f636f64652d1224080112206164645f6d6f72655f6578636c616d6174696f6e5f6d61726b735f636f64652d2a0608c0b89fdc0532146164645f6d6f72655f6578636c616d6174696f6e38014a09657874656e73696f6e"}, + { + "Vote_with_nrp_ext", &cmtcons.Message{Sum: &cmtcons.Message_Vote{ + Vote: &cmtcons.Vote{Vote: vextPbNonRp}, + }}, + "3281010a7f0802100122480a206164645f6d6f72655f6578636c616d6174696f6e5f6d61726b735f636f64652d1224080112206164645f6d6f72655f6578636c616d6174696f6e5f6d61726b735f636f64652d2a0608c0b89fdc0532146164645f6d6f72655f6578636c616d6174696f6e38014a09657874656e73696f6e5a0430783031", + }, + { + "Vote_with_all_vote_ext", &cmtcons.Message{Sum: &cmtcons.Message_Vote{ + Vote: &cmtcons.Vote{Vote: vextAllPb}, + }}, + "3281010a7f0802100122480a206164645f6d6f72655f6578636c616d6174696f6e5f6d61726b735f636f64652d1224080112206164645f6d6f72655f6578636c616d6174696f6e5f6d61726b735f636f64652d2a0608c0b89fdc0532146164645f6d6f72655f6578636c616d6174696f6e38014a09657874656e73696f6e5a0430783031", + }, {"HasVote", &cmtcons.Message{Sum: &cmtcons.Message_HasVote{ HasVote: &cmtcons.HasVote{Height: 1, Round: 1, Type: cmtproto.PrevoteType, Index: 1}}}, "3a080801100118012001"}, diff --git a/privval/file_test.go b/privval/file_test.go index 99088ffe2f6..d02e0c1c6b9 100644 --- a/privval/file_test.go +++ b/privval/file_test.go @@ -363,6 +363,26 @@ func TestVoteExtensionsAreAlwaysSigned(t *testing.T) { assert.False(t, pubKey.VerifySignature(vesb1, vpb2.ExtensionSignature)) assert.True(t, pubKey.VerifySignature(venrpsb3, vpb2.NonRpExtensionSignature)) assert.False(t, pubKey.VerifySignature(venrpsb1, vpb2.NonRpExtensionSignature)) + + // Check signing vote with canonical vote extension and _NO_ non replay-protected extension + vote3 := vote1.Copy() + vote3.Extension = []byte("new extension") + vote3.NonRpExtension = nil + vpb3 := vote3.ToProto() + + err = privVal.SignVote("mychainid", vpb3) + require.NoError(t, err, "expected no error signing same vote without non-replay-protected extension") + + // We need to ensure that a valid new extension signature has been created + // that validates against the vote extension sign bytes with the new + // extension, and does not validate against the vote extension sign bytes + // with the old extension. + vesb4, venrpsb4 := types.VoteExtensionSignBytes("mychainid", vpb3) + assert.True(t, pubKey.VerifySignature(vesb4, vpb3.ExtensionSignature)) + assert.False(t, pubKey.VerifySignature(vesb1, vpb3.ExtensionSignature)) + assert.True(t, pubKey.VerifySignature(venrpsb4, vpb3.NonRpExtensionSignature)) + // Non-replay-protected signatures will not differ from vote 1 as content is the same (empty byte slice) + assert.True(t, pubKey.VerifySignature(venrpsb1, vpb3.NonRpExtensionSignature)) } func newVote(addr types.Address, idx int32, height int64, round int32, diff --git a/privval/msgs_test.go b/privval/msgs_test.go index f30fc917aa7..66c5c550e25 100644 --- a/privval/msgs_test.go +++ b/privval/msgs_test.go @@ -30,7 +30,7 @@ func exampleVote() *types.Vote { ValidatorAddress: crypto.AddressHash([]byte("validator_address")), ValidatorIndex: 56789, Extension: []byte("extension"), - // NonRpExtension: []byte("nrp-extension"), // TODO bernd: adapt related test and enable this + NonRpExtension: []byte{}, } } @@ -63,6 +63,9 @@ func TestPrivvalVectors(t *testing.T) { vote := exampleVote() votepb := vote.ToProto() + // Simple vote with additional non-RP vote extension + vote.NonRpExtension = []byte("non-rp-extension") + voteNonRpExt := vote.ToProto() // Generate a simple proposal proposal := exampleProposal() proposalpb := proposal.ToProto() @@ -83,6 +86,8 @@ func TestPrivvalVectors(t *testing.T) { {"Vote Request", &privproto.SignVoteRequest{Vote: votepb}, "1a81010a7f080210031802224a0a208b01023386c371778ecb6368573e539afc3cc860ec3a2f614e54fe5652f4fc80122608c0843d122072db3d959635dff1bb567bedaa70573392c5159666a3f8caf11e413aac52207a2a0608f49a8ded0532146af1f4111082efb388211bc72c55bcd61e9ac3d538d5bb034a09657874656e73696f6e"}, {"Vote Response", &privproto.SignedVoteResponse{Vote: *votepb, Error: nil}, "2281010a7f080210031802224a0a208b01023386c371778ecb6368573e539afc3cc860ec3a2f614e54fe5652f4fc80122608c0843d122072db3d959635dff1bb567bedaa70573392c5159666a3f8caf11e413aac52207a2a0608f49a8ded0532146af1f4111082efb388211bc72c55bcd61e9ac3d538d5bb034a09657874656e73696f6e"}, {"Vote Response with error", &privproto.SignedVoteResponse{Vote: cmtproto.Vote{}, Error: remoteError}, "22250a11220212002a0b088092b8c398feffffff0112100801120c697427732061206572726f72"}, + {"Vote with NRP-VE Request", &privproto.SignVoteRequest{Vote: voteNonRpExt}, "1a94010a9101080210031802224a0a208b01023386c371778ecb6368573e539afc3cc860ec3a2f614e54fe5652f4fc80122608c0843d122072db3d959635dff1bb567bedaa70573392c5159666a3f8caf11e413aac52207a2a0608f49a8ded0532146af1f4111082efb388211bc72c55bcd61e9ac3d538d5bb034a09657874656e73696f6e5a106e6f6e2d72702d657874656e73696f6e"}, + {"Vote with NRP-VE Response", &privproto.SignedVoteResponse{Vote: *voteNonRpExt, Error: nil}, "2294010a9101080210031802224a0a208b01023386c371778ecb6368573e539afc3cc860ec3a2f614e54fe5652f4fc80122608c0843d122072db3d959635dff1bb567bedaa70573392c5159666a3f8caf11e413aac52207a2a0608f49a8ded0532146af1f4111082efb388211bc72c55bcd61e9ac3d538d5bb034a09657874656e73696f6e5a106e6f6e2d72702d657874656e73696f6e"}, {"Proposal Request", &privproto.SignProposalRequest{Proposal: proposalpb}, "2a700a6e08011003180220022a4a0a208b01023386c371778ecb6368573e539afc3cc860ec3a2f614e54fe5652f4fc80122608c0843d122072db3d959635dff1bb567bedaa70573392c5159666a3f8caf11e413aac52207a320608f49a8ded053a10697427732061207369676e6174757265"}, {"Proposal Response", &privproto.SignedProposalResponse{Proposal: *proposalpb, Error: nil}, "32700a6e08011003180220022a4a0a208b01023386c371778ecb6368573e539afc3cc860ec3a2f614e54fe5652f4fc80122608c0843d122072db3d959635dff1bb567bedaa70573392c5159666a3f8caf11e413aac52207a320608f49a8ded053a10697427732061207369676e6174757265"}, {"Proposal Response with error", &privproto.SignedProposalResponse{Proposal: cmtproto.Proposal{}, Error: remoteError}, "32250a112a021200320b088092b8c398feffffff0112100801120c697427732061206572726f72"}, diff --git a/state/execution_test.go b/state/execution_test.go index 3e029476c76..db4dff403db 100644 --- a/state/execution_test.go +++ b/state/execution_test.go @@ -1101,7 +1101,7 @@ func TestCreateProposalAbsentVoteExtensions(t *testing.T) { blockID := types.BlockID{Hash: block.Hash(), PartSetHeader: bps.Header()} pa, _ := state.Validators.GetByIndex(0) lastCommit, _, _ := makeValidCommit(testCase.height-1, blockID, state.Validators, privVals) - stripSignatures(lastCommit) // TODO bernd: check with Sergio why we strip signatures here as it's checked in createProposalBlock + stripSignatures(lastCommit) // remove vote extensions and its signatures as required by test case if testCase.expectPanic { require.Panics(t, func() { blockExec.CreateProposalBlock(ctx, testCase.height, state, lastCommit, pa) //nolint:errcheck diff --git a/test/e2e/app/app.go b/test/e2e/app/app.go index 1b9ea944a55..44d888f3f02 100644 --- a/test/e2e/app/app.go +++ b/test/e2e/app/app.go @@ -498,7 +498,8 @@ func (app *Application) ExtendVote(_ context.Context, req *abci.RequestExtendVot } ext = ext[:extLen] - nonRpExt := fmt.Sprintf("%d|%x", req.Height, ext) // We include the height as a basic replay protection mechanism + // Replay protection mechanism consists of: (a) the randomness of the extension (nonce), and (b) including the height + nonRpExt := fmt.Sprintf("%d|%x", req.Height, ext) app.logger.Info("generated vote extension", "num", num, "ext", fmt.Sprintf("%x", ext), "height", appHeight, "nonRpExt", nonRpExt) return &abci.ResponseExtendVote{ VoteExtension: ext, @@ -821,7 +822,7 @@ func parseVoteExtensions(expHeight int64, ext, nonRpExt []byte) (int64, error) { height, ) } - xExt := fmt.Sprintf("%x", ext) + xExt := hex.EncodeToString(ext) if parts[1] != xExt { return 0, fmt.Errorf("non replay protected vote extension contains incorrect data (%s!=%s)", xExt, diff --git a/test/e2e/networks/ci.toml b/test/e2e/networks/ci.toml index ff968d10e71..8b191e62ab8 100644 --- a/test/e2e/networks/ci.toml +++ b/test/e2e/networks/ci.toml @@ -3,8 +3,8 @@ ipv6 = true initial_height = 1000 -vote_extensions_update_height = 1004 -vote_extensions_enable_height = 1007 +vote_extensions_update_height = -1 +vote_extensions_enable_height = 1000 evidence = 5 initial_state = { initial01 = "a", initial02 = "b", initial03 = "c" } prepare_proposal_delay = "100ms" diff --git a/test/e2e/networks/single.toml b/test/e2e/networks/single.toml index 662c3eb41fd..6bc1bb927d9 100644 --- a/test/e2e/networks/single.toml +++ b/test/e2e/networks/single.toml @@ -1,5 +1,4 @@ vote_extensions_update_height = -1 vote_extensions_enable_height = 1 -[node.validator1] -[node.validator2] +[node.validator] diff --git a/types/block.go b/types/block.go index 69e0d86ffc3..b2451589283 100644 --- a/types/block.go +++ b/types/block.go @@ -739,7 +739,7 @@ func NewExtendedCommitSigAbsent() ExtendedCommitSig { // 2. first 6 bytes of vote extension // 3. first 6 bytes of vote extension signature // 4. first 6 bytes of non-replay-protected vote extension -// 5. first 6 bytes of non-replay-protected vote extension signature +// 5. first 6 bytes of non-replay-protected vote extension signature. func (ecs ExtendedCommitSig) String() string { return fmt.Sprintf("ExtendedCommitSig{%s with %X %X %X %X}", ecs.CommitSig, diff --git a/types/vote.go b/types/vote.go index cdaa1739136..18020d799c8 100644 --- a/types/vote.go +++ b/types/vote.go @@ -70,7 +70,7 @@ type Vote struct { ValidatorAddress Address `json:"validator_address"` ValidatorIndex int32 `json:"validator_index"` Signature []byte `json:"signature"` - Extension []byte `json:"extension"` //TODO bernd "Find all refences here" + Extension []byte `json:"extension"` ExtensionSignature []byte `json:"extension_signature"` NonRpExtension []byte `json:"non_rp_extension"` NonRpExtensionSignature []byte `json:"non_rp_extension_signature"` @@ -369,15 +369,16 @@ func (vote *Vote) ValidateBasic() error { return fmt.Errorf("vote extension signature absent on vote with extension") } if len(vote.NonRpExtensionSignature) == 0 && len(vote.NonRpExtension) != 0 { - return fmt.Errorf("vote extension signature absent on vote with extension") + return errors.New("non replay protected vote extension signature absent on vote with non-rp extension") } // Vote extensions and non replay protected vote extensions must go together - // one is present iff the other is present + // If one _signature_ is present the other must be as well. + // Note that even if no vote extension information (replay/non-replay protected) was provided, + // the signature must be present. if (len(vote.NonRpExtensionSignature) == 0) != (len(vote.ExtensionSignature) == 0) { - return fmt.Errorf("vote extension and non replay protected vote extension must go together") + return errors.New("vote extension and non replay protected vote extension must go together") } - } return nil