From e8b3a5e7a1528b1db04c348baa305757b865204e Mon Sep 17 00:00:00 2001 From: David Anderson Date: Fri, 22 May 2020 02:41:18 +0000 Subject: [PATCH] wgengine/filter: implement a destination IP pre-filter. Signed-off-by: David Anderson --- ipn/local.go | 51 +++++++++++++++++++++------- wgengine/filter/filter.go | 51 ++++++++++++++++++++-------- wgengine/filter/filter_test.go | 19 ++++++++--- wgengine/magicsock/magicsock_test.go | 4 +-- 4 files changed, 93 insertions(+), 32 deletions(-) diff --git a/ipn/local.go b/ipn/local.go index 73c67e1a2..1a4e091f4 100644 --- a/ipn/local.go +++ b/ipn/local.go @@ -391,26 +391,36 @@ func (b *LocalBackend) Start(opts Options) error { // updateFilter updates the packet filter in wgengine based on the // given netMap and user preferences. func (b *LocalBackend) updateFilter(netMap *controlclient.NetworkMap) { - // TODO(apenwarr): don't replace filter at all if unchanged. - // TODO(apenwarr): print a diff instead of full filter. if netMap == nil { // Not configured yet, block everything b.logf("netmap packet filter: (not ready yet)") b.e.SetFilter(filter.NewAllowNone(b.logf)) - } else if b.shieldsAreUp() { + return + } + + b.mu.Lock() + advRoutes := b.prefs.AdvertiseRoutes + b.mu.Unlock() + localNets := wgCIDRsToFilter(netMap.Addresses, advRoutes) + + if b.shieldsAreUp() { // Shields up, block everything b.logf("netmap packet filter: (shields up)") - b.e.SetFilter(filter.NewAllowNone(b.logf)) + var prevFilter *filter.Filter // don't reuse old filter state + b.e.SetFilter(filter.New(filter.Matches{}, localNets, prevFilter, b.logf)) + return + } + + // TODO(apenwarr): don't replace filter at all if unchanged. + // TODO(apenwarr): print a diff instead of full filter. + now := time.Now() + if now.Sub(b.lastFilterPrint) > 1*time.Minute { + b.logf("netmap packet filter: %v", netMap.PacketFilter) + b.lastFilterPrint = now } else { - now := time.Now() - if now.Sub(b.lastFilterPrint) > 1*time.Minute { - b.logf("netmap packet filter: %v", netMap.PacketFilter) - b.lastFilterPrint = now - } else { - b.logf("netmap packet filter: (length %d)", len(netMap.PacketFilter)) - } - b.e.SetFilter(filter.New(netMap.PacketFilter, b.e.GetFilter(), b.logf)) + b.logf("netmap packet filter: (length %d)", len(netMap.PacketFilter)) } + b.e.SetFilter(filter.New(netMap.PacketFilter, localNets, b.e.GetFilter(), b.logf)) } // readPoller is a goroutine that receives service lists from @@ -788,6 +798,23 @@ func routerConfig(cfg *wgcfg.Config, prefs *Prefs, dnsDomains []string) *router. return rs } +// wgCIDRsToFilter converts lists of wgcfg.CIDR into a single list of +// filter.Net. +func wgCIDRsToFilter(cidrLists ...[]wgcfg.CIDR) (ret []filter.Net) { + for _, cidrs := range cidrLists { + for _, cidr := range cidrs { + if !cidr.IP.Is4() { + continue + } + ret = append(ret, filter.Net{ + IP: filter.NewIP(cidr.IP.IP()), + Mask: filter.Netmask(int(cidr.Mask)), + }) + } + } + return ret +} + func wgIPToNetaddr(ips []wgcfg.IP) (ret []netaddr.IP) { for _, ip := range ips { nip, ok := netaddr.FromStdIP(ip.IP()) diff --git a/wgengine/filter/filter.go b/wgengine/filter/filter.go index 300dc8c35..d775d2763 100644 --- a/wgengine/filter/filter.go +++ b/wgengine/filter/filter.go @@ -23,9 +23,22 @@ type filterState struct { // Filter is a stateful packet filter. type Filter struct { - logf logger.Logf + logf logger.Logf + // localNets is the list of IP prefixes that we know to be "local" + // to this node. All packets coming in over tailscale must have a + // destination within localNets, regardless of the policy filter + // below. A nil localNets rejects all incoming traffic. + localNets []Net + // matches is a list of match->action rules applied to all packets + // arriving over tailscale tunnels. Matches are checked in order, + // and processing stops at the first matching rule. The default + // policy if no rules match is to drop the packet. matches Matches - state *filterState + // state is the connection tracking state attached to this + // filter. It is used to allow incoming traffic that is a response + // to an outbound connection that this node made, even if those + // incoming packets don't get accepted by matches above. + state *filterState } // Response is a verdict: either a Drop, Accept, or noVerdict skip to @@ -75,21 +88,23 @@ var MatchAllowAll = Matches{ Match{[]NetPortRange{NetPortRangeAny}, []Net{NetAny}}, } -// NewAllowAll returns a packet filter that accepts everything. -func NewAllowAll(logf logger.Logf) *Filter { - return New(MatchAllowAll, nil, logf) +// NewAllowAll returns a packet filter that accepts everything to and +// from localNets. +func NewAllowAll(localNets []Net, logf logger.Logf) *Filter { + return New(MatchAllowAll, localNets, nil, logf) } // NewAllowNone returns a packet filter that rejects everything. func NewAllowNone(logf logger.Logf) *Filter { - return New(nil, nil, logf) + return New(nil, nil, nil, logf) } -// New creates a new packet Filter with the given Matches rules. -// If shareStateWith is non-nil, the returned filter shares state -// with the previous one, to enable rules to be changed at runtime -// without breaking existing flows. -func New(matches Matches, shareStateWith *Filter, logf logger.Logf) *Filter { +// New creates a new packet filter. The filter enforces that incoming +// packets must be destined to an IP in localNets, and must be allowed +// by matches. If shareStateWith is non-nil, the returned filter +// shares state with the previous one, to enable rules to be changed +// at runtime without breaking existing flows. +func New(matches Matches, localNets []Net, shareStateWith *Filter, logf logger.Logf) *Filter { var state *filterState if shareStateWith != nil { state = shareStateWith.state @@ -99,9 +114,10 @@ func New(matches Matches, shareStateWith *Filter, logf logger.Logf) *Filter { } } f := &Filter{ - logf: logf, - matches: matches, - state: state, + logf: logf, + matches: matches, + localNets: localNets, + state: state, } return f } @@ -159,6 +175,13 @@ func (f *Filter) RunOut(b []byte, q *packet.QDecode, rf RunFlags) Response { } func (f *Filter) runIn(q *packet.QDecode) (r Response, why string) { + // A compromised peer could try to send us packets for + // destinations we didn't explicitly advertise. This check is to + // prevent that. + if !ipInList(q.DstIP, f.localNets) { + return Drop, "destination not allowed" + } + switch q.IPProto { case packet.ICMP: if q.IsEchoResponse() || q.IsError() { diff --git a/wgengine/filter/filter_test.go b/wgengine/filter/filter_test.go index 8a2b25756..72d0d5bf1 100644 --- a/wgengine/filter/filter_test.go +++ b/wgengine/filter/filter_test.go @@ -55,7 +55,12 @@ func TestFilter(t *testing.T) { {Srcs: []Net{NetAny}, Dsts: netpr(0, 0, 443, 443)}, {Srcs: nets([]IP{0x99010101, 0x99010102, 0x99030303}), Dsts: ippr(0x01020304, 999, 999)}, } - acl := New(mm, nil, t.Logf) + // Expects traffic to 100.122.98.50, 1.2.3.4, 5.6.7.8, + // 102.102.102.102, 119.119.119.119, 8.1.0.0/16 + localNets := nets([]IP{0x647a6232, 0x01020304, 0x05060708, 0x66666666, 0x77777777}) + localNets = append(localNets, Net{IP(0x08010000), Netmask(16)}) + + acl := New(mm, localNets, nil, t.Logf) for _, ent := range []Matches{Matches{mm[0]}, mm} { b, err := json.Marshal(ent) @@ -83,12 +88,18 @@ func TestFilter(t *testing.T) { {Drop, qdecode(TCP, 0x08010101, 0x01020304, 0, 0)}, {Accept, qdecode(TCP, 0x08010101, 0x01020304, 0, 22)}, {Drop, qdecode(TCP, 0x08010101, 0x01020304, 0, 21)}, - {Accept, qdecode(TCP, 0x11223344, 0x22334455, 0, 443)}, - {Drop, qdecode(TCP, 0x11223344, 0x22334455, 0, 444)}, + {Accept, qdecode(TCP, 0x11223344, 0x08012233, 0, 443)}, + {Drop, qdecode(TCP, 0x11223344, 0x08012233, 0, 444)}, {Accept, qdecode(TCP, 0x11223344, 0x647a6232, 0, 999)}, {Accept, qdecode(TCP, 0x11223344, 0x647a6232, 0, 0)}, - // Stateful UDP. + // localNets prefilter - accepted by policy filter, but + // unexpected dst IP. + {Drop, qdecode(TCP, 0x08010101, 0x10203040, 0, 443)}, + + // Stateful UDP. Note each packet is run through the input + // filter, then the output filter (which sets conntrack + // state). // Initially empty cache {Drop, qdecode(UDP, 0x77777777, 0x66666666, 4242, 4343)}, // Return packet from previous attempt is allowed diff --git a/wgengine/magicsock/magicsock_test.go b/wgengine/magicsock/magicsock_test.go index d365e1a03..9b7b30aac 100644 --- a/wgengine/magicsock/magicsock_test.go +++ b/wgengine/magicsock/magicsock_test.go @@ -397,7 +397,7 @@ func TestTwoDevicePing(t *testing.T) { tun1 := tuntest.NewChannelTUN() tstun1 := tstun.WrapTUN(logf, tun1.TUN()) - tstun1.SetFilter(filter.NewAllowAll(logf)) + tstun1.SetFilter(filter.NewAllowAll([]filter.Net{filter.NetAny}, logf)) dev1 := device.NewDevice(tstun1, &device.DeviceOptions{ Logger: devLogger(t, "dev1", logf), CreateEndpoint: conn1.CreateEndpoint, @@ -412,7 +412,7 @@ func TestTwoDevicePing(t *testing.T) { tun2 := tuntest.NewChannelTUN() tstun2 := tstun.WrapTUN(logf, tun2.TUN()) - tstun2.SetFilter(filter.NewAllowAll(logf)) + tstun2.SetFilter(filter.NewAllowAll([]filter.Net{filter.NetAny}, logf)) dev2 := device.NewDevice(tstun2, &device.DeviceOptions{ Logger: devLogger(t, "dev2", logf), CreateEndpoint: conn2.CreateEndpoint,