While the code was correct, I broke it during a refactoring and
tests didn't detect it. This fixes that glitch.
Signed-off-by: David Anderson <danderson@tailscale.com>
Doesn't materially affect benchmarks, but shrinks match6 by 30 instructions
and halves memory loads.
Part of #19.
Signed-off-by: David Anderson <danderson@tailscale.com>
Part of #19.
name old time/op new time/op delta
Filter/icmp4-8 32.2ns ± 3% 32.5ns ± 2% ~ (p=0.524 n=10+8)
Filter/icmp6-8 49.7ns ± 6% 43.1ns ± 4% -13.12% (p=0.000 n=9+10)
Signed-off-by: David Anderson <danderson@tailscale.com>
The packet filter still rejects all IPv6, but decodes enough from v6
packets to do something smarter in a followup.
name time/op
Decode/tcp4-8 28.8ns ± 2%
Decode/tcp6-8 20.6ns ± 1%
Decode/udp4-8 28.2ns ± 1%
Decode/udp6-8 20.0ns ± 6%
Decode/icmp4-8 21.7ns ± 2%
Decode/icmp6-8 14.1ns ± 2%
Decode/unknown-8 9.43ns ± 2%
Signed-off-by: David Anderson <danderson@tailscale.com>
The goal is to move some of the shenanigans we have elsewhere into the filter
package, so that all the weird things to do with poking at the filter is in
a single place, behind clean APIs.
Signed-off-by: David Anderson <danderson@tailscale.com>
We still use the packet.* alloc-free types in the data path, but
the compilation from netaddr to packet happens within the filter
package.
Signed-off-by: David Anderson <danderson@tailscale.com>
control/controlclient, wgengine/filter: extract parsePacketFilter to new constructor in wgengine/filter
Signed-off-by: chungdaniel <daniel@tailscale.com>
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>
If a test calls log.Printf, 'go test' horrifyingly rearranges the
output to no longer be in chronological order, which makes debugging
virtually impossible. Let's stop that from happening by making
log.Printf panic if called from any module, no matter how deep, during
tests.
This required us to change the default error handler in at least one
http.Server, as well as plumbing a bunch of logf functions around,
especially in magicsock and wgengine, but also in logtail and backoff.
To add insult to injury, 'go test' also rearranges the output when a
parent test has multiple sub-tests (all the sub-test's t.Logf is always
printed after all the parent tests t.Logf), so we need to screw around
with a special Logf that can point at the "current" t (current_t.Logf)
in some places. Probably our entire way of using subtests is wrong,
since 'go test' would probably like to run them all in parallel if you
called t.Parallel(), but it definitely can't because the're all
manipulating the shared state created by the parent test. They should
probably all be separate toplevel tests instead, with common
setup/teardown logic. But that's a job for another time.
Signed-off-by: Avery Pennarun <apenwarr@tailscale.com>
This depends on improved support from the control server, to send the
new subnet width (Bits) fields. If these are missing, we fall back to
assuming their value is /32.
Conversely, if the server sends Bits fields to an older client, it will
interpret them as /32 addresses. Since the only rules we allow are
"accept" rules, this will be narrower or equal to the intended rule, so
older clients will simply reject hosts on the wider subnet (fail
closed).
With this change, the internal filter.Matches format has diverged
from the wire format used by controlclient, so move the wire format
into tailcfg and convert it to filter.Matches in controlclient.
Signed-off-by: Avery Pennarun <apenwarr@tailscale.com>
Longer term, we should probably update the packet filter to be fully
stateful, for both TCP and ICMP. That is, only ICMP packets related to
a session *we* initiated should be allowed back in. But this is
reasonably secure for now, since wireguard is already trimming most
traffic. The current code would not protect against eg. Ping-of-Death style
attacks from VPN nodes.
Fixestailscale/tailscale#290.
Signed-off-by: Avery Pennarun <apenwarr@tailscale.com>
This avoids a non-obvious data race, where the JSON decoder ends
up creating do-nothing writes into global variables.
==================
WARNING: DATA RACE
Write at 0x0000011e1860 by goroutine 201:
tailscale.com/wgengine/packet.(*IP).UnmarshalJSON()
/home/crawshaw/repo/corp/oss/wgengine/packet/packet.go:83 +0x2d9
encoding/json.(*decodeState).literalStore()
/home/crawshaw/go/go/src/encoding/json/decode.go:877 +0x445e
...
encoding/json.Unmarshal()
/home/crawshaw/go/go/src/encoding/json/decode.go:107 +0x1de
tailscale.com/control/controlclient.(*Direct).decodeMsg()
/home/crawshaw/repo/corp/oss/control/controlclient/direct.go:615 +0x1ab
tailscale.com/control/controlclient.(*Direct).PollNetMap()
/home/crawshaw/repo/corp/oss/control/controlclient/direct.go:525 +0x1053
tailscale.com/control/controlclient.(*Client).mapRoutine()
/home/crawshaw/repo/corp/oss/control/controlclient/auto.go:428 +0x3a6
Previous read at 0x0000011e1860 by goroutine 86:
tailscale.com/wgengine/filter.matchIPWithoutPorts()
/home/crawshaw/repo/corp/oss/wgengine/filter/match.go:108 +0x91
tailscale.com/wgengine/filter.(*Filter).runIn()
/home/crawshaw/repo/corp/oss/wgengine/filter/filter.go:147 +0x3c6
tailscale.com/wgengine/filter.(*Filter).RunIn()
/home/crawshaw/repo/corp/oss/wgengine/filter/filter.go:127 +0xb0
tailscale.com/wgengine.(*userspaceEngine).SetFilter.func1()
/home/crawshaw/repo/corp/oss/wgengine/userspace.go:390 +0xfc
github.com/tailscale/wireguard-go/device.(*Device).RoutineDecryption()
/home/crawshaw/repo/corp/wireguard-go/device/receive.go:295 +0xa1f
For #112
Signed-off-by: David Crawshaw <crawshaw@tailscale.com>
