diff --git a/computestorage/helpers.go b/computestorage/helpers.go index a0e329edec..c3608dcec8 100644 --- a/computestorage/helpers.go +++ b/computestorage/helpers.go @@ -8,11 +8,12 @@ import ( "path/filepath" "syscall" - "github.com/Microsoft/go-winio/pkg/security" "github.com/Microsoft/go-winio/vhd" "github.com/Microsoft/hcsshim/internal/memory" "github.com/pkg/errors" "golang.org/x/sys/windows" + + "github.com/Microsoft/hcsshim/internal/security" ) const defaultVHDXBlockSizeInMB = 1 diff --git a/test/vendor/github.com/Microsoft/go-winio/pkg/security/grantvmgroupaccess.go b/internal/security/grantvmgroupaccess.go similarity index 59% rename from test/vendor/github.com/Microsoft/go-winio/pkg/security/grantvmgroupaccess.go rename to internal/security/grantvmgroupaccess.go index fca241590c..bfcc157699 100644 --- a/test/vendor/github.com/Microsoft/go-winio/pkg/security/grantvmgroupaccess.go +++ b/internal/security/grantvmgroupaccess.go @@ -1,13 +1,13 @@ +//go:build windows // +build windows package security import ( + "fmt" "os" "syscall" "unsafe" - - "github.com/pkg/errors" ) type ( @@ -20,25 +20,37 @@ type ( securityInformation uint32 trusteeForm uint32 trusteeType uint32 +) - explicitAccess struct { - accessPermissions accessMask - accessMode accessMode - inheritance inheritMode - trustee trustee - } +type explicitAccess struct { + //nolint:structcheck + accessPermissions accessMask + //nolint:structcheck + accessMode accessMode + //nolint:structcheck + inheritance inheritMode + //nolint:structcheck + trustee trustee +} - trustee struct { - multipleTrustee *trustee - multipleTrusteeOperation int32 - trusteeForm trusteeForm - trusteeType trusteeType - name uintptr - } -) +type trustee struct { + //nolint:unused,structcheck + multipleTrustee *trustee + //nolint:unused,structcheck + multipleTrusteeOperation int32 + trusteeForm trusteeForm + trusteeType trusteeType + name uintptr +} const ( - accessMaskDesiredPermission accessMask = 1 << 31 // GENERIC_READ + AccessMaskNone accessMask = 0 + AccessMaskRead accessMask = 1 << 31 // GENERIC_READ + AccessMaskWrite accessMask = 1 << 30 // GENERIC_WRITE + AccessMaskExecute accessMask = 1 << 29 // GENERIC_EXECUTE + AccessMaskAll accessMask = 1 << 28 // GENERIC_ALL + + accessMaskDesiredPermission = AccessMaskRead accessModeGrant accessMode = 1 @@ -57,6 +69,7 @@ const ( shareModeRead shareMode = 0x1 shareModeWrite shareMode = 0x2 + //nolint:stylecheck // ST1003 sidVmGroup = "S-1-5-83-0" trusteeFormIsSid trusteeForm = 0 @@ -64,15 +77,24 @@ const ( trusteeTypeWellKnownGroup trusteeType = 5 ) -// GrantVMGroupAccess sets the DACL for a specified file or directory to +// GrantVmGroupAccess sets the DACL for a specified file or directory to // include Grant ACE entries for the VM Group SID. This is a golang re- // implementation of the same function in vmcompute, just not exported in // RS5. Which kind of sucks. Sucks a lot :/ -func GrantVmGroupAccess(name string) error { +func GrantVmGroupAccess(name string) error { //nolint:stylecheck // ST1003 + return GrantVmGroupAccessWithMask(name, accessMaskDesiredPermission) +} + +// GrantVmGroupAccessWithMask sets the desired DACL for a specified file or +// directory. +func GrantVmGroupAccessWithMask(name string, access accessMask) error { //nolint:stylecheck // ST1003 + if access == 0 || access<<4 != 0 { + return fmt.Errorf("invalid access mask: 0x%08x", access) + } // Stat (to determine if `name` is a directory). s, err := os.Stat(name) if err != nil { - return errors.Wrapf(err, "%s os.Stat %s", gvmga, name) + return fmt.Errorf("%s os.Stat %s: %w", gvmga, name, err) } // Get a handle to the file/directory. Must defer Close on success. @@ -80,7 +102,9 @@ func GrantVmGroupAccess(name string) error { if err != nil { return err // Already wrapped } - defer syscall.CloseHandle(fd) + defer func() { + _ = syscall.CloseHandle(fd) + }() // Get the current DACL and Security Descriptor. Must defer LocalFree on success. ot := objectTypeFileObject @@ -88,21 +112,25 @@ func GrantVmGroupAccess(name string) error { sd := uintptr(0) origDACL := uintptr(0) if err := getSecurityInfo(fd, uint32(ot), uint32(si), nil, nil, &origDACL, nil, &sd); err != nil { - return errors.Wrapf(err, "%s GetSecurityInfo %s", gvmga, name) + return fmt.Errorf("%s GetSecurityInfo %s: %w", gvmga, name, err) } - defer syscall.LocalFree((syscall.Handle)(unsafe.Pointer(sd))) + defer func() { + _, _ = syscall.LocalFree((syscall.Handle)(unsafe.Pointer(sd))) + }() // Generate a new DACL which is the current DACL with the required ACEs added. // Must defer LocalFree on success. - newDACL, err := generateDACLWithAcesAdded(name, s.IsDir(), origDACL) + newDACL, err := generateDACLWithAcesAdded(name, s.IsDir(), access, origDACL) if err != nil { return err // Already wrapped } - defer syscall.LocalFree((syscall.Handle)(unsafe.Pointer(newDACL))) + defer func() { + _, _ = syscall.LocalFree((syscall.Handle)(unsafe.Pointer(newDACL))) + }() // And finally use SetSecurityInfo to apply the updated DACL. if err := setSecurityInfo(fd, uint32(ot), uint32(si), uintptr(0), uintptr(0), newDACL, uintptr(0)); err != nil { - return errors.Wrapf(err, "%s SetSecurityInfo %s", gvmga, name) + return fmt.Errorf("%s SetSecurityInfo %s: %w", gvmga, name, err) } return nil @@ -111,7 +139,10 @@ func GrantVmGroupAccess(name string) error { // createFile is a helper function to call [Nt]CreateFile to get a handle to // the file or directory. func createFile(name string, isDir bool) (syscall.Handle, error) { - namep := syscall.StringToUTF16(name) + namep, err := syscall.UTF16FromString(name) + if err != nil { + return 0, fmt.Errorf("syscall.UTF16FromString %s: %w", name, err) + } da := uint32(desiredAccessReadControl | desiredAccessWriteDac) sm := uint32(shareModeRead | shareModeWrite) fa := uint32(syscall.FILE_ATTRIBUTE_NORMAL) @@ -120,18 +151,18 @@ func createFile(name string, isDir bool) (syscall.Handle, error) { } fd, err := syscall.CreateFile(&namep[0], da, sm, nil, syscall.OPEN_EXISTING, fa, 0) if err != nil { - return 0, errors.Wrapf(err, "%s syscall.CreateFile %s", gvmga, name) + return 0, fmt.Errorf("%s syscall.CreateFile %s: %w", gvmga, name, err) } return fd, nil } // generateDACLWithAcesAdded generates a new DACL with the two needed ACEs added. // The caller is responsible for LocalFree of the returned DACL on success. -func generateDACLWithAcesAdded(name string, isDir bool, origDACL uintptr) (uintptr, error) { +func generateDACLWithAcesAdded(name string, isDir bool, desiredAccess accessMask, origDACL uintptr) (uintptr, error) { // Generate pointers to the SIDs based on the string SIDs sid, err := syscall.StringToSid(sidVmGroup) if err != nil { - return 0, errors.Wrapf(err, "%s syscall.StringToSid %s %s", gvmga, name, sidVmGroup) + return 0, fmt.Errorf("%s syscall.StringToSid %s %s: %w", gvmga, name, sidVmGroup, err) } inheritance := inheritModeNoInheritance @@ -140,8 +171,8 @@ func generateDACLWithAcesAdded(name string, isDir bool, origDACL uintptr) (uintp } eaArray := []explicitAccess{ - explicitAccess{ - accessPermissions: accessMaskDesiredPermission, + { + accessPermissions: desiredAccess, accessMode: accessModeGrant, inheritance: inheritance, trustee: trustee{ @@ -154,7 +185,7 @@ func generateDACLWithAcesAdded(name string, isDir bool, origDACL uintptr) (uintp modifiedDACL := uintptr(0) if err := setEntriesInAcl(uintptr(uint32(1)), uintptr(unsafe.Pointer(&eaArray[0])), origDACL, &modifiedDACL); err != nil { - return 0, errors.Wrapf(err, "%s SetEntriesInAcl %s", gvmga, name) + return 0, fmt.Errorf("%s SetEntriesInAcl %s: %w", gvmga, name, err) } return modifiedDACL, nil diff --git a/internal/security/grantvmgroupaccess_test.go b/internal/security/grantvmgroupaccess_test.go new file mode 100644 index 0000000000..469fdf076f --- /dev/null +++ b/internal/security/grantvmgroupaccess_test.go @@ -0,0 +1,288 @@ +//go:build windows +// +build windows + +package security + +import ( + "fmt" + "os" + "os/exec" + "path/filepath" + "regexp" + "testing" +) + +const ( + vmAccountName = `NT VIRTUAL MACHINE\\Virtual Machines` + vmAccountSID = "S-1-5-83-0" +) + +// TestGrantVmGroupAccess verifies for the three case of a file, a directory, +// and a file in a directory that the appropriate ACEs are set, including +// inheritance in the second two examples. These are the expected ACES. Is +// verified by running icacls and comparing output. +// +// File: +// S-1-15-3-1024-2268835264-3721307629-241982045-173645152-1490879176-104643441-2915960892-1612460704:(R,W) +// S-1-5-83-1-3166535780-1122986932-343720105-43916321:(R,W) +// +// Directory: +// S-1-15-3-1024-2268835264-3721307629-241982045-173645152-1490879176-104643441-2915960892-1612460704:(OI)(CI)(R,W) +// S-1-5-83-1-3166535780-1122986932-343720105-43916321:(OI)(CI)(R,W) +// +// File in directory (inherited): +// S-1-15-3-1024-2268835264-3721307629-241982045-173645152-1490879176-104643441-2915960892-1612460704:(I)(R,W) +// S-1-5-83-1-3166535780-1122986932-343720105-43916321:(I)(R,W) + +func TestGrantVmGroupAccessDefault(t *testing.T) { + f1Path := filepath.Join(t.TempDir(), "gvmgafile") + f, err := os.Create(f1Path) + if err != nil { + t.Fatal(err) + } + defer func() { + _ = f.Close() + _ = os.Remove(f1Path) + }() + + dir2 := t.TempDir() + f2Path := filepath.Join(dir2, "find.txt") + find, err := os.Create(f2Path) + if err != nil { + t.Fatal(err) + } + defer func() { + _ = find.Close() + _ = os.Remove(f2Path) + }() + + if err := GrantVmGroupAccess(f1Path); err != nil { + t.Fatal(err) + } + + if err := GrantVmGroupAccess(dir2); err != nil { + t.Fatal(err) + } + + verifyVMAccountDACLs(t, + f1Path, + []string{`(R)`}, + ) + + // Two items here: + // - One explicit read only. + // - Other applies to this folder, subfolders and files + // (OI): object inherit + // (CI): container inherit + // (IO): inherit only + // (GR): generic read + // + // In properties for the directory, advanced security settings, this will + // show as a single line "Allow/Virtual Machines/Read/Inherited from none/This folder, subfolder and files + verifyVMAccountDACLs(t, + dir2, + []string{`(R)`, `(OI)(CI)(IO)(GR)`}, + ) + + verifyVMAccountDACLs(t, + f2Path, + []string{`(I)(R)`}, + ) + +} + +func TestGrantVMGroupAccess_File_DesiredPermissions(t *testing.T) { + type config struct { + name string + desiredAccess accessMask + expectedPermissions []string + } + + for _, cfg := range []config{ + { + name: "Read", + desiredAccess: AccessMaskRead, + expectedPermissions: []string{`(R)`}, + }, + { + name: "Write", + desiredAccess: AccessMaskWrite, + expectedPermissions: []string{`(W,Rc)`}, + }, + { + name: "Execute", + desiredAccess: AccessMaskExecute, + expectedPermissions: []string{`(Rc,S,X,RA)`}, + }, + { + name: "ReadWrite", + desiredAccess: AccessMaskRead | AccessMaskWrite, + expectedPermissions: []string{`(R,W)`}, + }, + { + name: "ReadExecute", + desiredAccess: AccessMaskRead | AccessMaskExecute, + expectedPermissions: []string{`(RX)`}, + }, + { + name: "WriteExecute", + desiredAccess: AccessMaskWrite | AccessMaskExecute, + expectedPermissions: []string{`(W,Rc,X,RA)`}, + }, + { + name: "ReadWriteExecute", + desiredAccess: AccessMaskRead | AccessMaskWrite | AccessMaskExecute, + expectedPermissions: []string{`(RX,W)`}, + }, + { + name: "All", + desiredAccess: AccessMaskAll, + expectedPermissions: []string{`(F)`}, + }, + } { + t.Run(cfg.name, func(t *testing.T) { + dir := t.TempDir() + fd, err := os.Create(filepath.Join(dir, "test.txt")) + if err != nil { + t.Fatalf("failed to create temporary file: %s", err) + } + defer func() { + _ = fd.Close() + _ = os.Remove(fd.Name()) + }() + + if err := GrantVmGroupAccessWithMask(fd.Name(), cfg.desiredAccess); err != nil { + t.Fatal(err) + } + verifyVMAccountDACLs(t, fd.Name(), cfg.expectedPermissions) + }) + } +} + +func TestGrantVMGroupAccess_Directory_Permissions(t *testing.T) { + type config struct { + name string + access accessMask + filePermissions []string + dirPermissions []string + } + + for _, cfg := range []config{ + { + name: "Read", + access: AccessMaskRead, + filePermissions: []string{`(I)(R)`}, + dirPermissions: []string{`(R)`, `(OI)(CI)(IO)(GR)`}, + }, + { + name: "Write", + access: AccessMaskWrite, + filePermissions: []string{`(I)(W,Rc)`}, + dirPermissions: []string{`(W,Rc)`, `(OI)(CI)(IO)(GW)`}, + }, + { + name: "Execute", + access: AccessMaskExecute, + filePermissions: []string{`(I)(Rc,S,X,RA)`}, + dirPermissions: []string{`(Rc,S,X,RA)`, `(OI)(CI)(IO)(GE)`}, + }, + { + name: "ReadWrite", + access: AccessMaskRead | AccessMaskWrite, + filePermissions: []string{`(I)(R,W)`}, + dirPermissions: []string{`(R,W)`, `(OI)(CI)(IO)(GR,GW)`}, + }, + { + name: "ReadExecute", + access: AccessMaskRead | AccessMaskExecute, + filePermissions: []string{`(I)(RX)`}, + dirPermissions: []string{`(RX)`, `(OI)(CI)(IO)(GR,GE)`}, + }, + { + name: "WriteExecute", + access: AccessMaskWrite | AccessMaskExecute, + filePermissions: []string{`(I)(W,Rc,X,RA)`}, + dirPermissions: []string{`(W,Rc,X,RA)`, `(OI)(CI)(IO)(GW,GE)`}, + }, + { + name: "ReadWriteExecute", + access: AccessMaskRead | AccessMaskWrite | AccessMaskExecute, + filePermissions: []string{`(I)(RX,W)`}, + dirPermissions: []string{`(RX,W)`, `(OI)(CI)(IO)(GR,GW,GE)`}, + }, + { + name: "All", + access: AccessMaskAll, + filePermissions: []string{`(I)(F)`}, + dirPermissions: []string{`(F)`, `(OI)(CI)(IO)(F)`}, + }} { + t.Run(cfg.name, func(t *testing.T) { + dir := t.TempDir() + fd, err := os.Create(filepath.Join(dir, "test.txt")) + if err != nil { + t.Fatalf("failed to create temporary file: %s", err) + } + defer func() { + _ = fd.Close() + _ = os.Remove(fd.Name()) + }() + + if err := GrantVmGroupAccessWithMask(dir, cfg.access); err != nil { + t.Fatal(err) + } + verifyVMAccountDACLs(t, dir, cfg.dirPermissions) + verifyVMAccountDACLs(t, fd.Name(), cfg.filePermissions) + }) + } +} + +func TestGrantVmGroupAccess_Invalid_AccessMask(t *testing.T) { + for _, access := range []accessMask{ + 0, // no bits set + 1, // invalid bit set + 0x02000001, // invalid extra bit set + } { + t.Run(fmt.Sprintf("AccessMask_0x%x", access), func(t *testing.T) { + dir := t.TempDir() + fd, err := os.Create(filepath.Join(dir, "test.txt")) + if err != nil { + t.Fatalf("failed to create temporary file: %s", err) + } + defer func() { + _ = fd.Close() + _ = os.Remove(fd.Name()) + }() + + if err := GrantVmGroupAccessWithMask(fd.Name(), access); err == nil { + t.Fatalf("expected an error for mask: %x", access) + } + }) + } +} + +func verifyVMAccountDACLs(t *testing.T, name string, permissions []string) { + cmd := exec.Command("icacls", name) + outb, err := cmd.CombinedOutput() + if err != nil { + t.Fatal(err) + } + out := string(outb) + + for _, p := range permissions { + // Avoid '(' and ')' being part of match groups + p = regexp.QuoteMeta(p) + + nameToCheck := vmAccountName + ":" + p + sidToCheck := vmAccountSID + ":" + p + + rxName := regexp.MustCompile(nameToCheck) + rxSID := regexp.MustCompile(sidToCheck) + + matchesName := rxName.FindAllStringIndex(out, -1) + matchesSID := rxSID.FindAllStringIndex(out, -1) + + if len(matchesName) != 1 && len(matchesSID) != 1 { + t.Fatalf("expected one match for %s or %s\n%s\n", nameToCheck, sidToCheck, out) + } + } +} diff --git a/internal/security/syscall_windows.go b/internal/security/syscall_windows.go new file mode 100644 index 0000000000..f0cdd7d20c --- /dev/null +++ b/internal/security/syscall_windows.go @@ -0,0 +1,7 @@ +package security + +//go:generate go run $GOPATH/src/golang.org/x/sys/windows/mkwinsyscall/mkwinsyscall.go -output zsyscall_windows.go syscall_windows.go + +//sys getSecurityInfo(handle syscall.Handle, objectType uint32, si uint32, ppsidOwner **uintptr, ppsidGroup **uintptr, ppDacl *uintptr, ppSacl *uintptr, ppSecurityDescriptor *uintptr) (win32err error) = advapi32.GetSecurityInfo +//sys setSecurityInfo(handle syscall.Handle, objectType uint32, si uint32, psidOwner uintptr, psidGroup uintptr, pDacl uintptr, pSacl uintptr) (win32err error) = advapi32.SetSecurityInfo +//sys setEntriesInAcl(count uintptr, pListOfEEs uintptr, oldAcl uintptr, newAcl *uintptr) (win32err error) = advapi32.SetEntriesInAclW diff --git a/vendor/github.com/Microsoft/go-winio/pkg/security/zsyscall_windows.go b/internal/security/zsyscall_windows.go similarity index 59% rename from vendor/github.com/Microsoft/go-winio/pkg/security/zsyscall_windows.go rename to internal/security/zsyscall_windows.go index 4a90cb3cc8..4084680e0f 100644 --- a/vendor/github.com/Microsoft/go-winio/pkg/security/zsyscall_windows.go +++ b/internal/security/zsyscall_windows.go @@ -45,26 +45,26 @@ var ( procSetSecurityInfo = modadvapi32.NewProc("SetSecurityInfo") ) -func getSecurityInfo(handle syscall.Handle, objectType uint32, si uint32, ppsidOwner **uintptr, ppsidGroup **uintptr, ppDacl *uintptr, ppSacl *uintptr, ppSecurityDescriptor *uintptr) (err error) { - r1, _, e1 := syscall.Syscall9(procGetSecurityInfo.Addr(), 8, uintptr(handle), uintptr(objectType), uintptr(si), uintptr(unsafe.Pointer(ppsidOwner)), uintptr(unsafe.Pointer(ppsidGroup)), uintptr(unsafe.Pointer(ppDacl)), uintptr(unsafe.Pointer(ppSacl)), uintptr(unsafe.Pointer(ppSecurityDescriptor)), 0) - if r1 != 0 { - err = errnoErr(e1) +func getSecurityInfo(handle syscall.Handle, objectType uint32, si uint32, ppsidOwner **uintptr, ppsidGroup **uintptr, ppDacl *uintptr, ppSacl *uintptr, ppSecurityDescriptor *uintptr) (win32err error) { + r0, _, _ := syscall.Syscall9(procGetSecurityInfo.Addr(), 8, uintptr(handle), uintptr(objectType), uintptr(si), uintptr(unsafe.Pointer(ppsidOwner)), uintptr(unsafe.Pointer(ppsidGroup)), uintptr(unsafe.Pointer(ppDacl)), uintptr(unsafe.Pointer(ppSacl)), uintptr(unsafe.Pointer(ppSecurityDescriptor)), 0) + if r0 != 0 { + win32err = syscall.Errno(r0) } return } -func setEntriesInAcl(count uintptr, pListOfEEs uintptr, oldAcl uintptr, newAcl *uintptr) (err error) { - r1, _, e1 := syscall.Syscall6(procSetEntriesInAclW.Addr(), 4, uintptr(count), uintptr(pListOfEEs), uintptr(oldAcl), uintptr(unsafe.Pointer(newAcl)), 0, 0) - if r1 != 0 { - err = errnoErr(e1) +func setEntriesInAcl(count uintptr, pListOfEEs uintptr, oldAcl uintptr, newAcl *uintptr) (win32err error) { + r0, _, _ := syscall.Syscall6(procSetEntriesInAclW.Addr(), 4, uintptr(count), uintptr(pListOfEEs), uintptr(oldAcl), uintptr(unsafe.Pointer(newAcl)), 0, 0) + if r0 != 0 { + win32err = syscall.Errno(r0) } return } -func setSecurityInfo(handle syscall.Handle, objectType uint32, si uint32, psidOwner uintptr, psidGroup uintptr, pDacl uintptr, pSacl uintptr) (err error) { - r1, _, e1 := syscall.Syscall9(procSetSecurityInfo.Addr(), 7, uintptr(handle), uintptr(objectType), uintptr(si), uintptr(psidOwner), uintptr(psidGroup), uintptr(pDacl), uintptr(pSacl), 0, 0) - if r1 != 0 { - err = errnoErr(e1) +func setSecurityInfo(handle syscall.Handle, objectType uint32, si uint32, psidOwner uintptr, psidGroup uintptr, pDacl uintptr, pSacl uintptr) (win32err error) { + r0, _, _ := syscall.Syscall9(procSetSecurityInfo.Addr(), 7, uintptr(handle), uintptr(objectType), uintptr(si), uintptr(psidOwner), uintptr(psidGroup), uintptr(pDacl), uintptr(pSacl), 0, 0) + if r0 != 0 { + win32err = syscall.Errno(r0) } return } diff --git a/internal/tools/grantvmgroupaccess/main.go b/internal/tools/grantvmgroupaccess/main.go index 5fa6644d77..c18a8f4134 100644 --- a/internal/tools/grantvmgroupaccess/main.go +++ b/internal/tools/grantvmgroupaccess/main.go @@ -3,18 +3,55 @@ package main import ( + "flag" "fmt" "os" - "github.com/Microsoft/go-winio/pkg/security" + "github.com/Microsoft/hcsshim/internal/security" ) func main() { - if len(os.Args) != 2 { - fmt.Fprintln(os.Stderr, "Usage: grantvmgroupaccess.exe file") + readFlag := flag.Bool("read", false, "Grant GENERIC_READ permission (DEFAULT)") + writeFlag := flag.Bool("write", false, "Grant GENERIC_WRITE permission") + executeFlag := flag.Bool("execute", false, "Grant GENERIC_EXECUTE permission") + allFlag := flag.Bool("all", false, "Grant GENERIC_ALL permission") + + flag.Usage = func() { + fmt.Fprintf(os.Stderr, "\nUsage of %s:\n\n", os.Args[0]) + fmt.Fprintf(os.Stderr, " %s [flags] file\n\n", os.Args[0]) + fmt.Fprintf(os.Stderr, "Flags:\n") + flag.PrintDefaults() + fmt.Fprintf(os.Stderr, "Examples:\n") + fmt.Fprintf(os.Stderr, " %s --read myfile.txt\n", os.Args[0]) + fmt.Fprintf(os.Stderr, " %s --read --execute myfile.txt\n", os.Args[0]) + } + + flag.Parse() + + if flag.NArg() == 0 { + flag.Usage() os.Exit(-1) } - if err := security.GrantVmGroupAccess(os.Args[1]); err != nil { + + desiredAccess := security.AccessMaskNone + if flag.NFlag() == 0 { + desiredAccess = security.AccessMaskRead + } + + if *readFlag { + desiredAccess |= security.AccessMaskRead + } + if *writeFlag { + desiredAccess |= security.AccessMaskWrite + } + if *executeFlag { + desiredAccess |= security.AccessMaskExecute + } + if *allFlag { + desiredAccess |= security.AccessMaskAll + } + + if err := security.GrantVmGroupAccessWithMask(flag.Arg(0), desiredAccess); err != nil { fmt.Fprintln(os.Stderr, err) os.Exit(-1) } diff --git a/internal/uvm/scsi.go b/internal/uvm/scsi.go index cc884c3bcd..e21d0ddfe2 100644 --- a/internal/uvm/scsi.go +++ b/internal/uvm/scsi.go @@ -12,7 +12,6 @@ import ( "path/filepath" "strings" - "github.com/Microsoft/go-winio/pkg/security" "github.com/pkg/errors" "github.com/sirupsen/logrus" @@ -22,6 +21,7 @@ import ( "github.com/Microsoft/hcsshim/internal/log" "github.com/Microsoft/hcsshim/internal/protocol/guestrequest" "github.com/Microsoft/hcsshim/internal/protocol/guestresource" + "github.com/Microsoft/hcsshim/internal/security" "github.com/Microsoft/hcsshim/internal/wclayer" ) diff --git a/test/vendor/github.com/Microsoft/go-winio/pkg/security/syscall_windows.go b/test/vendor/github.com/Microsoft/go-winio/pkg/security/syscall_windows.go deleted file mode 100644 index c40c2739b7..0000000000 --- a/test/vendor/github.com/Microsoft/go-winio/pkg/security/syscall_windows.go +++ /dev/null @@ -1,7 +0,0 @@ -package security - -//go:generate go run mksyscall_windows.go -output zsyscall_windows.go syscall_windows.go - -//sys getSecurityInfo(handle syscall.Handle, objectType uint32, si uint32, ppsidOwner **uintptr, ppsidGroup **uintptr, ppDacl *uintptr, ppSacl *uintptr, ppSecurityDescriptor *uintptr) (err error) [failretval!=0] = advapi32.GetSecurityInfo -//sys setSecurityInfo(handle syscall.Handle, objectType uint32, si uint32, psidOwner uintptr, psidGroup uintptr, pDacl uintptr, pSacl uintptr) (err error) [failretval!=0] = advapi32.SetSecurityInfo -//sys setEntriesInAcl(count uintptr, pListOfEEs uintptr, oldAcl uintptr, newAcl *uintptr) (err error) [failretval!=0] = advapi32.SetEntriesInAclW diff --git a/test/vendor/github.com/Microsoft/hcsshim/computestorage/helpers.go b/test/vendor/github.com/Microsoft/hcsshim/computestorage/helpers.go index a0e329edec..c3608dcec8 100644 --- a/test/vendor/github.com/Microsoft/hcsshim/computestorage/helpers.go +++ b/test/vendor/github.com/Microsoft/hcsshim/computestorage/helpers.go @@ -8,11 +8,12 @@ import ( "path/filepath" "syscall" - "github.com/Microsoft/go-winio/pkg/security" "github.com/Microsoft/go-winio/vhd" "github.com/Microsoft/hcsshim/internal/memory" "github.com/pkg/errors" "golang.org/x/sys/windows" + + "github.com/Microsoft/hcsshim/internal/security" ) const defaultVHDXBlockSizeInMB = 1 diff --git a/vendor/github.com/Microsoft/go-winio/pkg/security/grantvmgroupaccess.go b/test/vendor/github.com/Microsoft/hcsshim/internal/security/grantvmgroupaccess.go similarity index 59% rename from vendor/github.com/Microsoft/go-winio/pkg/security/grantvmgroupaccess.go rename to test/vendor/github.com/Microsoft/hcsshim/internal/security/grantvmgroupaccess.go index fca241590c..bfcc157699 100644 --- a/vendor/github.com/Microsoft/go-winio/pkg/security/grantvmgroupaccess.go +++ b/test/vendor/github.com/Microsoft/hcsshim/internal/security/grantvmgroupaccess.go @@ -1,13 +1,13 @@ +//go:build windows // +build windows package security import ( + "fmt" "os" "syscall" "unsafe" - - "github.com/pkg/errors" ) type ( @@ -20,25 +20,37 @@ type ( securityInformation uint32 trusteeForm uint32 trusteeType uint32 +) - explicitAccess struct { - accessPermissions accessMask - accessMode accessMode - inheritance inheritMode - trustee trustee - } +type explicitAccess struct { + //nolint:structcheck + accessPermissions accessMask + //nolint:structcheck + accessMode accessMode + //nolint:structcheck + inheritance inheritMode + //nolint:structcheck + trustee trustee +} - trustee struct { - multipleTrustee *trustee - multipleTrusteeOperation int32 - trusteeForm trusteeForm - trusteeType trusteeType - name uintptr - } -) +type trustee struct { + //nolint:unused,structcheck + multipleTrustee *trustee + //nolint:unused,structcheck + multipleTrusteeOperation int32 + trusteeForm trusteeForm + trusteeType trusteeType + name uintptr +} const ( - accessMaskDesiredPermission accessMask = 1 << 31 // GENERIC_READ + AccessMaskNone accessMask = 0 + AccessMaskRead accessMask = 1 << 31 // GENERIC_READ + AccessMaskWrite accessMask = 1 << 30 // GENERIC_WRITE + AccessMaskExecute accessMask = 1 << 29 // GENERIC_EXECUTE + AccessMaskAll accessMask = 1 << 28 // GENERIC_ALL + + accessMaskDesiredPermission = AccessMaskRead accessModeGrant accessMode = 1 @@ -57,6 +69,7 @@ const ( shareModeRead shareMode = 0x1 shareModeWrite shareMode = 0x2 + //nolint:stylecheck // ST1003 sidVmGroup = "S-1-5-83-0" trusteeFormIsSid trusteeForm = 0 @@ -64,15 +77,24 @@ const ( trusteeTypeWellKnownGroup trusteeType = 5 ) -// GrantVMGroupAccess sets the DACL for a specified file or directory to +// GrantVmGroupAccess sets the DACL for a specified file or directory to // include Grant ACE entries for the VM Group SID. This is a golang re- // implementation of the same function in vmcompute, just not exported in // RS5. Which kind of sucks. Sucks a lot :/ -func GrantVmGroupAccess(name string) error { +func GrantVmGroupAccess(name string) error { //nolint:stylecheck // ST1003 + return GrantVmGroupAccessWithMask(name, accessMaskDesiredPermission) +} + +// GrantVmGroupAccessWithMask sets the desired DACL for a specified file or +// directory. +func GrantVmGroupAccessWithMask(name string, access accessMask) error { //nolint:stylecheck // ST1003 + if access == 0 || access<<4 != 0 { + return fmt.Errorf("invalid access mask: 0x%08x", access) + } // Stat (to determine if `name` is a directory). s, err := os.Stat(name) if err != nil { - return errors.Wrapf(err, "%s os.Stat %s", gvmga, name) + return fmt.Errorf("%s os.Stat %s: %w", gvmga, name, err) } // Get a handle to the file/directory. Must defer Close on success. @@ -80,7 +102,9 @@ func GrantVmGroupAccess(name string) error { if err != nil { return err // Already wrapped } - defer syscall.CloseHandle(fd) + defer func() { + _ = syscall.CloseHandle(fd) + }() // Get the current DACL and Security Descriptor. Must defer LocalFree on success. ot := objectTypeFileObject @@ -88,21 +112,25 @@ func GrantVmGroupAccess(name string) error { sd := uintptr(0) origDACL := uintptr(0) if err := getSecurityInfo(fd, uint32(ot), uint32(si), nil, nil, &origDACL, nil, &sd); err != nil { - return errors.Wrapf(err, "%s GetSecurityInfo %s", gvmga, name) + return fmt.Errorf("%s GetSecurityInfo %s: %w", gvmga, name, err) } - defer syscall.LocalFree((syscall.Handle)(unsafe.Pointer(sd))) + defer func() { + _, _ = syscall.LocalFree((syscall.Handle)(unsafe.Pointer(sd))) + }() // Generate a new DACL which is the current DACL with the required ACEs added. // Must defer LocalFree on success. - newDACL, err := generateDACLWithAcesAdded(name, s.IsDir(), origDACL) + newDACL, err := generateDACLWithAcesAdded(name, s.IsDir(), access, origDACL) if err != nil { return err // Already wrapped } - defer syscall.LocalFree((syscall.Handle)(unsafe.Pointer(newDACL))) + defer func() { + _, _ = syscall.LocalFree((syscall.Handle)(unsafe.Pointer(newDACL))) + }() // And finally use SetSecurityInfo to apply the updated DACL. if err := setSecurityInfo(fd, uint32(ot), uint32(si), uintptr(0), uintptr(0), newDACL, uintptr(0)); err != nil { - return errors.Wrapf(err, "%s SetSecurityInfo %s", gvmga, name) + return fmt.Errorf("%s SetSecurityInfo %s: %w", gvmga, name, err) } return nil @@ -111,7 +139,10 @@ func GrantVmGroupAccess(name string) error { // createFile is a helper function to call [Nt]CreateFile to get a handle to // the file or directory. func createFile(name string, isDir bool) (syscall.Handle, error) { - namep := syscall.StringToUTF16(name) + namep, err := syscall.UTF16FromString(name) + if err != nil { + return 0, fmt.Errorf("syscall.UTF16FromString %s: %w", name, err) + } da := uint32(desiredAccessReadControl | desiredAccessWriteDac) sm := uint32(shareModeRead | shareModeWrite) fa := uint32(syscall.FILE_ATTRIBUTE_NORMAL) @@ -120,18 +151,18 @@ func createFile(name string, isDir bool) (syscall.Handle, error) { } fd, err := syscall.CreateFile(&namep[0], da, sm, nil, syscall.OPEN_EXISTING, fa, 0) if err != nil { - return 0, errors.Wrapf(err, "%s syscall.CreateFile %s", gvmga, name) + return 0, fmt.Errorf("%s syscall.CreateFile %s: %w", gvmga, name, err) } return fd, nil } // generateDACLWithAcesAdded generates a new DACL with the two needed ACEs added. // The caller is responsible for LocalFree of the returned DACL on success. -func generateDACLWithAcesAdded(name string, isDir bool, origDACL uintptr) (uintptr, error) { +func generateDACLWithAcesAdded(name string, isDir bool, desiredAccess accessMask, origDACL uintptr) (uintptr, error) { // Generate pointers to the SIDs based on the string SIDs sid, err := syscall.StringToSid(sidVmGroup) if err != nil { - return 0, errors.Wrapf(err, "%s syscall.StringToSid %s %s", gvmga, name, sidVmGroup) + return 0, fmt.Errorf("%s syscall.StringToSid %s %s: %w", gvmga, name, sidVmGroup, err) } inheritance := inheritModeNoInheritance @@ -140,8 +171,8 @@ func generateDACLWithAcesAdded(name string, isDir bool, origDACL uintptr) (uintp } eaArray := []explicitAccess{ - explicitAccess{ - accessPermissions: accessMaskDesiredPermission, + { + accessPermissions: desiredAccess, accessMode: accessModeGrant, inheritance: inheritance, trustee: trustee{ @@ -154,7 +185,7 @@ func generateDACLWithAcesAdded(name string, isDir bool, origDACL uintptr) (uintp modifiedDACL := uintptr(0) if err := setEntriesInAcl(uintptr(uint32(1)), uintptr(unsafe.Pointer(&eaArray[0])), origDACL, &modifiedDACL); err != nil { - return 0, errors.Wrapf(err, "%s SetEntriesInAcl %s", gvmga, name) + return 0, fmt.Errorf("%s SetEntriesInAcl %s: %w", gvmga, name, err) } return modifiedDACL, nil diff --git a/test/vendor/github.com/Microsoft/hcsshim/internal/security/syscall_windows.go b/test/vendor/github.com/Microsoft/hcsshim/internal/security/syscall_windows.go new file mode 100644 index 0000000000..f0cdd7d20c --- /dev/null +++ b/test/vendor/github.com/Microsoft/hcsshim/internal/security/syscall_windows.go @@ -0,0 +1,7 @@ +package security + +//go:generate go run $GOPATH/src/golang.org/x/sys/windows/mkwinsyscall/mkwinsyscall.go -output zsyscall_windows.go syscall_windows.go + +//sys getSecurityInfo(handle syscall.Handle, objectType uint32, si uint32, ppsidOwner **uintptr, ppsidGroup **uintptr, ppDacl *uintptr, ppSacl *uintptr, ppSecurityDescriptor *uintptr) (win32err error) = advapi32.GetSecurityInfo +//sys setSecurityInfo(handle syscall.Handle, objectType uint32, si uint32, psidOwner uintptr, psidGroup uintptr, pDacl uintptr, pSacl uintptr) (win32err error) = advapi32.SetSecurityInfo +//sys setEntriesInAcl(count uintptr, pListOfEEs uintptr, oldAcl uintptr, newAcl *uintptr) (win32err error) = advapi32.SetEntriesInAclW diff --git a/test/vendor/github.com/Microsoft/go-winio/pkg/security/zsyscall_windows.go b/test/vendor/github.com/Microsoft/hcsshim/internal/security/zsyscall_windows.go similarity index 59% rename from test/vendor/github.com/Microsoft/go-winio/pkg/security/zsyscall_windows.go rename to test/vendor/github.com/Microsoft/hcsshim/internal/security/zsyscall_windows.go index 4a90cb3cc8..4084680e0f 100644 --- a/test/vendor/github.com/Microsoft/go-winio/pkg/security/zsyscall_windows.go +++ b/test/vendor/github.com/Microsoft/hcsshim/internal/security/zsyscall_windows.go @@ -45,26 +45,26 @@ var ( procSetSecurityInfo = modadvapi32.NewProc("SetSecurityInfo") ) -func getSecurityInfo(handle syscall.Handle, objectType uint32, si uint32, ppsidOwner **uintptr, ppsidGroup **uintptr, ppDacl *uintptr, ppSacl *uintptr, ppSecurityDescriptor *uintptr) (err error) { - r1, _, e1 := syscall.Syscall9(procGetSecurityInfo.Addr(), 8, uintptr(handle), uintptr(objectType), uintptr(si), uintptr(unsafe.Pointer(ppsidOwner)), uintptr(unsafe.Pointer(ppsidGroup)), uintptr(unsafe.Pointer(ppDacl)), uintptr(unsafe.Pointer(ppSacl)), uintptr(unsafe.Pointer(ppSecurityDescriptor)), 0) - if r1 != 0 { - err = errnoErr(e1) +func getSecurityInfo(handle syscall.Handle, objectType uint32, si uint32, ppsidOwner **uintptr, ppsidGroup **uintptr, ppDacl *uintptr, ppSacl *uintptr, ppSecurityDescriptor *uintptr) (win32err error) { + r0, _, _ := syscall.Syscall9(procGetSecurityInfo.Addr(), 8, uintptr(handle), uintptr(objectType), uintptr(si), uintptr(unsafe.Pointer(ppsidOwner)), uintptr(unsafe.Pointer(ppsidGroup)), uintptr(unsafe.Pointer(ppDacl)), uintptr(unsafe.Pointer(ppSacl)), uintptr(unsafe.Pointer(ppSecurityDescriptor)), 0) + if r0 != 0 { + win32err = syscall.Errno(r0) } return } -func setEntriesInAcl(count uintptr, pListOfEEs uintptr, oldAcl uintptr, newAcl *uintptr) (err error) { - r1, _, e1 := syscall.Syscall6(procSetEntriesInAclW.Addr(), 4, uintptr(count), uintptr(pListOfEEs), uintptr(oldAcl), uintptr(unsafe.Pointer(newAcl)), 0, 0) - if r1 != 0 { - err = errnoErr(e1) +func setEntriesInAcl(count uintptr, pListOfEEs uintptr, oldAcl uintptr, newAcl *uintptr) (win32err error) { + r0, _, _ := syscall.Syscall6(procSetEntriesInAclW.Addr(), 4, uintptr(count), uintptr(pListOfEEs), uintptr(oldAcl), uintptr(unsafe.Pointer(newAcl)), 0, 0) + if r0 != 0 { + win32err = syscall.Errno(r0) } return } -func setSecurityInfo(handle syscall.Handle, objectType uint32, si uint32, psidOwner uintptr, psidGroup uintptr, pDacl uintptr, pSacl uintptr) (err error) { - r1, _, e1 := syscall.Syscall9(procSetSecurityInfo.Addr(), 7, uintptr(handle), uintptr(objectType), uintptr(si), uintptr(psidOwner), uintptr(psidGroup), uintptr(pDacl), uintptr(pSacl), 0, 0) - if r1 != 0 { - err = errnoErr(e1) +func setSecurityInfo(handle syscall.Handle, objectType uint32, si uint32, psidOwner uintptr, psidGroup uintptr, pDacl uintptr, pSacl uintptr) (win32err error) { + r0, _, _ := syscall.Syscall9(procSetSecurityInfo.Addr(), 7, uintptr(handle), uintptr(objectType), uintptr(si), uintptr(psidOwner), uintptr(psidGroup), uintptr(pDacl), uintptr(pSacl), 0, 0) + if r0 != 0 { + win32err = syscall.Errno(r0) } return } diff --git a/test/vendor/github.com/Microsoft/hcsshim/internal/uvm/scsi.go b/test/vendor/github.com/Microsoft/hcsshim/internal/uvm/scsi.go index cc884c3bcd..e21d0ddfe2 100644 --- a/test/vendor/github.com/Microsoft/hcsshim/internal/uvm/scsi.go +++ b/test/vendor/github.com/Microsoft/hcsshim/internal/uvm/scsi.go @@ -12,7 +12,6 @@ import ( "path/filepath" "strings" - "github.com/Microsoft/go-winio/pkg/security" "github.com/pkg/errors" "github.com/sirupsen/logrus" @@ -22,6 +21,7 @@ import ( "github.com/Microsoft/hcsshim/internal/log" "github.com/Microsoft/hcsshim/internal/protocol/guestrequest" "github.com/Microsoft/hcsshim/internal/protocol/guestresource" + "github.com/Microsoft/hcsshim/internal/security" "github.com/Microsoft/hcsshim/internal/wclayer" ) diff --git a/test/vendor/modules.txt b/test/vendor/modules.txt index b0d29873d5..e91c03c2c1 100644 --- a/test/vendor/modules.txt +++ b/test/vendor/modules.txt @@ -4,7 +4,6 @@ github.com/Microsoft/go-winio github.com/Microsoft/go-winio/backuptar github.com/Microsoft/go-winio/pkg/guid github.com/Microsoft/go-winio/pkg/process -github.com/Microsoft/go-winio/pkg/security github.com/Microsoft/go-winio/vhd # github.com/Microsoft/hcsshim v0.8.23 => ../ ## explicit; go 1.17 @@ -59,6 +58,7 @@ github.com/Microsoft/hcsshim/internal/resources github.com/Microsoft/hcsshim/internal/runhcs github.com/Microsoft/hcsshim/internal/safefile github.com/Microsoft/hcsshim/internal/schemaversion +github.com/Microsoft/hcsshim/internal/security github.com/Microsoft/hcsshim/internal/shimdiag github.com/Microsoft/hcsshim/internal/timeout github.com/Microsoft/hcsshim/internal/tools/securitypolicy/helpers diff --git a/vendor/github.com/Microsoft/go-winio/pkg/security/syscall_windows.go b/vendor/github.com/Microsoft/go-winio/pkg/security/syscall_windows.go deleted file mode 100644 index c40c2739b7..0000000000 --- a/vendor/github.com/Microsoft/go-winio/pkg/security/syscall_windows.go +++ /dev/null @@ -1,7 +0,0 @@ -package security - -//go:generate go run mksyscall_windows.go -output zsyscall_windows.go syscall_windows.go - -//sys getSecurityInfo(handle syscall.Handle, objectType uint32, si uint32, ppsidOwner **uintptr, ppsidGroup **uintptr, ppDacl *uintptr, ppSacl *uintptr, ppSecurityDescriptor *uintptr) (err error) [failretval!=0] = advapi32.GetSecurityInfo -//sys setSecurityInfo(handle syscall.Handle, objectType uint32, si uint32, psidOwner uintptr, psidGroup uintptr, pDacl uintptr, pSacl uintptr) (err error) [failretval!=0] = advapi32.SetSecurityInfo -//sys setEntriesInAcl(count uintptr, pListOfEEs uintptr, oldAcl uintptr, newAcl *uintptr) (err error) [failretval!=0] = advapi32.SetEntriesInAclW diff --git a/vendor/modules.txt b/vendor/modules.txt index 6462b39e28..a90aef84a6 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -9,7 +9,6 @@ github.com/Microsoft/go-winio/pkg/etw github.com/Microsoft/go-winio/pkg/etwlogrus github.com/Microsoft/go-winio/pkg/guid github.com/Microsoft/go-winio/pkg/process -github.com/Microsoft/go-winio/pkg/security github.com/Microsoft/go-winio/vhd # github.com/cenkalti/backoff/v4 v4.1.1 ## explicit; go 1.13