From 696bfe464c5ca16e1a952f5705ccf6f8ae71b934 Mon Sep 17 00:00:00 2001 From: Matthias Radestock Date: Tue, 28 Apr 2015 15:11:31 +0100 Subject: [PATCH 1/5] replace random nonces with message sequence numbers in tcp crypto Since TCP is reliable, we do not need to transmit the nonce itself; the recipient can simply construct the nonce from its own message counter. Hence there is no message envelope and we can thus remove a whole layer of encoding. --- router/connection.go | 2 +- router/crypto.go | 120 ++++++++++++++++--------------------------- router/handshake.go | 2 +- 3 files changed, 45 insertions(+), 79 deletions(-) diff --git a/router/connection.go b/router/connection.go index 4aacd611..8be52e26 100644 --- a/router/connection.go +++ b/router/connection.go @@ -446,7 +446,7 @@ func (conn *LocalConnection) receiveTCP(decoder *gob.Decoder) { usingPassword := conn.SessionKey != nil var receiver TCPReceiver if usingPassword { - receiver = NewEncryptedTCPReceiver(conn.SessionKey) + receiver = NewEncryptedTCPReceiver(conn.SessionKey, conn.outbound) } else { receiver = NewSimpleTCPReceiver() } diff --git a/router/crypto.go b/router/crypto.go index 31a9107c..59baa6c3 100644 --- a/router/crypto.go +++ b/router/crypto.go @@ -1,7 +1,6 @@ package router import ( - "bytes" "code.google.com/p/go-bit/bit" "code.google.com/p/go.crypto/nacl/box" "code.google.com/p/go.crypto/nacl/secretbox" @@ -39,22 +38,6 @@ func GenerateRandomNonce() ([24]byte, error) { return nonce, nil } -func EncryptPrefixNonce(plaintxt []byte, nonce *[24]byte, secret *[32]byte) []byte { - buf := make([]byte, 24, 24+len(plaintxt)+secretbox.Overhead) - copy(buf, nonce[:]) - // Seal *appends* to buf - return secretbox.Seal(buf, plaintxt, nonce, secret) -} - -func DecryptPrefixNonce(ciphertxt []byte, secret *[32]byte) ([]byte, bool) { - if len(ciphertxt) < secretbox.Overhead+24 { - return nil, false - } - var nonce [24]byte - copy(nonce[:], ciphertxt[:24]) - return secretbox.Open(nil, ciphertxt[24:], &nonce, secret) -} - func SetNonceLow15Bits(nonce *[24]byte, offset uint16) { // ensure top bit of offset is 0 offset = offset & ((1 << 15) - 1) @@ -419,7 +402,32 @@ func (di *NaClDecryptorInstance) advanceState(offsetNoFlags uint16) (*[24]byte, return di.nonce, di.usedOffsets, nil } -// TCP Senders +// TCP Senders/Receivers + +// The lowest 64 bits of the nonce contain the message sequence +// number. The top most bit indicates the connection polarity at the +// sender - '1' for outbound. The remaining 127 bits are zero. The +// polarity is needed so that the two ends of a connection do not use +// the same nonces. This is a requirement of the NaCl Security Model; +// see http://nacl.cr.yp.to/box.html. +type TCPCryptoState struct { + sessionKey *[32]byte + nonce [24]byte + seqNo uint64 +} + +func NewTCPCryptoState(sessionKey *[32]byte, outbound bool) *TCPCryptoState { + s := &TCPCryptoState{sessionKey: sessionKey} + if outbound { + s.nonce[0] |= (1 << 7) + } + return s +} + +func (s *TCPCryptoState) advance() { + s.seqNo++ + binary.BigEndian.PutUint64(s.nonce[16:24], s.seqNo) +} type TCPSender interface { Send([]byte) error @@ -430,17 +438,9 @@ type SimpleTCPSender struct { } type EncryptedTCPSender struct { + SimpleTCPSender sync.RWMutex - outerEncoder *gob.Encoder - innerEncoder *gob.Encoder - buffer *bytes.Buffer - sessionKey *[32]byte - msgCount int -} - -type EncryptedTCPMessage struct { - Number int - Body []byte + state *TCPCryptoState } func NewSimpleTCPSender(encoder *gob.Encoder) *SimpleTCPSender { @@ -451,36 +451,20 @@ func (sender *SimpleTCPSender) Send(msg []byte) error { return sender.encoder.Encode(msg) } -func NewEncryptedTCPSender(encoder *gob.Encoder, sessionKey *[32]byte) *EncryptedTCPSender { - buffer := new(bytes.Buffer) +func NewEncryptedTCPSender(encoder *gob.Encoder, sessionKey *[32]byte, outbound bool) *EncryptedTCPSender { return &EncryptedTCPSender{ - outerEncoder: encoder, - innerEncoder: gob.NewEncoder(buffer), - buffer: buffer, - sessionKey: sessionKey, - msgCount: 0} + SimpleTCPSender: *NewSimpleTCPSender(encoder), + state: NewTCPCryptoState(sessionKey, outbound)} } func (sender *EncryptedTCPSender) Send(msg []byte) error { - nonce, err := GenerateRandomNonce() - if err != nil { - return err - } sender.Lock() defer sender.Unlock() - wrappedMsg := EncryptedTCPMessage{Number: sender.msgCount, Body: msg} - buffer := sender.buffer - buffer.Reset() - err = sender.innerEncoder.Encode(wrappedMsg) - if err != nil { - return err - } - sender.msgCount += 1 - return sender.outerEncoder.Encode(EncryptPrefixNonce(buffer.Bytes(), &nonce, sender.sessionKey)) + encodedMsg := secretbox.Seal(nil, msg, &sender.state.nonce, sender.state.sessionKey) + sender.state.advance() + return sender.SimpleTCPSender.Send(encodedMsg) } -// TCP Receivers - type TCPReceiver interface { Decode([]byte) ([]byte, error) } @@ -489,10 +473,8 @@ type SimpleTCPReceiver struct { } type EncryptedTCPReceiver struct { - sessionKey *[32]byte - decoder *gob.Decoder - buffer *bytes.Buffer - msgCount int + SimpleTCPReceiver + state *TCPCryptoState } func NewSimpleTCPReceiver() *SimpleTCPReceiver { @@ -503,33 +485,17 @@ func (receiver *SimpleTCPReceiver) Decode(msg []byte) ([]byte, error) { return msg, nil } -func NewEncryptedTCPReceiver(sessionKey *[32]byte) *EncryptedTCPReceiver { - buffer := new(bytes.Buffer) +func NewEncryptedTCPReceiver(sessionKey *[32]byte, outbound bool) *EncryptedTCPReceiver { return &EncryptedTCPReceiver{ - sessionKey: sessionKey, - decoder: gob.NewDecoder(buffer), - buffer: buffer, - msgCount: 0} + SimpleTCPReceiver: *NewSimpleTCPReceiver(), + state: NewTCPCryptoState(sessionKey, !outbound)} } func (receiver *EncryptedTCPReceiver) Decode(msg []byte) ([]byte, error) { - plaintext, success := DecryptPrefixNonce(msg, receiver.sessionKey) + decodedMsg, success := secretbox.Open(nil, msg, &receiver.state.nonce, receiver.state.sessionKey) if !success { - return msg, fmt.Errorf("Unable to decrypt TCP msg") + return nil, fmt.Errorf("Unable to decrypt TCP msg") } - receiver.buffer.Reset() - _, err := receiver.buffer.Write(plaintext) - if err != nil { - return msg, err - } - wrappedMsg := new(EncryptedTCPMessage) - err = receiver.decoder.Decode(wrappedMsg) - if err != nil { - return msg, err - } - if wrappedMsg.Number != receiver.msgCount { - return msg, fmt.Errorf("Received TCP message with wrong sequence number; possible replay attack") - } - receiver.msgCount += 1 - return wrappedMsg.Body, nil + receiver.state.advance() + return decodedMsg, nil } diff --git a/router/handshake.go b/router/handshake.go index 48dd5125..1db0d86e 100644 --- a/router/handshake.go +++ b/router/handshake.go @@ -102,7 +102,7 @@ func (conn *LocalConnection) handshake(enc *gob.Encoder, dec *gob.Decoder, accep remotePublic[idx] = elem } conn.SessionKey = FormSessionKey(&remotePublic, private, conn.Router.Password) - conn.tcpSender = NewEncryptedTCPSender(enc, conn.SessionKey) + conn.tcpSender = NewEncryptedTCPSender(enc, conn.SessionKey, conn.outbound) conn.Decryptor = NewNaClDecryptor(conn.SessionKey) } else { if rpErr == nil { From b365c3806bb132580232b7e8b075e80a35b13a89 Mon Sep 17 00:00:00 2001 From: Matthias Radestock Date: Tue, 28 Apr 2015 15:12:15 +0100 Subject: [PATCH 2/5] replace random nonces with message sequence numbers in udp crypto --- router/crypto.go | 275 +++++++++++++++----------------------------- router/forwarder.go | 8 +- router/handshake.go | 2 +- 3 files changed, 98 insertions(+), 187 deletions(-) diff --git a/router/crypto.go b/router/crypto.go index 59baa6c3..77e72715 100644 --- a/router/crypto.go +++ b/router/crypto.go @@ -26,55 +26,6 @@ func FormSessionKey(remotePublicKey, localPrivateKey *[32]byte, secretKey []byte return &sessionKey } -func GenerateRandomNonce() ([24]byte, error) { - var nonce [24]byte - n, err := rand.Read(nonce[:]) - if err != nil { - return nonce, err - } - if n != 24 { - return nonce, fmt.Errorf("Did not read enough - wanted 24, got %v", n) - } - return nonce, nil -} - -func SetNonceLow15Bits(nonce *[24]byte, offset uint16) { - // ensure top bit of offset is 0 - offset = offset & ((1 << 15) - 1) - // grab top bit of nonce[22:24] (and clear out lower bits) - nonceBits := binary.BigEndian.Uint16(nonce[22:24]) & (1 << 15) - // Big endian => the MSB is stored at the *lowest* address. So - // that top bit in nonce[22] should stay as the top bit in - // nonce[22] - binary.BigEndian.PutUint16(nonce[22:24], nonceBits+offset) -} - -// Nonce encoding/decoding - -func EncodeNonce(df bool) (*[24]byte, []byte, error) { - nonce, err := GenerateRandomNonce() - if err != nil { - return nil, []byte{}, err - } - // wipe out lowest 15 bits, but encode the df right at the bottom - flags := uint16(0) - if df { - flags |= 1 - } - SetNonceLow15Bits(&nonce, flags) - // NB: need to make a copy since callers may modify the array - return &nonce, Concat(nonce[:]), nil -} - -func DecodeNonce(msg []byte) (bool, *[24]byte) { - flags := uint16(msg[23]) - df := 0 != (flags & 1) - var nonce [24]byte - // upper bound is exclusive so this avoids copying the flags - copy(nonce[:], msg[:23]) - return df, &nonce -} - // Frame Encryptors type Encryptor interface { @@ -96,12 +47,10 @@ type NonEncryptor struct { type NaClEncryptor struct { NonEncryptor buf []byte - offset uint16 - nonce *[24]byte - nonceChan chan *[24]byte prefixLen int sessionKey *[32]byte - sendNonce func([]byte) + nonce [24]byte + seqNo uint64 df bool } @@ -151,19 +100,19 @@ func (ne *NonEncryptor) TotalLen() int { return ne.buffered } -func NewNaClEncryptor(prefix []byte, sessionKey *[32]byte, sendNonce func([]byte), df bool) *NaClEncryptor { +func NewNaClEncryptor(prefix []byte, sessionKey *[32]byte, outbound bool, df bool) *NaClEncryptor { buf := make([]byte, MaxUDPPacketSize) prefixLen := copy(buf, prefix) - return &NaClEncryptor{ + ne := &NaClEncryptor{ NonEncryptor: *NewNonEncryptor([]byte{}), buf: buf, - offset: 0, - nonce: nil, - nonceChan: make(chan *[24]byte, ChannelSize), prefixLen: prefixLen, sessionKey: sessionKey, - sendNonce: sendNonce, df: df} + if outbound { + ne.nonce[0] |= (1 << 7) + } + return ne } func (ne *NaClEncryptor) Bytes() ([]byte, error) { @@ -171,52 +120,27 @@ func (ne *NaClEncryptor) Bytes() ([]byte, error) { if err != nil { return nil, err } - offsetFlags := ne.offset // We carry the DF flag in the (unencrypted portion of the) // payload, rather than just extracting it from the packet headers // at the receiving end, since we do not trust routers not to mess // with headers. As we have different decryptors for non-DF and // DF, that would result in hard to track down packet drops due to // crypto errors. + seqNoAndDF := ne.seqNo if ne.df { - offsetFlags |= (1 << 15) + seqNoAndDF |= (1 << 63) } ciphertext := ne.buf - binary.BigEndian.PutUint16(ciphertext[ne.prefixLen:], offsetFlags) - nonce := ne.nonce - if nonce == nil { - freshNonce, encodedNonce, err := EncodeNonce(ne.df) - if err != nil { - return nil, err - } - ne.sendNonce(encodedNonce) - ne.nonce = freshNonce - nonce = freshNonce - } - offset := ne.offset - SetNonceLow15Bits(nonce, offset) + binary.BigEndian.PutUint64(ciphertext[ne.prefixLen:], seqNoAndDF) + binary.BigEndian.PutUint64(ne.nonce[16:24], seqNoAndDF) // Seal *appends* to ciphertext - ciphertext = secretbox.Seal(ciphertext[:ne.prefixLen+2], plaintext, nonce, ne.sessionKey) - - offset = (offset + 1) & ((1 << 15) - 1) - if offset == 0 { - // need a new nonce please - ne.nonce = <-ne.nonceChan - } else if offset == 1<<14 { // half way through range, send new nonce - nonce, encodedNonce, err := EncodeNonce(ne.df) - if err != nil { - return nil, err - } - ne.nonceChan <- nonce - ne.sendNonce(encodedNonce) - } - ne.offset = offset - + ciphertext = secretbox.Seal(ciphertext[:ne.prefixLen+8], plaintext, &ne.nonce, ne.sessionKey) + ne.seqNo++ return ciphertext, nil } func (ne *NaClEncryptor) PacketOverhead() int { - return ne.prefixLen + 2 + secretbox.Overhead + ne.NonEncryptor.PacketOverhead() + return ne.prefixLen + 8 + secretbox.Overhead + ne.NonEncryptor.PacketOverhead() } func (ne *NaClEncryptor) TotalLen() int { @@ -244,12 +168,18 @@ type NaClDecryptor struct { } type NaClDecryptorInstance struct { - nonce *[24]byte - previousNonce *[24]byte + nonce [24]byte + currentWindow uint64 usedOffsets *bit.Set previousUsedOffsets *bit.Set - highestOffsetSeen uint16 - nonceChan chan *[24]byte +} + +func NewNaClDecryptorInstance(outbound bool) *NaClDecryptorInstance { + di := &NaClDecryptorInstance{usedOffsets: bit.New()} + if !outbound { + di.nonce[0] |= (1 << 7) + } + return di } type PacketDecodingError struct { @@ -288,118 +218,103 @@ func (nd *NonDecryptor) ReceiveNonce(msg []byte) { log.Println("Received Nonce on non-encrypted channel. Ignoring.") } -func NewNaClDecryptor(sessionKey *[32]byte) *NaClDecryptor { +func NewNaClDecryptor(sessionKey *[32]byte, outbound bool) *NaClDecryptor { return &NaClDecryptor{ NonDecryptor: *NewNonDecryptor(), sessionKey: sessionKey, - instance: &NaClDecryptorInstance{ - usedOffsets: bit.New(), - nonceChan: make(chan *[24]byte, ChannelSize)}, - instanceDF: &NaClDecryptorInstance{ - usedOffsets: bit.New(), - nonceChan: make(chan *[24]byte, ChannelSize)}} + instance: NewNaClDecryptorInstance(outbound), + instanceDF: NewNaClDecryptorInstance(outbound)} } func (nd *NaClDecryptor) Shutdown() { - close(nd.instance.nonceChan) - close(nd.instanceDF.nonceChan) } func (nd *NaClDecryptor) ReceiveNonce(msg []byte) { - df, nonce := DecodeNonce(msg) - if df { - nd.instanceDF.nonceChan <- nonce - } else { - nd.instance.nonceChan <- nonce - } } func (nd *NaClDecryptor) IterateFrames(packet []byte, consumer FrameConsumer) error { - buf, err := nd.decrypt(packet) - if err != nil { - return PacketDecodingError{Desc: fmt.Sprint("decryption failed; ", err)} + if len(packet) < 8 { + return PacketDecodingError{Desc: fmt.Sprintf("encrypted UDP packet too short; expected length >= 8, got %d", len(packet))} + } + buf, success := nd.decrypt(packet) + if !success { + return PacketDecodingError{Desc: fmt.Sprint("UDP packet decryption failed")} } return nd.NonDecryptor.IterateFrames(buf, consumer) } -func (nd *NaClDecryptor) decrypt(buf []byte) ([]byte, error) { - offset := binary.BigEndian.Uint16(buf[:2]) - df := (offset & (1 << 15)) != 0 - offsetNoFlags := offset & ((1 << 15) - 1) +func (nd *NaClDecryptor) decrypt(buf []byte) ([]byte, bool) { + seqNoAndDF := binary.BigEndian.Uint64(buf[:8]) + df := (seqNoAndDF & (1 << 63)) != 0 + seqNo := seqNoAndDF & ((1 << 63) - 1) var di *NaClDecryptorInstance if df { di = nd.instanceDF } else { di = nd.instance } - nonce, usedOffsets, err := di.advanceState(offsetNoFlags) - if err != nil { - return nil, err - } - offsetNoFlagsInt := int(offsetNoFlags) - if usedOffsets.Contains(offsetNoFlagsInt) { - return nil, fmt.Errorf("Suspected replay attack detected when decrypting UDP packet") - } - SetNonceLow15Bits(nonce, offsetNoFlags) - result, success := secretbox.Open(nil, buf[2:], nonce, nd.sessionKey) + binary.BigEndian.PutUint64(di.nonce[16:24], seqNoAndDF) + result, success := secretbox.Open(nil, buf[8:], &di.nonce, nd.sessionKey) if !success { - return nil, fmt.Errorf("Unable to decrypt UDP packet") + return nil, false } - usedOffsets.Add(offsetNoFlagsInt) - return result, nil + // Drop duplicates. We do this *after* decryption since we must + // not advance our state unless decryption succeeded. Doing so + // would open an easy attack vector where an adversary could + // inject a packet with a sequence number of (1 << 63) - 1, + // causing all subsequent genuine packets to get dropped. + offset, usedOffsets := di.advanceState(seqNo) + if usedOffsets == nil || usedOffsets.Contains(offset) { + // We have detected a possible replay attack, but it is + // possible we may have just received a very old packet, or + // duplication may have occurred in the network. So let's just + // drop the packet silently. + return nil, true + } + usedOffsets.Add(offset) + return result, success } -func (di *NaClDecryptorInstance) advanceState(offsetNoFlags uint16) (*[24]byte, *bit.Set, error) { - var ok bool - if di.nonce == nil { - if offsetNoFlags > (1 << 13) { - // offset is already beyond the first quarter and it's the - // first thing we've seen?! I don't think so. - return nil, nil, fmt.Errorf("Unexpected offset when decrypting UDP packet") - } - di.nonce, ok = <-di.nonceChan - if !ok { - return nil, nil, fmt.Errorf("Nonce chan closed") - } - di.highestOffsetSeen = offsetNoFlags - } else { - highestOffsetSeen := di.highestOffsetSeen - switch { - case offsetNoFlags < (1<<13) && highestOffsetSeen > ((1<<14)+(1<<13)) && - (highestOffsetSeen-offsetNoFlags) > ((1<<14)+(1<<13)): - // offset is in the first quarter, highestOffsetSeen is in - // the top quarter and under a quarter behind us. We - // interpret this as we need to move to the next nonce - di.previousUsedOffsets = di.usedOffsets - di.usedOffsets = bit.New() - di.previousNonce = di.nonce - di.nonce, ok = <-di.nonceChan - if !ok { - return nil, nil, fmt.Errorf("Nonce chan closed") - } - di.highestOffsetSeen = offsetNoFlags - case offsetNoFlags > highestOffsetSeen && - (offsetNoFlags-highestOffsetSeen) < (1<<13): - // offset is under a quarter above highestOffsetSeen. This - // is ok - maybe some packet loss - di.highestOffsetSeen = offsetNoFlags - case offsetNoFlags <= highestOffsetSeen && - (highestOffsetSeen-offsetNoFlags) < (1<<13): - // offset is within a quarter of the highest we've - // seen. This is ok - just assuming some out-of-order - // delivery. - case highestOffsetSeen < (1<<13) && offsetNoFlags > ((1<<14)+(1<<13)) && - (offsetNoFlags-highestOffsetSeen) > ((1<<14)+(1<<13)): - // offset is in the last quarter, highestOffsetSeen is in - // the first quarter, and offset is under a quarter behind - // us. This is ok - as above, just some out of order. But - // here it means we're dealing with the previous nonce - return di.previousNonce, di.previousUsedOffsets, nil - default: - return nil, nil, fmt.Errorf("Unexpected offset when decrypting UDP packet") - } +// We record seen message sequence numbers in a sliding window of +// 2*WindowSize which slides in WindowSize increments. This allows us +// to process out-of-order delivery within the window, while +// accurately discarding duplicates. By contrast any messages with +// sequence numbers below the window are discarded as potential +// duplicates. +// +// There are two sets, corresponding to the lower and upper half of +// the window. We slide the window s.t. that 2nd set always contains +// the highest seen sequence number. We do this regardless of how far +// ahead of the current window that sequence number might be, so we +// can cope with large gaps resulting from packet loss. + +const ( + WindowSize = 20 // bits +) + +func (di *NaClDecryptorInstance) advanceState(seqNo uint64) (int, *bit.Set) { + var ( + offset = int(seqNo & ((1 << WindowSize) - 1)) + window = seqNo >> WindowSize + ) + switch delta := int64(window - di.currentWindow); { + case delta < -1: + return offset, nil + case delta == -1: + return offset, di.previousUsedOffsets + default: + return offset, di.usedOffsets + case delta == +1: + di.currentWindow = window + di.previousUsedOffsets = di.usedOffsets + di.usedOffsets = bit.New() + return offset, di.usedOffsets + case delta > +1: + di.currentWindow = window + di.previousUsedOffsets = bit.New() + di.usedOffsets = bit.New() + return offset, di.usedOffsets } - return di.nonce, di.usedOffsets, nil } // TCP Senders/Receivers diff --git a/router/forwarder.go b/router/forwarder.go index d3f61847..a4cec472 100644 --- a/router/forwarder.go +++ b/router/forwarder.go @@ -30,8 +30,8 @@ func (conn *LocalConnection) ensureForwarders() error { usingPassword := conn.SessionKey != nil var encryptor, encryptorDF Encryptor if usingPassword { - encryptor = NewNaClEncryptor(conn.local.NameByte, conn.SessionKey, conn.sendNonce, false) - encryptorDF = NewNaClEncryptor(conn.local.NameByte, conn.SessionKey, conn.sendNonce, true) + encryptor = NewNaClEncryptor(conn.local.NameByte, conn.SessionKey, conn.outbound, false) + encryptorDF = NewNaClEncryptor(conn.local.NameByte, conn.SessionKey, conn.outbound, true) } else { encryptor = NewNonEncryptor(conn.local.NameByte) encryptorDF = NewNonEncryptor(conn.local.NameByte) @@ -62,10 +62,6 @@ func (conn *LocalConnection) stopForwarders() { conn.forwarderDF.Shutdown() } -func (conn *LocalConnection) sendNonce(encodedNonce []byte) { - conn.SendProtocolMsg(ProtocolMsg{ProtocolNonce, encodedNonce}) -} - // Called from peer.Relay[Broadcast] which is itself invoked from // router (both UDP listener process and sniffer process). Also called // from connection's heartbeat process, and from the connection's TCP diff --git a/router/handshake.go b/router/handshake.go index 1db0d86e..84dc9341 100644 --- a/router/handshake.go +++ b/router/handshake.go @@ -103,7 +103,7 @@ func (conn *LocalConnection) handshake(enc *gob.Encoder, dec *gob.Decoder, accep } conn.SessionKey = FormSessionKey(&remotePublic, private, conn.Router.Password) conn.tcpSender = NewEncryptedTCPSender(enc, conn.SessionKey, conn.outbound) - conn.Decryptor = NewNaClDecryptor(conn.SessionKey) + conn.Decryptor = NewNaClDecryptor(conn.SessionKey, conn.outbound) } else { if rpErr == nil { return fmt.Errorf("Remote network is encrypted. Password required.") From e223899982fd2b75c653db3f90a08618d6997392 Mon Sep 17 00:00:00 2001 From: Matthias Radestock Date: Tue, 28 Apr 2015 11:12:45 +0100 Subject: [PATCH 3/5] remove nonce transmission from protocol --- router/connection.go | 5 ----- router/consts.go | 1 - router/crypto.go | 9 --------- router/protocol.go | 3 +-- 4 files changed, 1 insertion(+), 17 deletions(-) diff --git a/router/connection.go b/router/connection.go index 8be52e26..5ba2e033 100644 --- a/router/connection.go +++ b/router/connection.go @@ -489,11 +489,6 @@ func (conn *LocalConnection) handleProtocolMsg(tag ProtocolTag, payload []byte) nil) case ProtocolFragmentationReceived: conn.setStackFrag(true) - case ProtocolNonce: - if conn.SessionKey == nil { - return fmt.Errorf("unexpected nonce on unencrypted connection") - } - conn.Decryptor.ReceiveNonce(payload) case ProtocolPMTUVerified: conn.pmtuVerified(int(binary.BigEndian.Uint16(payload))) case ProtocolGossipUnicast, ProtocolGossipBroadcast, ProtocolGossip: diff --git a/router/consts.go b/router/consts.go index 0aa13fd6..c0cc0b09 100644 --- a/router/consts.go +++ b/router/consts.go @@ -13,7 +13,6 @@ const ( DefaultPMTU = 65535 MaxUDPPacketSize = 65536 ChannelSize = 16 - UDPNonceSendAt = 8192 FragTestSize = 60001 PMTUDiscoverySize = 60000 TCPHeartbeat = 30 * time.Second diff --git a/router/crypto.go b/router/crypto.go index 77e72715..0d893469 100644 --- a/router/crypto.go +++ b/router/crypto.go @@ -9,7 +9,6 @@ import ( "encoding/binary" "encoding/gob" "fmt" - "log" "sync" ) @@ -153,7 +152,6 @@ type FrameConsumer func(src []byte, dst []byte, frame []byte) type Decryptor interface { IterateFrames([]byte, FrameConsumer) error - ReceiveNonce([]byte) Shutdown() } @@ -214,10 +212,6 @@ func (nd *NonDecryptor) IterateFrames(packet []byte, consumer FrameConsumer) err func (nd *NonDecryptor) Shutdown() { } -func (nd *NonDecryptor) ReceiveNonce(msg []byte) { - log.Println("Received Nonce on non-encrypted channel. Ignoring.") -} - func NewNaClDecryptor(sessionKey *[32]byte, outbound bool) *NaClDecryptor { return &NaClDecryptor{ NonDecryptor: *NewNonDecryptor(), @@ -229,9 +223,6 @@ func NewNaClDecryptor(sessionKey *[32]byte, outbound bool) *NaClDecryptor { func (nd *NaClDecryptor) Shutdown() { } -func (nd *NaClDecryptor) ReceiveNonce(msg []byte) { -} - func (nd *NaClDecryptor) IterateFrames(packet []byte, consumer FrameConsumer) error { if len(packet) < 8 { return PacketDecodingError{Desc: fmt.Sprintf("encrypted UDP packet too short; expected length >= 8, got %d", len(packet))} diff --git a/router/protocol.go b/router/protocol.go index 0fc548c6..441c208c 100644 --- a/router/protocol.go +++ b/router/protocol.go @@ -2,7 +2,7 @@ package router const ( Protocol = "weave" - ProtocolVersion = 15 + ProtocolVersion = 16 ) type ProtocolTag byte @@ -12,7 +12,6 @@ const ( ProtocolConnectionEstablished ProtocolFragmentationReceived ProtocolStartFragmentationTest - ProtocolNonce ProtocolPMTUVerified ProtocolGossip ProtocolGossipUnicast From 137776bcb296a88ae53cea03e5b6bab2d22903df Mon Sep 17 00:00:00 2001 From: Matthias Radestock Date: Tue, 28 Apr 2015 11:33:51 +0100 Subject: [PATCH 4/5] remove decryptor shutdown we only needed it because of the nonce channels --- router/connection.go | 1 - router/crypto.go | 7 ------- 2 files changed, 8 deletions(-) diff --git a/router/connection.go b/router/connection.go index 5ba2e033..ed0651a4 100644 --- a/router/connection.go +++ b/router/connection.go @@ -442,7 +442,6 @@ func (conn *LocalConnection) sendProtocolMsg(m ProtocolMsg) error { } func (conn *LocalConnection) receiveTCP(decoder *gob.Decoder) { - defer conn.Decryptor.Shutdown() usingPassword := conn.SessionKey != nil var receiver TCPReceiver if usingPassword { diff --git a/router/crypto.go b/router/crypto.go index 0d893469..9987ce58 100644 --- a/router/crypto.go +++ b/router/crypto.go @@ -152,7 +152,6 @@ type FrameConsumer func(src []byte, dst []byte, frame []byte) type Decryptor interface { IterateFrames([]byte, FrameConsumer) error - Shutdown() } type NonDecryptor struct { @@ -209,9 +208,6 @@ func (nd *NonDecryptor) IterateFrames(packet []byte, consumer FrameConsumer) err return nil } -func (nd *NonDecryptor) Shutdown() { -} - func NewNaClDecryptor(sessionKey *[32]byte, outbound bool) *NaClDecryptor { return &NaClDecryptor{ NonDecryptor: *NewNonDecryptor(), @@ -220,9 +216,6 @@ func NewNaClDecryptor(sessionKey *[32]byte, outbound bool) *NaClDecryptor { instanceDF: NewNaClDecryptorInstance(outbound)} } -func (nd *NaClDecryptor) Shutdown() { -} - func (nd *NaClDecryptor) IterateFrames(packet []byte, consumer FrameConsumer) error { if len(packet) < 8 { return PacketDecodingError{Desc: fmt.Sprintf("encrypted UDP packet too short; expected length >= 8, got %d", len(packet))} From cdf7542c2549b4bf849b939feccf3943b009d196 Mon Sep 17 00:00:00 2001 From: Matthias Radestock Date: Tue, 28 Apr 2015 14:44:34 +0100 Subject: [PATCH 5/5] update docs --- site/how-it-works.md | 122 ++++++++++++++++--------------------------- 1 file changed, 46 insertions(+), 76 deletions(-) diff --git a/site/how-it-works.md b/site/how-it-works.md index a8acf13f..a9fc77c8 100644 --- a/site/how-it-works.md +++ b/site/how-it-works.md @@ -282,25 +282,23 @@ between two peers. #### TCP TCP connection are only used to exchange topology information between -peers, via a message-based protocol. The router generates a fresh -192-bit random nonce for every message to be sent, and prepends the -nonce to the encrypted message, as is normal in NaCl, so that the -receiver knows the nonce. Encryption of each message is carried out -using NaCl's `secretbox.Seal` function using the ephemeral session -key. Each TCP connection has a monotonically incrementing message -counter, the current value of which is included in the encrypted part -of the message. Given the assumption that TCP is reliable and ordered, -a message received via TCP is only acted upon if the message counter -in the received message is the expected message counter. This prevents -replay attacks on the TCP connection. +peers, via a message-based protocol. Encryption of each message is +carried out by NaCl's `secretbox.Seal` function using the ephemeral +session key and a nonce. The nonce contains the message sequence +number, which is incremented for every message sent, and a bit +indicating the polarity of the connection at the sender ('1' for +outbound). The latter is required by the +[NaCl Security Model](http://nacl.cr.yp.to/box.html) in order to +ensure that the two ends of the connection do not use the same nonces. -As TCP connections do not carry captured traffic, minimising message -size or latency is not a major concern, so the potentially substantial -increase in length of messages by prepending the full nonce, or the -cost of generating a fresh random nonce for each message is not -considered likely to cause problems. The random nonces are created by -the go `crypto/rand` package, which implements a cryptographically -secure pseudorandom number generator. +Decryption of a message at the receiver is carried out by NaCl's +`secretbox.Open` function using the ephemeral session key and a +nonce. The receiver maintains its own message sequence number, which +it increments for every message it decrypted successfully. The nonce +is constructed from that sequence number and the connection +polarity. As a result the receiver will only be able to decrypt a +message if it has the expected sequence number. This prevents replay +attacks. #### UDP @@ -311,7 +309,7 @@ follows: +-----------------------------------+ | Name of sending peer | +-----------------------------------+ - | Nonce offset and flags | + | Message Sequence No and flags | +-----------------------------------+ | NaCl SecretBox overheads | +-----------------------------------+ -+ @@ -344,65 +342,37 @@ follows: This is very similar to the [non-crypto encapsulation](#encapsulation). -All of the frames are encrypted with the same ephemeral session key -and all must be decrypted by the receiving peer. Frames which are to -be forwarded on to some further peer will be re-encrypted with the -relevant ephemeral session keys for the onward connections. Thus all -traffic is fully decrypted on every peer it passes through. -Encryption is again done with the NaCl `secretbox.Seal` function. +All of the frames on a connection are encrypted with the same +ephemeral session key, and a nonce constructed from a message sequence +number, flags and the connection polarity. This is very similar to the +TCP encryption scheme, and encryption is again done with the NaCl +`secretbox.Seal` function. The main difference is that the message +sequence number and flags are transmitted as part of the message, +unencrypted. -The name of the sending peer enables the receiver to identify the peer -who sent this UDP packet, and in turn to determine which ephemeral -session key was used and which nonce, and perform decryption. To avoid -sending a fresh 192-bit nonce with every UDP packet, which would pose -an unacceptable overhead, each UDP packet only carries the lowest 15 -bits of the nonce, which is treated as an offset from the "established -nonce". The lifecycle of a nonce for UDP is: +The receiver uses the name of the sending peer to determine which +ephemeral session key and local cryptographic state to use for +decryption. Frames which are to be forwarded on to some further peer +will be re-encrypted with the relevant ephemeral session keys for the +onward connections. Thus all traffic is fully decrypted on every peer +it passes through. -1. A fresh nonce is generated and the most significant 177 bits are - sent to the receiving peer over the TCP connection. These upper - most 177 bits are used for the next 2^15 (32768) UDP messages. This - is the "established nonce". -2. Each UDP message carries the lowest 15 bits as a unique - counter. Thus the lowest 15 bits are combined with (appended to) - the uppermost 177 bits to form the unique nonce for that message. -3. Once the sending side has sent the 16384'th message on the current - nonce (50% of the way through the available range), it generates a - new nonce (upper 177 bits only) and sends that over the TCP - connection, thus hopefully ensuring it arrives and is ready before - it is needed. The sending side will switch to using the new nonce - once the full 32768 messages of the current nonce have been - used. At this point, the sending side resets the offset to 0. -4. The receiving side must deal with the fact that UDP is unreliable - and unordered. The receiving side keeps track of the highest offset - seen for each established nonce. The condition to switching to the - new nonce it has received via the TCP connection is: the highest - offset seen for the current nonce must be above 24576 (75% of the - available range) _and_ the current message just received must have - an offset less than 8192 (25% of the available range) _and_ the new - offset must be less than 8192 messages *ahead* of the highest seen - offset so far (assuming modulo 32768). If these conditions are met, - the highest offset is set to the offset of the current message and - the new nonce is used. If those conditions are not met, the - receiving side continues with the current nonce (according to the - rules below), updating the highest offset seen as appropriate. Thus - in order for the nonce to not be updated correctly would require a - loss of at least 8192 messages. -5. When the receiving side switches to the new nonce, it does not - discard the old nonce. If the highest offset seen is below 8192 - (25% of the range) _and_ the current message offset is above 24576 - (75% of the range), _and_ the current message offset is less than - 8192 behind the highest seen nonce (assuming modulo 32768), then - the current message is decoded with the old nonce. -6. In the remaining cases, the current message is only decoded if its - offset is within 8192 either side of the highest seen offset. -7. To avoid replay attacks, the receiving side keeps a set of which - offsets have been used with the current and previous nonce. If the - offset doesn't exist in the set, and the message can be correctly - decoded, the offset is added to the relevant set (thus we avoid - poisoning attacks). If the offset already exists in the set, or the - message cannot be correctly decoded, the message is not processed - further. +Decryption is once again carried out by NaCl's `secretbox.Open` +function using the ephemeral session key and nonce. The latter is +constructed from the message sequence number and flags that appeared +in the unencrypted portion of the received message, and the connection +polarity. + +To guard against replay attacks, the receiver maintains some state in +which it remembers the highest message sequence number seen. It could +simply reject messages with lower sequence numbers, but that could +result in excessive message loss when messages are re-ordered. The +receiver therefore additionally maintains a set of received message +sequence numbers in a window below the highest number seen, and only +rejects messages with a sequence number below that window, or +contained in the set. The window spans at least 2^20 message sequence +numbers, and hence any re-ordering between the most recent ~1 million +messages is handled without dropping messages. ### Further reading More details on the inner workings of weave can be found in the