Re: [PATCH] copy: fix SELinux context preservation for existing directories

[Bernhard Voelker]

On 01/05/2014 04:00 PM, Pádraig Brady wrote:

Hi Padraig,

I don't have a SELinux system here, so I can't add much to this, ...

> diff --git a/tests/cp/cp-a-selinux.sh b/tests/cp/cp-a-selinux.sh
> index 79b1c0a..02e9b63 100755
> --- a/tests/cp/cp-a-selinux.sh
> +++ b/tests/cp/cp-a-selinux.sh
> @@ -41,6 +41,21 @@ test -s err && fail=1   #there must be no stderr output 
> for -a
>  ls -Z e | grep $ctx || fail=1
>  ls -Z f | grep $ctx || fail=1
>  
> +# Check handling of existing dirs which requires specific handling
> +# due to recursion, and was handled incorrectly in coreutils-8.22
> +mkdir -p backup/existing_dir/ || framework_failure_
> +ls -Zd backup/existing_dir | grep $ctx && framework_failure_
> +touch backup/existing_dir/file || framework_failure_
> +chcon $ctx backup/existing_dir/file || framework_failure_
> +# Set the dir context to ensure it is reset
> +mkdir -p --context="$ctx" restore/existing_dir || framework_failure_
> +# Set the permissions of the source to show they're reset too
> +chmod o+rw restore/existing_dir
> +# Copy and ensure existing directories updated
> +cp -a backup/. restore/
> +ls -Zd restore/existing_dir | grep $ctx &&
> +  { ls -lZd restore/existing_dir; fail=1; }

... but the above chmod looks odd:
first, I don't see that o+rw is different from the default permissions
of the newly created directory (which is defined by the user's umask),
and second, the permission mode of the destination is not checked afterward.
I'd probably remove it here, as the check for the perms of the destination
should already be done in a different, non-root test.
OTOH, there's no test case yet verifying any interference between SELinux
contexts and the regular UNIX perms ...

Have a nice day,
Berny