On 09/30/2015 10:41 AM, Stephen Smalley wrote: > On 09/29/2015 05:03 PM, Stephen Smalley wrote: >> On 09/28/2015 04:00 PM, David Howells wrote: >>> >>> The attached patches provide security support for unioned files >>> where the >>> security involves an object-label-based LSM (such as SELinux) rather >>> than a >>> path-based LSM. >>> >>> [Note that a number of the bits that were in the original patch set >>> are now >>> upstream and I've rebased on Casey's changes to the security hook >>> system] >>> >>> The patches can be broken down into two sets: >>> >>> (1) A patch to add LSM hooks to handle copy up of a file, including >>> label >>> determination/setting and xattr filtration and a patch to have >>> overlayfs call the hooks during the copy-up procedure. >>> >>> (2) My SELinux implementations of these hooks. I do three things: >>> >>> (a) Don't copy up SELinux xattrs from the lower file to the upper >>> file. It is assumed that the upper file will be created >>> with the >>> attrs we want or the selinux_inode_copy_up() hook will >>> set it >>> appropriately. >>> >>> The reason there are two separate hooks here is that >>> selinux_inode_copy_up_xattr() might not ever be called if there >>> aren't actually any xattrs on the lower inode. >>> >>> (b) I try to derive a label to be used by file operations by, in >>> order >>> of preference: using the label on the union inode if there >>> is one >>> (the normal overlayfs case); using the override label set >>> on the >>> superblock, if provided; or trying to derive a new label by >>> sid >>> transition operation. >>> >>> (c) Using the label obtained in (b) in file_has_perm() rather >>> than >>> using the label on the lower inode. >>> >>> Now the steps I have outlined in (b) and (c) seem to be at odds with >>> what >>> Dan Walsh and Stephen Smalley want - but I'm not sure I follow what >>> that >>> is, let alone how to do it: >>> >>> Wanted to bring back the original proposal. Stephen suggested that >>> we could change back to the MLS way of handling labels. >>> >>> In MCS we base the MCS label of content created by a process on the >>> containing directory. Which means that if a process running as >>> s0:c1,c2 creates content in a directory labeled s0, it will get >>> created as s0. >>> >>> In MLS if a process running as s0:c1,c2 creates content in a >>> directory it labels it s0:c1,c2. No matter what the label of the >>> directory is. (Well actually if the directory is not ranged the >>> process will not be able to create the content.) >>> >>> We changed the default for MCS in Rawhide for about a week, when I >>> realized this was a huge problem for containers sharing content. >>> Currently if you want two containers to share the same volume >>> mount, we label the content as svirt_sandbox_file_t:s0 If one >>> process running as s0:c1,c2 creates content it gets created as s0, >>> if the second container process is running as s0:c3,c4, it can >>> still read/write the content. If we changed the default the object >>> would get created as s0:c1,c2 and process runing as s0:c3,c4 would >>> be blocked. >>> >>> So I had it reverted back to the standard MCS Mode. >>> >>> If we could get the default to be MLS mode on COW file systems and >>> MCS on Volumes, we would get the best of both worlds. >> >> How are you testing this? >> I tried as follows: >> >> # Make sure we have a policy that is using xattrs to label overlay >> inodes. >> $ seinfo --fs_use | grep overlay >> fs_use_xattr overlay system_u:object_r:fs_t:s0 >> >> # Define some types for the different directories involved. >> $ cat overlay.te >> policy_module(overlay, 1.0) >> >> type lower_t; >> files_type(lower_t) >> >> type upper_t; >> files_type(upper_t) >> >> type work_t; >> files_type(work_t) >> >> type merged_t; >> files_type(merged_t) >> >> $ make -f /usr/share/selinux/devel/Makefile overlay.pp >> $ sudo semodule -i overlay.pp >> >> # Create and label the different directories involved. >> $ mkdir lower upper work merged >> $ chcon -t lower_t lower >> $ chcon -t upper_t upper >> $ chcon -t work_t work >> $ chcon -t merged_t merged >> >> # Populate lower >> $ echo "lower" > lower/a >> $ mkdir lower/b >> >> # Mount overlay >> $ sudo mount -t overlay -o lowerdir=lower,upperdir=upper,workdir=work >> merged >> >> # Look at the merged dir and labels. >> $ ls -Z merged >> unconfined_u:object_r:lower_t:s0 a >> unconfined_u:object_r:lower_t:s0 b >> >> # Write/create some files/directories. >> $ echo "foo" >> merged/a >> $ mkdir merged/b/c >> $ mkdir merged/c >> >> # Look again. >> $ ls -ZR merged >> merged: >> unconfined_u:object_r:lower_t:s0 a unconfined_u:object_r:upper_t:s0 c >> unconfined_u:object_r:lower_t:s0 b >> >> merged/b: >> unconfined_u:object_r:work_t:s0 c >> >> merged/b/c: >> >> $ ls -ZR upper >> upper: >> unconfined_u:object_r:work_t:s0 a unconfined_u:object_r:upper_t:s0 c >> unconfined_u:object_r:work_t:s0 b >> >> upper/b: >> unconfined_u:object_r:work_t:s0 c >> >> upper/b/c: >> >> Note that the copied-up file (a) and directory (b) are labeled lower_t >> in the overlay, but work_t in the upper dir, and neither of those is >> really what we want IIUC. >> >> And that's just the labeling question. Then there is the question of >> what permission checks were applied during those copy-up operations and >> whether the current process ends up needing write permissions to them >> all. > > Also, the labels on the overlay inodes change if you umount and then > mount it again: > > $ sudo umount merged > $ sudo mount -t overlay overlay -o > lowerdir=lower,upperdir=upper,workdir=work merged > $ ls -ZR merged > merged: > unconfined_u:object_r:work_t:s0 a unconfined_u:object_r:upper_t:s0 c > unconfined_u:object_r:work_t:s0 b > > merged/b: > unconfined_u:object_r:work_t:s0 c > > merged/b/c: > > merged/c: > > It seems to me that either the copied-up files should be labeled > upper_t (i.e. from upperdir) or merged_t (i.e. from the overlay). But > certainly not lower_t (which is supposed to be read-only to the > container) or work_t (which isn't supposed to be directly accessed by > processes in the first place). > Yes lower_t should be read only and we want to transition the directories to upper_t, or more specifically upper_t:MCS label. lower_t:s0 -> upper_t:s0:c1,c2 For Container images. -- To unsubscribe from this list: send the line "unsubscribe linux-unionfs" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
