Re: [OE-core] [PATCH] shadow: 'useradd' copies root's extended attributes

[José Bollo]

On Wed, 10 Jan 2018 17:50:19 +0800
wenzong fan  wrote:

> On 01/10/2018 01:01 AM, Patrick Ohly wrote:
> > On Fri, 2018-01-05 at 01:07 +, Fan, Wenzong wrote:  
> >> It works and will override the labels of home dir that SELinux
> >> applied, that's the issue.
> >>
> >> For SELinux enabled system, the user's home dir should have lavel
> >> 'user_home_dir_t' instead of 'etc_t', it prevents users from
> >> creating files in their home dir.  
> > 
> > Sounds like the "copy xattr" function needs to become a bit
> > smarter: it needs to understand some of the semantic involved and
> > skip those SELinux xattrs that are always meant to be set
> > dynamically by the running kernel.
> > 
> > Wenzong, which xattrs are those? Do you agree with the proposed
> > solution?  
> 
> The xattr for selinux is "security.selinux":
> 
> $ getfattr -n security.selinux /home/t1
> security.selinux="user_u:object_r:user_home_dir_t:s0-s15:c0.c1023"
> 
> I think the "attr_copy_file()" is doing right thing, but it should be 
> used in a limited situation, such as only for Smack ...
> 
> Thanks
> Wenzong

The LSM "SELinux" is complicated enough to change label of template
files to label of instance files correctly. The approach with Smack is
different and the template files embed the expected complex hierarchy
that otherwise could only be created with a program.

A possible approach would be with smack to add a program for creating
homes. Conversely, SELinux could consider to use template approach too
instead of increasing its rules set (with templating splitted in two
parts: files and "creation" rules).

>From "man 7 xattr" we know:
 - extended attributes are namespaced
 - the fully qualified name is "namespace.attribute"
 - actual namespaces are security, system, trusted, and user

A possibility would be to filter the copied extended attributes. For
SELinux we can just tell to not copy "security" attributes. See
manual of the command "tar" (recent version) that has options
--xattrs-exclude and --xattr-include.

Is there a need to copy extended attributes except for Smack?

> > Jose, can you look into updating your patch accordingly?

Perhaps yes but not now because I don't now what to do.

Best regards
Jose
-- 
___
Openembedded-core mailing list
Openembedded-core@lists.openembedded.org
http://lists.openembedded.org/mailman/listinfo/openembedded-core

Re: [OE-core] [PATCH] shadow: 'useradd' copies root's extended attributes

[José Bollo]

On Thu, 4 Jan 2018 17:28:27 +0800
wenzong fan <wenzong@windriver.com> wrote:

> Hi José Bollo,
> 
> This will override the labels of user's home directories that set by 
> SELinux.
> 
> For example, if I run below command on SELinux enabled system:
> 
> $ useradd test
> 
> SELinux will label it as "user_u:object_r:user_home_dir_t:SystemLow" 
> first, and then useradd will reset the label as 
> "system_u:object_r:etc_t:SystemLow".
> 
> I got strace logs below:
> 
> 723   openat(AT_FDCWD, "/proc/thread-self/attr/fscreate", 
> O_RDWR|O_CLOEXEC) = 11
> 723   write(11, "user_u:object_r:user_home_dir_t:"..., 35) = 35 
> 
> 723   close(11) = 0
> 723   mkdir("/home/t1", 000)= 0
> 723   chown("/home/t1", 1000, 1000) = 0
> 723   chmod("/home/t1", 0755)   = 0
> 
> # SELinux labelled it as "user_home_dir_t" here.
> 
> 723   llistxattr("/etc/skel", NULL, 0)  = 17
> 723   llistxattr("/etc/skel", "security.selinux\0", 17) = 17
> 723   openat(AT_FDCWD, "/etc/xattr.conf", O_RDONLY) = -1 ENOENT (No
> such file or directory)
> 723   lgetxattr("/etc/skel", "security.selinux", NULL, 0) = 27
> 723   lgetxattr("/etc/skel", "security.selinux", 
> "system_u:object_r:etc_t:s0", 27) = 27
> 723   lsetxattr("/home/t1", "security.selinux", 
> "system_u:object_r:etc_t:s0", 27, 0) = 0
> 
> # useradd reset the label as "etc_t" here.
> 
> Do you agree to move the patch to Smack specific layer? Such as 
> meta-security?

I agree.

SELinux is the king of monsters and pushes others in corners...

Best regards
José

