From 247ebaaf191f2588870019fcbc8237aa70224c82 Mon Sep 17 00:00:00 2001 From: Matthias Radestock Date: Sat, 17 Jan 2015 18:45:19 +0000 Subject: [PATCH] ignore heartbeats from the 'wrong' connection The router may receive heartbeat packets that originated from (the other end of) a different connection with the remote peer than the current one. If we don't detect that, we may erroneously tell the remote peer to mark the connection as 'established'. Which can lead to a crash. To enable detection, we include the connection UID - which is the same at both ends - in the heartbeat frame. That way we can simply ignore heartbeats with a different connection UID than the current connection to the remote peer. Previously heartbeat frames were zero length. In order to include the 64 bit connection UID we could just send 8 octets instead. However, that would potentially trip up the Ethernet decoder at the other end, and in any case we might as well make heartbeats fit the format of other special frames, namely having an all-zero Ethernet header. This in fact results in a simplification of the special frame handling, since heartbeats are no longer *extra* special. Fixes #352. --- router/connection.go | 8 +++++--- router/consts.go | 2 +- router/router.go | 17 ++++++++++------- 3 files changed, 16 insertions(+), 11 deletions(-) diff --git a/router/connection.go b/router/connection.go index 1923621b..3a13a845 100644 --- a/router/connection.go +++ b/router/connection.go @@ -139,8 +139,8 @@ func (conn *LocalConnection) Shutdown(err error) { // Heartbeating serves two purposes: a) keeping NAT paths alive, and // b) updating a remote peer's knowledge of our address, in the event // it changes (e.g. because NAT paths expired). -func (conn *LocalConnection) ReceivedHeartbeat(remoteUDPAddr *net.UDPAddr) { - if remoteUDPAddr == nil { +func (conn *LocalConnection) ReceivedHeartbeat(remoteUDPAddr *net.UDPAddr, connUID uint64) { + if remoteUDPAddr == nil || connUID != conn.UID { return } conn.queryChan <- &ConnectionInteraction{ @@ -176,10 +176,12 @@ func (conn *LocalConnection) run(queryChan <-chan *ConnectionInteraction, accept } log.Printf("->[%s] completed handshake with %s\n", conn.remoteTCPAddr, conn.remote.Name) + heartbeatFrameBytes := make([]byte, EthernetOverhead + 8) + binary.BigEndian.PutUint64(heartbeatFrameBytes[EthernetOverhead:], conn.UID) conn.heartbeatFrame = &ForwardedFrame{ srcPeer: conn.local, dstPeer: conn.remote, - frame: []byte{}} + frame: heartbeatFrameBytes} go conn.receiveTCP(dec) conn.Router.Ourself.AddConnection(conn) diff --git a/router/consts.go b/router/consts.go index 5bdd14f8..84baddd0 100644 --- a/router/consts.go +++ b/router/consts.go @@ -7,7 +7,7 @@ import ( const ( Protocol = "weave" - ProtocolVersion = 9 + ProtocolVersion = 10 EthernetOverhead = 14 UDPOverhead = 28 // 20 bytes for IPv4, 8 bytes for UDP Port = 6783 diff --git a/router/router.go b/router/router.go index c89b168b..aa0f05d7 100644 --- a/router/router.go +++ b/router/router.go @@ -247,7 +247,9 @@ func (router *Router) handleUDPPacketFunc(dec *EthernetDecoder, po PacketSink) F dec.DecodeLayers(frame) decodedLen := len(dec.decoded) - + if decodedLen == 0 { + return nil + } // Handle special frames produced internally (rather than // captured/forwarded) by the remote router. // @@ -256,18 +258,19 @@ func (router *Router) handleUDPPacketFunc(dec *EthernetDecoder, po PacketSink) F // efficient to do so, as we want to optimise for the common // (i.e. non-special) frames. These always need decoding, and // detecting special frames is cheaper post decoding than pre. - if decodedLen == 0 || (decodedLen == 1 && dec.IsSpecial()) { + if decodedLen == 1 && dec.IsSpecial() { if srcPeer != relayConn.Remote() || dstPeer != router.Ourself.Peer { // A special frame not originating from the remote, or // not for us? How odd; let's just drop it. return nil } - if frameLen == 0 { - relayConn.ReceivedHeartbeat(sender) - } else if frameLen == FragTestSize && bytes.Equal(frame, FragTest) { + switch { + case frameLen == EthernetOverhead + 8: + relayConn.ReceivedHeartbeat(sender, binary.BigEndian.Uint64(frame[EthernetOverhead:])) + case frameLen == FragTestSize && bytes.Equal(frame, FragTest): relayConn.SendTCP(ProtocolFragmentationReceivedByte) - } else if frameLen == PMTUDiscoverySize && bytes.Equal(frame, PMTUDiscovery) { - } else if decodedLen == 1 { + case frameLen == PMTUDiscoverySize && bytes.Equal(frame, PMTUDiscovery): + default: frameLenBytes := []byte{0, 0} binary.BigEndian.PutUint16(frameLenBytes, uint16(frameLen-EthernetOverhead)) relayConn.SendTCP(Concat(ProtocolPMTUVerifiedByte, frameLenBytes))