policycoreutils: semodule: Enable CIL logging
diff mbox series

Message ID 20191021055505.25372-1-jason@perfinion.com
State Accepted
Headers show
Series
  • policycoreutils: semodule: Enable CIL logging
Related show

Commit Message

Jason Zaman Oct. 21, 2019, 5:55 a.m. UTC
semodule -v will turn on semodule's own verbose logging but not logging
from CIL. This change makes the verbose flag also set cil's log level.

By default (ie no -v flag), this will enable CIL_ERR, and each -v will
increase the level from there.

Tested with a duplicated fcontext in the policy.
Before this change:
    # semodule -v -B
    Committing changes:
    Problems processing filecon rules
    Failed post db handling
    semodule:  Failed!

After this change:
    # semodule -v -B
    [ ... snip ... ]
    Found conflicting filecon rules
      at /var/lib/selinux/mcs/tmp/modules/400/mycustom/cil:159
      at /var/lib/selinux/mcs/tmp/modules/400/mycustom/cil:158
    Problems processing filecon rules
    Failed post db handling
    semodule:  Failed!

Closes: https://github.com/SELinuxProject/selinux/issues/176
Signed-off-by: Jason Zaman <jason@perfinion.com>
---

I also opened a PR here to run travis tests: https://github.com/SELinuxProject/selinux/pull/182

This only affects semodule -v, I tested out setsebool and it doesnt die
on a duplicated fcontext so I skipped it there. Should all the tools set
it or only as-needed? Do we want to make some general guidelines for
what kind of tools should set the CIL logging?

-- Jason


 policycoreutils/semodule/semodule.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

Comments

Stephen Smalley Oct. 23, 2019, 2:23 p.m. UTC | #1
On 10/21/19 1:55 AM, Jason Zaman wrote:
> semodule -v will turn on semodule's own verbose logging but not logging
> from CIL. This change makes the verbose flag also set cil's log level.
> 
> By default (ie no -v flag), this will enable CIL_ERR, and each -v will
> increase the level from there.
> 
> Tested with a duplicated fcontext in the policy.
> Before this change:
>      # semodule -v -B
>      Committing changes:
>      Problems processing filecon rules
>      Failed post db handling
>      semodule:  Failed!
> 
> After this change:
>      # semodule -v -B
>      [ ... snip ... ]
>      Found conflicting filecon rules
>        at /var/lib/selinux/mcs/tmp/modules/400/mycustom/cil:159
>        at /var/lib/selinux/mcs/tmp/modules/400/mycustom/cil:158
>      Problems processing filecon rules
>      Failed post db handling
>      semodule:  Failed!
> 
> Closes: https://github.com/SELinuxProject/selinux/issues/176
> Signed-off-by: Jason Zaman <jason@perfinion.com>
> ---
> 
> I also opened a PR here to run travis tests: https://github.com/SELinuxProject/selinux/pull/182
> 
> This only affects semodule -v, I tested out setsebool and it doesnt die
> on a duplicated fcontext so I skipped it there. Should all the tools set
> it or only as-needed? Do we want to make some general guidelines for
> what kind of tools should set the CIL logging?

Thanks, applied. Not sure about whether other tools should set the cil 
log level.  I think current cil logging leaves a lot to be desired, 
especially since the pathnames identified in the output when invoked 
from semodule/libsemanage are temporary.  It would be preferable IMHO to 
display the actual conflicting rules themselves and not just the file 
name and line number.

> 
> -- Jason
> 
> 
>   policycoreutils/semodule/semodule.c | 5 ++++-
>   1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/policycoreutils/semodule/semodule.c b/policycoreutils/semodule/semodule.c
> index a76797f5..a1f75e16 100644
> --- a/policycoreutils/semodule/semodule.c
> +++ b/policycoreutils/semodule/semodule.c
> @@ -22,6 +22,7 @@
>   #include <libgen.h>
>   #include <limits.h>
>   
> +#include <sepol/cil/cil.h>
>   #include <semanage/modules.h>
>   
>   enum client_modes {
> @@ -238,7 +239,7 @@ static void parse_command_line(int argc, char **argv)
>   			set_mode(LIST_M, optarg);
>   			break;
>   		case 'v':
> -			verbose = 1;
> +			verbose++;
>   			break;
>   		case 'r':
>   			set_mode(REMOVE_M, optarg);
> @@ -350,6 +351,8 @@ int main(int argc, char *argv[])
>   	}
>   	parse_command_line(argc, argv);
>   
> +	cil_set_log_level(CIL_ERR + verbose);
> +
>   	if (build)
>   		commit = 1;
>   
>

Patch
diff mbox series

diff --git a/policycoreutils/semodule/semodule.c b/policycoreutils/semodule/semodule.c
index a76797f5..a1f75e16 100644
--- a/policycoreutils/semodule/semodule.c
+++ b/policycoreutils/semodule/semodule.c
@@ -22,6 +22,7 @@ 
 #include <libgen.h>
 #include <limits.h>
 
+#include <sepol/cil/cil.h>
 #include <semanage/modules.h>
 
 enum client_modes {
@@ -238,7 +239,7 @@  static void parse_command_line(int argc, char **argv)
 			set_mode(LIST_M, optarg);
 			break;
 		case 'v':
-			verbose = 1;
+			verbose++;
 			break;
 		case 'r':
 			set_mode(REMOVE_M, optarg);
@@ -350,6 +351,8 @@  int main(int argc, char *argv[])
 	}
 	parse_command_line(argc, argv);
 
+	cil_set_log_level(CIL_ERR + verbose);
+
 	if (build)
 		commit = 1;