diff --git a/cmd/tailscale/depaware.txt b/cmd/tailscale/depaware.txt index 643040420..fc2a1d061 100644 --- a/cmd/tailscale/depaware.txt +++ b/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+ diff --git a/cmd/tailscaled/depaware.txt b/cmd/tailscaled/depaware.txt index 6e3ed913f..9740a3c9a 100644 --- a/cmd/tailscaled/depaware.txt +++ b/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+ diff --git a/ipn/store.go b/ipn/store.go index 58f6bbc38..c4f0d2c6d 100644 --- a/ipn/store.go +++ b/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 } diff --git a/paths/migrate.go b/paths/migrate.go index 7d1c74aae..8e4f3d595 100644 --- a/paths/migrate.go +++ b/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) diff --git a/paths/paths.go b/paths/paths.go index 1215ebb2f..c1f567206 100644 --- a/paths/paths.go +++ b/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) +} diff --git a/paths/paths_unix.go b/paths/paths_unix.go index fb11437c2..0bc413ad1 100644 --- a/paths/paths_unix.go +++ b/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 +} diff --git a/paths/paths_windows.go b/paths/paths_windows.go new file mode 100644 index 000000000..75d49100a --- /dev/null +++ b/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) +}