Bug 1814689

This is the patch that went upstream:

commit 862368c92def52e3bccce571a46cd99dce34fc78
Author: Jeff Layton <jlayton>
Date:   Wed Oct 30 14:12:06 2019 -0400

    Allow cephfs to use xattrs for storing contexts
    
    cephfs recently gained the ability to store SELinux contexts in an xattr
    (like most local filesystems). Change the policy to allow for this.
    
    Signed-off-by: Jeff Layton <jlayton>

I am a bit concerned about breaking existing setups though. Do need some sort of transition mechanism to ensure that existing setups don't break when the policy is updated? Some thoughts:

1) can we make it so that files without a security.selinux xattr default to system_u:object_r:cephfs_t:s0, without requiring the user to mount with defcontext= ? That would probably make the transition seamless for files that preexisted the policy change, and also (probably?) on kernels that don't support security.selinux xattr.

2) do we need a new SELinux boolean (e.g. cephfs_use_xattr_context)? That might a way to allow users to opt-in if we're concerned about breaking existing setups. I'm not sure what the semantics should be there however.

Hi Jeff, 

What is expected here from us please? Can you make it clear? What is the target? RHEL-8.3? Can we have a meeting about it? 

Thanks,
Lukas.

Oops, I think I didn't quite get this tested correctly originally. The selinux-policy package on my centos8 box is overlaying the mount with system_u:object_r:cephfs_t:s0. Is there an updated set of selinux-policy packages I can install to test this properly?

