1610 Commits (1d035db4df78bcca717417ec45cecfaf4d12ee1d)

Author	SHA1	Message	Date
Nick Khyl	7aef4fd44d	ipn/ipn{local,server}: extract logic that determines the "best" Tailscale profile to use ... In this PR, we further refactor LocalBackend and Unattended Mode to extract the logic that determines which profile should be used at the time of the check, such as when a LocalAPI client connects or disconnects. We then update (*LocalBackend).switchProfileLockedOnEntry to to switch to the profile returned by (*LocalBackend).resolveBestProfileLocked() rather than to the caller-specified specified profile, and rename it to switchToBestProfileLockedOnEntry. This is done in preparation for updating (*LocalBackend).getBackgroundProfileIDLocked to support Always-On mode by determining which profile to use based on which users, if any, are currently logged in and have an active foreground desktop session. Updates #14823 Updates tailscale/corp#26247 Signed-off-by: Nick Khyl <nickk@tailscale.com>	12 months ago
Nick Khyl	9b32ba7f54	ipn/ipn{local,server}: move "staying alive in server mode" from ipnserver to LocalBackend ... Currently, we disconnect Tailscale and reset LocalBackend on Windows when the last LocalAPI client disconnects, unless Unattended Mode is enabled for the current profile. And the implementation is somewhat racy since the current profile could theoretically change after (*ipnserver.Server).addActiveHTTPRequest checks (*LocalBackend).InServerMode() and before it calls (*LocalBackend).SetCurrentUser(nil) (or, previously, (*LocalBackend).ResetForClientDisconnect). Additionally, we might want to keep Tailscale running and connected while a user is logged in rather than tying it to whether a LocalAPI client is connected (i.e., while the GUI is running), even when Unattended Mode is disabled for a profile. This includes scenarios where the new AlwaysOn mode is enabled, as well as when Tailscale is used on headless Windows editions, such as Windows Server Core, where the GUI is not supported. It may also be desirable to switch to the "background" profile when a user logs off from their device or implement other similar features. To facilitate these improvements, we move the logic from ipnserver.Server to ipnlocal.LocalBackend, where it determines whether to keep Tailscale running when the current user disconnects. We also update the logic that determines whether a connection should be allowed to better reflect the fact that, currently, LocalAPI connections are not allowed unless: - the current UID is "", meaning that either we are not on a multi-user system or Tailscale is idle; - the LocalAPI client belongs to the current user (their UIDs are the same); - the LocalAPI client is Local System (special case; Local System is always allowed). Whether Unattended Mode is enabled only affects the error message returned to the Local API client when the connection is denied. Updates #14823 Signed-off-by: Nick Khyl <nickk@tailscale.com>	12 months ago
Nick Khyl	bc0cd512ee	ipn/desktop: add a new package for managing desktop sessions on Windows ... This PR adds a new package, ipn/desktop, which provides a platform-agnostic interface for enumerating desktop sessions and registering session callbacks. Currently, it is implemented only for Windows. Updates #14823 Signed-off-by: Nick Khyl <nickk@tailscale.com>	12 months ago
Nick Khyl	5eacf61844	ipn/ipnauth: implement WindowsActor ... WindowsActor is an ipnauth.Actor implementation that represents a logged-in Windows user by wrapping their Windows user token. Updates #14823 Signed-off-by: Nick Khyl <nickk@tailscale.com>	12 months ago
Nick Khyl	e9e2bc5bd7	ipn/ipn{auth,server}: update ipnauth.Actor to carry a context ... The context carries additional information about the actor, such as the request reason, and is canceled when the actor is done. Additionally, we implement three new ipn.Actor types that wrap other actors to modify their behavior: - WithRequestReason, which adds a request reason to the actor; - WithoutClose, which narrows the actor's interface to prevent it from being closed; - WithPolicyChecks, which adds policy checks to the actor's CheckProfileAccess method. Updates #14823 Signed-off-by: Nick Khyl <nickk@tailscale.com>	12 months ago
Brad Fitzpatrick	9706c9f4ff	types/netmap,*: pass around UserProfiles as views (pointers) instead ... Smaller. Updates tailscale/corp#26058 (@andrew-d noticed during this) Change-Id: Id33cddd171aaf8f042073b6d3c183b0a746e9931 Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>	12 months ago
Nick Khyl	48dd4bbe21	ipn/ipn{local,server}: remove ResetForClientDisconnect in favor of SetCurrentUser(nil) ... There’s (*LocalBackend).ResetForClientDisconnect, and there’s also (*LocalBackend).resetForProfileChangeLockedOnEntry. Both methods essentially did the same thing but in slightly different ways. For example, resetForProfileChangeLockedOnEntry didn’t reset the control client until (*LocalBackend).Start() was called at the very end and didn’t reset the keyExpired flag, while ResetForClientDisconnect didn’t reinitialize TKA. Since SetCurrentUser can be called with a nil argument to reset the currently connected user and internally calls resetForProfileChangeLockedOnEntry, we can remove ResetForClientDisconnect and let SetCurrentUser and resetForProfileChangeLockedOnEntry handle it. Updates #14823 Signed-off-by: Nick Khyl <nickk@tailscale.com>	12 months ago
Nick Khyl	122255765a	ipn/ipnlocal: fix (*profileManager).DefaultUserProfileID for users other than current ... Currently, profileManager filters profiles based on their creator/owner and the "current user"'s UID. This causes DefaultUserProfileID(uid) to work incorrectly when the UID doesn't match the current user. While we plan to remove the concept of the "current user" completely, we're not there yet. In this PR, we fix DefaultUserProfileID by updating profileManager to allow checking profile access for a given UID and modifying helper methods to accept UID as a parameter when returning matching profiles. Updates #14823 Signed-off-by: Nick Khyl <nickk@tailscale.com>	12 months ago
Jonathan Nobels	1bf4c6481a	safesocket: add ability for Darwin clients to set explicit credentials (#14702) ... updates tailscale/corp#25687 The darwin appstore and standalone clients now support XPC and the keychain for passing user credentials securely between the gui process and an NEVPNExtension hosted tailscaled. Clients that can communicate directly with the network extension, via XPC or the keychain, are now expected to call SetCredentials and supply credentials explicitly, fixing issues with the cli breaking if the current user cannot read the contents of /Library/Tailscale due to group membership restrictions. This matches how those clients source and supply credentials to the localAPI http client. Non-platform-specific code that has traditionally been in the client is moved to safesocket. /Libraray/Tailscaled/sameuserproof has its permissions changed to that it's readably only by users in the admin group. This restricts standalone CLI access for and direct use of localAPI to admins. Signed-off-by: Jonathan Nobels <jonathan@tailscale.com>	12 months ago
Brad Fitzpatrick	05ac21ebe4	all: use new LocalAPI client package location ... It was moved in f57fa3cbc3. Updates tailscale/corp#22748 Change-Id: I19f965e6bded1d4c919310aa5b864f2de0cd6220 Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>	12 months ago
Nick Khyl	9726e1f208	ipn/{ipnserver,localapi},tsnet: use ipnauth.Self as the actor in tsnet localapi handlers ... With #14843 merged, (*localapi.Handler).servePrefs() now requires a non-nil actor, and other places may soon require it as well. In this PR, we update localapi.NewHandler with a new required parameter for the actor. We then update tsnet to use ipnauth.Self. We also rearrange the code in (*ipnserver.Server).serveHTTP() to pass the actor via Handler's constructor instead of the field. Updates #14823 Signed-off-by: Nick Khyl <nickk@tailscale.com>	12 months ago
Nick Khyl	00fe8845b1	ipn/{ipnauth,ipnlocal,ipnserver}: move the AlwaysOn policy check from ipnserver to ipnauth ... In this PR, we move the code that checks the AlwaysOn policy from ipnserver.actor to ipnauth. It is intended to be used by ipnauth.Actor implementations, and we temporarily make it exported while these implementations reside in ipnserver and in corp. We'll unexport it later. We also update [ipnauth.Actor.CheckProfileAccess] to accept an auditLogger, which is called to write details about the action to the audit log when required by the policy, and update LocalBackend.EditPrefsAs to use an auditLogger that writes to the regular backend log. Updates tailscale/corp#26146 Signed-off-by: Nick Khyl <nickk@tailscale.com>	12 months ago
Adrian Dewhurst	97c4c0ecf0	ipn/ipnlocal: add VIP service IPs to localnets ... Without adding this, the packet filter rejects traffic to VIP service addresses before checking the filters sent in the netmap. Fixes tailscale/corp#26241 Change-Id: Idd54448048e9b786cf4873fd33b3b21e03d3ad4c Signed-off-by: Adrian Dewhurst <adrian@tailscale.com>	12 months ago
Adrian Dewhurst	600f25dac9	tailcfg: add JSON unmarshal helper for view of node/peer capabilities ... Many places that need to work with node/peer capabilities end up with a something-View and need to either reimplement the helper code or make an expensive copy. We have the machinery to easily handle this now. Updates #cleanup Change-Id: Ic3f55be329f0fc6c178de26b34359d0e8c6ca5fc Signed-off-by: Adrian Dewhurst <adrian@tailscale.com>	12 months ago
James Tucker	10fe10ea10	derp/derphttp,ipn/localapi,net/captivedetection: add cache resistance to captive portal detection ... Observed on some airlines (British Airways, WestJet), Squid is configured to cache and transform these results, which is disruptive. The server and client should both actively request that this is not done by setting Cache-Control headers. Send a timestamp parameter to further work against caches that do not respect the cache-control headers. Updates #14856 Signed-off-by: James Tucker <james@tailscale.com>	12 months ago
Nick Khyl	d832467461	client/tailscale,ipn/ipn{local,server},util/syspolicy: implement the AlwaysOn.OverrideWithReason policy setting ... In this PR, we update client/tailscale.LocalClient to allow sending requests with an optional X-Tailscale-Reason header. We then update ipn/ipnserver.{actor,Server} to retrieve this reason, if specified, and use it to determine whether ipnauth.Disconnect is allowed when the AlwaysOn.OverrideWithReason policy setting is enabled. For now, we log the reason, along with the profile and OS username, to the backend log. Finally, we update LocalBackend to remember when a disconnect was permitted and do not reconnect automatically unless the policy changes. Updates tailscale/corp#26146 Signed-off-by: Nick Khyl <nickk@tailscale.com>	12 months ago
Nick Khyl	a0537dc027	ipn/ipnlocal: fix a panic in setPrefsLockedOnEntry when cc is nil ... The AlwaysOn policy can be applied by (*LocalBackend).applySysPolicy, flipping WantRunning from false to true before (*LocalBackend).Start() has been called for the first time and set a control client in b.cc. This results in a nil pointer dereference and a panic when setPrefsLockedOnEntry applies the change and calls controlclient.Client.Login(). In this PR, we fix it by only doing a login if b.cc has been set. Updates #14823 Signed-off-by: Nick Khyl <nickk@tailscale.com>	1 year ago
Percy Wegmann	2e95313b8b	ssh,tempfork/gliderlabs/ssh: replace github.com/tailscale/golang-x-crypto/ssh with golang.org/x/crypto/ssh ... The upstream crypto package now supports sending banners at any time during authentication, so the Tailscale fork of crypto/ssh is no longer necessary. github.com/tailscale/golang-x-crypto is still needed for some custom ACME autocert functionality. tempfork/gliderlabs is still necessary because of a few other customizations, mostly related to TTY handling. Originally implemented in 46fd4e58a2, which was reverted in b60f6b849a to keep the change out of v1.80. Updates #8593 Signed-off-by: Percy Wegmann <percy@tailscale.com>	1 year ago
Nick Khyl	0a51bbc765	ipn/ipnauth,util/syspolicy: improve comments ... Updates #cleanup Updates #14823 Signed-off-by: Nick Khyl <nickk@tailscale.com>	1 year ago
Nick Khyl	02ad21717f	ipn/ipn{auth,server,local}: initial support for the always-on mode ... In this PR, we update LocalBackend to set WantRunning=true when applying policy settings to the current profile's prefs, if the "always-on" mode is enabled. We also implement a new (*LocalBackend).EditPrefsAs() method, which is like EditPrefs but accepts an actor (e.g., a LocalAPI client's identity) that initiated the change. If WantRunning is being set to false, the new EditPrefsAs method checks whether the actor has ipnauth.Disconnect access to the profile and propagates an error if they do not. Finally, we update (*ipnserver.actor).CheckProfileAccess to allow a disconnect only if the "always-on" mode is not enabled by the AlwaysOn policy setting. This is not a comprehensive solution to the "always-on" mode across platforms, as instead of disconnecting a user could achieve the same effect by creating a new empty profile, initiating a reauth, or by deleting the profile. These are the things we should address in future PRs. Updates #14823 Signed-off-by: Nick Khyl <nickk@tailscale.com>	1 year ago
Nick Khyl	535a3dbebd	ipn/ipnauth: implement an Actor representing tailscaled itself ... Updates #14823 Signed-off-by: Nick Khyl <nickk@tailscale.com>	1 year ago
Nick Khyl	081595de63	ipn/{ipnauth, ipnserver}: extend the ipnauth.Actor interface with a CheckProfileAccess method ... The implementations define it to verify whether the actor has the requested access to a login profile. Updates #14823 Signed-off-by: Nick Khyl <nickk@tailscale.com>	1 year ago
Nick Khyl	4e7f4086b2	ipn: generate LoginProfileView and use it instead of *LoginProfile where appropriate ... Conventionally, we use views (e.g., ipn.PrefsView, tailcfg.NodeView, etc.) when dealing with structs that shouldn't be mutated. However, ipn.LoginProfile has been an exception so far, with a mix of passing and returning LoginProfile by reference (allowing accidental mutations) and by value (which is wasteful, given its current size of 192 bytes). In this PR, we generate an ipn.LoginProfileView and use it instead of passing/returning LoginProfiles by mutable reference or copying them when passing/returning by value. Now, LoginProfiles can only be mutated by (*profileManager).setProfilePrefs. Updates #14823 Signed-off-by: Nick Khyl <nickk@tailscale.com>	1 year ago
Percy Wegmann	b60f6b849a	Revert "ssh,tempfork/gliderlabs/ssh: replace github.com/tailscale/golang-x-crypto/ssh with golang.org/x/crypto/ssh" ... This reverts commit 46fd4e58a2. We don't want to include this in 1.80 yet, but can add it back post 1.80. Updates #8593 Signed-off-by: Percy Wegmann <percy@tailscale.com>	1 year ago
Percy Wegmann	46fd4e58a2	ssh,tempfork/gliderlabs/ssh: replace github.com/tailscale/golang-x-crypto/ssh with golang.org/x/crypto/ssh ... The upstream crypto package now supports sending banners at any time during authentication, so the Tailscale fork of crypto/ssh is no longer necessary. github.com/tailscale/golang-x-crypto is still needed for some custom ACME autocert functionality. tempfork/gliderlabs is still necessary because of a few other customizations, mostly related to TTY handling. Updates #8593 Signed-off-by: Percy Wegmann <percy@tailscale.com>	1 year ago
Brad Fitzpatrick	ba1f9a3918	types/persist: remove Persist.LegacyFrontendPrivateMachineKey ... It was a temporary migration over four years ago. It's no longer relevant. Updates #610 Change-Id: I1f00c9485fab13ede6f77603f7d4235222c2a481 Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>	1 year ago
Brad Fitzpatrick	2691b9f6be	tempfork/acme: add new package for x/crypto package acme fork, move ... We've been maintaining temporary dev forks of golang.org/x/crypto/{acme,ssh} in https://github.com/tailscale/golang-x-crypto instead of using this repo's tempfork directory as we do with other packages. The reason we were doing that was because x/crypto/ssh depended on x/crypto/ssh/internal/poly1305 and I hadn't noticed there are forwarding wrappers already available in x/crypto/poly1305. It also depended internal/bcrypt_pbkdf but we don't use that so it's easy to just delete that calling code in our tempfork/ssh. Now that our SSH changes have been upstreamed, we can soon unfork from SSH. That leaves ACME remaining. This change copies our tailscale/golang-x-crypto/acme code to tempfork/acme but adds a test that our vendored copied still matches our tailscale/golang-x-crypto repo, where we can continue to do development work and rebases with upstream. A comment on the new test describes the expected workflow. While we could continue to just import & use tailscale/golang-x-crypto/acme, it seems a bit nicer to not have that entire-fork-of-x-crypto visible at all in our transitive deps and the questions that invites. Showing just a fork of an ACME client is much less scary. It does add a step to the process of hacking on the ACME client code, but we do that approximately never anyway, and the extra step is very incremental compared to the existing tedious steps. Updates #8593 Updates #10238 Change-Id: I8af4378c04c1f82e63d31bf4d16dba9f510f9199 Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>	1 year ago
Brad Fitzpatrick	68a66ee81b	feature/capture: move packet capture to feature/*, out of iOS + CLI ... We had the debug packet capture code + Lua dissector in the CLI + the iOS app. Now we don't, with tests to lock it in. As a bonus, tailscale.com/net/packet and tailscale.com/net/flowtrack no longer appear in the CLI's binary either. A new build tag ts_omit_capture disables the packet capture code and was added to build_dist.sh's --extra-small mode. Updates #12614 Change-Id: I79b0628c0d59911bd4d510c732284d97b0160f10 Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>	1 year ago
Tom Proctor	2089f4b603	ipn/ipnlocal: add debug envknob for ACME directory URL (#14771) ... Adds an envknob setting for changing the client's ACME directory URL. This allows testing cert issuing against LE's staging environment, as well as enabling local-only test environments, which is useful for avoiding the production rate limits in test and development scenarios. Fixes #14761 Change-Id: I191c840c0ca143a20e4fa54ea3b2f9b7cbfc889f Signed-off-by: Tom Proctor <tomhjp@users.noreply.github.com>	1 year ago
Tom Proctor	69bc164c62	ipn/ipnlocal: include DNS SAN in cert CSR (#14764) ... The CN field is technically deprecated; set the requested name in a DNS SAN extension in addition to maximise compatibility with RFC 8555. Fixes #14762 Change-Id: If5d27f1e7abc519ec86489bf034ac98b2e613043 Signed-off-by: Tom Proctor <tomhjp@users.noreply.github.com>	1 year ago
Andrew Lytvynov	f1710f4a42	appc,ipn/ipnlocal: log DNS parsing errors in app connectors (#14607) ... 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>	1 year ago
Brad Fitzpatrick	1562a6f2f2	feature/*: make Wake-on-LAN conditional, start supporting modular features ... This pulls out the Wake-on-LAN (WoL) code out into its own package (feature/wakeonlan) that registers itself with various new hooks around tailscaled. Then a new build tag (ts_omit_wakeonlan) causes the package to not even be linked in the binary. Ohter new packages include: * feature: to just record which features are loaded. Future: dependencies between features. * feature/condregister: the package with all the build tags that tailscaled, tsnet, and the Tailscale Xcode project extension can empty (underscore) import to load features as a function of the defined build tags. Future commits will move of our "ts_omit_foo" build tags into this style. Updates #12614 Change-Id: I9c5378dafb1113b62b816aabef02714db3fc9c4a Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>	1 year ago
Andrew Lytvynov	3fb8a1f6bf	ipn/ipnlocal: re-advertise appc routes on startup, take 2 (#14740) ... * 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/25965 Fixes #14606 Signed-off-by: Andrew Lytvynov <awly@tailscale.com> --------- Signed-off-by: Andrew Lytvynov <awly@tailscale.com>	1 year ago
Adrian Dewhurst	0fa7b4a236	tailcfg: add ServiceName ... Rather than using a string everywhere and needing to clarify that the string should have the svc: prefix, create a separate type for Service names. Updates tailscale/corp#24607 Change-Id: I720e022f61a7221644bb60955b72cacf42f59960 Signed-off-by: Adrian Dewhurst <adrian@tailscale.com>	1 year ago
KevinLiang10	550923d953	fix handler related and some nit ... Signed-off-by: KevinLiang10 <37811973+KevinLiang10@users.noreply.github.com>	1 year ago
KevinLiang10	8c8750f1b3	ipn/ipnlocal: Support TCP and Web VIP services ... This commit intend to provide support for TCP and Web VIP services and also allow user to use Tun for VIP services if they want to. The commit includes: 1.Setting TCP intercept function for VIP Services. 2.Update netstack to send packet written from WG to netStack handler for VIP service. 3.Return correct TCP hander for VIP services when netstack acceptTCP. This commit also includes unit tests for if the local backend setServeConfig would set correct TCP intercept function and test if a hander gets returned when getting TCPHandlerForDst. The shouldProcessInbound check is not unit tested since the test result just depends on mocked functions. There should be an integration test to cover shouldProcessInbound and if the returned TCP handler actually does what the serveConfig says. Updates tailscale/corp#24604 Signed-off-by: KevinLiang10 <37811973+KevinLiang10@users.noreply.github.com>	1 year ago
Brad Fitzpatrick	150cd30b1d	ipn/ipnlocal: also use LetsEncrypt-baked-in roots for cert validation ... We previously baked in the LetsEncrypt x509 root CA for our tlsdial package. This moves that out into a new "bakedroots" package and is now also shared by ipn/ipnlocal's cert validation code (validCertPEM) that decides whether it's time to fetch a new cert. Otherwise, a machine without LetsEncrypt roots locally in its system roots is unable to use tailscale cert/serve and fetch certs. Fixes #14690 Change-Id: Ic88b3bdaabe25d56b9ff07ada56a27e3f11d7159 Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>	1 year ago
Brad Fitzpatrick	51adaec35a	Revert "ipn/ipnlocal: re-advertise appc routes on startup (#14609)" ... This reverts commit 1b303ee5ba (#14609). It caused a deadlock; see tailscale/corp#25965 Updates tailscale/corp#25965 Updates #13680 Updates #14606	1 year ago
Irbe Krumina	69a985fb1e	ipn/ipnlocal,tailcfg: communicate to control whether funnel is enabled (#14688) ... Adds a new Hostinfo.IngressEnabled bool field that holds whether funnel is currently enabled for the node. Triggers control update when this value changes. Bumps capver so that control can distinguish the new field being false vs non-existant in previous clients. This is part of a fix for an issue where nodes with any AllowFunnel block set in their serve config are being displayed as if actively routing funnel traffic in the admin panel. Updates tailscale/tailscale#11572 Updates tailscale/corp#25931 Signed-off-by: Irbe Krumina <irbe@tailscale.com>	1 year ago
Irbe Krumina	6c30840cac	ipn: [serve] warn that foreground funnel won't work if shields are up (#14685) ... We throw error early with a warning if users attempt to enable background funnel for a node that does not allow incoming connections (shields up), but if it done in foreground mode, we just silently fail (the funnel command succeeds, but the connections are not allowed). This change makes sure that we also error early in foreground mode. Updates tailscale/tailscale#11049 Signed-off-by: Irbe Krumina <irbe@tailscale.com>	1 year ago
Andrea Gottardo	c79b736a85	ipnlocal: allow overriding os.Hostname() via syspolicy (#14676) ... Updates tailscale/corp#25936 This defines a new syspolicy 'Hostname' and allows an IT administrator to override the value we normally read from os.Hostname(). This is particularly useful on Android and iOS devices, where the hostname we get from the OS is really just the device model (a platform restriction to prevent fingerprinting). If we don't implement this, all devices on the customer's side will look like `google-pixel-7a-1`, `google-pixel-7a-2`, `google-pixel-7a-3`, etc. and it is not feasible for the customer to use the API or worse the admin console to manually fix these names. Apply code review comment by @nickkhyl Signed-off-by: Andrea Gottardo <andrea@gottardo.me> Co-authored-by: Nick Khyl <1761190+nickkhyl@users.noreply.github.com>	1 year ago
Nick Khyl	0481042738	ipn/ipnserver: fix a deadlock in (*Server).blockWhileIdentityInUse ... If the server was in use at the time of the initial check, but disconnected and was removed from the activeReqs map by the time we registered a waiter, the ready channel will never be closed, resulting in a deadlock. To avoid this, we check whether the server is still busy after registering the wait. Fixes #14655 Signed-off-by: Nick Khyl <nickk@tailscale.com>	1 year ago
Nick Khyl	62fb857857	ipn/ipnserver: fix TestConcurrentOSUserSwitchingOnWindows ... I made a last-minute change in #14626 to split a single loop that created 1_000 concurrent connections into an inner and outer loop that create 100 concurrent connections 10 times. This introduced a race because the last user's connection may still be active (from the server's perspective) when a new outer iteration begins. Since every new client gets a unique ClientID, but we reuse usernames and UIDs, the server may let a user in (as the UID matches, which is fine), but the test might then fail due to a ClientID mismatch: server_test.go:232: CurrentUser(Initial): got &{S-1-5-21-1-0-0-1001 User-4 <nil> Client-2 false false}; want &{S-1-5-21-1-0-0-1001 User-4 <nil> Client-114 false false} In this PR, we update (*testIPNServer).blockWhileInUse to check whether the server is currently busy and wait until it frees up. We then call blockWhileInUse at the end of each outer iteration so that the server is always in a known idle state at the beginning of the inner loop. We also check that the current user is not set when the server is idle. Updates tailscale/corp#25804 Updates #14655 (found when working on it) Signed-off-by: Nick Khyl <nickk@tailscale.com>	1 year ago
Andrew Lytvynov	1b303ee5ba	ipn/ipnlocal: re-advertise appc routes on startup (#14609) ... 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>	1 year ago
Nick Khyl	6fac2903e1	ipn/ipnserver: fix race condition where LocalBackend is reset after a different user connects ... In this commit, we add a failing test to verify that ipn/ipnserver.Server correctly sets and unsets the current user when two different clients send requests concurrently (A sends request, B sends request, A's request completes, B's request completes). The expectation is that the user who wins the race becomes the current user from the LocalBackend's perspective, remaining in this state until they disconnect, after which a different user should be able to connect and use the LocalBackend. We then fix the second of two bugs in (*Server).addActiveHTTPRequest, where a race condition causes the LocalBackend's state to be reset after a new client connects, instead of after the last active request of the previous client completes and the server becomes idle. Fixes tailscale/corp#25804 Signed-off-by: Nick Khyl <nickk@tailscale.com>	1 year ago
Nick Khyl	f33f5f99c0	ipn/{ipnlocal,ipnserver}: remove redundant (*LocalBackend).ResetForClientDisconnect ... In this commit, we add a failing test to verify that ipn/ipnserver.Server correctly sets and unsets the current user when two different users connect sequentially (A connects, A disconnects, B connects, B disconnects). We then fix the test by updating (*ipn/ipnserver.Server).addActiveHTTPRequest to avoid calling (*LocalBackend).ResetForClientDisconnect again after a new user has connected and been set as the current user with (*LocalBackend).SetCurrentUser(). Since ipn/ipnserver.Server does not allow simultaneous connections from different Windows users and relies on the LocalBackend's current user, and since we already reset the LocalBackend's state by calling ResetForClientDisconnect when the last active request completes (indicating the server is idle and can accept connections from any Windows user), it is unnecessary to track the last connected user on the ipnserver.Server side or call ResetForClientDisconnect again when the user changes. Additionally, the second call to ResetForClientDisconnect occurs after the new user has been set as the current user, resetting the correct state for the new user instead of the old state of the now-disconnected user, causing issues. Updates tailscale/corp#25804 Signed-off-by: Nick Khyl <nickk@tailscale.com>	1 year ago
Nick Khyl	c3c4c96489	ipn/{ipnauth,ipnlocal,ipnserver}, client/tailscale: make ipnserver.Server testable ... We update client/tailscale.LocalClient to allow specifying an optional Transport (http.RoundTripper) for LocalAPI HTTP requests, and implement one that injects an ipnauth.TestActor via request headers. We also add several functions and types to make testing an ipn/ipnserver.Server possible (or at least easier). We then use these updates to write basic tests for ipnserver.Server, ensuring it works on non-Windows platforms and correctly sets and unsets the LocalBackend's current user when a Windows user connects and disconnects. We intentionally omit tests for switching between different OS users and will add them in follow-up commits. Updates tailscale/corp#25804 Signed-off-by: Nick Khyl <nickk@tailscale.com>	1 year ago
Nick Khyl	d0ba91bdb2	ipn/ipnserver: use ipnauth.Actor instead of *ipnserver.actor whenever possible ... In preparation for adding test coverage for ipn/ipnserver.Server, we update it to use ipnauth.Actor instead of its concrete implementation where possible. Updates tailscale/corp#25804 Signed-off-by: Nick Khyl <nickk@tailscale.com>	1 year ago
Brad Fitzpatrick	2fc4455e6d	all: add Node.HomeDERP int, phase out "127.3.3.40:$region" hack [capver 111] ... This deprecates the old "DERP string" packing a DERP region ID into an IP:port of 127.3.3.40:$REGION_ID and just uses an integer, like PeerChange.DERPRegion does. We still support servers sending the old form; they're converted to the new form internally right when they're read off the network. Updates #14636 Change-Id: I9427ec071f02a2c6d75ccb0fcbf0ecff9f19f26f Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>	1 year ago
Nick Khyl	66269dc934	ipn/ipnlocal: allow Peer API access via either V4MasqAddr or V6MasqAddr when both are set ... This doesn't seem to have any immediate impact, but not allowing access via the IPv6 masquerade address when an IPv4 masquerade address is also set seems like a bug. Updates #cleanup Updates #14570 (found when working on it) Signed-off-by: Nick Khyl <nickk@tailscale.com>	1 year ago