Commit Graph

153 Commits (861dc3631c4337c13d19308d9e0958d030bfcbf3)

Author SHA1 Message Date
Nick Khyl e7b5e8c8cd ipn/ipnserver: remove IdleTimeout
We no longer need this on Windows, and it was never required on other platforms.
It just results in more short-lived connections unless we use HTTP/2.

Updates tailscale/corp#18342

Signed-off-by: Nick Khyl <nickk@tailscale.com>
3 months ago
Nick Khyl 961ee321e8 ipn/{ipnauth,ipnlocal,ipnserver,localapi}: start baby step toward moving access checks from the localapi.Handler to the LocalBackend
Currently, we use PermitRead/PermitWrite/PermitCert permission flags to determine which operations are allowed for a LocalAPI client.
These checks are performed when localapi.Handler handles a request. Additionally, certain operations (e.g., changing the serve config)
requires the connected user to be a local admin. This approach is inherently racey and is subject to TOCTOU issues.
We consider it to be more critical on Windows environments, which are inherently multi-user, and therefore we prevent more than one
OS user from connecting and utilizing the LocalBackend at the same time. However, the same type of issues is also applicable to other
platforms when switching between profiles that have different OperatorUser values in ipn.Prefs.

We'd like to allow more than one Windows user to connect, but limit what they can see and do based on their access rights on the device
(e.g., an local admin or not) and to the currently active LoginProfile (e.g., owner/operator or not), while preventing TOCTOU issues on Windows
and other platforms. Therefore, we'd like to pass an actor from the LocalAPI to the LocalBackend to represent the user performing the operation.
The LocalBackend, or the profileManager down the line, will then check the actor's access rights to perform a given operation on the device
and against the current (and/or the target) profile.

This PR does not change the current permission model in any way, but it introduces the concept of an actor and includes some preparatory
work to pass it around. Temporarily, the ipnauth.Actor interface has methods like IsLocalSystem and IsLocalAdmin, which are only relevant
to the current permission model. It also lacks methods that will actually be used in the new model. We'll be adding these gradually in the next
PRs and removing the deprecated methods and the Permit* flags at the end of the transition.

Updates tailscale/corp#18342

Signed-off-by: Nick Khyl <nickk@tailscale.com>
3 months ago
Brad Fitzpatrick c6af5bbfe8 all: add test for package comments, fix, add comments as needed
Updates #cleanup

Change-Id: Ic4304e909d2131a95a38b26911f49e7b1729aaef
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
5 months ago
Brad Fitzpatrick 727c0d6cfd ipn/ipnserver: close a small race in ipnserver, ~simplify code
There was a small window in ipnserver after we assigned a LocalBackend
to the ipnserver's atomic but before we Start'ed it where our
initalization Start could conflict with API calls from the LocalAPI.

Simplify that a bit and lay out the rules in the docs.

Updates #12028

Change-Id: Ic5f5e4861e26340599184e20e308e709edec68b1
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
7 months ago
Brad Fitzpatrick 6d69fc137f ipn/{ipnlocal,localapi},wgengine{,/magicsock}: plumb health.Tracker
Down to 25 health.Global users. After this remains controlclient &
net/dns & wgengine/router.

Updates #11874
Updates #4136

Change-Id: I6dd1856e3d9bf523bdd44b60fb3b8f7501d5dc0d
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
7 months ago
Brad Fitzpatrick 7c1d6e35a5 all: use Go 1.22 range-over-int
Updates #11058

Change-Id: I35e7ef9b90e83cac04ca93fd964ad00ed5b48430
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
7 months ago
Will Norris 236531c5fc ipn/ipnserver: always allow Windows SYSTEM user to connect
When establishing connections to the ipnserver, we validate that the
local user is allowed to connect.  If Tailscale is currently being
managed by a different user (primarily for multi-user Windows installs),
we don't allow the connection.

With the new device web UI, the inbound connection is coming from
tailscaled itself, which is often running as "NT AUTHORITY\SYSTEM".
In this case, we still want to allow the connection, even though it
doesn't match the user running the Tailscale GUI. The SYSTEM user has
full access to everything on the system anyway, so this doesn't escalate
privileges.

Eventually, we want the device web UI to run outside of the tailscaled
process, at which point this exception would probably not be needed.

Updates tailscale/corp#16393

