On Fri, Mar 10, 2017 at 8:49 PM, James Carter <jwcart2@xxxxxxxxxxxxx> wrote:
> It would sometimes be helpful for debugging or verification purposes to be able to convert
> a binary policy to a human-readable form.
>
> This patchset adds libsepol functions that take a kernel policydb in and outputs either
> a CIL or policy.conf text.
>
> Checkpolicy is modified to generate CIL text from a binary policy if using the "-C" option
> and to add the "-F" option to generate policy.conf text from a binary policy.
>
> Where possible rules are sorted in alphabetical or numerical order to aid in debugging.
>
> James Carter (3):
> libsepol: Add ability to convert binary policy to CIL
> libsepol: Add ability to convert binary policy to policy.conf file
> checkpolicy: Add options to convert binary policy to CIL or a
> policy.conf
Hello,
I agree such features are useful when analyzing policies. I have
tested the patches and compiled with some warnings flags (I have not
done a "real" code review). Here are some comments:
- First, the documentation (at least checkpolicy manpage) needs to be
updated with the new options. Also I spent some time trying to
understand what I was doing wrong with options -C and -F until I read
the third patch and understood that "-b" needs to be used as well.
- Then, when using "checkpolicy -bF" on my policy, I got blocks such as:
if ((git_cgi_enable_homedirs && use_samba_home_dirs)) {
allow httpd_git_script_t cifs_t:dir { getattr search open };
allow httpd_git_script_t cifs_t:dir { ioctl read getattr lock
search open };
allow httpd_git_script_t cifs_t:dir { ioctl read getattr lock
search open };
allow httpd_git_script_t cifs_t:file { ioctl read getattr lock open };
allow httpd_git_script_t cifs_t:filesystem { getattr };
} else { dontaudit httpd_git_script_t cifs_t:file { ioctl
read getattr lock open };
}
The blocks have indentation issues (the first line of the if statement
has too much spaces and a "\n" is missing at the beginning of the else
block. Moreover the permissions are not sorted in alphabetical order
but this may be included in the "Where possible" part of your message.
- Third, the first patch introduces functions with printf-like
arguments and defines them with __attribute__((format(printf...))) in
the .c file. This is fine for the static functions but the other ones
need to have attributes in kernel_to_common.h instead (this enables
the compiler to check the format strings at build time when compiling
other files).
- Finally when compiling with -Wwrite-strings, gcc reports some issues
with literal strings assigned to non-const char* variables. I have
quickly written a short patch to make some variables const char* and
checkpolicy seems to work fine with it. The patch is attached to this
email if you want to consider it.
Cheers,
Nicolas
From c25a08efc3f6596c0f3bfdcb2e36abca68b5b3e8 Mon Sep 17 00:00:00 2001
From: Nicolas Iooss <nicolas.iooss@xxxxxxx>
Date: Sat, 11 Mar 2017 10:26:45 +0100
Subject: [PATCH 1/1] fix -Wwrite-string warnings
---
libsepol/src/kernel_to_cil.c | 11 ++++++-----
libsepol/src/kernel_to_conf.c | 12 +++++++-----
2 files changed, 13 insertions(+), 10 deletions(-)
diff --git a/libsepol/src/kernel_to_cil.c b/libsepol/src/kernel_to_cil.c
index 143f7faa4a07..0854426fae33 100644
--- a/libsepol/src/kernel_to_cil.c
+++ b/libsepol/src/kernel_to_cil.c
@@ -266,7 +266,7 @@ static int class_constraint_rules_to_strs(struct policydb *pdb, char *classkey,
char *expr = NULL;
int is_mls;
char *perms;
- char *format_str;
+ const char *format_str;
struct strs *strs;
for (curr = constraint_rules; curr != NULL; curr = curr->next) {
@@ -307,7 +307,7 @@ static int class_validatetrans_rules_to_strs(struct policydb *pdb, char *classke
struct constraint_node *curr;
char *expr = NULL;
int is_mls;
- char *format_str;
+ const char *format_str;
struct strs *strs;
int rc = 0;
@@ -994,7 +994,8 @@ static char *cats_ebitmap_to_str(struct ebitmap *cats, char **val_to_name)
{
struct ebitmap_node *node;
uint32_t i, start, range;
- char *catsbuf, *p, *fmt;
+ char *catsbuf, *p;
+ const char *fmt;
int len, remaining;
remaining = (int)cats_ebitmap_len(cats, val_to_name);
@@ -1605,10 +1606,10 @@ static char *xperms_to_str(avtab_extended_perms_t *xperms)
static char *avtab_node_to_str(struct policydb *pdb, avtab_key_t *key, avtab_datum_t *datum)
{
- const char *flavor;
+ const char *flavor, *tgt;
uint32_t data = datum->data;
type_datum_t *type;
- char *src, *tgt, *class, *perms, *new;
+ char *src, *class, *perms, *new;
char *rule = NULL;
switch (0xFFF & key->specified) {
diff --git a/libsepol/src/kernel_to_conf.c b/libsepol/src/kernel_to_conf.c
index 5e5a8647b426..898fd1baf38f 100644
--- a/libsepol/src/kernel_to_conf.c
+++ b/libsepol/src/kernel_to_conf.c
@@ -260,7 +260,8 @@ static int class_constraint_rules_to_strs(struct policydb *pdb, char *classkey,
int rc = 0;
struct constraint_node *curr;
int is_mls;
- char *format_str, *flavor, *perms, *expr;
+ const char *format_str, *flavor;
+ char *perms, *expr;
struct strs *strs;
for (curr = constraint_rules; curr != NULL; curr = curr->next) {
@@ -305,7 +306,8 @@ static int class_validatetrans_rules_to_strs(struct policydb *pdb, char *classke
{
struct constraint_node *curr;
int is_mls;
- char *flavor, *expr;
+ const char *flavor;
+ char *expr;
struct strs *strs;
int rc = 0;
@@ -969,7 +971,8 @@ static char *cats_ebitmap_to_str(struct ebitmap *cats, char **val_to_name)
{
struct ebitmap_node *node;
uint32_t i, start, range, first;
- char *catsbuf, *p, *fmt;
+ char *catsbuf, *p;
+ const char *fmt;
char sep;
int len, remaining;
@@ -1573,10 +1576,9 @@ exit:
static char *avtab_node_to_str(struct policydb *pdb, avtab_key_t *key, avtab_datum_t *datum)
{
- const char *flavor;
+ const char *flavor, *src, *tgt, *class, *perms, *new;
uint32_t data = datum->data;
type_datum_t *type;
- char *src, *tgt, *class, *perms, *new;
char *rule = NULL;
switch (0xFFF & key->specified) {
--
2.11.1
_______________________________________________
Selinux mailing list
Selinux@xxxxxxxxxxxxx
To unsubscribe, send email to Selinux-leave@xxxxxxxxxxxxx.
To get help, send an email containing "help" to Selinux-request@xxxxxxxxxxxxx.
