[4/7] server: Setting a security descriptor should not replace an existing owner or group if only a DACL is being set.

Joris van der Wel joris at jorisvanderwel.com
Sat Jun 28 07:22:06 CDT 2014


server: Setting a security descriptor should not replace an existing owner
or group if only a DACL is being set.

---
 dlls/advapi32/tests/security.c | 95
++++++++++++++++++++++++++++++++++++++++++
 server/object.c                | 29 +++++++++++--
 2 files changed, 120 insertions(+), 4 deletions(-)
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.winehq.org/pipermail/wine-patches/attachments/20140628/8de0bed1/attachment.html>
-------------- next part --------------
From 908c5a99232b8ca8fab349162218be40fc6f3785 Mon Sep 17 00:00:00 2001
From: Joris van der Wel <joris at jorisvanderwel.com>
Date: Thu, 26 Jun 2014 21:46:22 +0200
Subject: server: Setting a security descriptor should not replace an existing
 owner or group if only a DACL is being set.

---
 dlls/advapi32/tests/security.c | 95 ++++++++++++++++++++++++++++++++++++++++++
 server/object.c                | 29 +++++++++++--
 2 files changed, 120 insertions(+), 4 deletions(-)

diff --git a/dlls/advapi32/tests/security.c b/dlls/advapi32/tests/security.c
index f3ccc8e..475dd62 100644
--- a/dlls/advapi32/tests/security.c
+++ b/dlls/advapi32/tests/security.c
@@ -234,6 +234,65 @@ static SECURITY_DESCRIPTOR* test_get_security_descriptor(HANDLE handle, int line
     return sd;
 }
 