Signed-off-by: Will Norris <will@tailscale.com>
11 months ago
Matt Layher a217f1fccf all: fix nilness issues
Signed-off-by: Matt Layher <mdlayher@gmail.com>
12 months ago
Andrew Lytvynov 55cd5c575b
ipn/localapi: only perform local-admin check in serveServeConfig (#10163)
On unix systems, the check involves executing sudo, which is slow.
Instead of doing it for every incoming request, move the logic into
localapi serveServeConfig handler and do it as needed.

Updates tailscale/corp#15405

Signed-off-by: Andrew Lytvynov <awly@tailscale.com>
1 year ago
Andrew Lytvynov 9b158db2c6
ipn/localapi: require root or sudo+operator access for SetServeConfig (#10142)
For an operator user, require them to be able to `sudo tailscale` to use
`tailscale serve`. This is similar to the Windows elevated token check.

Updates tailscale/corp#15405

Signed-off-by: Andrew Lytvynov <awly@tailscale.com>
1 year ago
Aaron Klotz fbc18410ad ipn/ipnauth: improve the Windows token administrator check
(*Token).IsAdministrator is supposed to return true even when the user is
running with a UAC limited token. The idea is that, for the purposes of
this check, we don't care whether the user is *currently* running with
full Admin rights, we just want to know whether the user can
*potentially* do so.

We accomplish this by querying for the token's "linked token," which
should be the fully-elevated variant, and checking its group memberships.

We also switch ipn/ipnserver/(*Server).connIsLocalAdmin to use the elevation
check to preserve those semantics for tailscale serve; I want the
IsAdministrator check to be used for less sensitive things like toggling
auto-update on and off.

Fixes #10036

Signed-off-by: Aaron Klotz <aaron@tailscale.com>
1 year ago
Aaron Klotz 95671b71a6 ipn, safesocket: use Windows token in LocalAPI
On Windows, the idiomatic way to check access on a named pipe is for
the server to impersonate the client on its current OS thread, perform
access checks using the client's access token, and then revert the OS
thread's access token back to its true self.

The access token is a better representation of the client's rights than just
a username/userid check, as it represents the client's effective rights
at connection time, which might differ from their normal rights.

This patch updates safesocket to do the aforementioned impersonation,
extract the token handle, and then revert the impersonation. We retain
the token handle for the remaining duration of the connection (the token
continues to be valid even after we have reverted back to self).

Since the token is a property of the connection, I changed ipnauth to wrap
the concrete net.Conn to include the token. I then plumbed that change
through ipnlocal, ipnserver, and localapi as necessary.

I also added a PermitLocalAdmin flag to the localapi Handler which I intend
to use for controlling access to a few new localapi endpoints intended
for configuring auto-update.

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

Signed-off-by: Aaron Klotz <aaron@tailscale.com>
1 year ago
Andrew Dunham 60ab8089ff logpolicy, various: allow overriding log function
This allows sending logs from the "logpolicy" package (and associated
callees) to something other than the log package. The behaviour for
tailscaled remains the same, passing in log.Printf

Updates #8249

Signed-off-by: Andrew Dunham <andrew@du.nham.ca>
Change-Id: Ie1d43b75fa7281933d9225bffd388462c08a5f31
1 year ago
Brad Fitzpatrick 6e967446e4 tsd: add package with System type to unify subsystem init, discovery
This is part of an effort to clean up tailscaled initialization between
tailscaled, tailscaled Windows service, tsnet, and the mac GUI.

Updates #8036

Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
2 years 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
Maisem Ali 7300b908fb logpolicy: split out DialContext into a func
Updates tailscale/corp#10030

Signed-off-by: Maisem Ali <maisem@tailscale.com>
2 years ago
Will Norris 57a008a1e1 all: pass log IDs as the proper type rather than strings
This change focuses on the backend log ID, which is the mostly commonly
used in the client.  Tests which don't seem to make use of the log ID
just use the zero value.

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
Anton Tolchanov 53c4892841 ipn/ipnserver: propagate http.Serve error
This ensures that we capture error returned by `Serve` and exit with a
non-zero exit code if it happens.

Signed-off-by: Anton Tolchanov <anton@tailscale.com>
2 years ago
Brad Fitzpatrick 964d723aba ipn/{ipnserver,localapi}: fix InUseOtherUser handling with WatchIPNBus
Updates tailscale/corp#8222

Change-Id: I2d6fa6514c7b8d0f89fded35a2d44e7df27e6fb1
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
2 years ago
Brad Fitzpatrick 3b0de97e07 cmd/tailscaled: unify the two Windows paths + separate IPN server path
tailscaled on Windows had two entirely separate start-up paths for running
as a service vs in the foreground. It's been causing problems for ages.
This unifies the two paths, making them be the same as the path used
for every other platform.

Also, it uses the new async LocalBackend support in ipnserver.Server
so the Server can start serving HTTP immediately, even if tun takes
awhile to come up.

Updates #6535

Change-Id: Icc8c4f96d4887b54a024d7ac15ad11096b5a58cf
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
2 years ago
Brad Fitzpatrick e8cc78b1af ipn/ipnserver: change Server to let LocalBackend be supplied async
This is step 1 of de-special-casing of Windows and letting the
LocalAPI HTTP server start serving immediately, even while the rest of
the world (notably the Engine and its TUN device) are being created,
which can take a few to dozens of seconds on Windows.

With this change, the ipnserver.New function changes to not take an
Engine and to return immediately, not returning an error, and let its
Run run immediately. If its ServeHTTP is called when it doesn't yet
have a LocalBackend, it returns an error. A TODO in there shows where
a future handler will serve status before an engine is available.

Future changes will:

* delete a bunch of tailscaled_windows.go code and use this new API
* add the ipnserver.Server ServerHTTP handler to await the engine
  being available
* use that handler in the Windows GUI client

Updates #6522

Change-Id: Iae94e68c235e850b112a72ea24ad0e0959b568ee
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
2 years ago
Brad Fitzpatrick 8049053f86 ipn/*: make new WindowsUserID type to consolidate docs
The "userID is empty everywhere but Windows" docs on lots of places
but not everywhere while using just a string type was getting
confusing. This makes a new type to wrap up those rules, however
weird/historical they might be.

Change-Id: I142e85a8e38760988d6c0c91d0efecedade81b9b
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
2 years ago
Brad Fitzpatrick f45106d47c ipn/ipnserver: move Windows-specific code to tailscaled_windows.go
We'll eventually remove it entirely, but for now move get it out of ipnserver
where it's distracting and move it to its sole caller.

Updates #6522

Change-Id: I9c6f6a91bf9a8e3c5ea997952b7c08c81723d447
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
2 years ago
Brad Fitzpatrick b0545873e5 ipn/ipnserver: remove protoSwitchConn shenanigans; just use http.Server early
Now that everything's just HTTP, there's no longer a need to have a
header-sniffing net.Conn wraper that dispatches which route to
take. Refactor to just use an http.Server earlier instead.

Updates #6417

Change-Id: I12a2054db4e56f48660c46f81233db224fdc77cb
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
2 years ago
Brad Fitzpatrick f3ba268a96 ipn/ipnserver: move BabysitProc to tailscaled_windows.go
It's only used by Windows. No need for it to be in ipn/ipnserver,
which we're trying to trim down.

Change-Id: Idf923ac8b6cdae8b5338ec26c16fb8b5ea548071
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
2 years ago
Brad Fitzpatrick 7e016c1d90 ipn/ipnserver: remove IPN protocol server
Unused in this repo as of the earlier #6450 (300aba61a6)
and unused in the Windows GUI as of tailscale/corp#8065.

With this ipn.BackendServer is no longer used and could also be
removed from this repo. The macOS and iOS clients still temporarily
depend on it, but I can move it to that repo instead while and let its
migration proceed on its own schedule while we clean this repo up.

Updates #6417
Updates tailscale/corp#8051

Change-Id: Ie13f82af3eb9f96b3a21c56cdda51be31ddebdcf
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
2 years ago
Brad Fitzpatrick 0a842f353c ipn/ipnserver: move more connection acceptance logic to LocalBackend
Follow-up to #6467 and #6506.

LocalBackend knows the server-mode state, so move more auth checking
there, removing some bookkeeping from ipnserver.Server.

Updates #6417
Updates tailscale/corp#8051

Change-Id: Ic5d14a077bf0dccc92a3621bd2646bab2cc5b837
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
2 years ago
Brad Fitzpatrick 5ea7c7d603 ipn/{ipnlocal,ipnserver}: add some comments
Change-Id: Ieb5917edaf572342b755caa458693512c7aece81
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
2 years ago
Maisem Ali a3cd171773 ipn/ipnserver: remove Server.serverModeUser
We can just rely on LocalBackend.CurrentUser

Signed-off-by: Maisem Ali <maisem@tailscale.com>
2 years ago
Brad Fitzpatrick 250edeb3da ipn/ipnserver: only permit the pre-HTTP LocalAPI protocol on Windows
Updates #6417

Change-Id: I1c9dbee3f72969f703b3ff2dbbaa145a17db868b
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
2 years ago
Brad Fitzpatrick 7bff7345cc ipn/ipnauth: start splitting ipnserver into new ipnauth package
We're trying to gut 90% of the ipnserver package. A lot will get
deleted, some will move to LocalBackend, and a lot is being moved into
this new ipn/ipnauth package which will be leaf-y and testable.

This is a baby step towards moving some stuff to ipnauth.

Update #6417
Updates tailscale/corp#8051

Change-Id: I28bc2126764f46597d92a2d72565009dc6927ee0
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
2 years ago
Tom DNetto 2a991a3541 ipn/{localapi,ipnserver}: set a CSP for ServeHTMLStatus, refactor host check
Signed-off-by: Tom DNetto <tom@tailscale.com>
2 years ago
Brad Fitzpatrick f18dde6ad1 ipn/ipnserver: validate Host header on debug ServeHTMLStatus status
Updates tailscale/corp#7948

Change-Id: I3a8c64f353af1eeae620812b2700ce4af4fbbc88
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
2 years ago
Maisem Ali 235309adc4 all: store NL keys per profile
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>
2 years ago
Maisem Ali 4d330bac14 ipn/ipnlocal: add support for multiple user profiles
Signed-off-by: Maisem Ali <maisem@tailscale.com>
2 years ago
Brad Fitzpatrick da8def8e13 all: remove old +build tags
The //go:build syntax was introduced in Go 1.17:

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

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

Done with:

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

Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
2 years ago
Josh Soref d4811f11a0 all: fix spelling mistakes
Signed-off-by: Josh Soref <2119212+jsoref@users.noreply.github.com>
2 years ago
Tom DNetto e9b98dd2e1 control/controlclient,ipn/ipnlocal: wire tka enable/disable
Signed-off-by: Tom DNetto <tom@tailscale.com>
2 years ago
Brad Fitzpatrick 65c24b6334 envknob: generalize Windows tailscaled-env.txt support
ipnserver previously had support for a Windows-only environment
variable mechanism that further only worked when Windows was running
as a service, not from a console.

But we want it to work from tailscaed too, and we want it to work on
macOS and Synology. So move it to envknob, now that envknob can change
values at runtime post-init.

A future change will wire this up for more platforms, and do something
more for CLI flags like --port, which the bug was originally about.

Updates #5114

Change-Id: I9fd69a9a91bb0f308fc264d4a6c33e0cbe352d71
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
2 years ago
Eng Zer Jun f0347e841f refactor: move from io/ioutil to io and os packages
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>
2 years ago
Andrew Dunham b8596f2a2f
net/dnsfallback: cache most recent DERP map on disk (#5545)
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>
2 years ago
Tom DNetto 79905a1162 tka: make storage a parameter rather than an Authority struct member
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>
2 years ago
Tom DNetto 4001d0bf25 assorted: plumb tka initialization & network-lock key into tailscaled
- 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>
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 d17849461c ipn/{ipnserver,ipnlocal}: support incoming Taildrop on QNAP
Signed-off-by: Denton Gentry <dgentry@tailscale.com>
2 years ago
Brad Fitzpatrick a9b4bf1535 ipn/ipnserver, cmd/tailscaled: fix peerapi on Windows
We weren't wiring up netstack.Impl to the LocalBackend in some cases
on Windows. This fixes Windows 7 when run as a service.

Updates #4750 (fixes after pull in to corp repo)

Change-Id: I9ce51b797710f2bedfa90545776b7628c7528e99
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
3 years ago
Brad Fitzpatrick cc91a05686 ipn/ipnserver: fix build on js/wasm
Broken by 3dedcd1640 but we don't have CI coverage yet.

Updates #3157

Change-Id: Ie8e95ebd36264887fdeed16fc9f25a857d48124b
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
3 years ago