| Message ID | 20220607150145.29757-1-cgzones@googlemail.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 88a703399f3f |
| Headers | show |
| Series | libsepol: fix validation of user declarations in modules | expand |
On Tue, Jun 7, 2022 at 11:54 AM Christian Göttsche <cgzones@googlemail.com> wrote: > > Users are allowed to be declared in modules. Modules do not get expanded > leaving the `struct user_datum` members `exp_range` and `exp_dfltlevel` > empty. > Do no validate the expanded range and level for modular polices. > > Reported-by: bauen1 <j2468h@gmail.com> > Signed-off-by: Christian Göttsche <cgzones@googlemail.com> Acked-by: James Carter <jwcart2@gmail.com> > --- > libsepol/src/policydb_validate.c | 12 ++++++------ > 1 file changed, 6 insertions(+), 6 deletions(-) > > diff --git a/libsepol/src/policydb_validate.c b/libsepol/src/policydb_validate.c > index da18282b..99d4eb7f 100644 > --- a/libsepol/src/policydb_validate.c > +++ b/libsepol/src/policydb_validate.c > @@ -18,7 +18,7 @@ typedef struct validate { > typedef struct map_arg { > validate_t *flavors; > sepol_handle_t *handle; > - int mls; > + policydb_t *policy; > } map_arg_t; > > static int create_gap_ebitmap(char **val_to_name, uint32_t nprim, ebitmap_t *gaps) > @@ -571,7 +571,7 @@ static int validate_mls_range(mls_range_t *range, validate_t *sens, validate_t * > return -1; > } > > -static int validate_user_datum(sepol_handle_t *handle, user_datum_t *user, validate_t flavors[], int mls) > +static int validate_user_datum(sepol_handle_t *handle, user_datum_t *user, validate_t flavors[], policydb_t *p) > { > if (validate_value(user->s.value, &flavors[SYM_USERS])) > goto bad; > @@ -581,9 +581,9 @@ static int validate_user_datum(sepol_handle_t *handle, user_datum_t *user, valid > goto bad; > if (validate_mls_semantic_level(&user->dfltlevel, &flavors[SYM_LEVELS], &flavors[SYM_CATS])) > goto bad; > - if (mls && validate_mls_range(&user->exp_range, &flavors[SYM_LEVELS], &flavors[SYM_CATS])) > + if (p->mls && p->policy_type != POLICY_MOD && validate_mls_range(&user->exp_range, &flavors[SYM_LEVELS], &flavors[SYM_CATS])) > goto bad; > - if (mls && validate_mls_level(&user->exp_dfltlevel, &flavors[SYM_LEVELS], &flavors[SYM_CATS])) > + if (p->mls && p->policy_type != POLICY_MOD && validate_mls_level(&user->exp_dfltlevel, &flavors[SYM_LEVELS], &flavors[SYM_CATS])) > goto bad; > if (user->bounds && validate_value(user->bounds, &flavors[SYM_USERS])) > goto bad; > @@ -599,7 +599,7 @@ static int validate_user_datum_wrapper(__attribute__((unused)) hashtab_key_t k, > { > map_arg_t *margs = args; > > - return validate_user_datum(margs->handle, d, margs->flavors, margs->mls); > + return validate_user_datum(margs->handle, d, margs->flavors, margs->policy); > } > > static int validate_bool_datum(sepol_handle_t *handle, cond_bool_datum_t *boolean, validate_t flavors[]) > @@ -689,7 +689,7 @@ static int validate_datum(__attribute__ ((unused))hashtab_key_t k, hashtab_datum > > static int validate_datum_array_entries(sepol_handle_t *handle, policydb_t *p, validate_t flavors[]) > { > - map_arg_t margs = { flavors, handle, p->mls }; > + map_arg_t margs = { flavors, handle, p }; > > if (hashtab_map(p->p_commons.table, validate_common_datum_wrapper, &margs)) > goto bad; > -- > 2.36.1 >
On Tue, Jun 7, 2022 at 5:02 PM Christian Göttsche <cgzones@googlemail.com> wrote: > > Users are allowed to be declared in modules. Modules do not get expanded > leaving the `struct user_datum` members `exp_range` and `exp_dfltlevel` > empty. > Do no validate the expanded range and level for modular polices. > > Reported-by: bauen1 <j2468h@gmail.com> > Signed-off-by: Christian Göttsche <cgzones@googlemail.com> > --- > libsepol/src/policydb_validate.c | 12 ++++++------ > 1 file changed, 6 insertions(+), 6 deletions(-) > > diff --git a/libsepol/src/policydb_validate.c b/libsepol/src/policydb_validate.c > index da18282b..99d4eb7f 100644 > --- a/libsepol/src/policydb_validate.c > +++ b/libsepol/src/policydb_validate.c > @@ -18,7 +18,7 @@ typedef struct validate { > typedef struct map_arg { > validate_t *flavors; > sepol_handle_t *handle; > - int mls; > + policydb_t *policy; > } map_arg_t; Hello, As the policy is not modified, could this pointer be "const policydb_t *policy;"? (And the last parameter of validate_user_datum be "const policydb_t *p"). On the other hand, as policydb_validate.c does not use any const pointer, feel free to disregard my comment. This patch nevertheless looks good to me too. Thanks! Nicolas
On Wed, 8 Jun 2022 at 21:16, Nicolas Iooss <nicolas.iooss@m4x.org> wrote: > > On Tue, Jun 7, 2022 at 5:02 PM Christian Göttsche > <cgzones@googlemail.com> wrote: > > > > Users are allowed to be declared in modules. Modules do not get expanded > > leaving the `struct user_datum` members `exp_range` and `exp_dfltlevel` > > empty. > > Do no validate the expanded range and level for modular polices. > > > > Reported-by: bauen1 <j2468h@gmail.com> > > Signed-off-by: Christian Göttsche <cgzones@googlemail.com> > > --- > > libsepol/src/policydb_validate.c | 12 ++++++------ > > 1 file changed, 6 insertions(+), 6 deletions(-) > > > > diff --git a/libsepol/src/policydb_validate.c b/libsepol/src/policydb_validate.c > > index da18282b..99d4eb7f 100644 > > --- a/libsepol/src/policydb_validate.c > > +++ b/libsepol/src/policydb_validate.c > > @@ -18,7 +18,7 @@ typedef struct validate { > > typedef struct map_arg { > > validate_t *flavors; > > sepol_handle_t *handle; > > - int mls; > > + policydb_t *policy; > > } map_arg_t; > > Hello, > As the policy is not modified, could this pointer be "const policydb_t > *policy;"? (And the last parameter of validate_user_datum be "const > policydb_t *p"). On the other hand, as policydb_validate.c does not > use any const pointer, feel free to disregard my comment. > Since policydb_validate.c does not use const pointer at all yet I followed that style. I might prepare a patch to constify all pointers and refactor the logging mechanism, since I like to add validation into the kernel to avoid crashes like [1], especially if SELinux gets namespace support [2]. [1]: https://github.com/SELinuxProject/selinux-testsuite/issues/76 [2]: https://patchwork.kernel.org/project/selinux/list/?series=632975 > This patch nevertheless looks good to me too. > Thanks! > Nicolas >
On Wed, Jun 8, 2022 at 2:41 PM James Carter <jwcart2@gmail.com> wrote: > > On Tue, Jun 7, 2022 at 11:54 AM Christian Göttsche > <cgzones@googlemail.com> wrote: > > > > Users are allowed to be declared in modules. Modules do not get expanded > > leaving the `struct user_datum` members `exp_range` and `exp_dfltlevel` > > empty. > > Do no validate the expanded range and level for modular polices. > > > > Reported-by: bauen1 <j2468h@gmail.com> > > Signed-off-by: Christian Göttsche <cgzones@googlemail.com> > > Acked-by: James Carter <jwcart2@gmail.com> > Merged. Thanks, Jim > > --- > > libsepol/src/policydb_validate.c | 12 ++++++------ > > 1 file changed, 6 insertions(+), 6 deletions(-) > > > > diff --git a/libsepol/src/policydb_validate.c b/libsepol/src/policydb_validate.c > > index da18282b..99d4eb7f 100644 > > --- a/libsepol/src/policydb_validate.c > > +++ b/libsepol/src/policydb_validate.c > > @@ -18,7 +18,7 @@ typedef struct validate { > > typedef struct map_arg { > > validate_t *flavors; > > sepol_handle_t *handle; > > - int mls; > > + policydb_t *policy; > > } map_arg_t; > > > > static int create_gap_ebitmap(char **val_to_name, uint32_t nprim, ebitmap_t *gaps) > > @@ -571,7 +571,7 @@ static int validate_mls_range(mls_range_t *range, validate_t *sens, validate_t * > > return -1; > > } > > > > -static int validate_user_datum(sepol_handle_t *handle, user_datum_t *user, validate_t flavors[], int mls) > > +static int validate_user_datum(sepol_handle_t *handle, user_datum_t *user, validate_t flavors[], policydb_t *p) > > { > > if (validate_value(user->s.value, &flavors[SYM_USERS])) > > goto bad; > > @@ -581,9 +581,9 @@ static int validate_user_datum(sepol_handle_t *handle, user_datum_t *user, valid > > goto bad; > > if (validate_mls_semantic_level(&user->dfltlevel, &flavors[SYM_LEVELS], &flavors[SYM_CATS])) > > goto bad; > > - if (mls && validate_mls_range(&user->exp_range, &flavors[SYM_LEVELS], &flavors[SYM_CATS])) > > + if (p->mls && p->policy_type != POLICY_MOD && validate_mls_range(&user->exp_range, &flavors[SYM_LEVELS], &flavors[SYM_CATS])) > > goto bad; > > - if (mls && validate_mls_level(&user->exp_dfltlevel, &flavors[SYM_LEVELS], &flavors[SYM_CATS])) > > + if (p->mls && p->policy_type != POLICY_MOD && validate_mls_level(&user->exp_dfltlevel, &flavors[SYM_LEVELS], &flavors[SYM_CATS])) > > goto bad; > > if (user->bounds && validate_value(user->bounds, &flavors[SYM_USERS])) > > goto bad; > > @@ -599,7 +599,7 @@ static int validate_user_datum_wrapper(__attribute__((unused)) hashtab_key_t k, > > { > > map_arg_t *margs = args; > > > > - return validate_user_datum(margs->handle, d, margs->flavors, margs->mls); > > + return validate_user_datum(margs->handle, d, margs->flavors, margs->policy); > > } > > > > static int validate_bool_datum(sepol_handle_t *handle, cond_bool_datum_t *boolean, validate_t flavors[]) > > @@ -689,7 +689,7 @@ static int validate_datum(__attribute__ ((unused))hashtab_key_t k, hashtab_datum > > > > static int validate_datum_array_entries(sepol_handle_t *handle, policydb_t *p, validate_t flavors[]) > > { > > - map_arg_t margs = { flavors, handle, p->mls }; > > + map_arg_t margs = { flavors, handle, p }; > > > > if (hashtab_map(p->p_commons.table, validate_common_datum_wrapper, &margs)) > > goto bad; > > -- > > 2.36.1 > >
diff --git a/libsepol/src/policydb_validate.c b/libsepol/src/policydb_validate.c index da18282b..99d4eb7f 100644 --- a/libsepol/src/policydb_validate.c +++ b/libsepol/src/policydb_validate.c @@ -18,7 +18,7 @@ typedef struct validate { typedef struct map_arg { validate_t *flavors; sepol_handle_t *handle; - int mls; + policydb_t *policy; } map_arg_t; static int create_gap_ebitmap(char **val_to_name, uint32_t nprim, ebitmap_t *gaps) @@ -571,7 +571,7 @@ static int validate_mls_range(mls_range_t *range, validate_t *sens, validate_t * return -1; } -static int validate_user_datum(sepol_handle_t *handle, user_datum_t *user, validate_t flavors[], int mls) +static int validate_user_datum(sepol_handle_t *handle, user_datum_t *user, validate_t flavors[], policydb_t *p) { if (validate_value(user->s.value, &flavors[SYM_USERS])) goto bad; @@ -581,9 +581,9 @@ static int validate_user_datum(sepol_handle_t *handle, user_datum_t *user, valid goto bad; if (validate_mls_semantic_level(&user->dfltlevel, &flavors[SYM_LEVELS], &flavors[SYM_CATS])) goto bad; - if (mls && validate_mls_range(&user->exp_range, &flavors[SYM_LEVELS], &flavors[SYM_CATS])) + if (p->mls && p->policy_type != POLICY_MOD && validate_mls_range(&user->exp_range, &flavors[SYM_LEVELS], &flavors[SYM_CATS])) goto bad; - if (mls && validate_mls_level(&user->exp_dfltlevel, &flavors[SYM_LEVELS], &flavors[SYM_CATS])) + if (p->mls && p->policy_type != POLICY_MOD && validate_mls_level(&user->exp_dfltlevel, &flavors[SYM_LEVELS], &flavors[SYM_CATS])) goto bad; if (user->bounds && validate_value(user->bounds, &flavors[SYM_USERS])) goto bad; @@ -599,7 +599,7 @@ static int validate_user_datum_wrapper(__attribute__((unused)) hashtab_key_t k, { map_arg_t *margs = args; - return validate_user_datum(margs->handle, d, margs->flavors, margs->mls); + return validate_user_datum(margs->handle, d, margs->flavors, margs->policy); } static int validate_bool_datum(sepol_handle_t *handle, cond_bool_datum_t *boolean, validate_t flavors[]) @@ -689,7 +689,7 @@ static int validate_datum(__attribute__ ((unused))hashtab_key_t k, hashtab_datum static int validate_datum_array_entries(sepol_handle_t *handle, policydb_t *p, validate_t flavors[]) { - map_arg_t margs = { flavors, handle, p->mls }; + map_arg_t margs = { flavors, handle, p }; if (hashtab_map(p->p_commons.table, validate_common_datum_wrapper, &margs)) goto bad;
Users are allowed to be declared in modules. Modules do not get expanded leaving the `struct user_datum` members `exp_range` and `exp_dfltlevel` empty. Do no validate the expanded range and level for modular polices. Reported-by: bauen1 <j2468h@gmail.com> Signed-off-by: Christian Göttsche <cgzones@googlemail.com> --- libsepol/src/policydb_validate.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-)