[PATCH] policycoreutils, python: Fix bad manpage formatting in "SEE ALSO"

[PATCH] policycoreutils, python: Fix bad manpage formatting in "SEE ALSO"

[Alan Jenkins]

Fix missing and surplus commas.  Fix the following formatting errors:

    .BR selinux(8)

renders the the "(8)" in bold as well as the "selinux".  This is wrong.

    .B selinux
    (8)

renders with a space between "selinux" and "(8)", this is wrong.

    .B selinux (8)

commits both of the above mistakes.

    .BR selinux (8), apparmor (8)

omits the space separating "selinux(8)," and "apparmor(8)", this is wrong.
Correct all the above using the following markup:

    .BR selinux (8),
    .BR apparmor (8)

Signed-off-by: Alan Jenkins <alan.christopher.jenkins@gmail.com>
---
 policycoreutils/load_policy/load_policy.8 |  3 +--
 policycoreutils/newrole/newrole.1         |  3 +--
 policycoreutils/run_init/run_init.8       |  6 ++----
 policycoreutils/scripts/fixfiles.8        |  3 ++-
 policycoreutils/secon/secon.1             |  3 +--
 policycoreutils/semodule/semodule.8       |  3 ++-
 python/semanage/semanage-boolean.8        |  8 ++++----
 python/semanage/semanage-dontaudit.8      |  4 ++--
 python/semanage/semanage-export.8         |  6 +++---
 python/semanage/semanage-fcontext.8       |  4 ++--
 python/semanage/semanage-interface.8      |  4 ++--
 python/semanage/semanage-login.8          |  6 +++---
 python/semanage/semanage-module.8         |  6 +++---
 python/semanage/semanage-permissive.8     |  4 ++--
 python/semanage/semanage-port.8           |  4 ++--
 python/semanage/semanage-user.8           |  6 +++---
 python/semanage/semanage.8                | 26 +++++++++++++-------------
 17 files changed, 48 insertions(+), 51 deletions(-)

diff --git a/policycoreutils/load_policy/load_policy.8 b/policycoreutils/load_policy/load_policy.8
index 38c01b6..5f5550d 100644
--- a/policycoreutils/load_policy/load_policy.8
+++ b/policycoreutils/load_policy/load_policy.8
@@ -35,8 +35,7 @@ Policy load failed
 .B 3
 Initial policy load failed and enforcing mode requested
 .SH SEE ALSO
-.B booleans
-(8),
+.BR booleans (8)
 .SH AUTHORS
 .nf
 This manual page was written by Dan Walsh <dwalsh@redhat.com>.
diff --git a/policycoreutils/newrole/newrole.1 b/policycoreutils/newrole/newrole.1
index 96cdd14..0d9738a 100644
--- a/policycoreutils/newrole/newrole.1
+++ b/policycoreutils/newrole/newrole.1
@@ -110,8 +110,7 @@ Running a program in a given role or level:
 /etc/selinux/newrole_pam.conf - optional mapping of commands to separate pam service names
 .br
 .SH SEE ALSO
-.B runcon
-(1)
+.BR runcon (1)
 .SH AUTHORS
 .nf
 Anthony Colatrella
diff --git a/policycoreutils/run_init/run_init.8 b/policycoreutils/run_init/run_init.8
index 9fb5249..a031d5d 100644
--- a/policycoreutils/run_init/run_init.8
+++ b/policycoreutils/run_init/run_init.8
@@ -20,10 +20,8 @@ not required.  Check your PAM documentation.
 .br
 /etc/selinux/POLICYTYPE/contexts/initrc_context - contains the context to run init scripts under
 .SH SEE ALSO
-.B newrole
-(1),
-.B runcon
-(1)
+.BR newrole (1),
+.BR runcon (1)
 .SH AUTHORS
 .nf
 Wayne Salamon (wsalamon@tislabs.com) 
diff --git a/policycoreutils/scripts/fixfiles.8 b/policycoreutils/scripts/fixfiles.8
index 1b9a2d6..0099b9b 100644
--- a/policycoreutils/scripts/fixfiles.8
+++ b/policycoreutils/scripts/fixfiles.8
@@ -89,5 +89,6 @@ This man page was written by Richard Hally <rhally@mindspring.com>.
 The script  was written by Dan Walsh <dwalsh@redhat.com>
 
 .SH "SEE ALSO"
-.BR setfiles (8), restorecon(8)
+.BR setfiles (8),
+.BR restorecon (8)
 
diff --git a/policycoreutils/secon/secon.1 b/policycoreutils/secon/secon.1
index a0ff795..501b5cb 100644
--- a/policycoreutils/secon/secon.1
+++ b/policycoreutils/secon/secon.1
@@ -116,8 +116,7 @@ If none of \fB\-\-user\fR, \fB\-\-role\fR, \fB\-\-type\fR, \fB\-\-level\fR or
 Then all of them will be output.
 .PP
 .SH SEE ALSO
