disco: support parsing/encoding endpoints in call-me-maybe frames

Updates #1172

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

disco/disco.go

@@ -70,7 +70,7 @@ func Parse(p []byte) (Message, error) {
 	case TypePong:
 		return parsePong(ver, p)
 	case TypeCallMeMaybe:
-		return CallMeMaybe{}, nil
+		return parseCallMeMaybe(ver, p)
 	default:
 		return nil, fmt.Errorf("unknown message type 0x%02x", byte(t))
 	}
@@ -122,13 +122,49 @@ func parsePing(ver uint8, p []byte) (m *Ping, err error) {
 //
 // The recipient may choose to not open a path back, if it's already
 // happy with its path. But usually it will.
-type CallMeMaybe struct{}
-
-func (CallMeMaybe) AppendMarshal(b []byte) []byte {
-	ret, _ := appendMsgHeader(b, TypeCallMeMaybe, v0, 0)
+type CallMeMaybe struct {
+	// MyNumber is what the peer believes its endpoints are.
+	// Tailscale clients before 1.4 did not populate this
+	// so these values should merely augment whetever the control
+	// server sends. But because the client could've been idle
+	// before it reached out to us, the control plane might
+	// have stale info and these endpoints in CallMeMaybe
+	// might contain the just-obtained-milliseconds-ago
+	// STUN endpoint.
+	MyNumber []netaddr.IPPort
+}
+
+const epLength = 16 + 2 // 16 byte IP address + 2 byte port
+
+func (m *CallMeMaybe) AppendMarshal(b []byte) []byte {
+	ret, p := appendMsgHeader(b, TypeCallMeMaybe, v0, epLength*len(m.MyNumber))
+	for _, ipp := range m.MyNumber {
+		a := ipp.IP.As16()
+		copy(p[:], a[:])
+		binary.BigEndian.PutUint16(p[16:], ipp.Port)
+		p = p[epLength:]
+	}
 	return ret
 }
 
+func parseCallMeMaybe(ver uint8, p []byte) (m *CallMeMaybe, err error) {
+	m = new(CallMeMaybe)
+	if len(p)%epLength != 0 || ver != 0 || len(p) == 0 {
+		return m, nil
+	}
+	m.MyNumber = make([]netaddr.IPPort, 0, len(p)/epLength)
+	for len(p) > 0 {
+		var a [16]byte
+		copy(a[:], p)
+		m.MyNumber = append(m.MyNumber, netaddr.IPPort{
+			IP:   netaddr.IPFrom16(a),
+			Port: binary.BigEndian.Uint16(p[16:18]),
+		})
+		p = p[epLength:]
+	}
+	return m, nil
+}
+
 // Pong is a response a Ping.
 //
 // It includes the sender's source IP + port, so it's effectively a
@@ -171,7 +207,7 @@ func MessageSummary(m Message) string {
 		return fmt.Sprintf("ping tx=%x", m.TxID[:6])
 	case *Pong:
 		return fmt.Sprintf("pong tx=%x", m.TxID[:6])
-	case CallMeMaybe:
+	case *CallMeMaybe:
 		return "call-me-maybe"
 	default:
 		return fmt.Sprintf("%#v", m)

disco/disco_test.go

@@ -44,9 +44,19 @@ func TestMarshalAndParse(t *testing.T) {
 		},
 		{
 			name: "call_me_maybe",
-			m:    CallMeMaybe{},
+			m:    &CallMeMaybe{},
 			want: "03 00",
 		},
+		{
+			name: "call_me_maybe_endpoints",
+			m: &CallMeMaybe{
+				MyNumber: []netaddr.IPPort{
+					netaddr.MustParseIPPort("1.2.3.4:567"),
+					netaddr.MustParseIPPort("[2001::3456]:789"),
+				},
+			},
+			want: "03 00 00 00 00 00 00 00 00 00 00 00 ff ff 01 02 03 04 02 37 20 01 00 00 00 00 00 00 00 00 00 00 00 00 34 56 03 15",
+		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {

wgengine/magicsock/magicsock.go

@@ -1843,7 +1843,7 @@ func (c *Conn) handleDiscoMessage(msg []byte, src netaddr.IPPort) bool {
 			return true
 		}
 		de.handlePongConnLocked(dm, src)
-	case disco.CallMeMaybe:
+	case *disco.CallMeMaybe:
 		if src.IP != derpMagicIPAddr {
 			// CallMeMaybe messages should only come via DERP.
 			c.logf("[unexpected] CallMeMaybe packets should only come via DERP")
@@ -3241,7 +3241,7 @@ func (de *discoEndpoint) sendPingsLocked(now time.Time, sendCallMeMaybe bool) {
 		// so our firewall ports are probably open and now would be a good time
 		// for them to connect.
 		time.AfterFunc(5*time.Millisecond, func() {
-			de.sendDiscoMessage(derpAddr, disco.CallMeMaybe{}, discoLog)
+			de.sendDiscoMessage(derpAddr, &disco.CallMeMaybe{}, discoLog)
 		})
 	}
 }