diff --git a/pkg/security/grantvmgroupaccess.go b/pkg/security/grantvmgroupaccess.go index 4fcf671..2df31b6 100644 --- a/pkg/security/grantvmgroupaccess.go +++ b/pkg/security/grantvmgroupaccess.go @@ -36,7 +36,7 @@ type ( ) const ( - accessMaskDesiredPermission accessMask = 0x12019f + accessMaskDesiredPermission accessMask = 1 << 31 // GENERIC_READ accessModeGrant accessMode = 1 @@ -55,8 +55,7 @@ const ( shareModeRead shareMode = 0x1 shareModeWrite shareMode = 0x2 - sidVmGroup = "S-1-5-83-1-3166535780-1122986932-343720105-43916321" - sidVmWorkerProcessCapability = "S-1-15-3-1024-2268835264-3721307629-241982045-173645152-1490879176-104643441-2915960892-1612460704" + sidVmGroup = "S-1-5-83-0" trusteeFormIsSid trusteeForm = 0 @@ -64,9 +63,9 @@ const ( ) // GrantVMGroupAccess sets the DACL for a specified file or directory to -// include Grant ACE entries for both the VM Group SID, and the VM Worker Process -// Capability 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 :/ +// 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 { // Stat (to determine if `name` is a directory). s, err := os.Stat(name) @@ -91,56 +90,6 @@ func GrantVmGroupAccess(name string) error { } defer syscall.LocalFree((syscall.Handle)(unsafe.Pointer(sd))) - // Just a very large comment in case you ever want to debug this... Shows how - // to use a winio library function to examine the security descriptor, and - // how to decode obtained SD. This example if from a VHD which is attached to a - // B in Hyper-V. The only real thing of note is the ACE with the VM WP Capability SID - // The other ACEs aren't particularly interesting. Except to note that the second - // SID is the SID of the worker process. We don't do that in this code, rather - // add the VM Group SID. To debug would need to add `fmt` to imports. Will also - // need `//sys getSecurityDescriptorLength(sd uintptr) (len uint32) = advapi32.GetSecurityDescriptorLength` - // defined and regenerated. - // - // >> sdByteArray := make([]byte, getSecurityDescriptorLength(sd)) - // >> copy(sdByteArray, (*[0xffff]byte)(unsafe.Pointer(sd))[:len(sdByteArray)]) - // >> sddl, err := winio.SecurityDescriptorToSddl(sdByteArray) - // >> if err != nil { - // >> return err - // >> } - // >> fmt.Println("SDDL:", sddl) - // - // Example output (pretty-printed) - first one (or two potentially) are the interesting ones: - // D:AI - // (A;;0x12019f;;;S-1-15-3-1024-2268835264-3721307629-241982045-173645152-1490879176-104643441-2915960892-1612460704) - // (A;;0x12019f;;;S-1-5-83-1-3166535780-1122986932-343720105-43916321) - // (A;ID;FA;;;BA) - // (A;ID;FA;;;SY) - // (A;ID;0x1301bf;;;AU) - // (A;ID;0x1200a9;;;BU) - // - // And what ICACLS on that file shows - // S-1-15-3-1024-2268835264-3721307629-241982045-173645152-1490879176-104643441-2915960892-1612460704:(R,W) - // NT VIRTUAL MACHINE\BCBD8064-6BB4-42EF-A9C0-7C14211C9E02:(R,W) - // BUILTIN\Administrators:(I)(F) - // NT AUTHORITY\SYSTEM:(I)(F) - // NT AUTHORITY\Authenticated Users:(I)(M) - // BUILTIN\Users:(I)(RX) - // - // Translating D:AI(A;;0x12019f;;;S-1-15-3-1024-2268835264-3721307629-241982045-173645152-1490879176-104643441-2915960892-1612460704) - // https://docs.microsoft.com/en-us/windows/desktop/secauthz/security-descriptor-string-format - // - D:AI DACL:AutoInherited followed by ACEs, of which the first one is: - // - (A;;0x12019f;;;S-1-15-3-1024-2268835264-3721307629-241982045-173645152-1490879176-104643441-2915960892-1612460704) - // Format is ace_type;ace_flags;rights;object_guid;inherit_object_guid;account_sid;(resource_attribute) - // ace_type = Allowed - // ace_flags = 0 - // rights = 0x12019f (exercise for reader to decode) - // object_guid=blank - // inherit_object_guid=blank - // account_sid=VM Worker Process Capability SID - // - // Translating (A;;0x12019f;;;S-1-5-83-1-3166535780-1122986932-343720105-43916321) - // is simples. It's the SID of the specific VMWP. - // 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) @@ -178,14 +127,10 @@ func createFile(name string, isDir bool) (syscall.Handle, error) { // The caller is responsible for LocalFree of the returned DACL on success. func generateDACLWithAcesAdded(name string, isDir bool, origDACL uintptr) (uintptr, error) { // Generate pointers to the SIDs based on the string SIDs - sid1, err := syscall.StringToSid(sidVmGroup) + sid, err := syscall.StringToSid(sidVmGroup) if err != nil { return 0, errors.Wrapf(err, "%s syscall.StringToSid %s %s", gvmga, name, sidVmGroup) } - sid2, err := syscall.StringToSid(sidVmWorkerProcessCapability) - if err != nil { - return 0, errors.Wrapf(err, "%s syscall.StringToSid %s %s", gvmga, name, sidVmWorkerProcessCapability) - } inheritance := inheritModeNoInheritance if isDir { @@ -200,23 +145,13 @@ func generateDACLWithAcesAdded(name string, isDir bool, origDACL uintptr) (uintp trustee: trustee{ trusteeForm: trusteeFormIsSid, trusteeType: trusteeTypeWellKnownGroup, - name: uintptr(unsafe.Pointer(sid1)), - }, - }, - explicitAccess{ - accessPermissions: accessMaskDesiredPermission, - accessMode: accessModeGrant, - inheritance: inheritance, - trustee: trustee{ - trusteeForm: trusteeFormIsSid, - trusteeType: trusteeTypeWellKnownGroup, - name: uintptr(unsafe.Pointer(sid2)), + name: uintptr(unsafe.Pointer(sid)), }, }, } modifiedDACL := uintptr(0) - if err := setEntriesInAcl(uintptr(uint32(2)), uintptr(unsafe.Pointer(&eaArray[0])), origDACL, &modifiedDACL); err != nil { + 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) } diff --git a/pkg/security/grantvmgroupaccess_test.go b/pkg/security/grantvmgroupaccess_test.go index 341b38d..3a1705f 100644 --- a/pkg/security/grantvmgroupaccess_test.go +++ b/pkg/security/grantvmgroupaccess_test.go @@ -58,25 +58,32 @@ func TestGrantVmGroupAccess(t *testing.T) { verifyicacls(t, f.Name(), - "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)", + []string{`NT VIRTUAL MACHINE\\Virtual Machines:(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 verifyicacls(t, d, - "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)", + []string{`NT VIRTUAL MACHINE\\Virtual Machines:(R)`, `NT VIRTUAL MACHINE\\Virtual Machines:(OI)(CI)(IO)(GR)`}, ) verifyicacls(t, find.Name(), - "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)", + []string{`NT VIRTUAL MACHINE\\Virtual Machines:(I)(R)`}, ) } -func verifyicacls(t *testing.T, name string, ace1 string, ace2 string) { +func verifyicacls(t *testing.T, name string, aces []string) { cmd := exec.Command("icacls", name) outb, err := cmd.CombinedOutput() if err != nil { @@ -84,21 +91,15 @@ func verifyicacls(t *testing.T, name string, ace1 string, ace2 string) { } out := string(outb) - // Avoid () being part of match groups - ace1 = strings.Replace(ace1, "(", "\\(", -1) - ace1 = strings.Replace(ace1, ")", "\\)", -1) - ace2 = strings.Replace(ace2, "(", "\\(", -1) - ace2 = strings.Replace(ace2, ")", "\\)", -1) + for _, ace := range aces { + // Avoid '(' and ')' being part of match groups + ace = strings.Replace(ace, "(", "\\(", -1) + ace = strings.Replace(ace, ")", "\\)", -1) - rx1 := regexp.MustCompile(ace1) - matches1 := rx1.FindAllStringIndex(out, -1) - if len(matches1) != 1 { - t.Fatalf("expected one match for %s got %d", ace1, len(matches1)) - } - - rx2 := regexp.MustCompile(ace1) - matches2 := rx2.FindAllStringIndex(out, -1) - if len(matches2) != 1 { - t.Fatalf("expected one match for %s got %d", ace2, len(matches2)) + rx := regexp.MustCompile(ace) + matches := rx.FindAllStringIndex(out, -1) + if len(matches) != 1 { + t.Fatalf("expected one match for %s got %d\n%s", ace, len(matches), out) + } } }