* [PATCH 1/2] libsepol: reject xperm av rules in conditional statements
@ 2022-03-08 18:58 Christian Göttsche
2022-03-08 18:58 ` [PATCH 2/2] libsepol: validate boolean datum arrays Christian Göttsche
2022-03-11 14:57 ` [PATCH 1/2] libsepol: reject xperm av rules in conditional statements James Carter
0 siblings, 2 replies; 4+ messages in thread
From: Christian Göttsche @ 2022-03-08 18:58 UTC (permalink / raw)
To: selinux
Extended permission and neverallow rules are not permitted in
conditional statements.
This causes issues on policy optimization where avtab_search() might
return a non extended permission rule when searching for one.
Found by oss-fuzz (#45327)
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
---
libsepol/src/policydb_validate.c | 28 +++++++++++++++++-----------
1 file changed, 17 insertions(+), 11 deletions(-)
diff --git a/libsepol/src/policydb_validate.c b/libsepol/src/policydb_validate.c
index 735c7a33..72063351 100644
--- a/libsepol/src/policydb_validate.c
+++ b/libsepol/src/policydb_validate.c
@@ -658,7 +658,7 @@ bad:
* Functions to validate a kernel policydb
*/
-static int validate_avtab_key(avtab_key_t *key, validate_t flavors[])
+static int validate_avtab_key(avtab_key_t *key, int conditional, validate_t flavors[])
{
if (validate_value(key->source_type, &flavors[SYM_TYPES]))
goto bad;
@@ -670,13 +670,16 @@ static int validate_avtab_key(avtab_key_t *key, validate_t flavors[])
case AVTAB_ALLOWED:
case AVTAB_AUDITALLOW:
case AVTAB_AUDITDENY:
- case AVTAB_XPERMS_ALLOWED:
- case AVTAB_XPERMS_AUDITALLOW:
- case AVTAB_XPERMS_DONTAUDIT:
case AVTAB_TRANSITION:
case AVTAB_MEMBER:
case AVTAB_CHANGE:
break;
+ case AVTAB_XPERMS_ALLOWED:
+ case AVTAB_XPERMS_AUDITALLOW:
+ case AVTAB_XPERMS_DONTAUDIT:
+ if (conditional)
+ goto bad;
+ break;
default:
goto bad;
}
@@ -691,7 +694,7 @@ static int validate_avtab_key_and_datum(avtab_key_t *k, avtab_datum_t *d, void *
{
validate_t *flavors = (validate_t *)args;
- if (validate_avtab_key(k, flavors))
+ if (validate_avtab_key(k, 0, flavors))
return -1;
if ((k->specified & AVTAB_TYPE) && validate_value(d->data, &flavors[SYM_TYPES]))
@@ -716,7 +719,7 @@ static int validate_cond_av_list(sepol_handle_t *handle, cond_av_list_t *cond_av
for (; cond_av; cond_av = cond_av->next) {
for (avtab_ptr = cond_av->node; avtab_ptr; avtab_ptr = avtab_ptr->next) {
- if (validate_avtab_key(&avtab_ptr->key, flavors)) {
+ if (validate_avtab_key(&avtab_ptr->key, 1, flavors)) {
ERR(handle, "Invalid cond av list");
return -1;
}
@@ -726,7 +729,7 @@ static int validate_cond_av_list(sepol_handle_t *handle, cond_av_list_t *cond_av
return 0;
}
-static int validate_avrules(sepol_handle_t *handle, avrule_t *avrule, validate_t flavors[])
+static int validate_avrules(sepol_handle_t *handle, avrule_t *avrule, int conditional, validate_t flavors[])
{
class_perm_node_t *class;
@@ -746,14 +749,17 @@ static int validate_avrules(sepol_handle_t *handle, avrule_t *avrule, validate_t
case AVRULE_AUDITALLOW:
case AVRULE_AUDITDENY:
case AVRULE_DONTAUDIT:
- case AVRULE_NEVERALLOW:
case AVRULE_TRANSITION:
case AVRULE_MEMBER:
case AVRULE_CHANGE:
+ break;
+ case AVRULE_NEVERALLOW:
case AVRULE_XPERMS_ALLOWED:
case AVRULE_XPERMS_AUDITALLOW:
case AVRULE_XPERMS_DONTAUDIT:
case AVRULE_XPERMS_NEVERALLOW:
+ if (conditional)
+ goto bad;
break;
default:
goto bad;
@@ -814,9 +820,9 @@ static int validate_cond_list(sepol_handle_t *handle, cond_list_t *cond, validat
goto bad;
if (validate_cond_av_list(handle, cond->false_list, flavors))
goto bad;
- if (validate_avrules(handle, cond->avtrue_list, flavors))
+ if (validate_avrules(handle, cond->avtrue_list, 1, flavors))
goto bad;
- if (validate_avrules(handle, cond->avfalse_list, flavors))
+ if (validate_avrules(handle, cond->avfalse_list, 1, flavors))
goto bad;
if (validate_bool_id_array(handle, cond->bool_ids, cond->nbools, &flavors[SYM_BOOLS]))
goto bad;
@@ -1098,7 +1104,7 @@ static int validate_avrule_blocks(sepol_handle_t *handle, avrule_block_t *avrule
for (decl = avrule_block->branch_list; decl != NULL; decl = decl->next) {
if (validate_cond_list(handle, decl->cond_list, flavors))
goto bad;
- if (validate_avrules(handle, decl->avrules, flavors))
+ if (validate_avrules(handle, decl->avrules, 0, flavors))
goto bad;
if (validate_role_trans_rules(handle, decl->role_tr_rules, flavors))
goto bad;
--
2.35.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH 2/2] libsepol: validate boolean datum arrays
2022-03-08 18:58 [PATCH 1/2] libsepol: reject xperm av rules in conditional statements Christian Göttsche
@ 2022-03-08 18:58 ` Christian Göttsche
2022-03-11 14:57 ` [PATCH 1/2] libsepol: reject xperm av rules in conditional statements James Carter
1 sibling, 0 replies; 4+ messages in thread
From: Christian Göttsche @ 2022-03-08 18:58 UTC (permalink / raw)
To: selinux
Validate the boolean entries in the policy.
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
---
libsepol/src/policydb_validate.c | 43 ++++++++++++++++++++++++++++++++
1 file changed, 43 insertions(+)
diff --git a/libsepol/src/policydb_validate.c b/libsepol/src/policydb_validate.c
index 72063351..b51dd366 100644
--- a/libsepol/src/policydb_validate.c
+++ b/libsepol/src/policydb_validate.c
@@ -577,6 +577,41 @@ static int validate_user_datum_wrapper(__attribute__((unused)) hashtab_key_t k,
return validate_user_datum(margs->handle, d, margs->flavors, margs->mls);
}
+static int validate_bool_datum(sepol_handle_t *handle, cond_bool_datum_t *boolean, validate_t flavors[])
+{
+ if (validate_value(boolean->s.value, &flavors[SYM_BOOLS]))
+ goto bad;
+
+ switch (boolean->state) {
+ case 0:
+ case 1:
+ break;
+ default:
+ goto bad;
+ }
+
+ switch (boolean->flags) {
+ case 0:
+ case COND_BOOL_FLAGS_TUNABLE:
+ break;
+ default:
+ goto bad;
+ }
+
+ return 0;
+
+bad:
+ ERR(handle, "Invalid bool datum");
+ return -1;
+}
+
+static int validate_bool_datum_wrapper(__attribute__((unused)) hashtab_key_t k, hashtab_datum_t d, void *args)
+{
+ map_arg_t *margs = args;
+
+ return validate_bool_datum(margs->handle, d, margs->flavors);
+}
+
static int validate_datum_array_gaps(sepol_handle_t *handle, policydb_t *p, validate_t flavors[])
{
unsigned int i;
@@ -607,6 +642,11 @@ static int validate_datum_array_gaps(sepol_handle_t *handle, policydb_t *p, vali
goto bad;
}
+ for (i = 0; i < p->p_bools.nprim; i++) {
+ if (bool_xnor(p->bool_val_to_struct[i], ebitmap_get_bit(&flavors[SYM_BOOLS].gaps, i)))
+ goto bad;
+ }
+
return 0;
bad:
@@ -647,6 +687,9 @@ static int validate_datum_array_entries(sepol_handle_t *handle, policydb_t *p, v
if (hashtab_map(p->p_cats.table, validate_datum, &flavors[SYM_CATS]))
goto bad;
+ if (hashtab_map(p->p_bools.table, validate_bool_datum_wrapper, &margs))
+ goto bad;
+
return 0;
bad:
--
2.35.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH 1/2] libsepol: reject xperm av rules in conditional statements
2022-03-08 18:58 [PATCH 1/2] libsepol: reject xperm av rules in conditional statements Christian Göttsche
2022-03-08 18:58 ` [PATCH 2/2] libsepol: validate boolean datum arrays Christian Göttsche
@ 2022-03-11 14:57 ` James Carter
2022-03-11 16:04 ` James Carter
1 sibling, 1 reply; 4+ messages in thread
From: James Carter @ 2022-03-11 14:57 UTC (permalink / raw)
To: Christian Göttsche; +Cc: SElinux list
On Tue, Mar 8, 2022 at 2:35 PM Christian Göttsche
<cgzones@googlemail.com> wrote:
>
> Extended permission and neverallow rules are not permitted in
> conditional statements.
>
> This causes issues on policy optimization where avtab_search() might
> return a non extended permission rule when searching for one.
>
> Found by oss-fuzz (#45327)
>
> Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
For both patches:
Acked-by: James Carter <jwcart2@gmail.com>
> ---
> libsepol/src/policydb_validate.c | 28 +++++++++++++++++-----------
> 1 file changed, 17 insertions(+), 11 deletions(-)
>
> diff --git a/libsepol/src/policydb_validate.c b/libsepol/src/policydb_validate.c
> index 735c7a33..72063351 100644
> --- a/libsepol/src/policydb_validate.c
> +++ b/libsepol/src/policydb_validate.c
> @@ -658,7 +658,7 @@ bad:
> * Functions to validate a kernel policydb
> */
>
> -static int validate_avtab_key(avtab_key_t *key, validate_t flavors[])
> +static int validate_avtab_key(avtab_key_t *key, int conditional, validate_t flavors[])
> {
> if (validate_value(key->source_type, &flavors[SYM_TYPES]))
> goto bad;
> @@ -670,13 +670,16 @@ static int validate_avtab_key(avtab_key_t *key, validate_t flavors[])
> case AVTAB_ALLOWED:
> case AVTAB_AUDITALLOW:
> case AVTAB_AUDITDENY:
> - case AVTAB_XPERMS_ALLOWED:
> - case AVTAB_XPERMS_AUDITALLOW:
> - case AVTAB_XPERMS_DONTAUDIT:
> case AVTAB_TRANSITION:
> case AVTAB_MEMBER:
> case AVTAB_CHANGE:
> break;
> + case AVTAB_XPERMS_ALLOWED:
> + case AVTAB_XPERMS_AUDITALLOW:
> + case AVTAB_XPERMS_DONTAUDIT:
> + if (conditional)
> + goto bad;
> + break;
> default:
> goto bad;
> }
> @@ -691,7 +694,7 @@ static int validate_avtab_key_and_datum(avtab_key_t *k, avtab_datum_t *d, void *
> {
> validate_t *flavors = (validate_t *)args;
>
> - if (validate_avtab_key(k, flavors))
> + if (validate_avtab_key(k, 0, flavors))
> return -1;
>
> if ((k->specified & AVTAB_TYPE) && validate_value(d->data, &flavors[SYM_TYPES]))
> @@ -716,7 +719,7 @@ static int validate_cond_av_list(sepol_handle_t *handle, cond_av_list_t *cond_av
>
> for (; cond_av; cond_av = cond_av->next) {
> for (avtab_ptr = cond_av->node; avtab_ptr; avtab_ptr = avtab_ptr->next) {
> - if (validate_avtab_key(&avtab_ptr->key, flavors)) {
> + if (validate_avtab_key(&avtab_ptr->key, 1, flavors)) {
> ERR(handle, "Invalid cond av list");
> return -1;
> }
> @@ -726,7 +729,7 @@ static int validate_cond_av_list(sepol_handle_t *handle, cond_av_list_t *cond_av
> return 0;
> }
>
> -static int validate_avrules(sepol_handle_t *handle, avrule_t *avrule, validate_t flavors[])
> +static int validate_avrules(sepol_handle_t *handle, avrule_t *avrule, int conditional, validate_t flavors[])
> {
> class_perm_node_t *class;
>
> @@ -746,14 +749,17 @@ static int validate_avrules(sepol_handle_t *handle, avrule_t *avrule, validate_t
> case AVRULE_AUDITALLOW:
> case AVRULE_AUDITDENY:
> case AVRULE_DONTAUDIT:
> - case AVRULE_NEVERALLOW:
> case AVRULE_TRANSITION:
> case AVRULE_MEMBER:
> case AVRULE_CHANGE:
> + break;
> + case AVRULE_NEVERALLOW:
> case AVRULE_XPERMS_ALLOWED:
> case AVRULE_XPERMS_AUDITALLOW:
> case AVRULE_XPERMS_DONTAUDIT:
> case AVRULE_XPERMS_NEVERALLOW:
> + if (conditional)
> + goto bad;
> break;
> default:
> goto bad;
> @@ -814,9 +820,9 @@ static int validate_cond_list(sepol_handle_t *handle, cond_list_t *cond, validat
> goto bad;
> if (validate_cond_av_list(handle, cond->false_list, flavors))
> goto bad;
> - if (validate_avrules(handle, cond->avtrue_list, flavors))
> + if (validate_avrules(handle, cond->avtrue_list, 1, flavors))
> goto bad;
> - if (validate_avrules(handle, cond->avfalse_list, flavors))
> + if (validate_avrules(handle, cond->avfalse_list, 1, flavors))
> goto bad;
> if (validate_bool_id_array(handle, cond->bool_ids, cond->nbools, &flavors[SYM_BOOLS]))
> goto bad;
> @@ -1098,7 +1104,7 @@ static int validate_avrule_blocks(sepol_handle_t *handle, avrule_block_t *avrule
> for (decl = avrule_block->branch_list; decl != NULL; decl = decl->next) {
> if (validate_cond_list(handle, decl->cond_list, flavors))
> goto bad;
> - if (validate_avrules(handle, decl->avrules, flavors))
> + if (validate_avrules(handle, decl->avrules, 0, flavors))
> goto bad;
> if (validate_role_trans_rules(handle, decl->role_tr_rules, flavors))
> goto bad;
> --
> 2.35.1
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH 1/2] libsepol: reject xperm av rules in conditional statements
2022-03-11 14:57 ` [PATCH 1/2] libsepol: reject xperm av rules in conditional statements James Carter
@ 2022-03-11 16:04 ` James Carter
0 siblings, 0 replies; 4+ messages in thread
From: James Carter @ 2022-03-11 16:04 UTC (permalink / raw)
To: Christian Göttsche; +Cc: SElinux list
On Fri, Mar 11, 2022 at 9:57 AM James Carter <jwcart2@gmail.com> wrote:
>
> On Tue, Mar 8, 2022 at 2:35 PM Christian Göttsche
> <cgzones@googlemail.com> wrote:
> >
> > Extended permission and neverallow rules are not permitted in
> > conditional statements.
> >
> > This causes issues on policy optimization where avtab_search() might
> > return a non extended permission rule when searching for one.
> >
> > Found by oss-fuzz (#45327)
> >
> > Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
>
> For both patches:
> Acked-by: James Carter <jwcart2@gmail.com>
>
These patches have been merged.
Thanks,
Jim
> > ---
> > libsepol/src/policydb_validate.c | 28 +++++++++++++++++-----------
> > 1 file changed, 17 insertions(+), 11 deletions(-)
> >
> > diff --git a/libsepol/src/policydb_validate.c b/libsepol/src/policydb_validate.c
> > index 735c7a33..72063351 100644
> > --- a/libsepol/src/policydb_validate.c
> > +++ b/libsepol/src/policydb_validate.c
> > @@ -658,7 +658,7 @@ bad:
> > * Functions to validate a kernel policydb
> > */
> >
> > -static int validate_avtab_key(avtab_key_t *key, validate_t flavors[])
> > +static int validate_avtab_key(avtab_key_t *key, int conditional, validate_t flavors[])
> > {
> > if (validate_value(key->source_type, &flavors[SYM_TYPES]))
> > goto bad;
> > @@ -670,13 +670,16 @@ static int validate_avtab_key(avtab_key_t *key, validate_t flavors[])
> > case AVTAB_ALLOWED:
> > case AVTAB_AUDITALLOW:
> > case AVTAB_AUDITDENY:
> > - case AVTAB_XPERMS_ALLOWED:
> > - case AVTAB_XPERMS_AUDITALLOW:
> > - case AVTAB_XPERMS_DONTAUDIT:
> > case AVTAB_TRANSITION:
> > case AVTAB_MEMBER:
> > case AVTAB_CHANGE:
> > break;
> > + case AVTAB_XPERMS_ALLOWED:
> > + case AVTAB_XPERMS_AUDITALLOW:
> > + case AVTAB_XPERMS_DONTAUDIT:
> > + if (conditional)
> > + goto bad;
> > + break;
> > default:
> > goto bad;
> > }
> > @@ -691,7 +694,7 @@ static int validate_avtab_key_and_datum(avtab_key_t *k, avtab_datum_t *d, void *
> > {
> > validate_t *flavors = (validate_t *)args;
> >
> > - if (validate_avtab_key(k, flavors))
> > + if (validate_avtab_key(k, 0, flavors))
> > return -1;
> >
> > if ((k->specified & AVTAB_TYPE) && validate_value(d->data, &flavors[SYM_TYPES]))
> > @@ -716,7 +719,7 @@ static int validate_cond_av_list(sepol_handle_t *handle, cond_av_list_t *cond_av
> >
> > for (; cond_av; cond_av = cond_av->next) {
> > for (avtab_ptr = cond_av->node; avtab_ptr; avtab_ptr = avtab_ptr->next) {
> > - if (validate_avtab_key(&avtab_ptr->key, flavors)) {
> > + if (validate_avtab_key(&avtab_ptr->key, 1, flavors)) {
> > ERR(handle, "Invalid cond av list");
> > return -1;
> > }
> > @@ -726,7 +729,7 @@ static int validate_cond_av_list(sepol_handle_t *handle, cond_av_list_t *cond_av
> > return 0;
> > }
> >
> > -static int validate_avrules(sepol_handle_t *handle, avrule_t *avrule, validate_t flavors[])
> > +static int validate_avrules(sepol_handle_t *handle, avrule_t *avrule, int conditional, validate_t flavors[])
> > {
> > class_perm_node_t *class;
> >
> > @@ -746,14 +749,17 @@ static int validate_avrules(sepol_handle_t *handle, avrule_t *avrule, validate_t
> > case AVRULE_AUDITALLOW:
> > case AVRULE_AUDITDENY:
> > case AVRULE_DONTAUDIT:
> > - case AVRULE_NEVERALLOW:
> > case AVRULE_TRANSITION:
> > case AVRULE_MEMBER:
> > case AVRULE_CHANGE:
> > + break;
> > + case AVRULE_NEVERALLOW:
> > case AVRULE_XPERMS_ALLOWED:
> > case AVRULE_XPERMS_AUDITALLOW:
> > case AVRULE_XPERMS_DONTAUDIT:
> > case AVRULE_XPERMS_NEVERALLOW:
> > + if (conditional)
> > + goto bad;
> > break;
> > default:
> > goto bad;
> > @@ -814,9 +820,9 @@ static int validate_cond_list(sepol_handle_t *handle, cond_list_t *cond, validat
> > goto bad;
> > if (validate_cond_av_list(handle, cond->false_list, flavors))
> > goto bad;
> > - if (validate_avrules(handle, cond->avtrue_list, flavors))
> > + if (validate_avrules(handle, cond->avtrue_list, 1, flavors))
> > goto bad;
> > - if (validate_avrules(handle, cond->avfalse_list, flavors))
> > + if (validate_avrules(handle, cond->avfalse_list, 1, flavors))
> > goto bad;
> > if (validate_bool_id_array(handle, cond->bool_ids, cond->nbools, &flavors[SYM_BOOLS]))
> > goto bad;
> > @@ -1098,7 +1104,7 @@ static int validate_avrule_blocks(sepol_handle_t *handle, avrule_block_t *avrule
> > for (decl = avrule_block->branch_list; decl != NULL; decl = decl->next) {
> > if (validate_cond_list(handle, decl->cond_list, flavors))
> > goto bad;
> > - if (validate_avrules(handle, decl->avrules, flavors))
> > + if (validate_avrules(handle, decl->avrules, 0, flavors))
> > goto bad;
> > if (validate_role_trans_rules(handle, decl->role_tr_rules, flavors))
> > goto bad;
> > --
> > 2.35.1
> >
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2022-03-11 16:04 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-03-08 18:58 [PATCH 1/2] libsepol: reject xperm av rules in conditional statements Christian Göttsche
2022-03-08 18:58 ` [PATCH 2/2] libsepol: validate boolean datum arrays Christian Göttsche
2022-03-11 14:57 ` [PATCH 1/2] libsepol: reject xperm av rules in conditional statements James Carter
2022-03-11 16:04 ` James Carter
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.