The earlier 5f6d63936f was not complete.
Updates tailscale/corp#7515
Change-Id: I35efca51d1584c48ef6834a7d29cd42d7c943628
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
We would end up with duplicate profiles for the node as the UserID
would have chnaged. In order to correctly deduplicate profiles, we
need to look at both the UserID and the NodeID. A single machine can
only ever have 1 profile per NodeID and 1 profile per UserID.
Note: UserID of a Node can change when the node is tagged/untagged,
and the NodeID of a device can change when the node is deleted so we
need to check for both.
Updates #713
Signed-off-by: Maisem Ali <maisem@tailscale.com>
The fix in 4fc8538e2 was sufficient for IPv6. Browsers (can?) send the
IPv6 literal, even without a port number, in brackets.
Updates tailscale/corp#7948
Change-Id: I0e429d3de4df8429152c12f251ab140b0c8f6b77
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
I was too late with review feedback to 513780f4f8.
Updates tailscale/corp#7948
Change-Id: I8fa3b4eba4efaff591a2d0bfe6ab4795638b7c3a
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
We were not updating the LoginProfile.UserProfile when a netmap
updated the UserProfile (e.g. when a node was tagged via the admin panel).
Updates #713
Signed-off-by: Maisem Ali <maisem@tailscale.com>
This moves the NetworkLock key from a dedicated StateKey to be part of the persist.Persist struct.
This struct is stored as part for ipn.Prefs and is also the place where we store the NodeKey.
It also moves the ChonkDir from "/tka" to "/tka-profile/<profile-id>". The rename was intentional
to be able to delete the "/tka" dir if it exists.
This means that we will have a unique key per profile, and a unique directory per profile.
Note: `tailscale logout` will delete the entire profile, including any keys. It currently does not
delete the ChonkDir.
Signed-off-by: Maisem Ali <maisem@tailscale.com>
We do not need to wait for it to complete. And we might have to
call Shutdown from callback from the controlclient which might
already be holding a lock that Shutdown requires.
Updates #713
Signed-off-by: Maisem Ali <maisem@tailscale.com>
Found by tests in another repo. TKA code wasn't always checking enough to be sure a node-key was set for the current state.
Signed-off-by: Tom DNetto <tom@tailscale.com>
The health package was turning into a rando dumping ground. Make a new
Warnable type instead that callers can request an instance of, and
then Set it locally in their code without the health package being
aware of all the things that are warnable. (For plenty of things the
health package will want to know details of how Tailscale works so it
can better prioritize/suppress errors, but lots of the warnings are
pretty leaf-y and unrelated)
This just moves two of the health warnings. Can probably move more
later.
Change-Id: I51e50e46eb633f4e96ced503d3b18a1891de1452
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
This makes it so that the backend also restarts when users change,
otherwise an extra call to Start was required.
Updates #713
Signed-off-by: Maisem Ali <maisem@tailscale.com>
Noticed this while debugging something else, we would reset all routes if
either `--advertise-exit-node` or `--advertise-routes` were set. This handles
correctly updating them.
Also added tests.
Signed-off-by: Maisem Ali <maisem@tailscale.com>
It left the envknob turned on which meant that running all the tests
in the package had different behavior than running just any one test.
Signed-off-by: Maisem Ali <maisem@tailscale.com>
Example output:
# Health check:
# - Some peers are advertising routes but --accept-routes is false
Also, move "tailscale status" health checks to the bottom, where they
won't be lost in large netmaps.
Updates #2053
Updates #6266
Change-Id: I5ae76a0cd69a452ce70063875cd7d974bfeb8f1a
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
Numerous issues have been filed concerning an inability to install and run
Tailscale headlessly in unattended mode, particularly after rebooting. The
server mode `Prefs` stored in `server-state.conf` were not being updated with
`Persist` state once the node had been succesfully logged in.
Users have been working around this by finagling with the GUI to make it force
a state rewrite. This patch makes that unnecessary by ensuring the required
server mode state is updated when prefs are updated by the control client.
Fixes https://github.com/tailscale/tailscale/issues/3186
Signed-off-by: Aaron Klotz <aaron@tailscale.com>
Even if the name is right, or is configured on a different port.
Updates tailscale/corp#7515
Change-Id: I8b721968f3241af10d98431e1b5ba075223e6cd3
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
cmd/viewer couldn't deal with that map-of-map. Add a wrapper type
instead, which also gives us a place to add future stuff.
Updates tailscale/corp#7515
Change-Id: I44a4ca1915300ea8678e5b0385056f0642ccb155
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
Leave only the HTTP/auth bits in localapi.
Change-Id: I8e23fb417367f1e0e31483e2982c343ca74086ab
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
Not for end users (unless directed by support). Mostly for ease of
development for some upcoming webserver work.
Change-Id: I43acfed217514567acb3312367b24d620e739f88
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
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>
It is currently a `ipn.PrefsView` which means when we do a JSON roundtrip,
we go from an invalid Prefs to a valid one.
This makes it a pointer, which fixes the JSON roundtrip.
This was introduced in 0957bc5af2.
Signed-off-by: Maisem Ali <maisem@tailscale.com>
This switches from using an atomic.Bool to a mutex for reasons that are
described in the commit, and should address the flakes that we're still
seeing.
Fixes#3020
Change-Id: I4e39471c0eb95886db03020ea1ccf688c7564a11
Signed-off-by: Andrew Dunham <andrew@tailscale.com>
* Plumb disablement values through some of the internals of TKA enablement.
* Transmit the node's TKA hash at the end of sync so the control plane understands each node's head.
* Implement /machine/tka/disable RPC to actuate disablement on the control plane.
There is a partner PR for the control server I'll send shortly.
Signed-off-by: Tom DNetto <tom@tailscale.com>
This makes tags, creation time, exit node option and primary routes
for the current node exposed via `tailscale status --json`
Signed-off-by: Anton Tolchanov <anton@tailscale.com>
Poller.C and Poller.c were duplicated for one caller. Add an accessor
returning the receive-only version instead. It'll inline.
Poller.Err was unused. Remove.
Then Poller is opaque.
The channel usage and shutdown was a bit sketchy. Clean it up.
And document some things.
Change-Id: I5669e54f51a6a13492cf5485c83133bda7ea3ce9
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
Running corp/ipn#TestNetworkLockE2E has a 1/300 chance of failing, and
deskchecking suggests thats whats happening are two netmaps are racing each
other to be processed through tkaSyncIfNeededLocked. This happens in the
first place because we release b.mu during network RPCs.
To fix this, we make the tka sync logic an exclusive section, so two
netmaps will need to wait for tka sync to complete serially (which is what
we would want anyway, as the second run through probably wont need to
sync).
Signed-off-by: Tom DNetto <tom@tailscale.com>
The macOS and iOS apps that used the /localapi/v0/file-targets handler
were getting too many candidate targets. They wouldn't actually accept
the file. This is effectively just a UI glitch in the wrong hosts
being listed as valid targets from the source side.
Change-Id: I6907a5a1c3c66920e5ec71601c044e722e7cb888
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
* tka.State.staticValidateCheckpoint could call methods on a contained key prior to calling StaticValidate on that key
* Remove broken backoff / RPC retry logic from tka methods in ipn/ipnlocal, to be fixed at a later time
* Fix NetworkLockModify() which would attempt to take b.mu twice and deadlock, remove now-unused dependence on netmap
* Add methods on ipnlocal.LocalBackend to be used in integration tests
* Use TAILSCALE_USE_WIP_CODE as the feature flag so it can be manipulated in tests
Signed-off-by: Tom DNetto <tom@tailscale.com>
And add a CLI/localapi and c2n mechanism to enable it for a fixed
amount of time.
Updates #1548
Change-Id: I71674aaf959a9c6761ff33bbf4a417ffd42195a7
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
Most visible when using tsnet.Server, but could have resulted in dropped
messages in a few other places too.
Fixes#5743
Signed-off-by: Mihai Parparita <mihai@tailscale.com>
* and move goroutine scrubbing code to its own package for reuse
* bump capver to 45
Change-Id: I9b4dfa5af44d2ecada6cc044cd1b5674ee427575
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
* tailcfg, control/controlhttp, control/controlclient: add ControlDialPlan field
This field allows the control server to provide explicit information
about how to connect to it; useful if the client's link status can
change after the initial connection, or if the DNS settings pushed by
the control server break future connections.
Change-Id: I720afe6289ec27d40a41b3dcb310ec45bd7e5f3e
Signed-off-by: Andrew Dunham <andrew@tailscale.com>
It was checking if the sshServer was initialized as a proxy, but that
could either not have been initialized yet or Tailscale SSH could have
been disabled after intialized.
Also bump tailcfg.CurrentCapabilityVersion
Signed-off-by: Maisem Ali <maisem@tailscale.com>
For control to fetch a list of Tailscale SSH username candidates to
filter against the Tailnet's SSH policy to present some valid
candidates to a user.
Updates #3802
Updates tailscale/corp#7007
Change-Id: I3dce57b7a35e66891d5e5572e13ae6ef3c898498
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
The GitHub CodeQL scanner flagged the localapi's cert domain usage as a problem
because user input in the URL made it to disk stat checks.
The domain is validated against the ipnstate.Status later, and only
authenticated root/configured users can hit this, but add some
paranoia anyway.
Change-Id: I373ef23832f1d8b3a27208bc811b6588ae5a1ddd
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
The io/ioutil package has been deprecated as of Go 1.16 [1]. This commit
replaces the existing io/ioutil functions with their new definitions in
io and os packages.
Reference: https://golang.org/doc/go1.16#ioutil
Signed-off-by: Eng Zer Jun <engzerjun@gmail.com>
This is especially helpful as we launch newer DERPs over time, and older
clients have progressively out-of-date static DERP maps baked in. After
this, as long as the client has successfully connected once, it'll cache
the most recent DERP map it knows about.
Resolves an in-code comment from @bradfitz
Signed-off-by: Andrew Dunham <andrew@du.nham.ca>
This will be needed to support preauth-keys with network lock in the future,
so getting the core mechanics out of the way now.
Signed-off-by: Tom DNetto <tom@tailscale.com>
This lets the control plane can make HTTP requests to nodes.
Then we can use this for future things rather than slapping more stuff
into MapResponse, etc.
Change-Id: Ic802078c50d33653ae1f79d1e5257e7ade4408fd
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
Updates #5435
Based on the discussion in #5435, we can better support transactional data models
by making the underlying storage layer a parameter (which can be specialized for
the request) rather than a long-lived member of Authority.
Now that Authority is just an instantaneous snapshot of state, we can do things
like provide idempotent methods and make it cloneable, too.
Signed-off-by: Tom DNetto <tom@tailscale.com>
The CapabilityFileSharingTarget capability added by eb32847d85
is meant to control the ability to share with nodes not owned by the
current user, not to restrict all sharing (the coordination server is
not currently populating the capability at all)
Fixestailscale/corp#6669
Signed-off-by: Mihai Parparita <mihai@tailscale.com>
Needed to identify the node. A serverside-check the machine key (used
to authenticate the noise session) is that of the specified NodeID
ensures the authenticity of the request.
Signed-off-by: Tom DNetto <tom@tailscale.com>
Hashing []any is slow since hashing of interfaces is slow.
Hashing of interfaces is slow since we pessimistically assume
that cycles can occur through them and start cycle tracking.
Drop the variadic signature of Update and fix callers to pass in
an anonymous struct so that we are hashing concrete types
near the root of the value tree.
Signed-off-by: Joe Tsai <joetsai@digital-static.net>
Signed-off-by: Joe Tsai <joetsai@digital-static.net>
- A network-lock key is generated if it doesn't already exist, and stored in the StateStore. The public component is communicated to control during registration.
- If TKA state exists on the filesystem, a tailnet key authority is initialized (but nothing is done with it for now).
Signed-off-by: Tom DNetto <tom@tailscale.com>
This adds the inverse to CapabilityFileSharingSend so that senders can
identify who they can Taildrop to.
Updates #2101
Signed-off-by: Maisem Ali <maisem@tailscale.com>
If the host only has RSA, use its RSA + generate ecdsa + ed25519, etc.
Perhaps fixes https://twitter.com/colek42c/status/1550554439299244032 and
something else that was reported.
Change-Id: I88dc475c8e3d95b6f25288ff7664b8e72655fd16
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
Together with 06aa141632 this minimizes
the number of NEPacketTunnelNetworkSettings updates that we have to do,
and thus avoids Chrome interrupting outstanding requests due to
(perceived) network changes.
Updates #3102
Signed-off-by: Mihai Parparita <mihai@tailscale.com>
And remove the GCP special-casing from ipn/ipnlocal; do it only in the
forwarder for *.internal.
Fixes#4980Fixes#4981
Change-Id: I5c481e96d91f3d51d274a80fbd37c38f16dfa5cb
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
This does three things:
* If you're on GCP, it adds a *.internal DNS split route to the
metadata server, so we never break GCP DNS names. This lets people
have some Tailscale nodes on GCP and some not (e.g. laptops at home)
without having to add a Tailnet-wide *.internal DNS route.
If you already have such a route, though, it won't overwrite it.
* If the 100.100.100.100 DNS forwarder has nowhere to forward to,
it forwards it to the GCP metadata IP, which forwards to 8.8.8.8.
This means there are never errNoUpstreams ("upstream nameservers not set")
errors on GCP due to e.g. mangled /etc/resolv.conf (GCP default VMs
don't have systemd-resolved, so it's likely a DNS supremacy fight)
* makes the DNS fallback mechanism use the GCP metadata IP as a
fallback before our hosted HTTP-based fallbacks
I created a default GCP VM from their web wizard. It has no
systemd-resolved.
I then made its /etc/resolv.conf be empty and deleted its GCP
hostnames in /etc/hosts.
I then logged in to a tailnet with no global DNS settings.
With this, tailscaled writes /etc/resolv.conf (direct mode, as no
systemd-resolved) and sets it to 100.100.100.100, which then has
regular DNS via the metadata IP and *.internal DNS via the metadata IP
as well. If the tailnet configures explicit DNS servers, those are used
instead, except for *.internal.
This also adds a new util/cloudenv package based on version/distro
where the cloud type is only detected once. We'll likely expand it in
the future for other clouds, doing variants of this change for other
popular cloud environments.
Fixes#4911
RELNOTES=Google Cloud DNS improvements
Change-Id: I19f3c2075983669b2b2c0f29a548da8de373c7cf
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
Client.SetExpirySooner isn't part of the state machine. Remove it from
the Client interface.
And fix a use of LocalBackend.cc without acquiring the lock that
guards that field.
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
On DSM7 as a non-root user it'll run into problems.
And we haven't tested on DSM6, even though it might work, but I doubt
it.
Updates #3802
Updates tailscale/corp#5468
Change-Id: I75729042e4788f03f9eb82057482a44b319f04f3
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
Also lazify SSHServer initialization to allow restarting the server on a
subsequent `tailscale up`
Updates #3802
Signed-off-by: Maisem Ali <maisem@tailscale.com>
Ideally we would re-establish these sessions when tailscaled comes back
up, however we do not do that yet so this is better than leaking the
sessions.
Updates #3802
Signed-off-by: Maisem Ali <maisem@tailscale.com>
This fixes the "tailscale up --authkey=... --ssh" path (or any "up"
path that used Start instead of EditPrefs) which wasn't setting the
bit.
Updates #3802
Change-Id: Ifca532ec58296fedcedb5582312dfee884367ed7
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
Currently, when SetNetInfo is called it sets the value on
hostinfo.NetInfo. However, when SetHostInfo is called it overwrites the
hostinfo field which may mean it also clears out the NetInfo it had just
received.
This commit stores NetInfo separately and combines it into Hostinfo as
needed so that control is always notified of the latest values.
Also, remove unused copies of Hostinfo from ipn.Status and
controlclient.Auto.
Updates #tailscale/corp#4824 (maybe fixes)
Signed-off-by: Maisem Ali <maisem@tailscale.com>
Currently the ssh session isn't terminated cleanly, instead the packets
are just are no longer routed to the in-proc SSH server. This makes it
so that clients get a disconnection when the `RunSSH` pref changes to
`false`.
Updates #3802
Signed-off-by: Maisem Ali <maisem@tailscale.com>
No callers remain (last one was removed with
tailscale/corp@1c095ae08f), and it's
pretty esoteric.
Signed-off-by: Mihai Parparita <mihai@tailscale.com>
The Mac client was using it, but it had the effect of the `RouteAll`
("Use Tailscale subnets") pref always being enabled at startup,
regardless of the persisted value.
enforceDefaults was added to handle cases from ~2 years ago where
we ended up with persisted `"RouteAll": false` values in the keychain,
but that should no longer be a concern. New users will get the default
of it being enabled via `NewPrefs`.
There will be a corresponding Mac client change to stop passing in
enforceDefaults.
For #3962
Signed-off-by: Mihai Parparita <mihai@tailscale.com>
The connections returned from SystemDial are automatically closed when
there is a major link change.
Also plumb through the dialer to the noise client so that connections
are auto-reset when moving from cellular to WiFi etc.
Updates #3363
Signed-off-by: Maisem Ali <maisem@tailscale.com>
No CLI support yet. Just the curl'able version if you know the peerapi
port. (like via a TSMP ping)
Updates #306
Change-Id: I0662ba6530f7ab58d0ddb24e3664167fcd1c4bcf
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
For tests.
Now that we can always listen (whereas we used to fail prior to
a2c330c496), some goroutine leak
checks were failing in tests in another repo after that change.
Change-Id: Id95a4b71167eca61962a48616d79741b9991e0bc
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
The previous commit (1b89662eff) this for Android, but we can also use
this on any platform if we we would otherwise fail.
Change-Id: I4cd78b40e9e77fca5cc8e717dd48ac173101bed4
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
We intercept the peerapi port in netstack anyway, so there's no reason
the linux kernel on Android needs to know about it. It's only getting
in the way and causing problems for reasons we don't fully understand.
But we don't even need to understand it because it's not relevant
anymore.
Instead, provide a dummy net.Listener that just sits and blocks to
pacify the rest of the code that assumes it can be stuck in a
Listener.Accept call and call Listener.Close and Listener.Addr.
We'll likely do this for all platforms in the future, if/when we also
link in netstack on iOS.
Updates #4449
Updates #4293
Updates #3986
Change-Id: Ic2d3fe2f3cee60fc527356a3368830f17aeb75ae
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
Two changes in one:
* make DoH upgrades an explicitly scheduled send earlier, when we come
up with the resolvers-and-delay send plan. Previously we were
getting e.g. four Google DNS IPs and then spreading them out in
time (for back when we only did UDP) but then later we added DoH
upgrading at the UDP packet layer, which resulted in sometimes
multiple DoH queries to the same provider running (each doing happy
eyeballs dialing to 4x IPs themselves) for each of the 4 source IPs.
Instead, take those 4 Google/Cloudflare IPs and schedule 5 things:
first the DoH query (which can use all 4 IPs), and then each of the
4 IPs as UDP later.
* clean up the dnstype.Resolver.Addr confusion; half the code was
using it as an IP string (as documented) as half was using it as
an IP:port (from some prior type we used), primarily for tests.
Instead, document it was being primarily an IP string but also
accepting an IP:port for tests, then add an accessor method on it
to get the IPPort and use that consistently everywhere.
Change-Id: Ifdd72b9e45433a5b9c029194d50db2b9f9217b53
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
The default is still users can debug their own nodes. But like
cd916b728b did, this adds support for admins to grant additional
capabilities with the new tailcfg.CapabilityDebugPeer cap.
Updates #4217
Change-Id: Ifce3d9a1f8e8845797970a4f97b393194663d35f
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
Fail on unsupported platforms (must be Linux or macOS tailscaled with
WIP env) or when disabled by admin (with TS_DISABLE_SSH_SERVER=1)
Updates #3802
Change-Id: I5ba191ed0d8ba4ddabe9b8fc1c6a0ead8754b286
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
And rename to updateFilterLocked to prevent future mistakes.
Fixes#4427
Change-Id: I4d37b90027d5ff872a339ce8180f5723704848dc
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
Controlled by server-sent capability policy.
To be initially used for SSH servers to record sessions to other
nodes. Not yet productized into something user-accessible. (Notably,
the list of Taildrop targets from the sender side isn't augmented
yet.) This purely permits expanding the set of expands a node will
accept a drop from.
Updates #3802
Updates #4217
Change-Id: Id7a5bccd686490f8ef2cdc7dae7c07c440dc0085
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
Remove the weird netstack -> tailssh dependency and instead have tailssh
register itself with ipnlocal when linked.
This makes tailssh.server a singleton, so we can have a global map of
all sessions.
Updates #3802
Change-Id: Iad5caec3a26a33011796878ab66b8e7b49339f29
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
Currently peerIPs doesn't do any sorting of the routes it returns. This
is typically fine, however imagine the case of an HA subnet router
failover. When a route R moves from peer A to peer B, the output of
peerIPs changes. This in turn causes all the deephash check inside
wgengine to fail as the hashed value of [R1, R2] is different than
the hashed value of [R2, R1]. When the hash check failes, it causes
wgengine to reconfigure all routes in the OS. This is especially
problematic for macOS and iOS where we use the NetworkExtension.
This commit makes it that the peerIPs are always sorted when returned,
thus making the hash be consistent as long as the list of routes remains
static.
Signed-off-by: Maisem Ali <maisem@tailscale.com>
This defines a new magic IPv6 prefix, fd7a:115c:a1e0:b1a::/64, a
subset of our existing /48, where the final 32 bits are an IPv4
address, and the middle 32 bits are a user-chosen "site ID". (which
must currently be 0000:00xx; the top 3 bytes must be zero for now)
e.g., I can say my home LAN's "site ID" is "0000:00bb" and then
advertise its 10.2.0.0/16 IPv4 range via IPv6, like:
tailscale up --advertise-routes=fd7a:115c:a1e0:b1a::bb:10.2.0.0/112
(112 being /128 minuse the /96 v6 prefix length)
Then people in my tailnet can:
$ curl '[fd7a:115c:a1e0:b1a::bb:10.2.0.230]'
<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN" ....
Updates #3616, etc
RELNOTE=initial support for TS IPv6 addresses to route v4 "via" specific nodes
Change-Id: I9b49b6ad10410a24b5866b9fbc69d3cae1f600ef
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
Combine the code between `LocalBackend.CheckIPForwarding` and
`controlclient.ipForwardingBroken`.
Fixes#4300
Signed-off-by: Maisem Ali <maisem@tailscale.com>
While we rearrange/upstream things.
gliderlabs/ssh is forked into tempfork from our prior fork
at be8b7add40
x/crypto/ssh OTOH is forked at
https://github.com/tailscale/golang-x-crypto because it was gnarlier
to vendor with various internal packages, etc.
Its git history shows where it starts (2c7772ba30643b7a2026cbea938420dce7c6384d).
Updates #3802
Change-Id: I546e5cdf831cfc030a6c42557c0ad2c58766c65f
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
When `setWgengineStatus` is invoked concurrently from multiple
goroutines, it is possible that the call invoked with a newer status is
processed before a call with an older status. e.g. a status that has
endpoints might be followed by a status without endpoints. This causes
unnecessary work in the engine and can result in packet loss.
This patch adds an `AsOf time.Time` field to the status to specifiy when the
status was calculated, which later allows `setWgengineStatus` to ignore
any status messages it receives that are older than the one it has
already processed.
Updates tailscale/corp#2579
Signed-off-by: Maisem Ali <maisem@tailscale.com>
Also make IPPrefixSliceOf use Slice[netaddr.IPPrefix] as it also
provides additional functions besides the standard ones provided by
Slice[T].
Signed-off-by: Maisem Ali <maisem@tailscale.com>
And add a CapabilityVersion type, primarily for documentation.
This makes MapRequest.Version, RegisterRequest.Version, and
SetDNSRequest.Version all use the same version, which will avoid
confusing in the future if Register or SetDNS ever changed their
semantics on Version change. (Currently they're both always 1)
This will requre a control server change to allow a
SetDNSRequest.Version value other than 1 to be deployed first.
Change-Id: I073042a216e0d745f52ee2dbc45cf336b9f84b7c
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
Also move KubeStore and MemStore into their own package.
RELNOTE: tsnet now supports providing a custom ipn.StateStore.
Signed-off-by: Maisem Ali <maisem@tailscale.com>
For local dev testing initially. Product-wise, it'll probably only be
workable on the two unsandboxed builds.
Updates #3802
Change-Id: Ic352f966e7fb29aff897217d79b383131bf3f92b
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
I introduced a bug in 8fe503057d when unifying oneConnListener
implementations.
The NewOneConnListenerFrom API was easy to misuse (its Close method
closes the underlying Listener), and we did (via http.Serve, which
closes the listener after use, which meant we were close the peerapi's
listener, even though we only wanted its Addr)
Instead, combine those two constructors into one and pass in the Addr
explicitly, without delegating through to any Listener.
Change-Id: I061d7e5f842e0cada416e7b2dd62100d4f987125
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
e.g. the change to ipnlocal in this commit ultimately logs out:
{"logtail":{"client_time":"2022-02-17T20:40:30.511381153-08:00","server_time":"2022-02-18T04:40:31.057771504Z"},"type":"Hostinfo","val":{"GoArch":"amd64","Hostname":"tsdev","IPNVersion":"1.21.0-date.20220107","OS":"linux","OSVersion":"Debian 11.2 (bullseye); kernel=5.10.0-10-amd64"},"v":1}
Change-Id: I668646b19aeae4a2fed05170d7b279456829c844
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
(The name SSH_HostKeys is bad but SSHHostKeys is worse.)
Updates #3802
Change-Id: I2a889019c9e8b065b668dd58140db4fcab868a91
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
Make tailssh ask LocalBackend for the SSH hostkeys, as we'll need to
distribute them to peers.
For now only the hacky use-same-as-actual-host mode is implemented.
Updates #3802
Change-Id: I819dcb25c14e42e6692c441186c1dc744441592b
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
We need to capture some tailnet-related information for some Docker
features we're building. This exposes the tailnet name and MagicDNS
information via `tailscale status --json`.
Fixestailscale/corp#3670
Signed-off-by: Ross Zurowski <ross@rosszurowski.com>
Our previous Hostinfo logging was all as a side effect of telling
control. And it got marked as verbose (as it was)
This adds a one-time Hostinfo logging that's not verbose, early in
start-up.
Change-Id: I1896222b207457b9bb12ffa7cf361761fa4d3b3a
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
I was about to add a third copy, so unify them now instead.
Change-Id: I3b93896aa1249b1250a6b1df4829d57717f2311a
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
We're finding a bunch of host operating systems/firewalls interact poorly
with peerapi. We either get ICMP errors from the host or users need to run
commands to allow the peerapi port:
https://github.com/tailscale/tailscale/issues/3842#issuecomment-1025133727
... even though the peerapi should be an internal implementation detail.
Rather than fight the host OS & firewalls, this change handles the
server side of peerapi entirely in netstack (except on iOS), so it
never makes its way to the host OS where it might be messed with. Two
main downsides are:
1) netstack isn't as fast, but we don't really need speed for peerapi.
And actually, with fewer trips to/from the kernel, we might
actually make up for some of the netstack performance loss by
staying in userspace.
2) tcpdump / Wireshark etc packet captures will no longer see the peerapi
traffic. Oh well. Crawshaw's been wanting to add packet capture server
support to tailscaled, so we'll probably do that sooner now.
A future change might also then use peerapi for the client-side
(except on iOS).
Updates #3842 (probably fixes, as well as many exit node issues I bet)
Change-Id: Ibc25edbb895dc083d1f07bd3cab614134705aa39
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
Disabled by default.
To use, run tailscaled with:
TS_SSH_ALLOW_LOGIN=you@bar.com
And enable with:
$ TAILSCALE_USE_WIP_CODE=true tailscale up --ssh=true
Then ssh [any-user]@[your-tailscale-ip] for a root bash shell.
(both the "root" and "bash" part are temporary)
Updates #3802
Change-Id: I268f8c3c95c8eed5f3231d712a5dc89615a406f0
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
A new package can also later record/report which knobs are checked and
set. It also makes the code cleaner & easier to grep for env knobs.
Change-Id: Id8a123ab7539f1fadbd27e0cbeac79c2e4f09751
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
Fixes#3660
RELNOTE=MagicDNS now works over IPv6 when CGNAT IPv4 is disabled.
Change-Id: I001e983df5feeb65289abe5012dedd177b841b45
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
Make shrinkDefaultRoute a pure function.
Instead of calling interfaceRoutes, accept that information as parameters.
Hard-code those parameters in TestShrinkDefaultRoute.
Fixes#3580
Signed-off-by: Josh Bleecher Snyder <josh@tailscale.com>
One option was to just hide "offline" in the text output, but that
doesn't fix the JSON output.
The next option was to lie and say it's online in the JSON (which then
fixes the "offline" in the text output).
But instead, this sets the self node's "Online" to whether we're in an
active map poll.
Fixes#3564
Change-Id: I9b379989bd14655198959e37eec39bb570fb814a
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
magicsock was hanging onto its netmap on logout,
which caused tailscale status to display partial
information about a bunch of zombie peers.
After logout, there should be no peers.
Signed-off-by: Josh Bleecher Snyder <josh@tailscale.com>
I'm sick of this flaking. Even if this isn't the right fix, it
stops the alert fatigue.
Updates #3020
Change-Id: I4001c127d78f1056302f7741adec34210a72ee61
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
And it updates the build tag style on a couple files.
Change-Id: I84478d822c8de3f84b56fa1176c99d2ea5083237
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
It's been a bunch of releases now since the TailscaleIPs slice
replacement was added.
Change-Id: I3bd80e1466b3d9e4a4ac5bedba8b4d3d3e430a03
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
Allow users of CallbackRouter to supply a GetBaseConfig
implementation. This is expected to be used on Android,
which currently lacks both a) platform support for
Split-DNS and b) a way to retrieve the current DNS
servers.
iOS/macOS also use the CallbackRouter but have platform
support for SplitDNS, so don't need getBaseConfig.
Updates https://github.com/tailscale/tailscale/issues/2116
Updates https://github.com/tailscale/tailscale/issues/988
Signed-off-by: Denton Gentry <dgentry@tailscale.com>
To make ExitDNS cheaper.
Might not finish client-side support in December before 1.20, but at
least server support can start rolling out ahead of clients being
ready for it.
Tested with curl against peerapi.
Updates #1713
Change-Id: I676fed5fb1aef67e78c542a3bc93bddd04dd11fe
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
If the user has a "Taildrop" shared folder on startup and
the "tailscale" system user has read/write access to it,
then the user can "tailscale file cp" to their NAS.
Updates #2179 (would be fixes, but not super ideal/easy yet)
Change-Id: I68e59a99064b302abeb6d8cc84f7d2a09f764990
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
And simplify, unexport some tsdial/netstack stuff in the the process.
Fixes#3475
Change-Id: I186a5a5cbd8958e25c075b4676f7f6e70f3ff76e
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
This starts to refactor tsdial.Dialer's name resolution to have
different stages: in-memory MagicDNS vs system resolution. A future
change will plug in ExitDNS resolution.
This also plumbs a Dialer into netstack and unexports the dnsMap
internals.
And it removes some of the async AddNetworkMapCallback usage and
replaces it with synchronous updates of the Dialer's netmap
from LocalBackend, since the LocalBackend has the Dialer too.
Updates #3475
Change-Id: Idcb7b1169878c74f0522f5151031ccbc49fe4cb4
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
Without this, enabling an exit node immediately blackholes all traffic,
but doesn't correctly let it flow to the exit node until the next netmap
update.
Fixes#3447
Signed-off-by: David Anderson <danderson@tailscale.com>
In prep for moving stuff out of LocalBackend.
Change-Id: I9725aa9c3ebc7275f8c40e040b326483c0340127
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
Not done yet, but this move more of the outbound dial special casing
from random packages into tsdial, which aspires to be the one unified
place for all outbound dialing shenanigans.
Then this plumbs it all around, so everybody is ultimately
holding on to the same dialer.
As of this commit, macOS/iOS using an exit node should be able to
reach to the exit node's DoH DNS proxy over peerapi, doing the sockopt
to stay within the Network Extension.
A number of steps remain, including but limited to:
* move a bunch more random dialing stuff
* make netstack-mode tailscaled be able to use exit node's DNS proxy,
teaching tsdial's resolver to use it when an exit node is in use.
Updates #1713
Change-Id: I1e8ee378f125421c2b816f47bc2c6d913ddcd2f5
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
Don't be a DoH DNS server to peers unless the Tailnet admin has permitted
that peer autogroup:internet access.
Updates #1713
Change-Id: Iec69360d8e4d24d5187c26904b6a75c1dabc8979
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
If IP forwarding is disabled globally, but enabled per-interface on all interfaces,
don't complain. If only some interfaces have forwarding enabled, warn that some
subnet routing/exit node traffic may not work.
Fixes#1586
Signed-off-by: David Anderson <danderson@tailscale.com>
We were missing an argument here.
Also, switch to %q, in case anything weird
is happening with these strings.
Updates tailscale/corp#461
Signed-off-by: Josh Bleecher Snyder <josh@tailscale.com>
Fixes regression from 81cabf48ec which made
all map errors be sent to the frontend UI.
Fixes#3230
Change-Id: I7f142c801c7d15e268a24ddf901c3e6348b6729c
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
For debugging Synology. Like the existing goroutines handler, in that
it's owner-only.
Change-Id: I852f0626be8e1c0b6794c1e062111d14adc3e6ac
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
github.com/go-multierror/multierror served us well.
But we need a few feature from it (implement Is),
and it's not worth maintaining a fork of such a small module.
Instead, I did a clean room implementation inspired by its API.
Signed-off-by: Josh Bleecher Snyder <josh@tailscale.com>
At least until js/wasm starts using browser LocalStorage or something.
But for the foreseeable future, any login from a browser should
be considered ephemeral as the tab can close at any time and lose
the wireguard key, never to be seen again.
Updates #3157
Change-Id: I6c410d86dc7f9f233c3edd623313d9dee2085aac
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
iOS and Android no longer use these. They both now (as of today)
use the hostinfo.SetFoo setters instead.
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
Turns out the iOS client has been only sending the OS version it first
started at. This whole hostinfo-via-prefs mechanism was never a good idea.
Start removing it.
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
This fixes "tailscale cert" on Synology where the var directory is
typically like /volume2/@appdata/Tailscale, or any other tailscaled
user who specifies a non-standard state file location.
This is a interim fix on the way to #2932.
Fixes#2927
Updates #2932
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
So if the control plane knows that something's broken about the node, it can
include problem(s) in MapResponse and "tailscale status" will show it.
(and GUIs in the future, as it's in ipnstate.Status/JSON)
This also bumps the MapRequest.Version, though it's not strictly
required. Doesn't hurt.
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
LocalBackend.Shutdown's docs say:
> The backend can no longer be used after Shutdown returns.
Nevertheless, TestStateMachine blithely calls Shutdown, talks some smack,
and continues on, expecting things to work. Other uses of Shutdown
in the codebase are as intended.
Things mostly kinda work anyway, except that the wgengine.Engine has been
shut down, so calls to Reconfig fail. Those get logged:
> local.go:603: wgengine status error: engine closing; no status
but otherwise ignored.
However, the Reconfig failure caused one fewer call to pause/unpause
than normal. Now the assertCalls lines match the equivalent ones
earlier in the test.
I don't see an obvious correct replacement for Shutdown in the context
of this test; I'm not sure entirely what it is trying to accomplish.
It is possible that many of the tests remaining after the prior call
to Shutdown are now extraneous. They don't harm anything, though,
so err on the side of safety and leave them for now.
Signed-off-by: Josh Bleecher Snyder <josh@tailscale.com>
Use helpers and variadic functions to make the call sites
a lot easier to read, since they occur a lot.
Signed-off-by: Josh Bleecher Snyder <josh@tailscale.com>
Concurrent calls to LocalBackend.setWgengineStatus
could result in some of the status updates being dropped.
This was exacerbated by 92077ae78c,
which increases the probability of concurrent status updates,
causing test failures (tailscale/corp#2579).
It's going to take a bit of work to fix this test.
The ipnlocal state machine is difficult to reason about,
particularly in the face of concurrency.
We could fix the test trivially by throwing a new mutex around
setWgengineStatus to serialize calls to it,
but I'd like to at least try to do better than cosmetics.
In the meantime, commit the test.
Signed-off-by: Josh Bleecher Snyder <josh@tailscale.com>
In prep for other bug fixes & tests. It's hard to test when it was
intermingled into LocalBackend.authReconfig.
Now it's a pure function.
And rename variable 'uc' (user config?) to the since idiomatic
'prefs'.
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
We currently plumb full URLs for DNS resolvers from the control server
down to the client. But when we pass the values into the net/dns
package, we throw away any URL that isn't a bare IP. This commit
continues the plumbing, and gets the URL all the way to the built in
forwarder. (It stops before plumbing URLs into the OS configurations
that can handle them.)
For #2596
Signed-off-by: David Crawshaw <crawshaw@tailscale.com>
* Revert "Revert "types/key: add MachinePrivate and MachinePublic.""
This reverts commit 61c3b98a24.
Signed-off-by: David Anderson <danderson@tailscale.com>
* types/key: add ControlPrivate, with custom serialization.
ControlPrivate is just a MachinePrivate that serializes differently
in JSON, to be compatible with how the Tailscale control plane
historically serialized its private key.
Signed-off-by: David Anderson <danderson@tailscale.com>
Plumb throughout the codebase as a replacement for the mixed use of
tailcfg.MachineKey and wgkey.Private/Public.
Signed-off-by: David Anderson <danderson@tailscale.com>
And add health check errors to ipnstate.Status (tailscale status --json).
Updates #2746
Updates #2775
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
The number of packet filters can grow very large,
so this log entry can be very large.
We can get the packet filter server-side,
so reduce verbosity here to just the number of filters present.
Signed-off-by: Josh Bleecher Snyder <josh@tailscale.com>
Now that we have the easier-to-parse go:build build tags,
it is straightforward to simplify them. Yay.
Signed-off-by: Josh Bleecher Snyder <josh@tailscale.com>
The fact that Hash returns a [sha256.Size]byte leaks details about
the underlying hash implementation. This could very well be any other
hashing algorithm with a possible different block size.
Abstract this implementation detail away by declaring an opaque type
that is comparable. While we are changing the signature of UpdateHash,
rename it to just Update to reduce stutter (e.g., deephash.Update).
Signed-off-by: Joe Tsai <joetsai@digital-static.net>
With this, I can now:
* install Tailscale
* stop the GUI
* net stop Tailscale
* net start Tailscale
* tailscale up --unattended
(where the middle three steps simulate what would happen on a Windows
Server Core machine without a GUI)
Fixes#2137
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
Regression from 6d10655dc3, which added
UpdatePrefs but didn't write it out to disk.
I'd planned on adding tests to state_test.go which is why I'd earlier
added 46896a9311 to prepare for making
such persistence tests easier to write, but turns out state_test.go
didn't even test UpdatePrefs, so I'm staying out of there.
Instead, this is tested using integration tests.
Fixes#2321
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
We can't access b.netMap without holding b.mu.
We already grabbed it earlier in the function with the lock held.
Introduced in Nov 2020 in 7ea809897d.
Discovered during stress testing.
Apparently it's a pretty rare?
Signed-off-by: Josh Bleecher Snyder <josh@tailscale.com>
After allowing for custom DERP maps, it's convenient to be able to see their latency in
netcheck. This adds a query to the local tailscaled for the current DERPMap.
Updates #1264
Signed-off-by: julianknodt <julianknodt@gmail.com>
We were crashing on in initPeerAPIListener when called from
authReconfig when b.netMap is nil. But authReconfig already returns
before the call to initPeerAPIListener when b.netMap is nil, but it
releases the b.mu mutex before calling initPeerAPIListener which
reacquires it and assumes it's still nil.
The only thing that can be setting it to nil is setNetMapLocked, which
is called by ResetForClientDisconnect, Logout/logout, or Start, all of
which can happen during an authReconfig.
So be more defensive.
Fixes#1996
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
We used to use "redo" for that, but it was pretty vague.
Also, fix the build tags broken in interfaces_default_route_test.go from
a9745a0b68, moving those Linux-specific
tests to interfaces_linux_test.go.
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
The resulting empty Prefs had AllowSingleHosts=false and
Routeall=false, so that on iOS if you did these steps:
- Login and leave running
- Terminate the frontend
- Restart the frontend (fast path restart, missing prefs)
- Set WantRunning=false
- Set WantRunning=true
...then you would have Tailscale running, but with no routes. You would
also accidentally disable the ExitNodeID/IP prefs (symptom: the current
exit node setting didn't appear in the UI), but since nothing
else worked either, you probably didn't notice.
The fix was easy enough. It turns out we already knew about the
problem, so this also fixes one of the BUG entries in state_test.
Fixes: #1918 (BUG-1) and some as-yet-unreported bugs with exit nodes.
Signed-off-by: Avery Pennarun <apenwarr@tailscale.com>
Previously, there was no server round trip required to log out, so when
you asked ipnlocal to Logout(), it could clear the netmap immediately
and switch to NeedsLogin state.
In v1.8, we added a true Logout operation. ipn.Logout() would trigger
an async cc.StartLogout() and *also* immediately switch to NeedsLogin.
Unfortunately, some frontends would see NeedsLogin and immediately
trigger a new StartInteractiveLogin() operation, before the
controlclient auth state machine actually acted on the Logout command,
thus accidentally invalidating the entire logout operation, retaining
the netmap, and violating the user's expectations.
Instead, add a new LogoutFinished signal from controlclient
(paralleling LoginFinished) and, upon starting a logout, don't update
the ipn state machine until it's received.
Updates: #1918 (BUG-2)
Signed-off-by: Avery Pennarun <apenwarr@tailscale.com>
On clean installs we didn't set use iptables, but during upgrades it
looks like we could use old prefs that directed us to go into the iptables
paths that might fail on Synology.
Updates #1995Fixestailscale/tailscale-synology#57 (I think)
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
This leads to a cleaner separation of intent vs. implementation
(Routes is now the only place specifying who handles DNS requests),
and allows for cleaner expression of a configuration that creates
MagicDNS records without serving them to the OS.
Signed-off-by: David Anderson <danderson@tailscale.com>
This code path is very tricky since it was originally designed for the
"re-authenticate to refresh my keys" use case, which didn't want to
lose the original session even if the refresh cycle failed. This is why
it acts differently from the Logout(); Login(); case.
Maybe that's too fancy, considering that it probably never quite worked
at all, for switching between users without logging out first. But it
works now.
This was more invasive than I hoped, but the necessary fixes actually
removed several other suspicious BUG: lines from state_test.go, so I'm
pretty confident this is a significant net improvement.
Fixestailscale/corp#1756.
Signed-off-by: Avery Pennarun <apenwarr@tailscale.com>
If the engine was shutting down from a previous session
(e.closing=true), it would return an error code when trying to get
status. In that case, ipnlocal would never unblock any callers that
were waiting on the status.
Not sure if this ever happened in real life, but I accidentally
triggered it while writing a test.
Signed-off-by: Avery Pennarun <apenwarr@tailscale.com>
Yes, it printed, but that was an implementation detail for hashing.
And coming optimization will make it print even less.
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
Needed for the "up checker" to map back from exit node stable IDs (the
ipn.Prefs.ExitNodeID) back to an IP address in error messages.
But also previously requested so people can use it to then make API
calls. The upcoming "tailscale admin" subcommand will probably need it
too.
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
This is needed because the original opts.Prefs field was at some point
subverted for use in frontend->backend state migration for backward
compatibility on some platforms. We still need that feature, but we
also need the feature of providing the full set of prefs from
`tailscale up`, *not* including overwriting the prefs.Persist keys, so
we can't use the original field from `tailscale up`.
`tailscale up` had attempted to compensate for that by doing SetPrefs()
before Start(), but that violates the ipn.Backend contract, which says
you should call Start() before anything else (that's why it's called
Start()). As a result, doing SetPrefs({ControlURL=...,
WantRunning=true}) would cause a connection to the *previous* control
server (because WantRunning=true), and then connect to the *new*
control server only after running Start().
This problem may have been avoided before, but only by pure luck.
It turned out to be relatively harmless since the connection to the old
control server was immediately closed and replaced anyway, but it
created a race condition that could have caused spurious notifications
or rejected keys if the server responded quickly.
As already covered by existing TODOs, a better fix would be to have
Start() get out of the business of state migration altogether. But
we're approaching a release so I want to make the minimum possible fix.
Fixes#1840.
Signed-off-by: Avery Pennarun <apenwarr@tailscale.com>
Per discussion, we want to have only one test assertion library,
and we want to start by exploring quicktest.
This was a mostly mechanical translation.
I think we could make this nicer by defining a few helper
closures at the beginning of the test. Later.
Signed-off-by: Josh Bleecher Snyder <josharian@gmail.com>
This removes the NewLocalBackendWithClientGen constructor added in
b4d04a065f and instead adds
LocalBackend.SetControlClientGetterForTesting, mirroring
LocalBackend.SetHTTPTestClient. NewLocalBackendWithClientGen was
weird in being exported but taking an unexported type. This was noted
during code review:
https://github.com/tailscale/tailscale/pull/1818#discussion_r623155669
which ended in:
"I'll leave it for y'all to clean up if you find some way to do it elegantly."
This is more idiomatic.
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
Without this, macOS would fail to display its menu state correctly if you
started it while !WantRunning. It relies on the netmap in order to show
the logged-in username.
Signed-off-by: Avery Pennarun <apenwarr@tailscale.com>
There was logic that would make a "down" tailscale backend (ie.
!WantRunning) refuse to do any network activity. Unfortunately, this
makes the macOS and iOS UI unable to render correctly if they start
while !WantRunning.
Now that we have Prefs.LoggedOut, use that instead. So `tailscale down`
will still allow the controlclient to connect its authroutine, but
pause the maproutine. `tailscale logout` will entirely stop all
activity.
This new behaviour is not obviously correct; it's a bit annoying that
`tailsale down` doesn't terminate all activity like you might expect.
Maybe we should redesign the UI code to render differently when
disconnected, and then revert this change.
Signed-off-by: Avery Pennarun <apenwarr@tailscale.com>
EditPrefs should be just a wrapper around the action of changing prefs,
but someone had added a side effect of calling Login() sometimes. The
side effect happened *after* running the state machine, which would
sometimes result in us going into NeedsLogin immediately before calling
cc.Login().
This manifested as the macOS app not being able to Connect if you
launched it with LoggedOut=false and WantRunning=false. Trying to
Connect() would sent us to the NeedsLogin state instead.
Signed-off-by: Avery Pennarun <apenwarr@tailscale.com>
A very long unit test that verifies the way the controlclient and
ipn.Backend interact.
This is a giant sequential test of the state machine. The test passes,
but only because it's asserting all the wrong behaviour. I marked all
the behaviour I think is wrong with BUG comments, and several
additional test opportunities with TODO.
Note: the new test supercedes TestStartsInNeedsLoginState, which was
checking for incorrect behaviour (although the new test still checks
for the same incorrect behaviour) and assumed .Start() would converge
before returning, which it happens to do, but only for this very
specific case, for the current implementation. You're supposed to wait
for the notifications.
Updates: tailscale/corp#1660
Signed-off-by: Avery Pennarun <apenwarr@tailscale.com>
With this change, shared node names resolve correctly on split DNS-supporting
operating systems.
Fixestailscale/corp#1706
Signed-off-by: David Anderson <danderson@tailscale.com>
Brad Fitzpatrick