From d422c87e44593f11390fb0a5341719cacf2ee27c Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Fri, 14 Oct 2022 16:39:11 +0200 Subject: [PATCH 1/4] sys: compile volume-path regex once, and update GoDoc Ideally, we would construct this lazily, but adding a function and a sync.Once felt like a bit "too much". Also updated the GoDoc for some functions to better describe what they do. Signed-off-by: Sebastiaan van Stijn --- sys/filesys_windows.go | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/sys/filesys_windows.go b/sys/filesys_windows.go index 12407301d..019831958 100644 --- a/sys/filesys_windows.go +++ b/sys/filesys_windows.go @@ -25,19 +25,22 @@ import ( "golang.org/x/sys/windows" ) -const ( - // SddlAdministratorsLocalSystem is local administrators plus NT AUTHORITY\System - SddlAdministratorsLocalSystem = "D:P(A;OICI;GA;;;BA)(A;OICI;GA;;;SY)" -) +// SddlAdministratorsLocalSystem is local administrators plus NT AUTHORITY\System. +const SddlAdministratorsLocalSystem = "D:P(A;OICI;GA;;;BA)(A;OICI;GA;;;SY)" -// MkdirAllWithACL is a wrapper for MkdirAll that creates a directory -// ACL'd for Builtin Administrators and Local System. -func MkdirAllWithACL(path string, perm os.FileMode) error { +// volumePath is a regular expression to check if a path is a Windows +// volume path (e.g., "\\?\Volume{4c1b02c1-d990-11dc-99ae-806e6f6e6963}". +var volumePath = regexp.MustCompile(`^\\\\\?\\Volume{[a-z0-9-]+}$`) + +// MkdirAllWithACL is a custom version of os.MkdirAll modified for use on Windows +// so that it is both volume path aware, and to create a directory +// an appropriate SDDL defined ACL for Builtin Administrators and Local System. +func MkdirAllWithACL(path string, _ os.FileMode) error { return mkdirall(path, true) } -// MkdirAll implementation that is volume path aware for Windows. It can be used -// as a drop-in replacement for os.MkdirAll() +// MkdirAll is a custom version of os.MkdirAll that is volume path aware for +// Windows. It can be used as a drop-in replacement for os.MkdirAll. func MkdirAll(path string, _ os.FileMode) error { return mkdirall(path, false) } @@ -46,7 +49,7 @@ func MkdirAll(path string, _ os.FileMode) error { // so that it is both volume path aware, and can create a directory with // a DACL. func mkdirall(path string, adminAndLocalSystem bool) error { - if re := regexp.MustCompile(`^\\\\\?\\Volume{[a-z0-9-]+}$`); re.MatchString(path) { + if volumePath.MatchString(path) { return nil } From a983599e2bc8056baa28e27aab4d7ff4165b77e2 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Fri, 14 Oct 2022 16:44:35 +0200 Subject: [PATCH 2/4] sys: update volumePath regex to allow returning earlier The regex only matched volume paths without a trailing path-separator. In cases where a path would be passed with a trailing path-separator, it would depend on further code in mkdirall to strip the trailing slash, then to perform the regex again in the next iteration. While regexes aren't ideal, we're already executing this one, so we may as well use it to match those situations as well (instead of executing it twice), to allow us to return early. Signed-off-by: Sebastiaan van Stijn --- sys/filesys_windows.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/sys/filesys_windows.go b/sys/filesys_windows.go index 019831958..47b74db2a 100644 --- a/sys/filesys_windows.go +++ b/sys/filesys_windows.go @@ -29,8 +29,9 @@ import ( const SddlAdministratorsLocalSystem = "D:P(A;OICI;GA;;;BA)(A;OICI;GA;;;SY)" // volumePath is a regular expression to check if a path is a Windows -// volume path (e.g., "\\?\Volume{4c1b02c1-d990-11dc-99ae-806e6f6e6963}". -var volumePath = regexp.MustCompile(`^\\\\\?\\Volume{[a-z0-9-]+}$`) +// volume path (e.g., "\\?\Volume{4c1b02c1-d990-11dc-99ae-806e6f6e6963}" +// or "\\?\Volume{4c1b02c1-d990-11dc-99ae-806e6f6e6963}\"). +var volumePath = regexp.MustCompile(`^\\\\\?\\Volume{[a-z0-9-]+}\\?$`) // MkdirAllWithACL is a custom version of os.MkdirAll modified for use on Windows // so that it is both volume path aware, and to create a directory From 063c5f9804d299a68bae210e351b2be4cd01c640 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Fri, 14 Oct 2022 16:51:00 +0200 Subject: [PATCH 3/4] sys: create SecurityAttribute only once (Windows) The same attribute was generated for each path that was created, but always the same, so instead of generating it in each iteration, generate it once, and pass it to our mkdirall() implementation. Signed-off-by: Sebastiaan van Stijn --- sys/filesys_windows.go | 48 ++++++++++++++++++++++++------------------ 1 file changed, 27 insertions(+), 21 deletions(-) diff --git a/sys/filesys_windows.go b/sys/filesys_windows.go index 47b74db2a..769afb572 100644 --- a/sys/filesys_windows.go +++ b/sys/filesys_windows.go @@ -37,19 +37,23 @@ var volumePath = regexp.MustCompile(`^\\\\\?\\Volume{[a-z0-9-]+}\\?$`) // so that it is both volume path aware, and to create a directory // an appropriate SDDL defined ACL for Builtin Administrators and Local System. func MkdirAllWithACL(path string, _ os.FileMode) error { - return mkdirall(path, true) + sa, err := makeSecurityAttributes(SddlAdministratorsLocalSystem) + if err != nil { + return &os.PathError{Op: "mkdirall", Path: path, Err: err} + } + return mkdirall(path, sa) } // MkdirAll is a custom version of os.MkdirAll that is volume path aware for // Windows. It can be used as a drop-in replacement for os.MkdirAll. func MkdirAll(path string, _ os.FileMode) error { - return mkdirall(path, false) + return mkdirall(path, nil) } // mkdirall is a custom version of os.MkdirAll modified for use on Windows // so that it is both volume path aware, and can create a directory with // a DACL. -func mkdirall(path string, adminAndLocalSystem bool) error { +func mkdirall(path string, perm *windows.SecurityAttributes) error { if volumePath.MatchString(path) { return nil } @@ -83,19 +87,14 @@ func mkdirall(path string, adminAndLocalSystem bool) error { if j > 1 { // Create parent - err = mkdirall(path[0:j-1], adminAndLocalSystem) + err = mkdirall(path[0:j-1], perm) if err != nil { return err } } // Parent now exists; invoke os.Mkdir or mkdirWithACL and use its result. - if adminAndLocalSystem { - err = mkdirWithACL(path) - } else { - err = os.Mkdir(path, 0) - } - + err = mkdirWithACL(path, perm) if err != nil { // Handle arguments like "foo/." by // double-checking that directory doesn't exist. @@ -115,24 +114,31 @@ func mkdirall(path string, adminAndLocalSystem bool) error { // in golang to cater for creating a directory am ACL permitting full // access, with inheritance, to any subfolder/file for Built-in Administrators // and Local System. -func mkdirWithACL(name string) error { - sa := windows.SecurityAttributes{Length: 0} - sd, err := windows.SecurityDescriptorFromString(SddlAdministratorsLocalSystem) - if err != nil { - return &os.PathError{Op: "mkdir", Path: name, Err: err} +func mkdirWithACL(name string, sa *windows.SecurityAttributes) error { + if sa == nil { + return os.Mkdir(name, 0) } - sa.Length = uint32(unsafe.Sizeof(sa)) - sa.InheritHandle = 1 - sa.SecurityDescriptor = sd namep, err := windows.UTF16PtrFromString(name) if err != nil { return &os.PathError{Op: "mkdir", Path: name, Err: err} } - e := windows.CreateDirectory(namep, &sa) - if e != nil { - return &os.PathError{Op: "mkdir", Path: name, Err: e} + err = windows.CreateDirectory(namep, sa) + if err != nil { + return &os.PathError{Op: "mkdir", Path: name, Err: err} } return nil } + +func makeSecurityAttributes(sddl string) (*windows.SecurityAttributes, error) { + var sa windows.SecurityAttributes + sa.Length = uint32(unsafe.Sizeof(sa)) + sa.InheritHandle = 1 + var err error + sa.SecurityDescriptor, err = windows.SecurityDescriptorFromString(sddl) + if err != nil { + return nil, err + } + return &sa, nil +} From 972399538dbfa56f5eb8f3266580b0acb0fc4646 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Fri, 14 Oct 2022 16:56:31 +0200 Subject: [PATCH 4/4] sys: synchronize mkdirall() with latest os.MkDirAll() Signed-off-by: Sebastiaan van Stijn --- sys/filesys_windows.go | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/sys/filesys_windows.go b/sys/filesys_windows.go index 769afb572..67fc4048c 100644 --- a/sys/filesys_windows.go +++ b/sys/filesys_windows.go @@ -67,11 +67,7 @@ func mkdirall(path string, perm *windows.SecurityAttributes) error { if dir.IsDir() { return nil } - return &os.PathError{ - Op: "mkdir", - Path: path, - Err: syscall.ENOTDIR, - } + return &os.PathError{Op: "mkdir", Path: path, Err: syscall.ENOTDIR} } // Slow path: make sure parent exists and then call Mkdir for path. @@ -86,14 +82,14 @@ func mkdirall(path string, perm *windows.SecurityAttributes) error { } if j > 1 { - // Create parent - err = mkdirall(path[0:j-1], perm) + // Create parent. + err = mkdirall(fixRootDirectory(path[:j-1]), perm) if err != nil { return err } } - // Parent now exists; invoke os.Mkdir or mkdirWithACL and use its result. + // Parent now exists; invoke Mkdir and use its result. err = mkdirWithACL(path, perm) if err != nil { // Handle arguments like "foo/." by @@ -131,6 +127,17 @@ func mkdirWithACL(name string, sa *windows.SecurityAttributes) error { return nil } +// fixRootDirectory fixes a reference to a drive's root directory to +// have the required trailing slash. +func fixRootDirectory(p string) string { + if len(p) == len(`\\?\c:`) { + if os.IsPathSeparator(p[0]) && os.IsPathSeparator(p[1]) && p[2] == '?' && os.IsPathSeparator(p[3]) && p[5] == ':' { + return p + `\` + } + } + return p +} + func makeSecurityAttributes(sddl string) (*windows.SecurityAttributes, error) { var sa windows.SecurityAttributes sa.Length = uint32(unsafe.Sizeof(sa))