[PATCH 0/2] MR481: server: Use the token owner instead of the token user for default object owner.

[Jinoh Kang (＠iamahuman)]

Also use domain_users_sid for the default DACL, since Wine currently
chooses domain_users_sid as the current token owner.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/481

[PATCH 1/2] advapi32/tests: Test that default object owner and group match the token.

[Jinoh Kang]

From: Jinoh Kang <jinoh.kang.kr(a)gmail.com>

---
 dlls/advapi32/tests/security.c | 50 ++++++++++++++++++++++++++++++----
 1 file changed, 44 insertions(+), 6 deletions(-)

diff --git a/dlls/advapi32/tests/security.c b/dlls/advapi32/tests/security.c
index aeee279a86d..d1e9c00c154 100644
--- a/dlls/advapi32/tests/security.c
+++ b/dlls/advapi32/tests/security.c
@@ -6304,16 +6304,41 @@ static void test_TokenIntegrityLevel(void)
     CloseHandle(token);
 }
 
-static void test_default_dacl_owner_sid(void)
+static void test_default_dacl_owner_group_sid(void)
 {
-    HANDLE handle;
+    HANDLE handle, token;
     BOOL ret, defaulted, present, found;
     DWORD size, index;
     SECURITY_DESCRIPTOR *sd;
     SECURITY_ATTRIBUTES sa;
-    PSID owner;
+    PSID owner, group;
     ACL *dacl;
     ACCESS_ALLOWED_ACE *ace;
+    TOKEN_OWNER *token_owner;
+    TOKEN_PRIMARY_GROUP *token_primary_group;
+
+    ret = OpenProcessToken( GetCurrentProcess(), TOKEN_QUERY, &token );
+    ok(ret, "OpenProcessToken failed with error %ld\n", GetLastError());
+
+    ret = GetTokenInformation( token, TokenOwner, NULL, 0, &size );
+    ok(!ret, "Expected failure, got %d\n", ret);
+    ok(GetLastError() == ERROR_INSUFFICIENT_BUFFER,
+       "Expected ERROR_INSUFFICIENT_BUFFER, got %ld\n", GetLastError());
+
+    token_owner = HeapAlloc(GetProcessHeap(), 0, size);
+    ret = GetTokenInformation( token, TokenOwner, token_owner, size, &size );
+    ok(ret, "GetTokenInformation failed with error %ld\n", GetLastError());
+
+    ret = GetTokenInformation( token, TokenPrimaryGroup, NULL, 0, &size );
+    ok(!ret, "Expected failure, got %d\n", ret);
+    ok(GetLastError() == ERROR_INSUFFICIENT_BUFFER,
+       "Expected ERROR_INSUFFICIENT_BUFFER, got %ld\n", GetLastError());
+
+    token_primary_group = HeapAlloc(GetProcessHeap(), 0, size);
+    ret = GetTokenInformation( token, TokenPrimaryGroup, token_primary_group, size, &size );
+    ok(ret, "GetTokenInformation failed with error %ld\n", GetLastError());
+
+    CloseHandle( token );
 
     sd = HeapAlloc( GetProcessHeap(), 0, SECURITY_DESCRIPTOR_MIN_LENGTH );
     ret = InitializeSecurityDescriptor( sd, SECURITY_DESCRIPTOR_REVISION );
@@ -6326,11 +6351,11 @@ static void test_default_dacl_owner_sid(void)
     ok( handle != NULL, "error %lu\n", GetLastError() );
 
     size = 0;
-    ret = GetKernelObjectSecurity( handle, OWNER_SECURITY_INFORMATION|DACL_SECURITY_INFORMATION, NULL, 0, &size );
+    ret = GetKernelObjectSecurity( handle, OWNER_SECURITY_INFORMATION|GROUP_SECURITY_INFORMATION|DACL_SECURITY_INFORMATION, NULL, 0, &size );
     ok( !ret && GetLastError() == ERROR_INSUFFICIENT_BUFFER, "error %lu\n", GetLastError() );
 
     sd = HeapAlloc( GetProcessHeap(), 0, size );
-    ret = GetKernelObjectSecurity( handle, OWNER_SECURITY_INFORMATION|DACL_SECURITY_INFORMATION, sd, size, &size );
+    ret = GetKernelObjectSecurity( handle, OWNER_SECURITY_INFORMATION|GROUP_SECURITY_INFORMATION|DACL_SECURITY_INFORMATION, sd, size, &size );
     ok( ret, "error %lu\n", GetLastError() );
 
     owner = (void *)0xdeadbeef;
@@ -6339,6 +6364,16 @@ static void test_default_dacl_owner_sid(void)
     ok( ret, "error %lu\n", GetLastError() );
     ok( owner != (void *)0xdeadbeef, "owner not set\n" );
     ok( !defaulted, "owner defaulted\n" );
+    todo_wine
+    ok( EqualSid( owner, token_owner->Owner ), "owner shall equal token owner\n" );
+
+    group = (void *)0xdeadbeef;
+    defaulted = TRUE;
+    ret = GetSecurityDescriptorGroup( sd, &group, &defaulted );
+    ok( ret, "error %lu\n", GetLastError() );
+    ok( group != (void *)0xdeadbeef, "group not set\n" );
+    ok( !defaulted, "group defaulted\n" );
+    ok( EqualSid( group, token_primary_group->PrimaryGroup ), "group shall equal token primary group\n" );
 
     dacl = (void *)0xdeadbeef;
     present = FALSE;
@@ -6360,6 +6395,9 @@ static void test_default_dacl_owner_sid(void)
     HeapFree( GetProcessHeap(), 0, sa.lpSecurityDescriptor );
     HeapFree( GetProcessHeap(), 0, sd );
     CloseHandle( handle );
+
+    HeapFree( GetProcessHeap(), 0, token_primary_group );
+    HeapFree( GetProcessHeap(), 0, token_owner );
 }
 
 static void test_AdjustTokenPrivileges(void)
