diff --git a/whisper/whisperv6/api.go b/whisper/whisperv6/api.go index 6cbce26f06..8ae2882e1f 100644 --- a/whisper/whisperv6/api.go +++ b/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 } } diff --git a/whisper/whisperv6/doc.go b/whisper/whisperv6/doc.go index 699fd5c767..d5d7fed609 100644 --- a/whisper/whisperv6/doc.go +++ b/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 diff --git a/whisper/whisperv6/envelope.go b/whisper/whisperv6/envelope.go index 6bc1f2c6bc..881945e9a8 100644 --- a/whisper/whisperv6/envelope.go +++ b/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 } diff --git a/whisper/whisperv6/message.go b/whisper/whisperv6/message.go index b5c8279b1c..7def35f14f 100644 --- a/whisper/whisperv6/message.go +++ b/whisper/whisperv6/message.go @@ -25,6 +25,7 @@ import ( crand "crypto/rand" "encoding/binary" "errors" + mrand "math/rand" "strconv" "github.com/ethereum/go-ethereum/common" @@ -55,7 +56,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 +72,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 } @@ -89,81 +90,60 @@ func (msg *ReceivedMessage) isAsymmetricEncryption() bool { // NewSentMessage creates and initializes a non-signed, non-encrypted Whisper message. func newSentMessage(params *MessageParams) (*sentMessage, error) { + const payloadSizeFieldMaxSize = 4 msg := sentMessage{} - msg.Raw = make([]byte, 1, len(params.Payload)+len(params.Padding)+signatureLength+padSizeLimit) + msg.Raw = make([]byte, 1, + flagsLength+payloadSizeFieldMaxSize+len(params.Payload)+len(params.Padding)+signatureLength+padSizeLimit) msg.Raw[0] = 0 // set all the flags to zero - err := msg.appendPadding(params) - if err != nil { - return nil, err - } + msg.addPayloadSizeField(params.Payload) msg.Raw = append(msg.Raw, params.Payload...) - return &msg, nil + err := msg.appendPadding(params) + return &msg, err } -// getSizeOfLength returns the number of bytes necessary to encode the entire size padding (including these bytes) -func getSizeOfLength(b []byte) (sz int, err error) { - sz = intSize(len(b)) // first iteration - sz = intSize(len(b) + sz) // second iteration - if sz > 3 { - err = errors.New("oversized padding parameter") - } - return sz, err +// addPayloadSizeField appends the auxiliary field containing the size of payload +func (msg *sentMessage) addPayloadSizeField(payload []byte) { + fieldSize := getSizeOfPayloadSizeField(payload) + field := make([]byte, 4) + binary.LittleEndian.PutUint32(field, uint32(len(payload))) + field = field[:fieldSize] + msg.Raw = append(msg.Raw, field...) + msg.Raw[0] |= byte(fieldSize) } -// sizeOfIntSize returns minimal number of bytes necessary to encode an integer value -func intSize(i int) (s int) { - for s = 1; i >= 256; s++ { - i /= 256 +// getSizeOfPayloadSizeField returns the number of bytes necessary to encode the size of payload +func getSizeOfPayloadSizeField(payload []byte) int { + s := 1 + for i := len(payload); i >= 256; i /= 256 { + s++ } return s } -// appendPadding appends the pseudorandom padding bytes and sets the padding flag. -// The last byte contains the size of padding (thus, its size must not exceed 256). +// appendPadding appends the padding specified in params. +// If no padding is provided in params, then random padding is generated. func (msg *sentMessage) appendPadding(params *MessageParams) error { - rawSize := len(params.Payload) + 1 - if params.Src != nil { - rawSize += signatureLength + if len(params.Padding) != 0 { + // padding data was provided by the Dapp, just use it as is + msg.Raw = append(msg.Raw, params.Padding...) + return nil } - if params.KeySym != nil { - rawSize += AESNonceLength + rawSize := flagsLength + getSizeOfPayloadSizeField(params.Payload) + len(params.Payload) + if params.Src != nil { + rawSize += signatureLength } odd := rawSize % padSizeLimit - - if len(params.Padding) != 0 { - padSize := len(params.Padding) - padLengthSize, err := getSizeOfLength(params.Padding) - if err != nil { - return err - } - totalPadSize := padSize + padLengthSize - buf := make([]byte, 8) - binary.LittleEndian.PutUint32(buf, uint32(totalPadSize)) - buf = buf[:padLengthSize] - msg.Raw = append(msg.Raw, buf...) - msg.Raw = append(msg.Raw, params.Padding...) - msg.Raw[0] |= byte(padLengthSize) // number of bytes indicating the padding size - } else if odd != 0 { - totalPadSize := padSizeLimit - odd - if totalPadSize > 255 { - // this algorithm is only valid if padSizeLimit < 256. - // if padSizeLimit will ever change, please fix the algorithm - // (please see also ReceivedMessage.extractPadding() function). - panic("please fix the padding algorithm before releasing new version") - } - buf := make([]byte, totalPadSize) - _, err := crand.Read(buf[1:]) - if err != nil { - return err - } - if totalPadSize > 6 && !validateSymmetricKey(buf) { - return errors.New("failed to generate random padding of size " + strconv.Itoa(totalPadSize)) - } - buf[0] = byte(totalPadSize) - msg.Raw = append(msg.Raw, buf...) - msg.Raw[0] |= byte(0x1) // number of bytes indicating the padding size + paddingSize := padSizeLimit - odd + pad := make([]byte, paddingSize) + _, err := crand.Read(pad) + if err != nil { + return err } + if !validateDataIntegrity(pad, paddingSize) { + return errors.New("failed to generate random padding of size " + strconv.Itoa(paddingSize)) + } + msg.Raw = append(msg.Raw, pad...) return nil } @@ -176,11 +156,11 @@ func (msg *sentMessage) sign(key *ecdsa.PrivateKey) error { return nil } - msg.Raw[0] |= signatureFlag + msg.Raw[0] |= signatureFlag // it is important to set this flag before signing hash := crypto.Keccak256(msg.Raw) signature, err := crypto.Sign(hash, key) if err != nil { - msg.Raw[0] &= ^signatureFlag // clear the flag + msg.Raw[0] &= (0xFF ^ signatureFlag) // clear the flag return err } msg.Raw = append(msg.Raw, signature...) @@ -202,10 +182,9 @@ 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) { - return errors.New("invalid key provided for symmetric encryption") + if !validateDataIntegrity(key, aesKeyLength) { + return errors.New("invalid key provided for symmetric encryption, size: " + strconv.Itoa(len(key))) } - block, err := aes.NewCipher(key) if err != nil { return err @@ -214,20 +193,46 @@ func (msg *sentMessage) encryptSymmetric(key []byte) (err error) { if err != nil { return err } - - // never use more than 2^32 random nonces with a given key - salt := make([]byte, aesgcm.NonceSize()) - _, err = crand.Read(salt) + salt, err := generateSecureRandomData(aesNonceLength) // never use more than 2^32 random nonces with a given key if err != nil { return err - } else if !validateSymmetricKey(salt) { - return errors.New("crypto/rand failed to generate salt") } - - msg.Raw = append(aesgcm.Seal(nil, salt, msg.Raw, nil), salt...) + encrypted := aesgcm.Seal(nil, salt, msg.Raw, nil) + msg.Raw = append(encrypted, salt...) return nil } +// generateSecureRandomData generates random data where extra security is required. +// The purpose of this function is to prevent some bugs in software or in hardware +// from delivering not-very-random data. This is especially useful for AES nonce, +// where true randomness does not really matter, but it is very important to have +// a unique nonce for every message. +func generateSecureRandomData(length int) ([]byte, error) { + x := make([]byte, length) + y := make([]byte, length) + res := make([]byte, length) + + _, err := crand.Read(x) + if err != nil { + return nil, err + } else if !validateDataIntegrity(x, length) { + return nil, errors.New("crypto/rand failed to generate secure random data") + } + _, err = mrand.Read(y) + if err != nil { + return nil, err + } else if !validateDataIntegrity(y, length) { + return nil, errors.New("math/rand failed to generate secure random data") + } + for i := 0; i < length; i++ { + res[i] = x[i] ^ y[i] + } + if !validateDataIntegrity(res, length) { + return nil, errors.New("failed to generate secure random data") + } + return res, nil +} + // Wrap bundles the message into an Envelope to transmit over the network. func (msg *sentMessage) Wrap(options *MessageParams) (envelope *Envelope, err error) { if options.TTL == 0 { @@ -259,12 +264,11 @@ func (msg *sentMessage) Wrap(options *MessageParams) (envelope *Envelope, err er // decryptSymmetric decrypts a message with a topic key, using AES-GCM-256. // nonce size should be 12 bytes (see cipher.gcmStandardNonceSize). func (msg *ReceivedMessage) decryptSymmetric(key []byte) error { - // In v6, symmetric messages are expected to contain the 12-byte - // "salt" at the end of the payload. - if len(msg.Raw) < AESNonceLength { + // symmetric messages are expected to contain the 12-byte nonce at the end of the payload + 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 { @@ -274,11 +278,7 @@ func (msg *ReceivedMessage) decryptSymmetric(key []byte) error { if err != nil { return err } - if len(salt) != aesgcm.NonceSize() { - 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) + decrypted, err := aesgcm.Open(nil, salt, msg.Raw[:len(msg.Raw)-aesNonceLength], nil) if err != nil { return err } @@ -296,8 +296,8 @@ func (msg *ReceivedMessage) decryptAsymmetric(key *ecdsa.PrivateKey) error { return err } -// Validate checks the validity and extracts the fields in case of success -func (msg *ReceivedMessage) Validate() bool { +// ValidateAndParse checks the message validity and extracts the fields in case of success. +func (msg *ReceivedMessage) ValidateAndParse() bool { end := len(msg.Raw) if end < 1 { return false @@ -308,40 +308,30 @@ func (msg *ReceivedMessage) Validate() bool { if end <= 1 { return false } - msg.Signature = msg.Raw[end:] + msg.Signature = msg.Raw[end : end+signatureLength] msg.Src = msg.SigToPubKey() if msg.Src == nil { return false } } - padSize, ok := msg.extractPadding(end) - if !ok { - return false + beg := 1 + payloadSize := 0 + sizeOfPayloadSizeField := int(msg.Raw[0] & SizeMask) // number of bytes indicating the size of payload + if sizeOfPayloadSizeField != 0 { + payloadSize = int(bytesToUintLittleEndian(msg.Raw[beg : beg+sizeOfPayloadSizeField])) + if payloadSize+1 > end { + return false + } + beg += sizeOfPayloadSizeField + msg.Payload = msg.Raw[beg : beg+payloadSize] } - msg.Payload = msg.Raw[1+padSize : end] + beg += payloadSize + msg.Padding = msg.Raw[beg:end] return true } -// extractPadding extracts the padding from raw message. -// although we don't support sending messages with padding size -// exceeding 255 bytes, such messages are perfectly valid, and -// can be successfully decrypted. -func (msg *ReceivedMessage) extractPadding(end int) (int, bool) { - paddingSize := 0 - sz := int(msg.Raw[0] & paddingMask) // number of bytes indicating the entire size of padding (including these bytes) - // could be zero -- it means no padding - if sz != 0 { - paddingSize = int(bytesToUintLittleEndian(msg.Raw[1 : 1+sz])) - if paddingSize < sz || paddingSize+1 > end { - return 0, false - } - msg.Padding = msg.Raw[1+sz : 1+paddingSize] - } - return paddingSize, true -} - // SigToPubKey returns the public key associated to the message's // signature. func (msg *ReceivedMessage) SigToPubKey() *ecdsa.PublicKey { @@ -355,7 +345,7 @@ func (msg *ReceivedMessage) SigToPubKey() *ecdsa.PublicKey { return pub } -// hash calculates the SHA3 checksum of the message flags, payload and padding. +// hash calculates the SHA3 checksum of the message flags, payload size field, payload and padding. func (msg *ReceivedMessage) hash() []byte { if isMessageSigned(msg.Raw[0]) { sz := len(msg.Raw) - signatureLength diff --git a/whisper/whisperv6/message_test.go b/whisper/whisperv6/message_test.go index 5f8b41edc6..12a269f5db 100644 --- a/whisper/whisperv6/message_test.go +++ b/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.") + } +} diff --git a/whisper/whisperv6/peer_test.go b/whisper/whisperv6/peer_test.go index b0709c927c..17f70129b5 100644 --- a/whisper/whisperv6/peer_test.go +++ b/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) + } } diff --git a/whisper/whisperv6/whisper.go b/whisper/whisperv6/whisper.go index 1b440a7f9a..d75ad04ac3 100644 --- a/whisper/whisperv6/whisper.go +++ b/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) diff --git a/whisper/whisperv6/whisper_test.go b/whisper/whisperv6/whisper_test.go index 23a289bfe2..838cb7b851 100644 --- a/whisper/whisperv6/whisper_test.go +++ b/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) {