(In reply to Jeff Layton from comment #14)
> Oops, I think I didn't quite get this tested correctly originally. The
> selinux-policy package on my centos8 box is overlaying the mount with
> system_u:object_r:cephfs_t:s0. Is there an updated set of selinux-policy
> packages I can install to test this properly?

So I finally figured out how to set up a cephfs mount on RHEL-8 and I tried it myself. It turns out I forgot to add "ceph" to another if condition... It needs to be added also to selinux_is_genfs_special_handling(). I'm currently brewing a new test kernel and then I'll re-test.

Anyway, you don't need to install new selinux-policy package to test the approach I proposed (just the kernel), as long as you run the two commands from comment #13.

Ok, great. Let me know when you have a kernel ready to go and I'll test it as well.

The kernel is now built:
https://brewweb.engineering.redhat.com/brew/taskinfo?taskID=27444099

I just tested it and it behaves exactly as intended. The existing unlabeled files show up with label cephfs_t and newly created files get a permanent label based on the parent's context (so they normally inherit the cephfs_t label). Changing file/directory labels using chcon works and the new context is applied when creating files inside a relabeled directory. This is all assuming the extra '(allow file_type cephfs_t (filesystem (associate)))' exists in the policy.

So to sum it up, we have two options:
OPTION 1 (Upstream approach):
  kernel: set CONFIG_CEPH_FS_SECURITY_LABEL=y
  policy: backport 862368c92def ("Allow cephfs to use xattrs for storing contexts")
  PROS:
    - consistency with upstream/Fedora
  CONS:
    - mounting with defcontext=... needed for unlabeled files to show up as cephfs_t as before

OPTION 2 (Approach with better backwards compat):
  kernel: set CONFIG_CEPH_FS_SECURITY_LABEL=y + add "ceph" to the list of filesystems for which combined genfs + xattrs labeling is used (RHEL-8-only patch)
  policy: add rule 'allow file_type cephfs_t:filesystem { associate };' (RHEL-8-only patch)
  PROS:
    - unlabeled CephFS trees still show up as cephfs_t (less dramatic change for customers)
  CONS:
    - differences from upstream (just a few lines of code, though)

We (the SELinux team) should have no problem supporting either option, so it's down to which is better for you (well, the CephFS customers, actually, but you should know their expectations better than us ;)

*** Bug 1767402 has been marked as a duplicate of this bug. ***

Ok, I tested 4.18.0-191.1.el8.ceph_labeling.x86_64 this morning. Looks good:

# ls -lZ
total 0
drwxr-xr-x.  2 root root unconfined_u:object_r:cephfs_t:s0  0 Mar 25 08:52 foo
drwxrwxrwx.  2 root root unconfined_u:object_r:cephfs_t:s0  0 Mar 25 08:37 scratch
drwxr-xr-x. 21 root root unconfined_u:object_r:cephfs_t:s0 57 Mar 25 08:44 test
# chcon system_u:object_r:tmpfs_t:s0 foo
# ls -lZ
total 0
drwxr-xr-x.  2 root root system_u:object_r:tmpfs_t:s0       0 Mar 25 08:52 foo
drwxrwxrwx.  2 root root unconfined_u:object_r:cephfs_t:s0  0 Mar 25 08:37 scratch
drwxr-xr-x. 21 root root unconfined_u:object_r:cephfs_t:s0 57 Mar 25 08:44 test
# mkdir foo/bar
# ls -laZ foo
total 0
drwxr-xr-x. 3 root root system_u:object_r:tmpfs_t:s0        1 Mar 25 08:53 .
drwxrwxrwt. 5 root root system_u:object_r:cephfs_t:s0       3 Mar 25 08:52 ..
drwxr-xr-x. 2 root root unconfined_u:object_r:user_tmp_t:s0 0 Mar 25 08:53 bar

...it didn't directly inherit the tmpfs_t context, but the files in /tmp all have a user_tmp_t context so I assume it did the right thing here.

(In reply to Jeff Layton from comment #19)
> ...it didn't directly inherit the tmpfs_t context, but the files in /tmp all
> have a user_tmp_t context so I assume it did the right thing here.

Yes, that's just the tmpfs_t type transition rule being applied as it should. I should've suggested a different type for testing (e.g. usr_t), but tmpfs_t works as well, just has different expected behavior.

*** Bug 1496492 has been marked as a duplicate of this bug. ***

Great. Should we transition this to a kernel bug? I've got an open "needinfo" request about QA tests for this on the other kernel bug (which just turned on the Kconfig option for security labels).

    https://bugzilla.redhat.com/show_bug.cgi?id=1814668#c9

Are there boilerplate QE tests to test SELinux enablement on filesystems, or do we need to add something special for CephFS?

(In reply to Jeff Layton from comment #22)
> Great. Should we transition this to a kernel bug?

We'll need a change in policy as well, so we need to keep this bug. I'll create a new one for the kernel change.

> I've got an open
> "needinfo" request about QA tests for this on the other kernel bug (which
> just turned on the Kconfig option for security labels).
> 
>     https://bugzilla.redhat.com/show_bug.cgi?id=1814668#c9
> 
> Are there boilerplate QE tests to test SELinux enablement on filesystems, or
> do we need to add something special for CephFS?

I have some test prepared for this (just needs some polishing), but I don't think this is big enough to be worth adding to CKI.

I may have found an issue. symlink inodes always seem to show a context of system_u:object_r:cephfs_t:s0. With a mainline kernel on f32, I see:

$ ls -alZ
total 1
drwxrwxrwt.  2 jlayton jlayton system_u:object_r:tmp_t:s0           2 Aug  3 13:16 .
drwxrwxrwt. 10 root    root    system_u:object_r:unlabeled_t:s0    10 Aug  3 13:04 ..
lrwxrwxrwx.  1 jlayton jlayton unconfined_u:object_r:user_tmp_t:s0  8 Aug  3 13:11 link -> testfile
-rw-rw-r--.  1 jlayton jlayton unconfined_u:object_r:user_tmp_t:s0  0 Jul 28 15:12 testfile


...but with the current centos8 kernels, I see:

$ ls -alZ
total 1
drwxrwxrwt.  2 jlayton jlayton system_u:object_r:tmp_t:s0           2 Aug  3 13:16 .
drwxrwxrwt. 10 root    root    system_u:object_r:cephfs_t:s0       10 Aug  3 13:04 ..
lrwxrwxrwx.  1 jlayton jlayton system_u:object_r:cephfs_t:s0        8 Aug  3 13:11 link -> testfile
-rw-rw-r--.  1 jlayton jlayton unconfined_u:object_r:user_tmp_t:s0  0 Jul 28 15:12 testfile


Note too that there is a fix going into mainline too that I haven't yet pulled into RHEL. Symlinks are currently being created without a context on cephfs:

    https://bugzilla.redhat.com/show_bug.cgi?id=1861509

Let me know if you'd rather I open a new bug for this.

(In reply to Jeff Layton from comment #33)
> I may have found an issue. symlink inodes always seem to show a context of
> system_u:object_r:cephfs_t:s0.

That is expected given the bug you mention below:

> Note too that there is a fix going into mainline too that I haven't yet
> pulled into RHEL. Symlinks are currently being created without a context on
> cephfs:

Recall that on RHEL-8 we fall back to cephfs_t instead of unlabeled_t to not potentially break existing users with files that haven't been explicitly labeled yet.

However, I think there might be another issue (in kernel/SELinux this time) that will still cause symlinks to get the wrong contexts on RHEL-8... [1] It's kind of a weird coincidence, because that bug isn't related with the Ceph one, yet they have almost the same effect :D Let me check with a scratch kernel with your fix applied and if my suspicion is confirmed, I'll file a separate BZ. Sorry for not realizing this earlier...

[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=7470d0d13fb680bb82b40f18831f7d4ee7a4bb62

I don't think it's really expected behavior.

To be clear, with the patch for BZ#1861509, the client creates the symlink with the correct context, as I can see it when I list it from the f32 client. It's just that the kernel on rhel8 box seems to show a context of system_u:object_r:cephfs_t:s0 on all symlinks, even when they do have an explicit context set.

Ah, I see, so if you observer the same also with your fix, then this is the symptom of the second bug... I also reproduced with my scratch builds and successfully tested a fix. I'll open a new BZ for it soon (I'll try to link it to the relevant bugs and put you in CC).

Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.

For information on the advisory (selinux-policy bug fix and enhancement update), and where to find the updated
files, follow the link below.

If the solution does not work for you, open a new bug report.

https://access.redhat.com/errata/RHBA-2020:4528

What version of OCP did this RHBA make it into?

(In reply to Dan Yocum from comment #41)
> What version of OCP did this RHBA make it into?

Dan,

This made RHEL 8.3 GA so it'd be in 4.7 GA and newer.