Check message address against signature

consensus/istanbul/core/testbackend_test.go

@@ -97,7 +97,6 @@ func (self *testSystemBackend) Broadcast(valSet istanbul.ValidatorSet, message [
 	return nil
 }
 func (self *testSystemBackend) Gossip(valSet istanbul.ValidatorSet, message []byte, msgCode uint64, ignoreCache bool) error {
-	testLogger.Warn("not sign any data")
 	return nil
 }
 
@@ -130,16 +129,15 @@ func (self *testSystemBackend) Verify(proposal istanbul.Proposal, src istanbul.V
 }
 
 func (self *testSystemBackend) Sign(data []byte) ([]byte, error) {
-	testLogger.Warn("not sign any data")
-	return data, nil
+	return self.Address().Bytes(), nil
 }
 
 func (self *testSystemBackend) CheckSignature([]byte, common.Address, []byte) error {
 	return nil
 }
 
 func (self *testSystemBackend) CheckValidatorSignature(data []byte, sig []byte) (common.Address, error) {
-	return common.Address{}, nil
+	return common.BytesToAddress(sig), nil
 }
 
 func (self *testSystemBackend) Hash(b interface{}) common.Hash {

consensus/istanbul/core/types_test.go

@@ -122,12 +122,15 @@ func testSubjectWithSignature(t *testing.T) {
 	}
 	expectedSig := []byte{0x01}
 
+	correctAddress := common.HexToAddress("0x1")
+	spooferAddress := common.HexToAddress("0x2")
+
 	subjectPayload, _ := Encode(s)
 	// 1. Encode test
 	m := &istanbul.Message{
 		Code:          istanbul.MsgPreprepare,
 		Msg:           subjectPayload,
-		Address:       common.HexToAddress("0x1234567890"),
+		Address:       correctAddress,
 		Signature:     expectedSig,
 		CommittedSeal: []byte{},
 	}
@@ -141,7 +144,7 @@ func testSubjectWithSignature(t *testing.T) {
 	// 2.1 Test normal validate func
 	decodedMsg := new(istanbul.Message)
 	err = decodedMsg.FromPayload(msgPayload, func(data []byte, sig []byte) (common.Address, error) {
-		return common.Address{}, nil
+		return correctAddress, nil
 	})
 	if err != nil {
 		t.Errorf("error mismatch: have %v, want nil", err)
@@ -170,6 +173,15 @@ func testSubjectWithSignature(t *testing.T) {
 	if err != istanbul.ErrUnauthorizedAddress {
 		t.Errorf("error mismatch: have %v, want %v", err, istanbul.ErrUnauthorizedAddress)
 	}
+
+	// 2.4 Test spoofing signature by another validator validate func
+	decodedMsg = new(istanbul.Message)
+	err = decodedMsg.FromPayload(msgPayload, func(data []byte, sig []byte) (common.Address, error) {
+		return spooferAddress, nil
+	})
+	if err != istanbul.ErrInvalidSigner {
+		t.Errorf("error mismatch: have %v, want ErrInvalidSigner", err)
+	}
 }
 
 func TestMessageEncodeDecode(t *testing.T) {

consensus/istanbul/errors.go

@@ -22,6 +22,8 @@ var (
 	// ErrUnauthorizedAddress is returned when given address cannot be found in
 	// current validator set.
 	ErrUnauthorizedAddress = errors.New("unauthorized address")
+	// ErrInvalidSigner is returned if a message's signature does not correspond to the address in msg.Address
+	ErrInvalidSigner = errors.New("signed by incorrect validator")
 	// ErrStoppedEngine is returned if the engine is stopped
 	ErrStoppedEngine = errors.New("stopped engine")
 	// ErrStartedEngine is returned if the engine is already started

consensus/istanbul/types.go

@@ -204,10 +204,15 @@ func (m *Message) FromPayload(b []byte, validateFn func([]byte, []byte) (common.
 			return err
 		}
 
-		_, err = validateFn(payload, m.Signature)
+		signed_val_addr, err := validateFn(payload, m.Signature)
+		if err != nil {
+			return err
+		}
+		if signed_val_addr != m.Address {
+			return ErrInvalidSigner
+		}
 	}
-	// Still return the message even the err is not nil
-	return err
+	return nil
 }
 
 func (m *Message) Payload() ([]byte, error) {