On Wed, 4 May 2022 at 14:52, Petr Lautrbach <plautrba@xxxxxxxxxx> wrote:
>
> Christian Göttsche <cgzones@xxxxxxxxxxxxxx> writes:
>
> > Add command line options to getsebool(8) to display either all enabled
> > or all disabled booleans.
>
> I'm curious what would you use this for?
I tried to list all enabled booleans and `getsebool -a | grep on`
listed also ones with 'on' in their name.
Using `getsebool -a | grep on'` works however, so this patch is not essential.
>
> Another comment is bellow
>
>
> > Signed-off-by: Christian Göttsche <cgzones@xxxxxxxxxxxxxx>
> > ---
> > libselinux/man/man8/getsebool.8 | 8 +++++++-
> > libselinux/utils/getsebool.c | 36 +++++++++++++++++++++++++++------
> > 2 files changed, 37 insertions(+), 7 deletions(-)
> >
> > diff --git a/libselinux/man/man8/getsebool.8 b/libselinux/man/man8/getsebool.8
> > index d70bf1e4..d8356d36 100644
> > --- a/libselinux/man/man8/getsebool.8
> > +++ b/libselinux/man/man8/getsebool.8
> > @@ -4,7 +4,7 @@ getsebool \- get SELinux boolean value(s)
> > .
> > .SH "SYNOPSIS"
> > .B getsebool
> > -.RB [ \-a ]
> > +.RB [ \-a | \-0 | \-1 ]
> > .RI [ boolean ]
> > .
> > .SH "DESCRIPTION"
> > @@ -26,6 +26,12 @@ their pending values as desired and then committing once.
> > .TP
> > .B \-a
> > Show all SELinux booleans.
> > +.TP
> > +.B \-0
> > +Show all disabled SELinux booleans.
> > +.TP
> > +.B \-1
> > +Show all enabled SELinux booleans.
> > .
> > .SH AUTHOR
> > This manual page was written by Dan Walsh <dwalsh@xxxxxxxxxx>.
> > diff --git a/libselinux/utils/getsebool.c b/libselinux/utils/getsebool.c
> > index 36994536..7fb0b58b 100644
> > --- a/libselinux/utils/getsebool.c
> > +++ b/libselinux/utils/getsebool.c
> > @@ -6,21 +6,31 @@
> > #include <string.h>
> > #include <selinux/selinux.h>
> >
> > +enum list_mode {
> > + SPECIFIED,
> > + ALL,
> > + DISABLED,
> > + ENABLED,
> > +};
> > +
> > static __attribute__ ((__noreturn__)) void usage(const char *progname)
> > {
> > - fprintf(stderr, "usage: %s -a or %s boolean...\n", progname, progname);
> > + fprintf(stderr, "usage: %s [-a|-0|-1] or %s boolean...\n", progname, progname);
> > exit(1);
> > }
> >
> > int main(int argc, char **argv)
> > {
> > - int i, get_all = 0, rc = 0, active, pending, len = 0, opt;
> > + int i, rc = 0, active, pending, len = 0, opt;
> > + enum list_mode mode = SPECIFIED;
> > char **names = NULL;
> >
> > - while ((opt = getopt(argc, argv, "a")) > 0) {
> > + while ((opt = getopt(argc, argv, "a01")) > 0) {
> > switch (opt) {
> > case 'a':
> > - if (argc > 2)
> > + case '0':
> > + case '1':
> > + if (argc > 2 || mode != SPECIFIED)
> > usage(argv[0]);
> > if (is_selinux_enabled() <= 0) {
> > fprintf(stderr, "%s: SELinux is disabled\n",
> > @@ -39,7 +49,17 @@ int main(int argc, char **argv)
> > printf("No booleans\n");
> > return 0;
> > }
> > - get_all = 1;
> > + switch (opt) {
> > + case 'a':
> > + mode = ALL;
> > + break;
> > + case '0':
> > + mode = DISABLED;
> > + break;
> > + case '1':
> > + mode = ENABLED;
> > + break;
> > + }
>
> switch(opt) inside switch(opt) block looks strange for me. Would it make
> sense to have just this switch to set mode and move the code from line
> 35 around is_selinux_enabled() and security_get_boolean_names() after the switch?
>
Yes, that would make sense; the change I made aimed for a minimal diff size.
> Petr
>
>
> > break;
> > default:
> > usage(argv[0]);
> > @@ -74,7 +94,7 @@ int main(int argc, char **argv)
> > for (i = 0; i < len; i++) {
> > active = security_get_boolean_active(names[i]);
> > if (active < 0) {
> > - if (get_all && errno == EACCES)
> > + if (mode != SPECIFIED && errno == EACCES)
> > continue;
> > fprintf(stderr, "Error getting active value for %s\n",
> > names[i]);
> > @@ -88,6 +108,10 @@ int main(int argc, char **argv)
> > rc = -1;
> > goto out;
> > }
> > + if ((mode == ENABLED && active == 0 && pending == 0) ||
> > + (mode == DISABLED && active == 1 && pending == 1)) {
> > + continue;
> > + }
> > char *alt_name = selinux_boolean_sub(names[i]);
> > if (! alt_name) {
> > perror("Out of memory\n");
> > --
> > 2.36.0
>
