| Message ID | 20170111124110.5721-2-alan.christopher.jenkins@gmail.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
On Wed, 2017-01-11 at 12:41 +0000, Alan Jenkins wrote: > fixfiles links to restorecon. However if you start with restorecon > "restore file(s) default SELinux security contexts", you can easily > miss the fixfiles script. fixfiles is more generally useful than > `restorecon -R`. For example `restorecon -R /` is not as good as > `fixfiles restore`, because the restorecon command will try to > relabel > `/sys` and fail noisily. Thanks, applied both patches. Wondering though about the behavior you describe above; restorecon -R /sys only issues one error message for me and otherwise works fine, # restorecon -R /sys Could not set context for /sys/fs/cgroup: Read-only file system > > Signed-off-by: Alan Jenkins <alan.christopher.jenkins@gmail.com> > --- > policycoreutils/setfiles/restorecon.8 | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/policycoreutils/setfiles/restorecon.8 > b/policycoreutils/setfiles/restorecon.8 > index b00bf4e..bd27113 100644 > --- a/policycoreutils/setfiles/restorecon.8 > +++ b/policycoreutils/setfiles/restorecon.8 > @@ -214,6 +214,7 @@ The program was written by Dan Walsh <dwalsh@redh > at.com>. > > .SH "SEE ALSO" > .BR setfiles (8), > +.BR fixfiles (8), > .BR load_policy (8), > .BR checkpolicy (8), > .BR customizable_types (5)
On 12/01/17 20:01, Stephen Smalley wrote: > On Wed, 2017-01-11 at 12:41 +0000, Alan Jenkins wrote: >> fixfiles links to restorecon. However if you start with restorecon >> "restore file(s) default SELinux security contexts", you can easily >> miss the fixfiles script. fixfiles is more generally useful than >> `restorecon -R`. For example `restorecon -R /` is not as good as >> `fixfiles restore`, because the restorecon command will try to >> relabel >> `/sys` and fail noisily. > Thanks, applied both patches. yay! > Wondering though about the behavior > you describe above; restorecon -R /sys only issues one error message > for me and otherwise works fine, > # restorecon -R /sys > Could not set context for /sys/fs/cgroup: Read-only file system It turned out fixfiles also generated similar noise. I suspect this involved `-v` (in both cases), sorry. Fedora Workstation 25: "fixfiles spams warnings about debugfs. (docs say it only touches "real" filesystems!)" https://bugzilla.redhat.com/show_bug.cgi?id=1412747 Perhaps the root cause is actually the same. I still prefer the messages from fixfiles though. It explicitly detected conflicting labels on hardlinks https://bugzilla.redhat.com/show_bug.cgi?id=1411371 and informed me in advance when it decided to traverse and relabel five of my virtual filesystems Checking / /boot /dev /dev/hugepages /dev/mqueue /dev/pts /dev/shm /home /run /run/user/1000 /run/user/1001 /run/user/42 /sys /sys/fs/pstore /sys/kernel/debug /tmp (I doubt devtmpfs files are _intended_ to be labeled like this either. OTOH the stupidity doesn't seem to affect it, so I won't complain there).
On Thu, 2017-01-12 at 20:47 +0000, Alan Jenkins wrote: > On 12/01/17 20:01, Stephen Smalley wrote: > > On Wed, 2017-01-11 at 12:41 +0000, Alan Jenkins wrote: > > > fixfiles links to restorecon. However if you start with > > > restorecon > > > "restore file(s) default SELinux security contexts", you can > > > easily > > > miss the fixfiles script. fixfiles is more generally useful than > > > `restorecon -R`. For example `restorecon -R /` is not as good > > > as > > > `fixfiles restore`, because the restorecon command will try to > > > relabel > > > `/sys` and fail noisily. > > Thanks, applied both patches. > yay! > > > Wondering though about the behavior > > you describe above; restorecon -R /sys only issues one error > > message > > for me and otherwise works fine, > > # restorecon -R /sys > > Could not set context for /sys/fs/cgroup: Read-only file system > > It turned out fixfiles also generated similar noise. I suspect this > involved `-v` (in both cases), sorry. > > Fedora Workstation 25: > "fixfiles spams warnings about debugfs. (docs say it only touches > "real" filesystems!)" https://bugzilla.redhat.com/show_bug.cgi?id=141 > 2747 > > Perhaps the root cause is actually the same. I still prefer the > messages from fixfiles though. It explicitly detected conflicting > labels on hardlinks > > https://bugzilla.redhat.com/show_bug.cgi?id=1411371 > > and informed me in advance when it decided to traverse and relabel > five of my virtual filesystems > Checking / /boot /dev /dev/hugepages /dev/mqueue /dev/pts /dev/shm > /home /run /run/user/1000 /run/user/1001 /run/user/42 /sys > /sys/fs/pstore /sys/kernel/debug /tmp > (I doubt devtmpfs files are _intended_ to be labeled like this > either. OTOH the stupidity doesn't seem to affect it, so I won't > complain there). By default, it includes any filesystem with the seclabel mount option from /proc/mounts; this is a kernel-generated option that indicates that the filesystem is known to support setting of security labels by userspace. restorecon (and setfiles) can be further instructed to skip specific directories via the -e option. fixfiles is just a wrapper script around setfiles and/or restorecon depending on its options. setfiles was the original SELinux filesystem labeling utility, oriented to labeling a list of filesystems based on a specified file_contexts configuration, potentially even on a SELinux-disabled host. fixfiles and restorecon were later additions by Red Hat as more user-friendly options, and then still later restorecon kept growing in functionality and duplicating setfiles that we coalesced them (restorecon is just a link that alters the default options and command- line interface). setfiles walks a filesystem at a time, which facilitates the hard link conflict detection logic. restorecon in contrast just recurses a directory tree (with -R) and doesn't care about filesystem boundaries, so if we wanted to turn on hard link conflict detection for it, we'd need to augment that logic to save the device numbers too. Possible but hasn't been a priority.
On 12/01/17 21:23, Stephen Smalley wrote: > On Thu, 2017-01-12 at 20:47 +0000, Alan Jenkins wrote: >> On 12/01/17 20:01, Stephen Smalley wrote: >>> On Wed, 2017-01-11 at 12:41 +0000, Alan Jenkins wrote: >>>> fixfiles links to restorecon. However if you start with >>>> restorecon >>>> "restore file(s) default SELinux security contexts", you can >>>> easily >>>> miss the fixfiles script. fixfiles is more generally useful than >>>> `restorecon -R`. For example `restorecon -R /` is not as good >>>> as >>>> `fixfiles restore`, because the restorecon command will try to >>>> relabel >>>> `/sys` and fail noisily. >>> Thanks, applied both patches. >> yay! >> >>> Wondering though about the behavior >>> you describe above; restorecon -R /sys only issues one error >>> message >>> for me and otherwise works fine, >>> # restorecon -R /sys >>> Could not set context for /sys/fs/cgroup: Read-only file system >> >> It turned out fixfiles also generated similar noise. I suspect this >> involved `-v` (in both cases), sorry. >> >> Fedora Workstation 25: >> "fixfiles spams warnings about debugfs. (docs say it only touches >> "real" filesystems!)" https://bugzilla.redhat.com/show_bug.cgi?id=141 >> 2747 >> >> Perhaps the root cause is actually the same. I still prefer the >> messages from fixfiles though. It explicitly detected conflicting >> labels on hardlinks >> >> https://bugzilla.redhat.com/show_bug.cgi?id=1411371 >> >> and informed me in advance when it decided to traverse and relabel >> five of my virtual filesystems >> Checking / /boot /dev /dev/hugepages /dev/mqueue /dev/pts /dev/shm >> /home /run /run/user/1000 /run/user/1001 /run/user/42 /sys >> /sys/fs/pstore /sys/kernel/debug /tmp >> (I doubt devtmpfs files are _intended_ to be labeled like this >> either. OTOH the stupidity doesn't seem to affect it, so I won't >> complain there). > By default, it includes any filesystem with the seclabel mount option > from /proc/mounts; this is a kernel-generated option that indicates > that the filesystem is known to support setting of security labels by > userspace. restorecon (and setfiles) can be further instructed to skip > specific directories via the -e option. fixfiles is just a wrapper > script around setfiles and/or restorecon depending on its options. > setfiles was the original SELinux filesystem labeling utility, > oriented to labeling a list of filesystems based on a specified > file_contexts configuration, potentially even on a SELinux-disabled > host. fixfiles and restorecon were later additions by Red Hat as more > user-friendly options, and then still later restorecon kept growing in > functionality and duplicating setfiles that we coalesced them > (restorecon is just a link that alters the default options and command- > line interface). > > setfiles walks a filesystem at a time, which facilitates the hard link > conflict detection logic. restorecon in contrast just recurses a > directory tree (with -R) and doesn't care about filesystem boundaries, > so if we wanted to turn on hard link conflict detection for it, we'd > need to augment that logic to save the device numbers too. Possible > but hasn't been a priority. Ok. My main puzzle here[*] is why `fixfiles` handles sysfs (/sys/) fine, but then there's floods of warnings about debugfs (/sys/kernel/debug/). The same seems to happen with /dev/ being fine, but not the other virtual fs's with seclabel which are mounted in subdirectories of /dev/. What frustrates me is that `fixfiles check` generates this flood of messages even immediately after `fixfiles restore`. (Or equivalently, that `fixfiles restore -v` does not converge to a smaller number of messages). My use case was to install the latest version of some software from source (fwupd), apply the labels expected by the distribution policy, and also check what that involved. I learnt about fixfiles from /usr/libexec/selinux/selinux-autorelabel. (I notice the autorelabel script "handles" any messages by redirecting to /dev/null. But, it's using `fixfiles restore` without `-v`, which doesn't generate these warnings. It seems it would also miss out on the nice percentage progress indicator. Weird). Thanks Alan [*] Ok, I was also surprised that fixfiles dug into my home directory. It's happy with `touch /home/alan-sysop/t` (user_home_t), but after I "move to wastebasket", it complains the file needs to marked as data_home_t. No idea what happens if you relabel and then try to restore files from the wastebasket, e.g. `.ssh` or `public_html`. But then `public_html` doesn't seem to get labeled when I create it either. Clearly there's something I don't understand.
On Thu, 2017-01-12 at 23:42 +0000, Alan Jenkins wrote: > My main puzzle here[*] is why `fixfiles` handles sysfs (/sys/) fine, > but > then there's floods of warnings about debugfs > (/sys/kernel/debug/). The > same seems to happen with /dev/ being fine, but not the other > virtual > fs's with seclabel which are mounted in subdirectories of /dev/. This is a bug/regression. Thanks for reporting it. In commit 36f1ccbb5743749c404ad8f960867923544b90d9, Dan added this warning but only if the user explicitly does a restorecon /path/to/foo and /path/to/foo does not have any matching label in file_contexts; in the case of a restorecon -R or setfiles, it isn't supposed to happen. The check on the recursive flag got dropped when this logic was taken into selinux_restorecon(3) in libselinux (commit f2e77865e144ab2e1313aa78d99b969f8f48695e). Will fix. The reason it occurs for /sys/kernel/debug is that there is an explicit entry in file_contexts that indicates that setfiles/restorecon should not try to label anything under /sys/kernel/debug (/sys/kernel/debug/.* <<none>>). With regard to sysfs, a bit of historical context: originally all sysfs nodes were created with a single security context, specified in kernel policy. Later, we needed finer-grained control to support libvirt labeling of specific nodes that needed to be accessible to VMs, so support was added to permit userspace labeling of sysfs nodes (hence restorecon/setfiles started working on sysfs). Then in Android we needed to label a number of sysfs nodes, so Android started doing the equivalent of restorecon -R /sys on boot (using an optimization to avoid walking parts of the sysfs tree that do not need to change from the default). Linux distributions don't do that presently but do label a few specific sysfs nodes via /usr/lib/tmpfiles.d/selinux-policy.conf. More recently, we introduced improved support in the kernel for per- file labeling of sysfs nodes at creation time as well (based on pathname from the root of sysfs), so we can specify those via genfscon statements in policy. So the kernel does assign an initial default, but userspace retains the ability to override selectively (if authorized by policy). > What frustrates me is that `fixfiles check` generates this flood of > messages even immediately after `fixfiles restore`. (Or > equivalently, > that `fixfiles restore -v` does not converge to a smaller number of > messages). Yes, agreed that we ought to fix that. > > My use case was to install the latest version of some software from > source (fwupd), apply the labels expected by the distribution > policy, > and also check what that involved. > > I learnt about fixfiles from /usr/libexec/selinux/selinux- > autorelabel. > > (I notice the autorelabel script "handles" any messages by > redirecting > to /dev/null. But, it's using `fixfiles restore` without `-v`, > which > doesn't generate these warnings. It seems it would also miss out on > the > nice percentage progress indicator. Weird). > > Thanks > Alan > > [*] Ok, I was also surprised that fixfiles dug into my home > directory. > It's happy with `touch /home/alan-sysop/t` (user_home_t), but after > I > "move to wastebasket", it complains the file needs to marked as > data_home_t. No idea what happens if you relabel and then try to > restore files from the wastebasket, e.g. `.ssh` or > `public_html`. But > then `public_html` doesn't seem to get labeled when I create it > either. > Clearly there's something I don't understand. Not sure about the different types on Trash vs Home. But if I create public_html directory, it does get assigned httpd_user_content_t automatically (probably a name-based type transition or alternatively restorecond).
On Fri, 2017-01-13 at 09:48 -0500, Stephen Smalley wrote: > On Thu, 2017-01-12 at 23:42 +0000, Alan Jenkins wrote: > > > > My main puzzle here[*] is why `fixfiles` handles sysfs (/sys/) > > fine, > > but > > then there's floods of warnings about debugfs > > (/sys/kernel/debug/). The > > same seems to happen with /dev/ being fine, but not the other > > virtual > > fs's with seclabel which are mounted in subdirectories of /dev/. > > This is a bug/regression. Thanks for reporting it. In commit > 36f1ccbb5743749c404ad8f960867923544b90d9, Dan added this warning but > only if the user explicitly does a restorecon /path/to/foo and > /path/to/foo does not have any matching label in file_contexts; in > the > case of a restorecon -R or setfiles, it isn't supposed to happen. > The > check on the recursive flag got dropped when this logic was taken > into > selinux_restorecon(3) in libselinux > (commit f2e77865e144ab2e1313aa78d99b969f8f48695e). Will fix. Actually, I am wrong about this being a regression (and I should have known that, since the buggy version is 2.5 and that precedes the latter commit). Looking at the first commit, the original logic was to display a warning if not recursive OR verbose, so it would unconditionally log a warning if you did restorecon /path/to/foo or restorecon -v /path/to/foo or restorecon -Rv /path/to/foo, just not if you did restorecon -R /path/to/foo. When it was moved to libselinux selinux_restorecon(3), it was changed to log a warning if verbose, so it logs a warning if you pass -v (with or without -R) but not if you just do restorecon /path/to/foo. The patch I sent makes it only log the warning if verbose and not recursive, so it will only log if you pass -v without -R. To be honest, I'm not sure what the point of this warning is; it is perfectly valid for an entry to have <<none>> to indicate that it should not be relabeled at all by restorecon/setfiles. Maybe we should just remove the warning altogether.
On Thu, 2017-01-12 at 20:47 +0000, Alan Jenkins wrote: > Perhaps the root cause is actually the same. I still prefer the > messages from fixfiles though. It explicitly detected conflicting > labels on hardlinks > > https://bugzilla.redhat.com/show_bug.cgi?id=1411371 On this topic, I have opened an issue for selinux, https://github.com/SELinuxProject/selinux/issues/43
On 01/13/2017 10:27 AM, Stephen Smalley wrote: > On Fri, 2017-01-13 at 09:48 -0500, Stephen Smalley wrote: >> On Thu, 2017-01-12 at 23:42 +0000, Alan Jenkins wrote: >>> My main puzzle here[*] is why `fixfiles` handles sysfs (/sys/) >>> fine, >>> but >>> then there's floods of warnings about debugfs >>> (/sys/kernel/debug/). The >>> same seems to happen with /dev/ being fine, but not the other >>> virtual >>> fs's with seclabel which are mounted in subdirectories of /dev/. >> This is a bug/regression. Thanks for reporting it. In commit >> 36f1ccbb5743749c404ad8f960867923544b90d9, Dan added this warning but >> only if the user explicitly does a restorecon /path/to/foo and >> /path/to/foo does not have any matching label in file_contexts; in >> the >> case of a restorecon -R or setfiles, it isn't supposed to happen. >> The >> check on the recursive flag got dropped when this logic was taken >> into >> selinux_restorecon(3) in libselinux >> (commit f2e77865e144ab2e1313aa78d99b969f8f48695e). Will fix. > Actually, I am wrong about this being a regression (and I should have > known that, since the buggy version is 2.5 and that precedes the latter > commit). Looking at the first commit, the original logic was to display > a warning if not recursive OR verbose, so it would unconditionally log > a warning if you did restorecon /path/to/foo or restorecon -v > /path/to/foo or restorecon -Rv /path/to/foo, just not if you did > restorecon -R /path/to/foo. When it was moved to libselinux > selinux_restorecon(3), it was changed to log a warning if verbose, so > it logs a warning if you pass -v (with or without -R) but not if you > just do restorecon /path/to/foo. The patch I sent makes it only log the > warning if verbose and not recursive, so it will only log if you pass > -v without -R. > > To be honest, I'm not sure what the point of this warning is; it is > perfectly valid for an entry to have <<none>> to indicate that it > should not be relabeled at all by restorecon/setfiles. Maybe we should > just remove the warning altogether. > The problem is people don't understand this. If a user sees a user_home_t on /tmp and runs restorecon on it he expects it to have a label with tmp_t in the name, and if the tool finishes silently he thinks he is done. This reveals to him that their is no default label, so perhaps he will do a chcon. Or `rm -f`.
On Fri, 2017-01-13 at 13:29 -0500, Daniel J Walsh wrote: > > On 01/13/2017 10:27 AM, Stephen Smalley wrote: > > > > On Fri, 2017-01-13 at 09:48 -0500, Stephen Smalley wrote: > > > > > > On Thu, 2017-01-12 at 23:42 +0000, Alan Jenkins wrote: > > > > > > > > My main puzzle here[*] is why `fixfiles` handles sysfs (/sys/) > > > > fine, > > > > but > > > > then there's floods of warnings about debugfs > > > > (/sys/kernel/debug/). The > > > > same seems to happen with /dev/ being fine, but not the other > > > > virtual > > > > fs's with seclabel which are mounted in subdirectories of > > > > /dev/. > > > This is a bug/regression. Thanks for reporting it. In commit > > > 36f1ccbb5743749c404ad8f960867923544b90d9, Dan added this warning > > > but > > > only if the user explicitly does a restorecon /path/to/foo and > > > /path/to/foo does not have any matching label in file_contexts; > > > in > > > the > > > case of a restorecon -R or setfiles, it isn't supposed to happen. > > > The > > > check on the recursive flag got dropped when this logic was taken > > > into > > > selinux_restorecon(3) in libselinux > > > (commit f2e77865e144ab2e1313aa78d99b969f8f48695e). Will fix. > > Actually, I am wrong about this being a regression (and I should > > have > > known that, since the buggy version is 2.5 and that precedes the > > latter > > commit). Looking at the first commit, the original logic was to > > display > > a warning if not recursive OR verbose, so it would unconditionally > > log > > a warning if you did restorecon /path/to/foo or restorecon -v > > /path/to/foo or restorecon -Rv /path/to/foo, just not if you did > > restorecon -R /path/to/foo. When it was moved to libselinux > > selinux_restorecon(3), it was changed to log a warning if verbose, > > so > > it logs a warning if you pass -v (with or without -R) but not if > > you > > just do restorecon /path/to/foo. The patch I sent makes it only log > > the > > warning if verbose and not recursive, so it will only log if you > > pass > > -v without -R. > > > > To be honest, I'm not sure what the point of this warning is; it is > > perfectly valid for an entry to have <<none>> to indicate that it > > should not be relabeled at all by restorecon/setfiles. Maybe we > > should > > just remove the warning altogether. > > > The problem is people don't understand this. If a user sees a > user_home_t on /tmp and runs > restorecon on it he expects it to have a label with tmp_t in the > name, > and if the tool finishes > silently he thinks he is done. This reveals to him that their is no > default label, so perhaps he > will do a chcon. Or `rm -f`. Old behavior (before moving to selinux_restorecon(3), <= 2.5): $ touch /tmp/foo $ chcon -t etc_t /tmp/foo $ restorecon /tmp/foo restorecon: Warning no default label for /tmp/foo $ restorecon -v /tmp/foo restorecon: Warning no default label for /tmp/foo $ restorecon -R /tmp/foo $ restorecon -Rv /tmp/foo restorecon: Warning no default label for /tmp/foo So we get the warning without -R or with -v. Seems kind of surprising that -R suppresses it but -Rv does not. New behavior (after moving to selinux_restorecon(3), 2.6, before my patch): $ restorecon /tmp/foo $ restorecon -v /tmp/foo Warning no default label for /tmp/foo $ restorecon -R /tmp/foo $ restorecon -Rv /tmp/foo Warning no default label for /tmp/foo Here we get the warning only with -v, independent of -R. Seems more consistent from a UI point of view. This however doesn't help Alan with his goal of enabling fixfiles check or fixfiles restore -v to show no extraneous output if everything is labeled correctly. New behavior after my patch: $ restorecon /tmp/foo $ restorecon -v /tmp/foo Warning no default label for /tmp/foo $ restorecon -R /tmp/foo $ restorecon -Rv /tmp/foo Here we only get the warning with -v without -R. This avoids the problem for fixfiles check but doesn't help your situation and is confusing usage as well. Also, I think you only want this warning if the user-supplied pathname has no default label. Which would mean we need to do this check and warning early, not down where we are checking or applying the labels to individual files within a tree walk.
On 13/01/17 19:38, Stephen Smalley wrote: > On Fri, 2017-01-13 at 13:29 -0500, Daniel J Walsh wrote: >> On 01/13/2017 10:27 AM, Stephen Smalley wrote: >>> On Fri, 2017-01-13 at 09:48 -0500, Stephen Smalley wrote: >>>> On Thu, 2017-01-12 at 23:42 +0000, Alan Jenkins wrote: >>>>> My main puzzle here[*] is why `fixfiles` handles sysfs (/sys/) >>>>> fine, >>>>> but >>>>> then there's floods of warnings about debugfs >>>>> (/sys/kernel/debug/). The >>>>> same seems to happen with /dev/ being fine, but not the other >>>>> virtual >>>>> fs's with seclabel which are mounted in subdirectories of >>>>> /dev/. >>>> This is a bug/regression. Thanks for reporting it. In commit >>>> 36f1ccbb5743749c404ad8f960867923544b90d9, Dan added this warning >>>> but >>>> only if the user explicitly does a restorecon /path/to/foo and >>>> /path/to/foo does not have any matching label in file_contexts; >>>> in >>>> the >>>> case of a restorecon -R or setfiles, it isn't supposed to happen. >>>> The >>>> check on the recursive flag got dropped when this logic was taken >>>> into >>>> selinux_restorecon(3) in libselinux >>>> (commit f2e77865e144ab2e1313aa78d99b969f8f48695e). Will fix. >>> Actually, I am wrong about this being a regression (and I should >>> have >>> known that, since the buggy version is 2.5 and that precedes the >>> latter >>> commit). Looking at the first commit, the original logic was to >>> display >>> a warning if not recursive OR verbose, so it would unconditionally >>> log >>> a warning if you did restorecon /path/to/foo or restorecon -v >>> /path/to/foo or restorecon -Rv /path/to/foo, just not if you did >>> restorecon -R /path/to/foo. When it was moved to libselinux >>> selinux_restorecon(3), it was changed to log a warning if verbose, >>> so >>> it logs a warning if you pass -v (with or without -R) but not if >>> you >>> just do restorecon /path/to/foo. The patch I sent makes it only log >>> the >>> warning if verbose and not recursive, so it will only log if you >>> pass >>> -v without -R. >>> >>> To be honest, I'm not sure what the point of this warning is; it is >>> perfectly valid for an entry to have <<none>> to indicate that it >>> should not be relabeled at all by restorecon/setfiles. Maybe we >>> should >>> just remove the warning altogether. >>> >> The problem is people don't understand this. If a user sees a >> user_home_t on /tmp and runs >> restorecon on it he expects it to have a label with tmp_t in the >> name, >> and if the tool finishes >> silently he thinks he is done. This reveals to him that their is no >> default label, so perhaps he >> will do a chcon. Or `rm -f`. > Old behavior (before moving to selinux_restorecon(3), <= 2.5): > $ touch /tmp/foo > $ chcon -t etc_t /tmp/foo > $ restorecon /tmp/foo > restorecon: Warning no default label for /tmp/foo > $ restorecon -v /tmp/foo > restorecon: Warning no default label for /tmp/foo > $ restorecon -R /tmp/foo > $ restorecon -Rv /tmp/foo > restorecon: Warning no default label for /tmp/foo > > So we get the warning without -R or with -v. Seems kind of surprising > that -R suppresses it but -Rv does not. > > New behavior (after moving to selinux_restorecon(3), 2.6, before my > patch): > $ restorecon /tmp/foo > $ restorecon -v /tmp/foo > Warning no default label > for /tmp/foo > $ restorecon -R /tmp/foo > $ restorecon -Rv /tmp/foo > Warning no > default label for /tmp/foo > > Here we get the warning only with -v, independent of -R. > Seems more consistent from a UI point of view. > This however doesn't help Alan with his goal of enabling fixfiles check or fixfiles restore -v to show no extraneous output if everything is labeled correctly. > > New behavior after my patch: > $ restorecon /tmp/foo > $ restorecon -v /tmp/foo > Warning no default label for /tmp/foo > $ restorecon -R /tmp/foo > $ restorecon -Rv /tmp/foo > > Here we only get the warning with -v without -R. > This avoids the problem for fixfiles check but doesn't help your situation and is confusing usage as well. > > Also, I think you only want this warning if the user-supplied pathname > has no default label. Which would mean we need to do this check and > warning early, not down where we are checking or applying the labels to individual files within a tree walk. Thank you for considering my report so seriously. Your suggestion sounds very promising! It sounds like a pretty nice compromise, if e.g. processing /var could omit the warnings caused by processing files under /var/tmp. The corner case I wasn't 100% sure about, is if the path you pass is e.g. `/tmp`. Looking at `file_contexts` it seems very sensible though. /tmp itself is ignored, so passing `/tmp` would still provide our friendly warning. (I guess the label on /tmp comes from the package, e.g. the `filesystem` rpm on Fedora). Alan
On 13/01/17 19:56, Alan Jenkins wrote: > On 13/01/17 19:38, Stephen Smalley wrote: >> On Fri, 2017-01-13 at 13:29 -0500, Daniel J Walsh wrote: >>> On 01/13/2017 10:27 AM, Stephen Smalley wrote: >>>> On Fri, 2017-01-13 at 09:48 -0500, Stephen Smalley wrote: >>>>> On Thu, 2017-01-12 at 23:42 +0000, Alan Jenkins wrote: >>>>>> My main puzzle here[*] is why `fixfiles` handles sysfs (/sys/) >>>>>> fine, >>>>>> but >>>>>> then there's floods of warnings about debugfs >>>>>> (/sys/kernel/debug/). The >>>>>> same seems to happen with /dev/ being fine, but not the other >>>>>> virtual >>>>>> fs's with seclabel which are mounted in subdirectories of >>>>>> /dev/. >>>>> This is a bug/regression. Thanks for reporting it. In commit >>>>> 36f1ccbb5743749c404ad8f960867923544b90d9, Dan added this warning >>>>> but >>>>> only if the user explicitly does a restorecon /path/to/foo and >>>>> /path/to/foo does not have any matching label in file_contexts; >>>>> in >>>>> the >>>>> case of a restorecon -R or setfiles, it isn't supposed to happen. >>>>> The >>>>> check on the recursive flag got dropped when this logic was taken >>>>> into >>>>> selinux_restorecon(3) in libselinux >>>>> (commit f2e77865e144ab2e1313aa78d99b969f8f48695e). Will fix. >>>> Actually, I am wrong about this being a regression (and I should >>>> have >>>> known that, since the buggy version is 2.5 and that precedes the >>>> latter >>>> commit). Looking at the first commit, the original logic was to >>>> display >>>> a warning if not recursive OR verbose, so it would unconditionally >>>> log >>>> a warning if you did restorecon /path/to/foo or restorecon -v >>>> /path/to/foo or restorecon -Rv /path/to/foo, just not if you did >>>> restorecon -R /path/to/foo. When it was moved to libselinux >>>> selinux_restorecon(3), it was changed to log a warning if verbose, >>>> so >>>> it logs a warning if you pass -v (with or without -R) but not if >>>> you >>>> just do restorecon /path/to/foo. The patch I sent makes it only log >>>> the >>>> warning if verbose and not recursive, so it will only log if you >>>> pass >>>> -v without -R. >>>> >>>> To be honest, I'm not sure what the point of this warning is; it is >>>> perfectly valid for an entry to have <<none>> to indicate that it >>>> should not be relabeled at all by restorecon/setfiles. Maybe we >>>> should >>>> just remove the warning altogether. >>>> >>> The problem is people don't understand this. If a user sees a >>> user_home_t on /tmp and runs >>> restorecon on it he expects it to have a label with tmp_t in the >>> name, >>> and if the tool finishes >>> silently he thinks he is done. This reveals to him that their is no >>> default label, so perhaps he >>> will do a chcon. Or `rm -f`. >> Old behavior (before moving to selinux_restorecon(3), <= 2.5): >> $ touch /tmp/foo >> $ chcon -t etc_t /tmp/foo >> $ restorecon /tmp/foo >> restorecon: Warning no default label for /tmp/foo >> $ restorecon -v /tmp/foo >> restorecon: Warning no default label for /tmp/foo >> $ restorecon -R /tmp/foo >> $ restorecon -Rv /tmp/foo >> restorecon: Warning no default label for /tmp/foo >> >> So we get the warning without -R or with -v. Seems kind of surprising >> that -R suppresses it but -Rv does not. >> >> New behavior (after moving to selinux_restorecon(3), 2.6, before my >> patch): >> $ restorecon /tmp/foo >> $ restorecon -v /tmp/foo >> Warning no default label >> for /tmp/foo >> $ restorecon -R /tmp/foo >> $ restorecon -Rv /tmp/foo >> Warning no >> default label for /tmp/foo >> >> Here we get the warning only with -v, independent of -R. >> Seems more consistent from a UI point of view. >> This however doesn't help Alan with his goal of enabling fixfiles >> check or fixfiles restore -v to show no extraneous output if >> everything is labeled correctly. >> >> New behavior after my patch: >> $ restorecon /tmp/foo >> $ restorecon -v /tmp/foo >> Warning no default label for /tmp/foo >> $ restorecon -R /tmp/foo >> $ restorecon -Rv /tmp/foo >> >> Here we only get the warning with -v without -R. >> This avoids the problem for fixfiles check but doesn't help your >> situation and is confusing usage as well. >> >> Also, I think you only want this warning if the user-supplied pathname >> has no default label. Which would mean we need to do this check and >> warning early, not down where we are checking or applying the labels >> to individual files within a tree walk. > > Thank you for considering my report so seriously. > > Your suggestion sounds very promising! > > It sounds like a pretty nice compromise, if e.g. processing /var could > omit the warnings caused by processing files under /var/tmp. > > The corner case I wasn't 100% sure about, is if the path you pass is > e.g. `/tmp`. Looking at `file_contexts` it seems very sensible > though. /tmp itself is ignored, so passing `/tmp` would still provide > our friendly warning. (I guess the label on /tmp comes from the > package, e.g. the `filesystem` rpm on Fedora). oops, no. We do label some files in /tmp, and /tmp _is_ labelled. So the compromise is not _quite_ as nice for providing warnings as I first thought. I.e. processing /tmp would not provide any warning. However attempting to process /tmp/my-chroot-which-dnf-created-with-selinux-labels should still warn, which is something I agree with. Alan
diff --git a/policycoreutils/setfiles/restorecon.8 b/policycoreutils/setfiles/restorecon.8 index b00bf4e..bd27113 100644 --- a/policycoreutils/setfiles/restorecon.8 +++ b/policycoreutils/setfiles/restorecon.8 @@ -214,6 +214,7 @@ The program was written by Dan Walsh <dwalsh@redhat.com>. .SH "SEE ALSO" .BR setfiles (8), +.BR fixfiles (8), .BR load_policy (8), .BR checkpolicy (8), .BR customizable_types (5)
fixfiles links to restorecon. However if you start with restorecon "restore file(s) default SELinux security contexts", you can easily miss the fixfiles script. fixfiles is more generally useful than `restorecon -R`. For example `restorecon -R /` is not as good as `fixfiles restore`, because the restorecon command will try to relabel `/sys` and fail noisily. Signed-off-by: Alan Jenkins <alan.christopher.jenkins@gmail.com> --- policycoreutils/setfiles/restorecon.8 | 1 + 1 file changed, 1 insertion(+)