Commit Graph

70 Commits (b040094b908cff76f63d2f81b93397706e2e4605)

Author SHA1 Message Date
Mihai Parparita 7330aa593e all: avoid repeated default interface lookups
On some platforms (notably macOS and iOS) we look up the default
interface to bind outgoing connections to. This is both duplicated
work and results in logspam when the default interface is not available
(i.e. when a phone has no connectivity, we log an error and thus cause
more things that we will try to upload and fail).

Fixed by passing around a netmon.Monitor to more places, so that we can
use its cached interface state.

Fixes #7850
Updates #7621

Signed-off-by: Mihai Parparita <mihai@tailscale.com>
2 years ago
Andrew Dunham 280255acae
various: add golangci-lint, fix issues (#7905)
This adds an initial and intentionally minimal configuration for
golang-ci, fixes the issues reported, and adds a GitHub Action to check
new pull requests against this linter configuration.

Signed-off-by: Andrew Dunham <andrew@du.nham.ca>
Change-Id: I8f38fbc315836a19a094d0d3e986758b9313f163
2 years ago
Brad Fitzpatrick 10f1c90f4d wgengine/magicsock, types/nettype, etc: finish ReadFromUDPAddrPort netip migration
So we're staying within the netip.Addr/AddrPort consistently and
avoiding allocs/conversions to the legacy net addr types.

Updates #5162

Change-Id: I59feba60d3de39f773e68292d759766bac98c917
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
2 years ago
Mihai Parparita edb02b63f8 net/sockstats: pass in logger to sockstats.WithSockStats
Using log.Printf may end up being printed out to the console, which
is not desirable. I noticed this when I was investigating some client
logs with `sockstats: trace "NetcheckClient" was overwritten by another`.
That turns to be harmless/expected (the netcheck client will fall back
to the DERP client in some cases, which does its own sockstats trace).

However, the log output could be visible to users if running the
`tailscale netcheck` CLI command, which would be needlessly confusing.

Updates tailscale/corp#9230

Signed-off-by: Mihai Parparita <mihai@tailscale.com>
2 years ago
Mihai Parparita 6ac6ddbb47 sockstats: switch label to enum
Makes it cheaper/simpler to persist values, and encourages reuse of
labels as opposed to generating an arbitrary number.

Updates tailscale/corp#9230
Updates #3363

Signed-off-by: Mihai Parparita <mihai@tailscale.com>
2 years ago
Andrew Dunham f6cd24499b net/portmapper: relax source port check for UPnP responses
Per a packet capture provided, some gateways will reply to a UPnP
discovery packet with a UDP packet with a source port that does not come
from the UPnP port. Accept these packets with a log message.

Updates #7377

Signed-off-by: Andrew Dunham <andrew@du.nham.ca>
Change-Id: I5d4d5b2a0275009ed60f15c20b484fe2025d094b
2 years ago
Andrew Dunham 51eb0b2cb7 net/portmapper: send UPnP protocol in upper-case
We were previously sending a lower-case "udp" protocol, whereas other
implementations like miniupnp send an upper-case "UDP" protocol. For
compatibility, use an upper-case protocol instead.

Updates #7377

Signed-off-by: Andrew Dunham <andrew@du.nham.ca>
Change-Id: I4aed204f94e4d51b7a256d29917af1536cb1b70f
2 years ago
Andrew Dunham d379a25ae4 net/portmapper: don't pick external ports below 1024
Some devices don't let you UPnP portmap a port below 1024, so let's just
avoid that range of ports entirely.

Updates #7377

Signed-off-by: Andrew Dunham <andrew@du.nham.ca>
Change-Id: Ib7603b1c9a019162cdc4fa21744a2cae48bb1d86
2 years ago
Andrew Dunham 3f8e8b04fd cmd/tailscale, cmd/tailscaled: move portmapper debugging into tailscale CLI
The debug flag on tailscaled isn't available in the macOS App Store
build, since we don't have a tailscaled binary; move it to the
'tailscale debug' CLI that is available on all platforms instead,
accessed over LocalAPI.

Updates #7377

Signed-off-by: Andrew Dunham <andrew@du.nham.ca>
Change-Id: I47bffe4461e036fab577c2e51e173f4003592ff7
2 years ago
Mihai Parparita 9cb332f0e2 sockstats: instrument networking code paths
Uses the hooks added by tailscale/go#45 to instrument the reads and
writes on the major code paths that do network I/O in the client. The
convention is to use "<package>.<type>:<label>" as the annotation for
the responsible code path.

Enabled on iOS, macOS and Android only, since mobile platforms are the
ones we're most interested in, and we are less sensitive to any
throughput degradation due to the per-I/O callback overhead (macOS is
also enabled for ease of testing during development).

For now just exposed as counters on a /v0/sockstats PeerAPI endpoint.

We also keep track of the current interface so that we can break out
the stats by interface.

Updates tailscale/corp#9230
Updates #3363

Signed-off-by: Mihai Parparita <mihai@tailscale.com>
2 years ago
Will Norris 947c14793a all: update tools that manage copyright headers
Update all code generation tools, and those that check for license
headers to use the new standard header.

Also update copyright statement in LICENSE file.

Fixes #6865

Signed-off-by: Will Norris <will@tailscale.com>
2 years ago
Will Norris 71029cea2d all: update copyright and license headers
This updates all source files to use a new standard header for copyright
and license declaration.  Notably, copyright no longer includes a date,
and we now use the standard SPDX-License-Identifier header.

This commit was done almost entirely mechanically with perl, and then
some minimal manual fixes.

Updates #6865

Signed-off-by: Will Norris <will@tailscale.com>
2 years ago
Jordan Whited 25a0091f69
net/portmapper: relax handling of UPnP resp (#6946)
Gateway devices operating as an HA pair w/VRRP or CARP may send UPnP
replies from static addresses rather than the floating gateway address.
This commit relaxes our source address verification such that we parse
responses from non-gateway IPs, and re-point the UPnP root desc
URL to the gateway IP. This ensures we are still interfacing with the
gateway device (assuming L2 security intact), even though we got a
root desc from a non-gateway address.

This relaxed handling is required for ANY port mapping to work on certain
OPNsense/pfsense distributions using CARP at the time of writing, as
miniupnpd may only listen on the static, non-gateway interface address
for PCP and PMP.

Fixes #5502

Signed-off-by: Jordan Whited <jordan@tailscale.com>
2 years ago
Denton Gentry b8fe89d15f net/portmapper: add test for Huawei router
Updates https://github.com/tailscale/tailscale/issues/6320

Signed-off-by: Denton Gentry <dgentry@tailscale.com>
2 years ago
Brad Fitzpatrick da8def8e13 all: remove old +build tags
The //go:build syntax was introduced in Go 1.17:

https://go.dev/doc/go1.17#build-lines

gofmt has kept the +build and go:build lines in sync since
then, but enough time has passed. Time to remove them.

Done with:

    perl -i -npe 's,^// \+build.*\n,,' $(git grep -l -F '+build')

Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
2 years ago
Josh Soref d4811f11a0 all: fix spelling mistakes
Signed-off-by: Josh Soref <2119212+jsoref@users.noreply.github.com>
2 years ago
Brad Fitzpatrick 4950fe60bd syncs, all: move to using Go's new atomic types instead of ours
Fixes #5185

Change-Id: I850dd532559af78c3895e2924f8237ccc328449d
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
2 years ago
Brad Fitzpatrick 5381437664 logtail, net/portmapper, wgengine/magicsock: use fmt.Appendf
Fixes #5206

Change-Id: I490bb92e774ce7c044040537e2cd864fcf1dbe5a
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
2 years ago
Brad Fitzpatrick 5f6abcfa6f all: migrate code from netaddr.FromStdAddr to Go 1.18
With caveat https://github.com/golang/go/issues/53607#issuecomment-1203466984
that then requires a new wrapper. But a simpler one at least.

Updates #5162

Change-Id: I0a5265065bfcd7f21e8dd65b2bd74cae90d76090
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
2 years ago
Brad Fitzpatrick 8725b14056 all: migrate more code code to net/netip directly
Instead of going through the tailscale.com/net/netaddr transitional
wrappers.

Updates #5162

Change-Id: I3dafd1c2effa1a6caa9b7151ecf6edd1a3fda3dd
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
2 years ago
Brad Fitzpatrick a12aad6b47 all: convert more code to use net/netip directly
perl -i -npe 's,netaddr.IPPrefixFrom,netip.PrefixFrom,' $(git grep -l -F netaddr.)
    perl -i -npe 's,netaddr.IPPortFrom,netip.AddrPortFrom,' $(git grep -l -F netaddr. )
    perl -i -npe 's,netaddr.IPPrefix,netip.Prefix,g' $(git grep -l -F netaddr. )
    perl -i -npe 's,netaddr.IPPort,netip.AddrPort,g' $(git grep -l -F netaddr. )
    perl -i -npe 's,netaddr.IP\b,netip.Addr,g' $(git grep -l -F netaddr. )
    perl -i -npe 's,netaddr.IPv6Raw\b,netip.AddrFrom16,g' $(git grep -l -F netaddr. )
    goimports -w .

Then delete some stuff from the net/netaddr shim package which is no
longer neeed.

Updates #5162

Change-Id: Ia7a86893fe21c7e3ee1ec823e8aba288d4566cd8
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
2 years ago
Brad Fitzpatrick 6a396731eb all: use various net/netip parse funcs directly
Mechanical change with perl+goimports.

Changed {Must,}Parse{IP,IPPrefix,IPPort} to their netip variants, then
goimports -d .

Finally, removed the net/netaddr wrappers, to prevent future use.

Updates #5162

Change-Id: I59c0e38b5fbca5a935d701645789cddf3d7863ad
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
2 years ago
Brad Fitzpatrick 7eaf5e509f net/netaddr: start migrating to net/netip via new netaddr adapter package
Updates #5162

Change-Id: Id7bdec303b25471f69d542f8ce43805328d56c12
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
2 years ago
Denton Gentry d7f452c0a1 net/portmapper: send discovery packet for IGD specifically.
There appear to be devices out there which send only their
first descriptor in response to a discovery packet for
`ssdp:all`, for example the Sagemcom FAST3890V3 only sends
urn:schemas-wifialliance-org:device:WFADevice:1

Send both ssdp:all and a discovery frame for
InternetGatewayDevice specifically.

Updates https://github.com/tailscale/tailscale/issues/3557

Signed-off-by: Denton Gentry <dgentry@tailscale.com>
2 years ago
Denton Gentry 09eaba91ad net/portmap: add a test for Sagemcom FAST3890V3.
Updates https://github.com/tailscale/tailscale/issues/3557

Signed-off-by: Denton Gentry <dgentry@tailscale.com>
2 years ago
Denton Gentry b5553c6ad2 net/portmap: run go fmt
Signed-off-by: Denton Gentry <dgentry@tailscale.com>
2 years ago
Denton Gentry de4c635e54 net/portmapper: make pcpCodeNotAuthorized log more descriptive
If PCP is present but disabled, turning it on might help
get direct connections.

Signed-off-by: Denton Gentry <dgentry@tailscale.com>
2 years ago
Denton Gentry 7fffddce8e net/portmapper: enable for iOS
In the 1.27 unstable releases we set the min-version to iOS15,
which means we have 50 MBytes of RAM in the Network Extension.
https://tailscale.com/blog/go-linker/

Include the UPnP/NAT-PMP/PCP portmapper support now that there
is memory for it.

Fixes https://github.com/tailscale/tailscale/issues/2495
Signed-off-by: Denton Gentry <dgentry@tailscale.com>
2 years ago
Mihai Parparita 27a1ad6a70
wasm: exclude code that's not used on iOS for Wasm too
It has similar size constraints. Saves ~1.9MB from the Wasm build.

Updates #3157

Signed-off-by: Mihai Parparita <mihai@tailscale.com>
2 years ago
Josh Bleecher Snyder 0868329936 all: use any instead of interface{}
My favorite part of generics.

Signed-off-by: Josh Bleecher Snyder <josh@tailscale.com>
3 years ago
Brad Fitzpatrick 7d9b1de3aa netcheck,portmapper,magicsock: ignore some UDP write errors on Linux
Treat UDP send EPERM errors as a lost UDP packet, not something super
fatal. That's just the Linux firewall preventing it from going out.

And add a leaf package net/neterror for that (and future) policy that
all three packages can share, with tests.

Updates #3619

Change-Id: Ibdb838c43ee9efe70f4f25f7fc7fdf4607ba9c1d
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
3 years ago
Josh Bleecher Snyder 2075c39fd7 net/portmapper: deflake TestPCPIntegration
Logging in goroutines after the test completed
caused data races and panics. Prevent that.

Signed-off-by: Josh Bleecher Snyder <josh@tailscale.com>
3 years ago
Josh Bleecher Snyder a8cc519c70 net/portmapper: improve handling of UPnP parse errors
Without the continue, we might overwrite our current meta
with a zero meta.

Log the error, so that we can check for anything unexpected.

Signed-off-by: Josh Bleecher Snyder <josh@tailscale.com>
3 years ago
Josh Bleecher Snyder fddf43f3d1 net/portmapper: fill out PCP/PMP client metrics
Signed-off-by: Josh Bleecher Snyder <josh@tailscale.com>
3 years ago
Josh Bleecher Snyder 9787ec6f4a net/portmapper: add UPnP client metrics
Signed-off-by: Josh Bleecher Snyder <josh@tailscale.com>
3 years ago
Josh Bleecher Snyder 40f11c50a1 net/portmapper: make PCP/PMP result codes stringers
Signed-off-by: Josh Bleecher Snyder <josh@tailscale.com>
3 years ago
Josh Bleecher Snyder 38d90fa330 net/portmapper: add clientmetrics for PCP/PMP responses
Signed-off-by: Josh Bleecher Snyder <josh@tailscale.com>
3 years ago
Josh Bleecher Snyder 999814e9e1 net/portmapper: handle pcp ADDRESS_MISMATCH response
These show up a fair amount in our logs.

Signed-off-by: Josh Bleecher Snyder <josh@tailscale.com>
3 years ago
Josh Bleecher Snyder 1a629a4715 net/portmapper: mark fewer PMP probe failures as unexpected
There are lots of lines in the logs of the form:

portmapper: unexpected PMP probe response: {OpCode:128 ResultCode:3
SecondsSinceEpoch:NNN MappingValidSeconds:0 InternalPort:0
ExternalPort:0 PublicAddr:0.0.0.0}

ResultCode 3 here means a network failure, e.g. the NAT box itself has
not obtained a DHCP lease. This is not an indication that something
is wrong in the Tailscale client, so use different wording here
to reflect that. Keep logging, so that we can analyze and debug
the reasons that PMP probes fail.

Signed-off-by: Josh Bleecher Snyder <josh@tailscale.com>
3 years ago
Josh Bleecher Snyder 758c37b83d net/netns: thread logf into control functions
So that darwin can log there without panicking during tests.

Signed-off-by: Josh Bleecher Snyder <josh@tailscale.com>
3 years ago
Denton Gentry 5302e4be96 net/portmapper: only print PCP/PMP if VerboseLogs
Make UPnP, NAT-PMP, and PCP packet reception logs be [v1] so
they will never appear on stdout and instead only go to logtail.

```
$ tailscale netcheck
2021/10/15 22:50:31 portmap: Got PMP response; IP: w.x.y.z, epoch: 1012707
2021/10/15 22:50:31 portmap: Got PCP response: epoch: 1012707

Report:
        * UDP: true
        * IPv4: yes, w.x.y.z:1511
        * IPv6: no
        * MappingVariesByDestIP: true
        * HairPinning: false
        * PortMapping: NAT-PMP, PCP
        * Nearest DERP: San Francisco
        * DERP latency:
                - sfo: 5.9ms   (San Francisco)
                - sea: 24ms    (Seattle)
                - dfw: 45ms    (Dallas)
                - ord: 53.7ms  (Chicago)
                - nyc: 74.1ms  (New York City)
                - tok: 111.1ms (Tokyo)
                - lhr: 139.4ms (London)
                - syd: 152.7ms (Sydney)
                - fra: 153.1ms (Frankfurt)
                - sin: 182.1ms (Singapore)
                - sao: 190.1ms (S_o Paulo)
                - blr: 218.6ms (Bangalore)
```

Signed-off-by: Denton Gentry <dgentry@tailscale.com>
3 years ago
David Anderson 060ba86baa net/portmapper: ignore IGD SSDP responses from !defaultgw
Now that we multicast the SSDP query, we can get IGD offers from
devices other than the current device's default gateway. We don't want
to accidentally bind ourselves to those.

Updates #3197

Signed-off-by: David Anderson <danderson@tailscale.com>
3 years ago
David Anderson 4a65b07e34 net/portmapper: also send UPnP SSDP query to the SSDP multicast address.
Fixes #3197

Signed-off-by: David Anderson <danderson@tailscale.com>
3 years ago
Brad Fitzpatrick 640134421e all: update tests to use tstest.MemLogger
And give MemLogger a mutex, as one caller had, which does match the logf
contract better.

Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
3 years ago
Brad Fitzpatrick 48bdffd395 net/portmapper: remove GITHUB_ACTIONS check
It's now redundant.

Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
3 years ago
David Anderson b49d9bc74d net/portmapper: fix "running a test" condition.
Fixes #2686.

Signed-off-by: David Anderson <danderson@tailscale.com>
3 years ago
Brad Fitzpatrick cd426eaf4c net/portmapper: fix t.Log-after-test-done race in tests
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
3 years ago
julianknodt b9bd7dbc5d net/portmapper: log upnp information
This logs some basic statistics for UPnP, so that tailscale can better understand what routers
are being used and how to connect to them.

Signed-off-by: julianknodt <julianknodt@gmail.com>
3 years ago
julianknodt 26b6fe7f02 net/portmapper: add PCP integration test
This adds a PCP test to the IGD test server, by hardcoding in a few observed packets from
Denton's box.

Signed-off-by: julianknodt <julianknodt@gmail.com>
3 years ago
Brad Fitzpatrick bdb93c5942 net/portmapper: actually test something in TestProbeIntegration
And use dynamic port numbers in tests, as Linux on GitHub Actions and
Windows in general have things running on these ports.

Co-Author: Julian Knodt <julianknodt@gmail.com>
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
3 years ago