-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
Rob Shearman wrote:
While I agree that there is a problem that needs to be fixed, I'm not sure this is the right approach. I think you need to take a step back and consider the meanings of the different SIDs in a token by default and how they map wine running inside the Unix permissions model. For example, maybe these mappings make sense:
security_local_sid -> user + group + others security_interactive_sid -> user + group + others alias_users_sid -> user + group + others?
Now it's likely that the bugs you are trying to fix are trying to set the SD for a file to alias_admins_sid or alias_users_sid. The mapping for alias_admins_sid is less clear - one could argue that all Wine users on a given system would present themselves as admins to apps, but then again the apps may be restricting permissions on a file because it contains sensitive data and should only be shared with other admins (which would be trusted as such, unlike other users on a system).
Currently, the wine NT DACL -> unix permission code works as follows: * The permissions start as deny all. * If the world SID is granted a permission, then that permission is added to both owner and others. * If the owner SID is explicitly granted a permission, then that permission is added to owner. * If the world SID is denied a permission, then that permission is removed from both owner and others. * If the owner SID is explicitly denied a permission, then that permission is removed from owner.
This means that if neither owner nor world are listed in the NT DACL, the owner of the file is denied all permissions on that file, even if they would have been granted access by one of their groups.
Windows Access Control Lists act as a series of grant/deny Access Control Entries, and the entire Access Control List must be consulted to determine the rights of the user. If none of the user, the user's groups or the world SID are mentioned in the DACL, then that user is denied all permissions. No SIDs are treated specially (not even SYSTEM).
Unix permissions and Access Control Lists act as a set of definitive Access Control Entries, and the first Access Control Entry that applies to the user or one of their groups is used to determine the rights of the user. The superuser is treated specially, and is always granted permission (unless squashed by the filesystem or a security module).
As the owner permission is consulted before any entries in the ACL, the owner permission cannot be overridden by an entry in the ACL.
I have modified my patch to only check the groups in the process token if the owner of the process is the owner of the file.
This patch: * in server/token.c: * exposes token_sid_present(), which tests whether an SID is present in the token. * in server/file.c::sd_to_mode(): * Sets the local boolean variable user_is_owner if the owner of the process and the owner of the file are the same. * Denies permissions from the owner of the file if user_is_owner is true and the SID denying permissions is present in the token. * Grants permissions to the owner of the file if user_is_owner is true and the SID granting permissions is present in the token, and is not a deny-only SID.
The patch seeks to ensure that the owner of the file is granted and/or denied permissions based on the permissions of their groups. At the moment, it only does this if the owner of the file is also the owner of the process, as I don't know how to look up the groups of other users.
The only other way I can see of doing this is to pre-process the SID and add a grant and/or deny ACE for the owner based on the access given by their groups.
server/file.c | 7 +++++-- server/security.h | 1 + server/token.c | 2 +- 3 files changed, 7 insertions(+), 3 deletions(-)
diff --git a/server/file.c b/server/file.c index a74de14..db9dd5f 100644 --- a/server/file.c +++ b/server/file.c @@ -461,6 +461,7 @@ mode_t sd_to_mode( const struct security_descriptor *sd, const SID *owner ) if (present && dacl) { const ACE_HEADER *ace = (const ACE_HEADER *)(dacl + 1); + int user_is_owner = security_equal_sid( owner, token_get_user( current->process->token) ); ULONG i; for (i = 0; i < dacl->AceCount; i++, ace = ace_next( ace )) { @@ -485,7 +486,8 @@ mode_t sd_to_mode( const struct security_descriptor *sd, const SID *owner ) if (access & FILE_EXECUTE) denied_mode |= S_IXUSR|S_IXGRP|S_IXOTH; } - else if (security_equal_sid( sid, owner )) + else if (security_equal_sid( sid, owner ) || + (user_is_owner && token_sid_present( current->process->token, sid, 1 ))) { unsigned int access = generic_file_map_access( ad_ace->Mask ); if (access & FILE_READ_DATA) @@ -509,7 +511,8 @@ mode_t sd_to_mode( const struct security_descriptor *sd, const SID *owner ) if (access & FILE_EXECUTE) new_mode |= S_IXUSR|S_IXGRP|S_IXOTH; } - else if (security_equal_sid( sid, owner )) + else if (security_equal_sid( sid, owner ) || + (user_is_owner && token_sid_present( current->process->token, sid, 0 ))) { unsigned int access = generic_file_map_access( aa_ace->Mask ); if (access & FILE_READ_DATA) diff --git a/server/security.h b/server/security.h index 39b1d2f..33cf5da 100644 --- a/server/security.h +++ b/server/security.h @@ -55,6 +55,7 @@ extern int token_check_privileges( struct token *token, int all_required, extern const ACL *token_get_default_dacl( struct token *token ); extern const SID *token_get_user( struct token *token ); extern const SID *token_get_primary_group( struct token *token ); +extern int token_sid_present( struct token *token, const SID *sid, int deny);
static inline const ACE_HEADER *ace_next( const ACE_HEADER *ace ) { diff --git a/server/token.c b/server/token.c index ce896ac..461e79d 100644 --- a/server/token.c +++ b/server/token.c @@ -776,7 +776,7 @@ int token_check_privileges( struct token *token, int all_required, return (enabled_count > 0); }
-static int token_sid_present( struct token *token, const SID *sid, int deny ) +int token_sid_present( struct token *token, const SID *sid, int deny ) { struct group *group;