Parcourir la source

ipn/ipnauth, util/winutil: add temporary LookupPseudoUser workaround 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 https://github.com/tailscale/tailscale/issues/869
Fixes https://github.com/tailscale/tailscale/issues/2894

Signed-off-by: Aaron Klotz <[email protected]>
Aaron Klotz il y a 3 ans
Parent
commit
6e33d2da2b

+ 1 - 0
cmd/derper/depaware.txt

@@ -183,6 +183,7 @@ tailscale.com/cmd/derper dependencies: (generated by github.com/tailscale/depawa
         net/url                                                      from crypto/x509+
         net/url                                                      from crypto/x509+
         os                                                           from crypto/rand+
         os                                                           from crypto/rand+
         os/exec                                                      from golang.zx2c4.com/wireguard/windows/tunnel/winipcfg+
         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                                                         from golang.org/x/crypto/acme/autocert+
         path/filepath                                                from crypto/x509+
         path/filepath                                                from crypto/x509+
         reflect                                                      from crypto/x509+
         reflect                                                      from crypto/x509+

+ 15 - 2
ipn/ipnauth/ipnauth.go

@@ -14,7 +14,6 @@ import (
 	"os/user"
 	"os/user"
 	"runtime"
 	"runtime"
 	"strconv"
 	"strconv"
-	"syscall"
 
 
 	"inet.af/peercred"
 	"inet.af/peercred"
 	"tailscale.com/envknob"
 	"tailscale.com/envknob"
@@ -22,6 +21,7 @@ import (
 	"tailscale.com/net/netstat"
 	"tailscale.com/net/netstat"
 	"tailscale.com/safesocket"
 	"tailscale.com/safesocket"
 	"tailscale.com/types/logger"
 	"tailscale.com/types/logger"
+	"tailscale.com/util/clientmetric"
 	"tailscale.com/util/groupmember"
 	"tailscale.com/util/groupmember"
 	"tailscale.com/util/pidowner"
 	"tailscale.com/util/pidowner"
 	"tailscale.com/util/winutil"
 	"tailscale.com/util/winutil"
@@ -122,11 +122,23 @@ func GetConnIdentity(logf logger.Logf, c net.Conn) (ci *ConnIdentity, err error)
 	return ci, nil
 	return ci, nil
 }
 }
 
 
+var metricIssue869Workaround = clientmetric.NewCounter("issue_869_workaround")
+
 // LookupUserFromID is a wrapper around os/user.LookupId that works around some
 // 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.
 // issues on Windows. On non-Windows platforms it's identical to user.LookupId.
 func LookupUserFromID(logf logger.Logf, uid string) (*user.User, error) {
 func LookupUserFromID(logf logger.Logf, uid string) (*user.User, error) {
 	u, err := user.LookupId(uid)
 	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
 		// The below workaround is only applicable when uid represents a
 		// valid security principal. Omitting this check causes us to succeed
 		// valid security principal. Omitting this check causes us to succeed
 		// even when uid represents a deleted user.
 		// even when uid represents a deleted user.
@@ -134,6 +146,7 @@ func LookupUserFromID(logf logger.Logf, uid string) (*user.User, error) {
 			return nil, err
 			return nil, err
 		}
 		}
 
 
+		metricIssue869Workaround.Add(1)
 		logf("[warning] issue 869: os/user.LookupId failed; ignoring")
 		logf("[warning] issue 869: os/user.LookupId failed; ignoring")
 		// Work around https://github.com/tailscale/tailscale/issues/869 for
 		// 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.
 		// now. We don't strictly need the username. It's just a nice-to-have.

+ 14 - 0
util/winutil/winutil.go

@@ -5,6 +5,10 @@
 // Package winutil contains misc Windows/Win32 helper functions.
 // Package winutil contains misc Windows/Win32 helper functions.
 package winutil
 package winutil
 
 
+import (
+	"os/user"
+)
+
 // RegBase is the registry path inside HKEY_LOCAL_MACHINE where registry settings
 // 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.
 // are stored. This constant is a non-empty string only when GOOS=windows.
 const RegBase = regBase
 const RegBase = regBase
@@ -62,3 +66,13 @@ func GetRegInteger(name string, defval uint64) uint64 {
 func IsSIDValidPrincipal(uid string) bool {
 func IsSIDValidPrincipal(uid string) bool {
 	return isSIDValidPrincipal(uid)
 	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 - 0
util/winutil/winutil_notwindows.go

@@ -6,6 +6,12 @@
 
 
 package winutil
 package winutil
 
 
+import (
+	"fmt"
+	"os/user"
+	"runtime"
+)
+
 const regBase = ``
 const regBase = ``
 
 
 func getPolicyString(name, defval string) string { return defval }
 func getPolicyString(name, defval string) string { return defval }
@@ -17,3 +23,7 @@ func getRegString(name, defval string) string { return defval }
 func getRegInteger(name string, defval uint64) uint64 { return defval }
 func getRegInteger(name string, defval uint64) uint64 { return defval }
 
 
 func isSIDValidPrincipal(uid string) bool { return false }
 func isSIDValidPrincipal(uid string) bool { return false }
+
+func lookupPseudoUser(uid string) (*user.User, error) {
+	return nil, fmt.Errorf("unimplemented on %v", runtime.GOOS)
+}

+ 60 - 0
util/winutil/winutil_windows.go

@@ -9,6 +9,7 @@ import (
 	"fmt"
 	"fmt"
 	"log"
 	"log"
 	"os/exec"
 	"os/exec"
+	"os/user"
 	"runtime"
 	"runtime"
 	"strings"
 	"strings"
 	"syscall"
 	"syscall"
@@ -492,3 +493,62 @@ func OpenKeyWait(k registry.Key, path RegistryPath, access uint32) (registry.Key
 		k = 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 - 0
util/winutil/winutil_windows_test.go

@@ -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)
+	}
+}