whisper: change the whisper message format so as to add the payload size (#15870)

* whisper: message format changed

* whisper: tests fixed

* whisper: style fixes

* whisper: fixed names, fixed failing tests

* whisper: fix merge issue in #15870

Occured while using the github online merge tool. Lesson learned.

* whisper: fix a gofmt error for #15870

whisper/whisperv6/api.go

@@ -278,7 +278,7 @@ func (api *PublicWhisperAPI) Post(ctx context.Context, req NewMessage) (bool, er
 		if params.KeySym, err = api.w.GetSymKey(req.SymKeyID); err != nil {
 			return false, err
 		}
-		if !validateSymmetricKey(params.KeySym) {
+		if !validateDataIntegrity(params.KeySym, aesKeyLength) {
 			return false, ErrInvalidSymmetricKey
 		}
 	}
@@ -384,7 +384,7 @@ func (api *PublicWhisperAPI) Messages(ctx context.Context, crit Criteria) (*rpc.
 		if err != nil {
 			return nil, err
 		}
-		if !validateSymmetricKey(key) {
+		if !validateDataIntegrity(key, aesKeyLength) {
 			return nil, ErrInvalidSymmetricKey
 		}
 		filter.KeySym = key
@@ -556,7 +556,7 @@ func (api *PublicWhisperAPI) NewMessageFilter(req Criteria) (string, error) {
 		if keySym, err = api.w.GetSymKey(req.SymKeyID); err != nil {
 			return "", err
 		}
-		if !validateSymmetricKey(keySym) {
+		if !validateDataIntegrity(keySym, aesKeyLength) {
 			return "", ErrInvalidSymmetricKey
 		}
 	}

whisper/whisperv6/doc.go

@@ -52,15 +52,16 @@ const (
 	p2pMessageCode       = 127 // peer-to-peer message (to be consumed by the peer, but not forwarded any further)
 	NumberOfMessageCodes = 128
 
-	paddingMask   = byte(3)
+	SizeMask      = byte(3) // mask used to extract the size of payload size field from the flags
 	signatureFlag = byte(4)
 
 	TopicLength     = 4  // in bytes
 	signatureLength = 65 // in bytes
 	aesKeyLength    = 32 // in bytes
-	AESNonceLength  = 12 // in bytes
+	aesNonceLength  = 12 // in bytes; for more info please see cipher.gcmStandardNonceSize & aesgcm.NonceSize()
 	keyIDSize       = 32 // in bytes
 	bloomFilterSize = 64 // in bytes
+	flagsLength     = 1
 
 	EnvelopeHeaderLength = 20
 
@@ -68,7 +69,7 @@ const (
 	DefaultMaxMessageSize = uint32(1024 * 1024)
 	DefaultMinimumPoW     = 0.2
 
-	padSizeLimit      = 256 // just an arbitrary number, could be changed without breaking the protocol (must not exceed 2^24)
+	padSizeLimit      = 256 // just an arbitrary number, could be changed without breaking the protocol
 	messageQueueLimit = 1024
 
 	expirationCycle   = time.Second

whisper/whisperv6/envelope.go

@@ -200,8 +200,7 @@ func (e *Envelope) OpenSymmetric(key []byte) (msg *ReceivedMessage, err error) {
 
 // Open tries to decrypt an envelope, and populates the message fields in case of success.
 func (e *Envelope) Open(watcher *Filter) (msg *ReceivedMessage) {
-	// The API interface forbids filters doing both symmetric and
-	// asymmetric encryption.
+	// The API interface forbids filters doing both symmetric and asymmetric encryption.
 	if watcher.expectsAsymmetricEncryption() && watcher.expectsSymmetricEncryption() {
 		return nil
 	}
@@ -219,7 +218,7 @@ func (e *Envelope) Open(watcher *Filter) (msg *ReceivedMessage) {
 	}
 
 	if msg != nil {
-		ok := msg.Validate()
+		ok := msg.ValidateAndParse()
 		if !ok {
 			return nil
 		}

whisper/whisperv6/message_test.go

@@ -18,9 +18,12 @@ package whisperv6
 
 import (
 	"bytes"
+	"crypto/aes"
+	"crypto/cipher"
 	mrand "math/rand"
 	"testing"
 
+	"github.com/ethereum/go-ethereum/common/hexutil"
 	"github.com/ethereum/go-ethereum/crypto"
 	"github.com/ethereum/go-ethereum/rlp"
 )
@@ -90,8 +93,8 @@ func singleMessageTest(t *testing.T, symmetric bool) {
 		t.Fatalf("failed to encrypt with seed %d: %s.", seed, err)
 	}
 
-	if !decrypted.Validate() {
-		t.Fatalf("failed to validate with seed %d.", seed)
+	if !decrypted.ValidateAndParse() {
+		t.Fatalf("failed to validate with seed %d, symmetric = %v.", seed, symmetric)
 	}
 
 	if !bytes.Equal(text, decrypted.Payload) {
@@ -206,7 +209,7 @@ func TestEnvelopeOpen(t *testing.T) {
 	InitSingleTest()
 
 	var symmetric bool
-	for i := 0; i < 256; i++ {
+	for i := 0; i < 32; i++ {
 		singleEnvelopeOpenTest(t, symmetric)
 		symmetric = !symmetric
 	}
@@ -417,30 +420,6 @@ func TestPadding(t *testing.T) {
 	}
 }
 
-func TestPaddingAppendedToSymMessages(t *testing.T) {
-	params := &MessageParams{
-		Payload: make([]byte, 246),
-		KeySym:  make([]byte, aesKeyLength),
-	}
-
-	// Simulate a message with a payload just under 256 so that
-	// payload + flag + aesnonce > 256. Check that the result
-	// is padded on the next 256 boundary.
-	msg := sentMessage{}
-	msg.Raw = make([]byte, len(params.Payload)+1+AESNonceLength)
-
-	err := msg.appendPadding(params)
-
-	if err != nil {
-		t.Fatalf("Error appending padding to message %v", err)
-		return
-	}
-
-	if len(msg.Raw) != 512 {
-		t.Errorf("Invalid size %d != 512", len(msg.Raw))
-	}
-}
-
 func TestPaddingAppendedToSymMessagesWithSignature(t *testing.T) {
 	params := &MessageParams{
 		Payload: make([]byte, 246),
@@ -456,10 +435,11 @@ func TestPaddingAppendedToSymMessagesWithSignature(t *testing.T) {
 	params.Src = pSrc
 
 	// Simulate a message with a payload just under 256 so that
-	// payload + flag + aesnonce > 256. Check that the result
+	// payload + flag + signature > 256. Check that the result
 	// is padded on the next 256 boundary.
 	msg := sentMessage{}
-	msg.Raw = make([]byte, len(params.Payload)+1+AESNonceLength+signatureLength)
+	const payloadSizeFieldMinSize = 1
+	msg.Raw = make([]byte, flagsLength+payloadSizeFieldMinSize+len(params.Payload))
 
 	err = msg.appendPadding(params)
 
@@ -468,7 +448,24 @@ func TestPaddingAppendedToSymMessagesWithSignature(t *testing.T) {
 		return
 	}
 
-	if len(msg.Raw) != 512 {
+	if len(msg.Raw) != 512-signatureLength {
 		t.Errorf("Invalid size %d != 512", len(msg.Raw))
 	}
 }
+
+func TestAesNonce(t *testing.T) {
+	key := hexutil.MustDecode("0x03ca634cae0d49acb401d8a4c6b6fe8c55b70d115bf400769cc1400f3258cd31")
+	block, err := aes.NewCipher(key)
+	if err != nil {
+		t.Fatalf("NewCipher failed: %s", err)
+	}
+	aesgcm, err := cipher.NewGCM(block)
+	if err != nil {
+		t.Fatalf("NewGCM failed: %s", err)
+	}
+	// This is the most important single test in this package.
+	// If it fails, whisper will not be working.
+	if aesgcm.NonceSize() != aesNonceLength {
+		t.Fatalf("Nonce size is wrong. This is a critical error. Apparently AES nonce size have changed in the new version of AES GCM package. Whisper will not be working until this problem is resolved.")
+	}
+}

whisper/whisperv6/peer_test.go

@@ -27,6 +27,7 @@ import (
 	"time"
 
 	"github.com/ethereum/go-ethereum/common"
+	"github.com/ethereum/go-ethereum/common/hexutil"
 	"github.com/ethereum/go-ethereum/crypto"
 	"github.com/ethereum/go-ethereum/p2p"
 	"github.com/ethereum/go-ethereum/p2p/discover"
@@ -85,7 +86,7 @@ type TestNode struct {
 
 var result TestData
 var nodes [NumNodes]*TestNode
-var sharedKey = []byte("some arbitrary data here")
+var sharedKey = hexutil.MustDecode("0x03ca634cae0d49acb401d8a4c6b6fe8c55b70d115bf400769cc1400f3258cd31")
 var sharedTopic = TopicType{0xF, 0x1, 0x2, 0}
 var expectedMessage = []byte("per rectum ad astra")
 var masterBloomFilter []byte
@@ -122,11 +123,6 @@ func TestSimulation(t *testing.T) {
 	// check if each node (except node #0) have received and decrypted exactly one message
 	checkPropagation(t, false)
 
-	for i := 1; i < NumNodes; i++ {
-		time.Sleep(20 * time.Millisecond)
-		sendMsg(t, true, i)
-	}
-
 	// check if corresponding protocol-level messages were correctly decoded
 	checkPowExchangeForNodeZero(t)
 	checkBloomFilterExchange(t)
@@ -389,20 +385,37 @@ func TestPeerBasic(t *testing.T) {
 }
 
 func checkPowExchangeForNodeZero(t *testing.T) {
+	const iterations = 200
+	for j := 0; j < iterations; j++ {
+		lastCycle := (j == iterations-1)
+		ok := checkPowExchangeForNodeZeroOnce(t, lastCycle)
+		if ok {
+			break
+		}
+		time.Sleep(50 * time.Millisecond)
+	}
+}
+
+func checkPowExchangeForNodeZeroOnce(t *testing.T, mustPass bool) bool {
 	cnt := 0
 	for i, node := range nodes {
 		for peer := range node.shh.peers {
 			if peer.peer.ID() == discover.PubkeyID(&nodes[0].id.PublicKey) {
 				cnt++
 				if peer.powRequirement != masterPow {
-					t.Fatalf("node %d: failed to set the new pow requirement.", i)
+					if mustPass {
+						t.Fatalf("node %d: failed to set the new pow requirement for node zero.", i)
+					} else {
+						return false
+					}
 				}
 			}
 		}
 	}
 	if cnt == 0 {
-		t.Fatalf("no matching peers found.")
+		t.Fatalf("looking for node zero: no matching peers found.")
 	}
+	return true
 }
 
 func checkPowExchange(t *testing.T) {
@@ -418,13 +431,31 @@ func checkPowExchange(t *testing.T) {
 	}
 }
 
-func checkBloomFilterExchange(t *testing.T) {
+func checkBloomFilterExchangeOnce(t *testing.T, mustPass bool) bool {
 	for i, node := range nodes {
 		for peer := range node.shh.peers {
 			if !bytes.Equal(peer.bloomFilter, masterBloomFilter) {
-				t.Fatalf("node %d: failed to exchange bloom filter requirement in round %d. \n%x expected \n%x got",
-					i, round, masterBloomFilter, peer.bloomFilter)
+				if mustPass {
+					t.Fatalf("node %d: failed to exchange bloom filter requirement in round %d. \n%x expected \n%x got",
+						i, round, masterBloomFilter, peer.bloomFilter)
+				} else {
+					return false
+				}
 			}
 		}
 	}
+
+	return true
+}
+
+func checkBloomFilterExchange(t *testing.T) {
+	const iterations = 200
+	for j := 0; j < iterations; j++ {
+		lastCycle := (j == iterations-1)
+		ok := checkBloomFilterExchangeOnce(t, lastCycle)
+		if ok {
+			break
+		}
+		time.Sleep(50 * time.Millisecond)
+	}
 }

whisper/whisperv6/whisper.go

@@ -19,7 +19,6 @@ package whisperv6
 import (
 	"bytes"
 	"crypto/ecdsa"
-	crand "crypto/rand"
 	"crypto/sha256"
 	"fmt"
 	"math"
@@ -444,11 +443,10 @@ func (whisper *Whisper) GetPrivateKey(id string) (*ecdsa.PrivateKey, error) {
 // GenerateSymKey generates a random symmetric key and stores it under id,
 // which is then returned. Will be used in the future for session key exchange.
 func (whisper *Whisper) GenerateSymKey() (string, error) {
-	key := make([]byte, aesKeyLength)
-	_, err := crand.Read(key)
+	key, err := generateSecureRandomData(aesKeyLength)
 	if err != nil {
 		return "", err
-	} else if !validateSymmetricKey(key) {
+	} else if !validateDataIntegrity(key, aesKeyLength) {
 		return "", fmt.Errorf("error in GenerateSymKey: crypto/rand failed to generate random data")
 	}
 
@@ -983,9 +981,16 @@ func validatePrivateKey(k *ecdsa.PrivateKey) bool {
 	return ValidatePublicKey(&k.PublicKey)
 }
 
-// validateSymmetricKey returns false if the key contains all zeros
-func validateSymmetricKey(k []byte) bool {
-	return len(k) > 0 && !containsOnlyZeros(k)
+// validateDataIntegrity returns false if the data have the wrong or contains all zeros,
+// which is the simplest and the most common bug.
+func validateDataIntegrity(k []byte, expectedSize int) bool {
+	if len(k) != expectedSize {
+		return false
+	}
+	if expectedSize > 3 && containsOnlyZeros(k) {
+		return false
+	}
+	return true
 }
 
 // containsOnlyZeros checks if the data contain only zeros.
@@ -1019,12 +1024,11 @@ func BytesToUintBigEndian(b []byte) (res uint64) {
 
 // GenerateRandomID generates a random string, which is then returned to be used as a key id
 func GenerateRandomID() (id string, err error) {
-	buf := make([]byte, keyIDSize)
-	_, err = crand.Read(buf)
+	buf, err := generateSecureRandomData(keyIDSize)
 	if err != nil {
 		return "", err
 	}
-	if !validateSymmetricKey(buf) {
+	if !validateDataIntegrity(buf, keyIDSize) {
 		return "", fmt.Errorf("error in generateRandomID: crypto/rand failed to generate random data")
 	}
 	id = common.Bytes2Hex(buf)

whisper/whisperv6/whisper_test.go

@@ -81,7 +81,7 @@ func TestWhisperBasic(t *testing.T) {
 	}
 
 	derived := pbkdf2.Key([]byte(peerID), nil, 65356, aesKeyLength, sha256.New)
-	if !validateSymmetricKey(derived) {
+	if !validateDataIntegrity(derived, aesKeyLength) {
 		t.Fatalf("failed validateSymmetricKey with param = %v.", derived)
 	}
 	if containsOnlyZeros(derived) {
@@ -448,24 +448,12 @@ func TestWhisperSymKeyManagement(t *testing.T) {
 	if !w.HasSymKey(id2) {
 		t.Fatalf("HasSymKey(id2) failed.")
 	}
-	if k1 == nil {
-		t.Fatalf("k1 does not exist.")
-	}
-	if k2 == nil {
-		t.Fatalf("k2 does not exist.")
+	if !validateDataIntegrity(k2, aesKeyLength) {
+		t.Fatalf("key validation failed.")
 	}
 	if !bytes.Equal(k1, k2) {
 		t.Fatalf("k1 != k2.")
 	}
-	if len(k1) != aesKeyLength {
-		t.Fatalf("wrong length of k1.")
-	}
-	if len(k2) != aesKeyLength {
-		t.Fatalf("wrong length of k2.")
-	}
-	if !validateSymmetricKey(k2) {
-		t.Fatalf("key validation failed.")
-	}
 }
 
 func TestExpiry(t *testing.T) {