From 79f4d3240b39a6eaf7f0a53a246dd760281c33a5 Mon Sep 17 00:00:00 2001 From: NHAS Date: Sun, 20 Dec 2020 21:59:12 +1300 Subject: [PATCH 1/4] Move AEAD encryption to struct This is so we can store the generated nonces to prevent reuse --- aead.go | 26 +++++++++++++++++--------- client.go | 19 ++++++++++--------- 2 files changed, 27 insertions(+), 18 deletions(-) diff --git a/aead.go b/aead.go index a9699c4f..78b9d291 100644 --- a/aead.go +++ b/aead.go @@ -2,10 +2,14 @@ package noise import ( "crypto/cipher" + "crypto/rand" "io" - "math/rand" ) +type aeadEncryption struct { + suite cipher.AEAD +} + func extendFront(buf []byte, n int) []byte { if len(buf) < n { clone := make([]byte, n+len(buf)) @@ -25,8 +29,12 @@ func extendBack(buf []byte, n int) []byte { return buf[:n] } -func encryptAEAD(suite cipher.AEAD, buf []byte) ([]byte, error) { - a, b := suite.NonceSize(), len(buf) +func (e *aeadEncryption) initialised() bool { + return e.suite == nil +} + +func (e *aeadEncryption) encrypt(buf []byte) ([]byte, error) { + a, b := e.suite.NonceSize(), len(buf) buf = extendFront(buf, a) buf = extendBack(buf, b) @@ -35,16 +43,16 @@ func encryptAEAD(suite cipher.AEAD, buf []byte) ([]byte, error) { return nil, err } - return append(buf[:a], suite.Seal(buf[a:a], buf[:a], buf[a:a+b], nil)...), nil + return append(buf[:a], e.suite.Seal(buf[a:a], buf[:a], buf[a:a+b], nil)...), nil } -func decryptAEAD(suite cipher.AEAD, buf []byte) ([]byte, error) { - if len(buf) < suite.NonceSize() { +func (e *aeadEncryption) decrypt(buf []byte) ([]byte, error) { + if len(buf) < e.suite.NonceSize() { return nil, io.ErrUnexpectedEOF } - nonce := buf[:suite.NonceSize()] - text := buf[suite.NonceSize():] + nonce := buf[:e.suite.NonceSize()] + text := buf[e.suite.NonceSize():] - return suite.Open(text[:0], nonce, text, nil) + return e.suite.Open(text[:0], nonce, text, nil) } diff --git a/client.go b/client.go index 85f9b9c3..4e1e813e 100644 --- a/client.go +++ b/client.go @@ -9,11 +9,12 @@ import ( "encoding/hex" "errors" "fmt" - "go.uber.org/zap" "io" "net" "sync" "time" + + "go.uber.org/zap" ) type clientSide bool @@ -52,7 +53,7 @@ type Client struct { addr string side clientSide - suite cipher.AEAD + suite aeadEncryption logger struct { sync.RWMutex @@ -300,11 +301,11 @@ func (c *Client) read() ([]byte, error) { return nil, err } - if c.suite == nil { + if c.suite.initialised() { return c.readerBuf[4 : size+4], nil } - buf, err := decryptAEAD(c.suite, c.readerBuf[4:size+4]) + buf, err := c.suite.decrypt(c.readerBuf[4 : size+4]) if err != nil { return nil, err } @@ -319,10 +320,10 @@ func (c *Client) write(data []byte) error { } } - if c.suite != nil { + if c.suite.initialised() { var err error - if data, err = encryptAEAD(c.suite, data); err != nil { + if data, err = c.suite.encrypt(data); err != nil { return err } } @@ -448,7 +449,7 @@ func (c *Client) handshake() { return } - c.suite = suite + c.suite = aeadEncryption{suite} // Send to our peer our overlay ID. @@ -600,10 +601,10 @@ Write: buf = buf[:0] buf = msg.marshal(buf) - if c.suite != nil { + if c.suite.initialised() { var err error - if buf, err = encryptAEAD(c.suite, buf); err != nil { + if buf, err = c.suite.encrypt(buf); err != nil { c.Logger().Warn("Got an error encrypting a message.", zap.Error(err)) c.reportError(err) break Write From 57e63891d092cc061fae1e55aeb25cad586291e0 Mon Sep 17 00:00:00 2001 From: NHAS Date: Mon, 21 Dec 2020 10:47:57 +1300 Subject: [PATCH 2/4] Add first implementation of repetition resistant nonces --- aead.go | 49 ++++++++++++++++++++++++++++++++++++++++++------- client.go | 14 +------------- 2 files changed, 43 insertions(+), 20 deletions(-) diff --git a/aead.go b/aead.go index 78b9d291..c7459f39 100644 --- a/aead.go +++ b/aead.go @@ -1,13 +1,41 @@ package noise import ( + "crypto/aes" "crypto/cipher" "crypto/rand" + "encoding/binary" "io" ) type aeadEncryption struct { suite cipher.AEAD + // GCM takes a 12 byte nonce by default + fixed [4]byte //Random part, 4 bytes + counter int64 //Counter 8 bytes +} + +func newAEAD(key []byte) (aeadEncryption, error) { + core, err := aes.NewCipher(key) + if err != nil { + return aeadEncryption{}, err + } + + suite, err := cipher.NewGCM(core) + if err != nil { + return aeadEncryption{}, err + } + + var encryption aeadEncryption + encryption.suite = suite + + //Generate fixed portion of repetition resistant nonce + //https://tools.ietf.org/id/draft-mcgrew-iv-gen-01.html + if _, err := rand.Read(encryption.fixed[:]); err != nil { + return aeadEncryption{}, err + } + + return encryption, nil } func extendFront(buf []byte, n int) []byte { @@ -34,16 +62,23 @@ func (e *aeadEncryption) initialised() bool { } func (e *aeadEncryption) encrypt(buf []byte) ([]byte, error) { - a, b := e.suite.NonceSize(), len(buf) + nonceSize, plaintextSize := e.suite.NonceSize(), len(buf) - buf = extendFront(buf, a) - buf = extendBack(buf, b) + buf = extendFront(buf, nonceSize) + buf = extendBack(buf, plaintextSize) - if _, err := rand.Read(buf[:a]); err != nil { - return nil, err - } + //Repetition resistant nonce https://tools.ietf.org/html/rfc5116#section-3.2 + + copy(buf[:nonceSize], e.fixed[:]) + + binary.BigEndian.PutUint64(buf[len(e.fixed):nonceSize], uint64(e.counter)) + + //Increment Nonce counter + e.counter++ - return append(buf[:a], e.suite.Seal(buf[a:a], buf[:a], buf[a:a+b], nil)...), nil + //Reuse the storage of buf, taking nonce buf[:nonceSize] and plaintext[nonceSize:nonceSize+plaintextSize] + //Put nonce on the front of the ciphertext + return append(buf[:nonceSize], e.suite.Seal(buf[nonceSize:nonceSize], buf[:nonceSize], buf[nonceSize:nonceSize+plaintextSize], nil)...), nil } func (e *aeadEncryption) decrypt(buf []byte) ([]byte, error) { diff --git a/client.go b/client.go index 4e1e813e..c97c3481 100644 --- a/client.go +++ b/client.go @@ -3,8 +3,6 @@ package noise import ( "bufio" "context" - "crypto/aes" - "crypto/cipher" "encoding/binary" "encoding/hex" "errors" @@ -437,20 +435,10 @@ func (c *Client) handshake() { // Use the derived shared key from Diffie-Hellman to encrypt/decrypt all future communications // with AES-256 Galois Counter Mode (GCM). - core, err := aes.NewCipher(shared[:]) + c.suite, err = newAEAD(shared[:]) if err != nil { c.reportError(fmt.Errorf("could not instantiate aes: %w", err)) - return } - - suite, err := cipher.NewGCM(core) - if err != nil { - c.reportError(fmt.Errorf("could not instantiate aes-gcm: %w", err)) - return - } - - c.suite = aeadEncryption{suite} - // Send to our peer our overlay ID. buf := c.node.id.Marshal() From 30866b29f3e15cea768daf1e19c04e2570578fc5 Mon Sep 17 00:00:00 2001 From: NHAS Date: Mon, 21 Dec 2020 11:12:38 +1300 Subject: [PATCH 3/4] Handle edge cases of nonce reuse --- aead.go | 35 ++++++++++++++++++++++++++--------- 1 file changed, 26 insertions(+), 9 deletions(-) diff --git a/aead.go b/aead.go index c7459f39..6642d80f 100644 --- a/aead.go +++ b/aead.go @@ -1,18 +1,20 @@ package noise import ( + "bytes" "crypto/aes" "crypto/cipher" "crypto/rand" "encoding/binary" "io" + "math" ) type aeadEncryption struct { suite cipher.AEAD // GCM takes a 12 byte nonce by default fixed [4]byte //Random part, 4 bytes - counter int64 //Counter 8 bytes + counter uint64 //Counter 8 bytes } func newAEAD(key []byte) (aeadEncryption, error) { @@ -29,13 +31,7 @@ func newAEAD(key []byte) (aeadEncryption, error) { var encryption aeadEncryption encryption.suite = suite - //Generate fixed portion of repetition resistant nonce - //https://tools.ietf.org/id/draft-mcgrew-iv-gen-01.html - if _, err := rand.Read(encryption.fixed[:]); err != nil { - return aeadEncryption{}, err - } - - return encryption, nil + return encryption, encryption.regenerateNonce() } func extendFront(buf []byte, n int) []byte { @@ -57,6 +53,17 @@ func extendBack(buf []byte, n int) []byte { return buf[:n] } +func (e *aeadEncryption) regenerateNonce() error { + e.counter = 0 + + //Generate fixed portion of repetition resistant nonce + //https://tools.ietf.org/id/draft-mcgrew-iv-gen-01.html + if _, err := rand.Read(e.fixed[:]); err != nil { + return err + } + return nil +} + func (e *aeadEncryption) initialised() bool { return e.suite == nil } @@ -71,11 +78,15 @@ func (e *aeadEncryption) encrypt(buf []byte) ([]byte, error) { copy(buf[:nonceSize], e.fixed[:]) - binary.BigEndian.PutUint64(buf[len(e.fixed):nonceSize], uint64(e.counter)) + binary.BigEndian.PutUint64(buf[len(e.fixed):nonceSize], e.counter) //Increment Nonce counter e.counter++ + if math.MaxUint64 == e.counter { // Stop nonce reuse after 2^64 messages + e.regenerateNonce() + } + //Reuse the storage of buf, taking nonce buf[:nonceSize] and plaintext[nonceSize:nonceSize+plaintextSize] //Put nonce on the front of the ciphertext return append(buf[:nonceSize], e.suite.Seal(buf[nonceSize:nonceSize], buf[:nonceSize], buf[nonceSize:nonceSize+plaintextSize], nil)...), nil @@ -89,5 +100,11 @@ func (e *aeadEncryption) decrypt(buf []byte) ([]byte, error) { nonce := buf[:e.suite.NonceSize()] text := buf[e.suite.NonceSize():] + // Handle edge case where both parties generate the same 4 starting bytes + // The best solution to this would be the parties generate a nonce prefix together + // This also has some chance of still generating the same data + if bytes.Equal(e.fixed[:], nonce[:4]) { + e.regenerateNonce() + } return e.suite.Open(text[:0], nonce, text, nil) } From 8b4bcd72b90c52257c202a85c5dff295d4aeb9cc Mon Sep 17 00:00:00 2001 From: NHAS Date: Tue, 22 Dec 2020 12:51:37 +1300 Subject: [PATCH 4/4] Fix small vulnerability An attacker would be able to cause the regeneration of nonces without verifying that the nonce was actually in use --- aead.go | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/aead.go b/aead.go index 6642d80f..5616291e 100644 --- a/aead.go +++ b/aead.go @@ -100,11 +100,16 @@ func (e *aeadEncryption) decrypt(buf []byte) ([]byte, error) { nonce := buf[:e.suite.NonceSize()] text := buf[e.suite.NonceSize():] + cleartext, err := e.suite.Open(text[:0], nonce, text, nil) + if err != nil { + return cleartext, err + } + // Handle edge case where both parties generate the same 4 starting bytes // The best solution to this would be the parties generate a nonce prefix together // This also has some chance of still generating the same data if bytes.Equal(e.fixed[:], nonce[:4]) { e.regenerateNonce() } - return e.suite.Open(text[:0], nonce, text, nil) + return cleartext, err }