-.B chcon
-(1)
+.BR chcon (1)
 .SH AUTHORS
 .nf
 James Antill (james.antill@redhat.com) 
diff --git a/policycoreutils/semodule/semodule.8 b/policycoreutils/semodule/semodule.8
index 0c5fdf7..849a042 100644
--- a/policycoreutils/semodule/semodule.8
+++ b/policycoreutils/semodule/semodule.8
@@ -127,7 +127,8 @@ $ semodule \-X 400 \-\-hll \-E puppet \-\-cil \-E wireshark
 .fi
 
 .SH SEE ALSO
-.B checkmodule(8), semodule_package(8)
+.BR checkmodule (8),
+.BR semodule_package (8)
 .SH AUTHORS
 .nf
 This manual page was written by Dan Walsh <dwalsh@redhat.com>.
diff --git a/python/semanage/semanage-boolean.8 b/python/semanage/semanage-boolean.8
index 0c48587..99a6260 100644
--- a/python/semanage/semanage-boolean.8
+++ b/python/semanage/semanage-boolean.8
@@ -52,10 +52,10 @@ List customized booleans
 # semanage boolean \-l \-C
 
 .SH "SEE ALSO"
-.B selinux (8),
-.B semanage (8)
-.B setsebool (8)
-.B getsebool (8)
+.BR selinux (8),
+.BR semanage (8),
+.BR setsebool (8),
+.BR getsebool (8)
 
 .SH "AUTHOR"
 This man page was written by Daniel Walsh <dwalsh@redhat.com>
diff --git a/python/semanage/semanage-dontaudit.8 b/python/semanage/semanage-dontaudit.8
index 3d29911..81accc6 100644
--- a/python/semanage/semanage-dontaudit.8
+++ b/python/semanage/semanage-dontaudit.8
@@ -27,8 +27,8 @@ Turn off dontaudit rules
 # semanage dontaudit off
 
 .SH "SEE ALSO"
-.B selinux (8),
-.B semanage (8)
+.BR selinux (8),
+.BR semanage (8)
 
 .SH "AUTHOR"
 This man page was written by Daniel Walsh <dwalsh@redhat.com>
diff --git a/python/semanage/semanage-export.8 b/python/semanage/semanage-export.8
index d688224..d422683 100644
--- a/python/semanage/semanage-export.8
+++ b/python/semanage/semanage-export.8
@@ -29,9 +29,9 @@ Import semanage modifications from another machine
 # semanage import \-f semanage.mods
 
 .SH "SEE ALSO"
-.B selinux (8),
-.B semanage (8),
-.B semanage-import (8)
+.BR selinux (8),
+.BR semanage (8),
+.BR semanage-import (8)
 
 .SH "AUTHOR"
 This man page was written by Daniel Walsh <dwalsh@redhat.com>
diff --git a/python/semanage/semanage-fcontext.8 b/python/semanage/semanage-fcontext.8
index 07c2831..561123a 100644
--- a/python/semanage/semanage-fcontext.8
+++ b/python/semanage/semanage-fcontext.8
@@ -80,8 +80,8 @@ execute the following commands.
 # restorecon \-R \-v /disk6
 
 .SH "SEE ALSO"
-.B selinux (8),
-.B semanage (8)
+.BR selinux (8),
+.BR semanage (8)
 
 .SH "AUTHOR"
 This man page was written by Daniel Walsh <dwalsh@redhat.com>
diff --git a/python/semanage/semanage-interface.8 b/python/semanage/semanage-interface.8
index fbab4b9..d9d526d 100644
--- a/python/semanage/semanage-interface.8
+++ b/python/semanage/semanage-interface.8
@@ -56,8 +56,8 @@ list all interface definitions
 # semanage interface \-l
 
 .SH "SEE ALSO"
-.B selinux (8),
-.B semanage (8)
+.BR selinux (8),
+.BR semanage (8)
 
 .SH "AUTHOR"
 This man page was written by Daniel Walsh <dwalsh@redhat.com>
diff --git a/python/semanage/semanage-login.8 b/python/semanage/semanage-login.8
index a2397a0..f451bdc 100644
--- a/python/semanage/semanage-login.8
+++ b/python/semanage/semanage-login.8
@@ -60,9 +60,9 @@ Assign all users in the engineering group to the staff_u user
 # semanage login \-a \-s staff_u %engineering
 
 .SH "SEE ALSO"
-.B selinux (8),
-.B semanage (8),
-.B semanage-user (8)
+.BR selinux (8),
+.BR semanage (8),
+.BR semanage-user (8)
 
 .SH "AUTHOR"
 This man page was written by Daniel Walsh <dwalsh@redhat.com>
diff --git a/python/semanage/semanage-module.8 b/python/semanage/semanage-module.8
index 42d6862..e005716 100644
--- a/python/semanage/semanage-module.8
+++ b/python/semanage/semanage-module.8
@@ -52,9 +52,9 @@ Install custom apache policy module
 # semanage module \-a myapache
 
 .SH "SEE ALSO"
