Merge pull request #2893 from tailscale/aaron/programdata-perms-2

ipn, paths: ensure that the state directory for Windows has the corre…

cmd/tailscale/depaware.txt

@@ -44,7 +44,7 @@ tailscale.com/cmd/tailscale dependencies: (generated by github.com/tailscale/dep
         tailscale.com/net/tlsdial                                    from tailscale.com/derp/derphttp
         tailscale.com/net/tsaddr                                     from tailscale.com/net/interfaces+
      💣 tailscale.com/net/tshttpproxy                                from tailscale.com/derp/derphttp+
-        tailscale.com/paths                                          from tailscale.com/cmd/tailscale/cli+
+     💣 tailscale.com/paths                                          from tailscale.com/cmd/tailscale/cli+
         tailscale.com/safesocket                                     from tailscale.com/cmd/tailscale/cli+
      💣 tailscale.com/syncs                                          from tailscale.com/net/interfaces+
         tailscale.com/tailcfg                                        from tailscale.com/cmd/tailscale/cli+

cmd/tailscaled/depaware.txt

@@ -131,7 +131,7 @@ tailscale.com/cmd/tailscaled dependencies: (generated by github.com/tailscale/de
         tailscale.com/net/tsaddr                                     from tailscale.com/ipn/ipnlocal+
      💣 tailscale.com/net/tshttpproxy                                from tailscale.com/control/controlclient+
         tailscale.com/net/tstun                                      from tailscale.com/cmd/tailscaled+
-        tailscale.com/paths                                          from tailscale.com/cmd/tailscaled+
+     💣 tailscale.com/paths                                          from tailscale.com/cmd/tailscaled+
         tailscale.com/portlist                                       from tailscale.com/ipn/ipnlocal
         tailscale.com/safesocket                                     from tailscale.com/ipn/ipnserver+
         tailscale.com/smallzstd                                      from tailscale.com/ipn/ipnserver+

ipn/store.go

@@ -19,6 +19,7 @@ import (
 
 	"tailscale.com/atomicfile"
 	"tailscale.com/kube"
+	"tailscale.com/paths"
 )
 
 // ErrStateNotExist is returned by StateStore.ReadState when the
@@ -182,7 +183,9 @@ func NewFileStore(path string) (*FileStore, error) {
 		if os.IsNotExist(err) {
 			// Write out an initial file, to verify that we can write
 			// to the path.
-			os.MkdirAll(filepath.Dir(path), 0755) // best effort
+			if err := paths.MkStateDir(filepath.Dir(path)); err != nil {
+				return nil, fmt.Errorf("creating state directory: %w", err)
+			}
 			if err = atomicfile.WriteFile(path, []byte("{}"), 0600); err != nil {
 				return nil, err
 			}

paths/migrate.go

@@ -35,7 +35,11 @@ func TryConfigFileMigration(logf logger.Logf, oldFile, newFile string) string {
 		return newFile
 	}
 
-	os.MkdirAll(filepath.Dir(newFile), 0700)
+	if err = MkStateDir(filepath.Dir(newFile)); err != nil {
+		logf("TryConfigFileMigration failed; MkStateDir: %v", err)
+		return oldFile
+	}
+
 	err = os.WriteFile(newFile, contents, 0600)
 	if err != nil {
 		removeErr := os.Remove(newFile)

paths/paths.go

@@ -59,3 +59,14 @@ func DefaultTailscaledStateFile() string {
 	}
 	return ""
 }
+
+// MkStateDir ensures that dirPath, the daemon's configurtaion directory
+// containing machine keys etc, both exists and has the correct permissions.
+// We want it to only be accessible to the user the daemon is running under.
+func MkStateDir(dirPath string) error {
+	if err := os.MkdirAll(dirPath, 0700); err != nil {
+		return err
+	}
+
+	return ensureStateDirPerms(dirPath)
+}

paths/paths_unix.go

@@ -61,3 +61,11 @@ func xdgDataHome() string {
 	}
 	return filepath.Join(os.Getenv("HOME"), ".local/share")
 }
+
+func ensureStateDirPerms(dirPath string) error {
+	// Unfortunately there are currently numerous tests that set up state files
+	// right off of /tmp, on which Chmod will of course fail. We should fix our
+	// test harnesses to not do that, at which point we can return an error.
+	os.Chmod(dirPath, 0700)
+	return nil
+}

paths/paths_windows.go

@@ -0,0 +1,145 @@
+// Copyright (c) 2021 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 paths
+
+import (
+	"os"
+	"unsafe"
+
+	"golang.org/x/sys/windows"
+)
+
+func getTokenInfo(token windows.Token, infoClass uint32) ([]byte, error) {
+	var desiredLen uint32
+	err := windows.GetTokenInformation(token, infoClass, nil, 0, &desiredLen)
+	if err != nil && err != windows.ERROR_INSUFFICIENT_BUFFER {
+		return nil, err
+	}
+
+	buf := make([]byte, desiredLen)
+	actualLen := desiredLen
+	err = windows.GetTokenInformation(token, infoClass, &buf[0], desiredLen, &actualLen)
+	return buf, err
+}
+
+func getTokenUserInfo(token windows.Token) (*windows.Tokenuser, error) {
+	buf, err := getTokenInfo(token, windows.TokenUser)
+	if err != nil {
+		return nil, err
+	}
+
+	return (*windows.Tokenuser)(unsafe.Pointer(&buf[0])), nil
+}
+
+func getTokenPrimaryGroupInfo(token windows.Token) (*windows.Tokenprimarygroup, error) {
+	buf, err := getTokenInfo(token, windows.TokenPrimaryGroup)
+	if err != nil {
+		return nil, err
+	}
+
+	return (*windows.Tokenprimarygroup)(unsafe.Pointer(&buf[0])), nil
+}
+
+type userSids struct {
+	User         *windows.SID
+	PrimaryGroup *windows.SID
+}
+
+func getCurrentUserSids() (*userSids, error) {
+	token, err := windows.OpenCurrentProcessToken()
+	if err != nil {
+		return nil, err
+	}
+	defer token.Close()
+
+	userInfo, err := getTokenUserInfo(token)
+	if err != nil {
+		return nil, err
+	}
+
+	primaryGroup, err := getTokenPrimaryGroupInfo(token)
+	if err != nil {
+		return nil, err
+	}
+
+	return &userSids{userInfo.User.Sid, primaryGroup.PrimaryGroup}, nil
+}
+
+// ensureStateDirPerms applies a restrictive ACL to the directory specified by dirPath.
+// It sets the following security attributes on the directory:
+// Owner: The user for the current process;
+// Primary Group: The primary group for the current process;
+// DACL: Full control to the current user and to the Administrators group.
+//       (We include Administrators so that admin users may still access logs;
+//        granting access exclusively to LocalSystem would require admins to use
+//        special tools to access the Log directory)
+// Inheritance: The directory does not inherit the ACL from its parent.
+//              However, any directories and/or files created within this
+//              directory *do* inherit the ACL that we are setting.
+func ensureStateDirPerms(dirPath string) error {
+	fi, err := os.Stat(dirPath)
+	if err != nil {
+		return err
+	}
+	if !fi.IsDir() {
+		return os.ErrInvalid
+	}
+
+	// We need the info for our current user as SIDs
+	sids, err := getCurrentUserSids()
+	if err != nil {
+		return err
+	}
+
+	// We also need the SID for the Administrators group so that admins may
+	// easily access logs.
+	adminGroupSid, err := windows.CreateWellKnownSid(windows.WinBuiltinAdministratorsSid)
+	if err != nil {
+		return err
+	}
+
+	// Munge the SIDs into the format required by EXPLICIT_ACCESS.
+	userTrustee := windows.TRUSTEE{nil, windows.NO_MULTIPLE_TRUSTEE,
+		windows.TRUSTEE_IS_SID, windows.TRUSTEE_IS_USER,
+		windows.TrusteeValueFromSID(sids.User)}
+
+	adminTrustee := windows.TRUSTEE{nil, windows.NO_MULTIPLE_TRUSTEE,
+		windows.TRUSTEE_IS_SID, windows.TRUSTEE_IS_WELL_KNOWN_GROUP,
+		windows.TrusteeValueFromSID(adminGroupSid)}
+
+	// We declare our access rights via this array of EXPLICIT_ACCESS structures.
+	// We set full access to our user and to Administrators.
+	// We configure the DACL such that any files or directories created within
+	// dirPath will also inherit this DACL.
+	explicitAccess := []windows.EXPLICIT_ACCESS{
+		windows.EXPLICIT_ACCESS{
+			windows.GENERIC_ALL,
+			windows.SET_ACCESS,
+			windows.SUB_CONTAINERS_AND_OBJECTS_INHERIT,
+			userTrustee,
+		},
+		windows.EXPLICIT_ACCESS{
+			windows.GENERIC_ALL,
+			windows.SET_ACCESS,
+			windows.SUB_CONTAINERS_AND_OBJECTS_INHERIT,
+			adminTrustee,
+		},
+	}
+
+	dacl, err := windows.ACLFromEntries(explicitAccess, nil)
+	if err != nil {
+		return err
+	}
+
+	// We now reset the file's owner, primary group, and DACL.
+	// We also must pass PROTECTED_DACL_SECURITY_INFORMATION so that our new ACL
+	// does not inherit any ACL entries from the parent directory.
+	const flags = windows.OWNER_SECURITY_INFORMATION |
+		windows.GROUP_SECURITY_INFORMATION |
+		windows.DACL_SECURITY_INFORMATION |
+		windows.PROTECTED_DACL_SECURITY_INFORMATION
+	return windows.SetNamedSecurityInfo(dirPath, windows.SE_FILE_OBJECT, flags,
+		sids.User, sids.PrimaryGroup, dacl, nil)
+}