In v1.78, we started acquiring the GP lock when reading policy settings. This led to a deadlock during
Tailscale installation via Group Policy Software Installation because the GP engine holds the write lock
for the duration of policy processing, which in turn waits for the installation to complete, which in turn
waits for the service to enter the running state.
In this PR, we prevent the acquisition of GP locks (aka EnterCriticalPolicySection) during service startup
and update the Windows Registry-based util/syspolicy/source.PlatformPolicyStore to handle this failure
gracefully. The GP lock is somewhat optional; it’s safe to read policy settings without it, but acquiring
the lock is recommended when reading multiple values to prevent the Group Policy engine from modifying
settings mid-read and to avoid inconsistent results.
Fixes#14416
Signed-off-by: Nick Khyl <nickk@tailscale.com>
This adds a package with GP-related functions and types to be used in the future PRs.
It also updates nrptRuleDatabase to use the new package instead of its own gpNotificationWatcher implementation.
Updates #12687
Signed-off-by: Nick Khyl <nickk@tailscale.com>
We cannot directly pass a flat domain name into NetUserGetInfo; we must
resolve the address of a domain controller first.
This PR implements the appropriate resolution mechanisms to do that, and
also exposes a couple of new utility APIs for future needs.
Fixes#12627
Signed-off-by: Aaron Klotz <aaron@tailscale.com>
This PR ties together pseudoconsoles, user profiles, s4u logons, and
process creation into what is (hopefully) a simple API for various
Tailscale services to obtain Windows access tokens without requiring
knowledge of any Windows passwords. It works both for domain-joined
machines (Kerberos) and non-domain-joined machines. The former case
is fairly straightforward as it is fully documented. OTOH, the latter
case is not documented, though it is fully defined in the C headers in
the Windows SDK. The documentation blanks were filled in by reading
the source code of Microsoft's Win32 port of OpenSSH.
We need to do a bit of acrobatics to make conpty work correctly while
creating a child process with an s4u token; see the doc comments above
startProcessInternal for details.
Updates #12383
Signed-off-by: Aaron Klotz <aaron@tailscale.com>
S4U logons do not automatically load the associated user profile. In this
PR we add UserProfile to handle that part. Windows docs indicate that
we should try to resolve a remote profile path when present, so we attempt
to do so when the local computer is joined to a domain.
Updates #12383
Signed-off-by: Aaron Klotz <aaron@tailscale.com>
AllocateContiguousBuffer is for allocating structs with trailing buffers
containing additional data. It is to be used for various Windows structures
containing pointers to data located immediately after the struct.
SetNTString performs in-place setting of windows.NTString and
windows.NTUnicodeString.
Updates #12383
Signed-off-by: Aaron Klotz <aaron@tailscale.com>
StartupInfoBuilder is a helper for constructing StartupInfoEx structures
featuring proc/thread attribute lists. Calling its setters triggers the
appropriate setting of fields, adjusting flags as necessary, and populating
the proc/thread attribute list as necessary. Currently it supports four
features: setting std handles, setting pseudo-consoles, specifying handles
for inheritance, and specifying jobs.
The conpty package simplifies creation of pseudo-consoles, their associated
pipes, and assignment of the pty to StartupInfoEx proc/thread attributes.
Updates #12383
Signed-off-by: Aaron Klotz <aaron@tailscale.com>
Fixestailscale/corp#19459
This PR adds the ability for users of the syspolicy handler to read string arrays from the MDM solution configured on the system.
Signed-off-by: Andrea Gottardo <andrea@gottardo.me>
Package winenv provides information about the current Windows environment.
This includes details such as whether the device is a server or workstation,
and if it is AD domain-joined, MDM-registered, or neither.
Updates tailscale/corp#18342
Signed-off-by: Nick Khyl <nickk@tailscale.com>
This PR is all about adding functionality that will enable the installer's
upgrade sequence to terminate processes belonging to the previous version,
and then subsequently restart instances belonging to the new version within
the session(s) corresponding to the processes that were killed.
There are multiple parts to this:
* We add support for the Restart Manager APIs, which allow us to query the
OS for a list of processes locking specific files;
* We add the RestartableProcess and RestartableProcesses types that query
additional information about the running processes that will allow us
to correctly restart them in the future. These types also provide the
ability to terminate the processes.
* We add the StartProcessInSession family of APIs that permit us to create
new processes within specific sessions. This is needed in order to
properly attach a new GUI process to the same RDP session and desktop that
its previously-terminated counterpart would have been running in.
* I tweaked the winutil token APIs again.
* A lot of this stuff is pretty hard to test without a very elaborate
harness, but I added a unit test for the most complicated part (though it
requires LocalSystem to run).
Updates https://github.com/tailscale/corp/issues/13998
Signed-off-by: Aaron Klotz <aaron@tailscale.com>
The Windows base registry key is already exported but the policy key was
not. util/osdiag currently replicates the string rather than the
preferred approach of reusing the constant.
Updates #cleanup
Change-Id: I6c1c45337896c744059b85643da2364fb3f232f2
Signed-off-by: Adrian Dewhurst <adrian@tailscale.com>
This PR changes the internal getTokenInfo function to use generics.
I also removed our own implementations for obtaining a token's user
and primary group in favour of calling the ones now available in
x/sys/windows.
Furthermore, I added two new functions for working with tokens, logon
session IDs, and Terminal Services / RDP session IDs.
I modified our privilege enabling code to allow enabling of multiple
privileges via one single function call.
Finally, I added the ProcessImageName function and updated the code in
tailscaled_windows.go to use that instead of directly calling the
underlying API.
All of these changes will be utilized by subsequent PRs pertaining to
this issue.
Updates https://github.com/tailscale/corp/issues/13998
Signed-off-by: Aaron Klotz <aaron@tailscale.com>
To safely request and drop privileges, runtime.Lock/UnlockOSThread and
windows.Impersonate/RevertToSelf should be called. Add these calls to
winutil.EnableCurrentThreadPrivilege so that callers don't need to worry
about it.
Updates https://github.com/tailscale/corp/issues/15488
Signed-off-by: Andrew Lytvynov <awly@tailscale.com>
(*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>
I didn't clean up the more idiomatic map[T]bool with true values, at
least yet. I just converted the relatively awkward struct{}-valued
maps.
Updates #cleanup
Change-Id: I758abebd2bb1f64bc7a9d0f25c32298f4679c14f
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
In order for the installer to restart the GUI correctly post-upgrade, we
need the GUI to be able to register its restart preferences.
This PR adds API support for doing so. I'm adding it to OSS so that it
is available should we need to do any such registrations on OSS binaries
in the future.
Updates https://github.com/tailscale/corp/issues/13998
Signed-off-by: Aaron Klotz <aaron@tailscale.com>
* We update wingoes to pick up new version information functionality
(See pe/version.go in the https://github.com/dblohm7/wingoes repo);
* We move the existing LogSupportInfo code (including necessary syscall
stubs) out of util/winutil into a new package, util/osdiag, and implement
the public LogSupportInfo function may be implemented for other platforms
as needed;
* We add a new reason argument to LogSupportInfo and wire that into
localapi's bugreport implementation;
* We add module information to the Windows implementation of LogSupportInfo
when reason indicates a bugreport. We enumerate all loaded modules in our
process, and for each one we gather debug, authenticode signature, and
version information.
Fixes#7802
Signed-off-by: Aaron Klotz <aaron@tailscale.com>
Previously, tailscale upgrade was doing the bare minimum for checking
authenticode signatures via `WinVerifyTrustEx`. This is fine, but we can do
better:
* WinVerifyTrustEx verifies that the binary's signature is valid, but it doesn't
determine *whose* signature is valid; tailscale upgrade should also ensure that
the binary is actually signed *by us*.
* I added the ability to check the signatures of MSI files.
* In future PRs I will be adding diagnostic logging that lists details about
every module (ie, DLL) loaded into our process. As part of that metadata, I
want to be able to extract information about who signed the binaries.
This code is modelled on some C++ I wrote for Firefox back in the day. See
https://searchfox.org/mozilla-central/rev/27e4816536c891d85d63695025f2549fd7976392/toolkit/xre/dllservices/mozglue/Authenticode.cpp
for reference.
Fixes#8284
Signed-off-by: Aaron Klotz <aaron@tailscale.com>
In order to improve our ability to understand the state of policies and
registry settings when troubleshooting, we enumerate all values in all subkeys.
x/sys/windows does not already offer this, so we need to call RegEnumValue
directly.
For now we're just logging this during startup, however in a future PR I plan to
also trigger this code during a bugreport. I also want to log more than just
registry.
Fixes#8141
Signed-off-by: Aaron Klotz <aaron@tailscale.com>
This adds an initial and intentionally minimal configuration for
golang-ci, fixes the issues reported, and adds a GitHub Action to check
new pull requests against this linter configuration.
Signed-off-by: Andrew Dunham <andrew@du.nham.ca>
Change-Id: I8f38fbc315836a19a094d0d3e986758b9313f163
I realized that a lot of the problems that we're seeing around migration and
LocalBackend state can be avoided if we drive Windows pref migration entirely
from within tailscaled. By doing it this way, tailscaled can automatically
perform the migration as soon as the connection with the client frontend is
established.
Since tailscaled is already running as LocalSystem, it already has access to
the user's local AppData directory. The profile manager already knows which
user is connected, so we simply need to resolve the user's prefs file and read
it from there.
Of course, to properly migrate this information we need to also check system
policies. I moved a bunch of policy resolution code out of the GUI and into
a new package in util/winutil/policy.
Updates #7626
Signed-off-by: Aaron Klotz <aaron@tailscale.com>
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>
Goal: one way for users to update Tailscale, downgrade, switch tracks,
regardless of platform (Windows, most Linux distros, macOS, Synology).
This is a start.
Updates #755, etc
Change-Id: I23466da1ba41b45f0029ca79a17f5796c2eedd92
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
I added util/winutil/LookupPseudoUser, which essentially consists of the bits
that I am in the process of adding to Go's standard library.
We check the provided SID for "S-1-5-x" where 17 <= x <= 20 (which are the
known pseudo-users) and then manually populate a os/user.User struct with
the correct information.
Fixes https://github.com/tailscale/tailscale/issues/869
Fixes https://github.com/tailscale/tailscale/issues/2894
Signed-off-by: Aaron Klotz <aaron@tailscale.com>
The //go:build syntax was introduced in Go 1.17:
https://go.dev/doc/go1.17#build-lines
gofmt has kept the +build and go:build lines in sync since
then, but enough time has passed. Time to remove them.
Done with:
perl -i -npe 's,^// \+build.*\n,,' $(git grep -l -F '+build')
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
It's normal for HKLM\SOFTWARE\Policies\Tailscale to not exist but that
currently produces a lot of log spam.
Signed-off-by: Adrian Dewhurst <adrian@tailscale.com>
I added new functions to winutil to obtain the state of a service and all
its depedencies, serialize them to JSON, and write them to a Logf.
When tstun.New returns a wrapped ERROR_DEVICE_NOT_AVAILABLE, we know that wintun
installation failed. We then log the service graph rooted at "NetSetupSvc".
We are interested in that specific service because network devices will not
install if that service is not running.
Updates https://github.com/tailscale/tailscale/issues/5531
Signed-off-by: Aaron Klotz <aaron@tailscale.com>
AFAICT this isn't documented on MSDN, but based on the issue referenced below,
NRPT rules are not working when a rule specifies > 50 domains.
This patch modifies our NRPT rule generator to split the list of domains
into chunks as necessary, and write a separate rule for each chunk.
For compatibility reasons, we continue to use the hard-coded rule ID, but
as additional rules are required, we generate new GUIDs. Those GUIDs are
stored under the Tailscale registry path so that we know which rules are ours.
I made some changes to winutils to add additional helper functions in support
of both the code and its test: I added additional registry accessors, and also
moved some token accessors from paths to util/winutil.
Fixes https://github.com/tailscale/coral/issues/63
Signed-off-by: Aaron Klotz <aaron@tailscale.com>
I wrote this code way back at the beginning of my tenure at Tailscale when we
had concerns about needing to restore deleted machine keys from backups.
We never ended up using this functionality, and the code is now getting in the
way, so we might as well remove it.
Signed-off-by: Aaron Klotz <aaron@tailscale.com>
It makes the most sense to have all our utility functions reside in one place.
There was nothing in corp that could not reasonably live in OSS.
I also updated `StartProcessAsChild` to no longer depend on `futureexec`,
thus reducing the amount of code that needed migration. I tested this change
with `tswin` and it is working correctly.
I have a follow-up PR to remove the corresponding code from corp.
The migrated code was mostly written by @alexbrainman.
Sourced from corp revision 03e90cfcc4dd7b8bc9b25eb13a26ec3a24ae0ef9
Signed-off-by: Aaron Klotz <aaron@tailscale.com>
This patch adds new functions to be used when accessing system policies,
and revises callers to use the new functions. They first attempt the new
registry path for policies, and if that fails, attempt to fall back to the
legacy path.
We keep non-policy variants of these functions because we should be able to
retain the ability to read settings from locations that are not exposed to
sysadmins for group policy edits.
The remaining changes will be done in corp.
Updates https://github.com/tailscale/tailscale/issues/3584
Signed-off-by: Aaron Klotz <aaron@tailscale.com>
It was broken on Windows:
Error: util\winutil\winutil_windows.go:15:7: regBase redeclared in this block
Error: D:\a\tailscale\tailscale\util\winutil\winutil_notwindows.go:7:17: previous declaration
Error: util\winutil\winutil_windows.go:29:6: getRegString redeclared in this block
Error: D:\a\tailscale\tailscale\util\winutil\winutil_notwindows.go:9:40: previous declaration
Error: util\winutil\winutil_windows.go:47:6: getRegInteger redeclared in this block
Error: D:\a\tailscale\tailscale\util\winutil\winutil_notwindows.go:11:48: previous declaration
Error: util\winutil\winutil_windows.go:77:6: isSIDValidPrincipal redeclared in this block
Error: D:\a\tailscale\tailscale\util\winutil\winutil_notwindows.go:13:38: previous declaration
Change-Id: Ib1ce4b647f5711547840c736b933a6c42bf09583
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
Our current workaround made the user check too lax, thus allowing deleted
users. This patch adds a helper function to winutil that checks that the
uid's SID represents a valid Windows security principal.
Now if `lookupUserFromID` determines that the SID is invalid, we simply
propagate the error.
Updates https://github.com/tailscale/tailscale/issues/869
Signed-off-by: Aaron Klotz <aaron@tailscale.com>
Nick Khyl
Aaron Klotz
Brad Fitzpatrick
Andrea Gottardo
Andrew Lytvynov
Adrian Dewhurst
Claire Wang
Andrew Dunham
Will Norris
Maisem Ali