diff --git a/net/packet/packet.go b/net/packet/packet.go index 001e469ba..5ec015ef3 100644 --- a/net/packet/packet.go +++ b/net/packet/packet.go @@ -211,9 +211,11 @@ func (q *Parsed) decode4(b []byte) { // Inter-tailscale messages. q.dataofs = q.subofs return - default: + case ipproto.Fragment: + // An IPProto value of 0xff (our Fragment constant for internal use) + // should never actually be used in the wild; if we see it, + // something's suspicious and we map it back to zero (unknown). q.IPProto = unknown - return } } else { // This is a fragment other than the first one. @@ -312,7 +314,10 @@ func (q *Parsed) decode6(b []byte) { // Inter-tailscale messages. q.dataofs = q.subofs return - default: + case ipproto.Fragment: + // An IPProto value of 0xff (our Fragment constant for internal use) + // should never actually be used in the wild; if we see it, + // something's suspicious and we map it back to zero (unknown). q.IPProto = unknown return } diff --git a/net/packet/packet_test.go b/net/packet/packet_test.go index 9931e8ceb..fadbf97ab 100644 --- a/net/packet/packet_test.go +++ b/net/packet/packet_test.go @@ -6,12 +6,16 @@ import ( "bytes" + "encoding/hex" "net/netip" "reflect" + "strings" "testing" + "unicode" "tailscale.com/tstest" "tailscale.com/types/ipproto" + "tailscale.com/util/must" ) const ( @@ -440,6 +444,17 @@ func TestParsedString(t *testing.T) { } } +// mustHexDecode is like hex.DecodeString, but panics on error +// and ignores whitespcae in s. +func mustHexDecode(s string) []byte { + return must.Get(hex.DecodeString(strings.Map(func(r rune) rune { + if unicode.IsSpace(r) { + return -1 + } + return r + }, s))) +} + func TestDecode(t *testing.T) { tests := []struct { name string @@ -459,6 +474,29 @@ func TestDecode(t *testing.T) { {"ipv4_sctp", sctpBuffer, sctpDecode}, {"ipv4_frag", tcp4MediumFragmentBuffer, tcp4MediumFragmentDecode}, {"ipv4_fragtooshort", tcp4ShortFragmentBuffer, tcp4ShortFragmentDecode}, + + {"ip97", mustHexDecode("4500 0019 d186 4000 4061 751d 644a 4603 6449 e549 6865 6c6c 6f"), Parsed{ + IPVersion: 4, + IPProto: 97, + Src: netip.MustParseAddrPort("100.74.70.3:0"), + Dst: netip.MustParseAddrPort("100.73.229.73:0"), + b: mustHexDecode("4500 0019 d186 4000 4061 751d 644a 4603 6449 e549 6865 6c6c 6f"), + length: 25, + subofs: 20, + }}, + + // This packet purports to use protocol 0xFF, which is verboten and + // used internally as a sentinel value for fragments. So test that + // we map packets using 0xFF to Unknown (0) instead. + {"bogus_proto_ff", mustHexDecode("4500 0019 d186 4000 40" + "FF" /* bogus FF */ + " 751d 644a 4603 6449 e549 6865 6c6c 6f"), Parsed{ + IPVersion: 4, + IPProto: ipproto.Unknown, // 0, not bogus 0xFF + Src: netip.MustParseAddrPort("100.74.70.3:0"), + Dst: netip.MustParseAddrPort("100.73.229.73:0"), + b: mustHexDecode("4500 0019 d186 4000 40" + "FF" /* bogus FF */ + " 751d 644a 4603 6449 e549 6865 6c6c 6f"), + length: 25, + subofs: 20, + }}, } for _, tt := range tests { diff --git a/net/tstun/wrap_test.go b/net/tstun/wrap_test.go index 1c5b005ce..41b8be32b 100644 --- a/net/tstun/wrap_test.go +++ b/net/tstun/wrap_test.go @@ -8,11 +8,13 @@ "bytes" "context" "encoding/binary" + "encoding/hex" "fmt" "net/netip" "strconv" "strings" "testing" + "unicode" "unsafe" "github.com/google/go-cmp/cmp" @@ -30,6 +32,7 @@ "tailscale.com/types/key" "tailscale.com/types/logger" "tailscale.com/types/netlogtype" + "tailscale.com/util/must" "tailscale.com/wgengine/filter" ) @@ -293,6 +296,17 @@ func TestWriteAndInject(t *testing.T) { } } +// mustHexDecode is like hex.DecodeString, but panics on error +// and ignores whitespcae in s. +func mustHexDecode(s string) []byte { + return must.Get(hex.DecodeString(strings.Map(func(r rune) rune { + if unicode.IsSpace(r) { + return -1 + } + return r + }, s))) +} + func TestFilter(t *testing.T) { chtun, tun := newChannelTUN(t.Logf, true) defer tun.Close() @@ -310,8 +324,9 @@ func TestFilter(t *testing.T) { drop bool data []byte }{ - {"junk_in", in, true, []byte("\x45not a valid IPv4 packet")}, - {"junk_out", out, true, []byte("\x45not a valid IPv4 packet")}, + {"short_in", in, true, []byte("\x45xxx")}, + {"short_out", out, true, []byte("\x45xxx")}, + {"ip97_out", out, false, mustHexDecode("4500 0019 d186 4000 4061 751d 644a 4603 6449 e549 6865 6c6c 6f")}, {"bad_port_in", in, true, udp4("5.6.7.8", "1.2.3.4", 22, 22)}, {"bad_port_out", out, false, udp4("1.2.3.4", "5.6.7.8", 22, 22)}, {"bad_ip_in", in, true, udp4("8.1.1.1", "1.2.3.4", 89, 89)}, @@ -386,9 +401,11 @@ func TestFilter(t *testing.T) { got, _ := stats.TestExtract() want := map[netlogtype.Connection]netlogtype.Counts{} + var wasUDP bool if !tt.drop { var p packet.Parsed p.Decode(tt.data) + wasUDP = p.IPProto == ipproto.UDP switch tt.dir { case in: conn := netlogtype.Connection{Proto: ipproto.UDP, Src: p.Dst, Dst: p.Src} @@ -398,8 +415,10 @@ func TestFilter(t *testing.T) { want[conn] = netlogtype.Counts{TxPackets: 1, TxBytes: uint64(len(tt.data))} } } - if diff := cmp.Diff(got, want, cmpopts.EquateEmpty()); diff != "" { - t.Errorf("stats.TestExtract (-got +want):\n%s", diff) + if wasUDP { + if diff := cmp.Diff(got, want, cmpopts.EquateEmpty()); diff != "" { + t.Errorf("stats.TestExtract (-got +want):\n%s", diff) + } } }) } @@ -525,6 +544,7 @@ func TestPeerAPIBypass(t *testing.T) { p.Decode(tt.pkt) tt.w.SetFilter(tt.filter) tt.w.disableTSMPRejected = true + tt.w.logf = t.Logf if got := tt.w.filterIn(p); got != tt.want { t.Errorf("got = %v; want %v", got, tt.want) } diff --git a/wgengine/filter/filter.go b/wgengine/filter/filter.go index f9520b8b2..f573a2d07 100644 --- a/wgengine/filter/filter.go +++ b/wgengine/filter/filter.go @@ -452,7 +452,7 @@ func (f *Filter) runIn4(q *packet.Parsed) (r Response, why string) { return Accept, "tsmp ok" default: if f.matches4.matchProtoAndIPsOnlyIfAllPorts(q) { - return Accept, "otherproto ok" + return Accept, "other-portless ok" } return Drop, unknownProtoString(q.IPProto) } @@ -512,7 +512,7 @@ func (f *Filter) runIn6(q *packet.Parsed) (r Response, why string) { return Accept, "tsmp ok" default: if f.matches6.matchProtoAndIPsOnlyIfAllPorts(q) { - return Accept, "otherproto ok" + return Accept, "other-portless ok" } return Drop, unknownProtoString(q.IPProto) } @@ -577,12 +577,7 @@ func (f *Filter) pre(q *packet.Parsed, rf RunFlags, dir direction) Response { return Drop } - switch q.IPProto { - case ipproto.Unknown: - // Unknown packets are dangerous; always drop them. - f.logRateLimit(rf, q, dir, Drop, "unknown") - return Drop - case ipproto.Fragment: + if q.IPProto == ipproto.Fragment { // Fragments after the first always need to be passed through. // Very small fragments are considered Junk by Parsed. f.logRateLimit(rf, q, dir, Accept, "fragment")