wgengine/filter: only log packets to/from non-default routes.

Fixes tailscale/corp#1429.

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

ipn/ipnlocal/local.go

@@ -117,8 +117,8 @@ func NewLocalBackend(logf logger.Logf, logid string, store ipn.StateStore, e wge
 		panic("ipn.NewLocalBackend: wgengine must not be nil")
 	}
 
-	// Default filter blocks everything, until Start() is called.
-	e.SetFilter(filter.NewAllowNone(logf))
+	// Default filter blocks everything and logs nothing, until Start() is called.
+	e.SetFilter(filter.NewAllowNone(logf, &netaddr.IPSet{}))
 
 	ctx, cancel := context.WithCancel(context.Background())
 	portpoll, err := portlist.NewPoller()
@@ -605,8 +605,13 @@ func (b *LocalBackend) updateFilter(netMap *netmap.NetworkMap, prefs *ipn.Prefs)
 		addrs        []netaddr.IPPrefix
 		packetFilter []filter.Match
 		localNetsB   netaddr.IPSetBuilder
+		logNetsB     netaddr.IPSetBuilder
 		shieldsUp    = prefs == nil || prefs.ShieldsUp // Be conservative when not ready
 	)
+	// Log traffic for Tailscale IPs.
+	logNetsB.AddPrefix(tsaddr.CGNATRange())
+	logNetsB.AddPrefix(tsaddr.TailscaleULARange())
+	logNetsB.RemovePrefix(tsaddr.ChromeOSVMRange())
 	if haveNetmap {
 		addrs = netMap.Addresses
 		for _, p := range addrs {
@@ -631,29 +636,34 @@ func (b *LocalBackend) updateFilter(netMap *netmap.NetworkMap, prefs *ipn.Prefs)
 				localNetsB.AddSet(s)
 			} else {
 				localNetsB.AddPrefix(r)
+				// When advertising a non-default route, we assume
+				// this is a corporate subnet that should be present
+				// in the audit logs.
+				logNetsB.AddPrefix(r)
 			}
 		}
 	}
 	localNets := localNetsB.IPSet()
+	logNets := logNetsB.IPSet()
 
-	changed := deepprint.UpdateHash(&b.filterHash, haveNetmap, addrs, packetFilter, localNets.Ranges(), shieldsUp)
+	changed := deepprint.UpdateHash(&b.filterHash, haveNetmap, addrs, packetFilter, localNets.Ranges(), logNets.Ranges(), shieldsUp)
 	if !changed {
 		return
 	}
 
 	if !haveNetmap {
 		b.logf("netmap packet filter: (not ready yet)")
-		b.e.SetFilter(filter.NewAllowNone(b.logf))
+		b.e.SetFilter(filter.NewAllowNone(b.logf, logNets))
 		return
 	}
 
 	oldFilter := b.e.GetFilter()
 	if shieldsUp {
 		b.logf("netmap packet filter: (shields up)")
-		b.e.SetFilter(filter.NewShieldsUpFilter(localNets, oldFilter, b.logf))
+		b.e.SetFilter(filter.NewShieldsUpFilter(localNets, logNets, oldFilter, b.logf))
 	} else {
 		b.logf("netmap packet filter: %v", packetFilter)
-		b.e.SetFilter(filter.New(packetFilter, localNets, oldFilter, b.logf))
+		b.e.SetFilter(filter.New(packetFilter, localNets, logNets, oldFilter, b.logf))
 	}
 }
 

net/packet/packet.go

@@ -116,6 +116,12 @@ func (q *Parsed) Decode(b []byte) {
 	}
 }
 
