Re: [PATCH 2/2] LSM: SafeSetID: gate setgid transitions

["Serge E. Hallyn" <serge@hallyn.com>]

On Fri, Feb 15, 2019 at 02:22:28PM -0800, mortonm@chromium.org wrote:
> From: Micah Morton <mortonm@chromium.org>
> 
> The SafeSetID LSM already gates setuid transitions for UIDs on the
> system whose use of CAP_SETUID has been 'restricted'. This patch
> implements the analogous functionality for setgid transitions, in order
> to restrict the use of CAP_SETGID for certain UIDs on the system. One
> notable consequence of this addition is that a process running under a
> restricted UID (i.e. one that is only allowed to setgid to certain
> approved GIDs) will not be allowed to call the setgroups() syscall to
> set its supplementary group IDs. For now, we leave such support for
> restricted setgroups() to future work, as it would require hooking the
> logic in setgroups() and verifying that the array of GIDs passed in from
> userspace only consists of approved GIDs.
> 
> Signed-off-by: Micah Morton <mortonm@chromium.org>
> ---
> Tested with slight mod to test in tools/testing/selftests/safesetid for
> testing setgid as well as setuid.
> 
>  security/safesetid/lsm.c        | 263 +++++++++++++++++++++++++++-----
>  security/safesetid/lsm.h        |  11 +-
>  security/safesetid/securityfs.c | 105 +++++++++----
>  3 files changed, 307 insertions(+), 72 deletions(-)
> 
> diff --git a/security/safesetid/lsm.c b/security/safesetid/lsm.c
> index cecd38e2ac80..5d9710b7bb04 100644
> --- a/security/safesetid/lsm.c
> +++ b/security/safesetid/lsm.c
> @@ -26,27 +26,30 @@ int safesetid_initialized;
>  
>  #define NUM_BITS 8 /* 128 buckets in hash table */
...
> +int add_safesetid_whitelist_uid_entry(kuid_t parent, kuid_t child)
>  {
> -	struct entry *new;
> +	struct id_entry *new;
>  
>  	/* Return if entry already exists */
>  	if (check_setuid_policy_hashtable_key_value(parent, child))
>  		return 0;
>  
> -	new = kzalloc(sizeof(struct entry), GFP_KERNEL);
> +	new = kzalloc(sizeof(struct id_entry), GFP_KERNEL);
> +	if (!new)
> +		return -ENOMEM;
> +	new->parent_kuid = __kuid_val(parent);
> +	new->child_kid = __kuid_val(child);
> +	spin_lock(&safesetid_whitelist_uid_hashtable_spinlock);
> +	hash_add_rcu(safesetid_whitelist_uid_hashtable,
> +		     &new->next,
> +		     __kuid_val(parent));

Do you care at all about the possibility of duplicate entries?

> +	spin_unlock(&safesetid_whitelist_uid_hashtable_spinlock);
> +	return 0;
> +}