+static void check_dacl_is_set(HANDLE Handle, BOOL set, int line)
+{
+    BOOL res;
+    SECURITY_DESCRIPTOR *queriedSD = NULL;
+    PACL queriedAcl = NULL;
+    BOOL dacl_present;
+    BOOL dacl_defaulted;
+    SECURITY_DESCRIPTOR_CONTROL control;
+    DWORD revision;
+    
+    queriedSD = test_get_security_descriptor( Handle, line );
+    queriedAcl = (PACL)0xdeadbeef;
+    
+    res = GetSecurityDescriptorDacl(queriedSD, &dacl_present, &queriedAcl, &dacl_defaulted);
+    ok_(__FILE__, line)(res, "GetSecurityDescriptorDacl failed with error %d\n", GetLastError());
+    
+    res = GetSecurityDescriptorControl(queriedSD, &control, &revision);
+    ok_(__FILE__, line)(res, "GetSecurityDescriptorControl failed with error %d\n", GetLastError());
+    
+    ok_(__FILE__, line)(dacl_present, "DACL should be present\n");
+    if (set)
+        ok_(__FILE__, line)(queriedAcl != NULL, "DACL is NULL\n");
+    else
+        ok_(__FILE__, line)(queriedAcl == NULL, "DACL is not NULL\n");
+    ok_(__FILE__, line)(!dacl_defaulted, "Defaulted is true\n");
+    ok_(__FILE__, line)((control & SE_DACL_DEFAULTED) == 0, "SE_DACL_DEFAULTED is set\n");
+    
+    HeapFree(GetProcessHeap(), 0, queriedSD);
+}
+
+static void check_group(HANDLE Handle, PSID expected, int line)
+{
+    BOOL res;
+    SECURITY_DESCRIPTOR *queriedSD = NULL;
+    PSID group;
+    BOOL group_defaulted;
+    SECURITY_DESCRIPTOR_CONTROL control;
+    DWORD revision;
+    LPSTR sid_str = NULL;
+    
+    queriedSD = test_get_security_descriptor( Handle, line );
+    
+    res = GetSecurityDescriptorGroup(queriedSD, &group, &group_defaulted);
+    ok_(__FILE__, line)(res, "GetSecurityDescriptorGroup failed with error %d\n", GetLastError());
+    
+    res = pConvertSidToStringSidA( group, &sid_str );
+    ok_(__FILE__, line)(res, "ConvertSidToStringSidA failed with error %d\n", GetLastError());
+    
+    res = GetSecurityDescriptorControl(queriedSD, &control, &revision);
+    ok_(__FILE__, line)(res, "GetSecurityDescriptorControl failed with error %d\n", GetLastError());
+    
+    ok_(__FILE__, line)(EqualSid(group, expected), "Group SIDs are not equal, actual value is %s\n", sid_str);
+    ok_(__FILE__, line)(!group_defaulted, "Defaulted is true\n");
+    ok_(__FILE__, line)((control & SE_GROUP_DEFAULTED) == 0, "SE_GROUP_DEFAULTED is set\n");
+    
+    HeapFree(GetProcessHeap(), 0, queriedSD);
+    LocalFree(sid_str);
+}
+
 static void test_sid(void)
 {
     struct sidRef refs[] = {
@@ -2504,6 +2563,9 @@ static void test_process_security(void)
     SECURITY_ATTRIBUTES psa;
     HANDLE token, event;
     DWORD size;
+    SID_IDENTIFIER_AUTHORITY SIDAuthWorld = { SECURITY_WORLD_SID_AUTHORITY };
+    PSID EveryoneSid = NULL;
+
 
     Acl = HeapAlloc(GetProcessHeap(), 0, 256);
     res = InitializeAcl(Acl, 256, ACL_REVISION);
@@ -2514,6 +2576,9 @@ static void test_process_security(void)
         return;
     }
     ok(res, "InitializeAcl failed with error %d\n", GetLastError());
+            
+    res = AllocateAndInitializeSid( &SIDAuthWorld, 1, SECURITY_WORLD_RID, 0, 0, 0, 0, 0, 0, 0, &EveryoneSid);
+    ok(res, "AllocateAndInitializeSid failed with error %d\n", GetLastError());
 
     /* get owner from the token we might be running as a user not admin */
     res = OpenProcessToken( GetCurrentProcess(), MAXIMUM_ALLOWED, &token );
@@ -2574,19 +2639,48 @@ static void test_process_security(void)
     CHECK_SET_SECURITY( event, SACL_SECURITY_INFORMATION, ERROR_ACCESS_DENIED );
     CHECK_SET_SECURITY( event, DACL_SECURITY_INFORMATION, ERROR_SUCCESS );
     /* NULL DACL is valid and means that everyone has access */
+    check_dacl_is_set( event, FALSE, __LINE__ );
     SecurityDescriptor->Control |= SE_DACL_PRESENT;
     CHECK_SET_SECURITY( event, DACL_SECURITY_INFORMATION, ERROR_SUCCESS );
+    check_dacl_is_set( event, FALSE, __LINE__ );
 
     /* Set owner and group and dacl */
     res = SetSecurityDescriptorOwner(SecurityDescriptor, AdminSid, FALSE);
     ok(res, "SetSecurityDescriptorOwner failed with error %d\n", GetLastError());
     CHECK_SET_SECURITY( event, OWNER_SECURITY_INFORMATION, ERROR_SUCCESS );
+    check_dacl_is_set( event, FALSE, __LINE__ );
+    
     res = SetSecurityDescriptorGroup(SecurityDescriptor, UsersSid, FALSE);
     ok(res, "SetSecurityDescriptorGroup failed with error %d\n", GetLastError());
     CHECK_SET_SECURITY( event, GROUP_SECURITY_INFORMATION, ERROR_SUCCESS );
+    check_dacl_is_set( event, FALSE, __LINE__ );
+    check_group( event, UsersSid, __LINE__ );
+    
     res = SetSecurityDescriptorDacl(SecurityDescriptor, TRUE, Acl, FALSE);
     ok(res, "SetSecurityDescriptorDacl failed with error %d\n", GetLastError());
     CHECK_SET_SECURITY( event, DACL_SECURITY_INFORMATION, ERROR_SUCCESS );
+    check_dacl_is_set( event, TRUE, __LINE__ );
+    check_group( event, UsersSid, __LINE__ );
+    /* set the user again, the previous DACL should remain set even though 
+       the security descriptor has a different DACL 
+       (DACL_SECURITY_INFORMATION is not given) */
+    res = SetSecurityDescriptorDacl(SecurityDescriptor, TRUE, NULL, FALSE);
+    ok(res, "SetSecurityDescriptorDacl failed with error %d\n", GetLastError());
+    CHECK_SET_SECURITY( event, OWNER_SECURITY_INFORMATION, ERROR_SUCCESS );
+    check_dacl_is_set( event, TRUE, __LINE__ );
+
+    /* set a different group, it should remain set even if we change the dacl */
+    res = SetSecurityDescriptorGroup(SecurityDescriptor, EveryoneSid, FALSE);
+    ok(res, "SetSecurityDescriptorGroup failed with error %d\n", GetLastError());
+    CHECK_SET_SECURITY( event, GROUP_SECURITY_INFORMATION, ERROR_SUCCESS );
+    check_group( event, EveryoneSid, __LINE__ );
+    CHECK_SET_SECURITY( event, DACL_SECURITY_INFORMATION, ERROR_SUCCESS );
+    check_dacl_is_set( event, FALSE, __LINE__ );
+    check_group( event, EveryoneSid, __LINE__ );
+    
+    res = SetSecurityDescriptorDacl(SecurityDescriptor, TRUE, Acl, FALSE);
+    ok(res, "SetSecurityDescriptorDacl failed with error %d\n", GetLastError());
+    
 
     sprintf(buffer, "%s tests/security.c test", myARGV[0]);
     memset(&startup, 0, sizeof(startup));
@@ -2612,6 +2706,7 @@ static void test_process_security(void)
     HeapFree(GetProcessHeap(), 0, owner);
     HeapFree(GetProcessHeap(), 0, Acl);
     HeapFree(GetProcessHeap(), 0, SecurityDescriptor);
+    FreeSid(EveryoneSid);
 }
 
 static void test_process_security_child(void)
