[PATCH 1/2] server: Add default security descriptor ownership for processes.
From: "Erich E. Hoover" <erich.e.hoover(a)gmail.com> From: Erich E. Hoover <erich.e.hoover(a)gmail.com> Signed-off-by: Vijay Kiran Kamuju <infyquest(a)gmail.com> --- dlls/advapi32/tests/security.c | 35 ++++++++++++++++++++++++++++++++++ server/process.c | 26 ++++++++++++++++++++++++- server/security.h | 1 + server/token.c | 2 ++ 4 files changed, 63 insertions(+), 1 deletion(-) diff --git a/dlls/advapi32/tests/security.c b/dlls/advapi32/tests/security.c index d9cae64da8b..3097a64fb24 100644 --- a/dlls/advapi32/tests/security.c +++ b/dlls/advapi32/tests/security.c @@ -4682,11 +4682,15 @@ static void test_acls(void) static void test_GetSecurityInfo(void) { + char domain_users_ptr[sizeof(TOKEN_USER) + sizeof(SID) + sizeof(DWORD)*SID_MAX_SUB_AUTHORITIES]; char b[sizeof(TOKEN_USER) + sizeof(SID) + sizeof(DWORD)*SID_MAX_SUB_AUTHORITIES]; char admin_ptr[sizeof(SID)+sizeof(ULONG)*SID_MAX_SUB_AUTHORITIES], dacl[100]; + PSID domain_users_sid = (PSID) domain_users_ptr, domain_sid; + SID_IDENTIFIER_AUTHORITY sia = { SECURITY_NT_AUTHORITY }; DWORD sid_size = sizeof(admin_ptr), l = sizeof(b); PSID admin_sid = (PSID) admin_ptr, user_sid; char sd[SECURITY_DESCRIPTOR_MIN_LENGTH]; + BOOL owner_defaulted, group_defaulted; ACL_SIZE_INFORMATION acl_size; PSECURITY_DESCRIPTOR pSD; ACCESS_ALLOWED_ACE *ace; @@ -4813,6 +4817,37 @@ static void test_GetSecurityInfo(void) } LocalFree(pSD); CloseHandle(obj); + + /* Obtain the "domain users" SID from the user SID */ + if (!AllocateAndInitializeSid(&sia, 4, *GetSidSubAuthority(user_sid, 0), + *GetSidSubAuthority(user_sid, 1), + *GetSidSubAuthority(user_sid, 2), + *GetSidSubAuthority(user_sid, 3), 0, 0, 0, 0, &domain_sid)) + { + win_skip("Failed to get current domain SID\n"); + return; + } + sid_size = sizeof(domain_users_ptr); + pCreateWellKnownSid(WinAccountDomainUsersSid, domain_sid, domain_users_sid, &sid_size); + FreeSid(domain_sid); + + /* Test querying the ownership of a process */ + ret = pGetSecurityInfo(GetCurrentProcess(), SE_KERNEL_OBJECT, + OWNER_SECURITY_INFORMATION|GROUP_SECURITY_INFORMATION, + NULL, NULL, NULL, NULL, &pSD); + ok(!ret, "GetNamedSecurityInfo failed with error %d\n", ret); + + bret = GetSecurityDescriptorOwner(pSD, &owner, &owner_defaulted); + ok(bret, "GetSecurityDescriptorOwner failed with error %d\n", GetLastError()); + ok(owner != NULL, "owner should not be NULL\n"); + ok(EqualSid(owner, admin_sid) || EqualSid(owner, user_sid), + "Process 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"); + ok(EqualSid(group, domain_users_sid), "Process group SID != Domain Users SID.\n"); + LocalFree(pSD); } static void test_GetSidSubAuthority(void) diff --git a/server/process.c b/server/process.c index 473d3b1a27a..f7af438b561 100644 --- a/server/process.c +++ b/server/process.c @@ -63,6 +63,7 @@ static void process_dump( struct object *obj, int verbose ); static struct object_type *process_get_type( struct object *obj ); static int process_signaled( struct object *obj, struct wait_queue_entry *entry ); static unsigned int process_map_access( struct object *obj, unsigned int access ); +static struct security_descriptor *process_get_sd( struct object *obj ); static void process_poll_event( struct fd *fd, int event ); static void process_destroy( struct object *obj ); static void terminate_process( struct process *process, struct thread *skip, int exit_code ); @@ -79,7 +80,7 @@ static const struct object_ops process_ops = no_signal, /* signal */ no_get_fd, /* get_fd */ process_map_access, /* map_access */ - default_get_sd, /* get_sd */ + process_get_sd, /* get_sd */ default_set_sd, /* set_sd */ no_lookup_name, /* lookup_name */ no_link_name, /* link_name */ @@ -661,6 +662,29 @@ static unsigned int process_map_access( struct object *obj, unsigned int access return access & ~(GENERIC_READ | GENERIC_WRITE | GENERIC_EXECUTE | GENERIC_ALL); } +static struct security_descriptor *process_get_sd( struct object *obj ) +{ + static struct security_descriptor *key_default_sd; + + if (obj->sd) return obj->sd; + + if (!key_default_sd) + { + size_t users_sid_len = security_sid_len( security_domain_users_sid ); + size_t admins_sid_len = security_sid_len( security_builtin_admins_sid ); + + key_default_sd = mem_alloc( sizeof(*key_default_sd) + admins_sid_len + users_sid_len ); + key_default_sd->control = SE_DACL_PRESENT; + key_default_sd->owner_len = admins_sid_len; + key_default_sd->group_len = users_sid_len; + key_default_sd->sacl_len = 0; + key_default_sd->dacl_len = 0; + memcpy( key_default_sd + 1, security_builtin_admins_sid, admins_sid_len ); + memcpy( (char *)(key_default_sd + 1) + admins_sid_len, security_domain_users_sid, users_sid_len ); + } + return key_default_sd; +} + static void process_poll_event( struct fd *fd, int event ) { struct process *process = get_fd_user( fd ); diff --git a/server/security.h b/server/security.h index 873bbc6afd6..606dbb2ab2c 100644 --- a/server/security.h +++ b/server/security.h @@ -47,6 +47,7 @@ extern const PSID security_local_user_sid; extern const PSID security_local_system_sid; extern const PSID security_builtin_users_sid; extern const PSID security_builtin_admins_sid; +extern const PSID security_domain_users_sid; extern const PSID security_high_label_sid; diff --git a/server/token.c b/server/token.c index 5c35bcc19f6..895809e7252 100644 --- a/server/token.c +++ b/server/token.c @@ -83,6 +83,7 @@ static const SID_N(5) local_user_sid = { SID_REVISION, 5, { SECURITY_NT_AUTHORIT static const SID_N(2) builtin_admins_sid = { SID_REVISION, 2, { SECURITY_NT_AUTHORITY }, { SECURITY_BUILTIN_DOMAIN_RID, DOMAIN_ALIAS_RID_ADMINS } }; static const SID_N(2) builtin_users_sid = { SID_REVISION, 2, { SECURITY_NT_AUTHORITY }, { SECURITY_BUILTIN_DOMAIN_RID, DOMAIN_ALIAS_RID_USERS } }; static const SID_N(3) builtin_logon_sid = { SID_REVISION, 3, { SECURITY_NT_AUTHORITY }, { SECURITY_LOGON_IDS_RID, 0, 0 } }; +static const SID_N(5) domain_users_sid = { SID_REVISION, 5, { SECURITY_NT_AUTHORITY }, { SECURITY_NT_NON_UNIQUE, 0, 0, 0, DOMAIN_GROUP_RID_USERS } }; const PSID security_world_sid = (PSID)&world_sid; static const PSID security_local_sid = (PSID)&local_sid; @@ -92,6 +93,7 @@ const PSID security_local_system_sid = (PSID)&local_system_sid; const PSID security_local_user_sid = (PSID)&local_user_sid; const PSID security_builtin_admins_sid = (PSID)&builtin_admins_sid; const PSID security_builtin_users_sid = (PSID)&builtin_users_sid; +const PSID security_domain_users_sid = (PSID)&domain_users_sid; const PSID security_high_label_sid = (PSID)&high_label_sid; static luid_t prev_luid_value = { 1000, 0 }; -- 2.17.0
From: "Erich E. Hoover" <erich.e.hoover(a)gmail.com> Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=15980 From: Erich E. Hoover <erich.e.hoover(a)gmail.com> Signed-off-by: Vijay Kiran Kamuju <infyquest(a)gmail.com> --- dlls/advapi32/tests/security.c | 50 ++++++++++++++++++++++++++++++++++ server/process.c | 28 +++++++++++++++++-- 2 files changed, 76 insertions(+), 2 deletions(-) diff --git a/dlls/advapi32/tests/security.c b/dlls/advapi32/tests/security.c index 3097a64fb24..1c26228046e 100644 --- a/dlls/advapi32/tests/security.c +++ b/dlls/advapi32/tests/security.c @@ -4687,10 +4687,12 @@ static void test_GetSecurityInfo(void) char admin_ptr[sizeof(SID)+sizeof(ULONG)*SID_MAX_SUB_AUTHORITIES], dacl[100]; PSID domain_users_sid = (PSID) domain_users_ptr, domain_sid; SID_IDENTIFIER_AUTHORITY sia = { SECURITY_NT_AUTHORITY }; + int domain_users_ace_id = -1, admins_ace_id = -1, i; DWORD sid_size = sizeof(admin_ptr), l = sizeof(b); PSID admin_sid = (PSID) admin_ptr, user_sid; char sd[SECURITY_DESCRIPTOR_MIN_LENGTH]; BOOL owner_defaulted, group_defaulted; + BOOL dacl_defaulted, dacl_present; ACL_SIZE_INFORMATION acl_size; PSECURITY_DESCRIPTOR pSD; ACCESS_ALLOWED_ACE *ace; @@ -4698,6 +4700,7 @@ static void test_GetSecurityInfo(void) PSID owner, group; BOOL bret = TRUE; PACL pDacl; + BYTE flags; DWORD ret; if (!pGetSecurityInfo || !pSetSecurityInfo) @@ -4848,6 +4851,53 @@ static void test_GetSecurityInfo(void) ok(group != NULL, "group should not be NULL\n"); ok(EqualSid(group, domain_users_sid), "Process group SID != Domain Users SID.\n"); LocalFree(pSD); + + /* Test querying the DACL of a process */ + ret = pGetSecurityInfo(GetCurrentProcess(), SE_KERNEL_OBJECT, DACL_SECURITY_INFORMATION, + NULL, NULL, NULL, NULL, &pSD); + ok(!ret, "GetSecurityInfo failed with error %d\n", ret); + + bret = GetSecurityDescriptorDacl(pSD, &dacl_present, &pDacl, &dacl_defaulted); + ok(bret, "GetSecurityDescriptorDacl failed with error %d\n", GetLastError()); + ok(dacl_present, "DACL should be present\n"); + ok(pDacl && IsValidAcl(pDacl), "GetSecurityDescriptorDacl returned invalid DACL.\n"); + bret = pGetAclInformation(pDacl, &acl_size, sizeof(acl_size), AclSizeInformation); + ok(bret, "GetAclInformation failed\n"); + ok(acl_size.AceCount != 0, "GetAclInformation returned no ACLs\n"); + for (i=0; i<acl_size.AceCount; i++) + { + bret = pGetAce(pDacl, i, (VOID **)&ace); + ok(bret, "Failed to get ACE %d.\n", i); + bret = EqualSid(&ace->SidStart, domain_users_sid); + if (bret) domain_users_ace_id = i; + bret = EqualSid(&ace->SidStart, admin_sid); + if (bret) admins_ace_id = i; + } + ok(domain_users_ace_id != -1 || broken(domain_users_ace_id == -1) /* win2k */, + "Domain Users ACE not found.\n"); + if (domain_users_ace_id != -1) + { + bret = pGetAce(pDacl, domain_users_ace_id, (VOID **)&ace); + ok(bret, "Failed to get Domain Users ACE.\n"); + flags = ((ACE_HEADER *)ace)->AceFlags; + ok(flags == (INHERIT_ONLY_ACE|CONTAINER_INHERIT_ACE), + "Domain Users ACE has unexpected flags (0x%x != 0x%x)\n", flags, + INHERIT_ONLY_ACE|CONTAINER_INHERIT_ACE); + ok(ace->Mask == GENERIC_READ, "Domain Users ACE has unexpected mask (0x%x != 0x%x)\n", + ace->Mask, GENERIC_READ); + } + ok(admins_ace_id != -1 || broken(admins_ace_id == -1) /* xp */, + "Builtin Admins ACE not found.\n"); + if (admins_ace_id != -1) + { + bret = pGetAce(pDacl, admins_ace_id, (VOID **)&ace); + ok(bret, "Failed to get Builtin Admins ACE.\n"); + flags = ((ACE_HEADER *)ace)->AceFlags; + ok(flags == 0x0, "Builtin Admins ACE has unexpected flags (0x%x != 0x0)\n", flags); + ok(ace->Mask == PROCESS_ALL_ACCESS || broken(ace->Mask == 0x1f0fff) /* win2k */, + "Builtin Admins ACE has unexpected mask (0x%x != 0x%x)\n", ace->Mask, PROCESS_ALL_ACCESS); + } + LocalFree(pSD); } static void test_GetSidSubAuthority(void) diff --git a/server/process.c b/server/process.c index f7af438b561..e9090ec12b2 100644 --- a/server/process.c +++ b/server/process.c @@ -672,15 +672,39 @@ static struct security_descriptor *process_get_sd( struct object *obj ) { size_t users_sid_len = security_sid_len( security_domain_users_sid ); size_t admins_sid_len = security_sid_len( security_builtin_admins_sid ); + size_t dacl_len = sizeof(ACL) + 2 * offsetof( ACCESS_ALLOWED_ACE, SidStart ) + + users_sid_len + admins_sid_len; + ACCESS_ALLOWED_ACE *aaa; + ACL *dacl; - key_default_sd = mem_alloc( sizeof(*key_default_sd) + admins_sid_len + users_sid_len ); + key_default_sd = mem_alloc( sizeof(*key_default_sd) + admins_sid_len + users_sid_len + + dacl_len ); key_default_sd->control = SE_DACL_PRESENT; key_default_sd->owner_len = admins_sid_len; key_default_sd->group_len = users_sid_len; key_default_sd->sacl_len = 0; - key_default_sd->dacl_len = 0; + key_default_sd->dacl_len = dacl_len; memcpy( key_default_sd + 1, security_builtin_admins_sid, admins_sid_len ); memcpy( (char *)(key_default_sd + 1) + admins_sid_len, security_domain_users_sid, users_sid_len ); + + dacl = (ACL *)((char *)(key_default_sd + 1) + admins_sid_len + users_sid_len); + dacl->AclRevision = ACL_REVISION; + dacl->Sbz1 = 0; + dacl->AclSize = dacl_len; + dacl->AceCount = 2; + dacl->Sbz2 = 0; + aaa = (ACCESS_ALLOWED_ACE *)(dacl + 1); + aaa->Header.AceType = ACCESS_ALLOWED_ACE_TYPE; + aaa->Header.AceFlags = INHERIT_ONLY_ACE | CONTAINER_INHERIT_ACE; + aaa->Header.AceSize = offsetof( ACCESS_ALLOWED_ACE, SidStart ) + users_sid_len; + aaa->Mask = GENERIC_READ; + memcpy( &aaa->SidStart, security_domain_users_sid, users_sid_len ); + aaa = (ACCESS_ALLOWED_ACE *)((char *)aaa + aaa->Header.AceSize); + aaa->Header.AceType = ACCESS_ALLOWED_ACE_TYPE; + aaa->Header.AceFlags = 0; + aaa->Header.AceSize = offsetof( ACCESS_ALLOWED_ACE, SidStart ) + admins_sid_len; + aaa->Mask = PROCESS_ALL_ACCESS; + memcpy( &aaa->SidStart, security_builtin_admins_sid, admins_sid_len ); } return key_default_sd; } -- 2.17.0
Signed-off-by: Erich E. Hoover <erich.e.hoover(a)gmail.com>
Vijay Kiran Kamuju <infyquest(a)gmail.com> writes:
+static struct security_descriptor *process_get_sd( struct object *obj ) +{ + static struct security_descriptor *key_default_sd; + + if (obj->sd) return obj->sd; + + if (!key_default_sd) + { + size_t users_sid_len = security_sid_len( security_domain_users_sid ); + size_t admins_sid_len = security_sid_len( security_builtin_admins_sid ); + + key_default_sd = mem_alloc( sizeof(*key_default_sd) + admins_sid_len + users_sid_len ); + key_default_sd->control = SE_DACL_PRESENT; + key_default_sd->owner_len = admins_sid_len; + key_default_sd->group_len = users_sid_len; + key_default_sd->sacl_len = 0; + key_default_sd->dacl_len = 0; + memcpy( key_default_sd + 1, security_builtin_admins_sid, admins_sid_len ); + memcpy( (char *)(key_default_sd + 1) + admins_sid_len, security_domain_users_sid, users_sid_len ); + } + return key_default_sd;
key_default_sd is clearly not the appropriate name here. -- Alexandre Julliard julliard(a)winehq.org
On Mon, Apr 22, 2019 at 11:13 AM Alexandre Julliard <julliard(a)winehq.org> wrote:
Vijay Kiran Kamuju <infyquest(a)gmail.com> writes:
+static struct security_descriptor *process_get_sd( struct object *obj ) +{ + static struct security_descriptor *key_default_sd; + + if (obj->sd) return obj->sd; + + if (!key_default_sd) + { + size_t users_sid_len = security_sid_len( security_domain_users_sid ); + size_t admins_sid_len = security_sid_len( security_builtin_admins_sid ); + + key_default_sd = mem_alloc( sizeof(*key_default_sd) + admins_sid_len + users_sid_len ); + key_default_sd->control = SE_DACL_PRESENT; + key_default_sd->owner_len = admins_sid_len; + key_default_sd->group_len = users_sid_len; + key_default_sd->sacl_len = 0; + key_default_sd->dacl_len = 0; + memcpy( key_default_sd + 1, security_builtin_admins_sid, admins_sid_len ); + memcpy( (char *)(key_default_sd + 1) + admins_sid_len, security_domain_users_sid, users_sid_len ); + } + return key_default_sd;
key_default_sd is clearly not the appropriate name here.
Sorry about that, I clearly started with the registry example (key_get_sd). Would you prefer process_default_sd or just default_sd? Best, Erich
"Erich E. Hoover" <erich.e.hoover(a)gmail.com> writes:
On Mon, Apr 22, 2019 at 11:13 AM Alexandre Julliard <julliard(a)winehq.org> wrote:
Vijay Kiran Kamuju <infyquest(a)gmail.com> writes:
+static struct security_descriptor *process_get_sd( struct object *obj ) +{ + static struct security_descriptor *key_default_sd; + + if (obj->sd) return obj->sd; + + if (!key_default_sd) + { + size_t users_sid_len = security_sid_len( security_domain_users_sid ); + size_t admins_sid_len = security_sid_len( security_builtin_admins_sid ); + + key_default_sd = mem_alloc( sizeof(*key_default_sd) + admins_sid_len + users_sid_len ); + key_default_sd->control = SE_DACL_PRESENT; + key_default_sd->owner_len = admins_sid_len; + key_default_sd->group_len = users_sid_len; + key_default_sd->sacl_len = 0; + key_default_sd->dacl_len = 0; + memcpy( key_default_sd + 1, security_builtin_admins_sid, admins_sid_len ); + memcpy( (char *)(key_default_sd + 1) + admins_sid_len, security_domain_users_sid, users_sid_len ); + } + return key_default_sd;
key_default_sd is clearly not the appropriate name here.
Sorry about that, I clearly started with the registry example (key_get_sd). Would you prefer process_default_sd or just default_sd?
Either is fine. -- Alexandre Julliard julliard(a)winehq.org
On Mon, Apr 22, 2019 at 7:37 PM Alexandre Julliard <julliard(a)winehq.org> wrote:
"Erich E. Hoover" <erich.e.hoover(a)gmail.com> writes:
On Mon, Apr 22, 2019 at 11:13 AM Alexandre Julliard <julliard(a)winehq.org> wrote:
Vijay Kiran Kamuju <infyquest(a)gmail.com> writes:
+static struct security_descriptor *process_get_sd( struct object *obj ) +{ + static struct security_descriptor *key_default_sd; + + if (obj->sd) return obj->sd; + + if (!key_default_sd) + { + size_t users_sid_len = security_sid_len( security_domain_users_sid ); + size_t admins_sid_len = security_sid_len( security_builtin_admins_sid ); + + key_default_sd = mem_alloc( sizeof(*key_default_sd) + admins_sid_len + users_sid_len ); + key_default_sd->control = SE_DACL_PRESENT; + key_default_sd->owner_len = admins_sid_len; + key_default_sd->group_len = users_sid_len; + key_default_sd->sacl_len = 0; + key_default_sd->dacl_len = 0; + memcpy( key_default_sd + 1, security_builtin_admins_sid, admins_sid_len ); + memcpy( (char *)(key_default_sd + 1) + admins_sid_len, security_domain_users_sid, users_sid_len ); + } + return key_default_sd;
key_default_sd is clearly not the appropriate name here.
Sorry about that, I clearly started with the registry example (key_get_sd). Would you prefer process_default_sd or just default_sd?
Either is fine.
+1 for process_default_sd
-- Alexandre Julliard julliard(a)winehq.org
participants (3)
-
Alexandre Julliard -
Erich E. Hoover -
Vijay Kiran Kamuju