+// StuffForTesting makes Parsed contain a len-bytes buffer. Used in
+// tests to build up a synthetic parse result with a non-zero buffer.
+func (q *Parsed) StuffForTesting(len int) {
+	q.b = make([]byte, len)
+}
+
 func (q *Parsed) decode4(b []byte) {
 	if len(b) < ip4HeaderLength {
 		q.IPVersion = 0

wgengine/filter/filter.go

@@ -25,6 +25,10 @@ type Filter struct {
 	// destination within local, regardless of the policy filter
 	// below.
 	local *netaddr.IPSet
+	// logIPs is the set of IPs that are allowed to appear in flow
+	// logs. If a packet is to or from an IP not in logIPs, it will
+	// never be logged.
+	logIPs *netaddr.IPSet
 	// matches4 and matches6 are lists of match->action rules
 	// applied to all packets arriving over tailscale
 	// tunnels. Matches are checked in order, and processing stops
@@ -125,24 +129,24 @@ func NewAllowAllForTest(logf logger.Logf) *Filter {
 	var sb netaddr.IPSetBuilder
 	sb.AddPrefix(any4)
 	sb.AddPrefix(any6)
-	return New(ms, sb.IPSet(), nil, logf)
+	return New(ms, sb.IPSet(), sb.IPSet(), nil, logf)
 }
 
 // NewAllowNone returns a packet filter that rejects everything.
-func NewAllowNone(logf logger.Logf) *Filter {
-	return New(nil, &netaddr.IPSet{}, nil, logf)
+func NewAllowNone(logf logger.Logf, logIPs *netaddr.IPSet) *Filter {
+	return New(nil, &netaddr.IPSet{}, logIPs, nil, logf)
 }
 
 // NewShieldsUpFilter returns a packet filter that rejects incoming connections.
 //
 // If shareStateWith is non-nil, the returned filter shares state with the previous one,
 // as long as the previous one was also a shields up filter.
-func NewShieldsUpFilter(localNets *netaddr.IPSet, shareStateWith *Filter, logf logger.Logf) *Filter {
+func NewShieldsUpFilter(localNets *netaddr.IPSet, logIPs *netaddr.IPSet, shareStateWith *Filter, logf logger.Logf) *Filter {
 	// Don't permit sharing state with a prior filter that wasn't a shields-up filter.
 	if shareStateWith != nil && !shareStateWith.shieldsUp {
 		shareStateWith = nil
 	}
-	f := New(nil, localNets, shareStateWith, logf)
+	f := New(nil, localNets, logIPs, shareStateWith, logf)
 	f.shieldsUp = true
 	return f
 }
@@ -152,7 +156,7 @@ func NewShieldsUpFilter(localNets *netaddr.IPSet, shareStateWith *Filter, logf l
 // by matches. If shareStateWith is non-nil, the returned filter
 // shares state with the previous one, to enable changing rules at
 // runtime without breaking existing stateful flows.
-func New(matches []Match, localNets *netaddr.IPSet, shareStateWith *Filter, logf logger.Logf) *Filter {
+func New(matches []Match, localNets *netaddr.IPSet, logIPs *netaddr.IPSet, shareStateWith *Filter, logf logger.Logf) *Filter {
 	var state *filterState
 	if shareStateWith != nil {
 		state = shareStateWith.state
@@ -166,6 +170,7 @@ func New(matches []Match, localNets *netaddr.IPSet, shareStateWith *Filter, logf
 		matches4: matchesFamily(matches, netaddr.IP.Is4),
 		matches6: matchesFamily(matches, netaddr.IP.Is6),
 		local:    localNets,
+		logIPs:   logIPs,
 		state:    state,
 	}
 	return f
@@ -210,12 +215,15 @@ var acceptBucket = rate.NewLimiter(rate.Every(10*time.Second), 3)
 var dropBucket = rate.NewLimiter(rate.Every(5*time.Second), 10)
 
 func (f *Filter) logRateLimit(runflags RunFlags, q *packet.Parsed, dir direction, r Response, why string) {
-	var verdict string
+	if !f.loggingAllowed(q) {
+		return
+	}
 
 	if r == Drop && omitDropLogging(q, dir) {
 		return
 	}
 
+	var verdict string
 	if r == Drop && (runflags&LogDrops) != 0 && dropBucket.Allow() {
 		verdict = "Drop"
 		runflags &= HexdumpDrops
@@ -491,6 +499,11 @@ func (f *Filter) pre(q *packet.Parsed, rf RunFlags, dir direction) Response {
 	return noVerdict
 }
 
+// loggingAllowed reports whether p can appear in logs at all.
+func (f *Filter) loggingAllowed(p *packet.Parsed) bool {
+	return f.logIPs.Contains(p.Src.IP) && f.logIPs.Contains(p.Dst.IP)
+}
+
 // omitDropLogging reports whether packet p, which has already been
 // deemed a packet to Drop, should bypass the [rate-limited] logging.
 // We don't want to log scary & spammy reject warnings for packets

wgengine/filter/filter_test.go

@@ -12,8 +12,10 @@ import (
 	"testing"
 
 	"github.com/google/go-cmp/cmp"
+	"golang.org/x/time/rate"
 	"inet.af/netaddr"
 	"tailscale.com/net/packet"
+	"tailscale.com/net/tsaddr"
 	"tailscale.com/types/logger"
 )
 
@@ -36,7 +38,9 @@ func newFilter(logf logger.Logf) *Filter {
 		localNets.AddPrefix(n)
 	}
 
-	return New(matches, localNets.IPSet(), nil, logf)
+	var logB netaddr.IPSetBuilder
+	logB.Complement()
+	return New(matches, localNets.IPSet(), logB.IPSet(), nil, logf)
 }
 
 func TestFilter(t *testing.T) {
@@ -298,7 +302,7 @@ func TestPreFilter(t *testing.T) {
 		{"udp", noVerdict, raw4default(packet.UDP, 0)},
 		{"icmp", noVerdict, raw4default(packet.ICMPv4, 0)},
 	}
-	f := NewAllowNone(t.Logf)
+	f := NewAllowNone(t.Logf, &netaddr.IPSet{})
 	for _, testPacket := range packets {
 		p := &packet.Parsed{}
 		p.Decode(testPacket.b)
@@ -376,6 +380,138 @@ func TestOmitDropLogging(t *testing.T) {
 	}
 }
 
+func TestLoggingPrivacy(t *testing.T) {
+	oldDrop := dropBucket
+	oldAccept := acceptBucket
+	dropBucket = rate.NewLimiter(2^32, 2^32)
+	acceptBucket = dropBucket
+	defer func() {
+		dropBucket = oldDrop
+		acceptBucket = oldAccept
+	}()
+
+	var (
+		logged     bool
+		testLogger logger.Logf
+	)
+	logf := func(format string, args ...interface{}) {
+		testLogger(format, args...)
+		logged = true
+	}
+
+	var logB netaddr.IPSetBuilder
+	logB.AddPrefix(netaddr.MustParseIPPrefix("100.64.0.0/10"))
+	logB.AddPrefix(tsaddr.TailscaleULARange())
+	f := newFilter(logf)
+	f.logIPs = logB.IPSet()
+
+	var (
+		ts4       = netaddr.IPPort{IP: tsaddr.CGNATRange().IP.Next(), Port: 1234}
+		internet4 = netaddr.IPPort{IP: netaddr.MustParseIP("8.8.8.8"), Port: 1234}
+		ts6       = netaddr.IPPort{IP: tsaddr.TailscaleULARange().IP.Next(), Port: 1234}
+		internet6 = netaddr.IPPort{IP: netaddr.MustParseIP("2001::1"), Port: 1234}
+	)
+
+	tests := []struct {
+		name   string
+		pkt    *packet.Parsed
+		dir    direction
+		logged bool
+	}{
+		{
+			name:   "ts_to_ts_v4_out",
+			pkt:    &packet.Parsed{IPVersion: 4, IPProto: packet.TCP, Src: ts4, Dst: ts4},
+			dir:    out,
+			logged: true,
+		},
+		{
+			name:   "ts_to_internet_v4_out",
+			pkt:    &packet.Parsed{IPVersion: 4, IPProto: packet.TCP, Src: ts4, Dst: internet4},
+			dir:    out,
+			logged: false,
+		},
+		{
+			name:   "internet_to_ts_v4_out",
+			pkt:    &packet.Parsed{IPVersion: 4, IPProto: packet.TCP, Src: internet4, Dst: ts4},
+			dir:    out,
+			logged: false,
+		},
+		{
+			name:   "ts_to_ts_v4_in",
+			pkt:    &packet.Parsed{IPVersion: 4, IPProto: packet.TCP, Src: ts4, Dst: ts4},
+			dir:    in,
+			logged: true,
+		},
+		{
+			name:   "ts_to_internet_v4_in",
+			pkt:    &packet.Parsed{IPVersion: 4, IPProto: packet.TCP, Src: ts4, Dst: internet4},
+			dir:    in,
+			logged: false,
+		},
+		{
+			name:   "internet_to_ts_v4_in",
+			pkt:    &packet.Parsed{IPVersion: 4, IPProto: packet.TCP, Src: internet4, Dst: ts4},
+			dir:    in,
+			logged: false,
+		},
+		{
+			name:   "ts_to_ts_v6_out",
+			pkt:    &packet.Parsed{IPVersion: 6, IPProto: packet.TCP, Src: ts6, Dst: ts6},
+			dir:    out,
+			logged: true,
+		},
+		{
+			name:   "ts_to_internet_v6_out",
+			pkt:    &packet.Parsed{IPVersion: 6, IPProto: packet.TCP, Src: ts6, Dst: internet6},
+			dir:    out,
+			logged: false,
+		},
+		{
+			name:   "internet_to_ts_v6_out",
+			pkt:    &packet.Parsed{IPVersion: 6, IPProto: packet.TCP, Src: internet6, Dst: ts6},
+			dir:    out,
+			logged: false,
+		},
+		{
+			name:   "ts_to_ts_v6_in",
+			pkt:    &packet.Parsed{IPVersion: 6, IPProto: packet.TCP, Src: ts6, Dst: ts6},
+			dir:    in,
+			logged: true,
+		},
+		{
+			name:   "ts_to_internet_v6_in",
+			pkt:    &packet.Parsed{IPVersion: 6, IPProto: packet.TCP, Src: ts6, Dst: internet6},
+			dir:    in,
+			logged: false,
+		},
+		{
+			name:   "internet_to_ts_v6_in",
+			pkt:    &packet.Parsed{IPVersion: 6, IPProto: packet.TCP, Src: internet6, Dst: ts6},
+			dir:    in,
+			logged: false,
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			test.pkt.StuffForTesting(1024)
+			logged = false
+			testLogger = t.Logf
+			switch test.dir {
+			case out:
+				f.RunOut(test.pkt, LogDrops|LogAccepts)
+			case in:
+				f.RunIn(test.pkt, LogDrops|LogAccepts)
+			default:
+				panic("unknown direction")
+			}
+			if logged != test.logged {
+				t.Errorf("logged = %v, want %v", logged, test.logged)
+			}
+		})
+	}
+}
+
 func mustIP(s string) netaddr.IP {
 	ip, err := netaddr.ParseIP(s)
 	if err != nil {

wgengine/tstun/tun_test.go

@@ -112,7 +112,7 @@ func setfilter(logf logger.Logf, tun *TUN) {
 	}
 	var sb netaddr.IPSetBuilder
 	sb.AddPrefix(netaddr.MustParseIPPrefix("1.2.0.0/16"))
-	tun.SetFilter(filter.New(matches, sb.IPSet(), nil, logf))
+	tun.SetFilter(filter.New(matches, sb.IPSet(), sb.IPSet(), nil, logf))
 }
 
 func newChannelTUN(logf logger.Logf, secure bool) (*tuntest.ChannelTUN, *TUN) {