Merge pull request #1347 from anmaxvl/port-grantvmgroupaccess-code

Port grantvmgroupaccess code from go-winio and extend functionality.

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

test/vendor/github.com/Microsoft/go-winio/pkg/security/grantvmgroupaccess.go → 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,52 +69,68 @@ const (
 	shareModeRead  shareMode = 0x1
 	shareModeWrite shareMode = 0x2
 
+	//nolint:stylecheck // ST1003
 	sidVmGroup = "S-1-5-83-0"
 
 	trusteeFormIsSid trusteeForm = 0
 
 	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.
 	fd, err := createFile(name, s.IsDir())
 	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
 	si := securityInformationDACL
 	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