diff mbox series

libselinux/utils/getsebool: add options to display en-/disabled booleans

Message ID 20220428172219.28520-1-cgzones@googlemail.com (mailing list archive)
State New
Headers show
Series libselinux/utils/getsebool: add options to display en-/disabled booleans | expand

Commit Message

Christian Göttsche April 28, 2022, 5:22 p.m. UTC
Add command line options to getsebool(8) to display either all enabled
or all disabled booleans.

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
---
 libselinux/man/man8/getsebool.8 |  8 +++++++-
 libselinux/utils/getsebool.c    | 36 +++++++++++++++++++++++++++------
 2 files changed, 37 insertions(+), 7 deletions(-)

Comments

Petr Lautrbach May 4, 2022, 12:52 p.m. UTC | #1
Christian Göttsche <cgzones@googlemail.com> 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?

Another comment is bellow


> Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
> ---
>  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@redhat.com>.
> 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?

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
Christian Göttsche May 10, 2022, 5:20 p.m. UTC | #2
On Wed, 4 May 2022 at 14:52, Petr Lautrbach <plautrba@redhat.com> wrote:
>
> Christian Göttsche <cgzones@googlemail.com> 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@googlemail.com>
> > ---
> >  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@redhat.com>.
> > 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
>
Petr Lautrbach May 17, 2022, 12:26 p.m. UTC | #3
Christian Göttsche <cgzones@googlemail.com> writes:

> On Wed, 4 May 2022 at 14:52, Petr Lautrbach <plautrba@redhat.com> wrote:
>>
>> Christian Göttsche <cgzones@googlemail.com> 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@googlemail.com>
>> > ---
>> >  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@redhat.com>.
>> > 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
>>
diff mbox series

Patch

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@redhat.com>.
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;
+			}
 			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");