On Tue, Feb 22, 2022 at 9:51 PM Christian Göttsche
<cgzones@xxxxxxxxxxxxxx> wrote:
>
> Evaluate expressions similar to the actual kernel security server such
> that invalid expressions, e.g. `t2 == t3` for validatetrans, are
> rejected.
>
> Signed-off-by: Christian Göttsche <cgzones@xxxxxxxxxxxxxx>
> ---
> v2:
> reject third task context in normal constraints
> ---
> libsepol/src/policydb_validate.c | 226 ++++++++++++++++++++++---------
> 1 file changed, 159 insertions(+), 67 deletions(-)
>
> diff --git a/libsepol/src/policydb_validate.c b/libsepol/src/policydb_validate.c
> index 735c7a33..3b0ea0e1 100644
> --- a/libsepol/src/policydb_validate.c
> +++ b/libsepol/src/policydb_validate.c
> @@ -223,90 +223,182 @@ bad:
> return -1;
> }
>
> -static int validate_constraint_nodes(sepol_handle_t *handle, unsigned int nperms, constraint_node_t *cons, validate_t flavors[])
> +/*
> + * Follow evaluation of expressions to find invalid ones.
> + * Keep in sync with kernel source security/selinux/ss/services.c::constraint_expr_eval()
> + */
> +static int validate_expression(sepol_handle_t *handle, constraint_expr_t *e, int validatetrans, validate_t flavors[])
> {
> - constraint_expr_t *cexp;
> -
> - for (; cons; cons = cons->next) {
> - if (nperms > 0 && cons->permissions == 0)
> - goto bad;
> - if (nperms > 0 && nperms != PERM_SYMTAB_SIZE && cons->permissions >= (UINT32_C(1) << nperms))
> - goto bad;
> + int sp = -1;
The function read_cons_helper() is used when reading in the policy and
it keeps track of the stack and will return an error if there is a
problem, so I don't think that this is going to be useful when
validating the policy.
Most of what you have here is concerned with keeping track of the
stack. There is more validation that can be done, however. See if the
patch that I sent to the list will meet your needs.
Thanks,
Jim
>
> - for (cexp = cons->expr; cexp; cexp = cexp->next) {
> - if (cexp->attr & CEXPR_USER) {
> - if (validate_ebitmap(&cexp->names, &flavors[SYM_USERS]))
> - goto bad;
> - if (validate_empty_type_set(cexp->type_names))
> - goto bad;
> - } else if (cexp->attr & CEXPR_ROLE) {
> - if (validate_ebitmap(&cexp->names, &flavors[SYM_ROLES]))
> - goto bad;
> - if (validate_empty_type_set(cexp->type_names))
> - goto bad;
> - } else if (cexp->attr & CEXPR_TYPE) {
> - if (validate_ebitmap(&cexp->names, &flavors[SYM_TYPES]))
> - goto bad;
> - if (validate_type_set(cexp->type_names, &flavors[SYM_TYPES]))
> - goto bad;
> - } else {
> - if (!ebitmap_is_empty(&cexp->names))
> - goto bad;
> - if (validate_empty_type_set(cexp->type_names))
> - goto bad;
> - }
> + for (; e; e = e->next) {
> + /* validate symbols (implied in kernel source) */
> + if (e->attr & CEXPR_USER) {
> + if (validate_ebitmap(&e->names, &flavors[SYM_USERS]))
> + goto bad;
> + if (validate_empty_type_set(e->type_names))
> + goto bad;
> + } else if (e->attr & CEXPR_ROLE) {
> + if (validate_ebitmap(&e->names, &flavors[SYM_ROLES]))
> + goto bad;
> + if (validate_empty_type_set(e->type_names))
> + goto bad;
> + } else if (e->attr & CEXPR_TYPE) {
> + if (validate_ebitmap(&e->names, &flavors[SYM_TYPES]))
> + goto bad;
> + if (validate_type_set(e->type_names, &flavors[SYM_TYPES]))
> + goto bad;
> + } else {
> + if (!ebitmap_is_empty(&e->names))
> + goto bad;
> + if (validate_empty_type_set(e->type_names))
> + goto bad;
> + }
>
> - if (cexp->expr_type == CEXPR_ATTR || cexp->expr_type == CEXPR_NAMES) {
> - switch (cexp->op) {
> - case CEXPR_EQ:
> - case CEXPR_NEQ:
> + switch (e->expr_type) {
> + case CEXPR_NOT:
> + if(sp < 0)
> + goto bad;
> + break;
> + case CEXPR_AND:
> + if(sp < 0)
> + goto bad;
> + sp--;
> + break;
> + case CEXPR_OR:
> + if(sp < 0)
> + goto bad;
> + sp--;
> + break;
> + case CEXPR_ATTR:
> + if (sp == (CEXPR_MAXDEPTH - 1))
> + return 0;
> + switch (e->attr) {
> + case CEXPR_USER:
> + break;
> + case CEXPR_TYPE:
> + break;
> + case CEXPR_ROLE:
> + switch (e->op) {
> case CEXPR_DOM:
> + ++sp;
> + continue;
> case CEXPR_DOMBY:
> + ++sp;
> + continue;
> case CEXPR_INCOMP:
> - break;
> + ++sp;
> + continue;
> default:
> - goto bad;
> - }
> -
> - switch (cexp->attr) {
> - case CEXPR_USER:
> - case CEXPR_USER | CEXPR_TARGET:
> - case CEXPR_USER | CEXPR_XTARGET:
> - case CEXPR_ROLE:
> - case CEXPR_ROLE | CEXPR_TARGET:
> - case CEXPR_ROLE | CEXPR_XTARGET:
> - case CEXPR_TYPE:
> - case CEXPR_TYPE | CEXPR_TARGET:
> - case CEXPR_TYPE | CEXPR_XTARGET:
> - case CEXPR_L1L2:
> - case CEXPR_L1H2:
> - case CEXPR_H1L2:
> - case CEXPR_H1H2:
> - case CEXPR_L1H1:
> - case CEXPR_L2H2:
> break;
> - default:
> - goto bad;
> }
> - } else {
> - switch (cexp->expr_type) {
> - case CEXPR_NOT:
> - case CEXPR_AND:
> - case CEXPR_OR:
> - break;
> + break;
> + case CEXPR_L1L2:
> + goto mls_ops;
> + case CEXPR_L1H2:
> + goto mls_ops;
> + case CEXPR_H1L2:
> + goto mls_ops;
> + case CEXPR_H1H2:
> + goto mls_ops;
> + case CEXPR_L1H1:
> + goto mls_ops;
> + case CEXPR_L2H2:
> + goto mls_ops;
> +mls_ops:
> + switch (e->op) {
> + case CEXPR_EQ:
> + ++sp;
> + continue;
> + case CEXPR_NEQ:
> + ++sp;
> + continue;
> + case CEXPR_DOM:
> + ++sp;
> + continue;
> + case CEXPR_DOMBY:
> + ++sp;
> + continue;
> + case CEXPR_INCOMP:
> + ++sp;
> + continue;
> default:
> goto bad;
> }
> + break;
> + default:
> + goto bad;
> + }
>
> - if (cexp->op != 0)
> + switch (e->op) {
> + case CEXPR_EQ:
> + ++sp;
> + break;
> + case CEXPR_NEQ:
> + ++sp;
> + break;
> + default:
> + goto bad;
> + }
> + break;
> + case CEXPR_NAMES:
> + if (sp == (CEXPR_MAXDEPTH-1))
> + return 0;
> + if (e->attr & CEXPR_TARGET)
> + ;
> + else if (e->attr & CEXPR_XTARGET) {
> + if (!validatetrans)
> goto bad;
> + }
> + if (e->attr & CEXPR_USER)
> + ;
> + else if (e->attr & CEXPR_ROLE)
> + ;
> + else if (e->attr & CEXPR_TYPE)
> + ;
> + else
> + goto bad;
>
> - if (cexp->attr != 0)
> - goto bad;
> + switch (e->op) {
> + case CEXPR_EQ:
> + ++sp;
> + break;
> + case CEXPR_NEQ:
> + ++sp;
> + break;
> + default:
> + goto bad;
> }
> + break;
> + default:
> + goto bad;
> }
> }
>
> + if (sp != 0)
> + goto bad;
> +
> + return 0;
> +
> +bad:
> + ERR(handle, "Invalid expression");
> + return -1;
> +}
> +
> +static int validate_constraint_nodes(sepol_handle_t *handle, unsigned int nperms, constraint_node_t *cons, int validatetrans, validate_t flavors[])
> +{
> + for (; cons; cons = cons->next) {
> + if (validatetrans && cons->permissions != 0)
> + goto bad;
> + if (!validatetrans && cons->permissions == 0)
> + goto bad;
> + if (!validatetrans && nperms != PERM_SYMTAB_SIZE && cons->permissions >= (UINT32_C(1) << nperms))
> + goto bad;
> +
> + if (validate_expression(handle, cons->expr, validatetrans, flavors))
> + goto bad;
> + }
> +
> return 0;
>
> bad:
> @@ -320,9 +412,9 @@ static int validate_class_datum(sepol_handle_t *handle, class_datum_t *class, va
> goto bad;
> if (class->permissions.nprim > PERM_SYMTAB_SIZE)
> goto bad;
> - if (validate_constraint_nodes(handle, class->permissions.nprim, class->constraints, flavors))
> + if (validate_constraint_nodes(handle, class->permissions.nprim, class->constraints, 0, flavors))
> goto bad;
> - if (validate_constraint_nodes(handle, 0, class->validatetrans, flavors))
> + if (validate_constraint_nodes(handle, class->permissions.nprim, class->validatetrans, 1, flavors))
> goto bad;
>
> switch (class->default_user) {
> --
> 2.35.1
>
