From 56113a8da7000d3264f014cd33bb1bceaff225e4 Mon Sep 17 00:00:00 2001 From: aneesh-n <99904+aneesh-n@users.noreply.github.com> Date: Sat, 3 Aug 2024 16:03:30 -0600 Subject: [PATCH 1/9] Skip EA processing for volumes that do not support EA --- internal/fs/ea_windows.go | 40 +++++++++++++++++++++++++++++++++ internal/restic/node_windows.go | 20 +++++++++++++++-- 2 files changed, 58 insertions(+), 2 deletions(-) diff --git a/internal/fs/ea_windows.go b/internal/fs/ea_windows.go index 08466c33f..e69f595a8 100644 --- a/internal/fs/ea_windows.go +++ b/internal/fs/ea_windows.go @@ -53,6 +53,15 @@ var ( errInvalidEaBuffer = errors.New("invalid extended attribute buffer") errEaNameTooLarge = errors.New("extended attribute name too large") errEaValueTooLarge = errors.New("extended attribute value too large") + + kernel32dll = syscall.NewLazyDLL("kernel32.dll") + + procGetVolumeInformationW = kernel32dll.NewProc("GetVolumeInformationW") +) + +const ( + // fileSupportsExtendedAttributes is a bitmask that indicates whether the file system supports extended attributes. + fileSupportsExtendedAttributes = 0x00000004 ) // ExtendedAttribute represents a single Windows EA. @@ -283,3 +292,34 @@ func setFileEA(handle windows.Handle, iosb *ioStatusBlock, buf *uint8, bufLen ui status = ntStatus(r0) return } + +// PathSupportsExtendedAttributes returns true if the path supports extended attributes. +func PathSupportsExtendedAttributes(path string) (bool, error) { + var ( + volumeName [syscall.MAX_PATH + 1]uint16 + fsName [syscall.MAX_PATH + 1]uint16 + volumeSerial uint32 + maxComponentLen uint32 + fileSystemFlags uint32 + ) + utf16Path, err := windows.UTF16PtrFromString(path) + if err != nil { + return false, err + } + ret, _, err := procGetVolumeInformationW.Call( + uintptr(unsafe.Pointer(utf16Path)), + uintptr(unsafe.Pointer(&volumeName[0])), + uintptr(len(volumeName)), + uintptr(unsafe.Pointer(&volumeSerial)), + uintptr(unsafe.Pointer(&maxComponentLen)), + uintptr(unsafe.Pointer(&fileSystemFlags)), + uintptr(unsafe.Pointer(&fsName[0])), + uintptr(len(fsName)), + ) + if ret == 0 { + return false, err + } + + supportsEAs := (fileSystemFlags & fileSupportsExtendedAttributes) != 0 + return supportsEAs, nil +} diff --git a/internal/restic/node_windows.go b/internal/restic/node_windows.go index 2785e0412..ea167b96d 100644 --- a/internal/restic/node_windows.go +++ b/internal/restic/node_windows.go @@ -32,6 +32,9 @@ var ( modAdvapi32 = syscall.NewLazyDLL("advapi32.dll") procEncryptFile = modAdvapi32.NewProc("EncryptFileW") procDecryptFile = modAdvapi32.NewProc("DecryptFileW") + + // eaUnsupportedVolumesMap is a map of volumes that do not support extended attributes. + eaUnsupportedVolumesMap = map[string]bool{} ) // mknod is not supported on Windows. @@ -358,7 +361,20 @@ func (node *Node) fillGenericAttributes(path string, fi os.FileInfo, stat *statT // Also do not allow processing of extended attributes for ADS. return false, nil } - if !strings.HasSuffix(filepath.Clean(path), `\`) { + if strings.HasSuffix(filepath.Clean(path), `\`) { + // This path is a volume + supportsEAs, err := fs.PathSupportsExtendedAttributes(path) + if err != nil { + return false, err + } + if supportsEAs { + delete(eaUnsupportedVolumesMap, filepath.VolumeName(path)) + } else { + // Add the volume to the map of volumes that do not support extended attributes. + eaUnsupportedVolumesMap[filepath.VolumeName(path)] = true + } + return supportsEAs, nil + } else { // Do not process file attributes and created time for windows directories like // C:, D: // Filepath.Clean(path) ends with '\' for Windows root drives only. @@ -375,8 +391,8 @@ func (node *Node) fillGenericAttributes(path string, fi os.FileInfo, stat *statT FileAttributes: &stat.FileAttributes, SecurityDescriptor: sd, }) + return !eaUnsupportedVolumesMap[filepath.VolumeName(path)], err } - return true, err } // windowsAttrsToGenericAttributes converts the WindowsAttributes to a generic attributes map using reflection From 041c0705e43003be2c0d100ca98379d0126b3935 Mon Sep 17 00:00:00 2001 From: aneesh-n <99904+aneesh-n@users.noreply.github.com> Date: Sat, 3 Aug 2024 16:19:59 -0600 Subject: [PATCH 2/9] Add changelog --- changelog/unreleased/pull-4980 | 10 ++++++++++ 1 file changed, 10 insertions(+) create mode 100644 changelog/unreleased/pull-4980 diff --git a/changelog/unreleased/pull-4980 b/changelog/unreleased/pull-4980 new file mode 100644 index 000000000..afa8a7406 --- /dev/null +++ b/changelog/unreleased/pull-4980 @@ -0,0 +1,10 @@ +Bugfix: Skip EA processing in Windows for volumes that do not support EA + +Restic was failing to backup files on some windows paths like network +drives because of errors while fetching ExtendedAttributes. +Either they return error codes like windows.E_NOT_SET or +windows.ERROR_INVALID_FUNCTION or it results in slower backups. +Restic now completely skips the attempt to fetch Extended Attributes +for such volumes where it is not supported. + +https://github.com/restic/restic/pull/4980 From 8c8a066c0e0988d840d5a0fb9f4d383615791ec0 Mon Sep 17 00:00:00 2001 From: aneesh-n <99904+aneesh-n@users.noreply.github.com> Date: Sat, 3 Aug 2024 18:06:47 -0600 Subject: [PATCH 3/9] Correct the bitmask for fileSupportsExtendedAttributes and add link --- internal/fs/ea_windows.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/internal/fs/ea_windows.go b/internal/fs/ea_windows.go index e69f595a8..b5d1e3cc8 100644 --- a/internal/fs/ea_windows.go +++ b/internal/fs/ea_windows.go @@ -61,7 +61,8 @@ var ( const ( // fileSupportsExtendedAttributes is a bitmask that indicates whether the file system supports extended attributes. - fileSupportsExtendedAttributes = 0x00000004 + // https://learn.microsoft.com/en-us/windows-hardware/drivers/ddi/ntifs/ns-ntifs-_file_fs_attribute_information + fileSupportsExtendedAttributes = 0x00800000 ) // ExtendedAttribute represents a single Windows EA. From 9dedba6dfc91eec3edfa569a775b161a4981d5e6 Mon Sep 17 00:00:00 2001 From: aneesh-n <99904+aneesh-n@users.noreply.github.com> Date: Sun, 4 Aug 2024 10:23:39 -0600 Subject: [PATCH 4/9] Address review comments --- internal/fs/ea_windows.go | 38 +++------------ internal/restic/node_windows.go | 84 +++++++++++++++++++-------------- 2 files changed, 55 insertions(+), 67 deletions(-) diff --git a/internal/fs/ea_windows.go b/internal/fs/ea_windows.go index b5d1e3cc8..d19a1ee6a 100644 --- a/internal/fs/ea_windows.go +++ b/internal/fs/ea_windows.go @@ -53,16 +53,6 @@ var ( errInvalidEaBuffer = errors.New("invalid extended attribute buffer") errEaNameTooLarge = errors.New("extended attribute name too large") errEaValueTooLarge = errors.New("extended attribute value too large") - - kernel32dll = syscall.NewLazyDLL("kernel32.dll") - - procGetVolumeInformationW = kernel32dll.NewProc("GetVolumeInformationW") -) - -const ( - // fileSupportsExtendedAttributes is a bitmask that indicates whether the file system supports extended attributes. - // https://learn.microsoft.com/en-us/windows-hardware/drivers/ddi/ntifs/ns-ntifs-_file_fs_attribute_information - fileSupportsExtendedAttributes = 0x00800000 ) // ExtendedAttribute represents a single Windows EA. @@ -295,32 +285,16 @@ func setFileEA(handle windows.Handle, iosb *ioStatusBlock, buf *uint8, bufLen ui } // PathSupportsExtendedAttributes returns true if the path supports extended attributes. -func PathSupportsExtendedAttributes(path string) (bool, error) { - var ( - volumeName [syscall.MAX_PATH + 1]uint16 - fsName [syscall.MAX_PATH + 1]uint16 - volumeSerial uint32 - maxComponentLen uint32 - fileSystemFlags uint32 - ) +func PathSupportsExtendedAttributes(path string) (supported bool, err error) { + var fileSystemFlags uint32 utf16Path, err := windows.UTF16PtrFromString(path) if err != nil { return false, err } - ret, _, err := procGetVolumeInformationW.Call( - uintptr(unsafe.Pointer(utf16Path)), - uintptr(unsafe.Pointer(&volumeName[0])), - uintptr(len(volumeName)), - uintptr(unsafe.Pointer(&volumeSerial)), - uintptr(unsafe.Pointer(&maxComponentLen)), - uintptr(unsafe.Pointer(&fileSystemFlags)), - uintptr(unsafe.Pointer(&fsName[0])), - uintptr(len(fsName)), - ) - if ret == 0 { + err = windows.GetVolumeInformation(utf16Path, nil, 0, nil, nil, &fileSystemFlags, nil, 0) + if err != nil { return false, err } - - supportsEAs := (fileSystemFlags & fileSupportsExtendedAttributes) != 0 - return supportsEAs, nil + supported = (fileSystemFlags & windows.FILE_SUPPORTS_EXTENDED_ATTRIBUTES) != 0 + return supported, nil } diff --git a/internal/restic/node_windows.go b/internal/restic/node_windows.go index ea167b96d..0d72ecb36 100644 --- a/internal/restic/node_windows.go +++ b/internal/restic/node_windows.go @@ -8,6 +8,7 @@ import ( "reflect" "runtime" "strings" + "sync" "syscall" "unsafe" @@ -33,8 +34,8 @@ var ( procEncryptFile = modAdvapi32.NewProc("EncryptFileW") procDecryptFile = modAdvapi32.NewProc("DecryptFileW") - // eaUnsupportedVolumesMap is a map of volumes that do not support extended attributes. - eaUnsupportedVolumesMap = map[string]bool{} + // eaSupportedVolumesMap is a map of volumes to boolean values indicating if they support extended attributes. + eaSupportedVolumesMap = sync.Map{} ) // mknod is not supported on Windows. @@ -354,45 +355,58 @@ func decryptFile(pathPointer *uint16) error { } // fillGenericAttributes fills in the generic attributes for windows like File Attributes, -// Created time etc. +// Created time and Security Descriptors. +// It also checks if the volume supports extended attributes and stores the result in a map +// so that it does not have to be checked again for subsequent calls for paths in the same volume. func (node *Node) fillGenericAttributes(path string, fi os.FileInfo, stat *statT) (allowExtended bool, err error) { if strings.Contains(filepath.Base(path), ":") { - //Do not process for Alternate Data Streams in Windows + // Do not process for Alternate Data Streams in Windows // Also do not allow processing of extended attributes for ADS. return false, nil } - if strings.HasSuffix(filepath.Clean(path), `\`) { - // This path is a volume - supportsEAs, err := fs.PathSupportsExtendedAttributes(path) - if err != nil { - return false, err - } - if supportsEAs { - delete(eaUnsupportedVolumesMap, filepath.VolumeName(path)) - } else { - // Add the volume to the map of volumes that do not support extended attributes. - eaUnsupportedVolumesMap[filepath.VolumeName(path)] = true - } - return supportsEAs, nil - } else { - // Do not process file attributes and created time for windows directories like - // C:, D: - // Filepath.Clean(path) ends with '\' for Windows root drives only. - var sd *[]byte - if node.Type == "file" || node.Type == "dir" { - if sd, err = fs.GetSecurityDescriptor(path); err != nil { - return true, err - } - } - - // Add Windows attributes - node.GenericAttributes, err = WindowsAttrsToGenericAttributes(WindowsAttributes{ - CreationTime: getCreationTime(fi, path), - FileAttributes: &stat.FileAttributes, - SecurityDescriptor: sd, - }) - return !eaUnsupportedVolumesMap[filepath.VolumeName(path)], err + volumeName := filepath.VolumeName(path) + allowExtended, err = checkAndStoreEASupport(volumeName) + if err != nil { + return false, err } + if strings.HasSuffix(filepath.Clean(path), `\`) { + // filepath.Clean(path) ends with '\' for Windows root volume paths only + // Do not process file attributes, created time and sd for windows root volume paths + // Security descriptors are not supported for root volume paths. + // Though file attributes and created time are supported for root volume paths, + // we ignore them and we do not want to replace them during every restore. + return allowExtended, nil + } + + var sd *[]byte + if node.Type == "file" || node.Type == "dir" { + if sd, err = fs.GetSecurityDescriptor(path); err != nil { + return true, err + } + } + // Add Windows attributes + node.GenericAttributes, err = WindowsAttrsToGenericAttributes(WindowsAttributes{ + CreationTime: getCreationTime(fi, path), + FileAttributes: &stat.FileAttributes, + SecurityDescriptor: sd, + }) + return allowExtended, err +} + +// checkAndStoreEASupport checks if a volume supports extended attributes and stores the result in a map +// If the result is already in the map, it returns the result from the map. +func checkAndStoreEASupport(volumeName string) (isEASupportedVolume bool, err error) { + eaSupportedValue, exists := eaSupportedVolumesMap.Load(volumeName) + if exists { + return eaSupportedValue.(bool), nil + } + + // Add backslash to the volume name to ensure it is a valid path + isEASupportedVolume, err = fs.PathSupportsExtendedAttributes(volumeName + `\`) + if err == nil { + eaSupportedVolumesMap.Store(volumeName, isEASupportedVolume) + } + return isEASupportedVolume, err } // windowsAttrsToGenericAttributes converts the WindowsAttributes to a generic attributes map using reflection From 89712f66406c779e469e2060621d2a50184fdf99 Mon Sep 17 00:00:00 2001 From: aneesh-n <99904+aneesh-n@users.noreply.github.com> Date: Sun, 4 Aug 2024 10:36:13 -0600 Subject: [PATCH 5/9] Formatted --- internal/restic/node_windows.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/internal/restic/node_windows.go b/internal/restic/node_windows.go index 0d72ecb36..a0c2ebe85 100644 --- a/internal/restic/node_windows.go +++ b/internal/restic/node_windows.go @@ -364,11 +364,13 @@ func (node *Node) fillGenericAttributes(path string, fi os.FileInfo, stat *statT // Also do not allow processing of extended attributes for ADS. return false, nil } + volumeName := filepath.VolumeName(path) allowExtended, err = checkAndStoreEASupport(volumeName) if err != nil { return false, err } + if strings.HasSuffix(filepath.Clean(path), `\`) { // filepath.Clean(path) ends with '\' for Windows root volume paths only // Do not process file attributes, created time and sd for windows root volume paths From c13725b5d042e87ee59756236715a6008ecb8044 Mon Sep 17 00:00:00 2001 From: aneesh-n <99904+aneesh-n@users.noreply.github.com> Date: Sun, 4 Aug 2024 11:05:40 -0600 Subject: [PATCH 6/9] Check EA support only for volumes, files and dirs --- internal/restic/node_windows.go | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/internal/restic/node_windows.go b/internal/restic/node_windows.go index a0c2ebe85..0ecaa5a68 100644 --- a/internal/restic/node_windows.go +++ b/internal/restic/node_windows.go @@ -365,23 +365,26 @@ func (node *Node) fillGenericAttributes(path string, fi os.FileInfo, stat *statT return false, nil } - volumeName := filepath.VolumeName(path) - allowExtended, err = checkAndStoreEASupport(volumeName) - if err != nil { - return false, err - } - if strings.HasSuffix(filepath.Clean(path), `\`) { // filepath.Clean(path) ends with '\' for Windows root volume paths only // Do not process file attributes, created time and sd for windows root volume paths // Security descriptors are not supported for root volume paths. // Though file attributes and created time are supported for root volume paths, // we ignore them and we do not want to replace them during every restore. + allowExtended, err = checkAndStoreEASupport(filepath.VolumeName(path)) + if err != nil { + return false, err + } return allowExtended, nil } var sd *[]byte if node.Type == "file" || node.Type == "dir" { + // Check EA support and get security descriptor for file/dir only + allowExtended, err = checkAndStoreEASupport(filepath.VolumeName(path)) + if err != nil { + return false, err + } if sd, err = fs.GetSecurityDescriptor(path); err != nil { return true, err } From 85639f5159d3bb22174b7a249aab5dd60f114e9f Mon Sep 17 00:00:00 2001 From: aneesh-n <99904+aneesh-n@users.noreply.github.com> Date: Sun, 4 Aug 2024 13:19:13 -0600 Subject: [PATCH 7/9] Add handling for relative paths, vss paths, UNC paths --- internal/restic/node_windows.go | 22 ++++++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-) diff --git a/internal/restic/node_windows.go b/internal/restic/node_windows.go index 0ecaa5a68..27dbbf9a3 100644 --- a/internal/restic/node_windows.go +++ b/internal/restic/node_windows.go @@ -398,9 +398,27 @@ func (node *Node) fillGenericAttributes(path string, fi os.FileInfo, stat *statT return allowExtended, err } -// checkAndStoreEASupport checks if a volume supports extended attributes and stores the result in a map +// checkAndStoreEASupport checks if the volume of the path supports extended attributes and stores the result in a map // If the result is already in the map, it returns the result from the map. -func checkAndStoreEASupport(volumeName string) (isEASupportedVolume bool, err error) { +func checkAndStoreEASupport(path string) (isEASupportedVolume bool, err error) { + // Check if it's a UNC path and format it correctly + if strings.HasPrefix(path, `\\?\UNC\`) { + // Convert \\?\UNC\ path to standard path to get the volume name correctly + path = `\\` + strings.TrimPrefix(path, `\\?\UNC\`) + } else if strings.HasPrefix(path, `\\?\GLOBALROOT`) { + // EAs are not supported for \\?\GLOBALROOT i.e. VSS snapshots + return false, nil + } else { + // Use the absolute path + path, err = filepath.Abs(path) + if err != nil { + return false, fmt.Errorf("failed to get absolute path: %w", err) + } + } + volumeName := filepath.VolumeName(path) + if volumeName == "" { + return false, nil + } eaSupportedValue, exists := eaSupportedVolumesMap.Load(volumeName) if exists { return eaSupportedValue.(bool), nil From 71632a8197a4862e0787a9651995a8caaad3ce3b Mon Sep 17 00:00:00 2001 From: aneesh-n <99904+aneesh-n@users.noreply.github.com> Date: Mon, 5 Aug 2024 16:03:43 -0600 Subject: [PATCH 8/9] Handle extended length paths --- internal/restic/node_windows.go | 23 ++++++++++++++++------- 1 file changed, 16 insertions(+), 7 deletions(-) diff --git a/internal/restic/node_windows.go b/internal/restic/node_windows.go index 27dbbf9a3..34df4e9c1 100644 --- a/internal/restic/node_windows.go +++ b/internal/restic/node_windows.go @@ -38,6 +38,12 @@ var ( eaSupportedVolumesMap = sync.Map{} ) +const ( + extendedPathPrefix = `\\?\` + uncPathPrefix = `\\?\UNC\` + globalRootPrefix = `\\?\GLOBALROOT\` +) + // mknod is not supported on Windows. func mknod(_ string, _ uint32, _ uint64) (err error) { return errors.New("device nodes cannot be created on windows") @@ -371,7 +377,7 @@ func (node *Node) fillGenericAttributes(path string, fi os.FileInfo, stat *statT // Security descriptors are not supported for root volume paths. // Though file attributes and created time are supported for root volume paths, // we ignore them and we do not want to replace them during every restore. - allowExtended, err = checkAndStoreEASupport(filepath.VolumeName(path)) + allowExtended, err = checkAndStoreEASupport(path) if err != nil { return false, err } @@ -381,7 +387,7 @@ func (node *Node) fillGenericAttributes(path string, fi os.FileInfo, stat *statT var sd *[]byte if node.Type == "file" || node.Type == "dir" { // Check EA support and get security descriptor for file/dir only - allowExtended, err = checkAndStoreEASupport(filepath.VolumeName(path)) + allowExtended, err = checkAndStoreEASupport(path) if err != nil { return false, err } @@ -401,11 +407,14 @@ func (node *Node) fillGenericAttributes(path string, fi os.FileInfo, stat *statT // checkAndStoreEASupport checks if the volume of the path supports extended attributes and stores the result in a map // If the result is already in the map, it returns the result from the map. func checkAndStoreEASupport(path string) (isEASupportedVolume bool, err error) { - // Check if it's a UNC path and format it correctly - if strings.HasPrefix(path, `\\?\UNC\`) { - // Convert \\?\UNC\ path to standard path to get the volume name correctly - path = `\\` + strings.TrimPrefix(path, `\\?\UNC\`) - } else if strings.HasPrefix(path, `\\?\GLOBALROOT`) { + // Check if it's an extended length path + if strings.HasPrefix(path, uncPathPrefix) { + // Convert \\?\UNC\ extended path to standard path to get the volume name correctly + path = `\\` + path[len(uncPathPrefix):] + } else if strings.HasPrefix(path, extendedPathPrefix) { + //Extended length path prefix needs to be trimmed to get the volume name correctly + path = path[len(extendedPathPrefix):] + } else if strings.HasPrefix(path, globalRootPrefix) { // EAs are not supported for \\?\GLOBALROOT i.e. VSS snapshots return false, nil } else { From 18e9d71d7abe152324cb6d0cb364d03e3d9979d5 Mon Sep 17 00:00:00 2001 From: aneesh-n <99904+aneesh-n@users.noreply.github.com> Date: Sat, 10 Aug 2024 10:38:04 -0600 Subject: [PATCH 9/9] Fix review comments --- changelog/unreleased/pull-4980 | 6 ++++-- internal/fs/sd_windows.go | 3 +++ internal/restic/node_windows.go | 2 +- 3 files changed, 8 insertions(+), 3 deletions(-) diff --git a/changelog/unreleased/pull-4980 b/changelog/unreleased/pull-4980 index afa8a7406..264f347fa 100644 --- a/changelog/unreleased/pull-4980 +++ b/changelog/unreleased/pull-4980 @@ -1,10 +1,12 @@ Bugfix: Skip EA processing in Windows for volumes that do not support EA Restic was failing to backup files on some windows paths like network -drives because of errors while fetching ExtendedAttributes. +drives because of errors while fetching extended attributes. Either they return error codes like windows.E_NOT_SET or windows.ERROR_INVALID_FUNCTION or it results in slower backups. -Restic now completely skips the attempt to fetch Extended Attributes +Restic now completely skips the attempt to fetch extended attributes for such volumes where it is not supported. https://github.com/restic/restic/pull/4980 +https://github.com/restic/restic/issues/4955 +https://github.com/restic/restic/issues/4950 diff --git a/internal/fs/sd_windows.go b/internal/fs/sd_windows.go index 2da1c5df4..0a73cbe53 100644 --- a/internal/fs/sd_windows.go +++ b/internal/fs/sd_windows.go @@ -11,6 +11,7 @@ import ( "unsafe" "github.com/restic/restic/internal/debug" + "github.com/restic/restic/internal/errors" "golang.org/x/sys/windows" ) @@ -60,6 +61,8 @@ func GetSecurityDescriptor(filePath string) (securityDescriptor *[]byte, err err if err != nil { return nil, fmt.Errorf("get low-level named security info failed with: %w", err) } + } else if errors.Is(err, windows.ERROR_NOT_SUPPORTED) { + return nil, nil } else { return nil, fmt.Errorf("get named security info failed with: %w", err) } diff --git a/internal/restic/node_windows.go b/internal/restic/node_windows.go index 34df4e9c1..ceb304d0c 100644 --- a/internal/restic/node_windows.go +++ b/internal/restic/node_windows.go @@ -392,7 +392,7 @@ func (node *Node) fillGenericAttributes(path string, fi os.FileInfo, stat *statT return false, err } if sd, err = fs.GetSecurityDescriptor(path); err != nil { - return true, err + return allowExtended, err } } // Add Windows attributes