[systemd-devel] [PATCH] sysusers: Preserve label of /etc/{passwd, group}

[Zbigniew Jędrzejewski-Szmek]

On Fri, Jul 11, 2014 at 02:23:43PM -0700, Colin Walters wrote:
> These files are specially labeled on SELinux systems, and we need to
> preserve that label.
> ---
>  src/shared/label.c      | 25 +++++++++++++++++++++++++
>  src/shared/label.h      |  1 +
>  src/sysusers/sysusers.c | 11 ++++++++++-
>  3 files changed, 36 insertions(+), 1 deletion(-)
> 

> From 715e1ff352601d841fc0e29ecddd9f0f5ed6fe46 Mon Sep 17 00:00:00 2001
> From: Colin Walters <walters at verbum.org>
> Date: Fri, 11 Jul 2014 15:03:29 -0400
> Subject: [PATCH] sysusers: Preserve label of /etc/{passwd,group}
> 
> These files are specially labeled on SELinux systems, and we need to
> preserve that label.
> ---
>  src/shared/label.c      | 25 +++++++++++++++++++++++++
>  src/shared/label.h      |  1 +
>  src/sysusers/sysusers.c | 11 ++++++++++-
>  3 files changed, 36 insertions(+), 1 deletion(-)
> 
> diff --git a/src/shared/label.c b/src/shared/label.c
> index 25a8b36..702b6d9 100644
> --- a/src/shared/label.c
> +++ b/src/shared/label.c
> @@ -272,6 +272,31 @@ int label_context_set(const char *path, mode_t mode) {
>          return r;
>  }
Hi,

please excuse my possibly ignorant questions, selinux is not my forte.
If the files are nonexistent, will this fail? But sysusers should be
able to create /etc from scratch.

Why cannot the same code as in write_string_file_atomic_label be used
instead?

> +int label_context_set_from_existing(const char *path) {
> +        int r = 0;
> +
> +#ifdef HAVE_SELINUX
> +        security_context_t con = NULL;
> +
> +        if (!use_selinux())
> +                return 0;
> +
> +        r = getfilecon(path, &con);
> +        if (r < 0)
> +                return r;
BTW, you're returning -1 here, but should be -errno.

> +
> +        r = setfscreatecon(con);
> +        if (r < 0)
> +                goto finish;
> +
> + finish:
And this label is not useful.

> +        freecon(con);
> +#endif
> +
> +        return r;
> +
> +}
> +
>  int label_socket_set(const char *label) {
>  
>  #ifdef HAVE_SELINUX
> diff --git a/src/shared/label.h b/src/shared/label.h
> index 7294820..4546784 100644
> --- a/src/shared/label.h
> +++ b/src/shared/label.h
> @@ -34,6 +34,7 @@ int label_socket_set(const char *label);
>  void label_socket_clear(void);
>  
>  int label_context_set(const char *path, mode_t mode);
> +int label_context_set_from_existing(const char *path);
>  void label_context_clear(void);
>  
>  void label_free(const char *label);
> diff --git a/src/sysusers/sysusers.c b/src/sysusers/sysusers.c
> index 68c552d..ecd611c 100644
> --- a/src/sysusers/sysusers.c
> +++ b/src/sysusers/sysusers.c
> @@ -312,7 +312,11 @@ static int write_files(void) {
>                  _cleanup_fclose_ FILE *original = NULL;
>  
>                  group_path = fix_root("/etc/group");
> +                r = label_context_set_from_existing(group_path);
> +                if (r < 0)
> +                        goto finish;
>                  r = fopen_temporary(group_path, &group, &group_tmp);
> +                label_context_clear();
>                  if (r < 0)
>                          goto finish;
>  
> @@ -388,9 +392,14 @@ static int write_files(void) {
>                  _cleanup_fclose_ FILE *original = NULL;
>  
>                  passwd_path = fix_root("/etc/passwd");
> -                r = fopen_temporary(passwd_path, &passwd, &passwd_tmp);
> +                r = label_context_set_from_existing(passwd_path);
>                  if (r < 0)
>                          goto finish;
> +                r = fopen_temporary(passwd_path, &passwd, &passwd_tmp);
> +                label_context_clear();
> +                if (r < 0) {
> +                        goto finish;
> +                }
>  
>                  if (fchmod(fileno(passwd), 0644) < 0) {
>                          r = -errno;

Zbyszek