[SSSD] [PATCH] SELINUX: Set permissive umask for selinux operations

Sumit Bose sbose at redhat.com
Tue Jan 27 09:20:17 UTC 2015


On Mon, Jan 26, 2015 at 06:18:26PM +0100, Jakub Hrozek wrote:
> Hi,
> 
> to reproduce, set a SELinux label for a user. Log in as the user and
> observer the permissions of /etc/selinux/targeted/modules/active
> changing to 0600. As a result, even local semanage tools didn't work..

> From dbe42c9ca20df500d09949512b86c86f7b4ce7ce Mon Sep 17 00:00:00 2001
> From: Jakub Hrozek <jhrozek at redhat.com>
> Date: Mon, 26 Jan 2015 16:55:42 +0100
> Subject: [PATCH] SELINUX: Set permissive umask for selinux operations
> 
> https://fedorahosted.org/sssd/ticket/2563
> 
> libsemanage calls mkdir() and then requires that the directory is created
> with permissions 0700. That doesn't work well for programs like sssd that
> set umask to a very restrictive value (like 177).
> ---
>  src/util/sss_semanage.c | 29 ++++++++++++++++++++++++++---
>  1 file changed, 26 insertions(+), 3 deletions(-)
> 
> diff --git a/src/util/sss_semanage.c b/src/util/sss_semanage.c
> index 3c566553f2085a696f79c5ee35ec6015824d56a6..4de0ce2bb4d52852cc7fa4df8a3e92b667c27c81 100644
> --- a/src/util/sss_semanage.c
> +++ b/src/util/sss_semanage.c
> @@ -216,8 +216,8 @@ done:
>      return ret;
>  }
>  
> -int set_seuser(const char *login_name, const char *seuser_name,
> -               const char *mls)
> +int _set_seuser(const char *login_name, const char *seuser_name,
> +                const char *mls)

would it be possilbe to make _set_seuser static as _del_seuser?

>  {
>      semanage_handle_t *handle = NULL;
>      semanage_seuser_key_t *key = NULL;
> @@ -282,7 +282,19 @@ done:
>      return ret;
>  }
>  
> -int del_seuser(const char *login_name)
> +int set_seuser(const char *login_name, const char *seuser_name,
> +               const char *mls)
> +{
> +    mode_t old_mask;
> +    int ret;
> +
> +    old_mask = umask(0);
> +    ret = _set_seuser(login_name, seuser_name, mls);
> +    umask(old_mask);

just a comment, please note that umask() will change the umask of the
whole process. With asynchronous processing we should always make sure
to change the umask only where it is needed.

bye,
Sumit

> +    return ret;
> +}
> +
> +static int _del_seuser(const char *login_name)
>  {
>      semanage_handle_t *handle = NULL;
>      semanage_seuser_key_t *key = NULL;
> @@ -353,6 +365,17 @@ done:
>      return ret;
>  }
>  
> +int del_seuser(const char *login_name)
> +{
> +    mode_t old_mask;
> +    int ret;
> +
> +    old_mask = umask(0);
> +    ret = _del_seuser(login_name);
> +    umask(old_mask);
> +    return ret;
> +}
> +
>  #else /* HAVE_SEMANAGE */
>  int set_seuser(const char *login_name, const char *seuser_name,
>                 const char *mls)
> -- 
> 2.1.0
> 

> _______________________________________________
> sssd-devel mailing list
> sssd-devel at lists.fedorahosted.org
> https://lists.fedorahosted.org/mailman/listinfo/sssd-devel




More information about the sssd-devel mailing list