Commit Graph

1776 Commits (d05e6dc09e7a36e2b6082ce259e33eb3eecd0c0c)

Author SHA1 Message Date
Brad Fitzpatrick d05e6dc09e util/syspolicy/policyclient: add policyclient.Client interface, start plumbing
This is step 2 of ~4, breaking up #14720 into reviewable chunks, with
the aim to make syspolicy be a build-time configurable feature.

Step 1 was #16984.

In this second step, the util/syspolicy/policyclient package is added
with the policyclient.Client interface.  This is the interface that's
always present (regardless of build tags), and is what code around the
tree uses to ask syspolicy/MDM questions.

There are two implementations of policyclient.Client for now:

1) NoPolicyClient, which only returns default values.
2) the unexported, temporary 'globalSyspolicy', which is implemented
   in terms of the global functions we wish to later eliminate.

This then starts to plumb around the policyclient.Client to most callers.

Future changes will plumb it more. When the last of the global func
callers are gone, then we can unexport the global functions and make a
proper policyclient.Client type and constructor in the syspolicy
package, removing the globalSyspolicy impl out of tsd.

The final change will sprinkle build tags in a few more places and
lock it in with dependency tests to make sure the dependencies don't
later creep back in.

Updates #16998
Updates #12614

Change-Id: Ib2c93d15c15c1f2b981464099177cd492d50391c
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
3 months ago
Brad Fitzpatrick cc532efc20 util/syspolicy/*: move syspolicy keys to new const leaf "pkey" package
This is step 1 of ~3, breaking up #14720 into reviewable chunks, with
the aim to make syspolicy be a build-time configurable feature.

In this first (very noisy) step, all the syspolicy string key
constants move to a new constant-only (code-free) package. This will
make future steps more reviewable, without this movement noise.

There are no code or behavior changes here.

The future steps of this series can be seen in #14720: removing global
funcs from syspolicy resolution and using an interface that's plumbed
around instead. Then adding build tags.

Updates #12614

Change-Id: If73bf2c28b9c9b1a408fe868b0b6a25b03eeabd1
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
3 months ago
Maisem Ali 882b05fff9 cmd/viewer: add field comments to generated view methods
Extract field comments from AST and include them in generated view
methods. Comments are preserved from the original struct fields to
provide documentation for the view accessors.

Fixes #16958

Signed-off-by: Maisem Ali <3953239+maisem@users.noreply.github.com>
3 months ago
Jonathan Nobels 80f5a00e76
ipn/local: add the suggested exit node to the ipn bus (#16748)
fixes tailscale/corp#26369

The suggested exit node is currently only calculated during a localAPI request.
For older UIs, this wasn't a bad choice - we could just fetch it on-demand when a menu
presented itself.  For newer incarnations however, this is an always-visible field
that needs to react to changes in the suggested exit node's value.

This change recalculates the suggested exit node ID on netmap updates and
broadcasts it on the IPN bus.   The localAPI version of this remains intact for the
time being.

Signed-off-by: Jonathan Nobels <jonathan@tailscale.com>
3 months ago
M. J. Fromberger b411ffb52f ipn/ipnlocal: remove UnlockEarly from doSetHostinfoFilterServices
Pull the lock-bearing code into a closure, and use a clone rather than a
shallow copy of the hostinfo record.

Updates #11649

Change-Id: I4f1d42c42ce45e493b204baae0d50b1cbf82b102
Signed-off-by: M. J. Fromberger <fromberger@tailscale.com>
3 months ago
M. J. Fromberger 9002e5fd6b ipn/ipnlocal: remove an unnecessary unlock shortcut
The early unlock on this branch was required because the "send" method goes on
to acquire the mutex itself. Rather than release the lock just to acquire it
again, call the underlying locked helper directly.

Updates #11649

Change-Id: I50d81864a00150fc41460b7486a9c65655f282f5
Signed-off-by: M. J. Fromberger <fromberger@tailscale.com>
3 months ago
M. J. Fromberger 2fb9472990 ipn/ipnlocal: remove unnecessary usees of lockAndGetUnlock
In places where we are locking the LocakBackend and immediately deferring an
unlock, and where there is no shortcut path in the control flow below the
deferral, we do not need the unlockOnce helper. Replace all these with use of
the lock directly.

Updates #11649

Change-Id: I3e6a7110dfc9ec6c1d38d2585c5367a0d4e76514
Signed-off-by: M. J. Fromberger <fromberger@tailscale.com>
3 months ago
M. J. Fromberger 16bd60f9ca
ipn,tsnet: update AdvertiseTags documentation (#16931)
Instead of referring to groups, which is a term of art for a different entity,
update the doc comments to more accurately describe what tags are in reference
to the policy document.

Updates #cleanup

Change-Id: Iefff6f84981985f834bae7c6a6c34044f53f2ea2
Signed-off-by: M. J. Fromberger <fromberger@tailscale.com>
3 months ago
M. J. Fromberger 6c8fef961e
ipn/ipnlocal: replace the LockedOnEntry pattern with conventional lock/unlock discipline (#16925)
There are several methods within the LocalBackend that used an unusual and
error-prone lock discipline whereby they require the caller to hold the backend
mutex on entry, but release it on the way out.

In #11650 we added some support code to make this pattern more visible.
Now it is time to eliminate the pattern (at least within this package).
This is intended to produce no semantic changes, though I am relying on
integration tests and careful inspection to achieve that.

To the extent possible I preserved the existing control flow. In a few places,
however, I replaced this with an unlock/lock closure. This means we will
sometimes reacquire a lock only to release it again one frame up the stack, but
these operations are not performance sensitive and the legibility gain seems
worthwhile.

We can probably also pull some of these out into separate methods, but I did
not do that here so as to avoid other variable scope changes that might be hard
to see. I would like to do some more cleanup separately.

As a follow-up, we could also remove the unlockOnce helper, but I did not do
that here either.

Updates #11649

Change-Id: I4c92d4536eca629cfcd6187528381c33f4d64e20
Signed-off-by: M. J. Fromberger <fromberger@tailscale.com>
3 months ago
Jordan Whited 86a5292c03
ipn/localapi: make tailscale debug derp STUNOnly-aware (#16927)
Fixes #16926

Signed-off-by: Jordan Whited <jordan@tailscale.com>
3 months ago
M. J. Fromberger 55698c8511
ipn/localapi: plumb an event bus through the localapi.Handler (#16892)
Some of the operations of the local API need an event bus to correctly
instantiate other components (notably including the portmapper).

This commit adds that, and as the parameter list is starting to get a bit long
and hard to read, I took the opportunity to move the arguments to a config
type. Only a few call sites needed to be updated and this API is not intended
for general use, so I did not bother to stage the change.

Updates #15160
Updates #16842

Change-Id: I7b057d71161bd859f5acb96e2f878a34c85be0ef
Signed-off-by: M. J. Fromberger <fromberger@tailscale.com>
4 months ago
Michael Ben-Ami 3f1851a6d9 types/dnstype, ipn/ipnlocal: allow other DNS resolvers with exit nodes
dnstype.Resolver adds a boolean UseWithExitNode that controls
whether the resolver should be used in tailscale exit node contexts
(not wireguard exit nodes). If UseWithExitNode resolvers are found,
they are installed as the global resolvers. If no UseWithExitNode resolvers
are found, the exit node resolver continues to be installed as the global
resolver. Split DNS Routes referencing UseWithExitNode resolvers are also
installed.

Updates #8237

Fixes tailscale/corp#30906
Fixes tailscale/corp#30907

Signed-off-by: Michael Ben-Ami <mzb@tailscale.com>
4 months ago
Joe Tsai fbb91758ac
cmd/viewer, types/views: implement support for json/v2 (#16852)
This adds support for having every viewer type implement
jsonv2.MarshalerTo and jsonv2.UnmarshalerFrom.

This provides a significant boost in performance
as the json package no longer needs to validate
the entirety of the JSON value outputted by MarshalJSON,
nor does it need to identify the boundaries of a JSON value
in order to call UnmarshalJSON.

For deeply nested and recursive MarshalJSON or UnmarshalJSON calls,
this can improve runtime from O(N²) to O(N).

This still references "github.com/go-json-experiment/json"
instead of the experimental "encoding/json/v2" package
now available in Go 1.25 under goexperiment.jsonv2
so that code still builds without the experiment tag.
Of note, the "github.com/go-json-experiment/json" package
aliases the standard library under the right build conditions.

Updates tailscale/corp#791

Signed-off-by: Joe Tsai <joetsai@digital-static.net>
4 months ago
jishudashu 23a0398136
ipn/ipnlocal, net/dns: use slices.Equal to simplify code (#16641)
Signed-off-by: jishudashu <979260390@qq.com>
4 months ago
Tom Proctor eed3e5dc61
ipn/store/kubestore,kube: fix cert error in admin UI (#16717)
Also adds a test to kube/kubeclient to defend against the error type
returned by the client changing in future.

Fixes tailscale/corp#30855

Change-Id: Id11d4295003e66ad5c29a687f1239333c21226a4

Signed-off-by: Tom Proctor <tomhjp@users.noreply.github.com>
4 months ago
M. J. Fromberger b34cdc9710
ipn,net,tsnet,wgengine: make an eventbus mandatory where it is used (#16594)
In the components where an event bus is already plumbed through, remove the
exceptions that allow it to be omitted, and update all the tests that relied on
those workarounds execute properly.

This change applies only to the places where we're already using the bus; it
does not enforce the existence of a bus in other components (yet),

Updates #15160

Change-Id: Iebb92243caba82b5eb420c49fc3e089a77454f65
Signed-off-by: M. J. Fromberger <fromberger@tailscale.com>
4 months ago
James Sanderson 5731869565 health: add an ETag to UnhealthyState for change detection
Updates tailscale/corp#30596

Signed-off-by: James Sanderson <jsanderson@tailscale.com>
4 months ago
Simon Law 1ef8fbf470
ipn/ipnlocal: send Hostinfo after resolveExitNode for "auto:any" (#16632)
In #16625, I introduced a mechanism for sending the selected exit node
to Control via tailcfg.Hostinfo.ExitNodeID as part of the MapRequest.
@nickkhyl pointed out that LocalBackend.doSetHostinfoFilterServices
needs to be triggered in order to actually send this update. This
patch adds that command. It also prevents the client from sending
"auto:any" in that field, because that’s not a real exit node ID.

This patch also fills in some missing checks in TestConfigureExitNode.

Updates tailscale/corp#30536

Signed-off-by: Simon Law <sfllaw@tailscale.com>
4 months ago
Simon Law 729d6532ff
tailcfg: add Hostinfo.ExitNodeID to report the selected exit node (#16625)
When a client selects a particular exit node, Control may use that as
a signal for deciding other routes.

This patch causes the client to report whenever the current exit node
changes, through tailcfg.Hostinfo.ExitNodeID. It relies on a properly
set ipn.Prefs.ExitNodeID, which should already be resolved by
`tailscale set`.

Updates tailscale/corp#30536

Signed-off-by: Simon Law <sfllaw@tailscale.com>
4 months ago
KevinLiang10 19faaff95c
cmd/tailscale/cli: revert key for web config for services to FQDN (#16627)
This commit reverts the key of Web field in ipn.ServiceConfig to use FQDN instead of service
name for the host part of HostPort. This change is because k8s operator already build base on
the assumption of the part being FQDN. We don't want to break the code with dependency.

Fixes tailscale/corp#30695

Signed-off-by: KevinLiang10 <37811973+KevinLiang10@users.noreply.github.com>
4 months ago
Jordan Whited 1677fb1905
wgengine/magicsock,all: allocate peer relay over disco instead of PeerAPI (#16603)
Updates tailscale/corp#30583
Updates tailscale/corp#30534
Updates tailscale/corp#30557

Signed-off-by: Dylan Bargatze <dylan@tailscale.com>
Signed-off-by: Jordan Whited <jordan@tailscale.com>
Co-authored-by: Dylan Bargatze <dylan@tailscale.com>
5 months ago
Tom Proctor f421907c38
all-kube: create Tailscale Service for HA kube-apiserver ProxyGroup (#16572)
Adds a new reconciler for ProxyGroups of type kube-apiserver that will
provision a Tailscale Service for each replica to advertise. Adds two
new condition types to the ProxyGroup, TailscaleServiceValid and
TailscaleServiceConfigured, to post updates on the state of that
reconciler in a way that's consistent with the service-pg reconciler.
The created Tailscale Service name is configurable via a new ProxyGroup
field spec.kubeAPISserver.ServiceName, which expects a string of the
form "svc:<dns-label>".

Lots of supporting changes were needed to implement this in a way that's
consistent with other operator workflows, including:

* Pulled containerboot's ensureServicesUnadvertised and certManager into
  kube/ libraries to be shared with k8s-proxy. Use those in k8s-proxy to
  aid Service cert sharing between replicas and graceful Service shutdown.
* For certManager, add an initial wait to the cert loop to wait until
  the domain appears in the devices's netmap to avoid a guaranteed error
  on the first issue attempt when it's quick to start.
* Made several methods in ingress-for-pg.go and svc-for-pg.go into
  functions to share with the new reconciler
* Added a Resource struct to the owner refs stored in Tailscale Service
  annotations to be able to distinguish between Ingress- and ProxyGroup-
  based Services that need cleaning up in the Tailscale API.
* Added a ListVIPServices method to the internal tailscale client to aid
  cleaning up orphaned Services
* Support for reading config from a kube Secret, and partial support for
  config reloading, to prevent us having to force Pod restarts when
  config changes.
* Fixed up the zap logger so it's possible to set debug log level.

Updates #13358

Change-Id: Ia9607441157dd91fb9b6ecbc318eecbef446e116
Signed-off-by: Tom Proctor <tomhjp@users.noreply.github.com>
5 months ago
Claus Lensbøl d334d9ba07
client/local,cmd/tailscale/cli,ipn/localapi: expose eventbus graph (#16597)
Make it possible to dump the eventbus graph as JSON or DOT to both debug
and document what is communicated via the bus.

Updates #15160

Signed-off-by: Claus Lensbøl <claus@tailscale.com>
5 months ago
KevinLiang10 e7238efafa
cmd/tailscale/cli: Add service flag to serve command (#16191)
* cmd/tailscale/cli: Add service flag to serve command

This commit adds the service flag to serve command which allows serving a service and add the service
to the advertisedServices field in prefs (What advertise command does that will be removed later).

When adding proxies, TCP proxies and WEB proxies work the same way as normal serve, just under a
different DNSname. There is a services specific L3 serving mode called Tun, can be set via --tun flag.
Serving a service is always in --bg mode. If --bg is explicitly set t o false, an error message will
be sent out. The restriction on proxy target being localhost or 127.0.0.1 also applies to services.

When removing proxies, TCP proxies can be removed with type and port flag and off argument. Web proxies
can be removed with type, port, setPath flag and off argument. To align with normal serve, when setPath
is not set, all handler under the hostport will be removed. When flags are not set but off argument was
passed by user, it will be a noop. Removing all config for a service will be available later with a new
subcommand clear.

Updates tailscale/corp#22954

Signed-off-by: KevinLiang10 <37811973+KevinLiang10@users.noreply.github.com>

* cmd/tailscale/cli: fix ai comments and fix a test

Signed-off-by: KevinLiang10 <37811973+KevinLiang10@users.noreply.github.com>

* cmd/tailscale/cli: Add a test for addServiceToPrefs

Signed-off-by: KevinLiang10 <37811973+KevinLiang10@users.noreply.github.com>

* cmd/tailscale/cli: fix comment

Signed-off-by: KevinLiang10 <37811973+KevinLiang10@users.noreply.github.com>

* add dnsName in error message

Signed-off-by: KevinLiang10 <37811973+KevinLiang10@users.noreply.github.com>

* change the cli input flag variable type

Signed-off-by: KevinLiang10 <37811973+KevinLiang10@users.noreply.github.com>

* replace FindServiceConfig with map lookup

Signed-off-by: KevinLiang10 <37811973+KevinLiang10@users.noreply.github.com>

* some code simplification and add asServiceName

This commit cotains code simplification for IsServingHTTPS, SetWebHandler, SetTCPForwarding

Signed-off-by: KevinLiang10 <37811973+KevinLiang10@users.noreply.github.com>

* replace IsServiceName with tailcfg.AsServiceName

Signed-off-by: KevinLiang10 <37811973+KevinLiang10@users.noreply.github.com>

* replace all assemble of host name for service with strings.Join

Signed-off-by: KevinLiang10 <37811973+KevinLiang10@users.noreply.github.com>

* cmd/tailscale/cli: adjust parameter order and update output message

This commit updates the parameter order for IsTCPForwardingOnPort and SetWebHandler.
Also updated the message msgServiceIPNotAssigned to msgServiceWaitingApproval to adapt to
latest terminologies around services.

Signed-off-by: KevinLiang10 <37811973+KevinLiang10@users.noreply.github.com>

* cmd/tailscale/cli: flip bool condition

This commit fixes a previous bug added that throws error when serve funnel without service.
It should've been the opposite, which throws error when serve funnel with service.

Signed-off-by: KevinLiang10 <37811973+KevinLiang10@users.noreply.github.com>

* cmd/tailscale/cli: change parameter of IsTCPForwardingOnPort

This commit changes the dnsName string parameter for IsTCPForwardingOnPort to
svcName tailcfg.ServiceName. This change is made to reduce ambiguity when
a single service might have different dnsNames

Signed-off-by: KevinLiang10 <37811973+KevinLiang10@users.noreply.github.com>

* ipn/ipnlocal: replace the key to webHandler for services

This commit changes the way we get the webhandler for vipServices. It used to use the host name
from request to find the webHandler, now everything targeting the vipService IP have the same
set of handlers. This commit also stores service:port instead of FQDN:port as the key in serviceConfig
for Web map.

Signed-off-by: KevinLiang10 <37811973+KevinLiang10@users.noreply.github.com>

* cmd/tailscale/cli: Updated use of service name.

This commit removes serviceName.IsEmpty and use direct comparison to  instead. In legacy code, when an empty service
name needs to be passed, a new constant noService is passed. Removed redundant code for checking service name validity
and string method for serviceNameFlag.

Signed-off-by: KevinLiang10 <37811973+KevinLiang10@users.noreply.github.com>

* cmd/tailscale/cli: Update bgBoolFlag

This commit update field name, set and string method of bgBoolFlag to make code cleaner.

Signed-off-by: KevinLiang10 <37811973+KevinLiang10@users.noreply.github.com>

* cmd/tailscale/cli: remove isDefaultService output from srvTypeAndPortFromFlags

This commit removes the isDefaultService out put as it's no longer needed. Also deleted redundant code.

Signed-off-by: KevinLiang10 <37811973+KevinLiang10@users.noreply.github.com>

* cmd/tailscale/cli: remove unnessesary variable declare in messageForPort

Signed-off-by: KevinLiang10 <37811973+KevinLiang10@users.noreply.github.com>

* replace bool output for AsServiceName with err

Signed-off-by: KevinLiang10 <37811973+KevinLiang10@users.noreply.github.com>

* cmd/tailscale/cli: Replace DNSName with NoService if DNSname only used to identify service

This commit moves noService constant to tailcfg, updates AsServiceName to return tailcfg.NoService if the input
is not a valid service name. This commit also removes using the local DNSName as scvName parameter. When a function
is only using DNSName to identify if it's working with a service, the input in replaced with svcName and expect
caller to pass tailcfg.NoService if it's a local serve. This commit also replaces some use of Sprintf with
net.JoinHostPort for ipn.HostPort creation.

Signed-off-by: KevinLiang10 <37811973+KevinLiang10@users.noreply.github.com>

* cmd/tailscale/cli: Remove the returned error for AsServiceName

Signed-off-by: KevinLiang10 <37811973+KevinLiang10@users.noreply.github.com>

* apply suggested code and comment

Signed-off-by: KevinLiang10 <37811973+KevinLiang10@users.noreply.github.com>

* replace local dnsName in test with tailcfg.NoService

Signed-off-by: KevinLiang10 <37811973+KevinLiang10@users.noreply.github.com>

* cmd/tailscale/cli: move noService back and use  else where

The constant serves the purpose of provide readability for passing as a function parameter. It's
more meaningful comparing to a . It can just be an empty string in other places.

Signed-off-by: KevinLiang10 <37811973+KevinLiang10@users.noreply.github.com>

* ipn: Make WebHandlerExists and RemoveTCPForwarding accept svcName

This commit replaces two functions' string input with svcName input since they only use the dnsName to
identify service. Also did some minor cleanups

Signed-off-by: KevinLiang10 <37811973+KevinLiang10@users.noreply.github.com>

---------

Signed-off-by: KevinLiang10 <37811973+KevinLiang10@users.noreply.github.com>
5 months ago
Simon Law e84e58c567 ipn/ipnlocal: use rendezvous hashing to traffic-steer exit nodes
With auto exit nodes enabled, the client picks exit nodes from the
ones advertised in the network map. Usually, it picks the one with the
highest priority score, but when the top spot is tied, it used to pick
randomly. Then, once it made a selection, it would strongly prefer to
stick with that exit node. It wouldn’t even consider another exit node
unless the client was shutdown or the exit node went offline. This is
to prevent flapping, where a client constantly chooses a different
random exit node.

The major problem with this algorithm is that new exit nodes don’t get
selected as often as they should. In fact, they wouldn’t even move
over if a higher scoring exit node appeared.

Let’s say that you have an exit node and it’s overloaded. So you spin
up a new exit node, right beside your existing one, in the hopes that
the traffic will be split across them. But since the client had this
strong affinity, they stick with the exit node they know and love.

Using rendezvous hashing, we can have different clients spread
their selections equally across their top scoring exit nodes. When an
exit node shuts down, its clients will spread themselves evenly to
their other equal options. When an exit node starts, a proportional
number of clients will migrate to their new best option.

Read more: https://en.wikipedia.org/wiki/Rendezvous_hashing

The trade-off is that starting up a new exit node may cause some
clients to move over, interrupting their existing network connections.
So this change is only enabled for tailnets with `traffic-steering`
enabled.

Updates tailscale/corp#29966
Fixes #16551

Signed-off-by: Simon Law <sfllaw@tailscale.com>
5 months ago
Jordan Whited 17c5116d46
ipn/ipnlocal: sort tailscale debug peer-relay-servers slice (#16579)
Updates tailscale/corp#30036

Signed-off-by: Jordan Whited <jordan@tailscale.com>
5 months ago
Jordan Whited 3c6d17e6f1
cmd/tailscale/cli,ipn/ipnlocal,wgengine/magicsock: implement tailscale debug peer-relay-servers (#16577)
Updates tailscale/corp#30036

Signed-off-by: Jordan Whited <jordan@tailscale.com>
5 months ago
Simon Law 205f822372 ipn/ipnlocal: check if suggested exit node is online
@nickkyl added an peer.Online check to suggestExitNodeUsingDERP, so it
should also check when running suggestExitNodeUsingTrafficSteering.

Updates tailscale/corp#29966

Signed-off-by: Simon Law <sfllaw@tailscale.com>
5 months ago
Simon Law bfb344905f ipn/ipnlocal: modernize nm.Peers with AppendMatchingPeers
Thanks to @nickkhyl for pointing out that NetMap.Peers doesn’t get
incremental updates since the last full NetMap update. Instead, he
recommends using ipn/ipnlocal.nodeBackend.AppendMatchingPeers.

Updates #cleanup

Signed-off-by: Simon Law <sfllaw@tailscale.com>
5 months ago
Simon Law c18ba4470b
ipn/ipnlocal: add traffic steering support to exit-node suggestions (#16527)
When `tailscale exit-node suggest` contacts the LocalAPI for a
suggested exit node, the client consults its netmap for peers that
contain the `suggest-exit-node` peercap. It currently uses a series of
heuristics to determine the exit node to suggest.

When the `traffic-steering` feature flag is enabled on its tailnet,
the client will defer to Control’s priority scores for a particular
peer. These scores, in `tailcfg.Hostinfo.Location.Priority`, were
historically only used for Mullvad exit nodes, but they have now been
extended to score any peer that could host a redundant resource.

Client capability version 119 is the earliest client that understands
these traffic steering scores. Control tells the client to switch to
rely on these scores by adding `tailcfg.NodeAttrTrafficSteering` to
its `AllCaps`.

Updates tailscale/corp#29966

Signed-off-by: Simon Law <sfllaw@tailscale.com>
5 months ago
Dylan Bargatze fed72e2aa9
cmd/tailscale, ipn/ipnstate, wgengine/magicsock: update ping output for peer relay (#16515)
Updates the output for "tailscale ping" to indicate if a peer relay was traversed, just like the output for DERP or direct connections.

Fixes tailscale/corp#30034

Signed-off-by: Dylan Bargatze <dylan@tailscale.com>
5 months ago
Claus Lensbøl fbc4c34cf7
ipn/localapi: do not break client on event marshalling errors (#16503)
Errors were mashalled without the correct newlines. Also, they could
generally be mashalled with more data, so an intermediate was introduced
to make them slightly nicer to look at.

Updates #15160

Signed-off-by: Claus Lensbøl <claus@tailscale.com>
5 months ago
Jordan Whited ae8641735d
cmd/tailscale/cli,ipn/ipnstate,wgengine/magicsock: label peer-relay (#16510)
Updates tailscale/corp#30033

Signed-off-by: Jordan Whited <jordan@tailscale.com>
5 months ago
Nick Khyl ff1803158a ipn/ipnlocal: change order of exit node refresh and netmap update so that clients receive the new netmap first
If the GUI receives a new exit node ID before the new netmap, it may treat the node as offline or invalid
if the previous netmap didn't include the peer at all, or if the peer was offline or not advertised as an exit node.
This may result in briefly issuing and dismissing a warning, or a similar issue, which isn't ideal.

In this PR, we change the operation order to send the new netmap to clients first before selecting the new exit node
and notifying them of the Exit Node change.

Updates tailscale/corp#30252 (an old issue discovered during testing this)

Signed-off-by: Nick Khyl <nickk@tailscale.com>
5 months ago
Nick Khyl 21a4058ec7 ipn/ipnlocal: add test to verify handling of unknown auto exit node expressions
We already check this for cases where ipn.Prefs.AutoExitNode is configured via syspolicy.
Configuring it directly through EditPrefs should behave the same, so we add a test for that as well.

Additionally, we clarify the implementation and future extensibility in (*LocalBackend).resolveAutoExitNodeLocked,
where the AutoExitNode is actually enforced.

Updates tailscale/corp#29969
Updates #cleanup

Signed-off-by: Nick Khyl <nickk@tailscale.com>
5 months ago
Nick Khyl cc2f4ac921 ipn: move ParseAutoExitNodeID from ipn/ipnlocal to ipn
So it can be used from the CLI without importing ipnlocal.

While there, also remove isAutoExitNodeID, a wrapper around parseAutoExitNodeID
that's no longer used.

Updates tailscale/corp#29969
Updates #cleanup

Signed-off-by: Nick Khyl <nickk@tailscale.com>
5 months ago
Nick Khyl 740b77df59 ipn/ipnlocal,util/syspolicy: add support for ExitNode.AllowOverride policy setting
When the policy setting is enabled, it allows users to override the exit node enforced by the ExitNodeID
or ExitNodeIP policy. It's primarily intended for use when ExitNodeID is set to auto:any, but it can also
be used with specific exit nodes. It does not allow disabling exit node usage entirely.

Once the exit node policy is overridden, it will not be enforced again until the policy changes,
the user connects or disconnects Tailscale, switches profiles, or disables the override.

Updates tailscale/corp#29969

Signed-off-by: Nick Khyl <nickk@tailscale.com>
5 months ago
Nick Khyl 2c630e126b ipn/ipnlocal: make applySysPolicy a method on LocalBackend
Now that applySysPolicy is only called by (*LocalBackend).reconcilePrefsLocked,
we can make it a method to avoid passing state via parameters and to support
future extensibility.

Also factor out exit node-specific logic into applyExitNodeSysPolicyLocked.

Updates tailscale/corp#29969

Signed-off-by: Nick Khyl <nickk@tailscale.com>
5 months ago
Nick Khyl 9bf99741dd ipn/ipnlocal: refactor resolveExitNodeInPrefsLocked, setExitNodeID and resolveExitNodeIP
Now that resolveExitNodeInPrefsLocked is the only caller of setExitNodeID,
and setExitNodeID is the only caller of resolveExitNodeIP, we can restructure
the code with resolveExitNodeInPrefsLocked now calling both
resolveAutoExitNodeLocked and resolveExitNodeIPLocked directly.

This prepares for factoring out resolveAutoExitNodeLocked and related
auto-exit-node logic into an ipnext extension in a future commit.

While there, we also update exit node by IP lookup to use (*nodeBackend).NodeByAddr
and (*nodeBackend).NodeByID instead of iterating over all peers in the most recent netmap.

Updates tailscale/corp#29969

Signed-off-by: Nick Khyl <nickk@tailscale.com>
5 months ago
Nick Khyl 1fe82d6ef5 cmd/tailscale/cli,ipn/ipnlocal: restrict logout when AlwaysOn mode is enabled
In this PR, we start passing a LocalAPI actor to (*LocalBackend).Logout to make it subject
to the same access check as disconnects made via tailscale down or the GUI.

We then update the CLI to allow `tailscale logout` to accept a reason, similar to `tailscale down`.

Updates tailscale/corp#26249

Signed-off-by: Nick Khyl <nickk@tailscale.com>
5 months ago
Nick Khyl ea4018b757 ipn/ipnlocal: fix missing defer in testExtension.Shutdown
Updates #cleanup

Signed-off-by: Nick Khyl <nickk@tailscale.com>
5 months ago
Nick Khyl f1c7b463cd ipn/{ipnauth,ipnlocal,localapi}: make EditPrefs return an error if changing exit node is restricted by policy
We extract checkEditPrefsAccessLocked, adjustEditPrefsLocked, and onEditPrefsLocked from the EditPrefs
execution path, defining when each step is performed and what behavior is allowed at each stage.

Currently, this is primarily used to support Always On mode, to handle the Exit Node enablement toggle,
and to report prefs edit metrics.

We then use it to enforce Exit Node policy settings by preventing users from setting an exit node
and making EditPrefs return an error when an exit node is restricted by policy. This enforcement is also
extended to the Exit Node toggle.

These changes prepare for supporting Exit Node overrides when permitted by policy and preventing logout
while Always On mode is enabled.

In the future, implementation of these methods can be delegated to ipnext extensions via the feature hooks.

Updates tailscale/corp#29969
Updates tailscale/corp#26249

Signed-off-by: Nick Khyl <nickk@tailscale.com>
5 months ago
Nick Khyl cb7b49941e ipn/ipnlocal: add (*LocalBackend).reconcilePrefsLocked
We have several places where we call applySysPolicy, suggestExitNodeLocked, and setExitNodeID.
While there are cases where we want to resolve the exit node specifically, such as when network
conditions change or a new netmap is received, we typically need to perform all three steps.
For example, enforcing policy settings may enable auto exit nodes or set an ExitNodeIP,
which in turn requires picking a suggested exit node or resolving the IP to an ID, respectively.

In this PR, we introduce (*LocalBackend).resolveExitNodeInPrefsLocked and (*LocalBackend).reconcilePrefsLocked,
with the latter calling both applySysPolicy and resolveExitNodeInPrefsLocked.

Consolidating these steps into a single extensibility point would also make it easier to support
future hooks registered by ipnext extensions.

Updates tailscale/corp#29969

Signed-off-by: Nick Khyl <nickk@tailscale.com>
5 months ago
Nick Khyl 381fdcc3f1 ipn/ipnlocal,util/syspolicy/source: retain existing exit node when using auto exit node, if it's allowed by policy
In this PR, we update setExitNodeID to retain the existing exit node if auto exit node is enabled,
the current exit node is allowed by policy, and no suggested exit node is available yet.

Updates tailscale/corp#29969

Signed-off-by: Nick Khyl <nickk@tailscale.com>
5 months ago
Nick Khyl 4c1c0bac8d ipn/ipnlocal: plumb nodeBackend into suggestExitNode to support delta updates, such as online status changes
Now that (*LocalBackend).suggestExitNodeLocked is never called with a non-current netmap
(the netMap parameter is always nil, indicating that the current netmap should be used),
we can remove the unused parameter.

Additionally, instead of suggestExitNodeLocked passing the most recent full netmap to suggestExitNode,
we now pass the current nodeBackend so it can access peers with delta updates applied.

Finally, with that fixed, we no longer need to skip TestUpdateNetmapDeltaAutoExitNode.

Updates tailscale/corp#29969
Fixes #16455

Signed-off-by: Nick Khyl <nickk@tailscale.com>
5 months ago
Nick Khyl 3e01652e4d ipn/ipnlocal: add (*LocalBackend).RefreshExitNode
In this PR, we add (*LocalBackend).RefreshExitNode which determines which exit node
to use based on the current prefs and netmap and switches to it if needed. It supports
both scenarios when an exit node is specified by IP (rather than ID) and needs to be resolved
once the netmap is ready as well as auto exit nodes.

We then use it in (*LocalBackend).SetControlClientStatus when the netmap changes,
and wherever (*LocalBackend).pickNewAutoExitNode was previously used.

Updates tailscale/corp#29969

Signed-off-by: Nick Khyl <nickk@tailscale.com>
5 months ago
Nick Khyl a8055b5f40 cmd/tailscale/cli,ipn,ipn/ipnlocal: add AutoExitNode preference for automatic exit node selection
With this change, policy enforcement and exit node resolution can happen in separate steps,
since enforcement no longer depends on resolving the suggested exit node. This keeps policy
enforcement synchronous (e.g., when switching profiles), while allowing exit node resolution
to be asynchronous on netmap updates, link changes, etc.

Additionally, the new preference will be used to let GUIs and CLIs switch back to "auto" mode
after a manual exit node override, which is necessary for tailscale/corp#29969.

Updates tailscale/corp#29969
Updates #16459

Signed-off-by: Nick Khyl <nickk@tailscale.com>
5 months ago
Nick Khyl 0098822981 ipn/ipnlocal: update suggestExitNode to skip offline candidates and fix TestSetControlClientStatusAutoExitNode
TestSetControlClientStatusAutoExitNode is broken similarly to TestUpdateNetmapDeltaAutoExitNode
as suggestExitNode didn't previously check the online status of exit nodes, and similarly to the other test
it succeeded because the test itself is also broken.

However, it is easier to fix as it sends out a full netmap update rather than a delta peer update,
so it doesn't depend on the same refactoring as TestSetControlClientStatusAutoExitNode.

Updates #16455
Updates tailscale/corp#29969

Signed-off-by: Nick Khyl <nickk@tailscale.com>
5 months ago
Nick Khyl 6ecc25b26a ipn/ipnlocal: skip TestUpdateNetmapDeltaAutoExitNode
suggestExitNode never checks whether an exit node candidate is online.
It also accepts a full netmap, which doesn't include changes from delta updates.
The test can't work correctly until both issues are fixed.

Previously, it passed only because the test itself is flawed.
It doesn't succeed because the currently selected node goes offline and a new one is chosen.
Instead, it succeeds because lastSuggestedExitNode is incorrect, and suggestExitNode picks
the correct node the first time it runs, based on the DERP map and the netcheck report.
The node in exitNodeIDWant just happens to be the optimal choice.

Fixing SuggestExitNode requires refactoring its callers first, which in turn reveals the flawed test,
as suggestExitNode ends up being called slightly earlier.

In this PR, we update the test to correctly fail due to existing bugs in SuggestExitNode,
and temporarily skip it until those issues are addressed in a future commit.

Updates #16455
Updates tailscale/corp#29969

Signed-off-by: Nick Khyl <nickk@tailscale.com>
5 months ago
Nick Khyl 56d772bd63 ipn/ipnlocal: simplify pickNewAutoExitNode
(*profileManager).CurrentPrefs() is always valid. Additionally, there's no value in cloning
and passing the full ipn.Prefs when editing preferences. Instead, ipn.MaskedPrefs should
only have ExitNodeID set.

Updates tailscale/corp#29969

Signed-off-by: Nick Khyl <nickk@tailscale.com>
5 months ago