Commit Graph

64 Commits (1016aa045fb3f31b40d4038828114a5f1fb3547b)

Author SHA1 Message Date
Brad Fitzpatrick ce8969d82b net/portmapper: add envknob to disable portmapper in localhost integration tests
Updates #11962

Change-Id: I8212cd814985b455d96986de0d4c45f119516cb3
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
6 months ago
Brad Fitzpatrick b9adbe2002 net/{interfaces,netmon}, all: merge net/interfaces package into net/netmon
In prep for most of the package funcs in net/interfaces to become
methods in a long-lived netmon.Monitor that can cache things.  (Many
of the funcs are very heavy to call regularly, whereas the long-lived
netmon.Monitor can subscribe to things from the OS and remember
answers to questions it's asked regularly later)

Updates tailscale/corp#10910
Updates tailscale/corp#18960
Updates #7967
Updates #3299

Change-Id: Ie4e8dedb70136af2d611b990b865a822cd1797e5
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
7 months ago
Brad Fitzpatrick 3672f29a4e net/netns, net/dns/resolver, etc: make netmon required in most places
The goal is to move more network state accessors to netmon.Monitor
where they can be cheaper/cached. But first (this change and others)
we need to make sure the one netmon.Monitor is plumbed everywhere.

Some notable bits:

* tsdial.NewDialer is added, taking a now-required netmon

* because a tsdial.Dialer always has a netmon, anything taking both
  a Dialer and a NetMon is now redundant; take only the Dialer and
  get the NetMon from that if/when needed.

* netmon.NewStatic is added, primarily for tests

Updates tailscale/corp#10910
Updates tailscale/corp#18960
Updates #7967
Updates #3299

Change-Id: I877f9cb87618c4eb037cee098241d18da9c01691
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
7 months ago
Brad Fitzpatrick a36cfb4d3d tailcfg, ipn/ipnlocal, wgengine/magicsock: add only-tcp-443 node attr
Updates tailscale/corp#17879

Change-Id: I0dc305d147b76c409cf729b599a94fa723aef0e0
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
8 months ago
Andrew Dunham fd94d96e2b net/portmapper: support legacy "urn:dslforum-org" portmapping services
These are functionally the same as the "urn:schemas-upnp-org" services
with a few minor changes, and are still used by older devices. Support
them to improve our ability to obtain an external IP on such networks.

Updates #10911

Signed-off-by: Andrew Dunham <andrew@du.nham.ca>
Change-Id: I05501fad9d6f0a3b8cf19fc95eee80e7d16cc2cf
10 months ago
Andrew Dunham fa3639783c net/portmapper: check returned epoch from PMP and PCP protocols
If the epoch that we see during a Probe is less than the existing epoch,
it means that the gateway has either restarted or reset its
configuration, and an existing mapping is no longer valid. Reset any
saved mapping(s) if we detect this case so that a future
createOrGetMapping will not attempt to re-use it.

Updates #10597

Signed-off-by: Andrew Dunham <andrew@du.nham.ca>
Change-Id: Ie3cddaf625cb94a29885f7a1eeea25dbf6b97b47
10 months ago
Andrew Dunham 3c333f6341 net/portmapper: add logs about obtained mapping(s)
This logs additional information about what mapping(s) are obtained
during the creation process, including whether we return an existing
cached mapping.

Updates #10597

Signed-off-by: Andrew Dunham <andrew@du.nham.ca>
Change-Id: I9ff25071f064c91691db9ab0b9365ccc5f948d6e
11 months ago
Andrew Dunham d05a572db4 net/portmapper: handle multiple UPnP discovery responses
Instead of taking the first UPnP response we receive and using that to
create port mappings, store all received UPnP responses, sort and
deduplicate them, and then try all of them to obtain an external
address.

Updates #10602

Signed-off-by: Andrew Dunham <andrew@du.nham.ca>
Change-Id: I783ccb1834834ee2a9ecbae2b16d801f2354302f
11 months ago
Andrew Dunham bac4890467 net/portmapper: be smarter about selecting a UPnP device
Previously, we would select the first WANIPConnection2 (and related)
client from the root device, without any additional checks. However,
some routers expose multiple UPnP devices in various states, and simply
picking the first available one can result in attempting to perform a
portmap with a device that isn't functional.

Instead, mimic what the miniupnpc code does, and prefer devices that are
(a) reporting as Connected, and (b) have a valid external IP address.
For our use-case, we additionally prefer devices that have an external
IP address that's a public address, to increase the likelihood that we
can obtain a direct connection from peers.

Finally, we split out fetching the root device (getUPnPRootDevice) from
selecting the best service within that root device (selectBestService),
and add some extensive tests for various UPnP server behaviours.

RELNOTE=Improve UPnP portmapping when multiple UPnP services exist

Updates #8364

Signed-off-by: Andrew Dunham <andrew@du.nham.ca>
Change-Id: I71795cd80be6214dfcef0fe83115a5e3fe4b8753
11 months ago
Andrew Dunham ef596aed9b net/portmapper: avoid alloc in getUPnPErrorsMetric
Updates #cleanup

Signed-off-by: Andrew Dunham <andrew@du.nham.ca>
Change-Id: Iea558024c038face24cc46584421998d10f13a66
1 year ago
Val 19e5f242e0 net/portmapper: convert UPnP metrics to new syncs.Map.LoadOrInit method
Simplify UPnP error metrics by using the new syncs.Map.LoadOrInit method.

Updates #cleanup

Signed-off-by: Val <valerie@tailscale.com>
1 year ago
Andrew Dunham d31460f793 net/portmapper: fix invalid UPnP metric name
Fixes #9551

Signed-off-by: Andrew Dunham <andrew@du.nham.ca>
Change-Id: I06f3a15a668be621675be6cbc7e5bdcc006e8570
1 year ago
Andrew Dunham d9ae7d670e net/portmapper: add clientmetric for UPnP error codes
This should allow us to gather a bit more information about errors that
we encounter when creating UPnP mappings. Since we don't have a
"LabelMap" construction for clientmetrics, do what sockstats does and
lazily register a new metric when we see a new code.

Updates #9343

Signed-off-by: Andrew Dunham <andrew@du.nham.ca>
Change-Id: Ibb5aadd6138beb58721f98123debcc7273b611ba
1 year ago
Brad Fitzpatrick 4e91cf20a8 control/controlknobs, all: add plumbed Knobs type, not global variables
Previously two tsnet nodes in the same process couldn't have disjoint
sets of controlknob settings from control as both would overwrite each
other's global variables.

This plumbs a new controlknobs.Knobs type around everywhere and hangs
the knobs sent by control on that instead.

Updates #9351

Change-Id: I75338646d36813ed971b4ffad6f9a8b41ec91560
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
1 year ago
Andrew Dunham c86a610eb3 cmd/tailscale, net/portmapper: add --log-http option to "debug portmap"
This option allows logging the raw HTTP requests and responses that the
portmapper Client makes when using UPnP. This can be extremely helpful
when debugging strange UPnP issues with users' devices, and might allow
us to avoid having to instruct users to perform a packet capture.

Updates #8992

Signed-off-by: Andrew Dunham <andrew@du.nham.ca>
Change-Id: I2c3cf6930b09717028deaff31738484cc9b008e4
1 year ago
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 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
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
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 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 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
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
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
Brad Fitzpatrick e6d4ab2dd6 net/portmapper: add start of self-contained portmapper integration tests
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
3 years ago
julianknodt 98d36ee18d net/portmapper: add hook for use with prev ip
PCP handles external IPs by allowing the client to specify them in the packet, which is more
explicit than requiring 2 packets from PMP, so allow for future changes to add it in easily.

Signed-off-by: julianknodt <julianknodt@gmail.com>
3 years ago