wgengine/filter: implement a destination IP pre-filter.

Signed-off-by: David Anderson <danderson@tailscale.com>

ipn/local.go

@@ -391,17 +391,28 @@ 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
-	} else {
+		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)
@@ -409,8 +420,7 @@ func (b *LocalBackend) updateFilter(netMap *controlclient.NetworkMap) {
 	} else {
 		b.logf("netmap packet filter: (length %d)", len(netMap.PacketFilter))
 	}
-		b.e.SetFilter(filter.New(netMap.PacketFilter, b.e.GetFilter(), b.logf))
+	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())

wgengine/filter/filter.go

@@ -24,7 +24,20 @@ type filterState struct {
 // Filter is a stateful packet filter.
 type Filter struct {
 	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 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
 }
 
@@ -75,21 +88,23 @@ var MatchAllowAll = Matches{
 	Match{[]NetPortRange{NetPortRangeAny}, []Net{NetAny}},
 }
 
-// NewAllowAll returns a packet filter that accepts everything.
+// NewAllowAll returns a packet filter that accepts everything to and
-func NewAllowAll(logf logger.Logf) *Filter {
+// from localNets.
-	return New(MatchAllowAll, nil, logf)
+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.
+// New creates a new packet filter. The filter enforces that incoming
-// If shareStateWith is non-nil, the returned filter shares state
+// packets must be destined to an IP in localNets, and must be allowed
-// with the previous one, to enable rules to be changed at runtime
+// by matches. If shareStateWith is non-nil, the returned filter
-// without breaking existing flows.
+// shares state with the previous one, to enable rules to be changed
-func New(matches Matches, shareStateWith *Filter, logf logger.Logf) *Filter {
+// 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
@@ -101,6 +116,7 @@ func New(matches Matches, shareStateWith *Filter, logf logger.Logf) *Filter {
 	f := &Filter{
 		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() {

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)},
+		{Accept, qdecode(TCP, 0x11223344, 0x08012233, 0, 443)},
-		{Drop, qdecode(TCP, 0x11223344, 0x22334455, 0, 444)},
+		{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

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,