Re: [PATCH] libselinux/utils/getsebool: add options to display en-/disabled booleans

[Petr Lautrbach <plautrba@redhat.com>]

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