From: "Erich E. Hoover" Subject: advapi32: Report world-access ownership by default for registry objects (try 2). Message-Id: Date: Tue, 12 Feb 2013 17:12:11 -0700 The attached patch fixes an issue where the Opera installer expects registry keys to return ownership information (Bug #32904), concluding the cleanup of [Get|Set]NamedSecurityInfo. This patch addresses the issue by ensuring that NULL owner and group information is never returned for registry keys and instead returns the World SID (S-1-1-0) when this information is unavailable, which is sufficient to keep the Opera installer from encountering an error. The included test shows that this behavior is essentially the case for system keys on Windows, though the Administrators SID is returned instead. Since GetNamedSecurityInfo historically returned the World SID, and several other routines use it as well) I've opted to use the World SID for now. It's worth noting that in my previous patch I mistakenly thought that the problem was that the DACL needed to be returned, but further testing has shown that the Opera installer is _just_ expecting the owner and group SIDs to be filled in. From 638ae4139a7427675476c96ab6ff54fc366d2a48 Mon Sep 17 00:00:00 2001 From: Erich Hoover Date: Tue, 12 Feb 2013 17:04:39 -0700 Subject: advapi32: Report world-access ownership by default for registry objects. --- dlls/advapi32/security.c | 35 +++++++++++++++++++++++++++++++++++ dlls/advapi32/tests/security.c | 33 +++++++++++++++++++++++++++++---- 2 files changed, 64 insertions(+), 4 deletions(-) diff --git a/dlls/advapi32/security.c b/dlls/advapi32/security.c index 0d5aba7..0ad37c8 100644 --- a/dlls/advapi32/security.c +++ b/dlls/advapi32/security.c @@ -5619,7 +5619,42 @@ DWORD WINAPI GetNamedSecurityInfoW( LPWSTR name, SE_OBJECT_TYPE type, case SE_REGISTRY_KEY: if (!(err = get_security_regkey( name, access, &handle ))) { + PISECURITY_DESCRIPTOR_RELATIVE sd; + DWORD n1 = 0, n2; + err = GetSecurityInfo( handle, type, info, owner, group, dacl, sacl, descriptor ); + /* If no ownership information is returned then report back a generic world-access SID, + * this behavior is needed by the Opera installer to keep it from failing to get the + * ownership of default system-wide registry keys. */ + sd = (*(PISECURITY_DESCRIPTOR_RELATIVE *)descriptor); + if (!err) + NtQuerySecurityObject( handle, info, NULL, 0, &n1 ); + if (!err && (info & OWNER_SECURITY_INFORMATION) && !sd->Owner) + { + char *buffer; + + n2 = n1 + sizeof(sidWorld); + *descriptor = LocalReAlloc( *descriptor, n2, LMEM_MOVEABLE ); + sd = (*(PISECURITY_DESCRIPTOR_RELATIVE *)descriptor); + sd->Owner = n1; + buffer = (char *)sd + n1; + memcpy( buffer, &sidWorld, sizeof(sidWorld) ); + if (owner) *owner = buffer; + n1 = n2; + } + if (!err && (info & GROUP_SECURITY_INFORMATION) && !sd->Group) + { + char *buffer; + + n2 = n1 + sizeof(sidWorld); + *descriptor = LocalReAlloc( *descriptor, n2, LMEM_MOVEABLE ); + sd = (*(PISECURITY_DESCRIPTOR_RELATIVE *)descriptor); + sd->Group = n1; + buffer = (char *)sd + n1; + memcpy( buffer, &sidWorld, sizeof(sidWorld) ); + if (group) *group = buffer; + n1 = n2; + } RegCloseKey( handle ); } break; diff --git a/dlls/advapi32/tests/security.c b/dlls/advapi32/tests/security.c index e4adab1..7fef6a4 100644 --- a/dlls/advapi32/tests/security.c +++ b/dlls/advapi32/tests/security.c @@ -3005,10 +3005,12 @@ static void test_SetEntriesInAclA(void) static void test_GetNamedSecurityInfoA(void) { - char admin_ptr[sizeof(SID)+sizeof(ULONG)*SID_MAX_SUB_AUTHORITIES], dacl[100], *user; + char admin_ptr[sizeof(SID)+sizeof(ULONG)*SID_MAX_SUB_AUTHORITIES], *user; + char system_ptr[sizeof(SID)+sizeof(ULONG)*SID_MAX_SUB_AUTHORITIES]; + PSID admin_sid = (PSID) admin_ptr, system_sid = (PSID) system_ptr, user_sid; DWORD sid_size = sizeof(admin_ptr), user_size; char invalid_path[] = "/an invalid file path"; - PSID admin_sid = (PSID) admin_ptr, user_sid; + char software_key[] = "MACHINE\\Software"; char sd[SECURITY_DESCRIPTOR_MIN_LENGTH]; SECURITY_DESCRIPTOR_CONTROL control; ACL_SIZE_INFORMATION acl_size; @@ -3113,10 +3115,10 @@ static void test_GetNamedSecurityInfoA(void) /* Create security descriptor information and test that it comes back the same */ pSD = &sd; - pDacl = (PACL)&dacl; + pDacl = HeapAlloc(GetProcessHeap(), 0, 100); InitializeSecurityDescriptor(pSD, SECURITY_DESCRIPTOR_REVISION); pCreateWellKnownSid(WinBuiltinAdministratorsSid, NULL, admin_sid, &sid_size); - bret = InitializeAcl(pDacl, sizeof(dacl), ACL_REVISION); + bret = InitializeAcl(pDacl, 100, ACL_REVISION); ok(bret, "Failed to initialize ACL.\n"); bret = pAddAccessAllowedAceEx(pDacl, ACL_REVISION, 0, GENERIC_ALL, user_sid); ok(bret, "Failed to add Current User to ACL.\n"); @@ -3130,6 +3132,7 @@ static void test_GetNamedSecurityInfoA(void) SetLastError(0xdeadbeef); error = pSetNamedSecurityInfoA(tmpfile, SE_FILE_OBJECT, DACL_SECURITY_INFORMATION, NULL, NULL, pDacl, NULL); + HeapFree(GetProcessHeap(), 0, pDacl); if (error != ERROR_SUCCESS && (GetLastError() == ERROR_CALL_NOT_IMPLEMENTED)) { win_skip("SetNamedSecurityInfoA is not implemented\n"); @@ -3178,6 +3181,28 @@ static void test_GetNamedSecurityInfoA(void) LocalFree(pSD); HeapFree(GetProcessHeap(), 0, user); CloseHandle(hTemp); + + /* Test querying the ownership of a built-in registry key */ + sid_size = sizeof(system_ptr); + pCreateWellKnownSid(WinLocalSystemSid, NULL, system_sid, &sid_size); + error = pGetNamedSecurityInfoA(software_key, SE_REGISTRY_KEY, + OWNER_SECURITY_INFORMATION|GROUP_SECURITY_INFORMATION, + NULL, NULL, NULL, NULL, &pSD); + ok(!error, "GetNamedSecurityInfo failed with error %d\n", error); + + bret = GetSecurityDescriptorOwner(pSD, &owner, &owner_defaulted); + ok(bret, "GetSecurityDescriptorOwner failed with error %d\n", GetLastError()); + ok(owner != NULL, "owner should not be NULL\n"); + todo_wine ok(EqualSid(owner, admin_sid), + "MACHINE\\Software owner SID != Administrators SID.\n"); + + bret = GetSecurityDescriptorGroup(pSD, &group, &group_defaulted); + ok(bret, "GetSecurityDescriptorGroup failed with error %d\n", GetLastError()); + ok(group != NULL, "group should not be NULL\n"); + todo_wine ok(EqualSid(group, admin_sid) + || broken(EqualSid(group, system_sid)) /* before Win7 */, + "MACHINE\\Software group SID != Local System SID.\n"); + LocalFree(pSD); } static void test_ConvertStringSecurityDescriptor(void) -- 1.7.9.5