fix: backport NRP-VoteExtension changes

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"},

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,

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"},

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

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,

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,

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