[prev in list] [next in list] [prev in thread] [next in thread] 

List:       busybox
Subject:    Re: [PATCH 1/1] copy_file(): Revise completion of SELinux security context preserve/set.
From:       Xabier Oneca  --  xOneca <xoneca () gmail ! com>
Date:       2020-03-25 22:24:46
Message-ID: CACkgH7301iC9HOXJOxeLz=EAf86u7EgPu-FTigU5t2nnmpkkQw () mail ! gmail ! com
[Download RAW message or body]

Hi Chris,

Thanks for the contribution.

Just one comment: please use bb_simple_perror_msg() instead of
bb_perror_msg() for single parameter uses (see commit:
https://git.busybox.net/busybox/commit/?id=0c97c9d43707)

Cheers,

> The existing setfscreatecon() at the beginning of copy_file() is the secure
> method for setting the context of new files, but it doesn't apply to
> existing files. Change the setfilecon() to only run on preexisting files.
>
> Signed-off-by: Chris PeBenito <chpebeni@linux.microsoft.com>
> ---
>  libbb/copy_file.c | 25 ++++++++++++++-----------
>  1 file changed, 14 insertions(+), 11 deletions(-)
>
> diff --git a/libbb/copy_file.c b/libbb/copy_file.c
> index 49d1ec9c6..37faa8dc6 100644
> --- a/libbb/copy_file.c
> +++ b/libbb/copy_file.c
> @@ -325,19 +325,22 @@ int FAST_FUNC copy_file(const char *source, const char *dest, int flags)
>                 if ((flags & (FILEUTILS_PRESERVE_SECURITY_CONTEXT|FILEUTILS_SET_SECURITY_CONTEXT))
>                  && is_selinux_enabled() > 0
>                 ) {
> +                       /* Failure to preserve the security context isn't fatal here since
> +                        * the copy has been done at this point. */
>                         security_context_t con;
> -                       if (getfscreatecon(&con) == -1) {
> -                               bb_simple_perror_msg("getfscreatecon");
> -                               return -1;
> -                       }
> -                       if (con) {
> -                               if (setfilecon(dest, con) == -1) {
> +                       if (getfscreatecon(&con) < 0)
> +                               bb_perror_msg("getfscreatecon");
> +
> +                       if (setfscreatecon(NULL) < 0)
> +                               bb_perror_msg("can't reset fscreate");
> +
> +                       /* setfscreatecon() only works when a file is created. If dest
> +                        * preexisted, use setfilecon instead */
> +                       if (con && dest_exists)
> +                               if (setfilecon(dest, con) < 0)
>                                         bb_perror_msg("setfilecon:%s,%s", dest, con);
> -                                       freecon(con);
> -                                       return -1;
> -                               }
> -                               freecon(con);
> -                       }
> +
> +                       freecon(con);
>                 }
>  #endif
>  #if ENABLE_FEATURE_CP_REFLINK
> --
> 2.21.1
_______________________________________________
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox
[prev in list] [next in list] [prev in thread] [next in thread]