wgengine/magicsock: pass src NodeKey to handleDiscoMessage for DERP disco msgs

And then use it to avoid another lookup-by-DiscoKey.

Updates #3088

(cherry picked from commit 3e80806804)
This commit is contained in:
Brad Fitzpatrick 2021-10-15 16:42:24 -07:00
parent 7b7ff1f2e4
commit e921482548
2 changed files with 26 additions and 11 deletions

View File

@ -1393,7 +1393,7 @@ func (c *Conn) setPeerLastDerpLocked(peer key.Public, regionID, homeID int) {
type derpReadResult struct { type derpReadResult struct {
regionID int regionID int
n int // length of data received n int // length of data received
src key.Public // may be zero until server deployment if v2+ src key.Public
// copyBuf is called to copy the data to dst. It returns how // copyBuf is called to copy the data to dst. It returns how
// much data was copied, which will be n if dst is large // much data was copied, which will be n if dst is large
// enough. copyBuf can only be called once. // enough. copyBuf can only be called once.
@ -1596,7 +1596,7 @@ func (c *Conn) receiveIP(b []byte, ipp netaddr.IPPort, cache *ippEndpointCache)
c.stunReceiveFunc.Load().(func([]byte, netaddr.IPPort))(b, ipp) c.stunReceiveFunc.Load().(func([]byte, netaddr.IPPort))(b, ipp)
return nil, false return nil, false
} }
if c.handleDiscoMessage(b, ipp) { if c.handleDiscoMessage(b, ipp, key.Public{}) {
return nil, false return nil, false
} }
if !c.havePrivateKey.Get() { if !c.havePrivateKey.Get() {
@ -1659,7 +1659,7 @@ func (c *Conn) processDERPReadResult(dm derpReadResult, b []byte) (n int, ep *en
} }
ipp := netaddr.IPPortFrom(derpMagicIPAddr, uint16(regionID)) ipp := netaddr.IPPortFrom(derpMagicIPAddr, uint16(regionID))
if c.handleDiscoMessage(b[:n], ipp) { if c.handleDiscoMessage(b[:n], ipp, dm.src) {
return 0, nil return 0, nil
} }
@ -1732,9 +1732,11 @@ func (c *Conn) sendDiscoMessage(dst netaddr.IPPort, dstKey tailcfg.NodeKey, dstD
// * nonce [24]byte // * nonce [24]byte
// * naclbox of payload (see tailscale.com/disco package for inner payload format) // * naclbox of payload (see tailscale.com/disco package for inner payload format)
// //
// For messages received over DERP, the addr will be derpMagicIP (with // For messages received over DERP, the src.IP() will be derpMagicIP (with
// port being the region) // src.Port() being the region ID) and the derpNodeSrc will be the node key
func (c *Conn) handleDiscoMessage(msg []byte, src netaddr.IPPort) (isDiscoMsg bool) { // it was received from at the DERP layer. derpNodeSrc is zero when received
// over UDP.
func (c *Conn) handleDiscoMessage(msg []byte, src netaddr.IPPort, derpNodeSrc key.Public) (isDiscoMsg bool) {
const headerLen = len(disco.Magic) + len(tailcfg.DiscoKey{}) + disco.NonceLen const headerLen = len(disco.Magic) + len(tailcfg.DiscoKey{}) + disco.NonceLen
if len(msg) < headerLen || string(msg[:len(disco.Magic)]) != disco.Magic { if len(msg) < headerLen || string(msg[:len(disco.Magic)]) != disco.Magic {
return false return false
@ -1828,11 +1830,19 @@ func (c *Conn) handleDiscoMessage(msg []byte, src netaddr.IPPort) (isDiscoMsg bo
case *disco.Pong: case *disco.Pong:
ep.handlePongConnLocked(dm, src) ep.handlePongConnLocked(dm, src)
case *disco.CallMeMaybe: case *disco.CallMeMaybe:
if src.IP() != derpMagicIPAddr { if src.IP() != derpMagicIPAddr || derpNodeSrc.IsZero() {
// CallMeMaybe messages should only come via DERP. // CallMeMaybe messages should only come via DERP.
c.logf("[unexpected] CallMeMaybe packets should only come via DERP") c.logf("[unexpected] CallMeMaybe packets should only come via DERP")
return return
} }
ep, ok := c.peerMap.endpointForNodeKey(tailcfg.NodeKey(derpNodeSrc))
if !ok {
c.logf("magicsock: disco: ignoring CallMeMaybe from %v; %v is unknown", sender.ShortString(), derpNodeSrc.ShortString())
return
}
if !ep.canP2P() {
return
}
c.logf("[v1] magicsock: disco: %v<-%v (%v, %v) got call-me-maybe, %d endpoints", c.logf("[v1] magicsock: disco: %v<-%v (%v, %v) got call-me-maybe, %d endpoints",
c.discoShort, ep.discoShort, c.discoShort, ep.discoShort,
ep.publicKey.ShortString(), derpStr(src.String()), ep.publicKey.ShortString(), derpStr(src.String()),

View File

@ -1158,7 +1158,7 @@ func TestDiscoMessage(t *testing.T) {
pkt = append(pkt, nonce[:]...) pkt = append(pkt, nonce[:]...)
pkt = box.Seal(pkt, []byte(payload), &nonce, c.discoPrivate.Public().B32(), peer1Priv.B32()) pkt = box.Seal(pkt, []byte(payload), &nonce, c.discoPrivate.Public().B32(), peer1Priv.B32())
got := c.handleDiscoMessage(pkt, netaddr.IPPort{}) got := c.handleDiscoMessage(pkt, netaddr.IPPort{}, key.Public{})
if !got { if !got {
t.Error("failed to open it") t.Error("failed to open it")
} }
@ -1434,15 +1434,20 @@ func TestSetNetworkMapChangingNodeKey(t *testing.T) {
}) })
} }
de, ok := conn.peerMap.endpointForDiscoKey(discoKey) de, ok := conn.peerMap.endpointForNodeKey(nodeKey2)
if ok && de.publicKey != nodeKey2 { if ok && de.publicKey != nodeKey2 {
t.Fatalf("discoEndpoint public key = %q; want %q", de.publicKey[:], nodeKey2[:]) t.Fatalf("discoEndpoint public key = %q; want %q", de.publicKey[:], nodeKey2[:])
} }
if de.discoKey != discoKey {
t.Errorf("discoKey = %v; want %v", de.discoKey, discoKey)
}
if _, ok := conn.peerMap.endpointForNodeKey(nodeKey1); ok {
t.Errorf("didn't expect to find node for key1")
}
log := buf.String() log := buf.String()
wantSub := map[string]int{ wantSub := map[string]int{
"magicsock: got updated network map; 1 peers": 2, "magicsock: got updated network map; 1 peers": 2,
"magicsock: disco key discokey:0000000000000000000000000000000000000000000000000000000000000001 changed from node key [TksxA] to [TksyA]": 1,
} }
for sub, want := range wantSub { for sub, want := range wantSub {
got := strings.Count(log, sub) got := strings.Count(log, sub)