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

@@ -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)

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

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))