GNU bug report logs - #25378
cp does not preserve SElinx context of sub folder

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 25378 in the body.
You can then email your comments to 25378 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: HE Henry <Henry.He <at> alcatel-lucent.com>
To: "bug-coreutils <at> gnu.org" <bug-coreutils <at> gnu.org>
Subject: cp does not preserve SElinx context of sub folder
Date: Fri, 6 Jan 2017 08:19:11 +0000

Hi coreutils team,

When using cp with --parents options, the SELinux context of sub folders are not preserved.

Example as below:

1. Before using cp:
[root <at> oame0 etc]# pwd
/etc
[root <at> oame0 etc]# ls -Z selinux/
-rw-r--r--. root root system_u:object_r:selinux_config_t:s0 config
-rw-r--r--. root root system_u:object_r:selinux_config_t:s0 semanage.conf
drwxr-xr-x. root root system_u:object_r:selinux_config_t:s0 targeted
[root <at> oame0 etc]#  ls -Z -d selinux/
drwxr-xr-x. root root system_u:object_r:selinux_config_t:s0 selinux/


2. Using cp to copy /etc/selinux/targeted/seusers with full path to /tmp
[root <at> oame0 etc]# cp -r --preserve=context --parents selinux/targeted/seusers  /tmp

3. After using cp, the SELinux context of sub folder are changed, like selinux, targeted 

[root <at> oame0 etc]# ls -Z /tmp/selinux/
drwx------. root root unconfined_u:object_r:user_tmp_t:s0 targeted
[root <at> oame0 etc]# ls -Z -d /tmp/selinux/
drwx------. root root unconfined_u:object_r:user_tmp_t:s0 /tmp/selinux/
[root <at> oame0 etc]# ls -Z -d /tmp/selinux/targeted/
drwx------. root root unconfined_u:object_r:user_tmp_t:s0 /tmp/selinux/targeted/
[root <at> oame0 etc]# ls -Z /tmp/selinux/targeted/   
-rw-------. root root system_u:object_r:selinux_config_t:s0 seusers

Thanks,
Henry

Message #10 received at 25378-done <at> debbugs.gnu.org (full text, mbox):

From: Pádraig Brady <P <at> draigBrady.com>
To: HE Henry <Henry.He <at> alcatel-lucent.com>, 25378-done <at> debbugs.gnu.org
Subject: Re: bug#25378: cp does not preserve SElinx context of sub folder
Date: Mon, 20 Feb 2017 19:11:15 -0800

[Message part 1 (text/plain, inline)]

On 06/01/17 00:19, HE Henry wrote:
> Hi coreutils team,
> 
> When using cp with --parents options, the SELinux context of sub folders are not preserved.
> 
> Example as below:
> 
> 1. Before using cp:
> [root <at> oame0 etc]# pwd
> /etc
> [root <at> oame0 etc]# ls -Z selinux/
> -rw-r--r--. root root system_u:object_r:selinux_config_t:s0 config
> -rw-r--r--. root root system_u:object_r:selinux_config_t:s0 semanage.conf
> drwxr-xr-x. root root system_u:object_r:selinux_config_t:s0 targeted
> [root <at> oame0 etc]#  ls -Z -d selinux/
> drwxr-xr-x. root root system_u:object_r:selinux_config_t:s0 selinux/
> 
> 
> 2. Using cp to copy /etc/selinux/targeted/seusers with full path to /tmp
> [root <at> oame0 etc]# cp -r --preserve=context --parents selinux/targeted/seusers  /tmp
> 
> 3. After using cp, the SELinux context of sub folder are changed, like selinux, targeted 
> 
> [root <at> oame0 etc]# ls -Z /tmp/selinux/
> drwx------. root root unconfined_u:object_r:user_tmp_t:s0 targeted
> [root <at> oame0 etc]# ls -Z -d /tmp/selinux/
> drwx------. root root unconfined_u:object_r:user_tmp_t:s0 /tmp/selinux/
> [root <at> oame0 etc]# ls -Z -d /tmp/selinux/targeted/
> drwx------. root root unconfined_u:object_r:user_tmp_t:s0 /tmp/selinux/targeted/
> [root <at> oame0 etc]# ls -Z /tmp/selinux/targeted/   
> -rw-------. root root system_u:object_r:selinux_config_t:s0 seusers

