derp: protocol negotiation, add v2: send src pub keys to clients in packets

Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>

derp/derp.go

@@ -38,6 +38,15 @@ const (
 	keepAlive  = 60 * time.Second
 )
 
+// protocolVersion is bumped whenever there's a wire-incompatible change.
+//   * version 1 (zero on wire): consistent box headers, in use by employee dev nodes a bit
+//   * version 2: received packets have src addrs in frameRecvPacket at beginning
+const protocolVersion = 2
+
+const (
+	protocolSrcAddrs = 2 // protocol version at which client expects src addresses
+)
+
 // frameType is the one byte frame type at the beginning of the frame
 // header.  The second field is a big-endian uint32 describing the
 // length of the remaining frame (not including the initial 5 bytes).
@@ -62,7 +71,7 @@ const (
 	frameClientInfo = frameType(0x02) // 32B pub key + 24B nonce + naclbox(json)
 	frameServerInfo = frameType(0x03) // 24B nonce + naclbox(json)
 	frameSendPacket = frameType(0x04) // 32B dest pub key + packet bytes
-	frameRecvPacket = frameType(0x05) // packet bytes
+	frameRecvPacket = frameType(0x05) // v0/1: packet bytes, v2: 32B src pub key + packet bytes
 	frameKeepAlive  = frameType(0x06) // no payload, no-op (to be replaced with ping/pong)
 )
 
@@ -155,3 +164,10 @@ func writeFrame(bw *bufio.Writer, t frameType, b []byte) error {
 	}
 	return bw.Flush()
 }
+
+func minInt(a, b int) int {
+	if a < b {
+		return a
+	}
+	return b
+}

derp/derp_client.go

@@ -23,6 +23,7 @@ type Client struct {
 	serverKey    key.Public // of the DERP server; not a machine or node key
 	privateKey   key.Private
 	publicKey    key.Public // of privateKey
+	protoVersion int        // min of server+client
 	logf         logger.Logf
 	nc           net.Conn
 	br           *bufio.Reader
@@ -46,11 +47,11 @@ func NewClient(privateKey key.Private, nc net.Conn, brw *bufio.ReadWriter, logf
 	if err := c.sendClientKey(); err != nil {
 		return nil, fmt.Errorf("derp.Client: failed to send client key: %v", err)
 	}
-	_, err := c.recvServerInfo()
+	info, err := c.recvServerInfo()
 	if err != nil {
 		return nil, fmt.Errorf("derp.Client: failed to receive server info: %v", err)
 	}
-
+	c.protoVersion = minInt(protocolVersion, info.Version)
 	return c, nil
 }
 
@@ -104,12 +105,19 @@ func (c *Client) recvServerInfo() (*serverInfo, error) {
 	return info, nil
 }
 
+type clientInfo struct {
+	Version int // `json:"version,omitempty"`
+}
+
 func (c *Client) sendClientKey() error {
 	var nonce [nonceLen]byte
 	if _, err := crand.Read(nonce[:]); err != nil {
 		return err
 	}
-	msg := []byte("{}") // no clientInfo for now
+	msg, err := json.Marshal(clientInfo{Version: protocolVersion})
+	if err != nil {
+		return err
+	}
 	msgbox := box.Seal(nil, msg, &nonce, c.serverKey.B32(), c.privateKey.B32())
 
 	buf := make([]byte, 0, nonceLen+keyLen+len(msgbox))
@@ -156,7 +164,12 @@ type ReceivedMessage interface {
 }
 
 // ReceivedPacket is a ReceivedMessage representing an incoming packet.
-type ReceivedPacket []byte
+type ReceivedPacket struct {
+	Source key.Public
+	// Data is the received packet bytes. It aliases the memory
+	// passed to Client.Recv.
+	Data []byte
+}
 
 func (ReceivedPacket) msg() {}
 
@@ -189,7 +202,18 @@ func (c *Client) Recv(b []byte) (m ReceivedMessage, err error) {
 			// require ack pongs.
 			continue
 		case frameRecvPacket:
-			return ReceivedPacket(b[:n]), nil
+			var rp ReceivedPacket
+			if c.protoVersion < protocolSrcAddrs {
+				rp.Data = b[:n]
+			} else {
+				if n < keyLen {
+					c.logf("[unexpected] dropping short packet from DERP server")
+					continue
+				}
+				copy(rp.Source[:], b[:keyLen])
+				rp.Data = b[keyLen:n]
+			}
+			return rp, nil
 		}
 	}
 }

derp/derp_server.go

@@ -237,7 +237,7 @@ func (s *Server) accept(nc net.Conn, brw *bufio.ReadWriter) error {
 		}
 
 		dst.mu.Lock()
