diff --git a/atomicfile/atomicfile.go b/atomicfile/atomicfile.go index 5c18e85a8..b3c8c93da 100644 --- a/atomicfile/atomicfile.go +++ b/atomicfile/atomicfile.go @@ -15,8 +15,9 @@ import ( ) // WriteFile writes data to filename+some suffix, then renames it into filename. -// The perm argument is ignored on Windows. If the target filename already -// exists but is not a regular file, WriteFile returns an error. +// The perm argument is ignored on Windows, but if the target filename already +// exists then the target file's attributes and ACLs are preserved. If the target +// filename already exists but is not a regular file, WriteFile returns an error. func WriteFile(filename string, data []byte, perm os.FileMode) (err error) { fi, err := os.Stat(filename) if err == nil && !fi.Mode().IsRegular() { @@ -47,5 +48,5 @@ func WriteFile(filename string, data []byte, perm os.FileMode) (err error) { if err := f.Close(); err != nil { return err } - return os.Rename(tmpName, filename) + return rename(tmpName, filename) } diff --git a/atomicfile/atomicfile_notwindows.go b/atomicfile/atomicfile_notwindows.go new file mode 100644 index 000000000..1ce2bb8ac --- /dev/null +++ b/atomicfile/atomicfile_notwindows.go @@ -0,0 +1,14 @@ +// Copyright (c) Tailscale Inc & AUTHORS +// SPDX-License-Identifier: BSD-3-Clause + +//go:build !windows + +package atomicfile + +import ( + "os" +) + +func rename(srcFile, destFile string) error { + return os.Rename(srcFile, destFile) +} diff --git a/atomicfile/atomicfile_windows.go b/atomicfile/atomicfile_windows.go new file mode 100644 index 000000000..c67762df2 --- /dev/null +++ b/atomicfile/atomicfile_windows.go @@ -0,0 +1,33 @@ +// Copyright (c) Tailscale Inc & AUTHORS +// SPDX-License-Identifier: BSD-3-Clause + +package atomicfile + +import ( + "os" + + "golang.org/x/sys/windows" +) + +func rename(srcFile, destFile string) error { + // Use replaceFile when possible to preserve the original file's attributes and ACLs. + if err := replaceFile(destFile, srcFile); err == nil || err != windows.ERROR_FILE_NOT_FOUND { + return err + } + // destFile doesn't exist. Just do a normal rename. + return os.Rename(srcFile, destFile) +} + +func replaceFile(destFile, srcFile string) error { + destFile16, err := windows.UTF16PtrFromString(destFile) + if err != nil { + return err + } + + srcFile16, err := windows.UTF16PtrFromString(srcFile) + if err != nil { + return err + } + + return replaceFileW(destFile16, srcFile16, nil, 0, nil, nil) +} diff --git a/atomicfile/atomicfile_windows_test.go b/atomicfile/atomicfile_windows_test.go new file mode 100644 index 000000000..4dec1493e --- /dev/null +++ b/atomicfile/atomicfile_windows_test.go @@ -0,0 +1,146 @@ +// Copyright (c) Tailscale Inc & AUTHORS +// SPDX-License-Identifier: BSD-3-Clause + +package atomicfile + +import ( + "os" + "testing" + "unsafe" + + "golang.org/x/sys/windows" +) + +var _SECURITY_RESOURCE_MANAGER_AUTHORITY = windows.SidIdentifierAuthority{[6]byte{0, 0, 0, 0, 0, 9}} + +// makeRandomSID generates a SID derived from a v4 GUID. +// This is basically the same algorithm used by browser sandboxes for generating +// random SIDs. +func makeRandomSID() (*windows.SID, error) { + guid, err := windows.GenerateGUID() + if err != nil { + return nil, err + } + + rids := *((*[4]uint32)(unsafe.Pointer(&guid))) + + var pSID *windows.SID + if err := windows.AllocateAndInitializeSid(&_SECURITY_RESOURCE_MANAGER_AUTHORITY, 4, rids[0], rids[1], rids[2], rids[3], 0, 0, 0, 0, &pSID); err != nil { + return nil, err + } + defer windows.FreeSid(pSID) + + // Make a copy that lives on the Go heap + return pSID.Copy() +} + +func getExistingFileSD(name string) (*windows.SECURITY_DESCRIPTOR, error) { + const infoFlags = windows.DACL_SECURITY_INFORMATION + return windows.GetNamedSecurityInfo(name, windows.SE_FILE_OBJECT, infoFlags) +} + +func getExistingFileDACL(name string) (*windows.ACL, error) { + sd, err := getExistingFileSD(name) + if err != nil { + return nil, err + } + + dacl, _, err := sd.DACL() + return dacl, err +} + +func addDenyACEForRandomSID(dacl *windows.ACL) (*windows.ACL, error) { + randomSID, err := makeRandomSID() + if err != nil { + return nil, err + } + + randomSIDTrustee := windows.TRUSTEE{nil, windows.NO_MULTIPLE_TRUSTEE, + windows.TRUSTEE_IS_SID, windows.TRUSTEE_IS_UNKNOWN, + windows.TrusteeValueFromSID(randomSID)} + + entries := []windows.EXPLICIT_ACCESS{ + { + windows.GENERIC_ALL, + windows.DENY_ACCESS, + windows.NO_INHERITANCE, + randomSIDTrustee, + }, + } + + return windows.ACLFromEntries(entries, dacl) +} + +func setExistingFileDACL(name string, dacl *windows.ACL) error { + return windows.SetNamedSecurityInfo(name, windows.SE_FILE_OBJECT, + windows.DACL_SECURITY_INFORMATION, nil, nil, dacl, nil) +} + +// makeOrigFileWithCustomDACL creates a new, temporary file with a custom +// DACL that we can check for later. It returns the name of the temporary +// file and the security descriptor for the file in SDDL format. +func makeOrigFileWithCustomDACL() (name, sddl string, err error) { + f, err := os.CreateTemp("", "foo*.tmp") + if err != nil { + return "", "", err + } + name = f.Name() + if err := f.Close(); err != nil { + return "", "", err + } + f = nil + defer func() { + if err != nil { + os.Remove(name) + } + }() + + dacl, err := getExistingFileDACL(name) + if err != nil { + return "", "", err + } + + // Add a harmless, deny-only ACE for a random SID that isn't used for anything + // (but that we can check for later). + dacl, err = addDenyACEForRandomSID(dacl) + if err != nil { + return "", "", err + } + + if err := setExistingFileDACL(name, dacl); err != nil { + return "", "", err + } + + sd, err := getExistingFileSD(name) + if err != nil { + return "", "", err + } + + return name, sd.String(), nil +} + +func TestPreserveSecurityInfo(t *testing.T) { + // Make a test file with a custom ACL. + origFileName, want, err := makeOrigFileWithCustomDACL() + if err != nil { + t.Fatalf("makeOrigFileWithCustomDACL returned %v", err) + } + t.Cleanup(func() { + os.Remove(origFileName) + }) + + if err := WriteFile(origFileName, []byte{}, 0); err != nil { + t.Fatalf("WriteFile returned %v", err) + } + + // We expect origFileName's security descriptor to be unchanged despite + // the WriteFile call. + sd, err := getExistingFileSD(origFileName) + if err != nil { + t.Fatalf("getExistingFileSD(%q) returned %v", origFileName, err) + } + + if got := sd.String(); got != want { + t.Errorf("security descriptor comparison failed: got %q, want %q", got, want) + } +} diff --git a/atomicfile/mksyscall.go b/atomicfile/mksyscall.go new file mode 100644 index 000000000..d8951a77c --- /dev/null +++ b/atomicfile/mksyscall.go @@ -0,0 +1,8 @@ +// Copyright (c) Tailscale Inc & AUTHORS +// SPDX-License-Identifier: BSD-3-Clause + +package atomicfile + +//go:generate go run golang.org/x/sys/windows/mkwinsyscall -output zsyscall_windows.go mksyscall.go + +//sys replaceFileW(replaced *uint16, replacement *uint16, backup *uint16, flags uint32, exclude unsafe.Pointer, reserved unsafe.Pointer) (err error) [int32(failretval)==0] = kernel32.ReplaceFileW diff --git a/atomicfile/zsyscall_windows.go b/atomicfile/zsyscall_windows.go new file mode 100644 index 000000000..f2f0b6d08 --- /dev/null +++ b/atomicfile/zsyscall_windows.go @@ -0,0 +1,52 @@ +// Code generated by 'go generate'; DO NOT EDIT. + +package atomicfile + +import ( + "syscall" + "unsafe" + + "golang.org/x/sys/windows" +) + +var _ unsafe.Pointer + +// Do the interface allocations only once for common +// Errno values. +const ( + errnoERROR_IO_PENDING = 997 +) + +var ( + errERROR_IO_PENDING error = syscall.Errno(errnoERROR_IO_PENDING) + errERROR_EINVAL error = syscall.EINVAL +) + +// errnoErr returns common boxed Errno values, to prevent +// allocations at runtime. +func errnoErr(e syscall.Errno) error { + switch e { + case 0: + return errERROR_EINVAL + case errnoERROR_IO_PENDING: + return errERROR_IO_PENDING + } + // TODO: add more here, after collecting data on the common + // error values see on Windows. (perhaps when running + // all.bat?) + return e +} + +var ( + modkernel32 = windows.NewLazySystemDLL("kernel32.dll") + + procReplaceFileW = modkernel32.NewProc("ReplaceFileW") +) + +func replaceFileW(replaced *uint16, replacement *uint16, backup *uint16, flags uint32, exclude unsafe.Pointer, reserved unsafe.Pointer) (err error) { + r1, _, e1 := syscall.Syscall6(procReplaceFileW.Addr(), 6, uintptr(unsafe.Pointer(replaced)), uintptr(unsafe.Pointer(replacement)), uintptr(unsafe.Pointer(backup)), uintptr(flags), uintptr(exclude), uintptr(reserved)) + if int32(r1) == 0 { + err = errnoErr(e1) + } + return +} diff --git a/cmd/derper/depaware.txt b/cmd/derper/depaware.txt index d4b406d9d..729122d79 100644 --- a/cmd/derper/depaware.txt +++ b/cmd/derper/depaware.txt @@ -85,7 +85,7 @@ tailscale.com/cmd/derper dependencies: (generated by github.com/tailscale/depawa google.golang.org/protobuf/runtime/protoimpl from github.com/prometheus/client_model/go+ google.golang.org/protobuf/types/known/timestamppb from github.com/prometheus/client_golang/prometheus+ tailscale.com from tailscale.com/version - tailscale.com/atomicfile from tailscale.com/cmd/derper+ + 💣 tailscale.com/atomicfile from tailscale.com/cmd/derper+ tailscale.com/client/tailscale from tailscale.com/derp tailscale.com/client/tailscale/apitype from tailscale.com/client/tailscale tailscale.com/derp from tailscale.com/cmd/derper+ diff --git a/cmd/k8s-operator/depaware.txt b/cmd/k8s-operator/depaware.txt index f757cda18..cb02038e3 100644 --- a/cmd/k8s-operator/depaware.txt +++ b/cmd/k8s-operator/depaware.txt @@ -643,7 +643,7 @@ tailscale.com/cmd/k8s-operator dependencies: (generated by github.com/tailscale/ sigs.k8s.io/yaml/goyaml.v2 from sigs.k8s.io/yaml tailscale.com from tailscale.com/version tailscale.com/appc from tailscale.com/ipn/ipnlocal - tailscale.com/atomicfile from tailscale.com/ipn+ + 💣 tailscale.com/atomicfile from tailscale.com/ipn+ tailscale.com/client/tailscale from tailscale.com/client/web+ tailscale.com/client/tailscale/apitype from tailscale.com/client/tailscale+ tailscale.com/client/web from tailscale.com/ipn/ipnlocal diff --git a/cmd/tailscale/depaware.txt b/cmd/tailscale/depaware.txt index e894e0674..9ccd6eebd 100644 --- a/cmd/tailscale/depaware.txt +++ b/cmd/tailscale/depaware.txt @@ -69,7 +69,7 @@ tailscale.com/cmd/tailscale dependencies: (generated by github.com/tailscale/dep software.sslmate.com/src/go-pkcs12 from tailscale.com/cmd/tailscale/cli software.sslmate.com/src/go-pkcs12/internal/rc2 from software.sslmate.com/src/go-pkcs12 tailscale.com from tailscale.com/version - tailscale.com/atomicfile from tailscale.com/cmd/tailscale/cli+ + 💣 tailscale.com/atomicfile from tailscale.com/cmd/tailscale/cli+ tailscale.com/client/tailscale from tailscale.com/client/web+ tailscale.com/client/tailscale/apitype from tailscale.com/client/tailscale+ tailscale.com/client/web from tailscale.com/cmd/tailscale/cli diff --git a/cmd/tailscaled/depaware.txt b/cmd/tailscaled/depaware.txt index 19254b616..8af347319 100644 --- a/cmd/tailscaled/depaware.txt +++ b/cmd/tailscaled/depaware.txt @@ -232,7 +232,7 @@ tailscale.com/cmd/tailscaled dependencies: (generated by github.com/tailscale/de gvisor.dev/gvisor/pkg/waiter from gvisor.dev/gvisor/pkg/context+ tailscale.com from tailscale.com/version tailscale.com/appc from tailscale.com/ipn/ipnlocal - tailscale.com/atomicfile from tailscale.com/ipn+ + 💣 tailscale.com/atomicfile from tailscale.com/ipn+ LD tailscale.com/chirp from tailscale.com/cmd/tailscaled tailscale.com/client/tailscale from tailscale.com/client/web+ tailscale.com/client/tailscale/apitype from tailscale.com/client/tailscale+