perlin-network / noise

A decentralized P2P networking stack written in Go.
https://godoc.org/github.com/perlin-network/noise
MIT License
1.78k stars 213 forks source link

aead.go should import crypto/rand rather than math/rand #289

Open mdouchement opened 3 years ago

mdouchement commented 3 years ago

The nonce should not be generated from a pseudo random input.

NHAS commented 3 years ago

This has been sitting here a while Im going to bump it.

This is very important, as said in the documentation for the Seal(...) function.

//The nonce must be NonceSize() bytes long and unique for all   
// time, for a given key.

As math/rand isnt cryptographically secure you have a much higher risk of repeating a nonce for a given key, which may lead to complete compromise of the plaintext, and authentication property.

Its also worth noting that even when you do switch to crypto/rand it is not advisable to send more than 2^32 messages with a specific key, due to the likelihood of nonce repetition.

This is (unfortunately) only said in a comment on the encryption example in the golang docs.

// Never use more than 2^32 random nonces with a given key because of the risk of a repeat.
    nonce := make([]byte, 12)
    if _, err := io.ReadFull(rand.Reader, nonce); err != nil {
        panic(err.Error())
    }

This would mean that after some number of messages < 2^32 a keychange would be required between peers.

NHAS commented 3 years ago

The better solution, rather changing the key, would be using an algorithm to produce repetition resistant nonces. A good basis for this is https://tools.ietf.org/id/draft-mcgrew-iv-gen-01.html, section 4.1 (although I'd recommend giving the rest of it a read as it's quite interesting).

Also, here is the RFC for this, which is a bit more precise: https://tools.ietf.org/html/rfc5116#section-3.2

damien-white commented 3 years ago

I have submitted a PR for this. There are no existing conflicts. It's a simple fix and makes the codebase more secure.