| Message ID | CAJfZ7==2e9bN7Z94sLDO3boHDgVPx8Gr+=pXWsvNEbdqkjc8fg@mail.gmail.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
On 03/11/2017 03:02 PM, Nicolas Iooss wrote: > On Fri, Mar 10, 2017 at 8:49 PM, James Carter <jwcart2@tycho.nsa.gov> 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. > Thanks for testing it out. I agree with all of your comments and suggestions above. Thanks for the patch as well. Jim > Cheers, > Nicolas >
From c25a08efc3f6596c0f3bfdcb2e36abca68b5b3e8 Mon Sep 17 00:00:00 2001 From: Nicolas Iooss <nicolas.iooss@m4x.org> 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