@@ -8502,7 +8540,7 @@ START_TEST(security)
     test_GetUserNameW();
     test_CreateRestrictedToken();
     test_TokenIntegrityLevel();
-    test_default_dacl_owner_sid();
+    test_default_dacl_owner_group_sid();
     test_AdjustTokenPrivileges();
     test_AddAce();
     test_AddMandatoryAce();
-- 
GitLab


https://gitlab.winehq.org/wine/wine/-/merge_requests/481

[PATCH 2/2] server: Use the token owner instead of the token user for default object owner.

[Jinoh Kang]

From: Jinoh Kang <jinoh.kang.kr(a)gmail.com>

Also use domain_users_sid for the default DACL, since Wine currently
chooses domain_users_sid as the current token owner.
---
 dlls/advapi32/tests/security.c | 1 -
 server/change.c                | 2 +-
 server/file.c                  | 4 ++--
 server/object.c                | 2 +-
 server/security.h              | 2 +-
 server/token.c                 | 6 +++---
 6 files changed, 8 insertions(+), 9 deletions(-)

diff --git a/dlls/advapi32/tests/security.c b/dlls/advapi32/tests/security.c
index d1e9c00c154..7795b0c462b 100644
--- a/dlls/advapi32/tests/security.c
+++ b/dlls/advapi32/tests/security.c
@@ -6364,7 +6364,6 @@ static void test_default_dacl_owner_group_sid(void)
     ok( ret, "error %lu\n", GetLastError() );
     ok( owner != (void *)0xdeadbeef, "owner not set\n" );
     ok( !defaulted, "owner defaulted\n" );
-    todo_wine
     ok( EqualSid( owner, token_owner->Owner ), "owner shall equal token owner\n" );
 
     group = (void *)0xdeadbeef;
