diff --git a/disco/disco.go b/disco/disco.go index 837784fa1..d91db6a0d 100644 --- a/disco/disco.go +++ b/disco/disco.go @@ -57,6 +57,16 @@ func LooksLikeDiscoWrapper(p []byte) bool { return string(p[:len(Magic)]) == Magic } +// Source returns the slice of p that represents the +// disco public key source, and whether p looks like +// a disco message. +func Source(p []byte) (src []byte, ok bool) { + if !LooksLikeDiscoWrapper(p) { + return nil, false + } + return p[len(Magic):][:keyLen], true +} + // Parse parses the encrypted part of the message from inside the // nacl secretbox. func Parse(p []byte) (Message, error) { diff --git a/net/tstun/wrap.go b/net/tstun/wrap.go index e6e5eed74..0b3c551c0 100644 --- a/net/tstun/wrap.go +++ b/net/tstun/wrap.go @@ -7,6 +7,7 @@ package tstun import ( + "bytes" "errors" "fmt" "io" @@ -19,7 +20,9 @@ import ( "golang.zx2c4.com/wireguard/device" "golang.zx2c4.com/wireguard/tun" "inet.af/netaddr" + "tailscale.com/disco" "tailscale.com/net/packet" + "tailscale.com/tailcfg" "tailscale.com/tstime/mono" "tailscale.com/types/ipproto" "tailscale.com/types/logger" @@ -76,6 +79,7 @@ type Wrapper struct { destIPActivity atomic.Value // of map[netaddr.IP]func() destMACAtomic atomic.Value // of [6]byte + discoKey atomic.Value // of tailcfg.DiscoKey // buffer stores the oldest unconsumed packet from tdev. // It is made a static buffer in order to avoid allocations. @@ -196,6 +200,30 @@ func (t *Wrapper) SetDestIPActivityFuncs(m map[netaddr.IP]func()) { t.destIPActivity.Store(m) } +// SetDiscoKey sets the current discovery key. +// +// It is only used for filtering out bogus traffic when network +// stack(s) get confused; see Issue 1526. +func (t *Wrapper) SetDiscoKey(k tailcfg.DiscoKey) { + t.discoKey.Store(k) +} + +// isSelfDisco reports whether packet p +// looks like a Disco packet from ourselves. +// See Issue 1526. +func (t *Wrapper) isSelfDisco(p *packet.Parsed) bool { + if p.IPProto != ipproto.UDP { + return false + } + pkt := p.Payload() + discoSrc, ok := disco.Source(pkt) + if !ok { + return false + } + selfDiscoPub, ok := t.discoKey.Load().(tailcfg.DiscoKey) + return ok && bytes.Equal(selfDiscoPub[:], discoSrc) +} + func (t *Wrapper) Close() error { var err error t.closeOnce.Do(func() { @@ -483,6 +511,16 @@ func (t *Wrapper) filterIn(buf []byte) filter.Response { } } + // Issue 1526 workaround: if we see disco packets over + // Tailscale from ourselves, then drop them, as that shouldn't + // happen unless a networking stack is confused, as it seems + // macOS in Network Extension mode might be. + if p.IPProto == ipproto.UDP && // disco is over UDP; avoid isSelfDisco call for TCP/etc + t.isSelfDisco(p) { + t.logf("[unexpected] received self disco package over tstun; dropping") + return filter.DropSilently + } + if t.PreFilterIn != nil { if res := t.PreFilterIn(p, t); res.IsDrop() { return res diff --git a/net/tstun/wrap_test.go b/net/tstun/wrap_test.go index 5b69c008c..d83278f97 100644 --- a/net/tstun/wrap_test.go +++ b/net/tstun/wrap_test.go @@ -15,7 +15,10 @@ import ( "golang.zx2c4.com/wireguard/tun/tuntest" "inet.af/netaddr" + "tailscale.com/disco" "tailscale.com/net/packet" + "tailscale.com/tailcfg" + "tailscale.com/tstest" "tailscale.com/tstime/mono" "tailscale.com/types/ipproto" "tailscale.com/types/logger" @@ -486,3 +489,32 @@ func TestPeerAPIBypass(t *testing.T) { }) } } + +// Issue 1526: drop disco frames from ourselves. +func TestFilterDiscoLoop(t *testing.T) { + var memLog tstest.MemLogger + discoPub := tailcfg.DiscoKey{1: 1, 2: 2} + tw := &Wrapper{logf: memLog.Logf} + tw.SetDiscoKey(discoPub) + uh := packet.UDP4Header{ + IP4Header: packet.IP4Header{ + IPProto: ipproto.UDP, + Src: netaddr.IPv4(1, 2, 3, 4), + Dst: netaddr.IPv4(5, 6, 7, 8), + }, + SrcPort: 9, + DstPort: 10, + } + discoPayload := fmt.Sprintf("%s%s%s", disco.Magic, discoPub[:], [disco.NonceLen]byte{}) + pkt := make([]byte, uh.Len()+len(discoPayload)) + uh.Marshal(pkt) + copy(pkt[uh.Len():], discoPayload) + + got := tw.filterIn(pkt) + if got != filter.DropSilently { + t.Errorf("got %v; want DropSilently", got) + } + if got, want := memLog.String(), "[unexpected] received self disco package over tstun; dropping\n"; got != want { + t.Errorf("log output mismatch\n got: %q\nwant: %q\n", got, want) + } +} diff --git a/wgengine/userspace.go b/wgengine/userspace.go index 8d2627e03..e2786be42 100644 --- a/wgengine/userspace.go +++ b/wgengine/userspace.go @@ -332,6 +332,8 @@ func NewUserspaceEngine(logf logger.Logf, conf Config) (_ Engine, reterr error) closePool.add(e.magicConn) e.magicConn.SetNetworkUp(e.linkMon.InterfaceState().AnyInterfaceUp()) + tsTUNDev.SetDiscoKey(e.magicConn.DiscoPublicKey()) + if conf.RespondToPing { e.tundev.PostFilterIn = echoRespondToAll }