whisper: message format changed

whisper/whisperv6/api.go

@@ -277,7 +277,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 !validateRandomData(params.KeySym, aesKeyLength) {
 			return false, ErrInvalidSymmetricKey
 		}
 	}
@@ -383,7 +383,7 @@ func (api *PublicWhisperAPI) Messages(ctx context.Context, crit Criteria) (*rpc.
 		if err != nil {
 			return nil, err
 		}
-		if !validateSymmetricKey(key) {
+		if !validateRandomData(key, aesKeyLength) {
 			return nil, ErrInvalidSymmetricKey
 		}
 		filter.KeySym = key
@@ -555,7 +555,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 !validateRandomData(keySym, aesKeyLength) {
 			return "", ErrInvalidSymmetricKey
 		}
 	}

whisper/whisperv6/doc.go

@@ -54,7 +54,7 @@ const (
 	TopicLength     = 4  // in bytes
 	signatureLength = 65 // in bytes
 	aesKeyLength    = 32 // in bytes
-	AESNonceLength  = 12 // in bytes; also returned by aesgcm.NonceSize()
+	aesNonceLength  = 12 // in bytes; for more info please see cipher.gcmStandardNonceSize & aesgcm.NonceSize()
 	keyIdSize       = 32 // in bytes
 	bloomFilterSize = 64 // in bytes
 

whisper/whisperv6/message.go

@@ -55,7 +55,7 @@ type sentMessage struct {
 }
 
 // ReceivedMessage represents a data packet to be received through the
-// Whisper protocol.
+// Whisper protocol and successfully decrypted.
 type ReceivedMessage struct {
 	Raw []byte
 
@@ -71,7 +71,7 @@ type ReceivedMessage struct {
 	Dst   *ecdsa.PublicKey // Message recipient (identity used to decode the message)
 	Topic TopicType
 
-	SymKeyHash   common.Hash // The Keccak256Hash of the key, associated with the Topic
+	SymKeyHash   common.Hash // The Keccak256Hash of the key
 	EnvelopeHash common.Hash // Message envelope hash to act as a unique id
 }
 
@@ -131,9 +131,6 @@ func (msg *sentMessage) appendPadding(params *MessageParams) error {
 	if params.Src != nil {
 		rawSize += signatureLength
 	}
-	if params.KeySym != nil {
-		rawSize += AESNonceLength
-	}
 	odd := rawSize % padSizeLimit
 	paddingSize := padSizeLimit - odd
 	pad := make([]byte, paddingSize)
@@ -141,7 +138,7 @@ func (msg *sentMessage) appendPadding(params *MessageParams) error {
 	if err != nil {
 		return err
 	}
-	if !validateSymmetricKey(pad) {
+	if !validateRandomData(pad, paddingSize) {
 		return errors.New("failed to generate random padding of size " + strconv.Itoa(paddingSize))
 	}
 	msg.Raw = append(msg.Raw, pad...)
@@ -183,8 +180,8 @@ func (msg *sentMessage) encryptAsymmetric(key *ecdsa.PublicKey) error {
 // encryptSymmetric encrypts a message with a topic key, using AES-GCM-256.
 // nonce size should be 12 bytes (see cipher.gcmStandardNonceSize).
 func (msg *sentMessage) encryptSymmetric(key []byte) (err error) {
-	if !validateSymmetricKey(key) {
+	if !validateRandomData(key, aesKeyLength) {
-		return errors.New("invalid key provided for symmetric encryption")
+		return errors.New("invalid key provided for symmetric encryption, size: " + strconv.Itoa(len(key)))
 	}
 	block, err := aes.NewCipher(key)
 	if err != nil {
@@ -194,7 +191,7 @@ func (msg *sentMessage) encryptSymmetric(key []byte) (err error) {
 	if err != nil {
 		return err
 	}
-	salt, err := generateSalt(aesgcm)
+	salt, err := generateSecureRandomData(aesNonceLength) // never use more than 2^32 random nonces with a given key
 	if err != nil {
 		return err
 	}
@@ -203,32 +200,35 @@ func (msg *sentMessage) encryptSymmetric(key []byte) (err error) {
 	return nil
 }
 
-func generateSalt(aesgcm cipher.AEAD) ([]byte, error) {
+// generateSecureRandomData generates random data where extra security is required.
-	// never use more than 2^32 random nonces with a given key
+// The purpose of this function is to prevent some bugs in software or in hardware
-	sz := aesgcm.NonceSize()
+// from delivering not-very-random data. This is especially useful for AES nonce,
-	x1 := make([]byte, sz)
+// where true randomness does not really matter, but it is very important to have
-	x2 := make([]byte, sz)
+// a unique nonce for every message.
-	salt := make([]byte, sz)
+func generateSecureRandomData(length int) ([]byte, error) {
+	x := make([]byte, length)
+	y := make([]byte, length)
+	res := make([]byte, length)
 
-	_, err := crand.Read(x1)
+	_, err := crand.Read(x)
 	if err != nil {
 		return nil, err
-	} else if !validateSymmetricKey(x1) {
+	} else if !validateRandomData(x, length) {
-		return nil, errors.New("crypto/rand failed to generate salt")
+		return nil, errors.New("crypto/rand failed to generate secure random data")
 	}
-	_, err = mrand.Read(x2)
+	_, err = mrand.Read(y)
 	if err != nil {
 		return nil, err
-	} else if !validateSymmetricKey(x2) {
+	} else if !validateRandomData(y, length) {
-		return nil, errors.New("math/rand failed to generate salt")
+		return nil, errors.New("math/rand failed to generate secure random data")
 	}
-	for i := 0; i < sz; i++ {
+	for i := 0; i < length; i++ {
-		salt[i] = x1[i] ^ x2[i]
+		res[i] = x[i] ^ y[i]
 	}
-	if !validateSymmetricKey(salt) {
+	if !validateRandomData(res, length) {
-		return nil, errors.New("failed to generate salt")
+		return nil, errors.New("failed to generate secure random data")
 	}
-	return salt, nil
+	return res, nil
 }
 
 // Wrap bundles the message into an Envelope to transmit over the network.
@@ -263,10 +263,10 @@ func (msg *sentMessage) Wrap(options *MessageParams) (envelope *Envelope, err er
 // nonce size should be 12 bytes (see cipher.gcmStandardNonceSize).
 func (msg *ReceivedMessage) decryptSymmetric(key []byte) error {
 	// symmetric messages are expected to contain the 12-byte nonce at the end of the payload
-	if len(msg.Raw) < AESNonceLength {
+	if len(msg.Raw) < aesNonceLength {
 		return errors.New("missing salt or invalid payload in symmetric message")
 	}
-	salt := msg.Raw[len(msg.Raw)-AESNonceLength:]
+	salt := msg.Raw[len(msg.Raw)-aesNonceLength:]
 
 	block, err := aes.NewCipher(key)
 	if err != nil {
@@ -276,11 +276,7 @@ func (msg *ReceivedMessage) decryptSymmetric(key []byte) error {
 	if err != nil {
 		return err
 	}
-	if len(salt) != aesgcm.NonceSize() {
+	decrypted, err := aesgcm.Open(nil, salt, msg.Raw[:len(msg.Raw)-aesNonceLength], nil)
-		log.Error("decrypting the message", "AES salt size", len(salt))
-		return errors.New("wrong AES salt size")
-	}
-	decrypted, err := aesgcm.Open(nil, salt, msg.Raw[:len(msg.Raw)-AESNonceLength], nil)
 	if err != nil {
 		return err
 	}

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"
 )
@@ -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, 1+1+len(params.Payload))
-
-	err := msg.appendPadding(params)
-
-	if err != nil {
-		t.Fatalf("Error appending padding to message %v", err)
-		return
-	}
-
-	if len(msg.Raw) != 512-AESNonceLength {
-		t.Errorf("Invalid size %d != 512", len(msg.Raw))
-	}
-}
-
 func TestPaddingAppendedToSymMessagesWithSignature(t *testing.T) {
 	params := &MessageParams{
 		Payload: make([]byte, 246),
@@ -456,7 +435,7 @@ 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, 1+1+len(params.Payload))
@@ -468,7 +447,24 @@ func TestPaddingAppendedToSymMessagesWithSignature(t *testing.T) {
 		return
 	}
 
-	if len(msg.Raw) != 512-AESNonceLength-signatureLength {
+	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 untill 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 = []byte("some arbitrary data here")
+var sharedKey []byte = hexutil.MustDecode("0x03ca634cae0d49acb401d8a4c6b6fe8c55b70d115bf400769cc1400f3258cd31")
 var sharedTopic TopicType = TopicType{0xF, 0x1, 0x2, 0}
 var expectedMessage []byte = []byte("per rectum ad astra")
 var masterBloomFilter []byte

whisper/whisperv6/whisper.go

@@ -19,7 +19,6 @@ package whisperv6
 import (
 	"bytes"
 	"crypto/ecdsa"
-	crand "crypto/rand"
 	"crypto/sha256"
 	"fmt"
 	"math"
@@ -442,11 +441,10 @@ func (w *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 (w *Whisper) GenerateSymKey() (string, error) {
-	key := make([]byte, aesKeyLength)
+	key, err := generateSecureRandomData(aesKeyLength)
-	_, err := crand.Read(key)
 	if err != nil {
 		return "", err
-	} else if !validateSymmetricKey(key) {
+	} else if !validateRandomData(key, aesKeyLength) {
 		return "", fmt.Errorf("error in GenerateSymKey: crypto/rand failed to generate random data")
 	}
 
@@ -983,9 +981,10 @@ func validatePrivateKey(k *ecdsa.PrivateKey) bool {
 	return ValidatePublicKey(&k.PublicKey)
 }
 
-// validateSymmetricKey returns false if the key contains all zeros
+// validateSymmetricKey returns false if the key contains all zeros,
-func validateSymmetricKey(k []byte) bool {
+// which is a simplest and the most common bug.
-	return len(k) > 0 && !containsOnlyZeros(k)
+func validateRandomData(k []byte, expectedSize int) bool {
+	return len(k) == expectedSize && !containsOnlyZeros(k)
 }
 
 // containsOnlyZeros checks if the data contain only zeros.
@@ -1019,12 +1018,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)
+	buf, err := generateSecureRandomData(keyIdSize)
-	_, err = crand.Read(buf)
 	if err != nil {
 		return "", err
 	}
-	if !validateSymmetricKey(buf) {
+	if !validateRandomData(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 !validateRandomData(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 {
+	if !validateRandomData(k2, aesKeyLength) {
-		t.Fatalf("k1 does not exist.")
+		t.Fatalf("key validation failed.")
-	}
-	if k2 == nil {
-		t.Fatalf("k2 does not exist.")
 	}
 	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) {