diff --git a/server/change.c b/server/change.c
index 6477b457f74..7a806abc017 100644
--- a/server/change.c
+++ b/server/change.c
@@ -391,7 +391,7 @@ static int dir_set_sd( struct object *obj, const struct security_descriptor *sd,
     else if (obj->sd)
         owner = sd_get_owner( obj->sd );
     else
-        owner = token_get_user( current->process->token );
+        owner = token_get_owner( current->process->token );
 
     if (set_info & DACL_SECURITY_INFORMATION)
     {
diff --git a/server/file.c b/server/file.c
index eb2dc5696ed..76c687833c9 100644
--- a/server/file.c
+++ b/server/file.c
@@ -245,7 +245,7 @@ static struct object *create_file( struct fd *root, const char *nameptr, data_si
     {
         const struct sid *owner = sd_get_owner( sd );
         if (!owner)
-            owner = token_get_user( current->process->token );
+            owner = token_get_owner( current->process->token );
         mode = sd_to_mode( sd, owner );
     }
     else if (options & FILE_DIRECTORY_FILE)
@@ -528,7 +528,7 @@ static int file_set_sd( struct object *obj, const struct security_descriptor *sd
     else if (obj->sd)
         owner = sd_get_owner( obj->sd );
     else
-        owner = token_get_user( current->process->token );
+        owner = token_get_owner( current->process->token );
 
     /* group and sacl not supported */
 
diff --git a/server/object.c b/server/object.c
index 333f9e7b5d6..89e541ffb6b 100644
--- a/server/object.c
+++ b/server/object.c
@@ -574,7 +574,7 @@ int set_sd_defaults_from_token( struct object *obj, const struct security_descri
     }
     else if (token)
     {
-        owner = token_get_user( token );
+        owner = token_get_owner( token );
         new_sd.owner_len = sid_len( owner );
     }
     else new_sd.owner_len = 0;
diff --git a/server/security.h b/server/security.h
index fa91b81b77c..58ab1594eae 100644
--- a/server/security.h
+++ b/server/security.h
@@ -73,7 +73,7 @@ extern int token_check_privileges( struct token *token, int all_required,
                                    const struct luid_attr *reqprivs,
                                    unsigned int count, struct luid_attr *usedprivs );
 extern const struct acl *token_get_default_dacl( struct token *token );
-extern const struct sid *token_get_user( struct token *token );
+extern const struct sid *token_get_owner( struct token *token );
 extern const struct sid *token_get_primary_group( struct token *token );
 extern unsigned int token_get_session_id( struct token *token );
 extern int token_sid_present( struct token *token, const struct sid *sid, int deny );
diff --git a/server/token.c b/server/token.c
index f817c1114f8..99f5e36e279 100644
--- a/server/token.c
+++ b/server/token.c
@@ -732,7 +732,7 @@ struct token *token_create_admin( unsigned primary, int impersonation_level, int
     /* on Windows, this value changes every time the user logs on */
     struct sid logon_sid = { SID_REVISION, 3, SECURITY_NT_AUTHORITY, { SECURITY_LOGON_IDS_RID, 0, 0 /* FIXME: should be randomly generated when tokens are inherited by new processes */ }};
     const struct sid *user_sid = security_unix_uid_to_sid( getuid() );
-    struct acl *default_dacl = create_default_dacl( user_sid );
+    struct acl *default_dacl = create_default_dacl( &domain_users_sid );
     const struct luid_attr admin_privs[] =
     {
         { SeChangeNotifyPrivilege, SE_PRIVILEGE_ENABLED },
@@ -1044,9 +1044,9 @@ const struct acl *token_get_default_dacl( struct token *token )
     return token->default_dacl;
 }
 
-const struct sid *token_get_user( struct token *token )
+const struct sid *token_get_owner( struct token *token )
 {
-    return token->user;
+    return token->owner;
 }
 
 const struct sid *token_get_primary_group( struct token *token )
-- 
GitLab

https://gitlab.winehq.org/wine/wine/-/merge_requests/481