-.B selinux (8),
-.B semanage (8)
-.B semodule (8)
+.BR selinux (8),
+.BR semanage (8),
+.BR semodule (8)
 
 .SH "AUTHOR"
 This man page was written by Daniel Walsh <dwalsh@redhat.com>
diff --git a/python/semanage/semanage-permissive.8 b/python/semanage/semanage-permissive.8
index ee30c85..1999a45 100644
--- a/python/semanage/semanage-permissive.8
+++ b/python/semanage/semanage-permissive.8
@@ -38,8 +38,8 @@ Make httpd_t (Web Server) a permissive domain
 # semanage permissive \-a httpd_t
 
 .SH "SEE ALSO"
-.B selinux (8),
-.B semanage (8)
+.BR selinux (8),
+.BR semanage (8)
 
 .SH "AUTHOR"
 This man page was written by Daniel Walsh <dwalsh@redhat.com>
diff --git a/python/semanage/semanage-port.8 b/python/semanage/semanage-port.8
index 666120e..a21287c 100644
--- a/python/semanage/semanage-port.8
+++ b/python/semanage/semanage-port.8
@@ -61,8 +61,8 @@ Allow sshd to listen on tcp port 8991
 # semanage port \-a \-t ssh_port_t \-p tcp 8991
 
 .SH "SEE ALSO"
-.B selinux (8),
-.B semanage (8)
+.BR selinux (8),
+.BR semanage (8)
 
 .SH "AUTHOR"
 This man page was written by Daniel Walsh <dwalsh@redhat.com>
diff --git a/python/semanage/semanage-user.8 b/python/semanage/semanage-user.8
index 2df03dd..30bc670 100644
--- a/python/semanage/semanage-user.8
+++ b/python/semanage/semanage-user.8
@@ -63,9 +63,9 @@ Add level for TopSecret Users
 # semanage user \-a \-R "staff_r" \-rs0\-TopSecret topsecret_u
 
 .SH "SEE ALSO"
-.B selinux (8),
-.B semanage (8)
-.B semanage\-login (8)
+.BR selinux (8),
+.BR semanage (8),
+.BR semanage\-login (8)
 
 .SH "AUTHOR"
 This man page was written by Daniel Walsh <dwalsh@redhat.com>
diff --git a/python/semanage/semanage.8 b/python/semanage/semanage.8
index ac39862..abc4736 100644
--- a/python/semanage/semanage.8
+++ b/python/semanage/semanage.8
@@ -66,19 +66,19 @@ modification.
 List help information
 
 .SH "SEE ALSO"
-.B selinux (8),
-.B semanage-boolean (8),
-.B semanage-dontaudit (8),
-.B semanage-export (8),
-.B semanage-fcontext (8),
-.B semanage-import (8),
-.B semanage-interface (8),
-.B semanage-login (8),
-.B semanage-module (8),
-.B semanage-node (8),
-.B semanage-permissive (8),
-.B semanage-port (8),
-.B semanage-user (8)
+.BR selinux (8),
+.BR semanage-boolean (8),
+.BR semanage-dontaudit (8),
+.BR semanage-export (8),
+.BR semanage-fcontext (8),
+.BR semanage-import (8),
+.BR semanage-interface (8),
+.BR semanage-login (8),
+.BR semanage-module (8),
+.BR semanage-node (8),
+.BR semanage-permissive (8),
+.BR semanage-port (8),
+.BR semanage-user (8)
 
 .SH "AUTHOR"
 This man page was written by Daniel Walsh <dwalsh@redhat.com>
-- 
2.9.3

[PATCH] restorecon manpage: link back to fixfiles

[Alan Jenkins]

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(+)

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)
-- 
2.9.3

Re: [PATCH] restorecon manpage: link back to fixfiles

[Stephen Smalley]

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)

Re: [PATCH] restorecon manpage: link back to fixfiles

[Alan Jenkins]

[-- Attachment #1: Type: text/plain, Size: 1667 bytes --]

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).

[-- Attachment #2: Type: text/html, Size: 2546 bytes --]

Re: [PATCH] restorecon manpage: link back to fixfiles

[Stephen Smalley]

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.

Re: [PATCH] restorecon manpage: link back to fixfiles

[Alan Jenkins]

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.

Re: [PATCH] restorecon manpage: link back to fixfiles

[Stephen Smalley]

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).

Re: [PATCH] restorecon manpage: link back to fixfiles

[Stephen Smalley]

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.

Re: [PATCH] restorecon manpage: link back to fixfiles

[Stephen Smalley]

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

Re: [PATCH] restorecon manpage: link back to fixfiles

[Daniel J Walsh]

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`.

Re: [PATCH] restorecon manpage: link back to fixfiles

[Stephen Smalley]

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.

Re: [PATCH] restorecon manpage: link back to fixfiles

[Alan Jenkins]

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

Re: [PATCH] restorecon manpage: link back to fixfiles

[Alan Jenkins]

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