diff --git a/server/object.c b/server/object.c
index 021c741..394183d 100644
--- a/server/object.c
+++ b/server/object.c
@@ -438,20 +438,41 @@ int default_set_sd( struct object *obj, const struct security_descriptor *sd,
 
     owner = sd_get_owner( sd );
     if (set_info & OWNER_SECURITY_INFORMATION && owner)
+    {
         new_sd.owner_len = sd->owner_len;
+    }
     else
     {
-        owner = token_get_user( current->process->token );
-        new_sd.owner_len = security_sid_len( owner );
+        owner = obj->sd ? sd_get_owner( obj->sd ) : NULL;
+        
+        if (owner)
+        {
+            new_sd.owner_len = obj->sd->owner_len;
+        }
+        else
+        {
+            owner = token_get_user( current->process->token );
+            new_sd.owner_len = security_sid_len( owner );
+        }
     }
 
     group = sd_get_group( sd );
     if (set_info & GROUP_SECURITY_INFORMATION && group)
+    {
         new_sd.group_len = sd->group_len;
+    }
     else
     {
-        group = token_get_primary_group( current->process->token );
-        new_sd.group_len = security_sid_len( group );
+        group = obj->sd ? sd_get_group( obj->sd ) : NULL;
+        if (group)
+        {
+            new_sd.group_len = obj->sd->group_len;
+        }
+        else
+        {
+            group = token_get_primary_group( current->process->token );
+            new_sd.group_len = security_sid_len( group );
+        }
     }
 
     new_sd.control |= SE_SACL_PRESENT;
-- 
1.8.1.msysgit.1



More information about the wine-patches mailing list