The attached should fix that.

thanks!
Pádraig

[cp-Z-parents.patch (text/x-patch, attachment)]

Message #13 received at 25378 <at> debbugs.gnu.org (full text, mbox):

From: Paul Eggert <eggert <at> cs.ucla.edu>
To: 25378 <at> debbugs.gnu.org, P <at> draigBrady.com, Henry.He <at> alcatel-lucent.com
Subject: Re: bug#25378: cp does not preserve SElinx context of sub folder
Date: Mon, 20 Feb 2017 20:46:12 -0800

Does 'mkdir -Z --parents' need a similar patch?

Minor points:

Please don't use 'extern' in front of function definitions. Just decls.

The usual style in .h files is to say "extern type function (args);" rather than 
to have a newline after the type.

Message #16 received at 25378 <at> debbugs.gnu.org (full text, mbox):

From: Pádraig Brady <P <at> draigBrady.com>
To: Paul Eggert <eggert <at> cs.ucla.edu>, 25378 <at> debbugs.gnu.org,
 Henry.He <at> alcatel-lucent.com
Subject: Re: bug#25378: cp does not preserve SElinx context of sub folder
Date: Mon, 20 Feb 2017 21:08:13 -0800

On 20/02/17 20:46, Paul Eggert wrote:
> Does 'mkdir -Z --parents' need a similar patch?

Already handled.

> 
> Minor points:
> 
> Please don't use 'extern' in front of function definitions. Just decls.
> 
> The usual style in .h files is to say "extern type function (args);" rather than 
> to have a newline after the type.
> 

Fixed (and still passing syntax checks)

thanks for the review,

Pádraig

Message #19 received at 25378 <at> debbugs.gnu.org (full text, mbox):

From: Bernhard Voelker <mail <at> bernhard-voelker.de>
To: 25378 <at> debbugs.gnu.org, P <at> draigBrady.com, Henry.He <at> alcatel-lucent.com
Subject: Re: bug#25378: cp does not preserve SElinx context of sub folder
Date: Tue, 21 Feb 2017 08:18:19 +0100

Hi Padraig,

only some minor remarks from my side.

On 02/21/2017 04:11 AM, Pádraig Brady wrote:
> From d17ca013f3aadcf697663830fa9ec34cba551f66 Mon Sep 17 00:00:00 2001
> From: =?UTF-8?q?P=C3=A1draig=20Brady?= <P <at> draigBrady.com>
> Date: Mon, 20 Feb 2017 18:46:49 -0800
> Subject: [PATCH] cp: set SElinux context for --parents directories
> 
> * src/copy.h (set_process_security_ctx, set_file_security_ctx):
_____________^

minor typo: s/c/h/

