In particular on Windows, the `transport.TPMCloser` we get is not safe
for concurrent use. This is especially noticeable because
`tpm.attestationKey.Clone` uses the same open handle as the original
key. So wrap the operations on ak.tpm with a mutex and make a deep copy
with a new connection in Clone.
Updates #15830
Updates #17662
Updates #17644
Signed-off-by: Andrew Lytvynov <awly@tailscale.com>
Whenever running on a platform that has a TPM (and tailscaled can access
it), default to encrypting the state. The user can still explicitly set
this flag to disable encryption.
Updates https://github.com/tailscale/corp/issues/32909
Signed-off-by: Andrew Lytvynov <awly@tailscale.com>
We will need this for unmarshaling node prefs: use the zero
HardwareAttestationKey implementation when parsing and later check
`IsZero` to see if anything was loaded.
Updates #15830
Signed-off-by: Andrew Lytvynov <awly@tailscale.com>
We can only register one key implementation per process. When running on
macOS or Android, trying to register a separate key implementation from
feature/tpm causes a panic.
Updates #15830
Signed-off-by: Andrew Lytvynov <awly@tailscale.com>
This function is behind a sync.Once so we should only see errors at
startup. In particular the error from `open` is useful to diagnose why
TPM might not be accessible.
Updates #15830
Signed-off-by: Andrew Lytvynov <awly@tailscale.com>
The tpmrm0 is a kernel-managed version of tpm0 that multiplexes multiple
concurrent connections. The basic tpm0 can only be accessed by one
application at a time, which can be pretty unreliable.
Updates #15830
Signed-off-by: Andrew Lytvynov <awly@tailscale.com>
Report whether the client is configured with state encryption (which
varies by platform and can be optional on some). Wire it up to
`--encrypt-state` in tailscaled, which is set for Linux/Windows, and set
defaults for other platforms. Macsys will also report this if full
Keychain migration is done.
Updates #15830
Signed-off-by: Andrew Lytvynov <awly@tailscale.com>
This method is only needed to migrate between store.FileStore and
tpm.tpmStore. We can make a runtime type assertion instead of
implementing an unused method for every platform.
Updates #15830
Signed-off-by: Andrew Lytvynov <awly@tailscale.com>
Add a new `--encrypt-state` flag to `cmd/tailscaled`. Based on that
flag, migrate the existing state file to/from encrypted format if
needed.
Updates #15830
Signed-off-by: Andrew Lytvynov <awly@tailscale.com>
Registering a new store is cheap, it just adds a map entry. No need to
lazy-init it with sync.Once and an intermediate slice holding init
functions.
Updates #cleanup
Signed-off-by: Andrew Lytvynov <awly@tailscale.com>
Instead of using the version package (which depends on
tailcfg.CurrentCapabilityVersion) to get the git commit hash, do it
directly using debug.BuildInfo. This way, when changing struct fields in
tailcfg, we can successfully `go generate` it without compiler errors.
Updates #9634
Updates https://github.com/tailscale/corp/issues/26717
Signed-off-by: Andrew Lytvynov <awly@tailscale.com>
https://github.com/tailscale/tailscale/pull/15395 changed the logic to
skip `EditPrefs` when the platform doesn't support auto-updates. But the
old logic would only fail `EditPrefs` if the auto-update value was
`true`. If it was `false`, `EditPrefs` would succeed and store `false`
in prefs. The new logic will keep the value `unset` even if the tailnet
default is `false`.
Fixes#15691
Signed-off-by: Andrew Lytvynov <awly@tailscale.com>
Default tags to `$TAGS` if set, so that people can choose arbitrary
subsets of features.
Updates #12614
Signed-off-by: Andrew Lytvynov <awly@tailscale.com>
When we have an old cert that is being rotated, include it in the order.
If we're in the ARI-recommended rotation window, LE should exclude us
from rate limits. If we're not within that window, the order still
succeeds, so there's no risk in including the old cert.
Fixes#15542
Signed-off-by: Andrew Lytvynov <awly@tailscale.com>
This method uses `path.Join` to build the URL. Turns out with 1.24 this
started stripping consecutive "/" characters, so "http://..." in baseURL
becomes "http:/...".
Also, `c.Tailnet` is a function that returns `c.tailnet`. Using it as a
path element would encode as a pointer instead of the tailnet name.
Finally, provide a way to prevent escaping of path elements e.g. for `?`
in `acl?details=1`.
Updates #15015
Signed-off-by: Andrew Lytvynov <awly@tailscale.com>
`routeAdvertiser` is the `iplocal.LocalBackend`. Calls to
`Advertise/UnadvertiseRoute` end up calling `EditPrefs` which in turn
calls `authReconfig` which finally calls `readvertiseAppConnectorRoutes`
which calls `AppConnector.DomainRoutes` and gets stuck on a mutex that
was already held when `routeAdvertiser` was called.
Make all calls to `routeAdvertiser` in `app.AppConnector` go through the
execqueue instead as a short-term fix.
Updates tailscale/corp#25965
Signed-off-by: Andrew Lytvynov <awly@tailscale.com>
Co-authored-by: Irbe Krumina <irbe@tailscale.com>
When running via tsnet, c2n will be hooked up so requests to update can
reach the node. But it will then apply whatever OS-specific update
function, upgrading the local tailscaled instead.
We can't update tsnet automatically, so refuse it.
Fixes#14892
Signed-off-by: Andrew Lytvynov <awly@tailscale.com>
If we fail to parse the upstream DNS response in an app connector, we
might miss new IPs for the target domain. Log parsing errors to be able
to diagnose that.
Updates #14606
Signed-off-by: Andrew Lytvynov <awly@tailscale.com>
* Reapply "ipn/ipnlocal: re-advertise appc routes on startup (#14609)"
This reverts commit 51adaec35a.
Signed-off-by: Andrew Lytvynov <awly@tailscale.com>
* ipn/ipnlocal: fix a deadlock in readvertiseAppConnectorRoutes
Don't hold LocalBackend.mu while calling the methods of
appc.AppConnector. Those methods could call back into LocalBackend and
try to acquire it's mutex.
Fixes https://github.com/tailscale/corp/issues/25965Fixes#14606
Signed-off-by: Andrew Lytvynov <awly@tailscale.com>
---------
Signed-off-by: Andrew Lytvynov <awly@tailscale.com>
There's at least one example of stored routes and advertised routes
getting out of sync. I don't know how they got there yet, but this would
backfill missing advertised routes on startup from stored routes.
Also add logging in LocalBackend.AdvertiseRoute to record when new
routes actually get put into prefs.
Updates #14606
Signed-off-by: Andrew Lytvynov <awly@tailscale.com>
Bump the versions to pick up some CVE patches. They don't affect us, but
customer scanners will complain.
Updates #cleanup
Signed-off-by: Andrew Lytvynov <awly@tailscale.com>
If previousEtag is empty, then we assume control ACLs were not modified
manually and push the local ACLs. Instead, we defaulted to localEtag
which would be different if local ACLs were different from control.
AFAIK this was always buggy, but never reported?
Fixes#14295
Signed-off-by: Andrew Lytvynov <awly@tailscale.com>
The v2 endpoint supports HTTP/2 bidirectional streaming and acks for
received bytes. This is used to detect when a recorder disappears to
more quickly terminate the session.
Updates https://github.com/tailscale/corp/issues/24023
Signed-off-by: Andrew Lytvynov <awly@tailscale.com>
By counting "/" elements in the pattern we catch many scenarios, but not
the root-level handler. If either of the patterns is "/", compare the
pattern length to pick the right one.
Updates https://github.com/tailscale/corp/issues/8027
Signed-off-by: Andrew Lytvynov <awly@tailscale.com>
When tailscaled restarts and our watch connection goes down, we get
stuck in an infinite loop printing `ipnbus error: EOF` (which ended up
consuming all the disk space on my laptop via the log file). Instead,
handle errors in `watchIPNBus` and reconnect after a short delay.
Updates #1708
Signed-off-by: Andrew Lytvynov <awly@tailscale.com>
In a situation when manual edits are made on the admin panel, around the
GitOps process, the pusher will be stuck if `--fail-on-manual-edits` is
set, as expected.
To recover from this, there are 2 options:
1. revert the admin panel changes to get back in sync with the code
2. check in the manual edits to code
The former will work well, since previous and local ETags will match
control ETag again. The latter will still fail, since local and control
ETags match, but previous does not.
For this situation, check the local ETag against control first and
ignore previous when things are already in sync.
Updates https://github.com/tailscale/corp/issues/22177
Signed-off-by: Andrew Lytvynov <awly@tailscale.com>
For cases where users want to be extra careful about not overwriting
manual changes, add a flag to hard-fail. This is only useful if the etag
cache is persistent or otherwise reliable. This flag should not be used
in ephemeral CI workers that won't persist the cache.
Updates https://github.com/tailscale/corp/issues/22177
Signed-off-by: Andrew Lytvynov <awly@tailscale.com>
Some users run "tailscale cert" in a cron job to renew their
certificates on disk. The time until the next cron job run may be long
enough for the old cert to expire with our default heristics.
Add a `--min-validity` flag which ensures that the returned cert is
valid for at least the provided duration (unless it's longer than the
cert lifetime set by Let's Encrypt).
Updates #8725
Signed-off-by: Andrew Lytvynov <awly@tailscale.com>
We added a workaround for --wait, but didn't confirm the other flags,
which were added in systemd 235 and 236. Check systemd version for
deciding when to set all 3 flags.
Fixes#12136
Signed-off-by: Andrew Lytvynov <awly@tailscale.com>
While `clientupdate.Updater` won't be able to apply updates on macsys,
we use `clientupdate.CanAutoUpdate` to gate the EditPrefs endpoint in
localAPI. We should allow the GUI client to set AutoUpdate.Apply on
macsys for it to properly get reported to the control plane. This also
allows the tailnet-wide default for auto-updates to propagate to macsys
clients.
Updates https://github.com/tailscale/corp/issues/21339
Signed-off-by: Andrew Lytvynov <awly@tailscale.com>
We do not support specific version updates or track switching on macOS.
Do not populate the flag to avoid confusion.
Updates #cleanup
Signed-off-by: Andrew Lytvynov <awly@tailscale.com>
Previously, we would only compare the current version to resolved latest
version for track. When running `tailscale update --track=stable` from
an unstable build, it would almost always fail because the stable
version is "older". But we should support explicitly switching tracks
like that.
Fixes#12347
Signed-off-by: Andrew Lytvynov <awly@tailscale.com>
We assume most containers are immutable and don't expect tailscale
running in them to auto-update. But there's no reason to prohibit it
outright.
Ignore the tailnet-wide default auto-update setting in containers, but
allow local users to turn on auto-updates via the CLI.
RELNOTE=Auto-updates are allowed in containers, but ignore the tailnet-wide default.
Fixes#12292
Signed-off-by: Andrew Lytvynov <awly@tailscale.com>
Clean up the updater goroutine on shutdown, in addition to doing that on
backend state change. This fixes a goroutine leak on shutdown in tests.
Updates #cleanup
When the client is disconnected from control for any reason (typically
just turned off), we should still attempt to update if auto-updates are
enabled. This may help users who turn tailscale on infrequently for
accessing resources.
RELNOTE: Apply auto-updates even if the node is down or disconnected
from the coordination server.
Updates #12117
Signed-off-by: Andrew Lytvynov <awly@tailscale.com>
Alpine APK repos are versioned, and contain different package sets.
Older APK releases and repos don't have the latest tailscale package.
When we report "no update available", check whether pkgs.tailscale.com
has a newer tarball release. If it does, it's possible that the system
is on an older Alpine release. Print additional messages to suggest the
user to upgrade their OS.
Fixes#11309
Signed-off-by: Andrew Lytvynov <awly@tailscale.com>
The `--wait` flag for `systemd-run` was added in systemd 232. While it
is quite old, it doesn't hurt to special-case them and skip the `--wait`
flag. The consequence is that we lose the update command output in logs,
but at least auto-updates will work.
Fixes#12136
Signed-off-by: Andrew Lytvynov <awly@tailscale.com>
After some analysis, stateful filtering is only necessary in tailnets
that use `autogroup:danger-all` in `src` in ACLs. And in those cases
users explicitly specify that hosts outside of the tailnet should be
able to reach their nodes. To fix local DNS breakage in containers, we
disable stateful filtering by default.
Updates #12108
Signed-off-by: Andrew Lytvynov <awly@tailscale.com>
This way the default gets populated on first start, when no existing
state exists to migrate. Also fix `ipn.PrefsFromBytes` to preserve empty
fields, rather than layering `NewPrefs` values on top.
Updates https://github.com/tailscale/corp/issues/19623
Signed-off-by: Andrew Lytvynov <awly@tailscale.com>
The tailscale package is in the community Alpine repo. Check if it's
commented out in `/etc/apk/repositories` and run `setup-apkrepos -c -1`
if it's not.
Fixes#11263
Signed-off-by: Andrew Lytvynov <awly@tailscale.com>
Since we already track active SSH connections, it's not hard to
proactively reject updates until those finish. We attempt to do the same
on the control side, but the detection latency for new connections is in
the minutes, which is not fast enough for common short sessions.
Handle a `force=true` query parameter to override this behavior, so that
control can still trigger an update on a server where some long-running
abandoned SSH session is open.
Updates https://github.com/tailscale/corp/issues/18556
Signed-off-by: Andrew Lytvynov <awly@tailscale.com>
Containers are typically immutable and should be updated as a whole (and
not individual packages within). Deny enablement of auto-updates in
containers.
Also, add the missing check in EditPrefs in LocalAPI, to catch cases
like tailnet default auto-updates getting enabled for nodes that don't
support it.
Updates #11544
Signed-off-by: Andrew Lytvynov <awly@tailscale.com>
We don't always have the same latest version for all platforms (like
with 1.64.2 is only Synology+Windows), so we should use the OS-specific
result from pkgs JSON response instead of the main Version field.
Updates #11795
Signed-off-by: Andrew Lytvynov <awly@tailscale.com>
Turns out, profileManager is not safe for concurrent use and I missed
all the locking infrastructure in LocalBackend, oops.
I was not able to reproduce the race even with `go test -count 100`, but
this seems like an obvious fix.
Fixes#11773
Signed-off-by: Andrew Lytvynov <awly@tailscale.com>
When both muxes match, and one of them is a wildcard "/" pattern (which
is common in browser muxes), choose the more specific pattern.
If both are non-wildcard matches, there is a pattern overlap, so return
an error.
Updates https://github.com/tailscale/corp/issues/8027
Signed-off-by: Andrew Lytvynov <awly@tailscale.com>
There's a vulnerability https://pkg.go.dev/vuln/GO-2024-2659 that
govulncheck flags, even though it's only reachable from tests and
cmd/sync-containers and cannot be exploited there.
Updates #cleanup
Signed-off-by: Andrew Lytvynov <awly@tailscale.com>
If the client uses the default Tailscale control URL, validate that all
PopBrowserURLs are under tailscale.com or *.tailscale.com. This reduces
the risk of a compromised control plane opening phishing pages for
example.
The client trusts control for many other things, but this is one easy
way to reduce that trust a bit.
Fixes#11393
Signed-off-by: Andrew Lytvynov <awly@tailscale.com>
According to
https://learn.microsoft.com/en-us/windows/win32/msi/standard-installer-command-line-options#promptrestart,
`/promptrestart` is ignored with `/quiet` is set, so msiexec.exe can
sometimes silently trigger a reboot. The best we can do to reduce
unexpected disruption is to just prevent restarts, until the user
chooses to do it. Restarts aren't normally needed for Tailscale updates,
but there seem to be some situations where it's triggered.
Updates #18254
Signed-off-by: Andrew Lytvynov <awly@tailscale.com>
The package info output can list multiple package versions, and not in
descending order. Find the newest version in the output, instead of the
first one.
Fixes#11309
Signed-off-by: Andrew Lytvynov <awly@tailscale.com>
When running as non-root non-operator user, you get this error:
```
$ tailscale serve 8080
Access denied: watch IPN bus access denied, must set ipn.NotifyNoPrivateKeys when not running as admin/root or operator
Use 'sudo tailscale serve 8080' or 'tailscale up --operator=$USER' to not require root.
```
It should fail, but the error message is confusing.
With this fix:
```
$ tailscale serve 8080
sending serve config: Access denied: serve config denied
Use 'sudo tailscale serve 8080' or 'tailscale up --operator=$USER' to not require root.
```
Updates #cleanup
Signed-off-by: Andrew Lytvynov <awly@tailscale.com>
ErrDenied was added in [our fork of
x/crypto/ssh](https://github.com/golang/crypto/commit/acc6f8fe8d618cba34d44e89fdde304f98c576df)
to short-circuit auth attempts once one fails.
In the case of our callbacks, this error is returned when SSH policy
check determines that a connection should not be allowed. Both
`NoClientAuthCallback` and `PublicKeyHandler` check the policy and will
fail anyway. The `fakePasswordHandler` returns true only if
`NoClientAuthCallback` succeeds the policy check, so it checks it
indirectly too.
The difference here is that a client might attempt all 2-3 auth methods
instead of just `none` but will fail to authenticate regardless.
Updates #8593
Signed-off-by: Andrew Lytvynov <awly@tailscale.com>
This type seems to be a migration shim for TCP tailscaled sockets
(instead of unix/windows pipes). The `port` field was never set, so it
was effectively used as a string (`path` field).
Remove the whole type and simplify call sites to pass the socket path
directly to `safesocket.Connect`.
Updates #cleanup
Signed-off-by: Andrew Lytvynov <awly@tailscale.com>
Run `staticcheck` with `U1000` to find unused code. This cleans up about
a half of it. I'll do the other half separately to keep PRs manageable.
Updates #cleanup
Signed-off-by: Andrew Lytvynov <awly@tailscale.com>
Using reflect.MethodByName disables some linked deadcode optimizations
and makes our binaries much bigger.
Difference before/after this commit:
```
-rwxr-xr-x 1 awly awly 30M Dec 19 15:28 tailscaled.after*
-rwxr-xr-x 1 awly awly 43M Dec 19 15:27 tailscaled.before*
```
Fixes#10627
Signed-off-by: Andrew Lytvynov <awly@tailscale.com>
When auto-update setting in local Prefs is unset, apply the tailnet
default value from control. This only happens once, when we apply the
default (or when the user manually overrides it), tailnet default no
longer affects the node.
Updates #16244
Signed-off-by: Andrew Lytvynov <awly@tailscale.com>
Instead of overloading the Version field, add an explicit Track field.
This fixes a bug where passing a track name in `args.Version` would keep
the track name in `updater.Version` and pass it down the code path to
commands like `apt-get install`. Now, `updater.Version` should always be
a version (or empty string).
Updates #cleanup
Signed-off-by: Andrew Lytvynov <awly@tailscale.com>
Some fields if `ipn.Prefs` are structs. `ipn.MaskedPrefs` has a single
level of boolean `*Set` flags, which doesn't map well to nested structs
within `ipn.Prefs`.
Change `MaskedPrefs` and `ApplyEdits` to support `FooSet` struct fields
that map to a nested struct of `ipn.Prefs` like `AutoUpdates`. Each
struct field in `MaskedPrefs` is just a bundle of more `Set` bool fields
or other structs. This allows you to have a `Set` flag for any
arbitrarily-nested field of `ipn.Prefs`.
Also, make `ApplyEdits` match fields between `Prefs` and `MaskedPrefs`
by name instead of order, to make it a bit less finicky. It's probably
slower but `ipn.ApplyEdits` should not be in any hot path.
As a result, `AutoUpdate.Check` and `AutoUpdate.Apply` fields don't
clobber each other when set individually.
Updates #16247
Signed-off-by: Andrew Lytvynov <awly@tailscale.com>
Andrew LytvynovandGitHub
