Re: [PATCH 1/1] libsepol/cil: Improve processing of context rules

2018-03-29 Thread Pierre-Hugues Husson
The commit message and the code match my needs, and the few tests (not
unit tests at all though) I have are passing.
I'll test it on real devices, and I'll report back if anything seem wrong.

Just as a reminder, the only problems I hit for the moment are on
genfscon, so that's the only thing I've checked.

Thanks!

2018-03-29 22:14 GMT+02:00 jwcart2 <jwca...@tycho.nsa.gov>:
> Pierre-Hugues Husson, I've tested and everything seems to work as I expect
> it, but does this meet your needs?
>
> Jim
>
>
> On 03/29/2018 04:06 PM, James Carter wrote:
>>
>> Improve the processing of netifcon, genfscon, ibpkeycon, ibendportcon,
>> portcon, nodecon, fsuse, filecon, iomemcon, ioportcon, pcidevicecon,
>> and devicetreecon rules.
>>
>> If the multiple-decls option is not used then report errors if duplicate
>> context rules are found. If it is used then remove duplicate context rules
>> and report errors when two rules are identical except for the context.
>>
>> This also changes the ordering of portcon and filecon rules. The protocol
>> of portcon rules will be compared if the port numbers are the same and the
>> path strings of filecon rules will be compared if the number of meta
>> characters, the stem length, string length and file types are the same.
>>
>> Based on an initial patch by Pierre-Hugues Husson (p...@phh.me)
>>
>> Signed-off-by: James Carter <jwca...@tycho.nsa.gov>
>> ---
>>   libsepol/cil/src/cil_post.c | 331
>> ++--
>>   1 file changed, 318 insertions(+), 13 deletions(-)
>>
>> diff --git a/libsepol/cil/src/cil_post.c b/libsepol/cil/src/cil_post.c
>> index a2122454..0b09cecc 100644
>> --- a/libsepol/cil/src/cil_post.c
>> +++ b/libsepol/cil/src/cil_post.c
>> @@ -53,6 +53,83 @@
>>   static int __cil_expr_to_bitmap(struct cil_list *expr, ebitmap_t *out,
>> int max, struct cil_db *db);
>>   static int __cil_expr_list_to_bitmap(struct cil_list *expr_list,
>> ebitmap_t *out, int max, struct cil_db *db);
>>   +static int cats_compare(struct cil_cats *a, struct cil_cats *b)
>> +{
>> +   struct cil_list_item *i, *j;
>> +   int rc;
>> +
>> +   if (a == b) return 0;
>> +   if (!a) return -1;
>> +   if (!b) return 1;
>> +
>> +   /* Expects cat expression to have been evaluated */
>> +   cil_list_for_each(i, a->datum_expr) {
>> +   cil_list_for_each(j, b->datum_expr) {
>> +   rc = strcmp(DATUM(i->data)->fqn,
>> DATUM(j->data)->fqn);
>> +   if (!rc) return rc;
>> +   }
>> +   }
>> +   return 0;
>> +}
>> +
>> +static int level_compare(struct cil_level *a, struct cil_level *b)
>> +{
>> +   int rc;
>> +
>> +   if (a == b) return 0;
>> +   if (!a) return -1;
>> +   if (!b) return 1;
>> +
>> +   if (a->sens != b->sens) {
>> +   rc = strcmp(DATUM(a->sens)->fqn, DATUM(b->sens)->fqn);
>> +   if (rc != 0) return rc;
>> +   }
>> +   if (a->cats != b->cats) {
>> +   return cats_compare(a->cats, b->cats);
>> +   }
>> +   return 0;
>> +}
>> +
>> +static int range_compare(struct cil_levelrange *a, struct cil_levelrange
>> *b)
>> +{
>> +   int rc;
>> +
>> +   if (a == b) return 0;
>> +   if (!a) return -1;
>> +   if (!b) return 1;
>> +
>> +   if (a->low != b->low) {
>> +   rc = level_compare(a->low, b->low);
>> +   if (rc != 0) return rc;
>> +   }
>> +   if (a->high != b->high) {
>> +   return level_compare(a->high, b->high);
>> +   }
>> +   return 0;
>> +}
>> +
>> +static int context_compare(struct cil_context *a, struct cil_context *b)
>> +{
>> +   int rc;
>> +
>> +   if (a->user != b->user) {
>> +   rc = strcmp(DATUM(a->user)->fqn, DATUM(b->user)->fqn);
>> +   if (rc != 0) return rc;
>> +   }
>> +   if (a->role != b->role) {
>> +   rc = strcmp(DATUM(a->role)->fqn, DATUM(b->role)->fqn);
>> +   if (rc != 0) return rc;
>> +   }
>> +   if (a->type != b->type) {
>> +   rc = strcmp(DATUM(a->type)->fqn, DATUM(b->type)->fqn);
>> +   if (rc != 0) return rc;
>> +   }
>> + 

Re: [PATCH v2 1/1] Detect identical genfscon

2018-03-29 Thread Pierre-Hugues Husson
2018-03-23 0:04 GMT+01:00 Pierre-Hugues Husson <p...@phh.me>:
> From: Pierre-Hugues Husson <phhus...@gmail.com>
>
> Currently secilc doesn't deal with duplicate genfscon rules
>
> This commit fixes this, and implements multiple_decls behaviour.
>
> To reduce the code changes, the compare function returns in its LSB
> whether the rules are only a matching rule match, or a full match.
> ---
>  libsepol/cil/src/cil_post.c | 34 --
>  1 file changed, 32 insertions(+), 2 deletions(-)
>
> diff --git a/libsepol/cil/src/cil_post.c b/libsepol/cil/src/cil_post.c
> index a2122454..c054e9ce 100644
> --- a/libsepol/cil/src/cil_post.c
> +++ b/libsepol/cil/src/cil_post.c
> @@ -53,6 +53,26 @@
>  static int __cil_expr_to_bitmap(struct cil_list *expr, ebitmap_t *out, int 
> max, struct cil_db *db);
>  static int __cil_expr_list_to_bitmap(struct cil_list *expr_list, ebitmap_t 
> *out, int max, struct cil_db *db);
>
> +/* compare function returns whether a,b have the same context in the LSB */
> +static int compact(void* array, uint32_t *count, int len, int 
> (*compare)(const void *, const void *), int multiple_decls) {
> +   char *a = (char*)array;
> +   uint32_t i, j = 0;
> +   int c;
> +   for(i=1; i<*count; i++) {
> +   c = compare(a+i*len, a+j*len);
> +   /* If LSB is set, it means the rules match except for the 
> context
> +* We never want this */
> +   if(c&1) return SEPOL_ERR;
> +
> +   if(!multiple_decls && c == 0) return SEPOL_ERR;
> +
> +   if(c) j++;
> +   if(i != j) memcpy(a+j*len, a+i*len, len);
> +   }
> +   *count = j;
I've just realized this should actually be j+1



[PATCH v2 1/1] Detect identical genfscon

2018-03-22 Thread Pierre-Hugues Husson
From: Pierre-Hugues Husson <phhus...@gmail.com>

Currently secilc doesn't deal with duplicate genfscon rules

This commit fixes this, and implements multiple_decls behaviour.

To reduce the code changes, the compare function returns in its LSB
whether the rules are only a matching rule match, or a full match.
---
 libsepol/cil/src/cil_post.c | 34 --
 1 file changed, 32 insertions(+), 2 deletions(-)

diff --git a/libsepol/cil/src/cil_post.c b/libsepol/cil/src/cil_post.c
index a2122454..c054e9ce 100644
--- a/libsepol/cil/src/cil_post.c
+++ b/libsepol/cil/src/cil_post.c
@@ -53,6 +53,26 @@
 static int __cil_expr_to_bitmap(struct cil_list *expr, ebitmap_t *out, int 
max, struct cil_db *db);
 static int __cil_expr_list_to_bitmap(struct cil_list *expr_list, ebitmap_t 
*out, int max, struct cil_db *db);
 
+/* compare function returns whether a,b have the same context in the LSB */
+static int compact(void* array, uint32_t *count, int len, int (*compare)(const 
void *, const void *), int multiple_decls) {
+   char *a = (char*)array;
+   uint32_t i, j = 0;
+   int c;
+   for(i=1; i<*count; i++) {
+   c = compare(a+i*len, a+j*len);
+   /* If LSB is set, it means the rules match except for the 
context
+* We never want this */
+   if(c&1) return SEPOL_ERR;
+
+   if(!multiple_decls && c == 0) return SEPOL_ERR;
+
+   if(c) j++;
+   if(i != j) memcpy(a+j*len, a+i*len, len);
+   }
+   *count = j;
+   return SEPOL_OK;
+}
+
 static int cil_verify_is_list(struct cil_list *list, enum cil_flavor flavor)
 {
struct cil_list_item *curr;
@@ -202,9 +222,14 @@ int cil_post_genfscon_compare(const void *a, const void *b)
struct cil_genfscon *agenfscon = *(struct cil_genfscon**)a;
struct cil_genfscon *bgenfscon = *(struct cil_genfscon**)b;
 
-   rc = strcmp(agenfscon->fs_str, bgenfscon->fs_str);
+   rc = 2*strcmp(agenfscon->fs_str, bgenfscon->fs_str);
if (rc == 0) {
-   rc = strcmp(agenfscon->path_str, bgenfscon->path_str);
+   rc = 2*strcmp(agenfscon->path_str, bgenfscon->path_str);
+   if(rc == 0) {
+   rc = strcmp(agenfscon->context_str, 
bgenfscon->context_str);
+   if(rc > 0) rc = 1;
+   if(rc < 0) rc = -1;
+   }
}
 
return rc;
@@ -2118,6 +2143,11 @@ static int cil_post_db(struct cil_db *db)
 
qsort(db->netifcon->array, db->netifcon->count, 
sizeof(db->netifcon->array), cil_post_netifcon_compare);
qsort(db->genfscon->array, db->genfscon->count, 
sizeof(db->genfscon->array), cil_post_genfscon_compare);
+   rc = compact(db->genfscon->array, >genfscon->count, 
sizeof(db->genfscon->array), cil_post_genfscon_compare, db->multiple_decls);
+   if (rc != SEPOL_OK) {
+   cil_log(CIL_INFO, "Failed to de-dupe genfscon\n");
+   goto exit;
+   }
qsort(db->ibpkeycon->array, db->ibpkeycon->count, 
sizeof(db->ibpkeycon->array), cil_post_ibpkeycon_compare);
qsort(db->ibendportcon->array, db->ibendportcon->count, 
sizeof(db->ibendportcon->array), cil_post_ibendportcon_compare);
qsort(db->portcon->array, db->portcon->count, 
sizeof(db->portcon->array), cil_post_portcon_compare);
-- 
2.15.1




[PATCH v2 0/1] Detect identical genfscon

2018-03-22 Thread Pierre-Hugues Husson
Currently secilc doesn't deal with duplicate genfscon rules.

This commit fixes this, and implements multiple_decls behaviour.

To reduce the code changes, the compare function returns in its LSB
whether the rules are only a matching rule match, or a full match.

One usecase is Android/Project Treble:
With Project Treble, vendor might include rules included in later
in framework.
In order to be able to update the framework in this case, we need
to remove identical rules.

This is a RFC version, this hasn't been properly tested.

v2:
- Respect multiple_decls behaviour
- Fail merge when context is different
- genfscon compare function returns partial or full match

Pierre-Hugues Husson (1):
  Detect identical genfscon

 libsepol/cil/src/cil_post.c | 34 --
 1 file changed, 32 insertions(+), 2 deletions(-)

-- 
2.15.1




[PATCH 0/1] Support multiple identical genfscon

2018-03-19 Thread Pierre-Hugues Husson
secilc has a multiple_decls option to allow for multiple type
declarations.
The next step is to allow multiple samples of the same rules.
This commit does this on genfscon

One usecase is Android/Project Treble:
With Project Treble, vendor might include rules included in later
in framework.
In order to be able to update the framework in this case, we need
to remove identical rules.

I have several pending questions before considering merging:

Should the "compact" function be somewhere else? Or perhaps there is already
some variant available?
Should the "compact" function simply take a cil_sort rather than a C array?
Should we compact all types indifferently?
If so, we need to guarantee that the _compare function returns 0 only when the
types rules are identical, and not just the same match rule. Is this already
the case?
How is memory allocation done/will compact impact the release of the memory?
In my understanding this is just one big chunk, so the size isn't used when
free-ing, so it should be ok


Pierre-Hugues Husson (1):
  Delete identical genfscon-s

 libsepol/cil/src/cil_post.c | 11 +++
 1 file changed, 11 insertions(+)

-- 
2.15.1




[PATCH 1/1] Delete identical genfscon-s

2018-03-19 Thread Pierre-Hugues Husson
From: Pierre-Hugues Husson <phhus...@gmail.com>

secilc has a multiple_decls option to allow for multiple type
declarations.
The next step is to allow multiple samples of the same rules.
This commit does this on genfscon

One usecase is Android/Project Treble:
With Project Treble, vendor might include rules included in later
in framework.
In order to be able to update the framework in this case, we need
to remove identical rules.
---
 libsepol/cil/src/cil_post.c | 11 +++
 1 file changed, 11 insertions(+)

diff --git a/libsepol/cil/src/cil_post.c b/libsepol/cil/src/cil_post.c
index a2122454..8446158e 100644
--- a/libsepol/cil/src/cil_post.c
+++ b/libsepol/cil/src/cil_post.c
@@ -53,6 +53,16 @@
 static int __cil_expr_to_bitmap(struct cil_list *expr, ebitmap_t *out, int 
max, struct cil_db *db);
 static int __cil_expr_list_to_bitmap(struct cil_list *expr_list, ebitmap_t 
*out, int max, struct cil_db *db);
 
+static int compact(void* array, int count, int len, int (*compar)(const void 
*, const void *)) {
+   char *a = (char*)array;
+   int i, j = 0;
+   for(i=1; i<count; i++) {
+   if(compar(a+i*len, a+j*len) != 0) j++;
+   if(i != j) memcpy(a+j*len, a+i*len, len);
+   }
+   return j;
+}
+
 static int cil_verify_is_list(struct cil_list *list, enum cil_flavor flavor)
 {
struct cil_list_item *curr;
@@ -2118,6 +2128,7 @@ static int cil_post_db(struct cil_db *db)
 
qsort(db->netifcon->array, db->netifcon->count, 
sizeof(db->netifcon->array), cil_post_netifcon_compare);
qsort(db->genfscon->array, db->genfscon->count, 
sizeof(db->genfscon->array), cil_post_genfscon_compare);
+   db->genfscon->count = compact(db->genfscon->array, db->genfscon->count, 
sizeof(db->genfscon->array), cil_post_genfscon_compare);
qsort(db->ibpkeycon->array, db->ibpkeycon->count, 
sizeof(db->ibpkeycon->array), cil_post_ibpkeycon_compare);
qsort(db->ibendportcon->array, db->ibendportcon->count, 
sizeof(db->ibendportcon->array), cil_post_ibendportcon_compare);
qsort(db->portcon->array, db->portcon->count, 
sizeof(db->portcon->array), cil_post_portcon_compare);
-- 
2.15.1