> diff --git a/src/cp.c b/src/cp.c
> index 88db3a3..6e18263 100644
> --- a/src/cp.c
> +++ b/src/cp.c
> @@ -394,6 +394,8 @@ make_dir_parents_private (char const *const_dir, size_t src_offset,
>  
>    *attr_list = NULL;
>  
> +  /* XXX: If all dirs present at the destination,
________^^^_____________^

why "XXX"?  Missing verb "are".


> diff --git a/tests/cp/cp-a-selinux.sh b/tests/cp/cp-a-selinux.sh
> index 19a9e64..de07406 100755
> --- a/tests/cp/cp-a-selinux.sh
> +++ b/tests/cp/cp-a-selinux.sh
> @@ -48,7 +48,6 @@ rm -f f
>  # due to recursion, and was handled incorrectly in coreutils-8.22
>  # Note standard permissions are updated for existing directories
>  # in the destination, so SELinux contexts should be updated too.
> -chmod o+rw restore/existing_dir

What was that change for?
(I don't have a SELinux system here, so the test is skipped here.)

>  mkdir -p backup/existing_dir/ || framework_failure_
>  ls -Zd backup/existing_dir > ed_ctx || fail=1
>  grep $ctx ed_ctx && framework_failure_
> @@ -57,11 +56,31 @@ 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_
>  # Copy and ensure existing directories updated
> -cp -a backup/. restore/
> +cp -a backup/. restore/ || fail=1

nice catch, thanks!

>  ls -Zd restore/existing_dir > ed_ctx || fail=1
>  grep $ctx ed_ctx &&
>    { ls -lZd restore/existing_dir; fail=1; }
>  
> +# Check context preserved with directories created with --parents,
> +# which was not handled before coreutils-8.27

Do we already know the next release will be "8.27"?
Maybe it's better to change to "<= coreutils-8.26".

Thanks & have a nice day,
Berny

Message #22 received at 25378 <at> debbugs.gnu.org (full text, mbox):

From: Pádraig Brady <P <at> draigBrady.com>
To: Bernhard Voelker <mail <at> bernhard-voelker.de>, 25378 <at> debbugs.gnu.org,
 Henry.He <at> alcatel-lucent.com
Subject: Re: bug#25378: cp does not preserve SElinx context of sub folder
Date: Mon, 20 Feb 2017 23:59:53 -0800

On 20/02/17 23:18, Bernhard Voelker wrote:
> Hi Padraig,
> 
> only some minor remarks from my side.
> 
> On 02/21/2017 04:11 AM, Pádraig Brady wrote:
>> From d17ca013f3aadcf697663830fa9ec34cba551f66 Mon Sep 17 00:00:00 2001
>> From: =?UTF-8?q?P=C3=A1draig=20Brady?= <P <at> draigBrady.com>
>> Date: Mon, 20 Feb 2017 18:46:49 -0800
>> Subject: [PATCH] cp: set SElinux context for --parents directories
>>
>> * src/copy.h (set_process_security_ctx, set_file_security_ctx):
> _____________^
> 
> minor typo: s/c/h/
> 
>> diff --git a/src/cp.c b/src/cp.c
>> index 88db3a3..6e18263 100644
>> --- a/src/cp.c
>> +++ b/src/cp.c
>> @@ -394,6 +394,8 @@ make_dir_parents_private (char const *const_dir, size_t src_offset,
>>  
>>    *attr_list = NULL;
>>  
>> +  /* XXX: If all dirs present at the destination,
> ________^^^_____________^
> 
> why "XXX"?  Missing verb "are".

Well I was going to follow up, but might as well here.
It was a bit surprising to me that the presence of all destination
directories would cause no attributes to be copied from the source.
Now it's not particular to SELinux, and also some attributes aren't
copied from the source at all in this code path, so if we were to
address it, it would be a separate patch. I.E. comment added for now.

>> diff --git a/tests/cp/cp-a-selinux.sh b/tests/cp/cp-a-selinux.sh
>> index 19a9e64..de07406 100755
>> --- a/tests/cp/cp-a-selinux.sh
>> +++ b/tests/cp/cp-a-selinux.sh
>> @@ -48,7 +48,6 @@ rm -f f
>>  # due to recursion, and was handled incorrectly in coreutils-8.22
>>  # Note standard permissions are updated for existing directories
>>  # in the destination, so SELinux contexts should be updated too.
>> -chmod o+rw restore/existing_dir
> 
> What was that change for?
> (I don't have a SELinux system here, so the test is skipped here.)

It looked like an unchecked vestige from a separate test script.
So I just removed it since in the area. I'll double check.

> 
>>  mkdir -p backup/existing_dir/ || framework_failure_
>>  ls -Zd backup/existing_dir > ed_ctx || fail=1
>>  grep $ctx ed_ctx && framework_failure_
>> @@ -57,11 +56,31 @@ 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_
>>  # Copy and ensure existing directories updated
>> -cp -a backup/. restore/
>> +cp -a backup/. restore/ || fail=1
> 
> nice catch, thanks!
> 
>>  ls -Zd restore/existing_dir > ed_ctx || fail=1
>>  grep $ctx ed_ctx &&
>>    { ls -lZd restore/existing_dir; fail=1; }
>>  
>> +# Check context preserved with directories created with --parents,
>> +# which was not handled before coreutils-8.27
> 
> Do we already know the next release will be "8.27"?
> Maybe it's better to change to "<= coreutils-8.26".

Well I was going to announce that I hope to release 8.27
in a couple of weeks, so consider that announced :)

thanks for the review,
Pádraig.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.