wgengine/filter: don't spam logs on dropped outgoing IPv6 ICMP or IPv4 IGMP

The OS (tries) to send these but we drop them. No need to worry the
user with spam that we're dropping it.

Fixes #402

Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>

wgengine/filter/filter.go

@@ -136,9 +136,13 @@ func maybeHexdump(flag RunFlags, b []byte) string {
 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.ParsedPacket, r Response, why string) {
+func (f *Filter) logRateLimit(runflags RunFlags, q *packet.ParsedPacket, dir direction, r Response, why string) {
 	var verdict string
 
+	if r == Drop && f.omitDropLogging(q, dir) {
+		return
+	}
+
 	if r == Drop && (runflags&LogDrops) != 0 && dropBucket.Allow() {
 		verdict = "Drop"
 		runflags &= HexdumpDrops
@@ -157,26 +161,28 @@ func (f *Filter) logRateLimit(runflags RunFlags, q *packet.ParsedPacket, r Respo
 
 // RunIn determines whether this node is allowed to receive q from a Tailscale peer.
 func (f *Filter) RunIn(q *packet.ParsedPacket, rf RunFlags) Response {
-	r := f.pre(q, rf)
+	dir := in
+	r := f.pre(q, rf, dir)
 	if r == Accept || r == Drop {
 		// already logged
 		return r
 	}
 
 	r, why := f.runIn(q)
-	f.logRateLimit(rf, q, r, why)
+	f.logRateLimit(rf, q, dir, r, why)
 	return r
 }
 
 // RunOut determines whether this node is allowed to send q to a Tailscale peer.
 func (f *Filter) RunOut(q *packet.ParsedPacket, rf RunFlags) Response {
-	r := f.pre(q, rf)
+	dir := out
+	r := f.pre(q, rf, dir)
 	if r == Drop || r == Accept {
 		// already logged
 		return r
 	}
 	r, why := f.runOut(q)
-	f.logRateLimit(rf, q, r, why)
+	f.logRateLimit(rf, q, dir, r, why)
 	return r
 }
 
@@ -252,33 +258,78 @@ func (f *Filter) runOut(q *packet.ParsedPacket) (r Response, why string) {
 	return Accept, "ok out"
 }
 
-func (f *Filter) pre(q *packet.ParsedPacket, rf RunFlags) Response {
+// direction is whether a packet was flowing in to this machine, or flowing out.
+type direction int
+
+const (
+	in direction = iota
+	out
+)
+
+func (f *Filter) pre(q *packet.ParsedPacket, rf RunFlags, dir direction) Response {
 	if len(q.Buffer()) == 0 {
 		// wireguard keepalive packet, always permit.
 		return Accept
 	}
 	if len(q.Buffer()) < 20 {
-		f.logRateLimit(rf, q, Drop, "too short")
+		f.logRateLimit(rf, q, dir, Drop, "too short")
 		return Drop
 	}
 
 	if q.IPVersion == 6 {
-		// TODO(bradfitz): don't log about normal broadcast
+		f.logRateLimit(rf, q, dir, Drop, "ipv6")
-		// IPv6 traffic like route announcements.
-		f.logRateLimit(rf, q, Drop, "ipv6")
 		return Drop
 	}
 	switch q.IPProto {
 	case packet.Unknown:
 		// Unknown packets are dangerous; always drop them.
-		f.logRateLimit(rf, q, Drop, "unknown")
+		f.logRateLimit(rf, q, dir, Drop, "unknown")
 		return Drop
 	case packet.Fragment:
 		// Fragments after the first always need to be passed through.
 		// Very small fragments are considered Junk by ParsedPacket.
-		f.logRateLimit(rf, q, Accept, "fragment")
+		f.logRateLimit(rf, q, dir, Accept, "fragment")
 		return Accept
 	}
 
 	return noVerdict
 }
+
+// ipv6AllRoutersLinkLocal is ff02::2 (All link-local routers).
+const ipv6AllRoutersLinkLocal = "\xff\x02\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x02"
+
+// omitDropLogging reports whether packet p, which has already been
+// deemded a packet to Drop, should bypass the [rate-limited] logging.
+// We don't want to log scary & spammy reject warnings for packets that
+// are totally normal, like IPv6 route announcements.
+func (f *Filter) omitDropLogging(p *packet.ParsedPacket, dir direction) bool {
+	switch dir {
+	case out:
+		switch p.IPVersion {
+		case 4:
+			// Omit logging about outgoing IGMP queries being dropped.
+			if p.IPProto == packet.IGMP {
+				return true
+			}
+		case 6:
+			b := p.Buffer()
+			if len(b) < 40 {
+				return false
+			}
+			src, dst := b[8:8+16], b[24:24+16]
+			// Omit logging for outgoing IPv6 ICMP-v6 queries to ff02::2,
+			// as sent by the OS, looking for routers.
+			if p.IPProto == packet.ICMPv6 {
+				if isLinkLocalV6(src) && string(dst) == ipv6AllRoutersLinkLocal {
+					return true
+				}
+			}
+		}
+	}
+	return false
+}
+
+// isLinkLocalV6 reports whether src is in fe80::/10.
+func isLinkLocalV6(src []byte) bool {
+	return len(src) == 16 && src[0] == 0xfe && src[1]>>6 == 0x80>>6
+}

wgengine/filter/filter_test.go

@@ -219,7 +219,7 @@ func TestPreFilter(t *testing.T) {
 	for _, testPacket := range packets {
 		p := &ParsedPacket{}
 		p.Decode(testPacket.b)
-		got := f.pre(p, LogDrops|LogAccepts)
+		got := f.pre(p, LogDrops|LogAccepts, in)
 		if got != testPacket.want {
 			t.Errorf("%q got=%v want=%v packet:\n%s", testPacket.desc, got, testPacket.want, packet.Hexdump(testPacket.b))
 		}

wgengine/packet/ip.go

@@ -47,6 +47,7 @@ const (
 	// Unknown represents an unknown or unsupported protocol; it's deliberately the zero value.
 	Unknown IPProto = 0x00
 	ICMP    IPProto = 0x01
+	IGMP    IPProto = 0x02
 	ICMPv6  IPProto = 0x3a
 	TCP     IPProto = 0x06
 	UDP     IPProto = 0x11