On Thu, Nov 5, 2020 at 12:22 PM Ondrej Mosnacek <omosnace@xxxxxxxxxx> wrote: > > On Thu, Nov 5, 2020 at 4:31 PM Paul Moore <paul@xxxxxxxxxxxxxx> wrote: > > On Thu, Nov 5, 2020 at 8:51 AM Stephen Smalley > > <stephen.smalley.work@xxxxxxxxx> wrote: > > > On Thu, Nov 5, 2020 at 7:44 AM Ondrej Mosnacek <omosnace@xxxxxxxxxx> wrote: > > > > > > > > Hello everyone, > > > > > > > > while trying to fix the NFS rootcontext= issue, I realized that this > > > > funny thing is possible: > > > > > > > > # mount -o rootcontext=system_u:object_r:lib_t:s0 -t tmpfs tmpfs /mnt > > > > # ls -lZd /mnt > > > > drwxrwxrwt. 2 root root system_u:object_r:lib_t:s0 40 nov 5 07:30 /mnt > > > > # mount > > > > [...] > > > > tmpfs on /mnt type tmpfs > > > > (rw,relatime,rootcontext=system_u:object_r:lib_t:s0,seclabel) > > > > # chcon -t bin_t /mnt > > > > # ls -lZd /mnt > > > > drwxrwxrwt. 2 root root system_u:object_r:bin_t:s0 40 nov 5 07:30 /mnt > > > > # mount > > > > [...] > > > > tmpfs on /mnt type tmpfs > > > > (rw,relatime,rootcontext=system_u:object_r:bin_t:s0,seclabel) > > > > > > > > I.e. if you mount a tree with rootcontext=<oldctx> and then relabel > > > > the root node to <newctx>, the displayed mount options will report > > > > rootcontext=<newctx> instead of rootcontext=<oldctx>. A side effect is > > > > that if you try to mount the same superblock again, it will only > > > > permit you to mount with rootcontext=<newctx>, not with > > > > rootcontext=<oldctx>. > > > > > > > > Is that intended, bad, or "weird, but doesn't matter" behavior? > > > > > > I'd say it is bad. > > > > > > > I have a halfway written patch to disallow altering the root node's > > > > context when mounted with rootcontext=, but I'm not sure if that's the > > > > right thing to do or not. > > > > > > Probably the better fix would be to save the original rootcontext SID > > > as a new field of the > > > superblock security struct and use that both when displaying the mount > > > options and when > > > comparing old and new mount options instead of what happens to be > > > assigned to the root > > > inode at the time. > > > > I worry that would be confusing, allowing the root inode to be > > relabeled yet still showing the old label in the mount options. It > > would seem that simply blocking a root inode relabel when a > > rootcontext is specified would be the cleanest choice, assuming > > existing systems do not rely on this behavior. > > > > Ondrej, Stephen, any idea how common this is on normal systems? My > > gut feeling says "not very common", but that is just a guess. > > I don't even know what use case it's supposed to solve :) I suppose if > you freshly format some storage drive and you want the root dir to be > labeled immediately after mounting, rootcontext= could be useful. For > such a use case Stephen's approach would make sense (you might still > want to eventually relabel the root dir to something else), but there > are some gotchas... For example, the label is initially set only in > the inode security struct, but not written to the xattrs (I only > looked at the code briefly, I could be wrong here), so you would still > need to manually set the label on the root directory after mounting > for the label to persist. And if you remount the superblock after > changing the label, the internal label will be reset to the old one > (which you are forced to specify in the mount options), but again not > persistently. That's all very unintuitive. > > So, IMHO, good ways to fix it are either disallowing changing the > label altogether, or making sure the label is propagated to the xattrs > (if supported) and allowing to remount with a different rootcontext= > option (or with no rootcontext). But that's only if I guessed the use > case correctly. I think the original use case was primarily tmpfs mounts, so that e.g. one could mount a tmpfs on /dev that is immediately labeled with device_t. Not fundamentally opposed to preventing relabeling afterward but always possible that could break existing userspace somewhere.