From e7d29220193a6f12b95831b6d86e8f49bd4e863d Mon Sep 17 00:00:00 2001 From: Michael Eischer Date: Sat, 2 Nov 2024 23:12:36 +0100 Subject: [PATCH] fs: make generic attribute reading file handle based --- internal/fs/meta_windows.go | 18 ++++++++++++++---- internal/fs/node_windows_test.go | 5 ++++- internal/fs/sd_windows.go | 22 +++++++++++----------- internal/fs/sd_windows_test.go | 6 +++++- 4 files changed, 34 insertions(+), 17 deletions(-) diff --git a/internal/fs/meta_windows.go b/internal/fs/meta_windows.go index 24822c0f1..fec9794d1 100644 --- a/internal/fs/meta_windows.go +++ b/internal/fs/meta_windows.go @@ -48,8 +48,19 @@ func xattrFromPath(path string) ([]restic.ExtendedAttribute, error) { return extendedAttrs, nil } -func (p *pathMetadataHandle) SecurityDescriptor() (*[]byte, error) { - return getSecurityDescriptor(p.name) +func (p *pathMetadataHandle) SecurityDescriptor() (buf *[]byte, err error) { + f, err := openMetadataHandle(p.name, 0) + if err != nil { + return nil, err + } + defer func() { + cerr := f.Close() + if err == nil { + err = cerr + } + }() + + return getSecurityDescriptor(windows.Handle(f.Fd())) } func (p *fdMetadataHandle) Xattr(_ bool) ([]restic.ExtendedAttribute, error) { @@ -58,8 +69,7 @@ func (p *fdMetadataHandle) Xattr(_ bool) ([]restic.ExtendedAttribute, error) { } func (p *fdMetadataHandle) SecurityDescriptor() (*[]byte, error) { - // FIXME - return getSecurityDescriptor(p.name) + return getSecurityDescriptor(windows.Handle(p.f.Fd())) } func openMetadataHandle(path string, flag int) (*os.File, error) { diff --git a/internal/fs/node_windows_test.go b/internal/fs/node_windows_test.go index f75df54d3..438d91b84 100644 --- a/internal/fs/node_windows_test.go +++ b/internal/fs/node_windows_test.go @@ -47,8 +47,11 @@ func testRestoreSecurityDescriptor(t *testing.T, sd string, tempDir string, file sdByteFromRestoredNode := getWindowsAttr(t, testPath, node).SecurityDescriptor // Get the security descriptor for the test path after the restore. - sdBytesFromRestoredPath, err := getSecurityDescriptor(testPath) + handle, err := openMetadataHandle(testPath, 0) + test.OK(t, err) + sdBytesFromRestoredPath, err := getSecurityDescriptor(windows.Handle(handle.Fd())) test.OK(t, errors.Wrapf(err, "Error while getting the security descriptor for: %s", testPath)) + test.OK(t, handle.Close()) // Compare the input SD and the SD got from the restored file. compareSecurityDescriptors(t, testPath, sdInputBytes, *sdBytesFromRestoredPath) diff --git a/internal/fs/sd_windows.go b/internal/fs/sd_windows.go index 6bffa4fe2..3b7824737 100644 --- a/internal/fs/sd_windows.go +++ b/internal/fs/sd_windows.go @@ -43,7 +43,7 @@ var lowRestoreSecurityFlags windows.SECURITY_INFORMATION = windows.DACL_SECURITY // getSecurityDescriptor takes the path of the file and returns the SecurityDescriptor for the file. // This needs admin permissions or SeBackupPrivilege for getting the full SD. // If there are no admin permissions, only the current user's owner, group and DACL will be got. -func getSecurityDescriptor(filePath string) (securityDescriptor *[]byte, err error) { +func getSecurityDescriptor(handle windows.Handle) (securityDescriptor *[]byte, err error) { onceBackup.Do(enableBackupPrivilege) var sd *windows.SECURITY_DESCRIPTOR @@ -51,9 +51,9 @@ func getSecurityDescriptor(filePath string) (securityDescriptor *[]byte, err err // store original value to avoid unrelated changes in the error check useLowerPrivileges := lowerPrivileges.Load() if useLowerPrivileges { - sd, err = getNamedSecurityInfoLow(filePath) + sd, err = securityInfoLow(handle) } else { - sd, err = getNamedSecurityInfoHigh(filePath) + sd, err = securityInfoHigh(handle) // Fallback to the low privilege version when receiving an access denied error. // For some reason the ERROR_PRIVILEGE_NOT_HELD error is not returned for removable media // but instead an access denied error is returned. Workaround that by just retrying with @@ -61,14 +61,14 @@ func getSecurityDescriptor(filePath string) (securityDescriptor *[]byte, err err // case from actual access denied errors. // see https://github.com/restic/restic/issues/5003#issuecomment-2452314191 for details if err != nil && isAccessDeniedError(err) { - sd, err = getNamedSecurityInfoLow(filePath) + sd, err = securityInfoLow(handle) } } if err != nil { if !useLowerPrivileges && isHandlePrivilegeNotHeldError(err) { // If ERROR_PRIVILEGE_NOT_HELD is encountered, fallback to backups/restores using lower non-admin privileges. lowerPrivileges.Store(true) - return getSecurityDescriptor(filePath) + return getSecurityDescriptor(handle) } else if errors.Is(err, windows.ERROR_NOT_SUPPORTED) { return nil, nil } else { @@ -141,14 +141,14 @@ func setSecurityDescriptor(filePath string, securityDescriptor *[]byte) error { return nil } -// getNamedSecurityInfoHigh gets the higher level SecurityDescriptor which requires admin permissions. -func getNamedSecurityInfoHigh(filePath string) (*windows.SECURITY_DESCRIPTOR, error) { - return windows.GetNamedSecurityInfo(fixpath(filePath), windows.SE_FILE_OBJECT, highSecurityFlags) +// securityInfoHigh gets the higher level SecurityDescriptor which requires admin permissions. +func securityInfoHigh(handle windows.Handle) (*windows.SECURITY_DESCRIPTOR, error) { + return windows.GetSecurityInfo(handle, windows.SE_FILE_OBJECT, highSecurityFlags) } -// getNamedSecurityInfoLow gets the lower level SecurityDescriptor which requires no admin permissions. -func getNamedSecurityInfoLow(filePath string) (*windows.SECURITY_DESCRIPTOR, error) { - return windows.GetNamedSecurityInfo(fixpath(filePath), windows.SE_FILE_OBJECT, lowBackupSecurityFlags) +// securityInfoLow gets the lower level SecurityDescriptor which requires no admin permissions. +func securityInfoLow(handle windows.Handle) (*windows.SECURITY_DESCRIPTOR, error) { + return windows.GetSecurityInfo(handle, windows.SE_FILE_OBJECT, lowBackupSecurityFlags) } // setNamedSecurityInfoHigh sets the higher level SecurityDescriptor which requires admin permissions. diff --git a/internal/fs/sd_windows_test.go b/internal/fs/sd_windows_test.go index c31b19b8b..76ff30823 100644 --- a/internal/fs/sd_windows_test.go +++ b/internal/fs/sd_windows_test.go @@ -11,6 +11,7 @@ import ( "github.com/restic/restic/internal/errors" "github.com/restic/restic/internal/test" + "golang.org/x/sys/windows" ) func TestSetGetFileSecurityDescriptors(t *testing.T) { @@ -51,9 +52,12 @@ func testSecurityDescriptors(t *testing.T, testSDs []string, testPath string) { err = setSecurityDescriptor(testPath, &sdInputBytes) test.OK(t, errors.Wrapf(err, "Error setting file security descriptor for: %s", testPath)) + handle, err := openMetadataHandle(testPath, 0) + test.OK(t, err) var sdOutputBytes *[]byte - sdOutputBytes, err = getSecurityDescriptor(testPath) + sdOutputBytes, err = getSecurityDescriptor(windows.Handle(handle.Fd())) test.OK(t, errors.Wrapf(err, "Error getting file security descriptor for: %s", testPath)) + test.OK(t, handle.Close()) compareSecurityDescriptors(t, testPath, sdInputBytes, *sdOutputBytes) }