-		err = s.sendPacket(dst.bw, c.key, contents)
+		err = s.sendPacket(dst.bw, &dst.info, c.key, contents)
 		dst.mu.Unlock()
 
 		if err != nil {
@@ -260,7 +260,7 @@ func (s *Server) sendClientKeepAlives(ctx context.Context, c *sclient) {
 	}
 }
 
-func (s *Server) verifyClient(clientKey key.Public, info *sclientInfo) error {
+func (s *Server) verifyClient(clientKey key.Public, info *clientInfo) error {
 	// TODO(crawshaw): implement policy constraints on who can use the DERP server
 	// TODO(bradfitz): ... and at what rate.
 	return nil
@@ -273,12 +273,20 @@ func (s *Server) sendServerKey(bw *bufio.Writer) error {
 	return writeFrame(bw, frameServerKey, buf)
 }
 
+type serverInfo struct {
+	Version int // `json:"version,omitempty"`
+}
+
 func (s *Server) sendServerInfo(bw *bufio.Writer, clientKey key.Public) error {
 	var nonce [24]byte
 	if _, err := crand.Read(nonce[:]); err != nil {
 		return err
 	}
-	msg := []byte("{}") // no serverInfo for now
+	msg, err := json.Marshal(serverInfo{Version: protocolVersion})
+	if err != nil {
+		return err
+	}
+
 	msgbox := box.Seal(nil, msg, &nonce, clientKey.B32(), s.privateKey.B32())
 	if err := writeFrameHeader(bw, frameServerInfo, nonceLen+uint32(len(msgbox))); err != nil {
 		return err
@@ -295,7 +303,7 @@ func (s *Server) sendServerInfo(bw *bufio.Writer, clientKey key.Public) error {
 // recvClientKey reads the frameClientInfo frame from the client (its
 // proof of identity) upon its initial connection. It should be
 // considered especially untrusted at this point.
-func (s *Server) recvClientKey(br *bufio.Reader) (clientKey key.Public, info *sclientInfo, err error) {
+func (s *Server) recvClientKey(br *bufio.Reader) (clientKey key.Public, info *clientInfo, err error) {
 	fl, err := readFrameTypeHeader(br, frameClientInfo)
 	if err != nil {
 		return key.Public{}, nil, err
@@ -325,19 +333,32 @@ func (s *Server) recvClientKey(br *bufio.Reader) (clientKey key.Public, info *sc
 	if !ok {
 		return key.Public{}, nil, fmt.Errorf("msgbox: cannot open len=%d with client key %x", msgLen, clientKey[:])
 	}
-	info = new(sclientInfo)
+	info = new(clientInfo)
 	if err := json.Unmarshal(msg, info); err != nil {
 		return key.Public{}, nil, fmt.Errorf("msg: %v", err)
 	}
 	return clientKey, info, nil
 }
 
-func (s *Server) sendPacket(bw *bufio.Writer, srcKey key.Public, contents []byte) error {
+func (s *Server) sendPacket(bw *bufio.Writer, dstInfo *clientInfo, srcKey key.Public, contents []byte) error {
 	s.packetsSent.Add(1)
 	s.bytesSent.Add(int64(len(contents)))
-	if err := writeFrameHeader(bw, frameRecvPacket, uint32(len(contents))); err != nil {
+
+	sendSrc := dstInfo.Version >= protocolSrcAddrs
+
+	pktLen := len(contents)
+	if sendSrc {
+		pktLen += len(srcKey)
+	}
+
+	if err := writeFrameHeader(bw, frameRecvPacket, uint32(pktLen)); err != nil {
+		return err
+	}
+	if sendSrc {
+		if _, err := bw.Write(srcKey[:]); err != nil {
 			return err
 		}
+	}
 	if _, err := bw.Write(contents); err != nil {
 		return err
 	}
@@ -373,7 +394,7 @@ func (s *Server) recvPacket(ctx context.Context, br *bufio.Reader, frameLen uint
 type sclient struct {
 	nc   net.Conn
 	key  key.Public
-	info sclientInfo
+	info clientInfo
 
 	keepAliveTimer *time.Timer
 	keepAliveReset chan struct{}
@@ -417,13 +438,6 @@ func (c *sclient) keepAliveLoop(ctx context.Context) error {
 	}
 }
 
-// sclientInfo is the client info sent by the client to the server.
-type sclientInfo struct {
-}
-
-type serverInfo struct {
-}
-
 func (s *Server) expVarFunc(f func() interface{}) expvar.Func {
 	return expvar.Func(func() interface{} {
 		s.mu.Lock()

derp/derp_test.go

@@ -86,7 +86,10 @@ func TestSendRecv(t *testing.T) {
 					t.Errorf("unexpected message type %T", m)
 					continue
 				case ReceivedPacket:
-					recvChs[i] <- []byte(m)
+					if m.Source.IsZero() {
+						t.Errorf("zero Source address in ReceivedPacket")
+					}
+					recvChs[i] <- m.Data
 				}
 			}
 		}(i)

derp/derphttp/derphttp_test.go

@@ -104,7 +104,7 @@ func TestSendRecv(t *testing.T) {
 					t.Errorf("unexpected message type %T", m)
 					continue
 				case derp.ReceivedPacket:
-					recvChs[i] <- []byte(m)
+					recvChs[i] <- m.Data
 				}
 			}
 		}(i)

wgengine/magicsock/magicsock.go

@@ -631,13 +631,18 @@ func (c *Conn) derpWriteChanOfAddr(addr *net.UDPAddr) chan<- derpWriteRequest {
 
 // derpReadResult is the type sent by runDerpClient to ReceiveIPv4
 // when a DERP packet is available.
+//
+// Notably, it doesn't include the derp.ReceivedPacket because we
+// don't want to give the receiver access to the aliased []byte.  To
+// get at the packet contents they need to call copyBuf to copy it
+// out, which also releases the buffer.
 type derpReadResult struct {
 	derpAddr *net.UDPAddr
 	n        int        // length of data received
-
+	src      key.Public // may be zero until server deployment if v2+
 	// copyBuf is called to copy the data to dst.  It returns how
 	// much data was copied, which will be n if dst is large
-	// enough.
+	// enough. copyBuf can only be called once.
 	copyBuf func(dst []byte) int
 }
 
@@ -648,9 +653,11 @@ var logDerpVerbose, _ = strconv.ParseBool(os.Getenv("DEBUG_DERP_VERBOSE"))
 func (c *Conn) runDerpReader(ctx context.Context, derpFakeAddr *net.UDPAddr, dc *derphttp.Client) {
 	didCopy := make(chan struct{}, 1)
 	var buf [derp.MaxPacketSize]byte
-	var bufValid int // bytes in buf that are valid
+
-	copyFn := func(dst []byte) int {
+	res := derpReadResult{derpAddr: derpFakeAddr}
-		n := copy(dst, buf[:bufValid])
+	var pkt derp.ReceivedPacket
+	res.copyBuf = func(dst []byte) int {
+		n := copy(dst, pkt.Data)
 		didCopy <- struct{}{}
 		return n
 	}
@@ -674,19 +681,21 @@ func (c *Conn) runDerpReader(ctx context.Context, derpFakeAddr *net.UDPAddr, dc
 		}
 		switch m := msg.(type) {
 		case derp.ReceivedPacket:
-			bufValid = len(m)
+			pkt = m
+			res.n = len(m.Data)
+			res.src = m.Source
+			if logDerpVerbose {
+				log.Printf("got derp %v packet: %q", derpFakeAddr, m.Data)
+			}
 		default:
 			// Ignore.
 			// TODO: handle endpoint notification messages.
 			continue
 		}
-		if logDerpVerbose {
-			log.Printf("got derp %v packet: %q", derpFakeAddr, buf[:bufValid])
-		}
 		select {
 		case <-c.donec():
 			return
-		case c.derpRecvCh <- derpReadResult{derpFakeAddr, bufValid, copyFn}:
+		case c.derpRecvCh <- res:
 			<-didCopy
 		}
 	}
@@ -771,6 +780,8 @@ func (c *Conn) ReceiveIPv4(b []byte) (n int, ep conn.Endpoint, addr *net.UDPAddr
 		}
 	}()
 
+	var addrSet *AddrSet
+
 	select {
 	case dm := <-c.derpRecvCh:
 		// Cancel the pconn read goroutine
@@ -797,6 +808,10 @@ func (c *Conn) ReceiveIPv4(b []byte) (n int, ep conn.Endpoint, addr *net.UDPAddr
 			return 0, nil, nil, err
 		}
 
+		// TODO: look up addrSet from dm.Source public key, if
+		// found (Source might be zero for a short period of
+		// time until DERP servers re-deployed)
+
 	case um := <-c.udpRecvCh:
 		if um.err != nil {
 			return 0, nil, nil, err
@@ -804,7 +819,9 @@ func (c *Conn) ReceiveIPv4(b []byte) (n int, ep conn.Endpoint, addr *net.UDPAddr
 		n, addr = um.n, um.addr
 	}
 
-	addrSet := c.findAddrSet(addr)
+	if addrSet == nil {
+		addrSet = c.findAddrSet(addr)
+	}
 	if addrSet == nil {
 		// The peer that sent this packet has roamed beyond the
 		// knowledge provided by the control server.