> Thanks
> Wenzong
> 
> On 03/15/2017 04:04 PM, José Bollo wrote:
> > On Thu, 09 Mar 2017 18:18:05 +0100
> > Patrick Ohly <patrick.o...@intel.com> wrote:
> >   
> >> On Thu, 2017-03-09 at 17:48 +0100, José Bollo wrote:  
> >>> On Thu, 09 Mar 2017 17:07:54 +0100
> >>> Patrick Ohly <patrick.o...@intel.com> wrote:  
> >>>> Can't you reorder and rebase the patches so that this
> >>>> 0001-useradd.c-create-parent-directories-when-necessary.patch
> >>>> applies on top of the patch which was submitted upstream?  
> >>>
> >>> I agree that it would be better to reorder. Better but less
> >>> conservative because an existing patch must be upgraded.  
> >>
> >> If upstream merges the proposed patch, then rebasing will be
> >> inevitable at some point, so we might as well do the cleaner
> >> solution now, even if the diff becomes larger.
> >>  
> >>>> "devtool modify shadow-native" might be useful for that. "git
> >>>> rebase -i" in workspace/sources/shadow-native", then finish with
> >>>> "devtool update-recipe shadow-native". I haven't tried whether
> >>>> "update-recipe" handles re-ordering patches. If it doesn't, just
> >>>> fix it manually.  
> >>>
> >>> I'll do and propose the new version soon.  
> >>
> >> Thanks!
> >>  
> > 
> > I pushed a new version of the patch this monday. I suppose that it
> > is waiting for approval.
> > 
> > Best regards
> > José Bollo
> >   

-- 
___
Openembedded-core mailing list
Openembedded-core@lists.openembedded.org
http://lists.openembedded.org/mailman/listinfo/openembedded-core

Re: [OE-core] [PATCH] shadow: 'useradd' copies root's extended attributes

[José Bollo]

On Thu, 09 Mar 2017 18:18:05 +0100
Patrick Ohly <patrick.o...@intel.com> wrote:

> On Thu, 2017-03-09 at 17:48 +0100, José Bollo wrote:
> > On Thu, 09 Mar 2017 17:07:54 +0100
> > Patrick Ohly <patrick.o...@intel.com> wrote:  
> > > Can't you reorder and rebase the patches so that this
> > > 0001-useradd.c-create-parent-directories-when-necessary.patch
> > > applies on top of the patch which was submitted upstream?  
> > 
> > I agree that it would be better to reorder. Better but less
> > conservative because an existing patch must be upgraded.  
> 
> If upstream merges the proposed patch, then rebasing will be
> inevitable at some point, so we might as well do the cleaner solution
> now, even if the diff becomes larger.
> 
> > > "devtool modify shadow-native" might be useful for that. "git
> > > rebase -i" in workspace/sources/shadow-native", then finish with
> > > "devtool update-recipe shadow-native". I haven't tried whether
> > > "update-recipe" handles re-ordering patches. If it doesn't, just
> > > fix it manually.  
> > 
> > I'll do and propose the new version soon.  
> 
> Thanks!
> 

I pushed a new version of the patch this monday. I suppose that it is
waiting for approval.

Best regards
José Bollo

-- 
___
Openembedded-core mailing list
Openembedded-core@lists.openembedded.org
http://lists.openembedded.org/mailman/listinfo/openembedded-core

Re: [OE-core] [PATCH] shadow: 'useradd' copies root's extended attributes

[José Bollo]

On Thu, 09 Mar 2017 17:07:54 +0100
Patrick Ohly <patrick.o...@intel.com> wrote:

> On Thu, 2017-03-09 at 15:07 +0100, jo...@nonadev.net wrote:
> > From: José Bollo <jose.bo...@iot.bzh>
> > 
> > The copy of extended attributes is interesting for
> > Smack systems because it allows to set the security
> > template of the user's home directories without
> > modifying the tools (useradd here). But the version
> > of useradd that copies the extended attributes doesn't
> > copy the extended attributes of the root. This can make
> > use of homes impossible! This patch corrects the issue
> > by copying the extended attributes of the root directory:
> > /home/user will get the extended attributes of /etc/skel.  
> 
> Makes sense to me.
> 
> > This includes 2 patches to implement the behaviour:
> > one for the target and one for the native.
> > 
> > The patch for the target was submitted upstream (see
> > http://lists.alioth.debian.org/pipermail/pkg-shadow-commits/2017-March/003804.html)
> > 
> > The patch for the native couldn't be submitted upstream
> > because it applies after the patch specific to open-embedded
> > that creates the parent directories:
> >   0001-useradd.c-create-parent-directories-when-necessary.patch  
> 
> Can't you reorder and rebase the patches so that this
> 0001-useradd.c-create-parent-directories-when-necessary.patch applies
> on top of the patch which was submitted upstream?

I agree that it would be better to reorder. Better but less
conservative because an existing patch must be upgraded.

> "devtool modify shadow-native" might be useful for that. "git rebase
> -i" in workspace/sources/shadow-native", then finish with "devtool
> update-recipe shadow-native". I haven't tried whether "update-recipe"
> handles re-ordering patches. If it doesn't, just fix it manually.

I'll do and propose the new version soon.
-- 
___
Openembedded-core mailing list
Openembedded-core@lists.openembedded.org
http://lists.openembedded.org/mailman/listinfo/openembedded-core