Skip to content

Commit

Permalink
ipn/ipnauth, util/winutil: add temporary LookupPseudoUser workaround …
Browse files Browse the repository at this point in the history
…to address os/user.LookupId errors on Windows

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 tailscale#869
Fixes tailscale#2894

Signed-off-by: Aaron Klotz <aaron@tailscale.com>
  • Loading branch information
dblohm7 authored and coadler committed Feb 2, 2023
1 parent 6f37081 commit fd096d6
Show file tree
Hide file tree
Showing 6 changed files with 131 additions and 2 deletions.
1 change: 1 addition & 0 deletions cmd/derper/depaware.txt
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,7 @@ tailscale.com/cmd/derper dependencies: (generated by github.com/tailscale/depawa
net/url from crypto/x509+
os from crypto/rand+
os/exec from golang.zx2c4.com/wireguard/windows/tunnel/winipcfg+
W os/user from tailscale.com/util/winutil
path from golang.org/x/crypto/acme/autocert+
path/filepath from crypto/x509+
reflect from crypto/x509+
Expand Down
17 changes: 15 additions & 2 deletions ipn/ipnauth/ipnauth.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,14 +14,14 @@ import (
"os/user"
"runtime"
"strconv"
"syscall"

"inet.af/peercred"
"tailscale.com/envknob"
"tailscale.com/ipn"
"tailscale.com/net/netstat"
"tailscale.com/safesocket"
"tailscale.com/types/logger"
"tailscale.com/util/clientmetric"
"tailscale.com/util/groupmember"
"tailscale.com/util/pidowner"
"tailscale.com/util/winutil"
Expand Down Expand Up @@ -122,18 +122,31 @@ func GetConnIdentity(logf logger.Logf, c net.Conn) (ci *ConnIdentity, err error)
return ci, nil
}

var metricIssue869Workaround = clientmetric.NewCounter("issue_869_workaround")

// LookupUserFromID is a wrapper around os/user.LookupId that works around some
// issues on Windows. On non-Windows platforms it's identical to user.LookupId.
func LookupUserFromID(logf logger.Logf, uid string) (*user.User, error) {
u, err := user.LookupId(uid)
if err != nil && runtime.GOOS == "windows" && errors.Is(err, syscall.Errno(0x534)) {
if err != nil && runtime.GOOS == "windows" {
// See if uid resolves as a pseudo-user. Temporary workaround until
// https://github.com/golang/go/issues/49509 resolves and ships.
if u, err := winutil.LookupPseudoUser(uid); err == nil {
return u, nil
}

// TODO(aaron): With LookupPseudoUser in place, I don't expect us to reach
// this point anymore. Leaving the below workaround in for now to confirm
// that pseudo-user resolution sufficiently handles this problem.

// The below workaround is only applicable when uid represents a
// valid security principal. Omitting this check causes us to succeed
// even when uid represents a deleted user.
if !winutil.IsSIDValidPrincipal(uid) {
return nil, err
}

metricIssue869Workaround.Add(1)
logf("[warning] issue 869: os/user.LookupId failed; ignoring")
// Work around https://github.com/tailscale/tailscale/issues/869 for
// now. We don't strictly need the username. It's just a nice-to-have.
Expand Down
14 changes: 14 additions & 0 deletions util/winutil/winutil.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,10 @@
// Package winutil contains misc Windows/Win32 helper functions.
package winutil

import (
"os/user"
)

// RegBase is the registry path inside HKEY_LOCAL_MACHINE where registry settings
// are stored. This constant is a non-empty string only when GOOS=windows.
const RegBase = regBase
Expand Down Expand Up @@ -62,3 +66,13 @@ func GetRegInteger(name string, defval uint64) uint64 {
func IsSIDValidPrincipal(uid string) bool {
return isSIDValidPrincipal(uid)
}

// LookupPseudoUser attempts to resolve the user specified by uid by checking
// against well-known pseudo-users on Windows. This is a temporary workaround
// until https://github.com/golang/go/issues/49509 is resolved and shipped.
//
// This function will only work on GOOS=windows. Trying to run it on any other
// OS will always return an error.
func LookupPseudoUser(uid string) (*user.User, error) {
return lookupPseudoUser(uid)
}
10 changes: 10 additions & 0 deletions util/winutil/winutil_notwindows.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,12 @@

package winutil

import (
"fmt"
"os/user"
"runtime"
)

const regBase = ``

func getPolicyString(name, defval string) string { return defval }
Expand All @@ -17,3 +23,7 @@ func getRegString(name, defval string) string { return defval }
func getRegInteger(name string, defval uint64) uint64 { return defval }

func isSIDValidPrincipal(uid string) bool { return false }

func lookupPseudoUser(uid string) (*user.User, error) {
return nil, fmt.Errorf("unimplemented on %v", runtime.GOOS)
}
60 changes: 60 additions & 0 deletions util/winutil/winutil_windows.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"fmt"
"log"
"os/exec"
"os/user"
"runtime"
"strings"
"syscall"
Expand Down Expand Up @@ -492,3 +493,62 @@ func OpenKeyWait(k registry.Key, path RegistryPath, access uint32) (registry.Key
k = key
}
}

func lookupPseudoUser(uid string) (*user.User, error) {
sid, err := windows.StringToSid(uid)
if err != nil {
return nil, err
}

// We're looking for SIDs "S-1-5-x" where 17 <= x <= 20.
// This is checking for the the "5"
if sid.IdentifierAuthority() != windows.SECURITY_NT_AUTHORITY {
return nil, fmt.Errorf(`SID %q does not use "NT AUTHORITY"`, uid)
}

// This is ensuring that there is only one sub-authority.
// In other words, only one value after the "5".
if sid.SubAuthorityCount() != 1 {
return nil, fmt.Errorf("SID %q should have only one subauthority", uid)
}

// Get that sub-authority value (this is "x" above) and check it.
rid := sid.SubAuthority(0)
if rid < 17 || rid > 20 {
return nil, fmt.Errorf("SID %q does not represent a known pseudo-user", uid)
}

// We've got one of the known pseudo-users. Look up the localized name of the
// account.
username, domain, _, err := sid.LookupAccount("")
if err != nil {
return nil, err
}

// This call is best-effort. If it fails, homeDir will be empty.
homeDir, _ := findHomeDirInRegistry(uid)

result := &user.User{
Uid: uid,
Gid: uid, // Gid == Uid with these accounts.
Username: fmt.Sprintf(`%s\%s`, domain, username),
Name: username,
HomeDir: homeDir,
}
return result, nil
}

// findHomeDirInRegistry finds the user home path based on the uid.
// This is borrowed from Go's std lib.
func findHomeDirInRegistry(uid string) (dir string, err error) {
k, err := registry.OpenKey(registry.LOCAL_MACHINE, `SOFTWARE\Microsoft\Windows NT\CurrentVersion\ProfileList\`+uid, registry.QUERY_VALUE)
if err != nil {
return "", err
}
defer k.Close()
dir, _, err = k.GetStringValue("ProfileImagePath")
if err != nil {
return "", err
}
return dir, nil
}
31 changes: 31 additions & 0 deletions util/winutil/winutil_windows_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
// Copyright (c) 2022 Tailscale Inc & AUTHORS All rights reserved.
// Use of this source code is governed by a BSD-style
// license that can be found in the LICENSE file.

package winutil

import (
"testing"
)

const (
localSystemSID = "S-1-5-18"
networkSID = "S-1-5-2"
)

func TestLookupPseudoUser(t *testing.T) {
localSystem, err := LookupPseudoUser(localSystemSID)
if err != nil {
t.Errorf("LookupPseudoUser(%q) error: %v", localSystemSID, err)
}
if localSystem.Gid != localSystemSID {
t.Errorf("incorrect Gid, got %q, want %q", localSystem.Gid, localSystemSID)
}
t.Logf("localSystem: %v", localSystem)

// networkSID is a built-in known group but not a pseudo-user.
_, err = LookupPseudoUser(networkSID)
if err == nil {
t.Errorf("LookupPseudoUser(%q) unexpectedly succeeded", networkSID)
}
}

0 comments on commit fd096d6

Please sign in to comment.