Christian Göttsche <cgzones@xxxxxxxxxxxxxx> writes:
> 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.
I see.
>>
>> 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.
Could you please change it?
>
>> 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
>>
