On Fri, Jul 7, 2023 at 2:12 PM James Carter <jwcart2@xxxxxxxxx> wrote:
>
> On Tue, Jun 20, 2023 at 5:11 AM Juraj Marcin <juraj@xxxxxxxxxxxxxxx> wrote:
> >
> > To move filename transitions to be part of avtab, we need to create
> > space for it in the avtab_datum structure which holds the rule for
> > a certain combination of stype, ttype and tclass.
> >
> > As only type transitions have a special variant that uses a filename, it
> > would be suboptimal to add a (mostly empty) pointer to some structure to
> > all avtab rules.
> >
> > Therefore, this patch adds a new structure to the avtab_datum and moves
> > the otype of the transition to this structure. In the next patch, this
> > structure will also hold filename transitions for the combination of
> > stype, ttype and tclass.
> >
> > Reviewed-by: Ondrej Mosnacek <omosnace@xxxxxxxxxx>
> > Signed-off-by: Juraj Marcin <juraj@xxxxxxxxxxxxxxx>
>
> I have this fear that I missed something, but I have tested
> compatibility as much as I can and everything seems to work fine. Even
> building older policy versions doesn't break anything that I can find.
>
> Patch 3 no longer applies cleanly with some of the recent dismod, but
> it is minor and I can handle it when I merge this series.
>
> If at some point we allow named type transitions in conditional
> policy, then CIL can handle all type transitions like named type
> transitions. But that is for another day.
>
> For this series of eight patches:
> Acked-by: James Carter <jwcart2@xxxxxxxxx>
>
These eight patches have been merged.
Thanks,
Jim
>
> > ---
> > checkpolicy/test/dispol.c | 2 +-
> > libsepol/cil/src/cil_binary.c | 26 +++++++++++++++-----
> > libsepol/include/sepol/policydb/avtab.h | 7 +++++-
> > libsepol/src/avtab.c | 32 ++++++++++++++++++++++++-
> > libsepol/src/expand.c | 8 +++++--
> > libsepol/src/kernel_to_cil.c | 3 ++-
> > libsepol/src/kernel_to_conf.c | 3 ++-
> > libsepol/src/optimize.c | 4 ++++
> > libsepol/src/policydb_validate.c | 4 +++-
> > libsepol/src/services.c | 5 +++-
> > libsepol/src/write.c | 17 ++++++++++---
> > 11 files changed, 93 insertions(+), 18 deletions(-)
> >
> > diff --git a/checkpolicy/test/dispol.c b/checkpolicy/test/dispol.c
> > index bee1a660..de1a5d11 100644
> > --- a/checkpolicy/test/dispol.c
> > +++ b/checkpolicy/test/dispol.c
> > @@ -180,7 +180,7 @@ static int render_av_rule(avtab_key_t * key, avtab_datum_t * datum, uint32_t wha
> > if (key->specified & AVTAB_TRANSITION) {
> > fprintf(fp, "type_transition ");
> > render_key(key, p, fp);
> > - render_type(datum->data, p, fp);
> > + render_type(datum->trans->otype, p, fp);
> > fprintf(fp, ";\n");
> > }
> > if (key->specified & AVTAB_MEMBER) {
> > diff --git a/libsepol/cil/src/cil_binary.c b/libsepol/cil/src/cil_binary.c
> > index c4ee2380..3f264594 100644
> > --- a/libsepol/cil/src/cil_binary.c
> > +++ b/libsepol/cil/src/cil_binary.c
> > @@ -975,28 +975,34 @@ static int __cil_insert_type_rule(policydb_t *pdb, uint32_t kind, uint32_t src,
> > int rc = SEPOL_OK;
> > avtab_key_t avtab_key;
> > avtab_datum_t avtab_datum;
> > + avtab_trans_t trans;
> > avtab_ptr_t existing;
> >
> > avtab_key.source_type = src;
> > avtab_key.target_type = tgt;
> > avtab_key.target_class = obj;
> >
> > + memset(&avtab_datum, 0, sizeof(avtab_datum_t));
> > + memset(&trans, 0, sizeof(avtab_trans_t));
> > +
> > switch (kind) {
> > case CIL_TYPE_TRANSITION:
> > avtab_key.specified = AVTAB_TRANSITION;
> > + trans.otype = res;
> > + avtab_datum.trans = &trans;
> > break;
> > case CIL_TYPE_CHANGE:
> > avtab_key.specified = AVTAB_CHANGE;
> > + avtab_datum.data = res;
> > break;
> > case CIL_TYPE_MEMBER:
> > avtab_key.specified = AVTAB_MEMBER;
> > + avtab_datum.data = res;
> > break;
> > default:
> > rc = SEPOL_ERR;
> > goto exit;
> > }
> > -
> > - avtab_datum.data = res;
> >
> > existing = avtab_search_node(&pdb->te_avtab, &avtab_key);
> > if (existing) {
> > @@ -1004,13 +1010,17 @@ static int __cil_insert_type_rule(policydb_t *pdb, uint32_t kind, uint32_t src,
> > * A warning should have been previously given if there is a
> > * non-duplicate rule using the same key.
> > */
> > - if (existing->datum.data != res) {
> > + uint32_t existing_otype =
> > + existing->key.specified & AVTAB_TRANSITION
> > + ? existing->datum.trans->otype
> > + : existing->datum.data;
> > + if (existing_otype != res) {
> > cil_log(CIL_ERR, "Conflicting type rules (scontext=%s tcontext=%s tclass=%s result=%s), existing=%s\n",
> > pdb->p_type_val_to_name[src - 1],
> > pdb->p_type_val_to_name[tgt - 1],
> > pdb->p_class_val_to_name[obj - 1],
> > pdb->p_type_val_to_name[res - 1],
> > - pdb->p_type_val_to_name[existing->datum.data - 1]);
> > + pdb->p_type_val_to_name[existing_otype - 1]);
> > cil_log(CIL_ERR, "Expanded from type rule (scontext=%s tcontext=%s tclass=%s result=%s)\n",
> > cil_rule->src_str, cil_rule->tgt_str, cil_rule->obj_str, cil_rule->result_str);
> > rc = SEPOL_ERR;
> > @@ -1037,13 +1047,17 @@ static int __cil_insert_type_rule(policydb_t *pdb, uint32_t kind, uint32_t src,
> >
> > search_datum = cil_cond_av_list_search(&avtab_key, other_list);
> > if (search_datum == NULL) {
> > - if (existing->datum.data != res) {
> > + uint32_t existing_otype =
> > + existing->key.specified & AVTAB_TRANSITION
> > + ? existing->datum.trans->otype
> > + : existing->datum.data;
> > + if (existing_otype != res) {
> > cil_log(CIL_ERR, "Conflicting type rules (scontext=%s tcontext=%s tclass=%s result=%s), existing=%s\n",
> > pdb->p_type_val_to_name[src - 1],
> > pdb->p_type_val_to_name[tgt - 1],
> > pdb->p_class_val_to_name[obj - 1],
> > pdb->p_type_val_to_name[res - 1],
> > - pdb->p_type_val_to_name[existing->datum.data - 1]);
> > + pdb->p_type_val_to_name[existing_otype - 1]);
> > cil_log(CIL_ERR, "Expanded from type rule (scontext=%s tcontext=%s tclass=%s result=%s)\n",
> > cil_rule->src_str, cil_rule->tgt_str, cil_rule->obj_str, cil_rule->result_str);
> > rc = SEPOL_ERR;
> > diff --git a/libsepol/include/sepol/policydb/avtab.h b/libsepol/include/sepol/policydb/avtab.h
> > index e4c48576..ca009c16 100644
> > --- a/libsepol/include/sepol/policydb/avtab.h
> > +++ b/libsepol/include/sepol/policydb/avtab.h
> > @@ -70,6 +70,10 @@ typedef struct avtab_key {
> > uint16_t specified; /* what fields are specified */
> > } avtab_key_t;
> >
> > +typedef struct avtab_trans {
> > + uint32_t otype; /* resulting type of the new object */
> > +} avtab_trans_t;
> > +
> > typedef struct avtab_extended_perms {
> >
> > #define AVTAB_XPERMS_IOCTLFUNCTION 0x01
> > @@ -81,7 +85,8 @@ typedef struct avtab_extended_perms {
> > } avtab_extended_perms_t;
> >
> > typedef struct avtab_datum {
> > - uint32_t data; /* access vector or type */
> > + uint32_t data; /* access vector, member or change value */
> > + avtab_trans_t *trans; /* transition value */
> > avtab_extended_perms_t *xperms;
> > } avtab_datum_t;
> >
> > diff --git a/libsepol/src/avtab.c b/libsepol/src/avtab.c
> > index 82fec783..4c292e8b 100644
> > --- a/libsepol/src/avtab.c
> > +++ b/libsepol/src/avtab.c
> > @@ -94,6 +94,7 @@ avtab_insert_node(avtab_t * h, int hvalue, avtab_ptr_t prev, avtab_key_t * key,
> > avtab_datum_t * datum)
> > {
> > avtab_ptr_t newnode;
> > + avtab_trans_t *trans;
> > avtab_extended_perms_t *xperms;
> >
> > newnode = (avtab_ptr_t) malloc(sizeof(struct avtab_node));
> > @@ -117,6 +118,16 @@ avtab_insert_node(avtab_t * h, int hvalue, avtab_ptr_t prev, avtab_key_t * key,
> > * So copy data so it is set in the avtab
> > */
> > newnode->datum.data = datum->data;
> > + } else if (key->specified & AVTAB_TRANSITION) {
> > + trans = calloc(1, sizeof(*trans));
> > + if (trans == NULL) {
> > + free(newnode);
> > + return NULL;
> > + }
> > + if (datum->trans) /* else caller populates transition */
> > + *trans = *(datum->trans);
> > +
> > + newnode->datum.trans = trans;
> > } else {
> > newnode->datum = *datum;
> > }
> > @@ -317,6 +328,8 @@ void avtab_destroy(avtab_t * h)
> > while (cur != NULL) {
> > if (cur->key.specified & AVTAB_XPERMS) {
> > free(cur->datum.xperms);
> > + } else if (cur->key.specified & AVTAB_TRANSITION) {
> > + free(cur->datum.trans);
> > }
> > temp = cur;
> > cur = cur->next;
> > @@ -440,6 +453,7 @@ int avtab_read_item(struct policy_file *fp, uint32_t vers, avtab_t * a,
> > uint32_t buf32[8], items, items2, val;
> > avtab_key_t key;
> > avtab_datum_t datum;
> > + avtab_trans_t trans;
> > avtab_extended_perms_t xperms;
> > unsigned set;
> > unsigned int i;
> > @@ -447,6 +461,7 @@ int avtab_read_item(struct policy_file *fp, uint32_t vers, avtab_t * a,
> >
> > memset(&key, 0, sizeof(avtab_key_t));
> > memset(&datum, 0, sizeof(avtab_datum_t));
> > + memset(&trans, 0, sizeof(avtab_trans_t));
> > memset(&xperms, 0, sizeof(avtab_extended_perms_t));
> >
> > if (vers < POLICYDB_VERSION_AVTAB) {
> > @@ -509,7 +524,14 @@ int avtab_read_item(struct policy_file *fp, uint32_t vers, avtab_t * a,
> > return -1;
> > }
> > key.specified = spec_order[i] | enabled;
> > - datum.data = le32_to_cpu(buf32[items++]);
> > + if (key.specified & AVTAB_TRANSITION) {
> > + trans.otype =
> > + le32_to_cpu(buf32[items++]);
> > + datum.trans = &trans;
> > + } else {
> > + datum.data =
> > + le32_to_cpu(buf32[items++]);
> > + }
> > rc = insertf(a, &key, &datum, p);
> > if (rc)
> > return rc;
> > @@ -571,6 +593,14 @@ int avtab_read_item(struct policy_file *fp, uint32_t vers, avtab_t * a,
> > for (i = 0; i < ARRAY_SIZE(xperms.perms); i++)
> > xperms.perms[i] = le32_to_cpu(buf32[i]);
> > datum.xperms = &xperms;
> > + } else if (key.specified & AVTAB_TRANSITION) {
> > + rc = next_entry(buf32, fp, sizeof(uint32_t));
> > + if (rc < 0) {
> > + ERR(fp->handle, "truncated entry");
> > + return -1;
> > + }
> > + trans.otype = le32_to_cpu(*buf32);
> > + datum.trans = &trans;
> > } else {
> > rc = next_entry(buf32, fp, sizeof(uint32_t));
> > if (rc < 0) {
> > diff --git a/libsepol/src/expand.c b/libsepol/src/expand.c
> > index 8795229a..6793a27d 100644
> > --- a/libsepol/src/expand.c
> > +++ b/libsepol/src/expand.c
> > @@ -1746,7 +1746,7 @@ static int expand_terule_helper(sepol_handle_t * handle,
> > if (conflict) {
> > avdatump = &node->datum;
> > if (specified & AVRULE_TRANSITION) {
> > - oldtype = avdatump->data;
> > + oldtype = avdatump->trans->otype;
> > } else if (specified & AVRULE_MEMBER) {
> > oldtype = avdatump->data;
> > } else if (specified & AVRULE_CHANGE) {
> > @@ -1789,7 +1789,11 @@ static int expand_terule_helper(sepol_handle_t * handle,
> > }
> >
> > avdatump = &node->datum;
> > - avdatump->data = remapped_data;
> > + if (specified & AVRULE_TRANSITION) {
> > + avdatump->trans->otype = remapped_data;
> > + } else {
> > + avdatump->data = remapped_data;
> > + }
> >
> > cur = cur->next;
> > }
> > diff --git a/libsepol/src/kernel_to_cil.c b/libsepol/src/kernel_to_cil.c
> > index f2b0d902..b1fd1bf7 100644
> > --- a/libsepol/src/kernel_to_cil.c
> > +++ b/libsepol/src/kernel_to_cil.c
> > @@ -1704,7 +1704,8 @@ 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)
> > {
> > - uint32_t data = datum->data;
> > + uint32_t data = key->specified & AVTAB_TRANSITION
> > + ? datum->trans->otype : datum->data;
> > type_datum_t *type;
> > const char *flavor, *tgt;
> > char *src, *class, *perms, *new;
> > diff --git a/libsepol/src/kernel_to_conf.c b/libsepol/src/kernel_to_conf.c
> > index 15161caa..7e1e1b49 100644
> > --- a/libsepol/src/kernel_to_conf.c
> > +++ b/libsepol/src/kernel_to_conf.c
> > @@ -1682,7 +1682,8 @@ exit:
> >
> > static char *avtab_node_to_str(struct policydb *pdb, avtab_key_t *key, avtab_datum_t *datum)
> > {
> > - uint32_t data = datum->data;
> > + uint32_t data = key->specified & AVTAB_TRANSITION
> > + ? datum->trans->otype : datum->data;
> > type_datum_t *type;
> > const char *flavor, *src, *tgt, *class, *perms, *new;
> > char *rule = NULL;
> > diff --git a/libsepol/src/optimize.c b/libsepol/src/optimize.c
> > index a38025ec..2d4a2d7a 100644
> > --- a/libsepol/src/optimize.c
> > +++ b/libsepol/src/optimize.c
> > @@ -308,6 +308,8 @@ static void optimize_avtab(policydb_t *p, const struct type_vec *type_map)
> > *cur = tmp->next;
> > if (tmp->key.specified & AVTAB_XPERMS)
> > free(tmp->datum.xperms);
> > + if (tmp->key.specified & AVTAB_TRANSITION)
> > + free(tmp->datum.trans);
> > free(tmp);
> >
> > tab->nel--;
> > @@ -427,6 +429,8 @@ static void optimize_cond_avtab(policydb_t *p, const struct type_vec *type_map)
> > *cur = tmp->next;
> > if (tmp->key.specified & AVTAB_XPERMS)
> > free(tmp->datum.xperms);
> > + if (tmp->key.specified & AVTAB_TRANSITION)
> > + free(tmp->datum.trans);
> > free(tmp);
> >
> > tab->nel--;
> > diff --git a/libsepol/src/policydb_validate.c b/libsepol/src/policydb_validate.c
> > index 3540f34a..f402b506 100644
> > --- a/libsepol/src/policydb_validate.c
> > +++ b/libsepol/src/policydb_validate.c
> > @@ -836,7 +836,9 @@ static int validate_avtab_key_and_datum(avtab_key_t *k, avtab_datum_t *d, void *
> > if (validate_avtab_key(k, 0, margs->policy, margs->flavors))
> > return -1;
> >
> > - if ((k->specified & AVTAB_TYPE) && validate_simpletype(d->data, margs->policy, margs->flavors))
> > + uint32_t otype = k->specified & AVTAB_TRANSITION
> > + ? d->trans->otype : d->data;
> > + if ((k->specified & AVTAB_TYPE) && validate_simpletype(otype, margs->policy, margs->flavors))
> > return -1;
> >
> > if ((k->specified & AVTAB_XPERMS) && validate_xperms(d->xperms))
> > diff --git a/libsepol/src/services.c b/libsepol/src/services.c
> > index 062510ab..72772dbd 100644
> > --- a/libsepol/src/services.c
> > +++ b/libsepol/src/services.c
> > @@ -1423,7 +1423,10 @@ static int sepol_compute_sid(sepol_security_id_t ssid,
> >
> > if (avdatum) {
> > /* Use the type from the type transition/member/change rule. */
> > - newcontext.type = avdatum->data;
> > + if (specified & AVTAB_TRANSITION)
> > + newcontext.type = avdatum->trans->otype;
> > + else
> > + newcontext.type = avdatum->data;
> > }
> >
> > /* Check for class-specific changes. */
> > diff --git a/libsepol/src/write.c b/libsepol/src/write.c
> > index 024fe628..0d3d5f14 100644
> > --- a/libsepol/src/write.c
> > +++ b/libsepol/src/write.c
> > @@ -190,14 +190,20 @@ static int avtab_write_item(policydb_t * p,
> > ERR(fp->handle, "missing node");
> > return POLICYDB_ERROR;
> > }
> > - buf32[items++] =
> > - cpu_to_le32(node->datum.data);
> > + uint32_t data =
> > + node->key.specified & AVTAB_TRANSITION
> > + ? node->datum.trans->otype
> > + : node->datum.data;
> > + buf32[items++] = cpu_to_le32(data);
> > set--;
> > node->merged = 1;
> > }
> > }
> > } else {
> > - buf32[items++] = cpu_to_le32(cur->datum.data);
> > + uint32_t data = cur->key.specified & AVTAB_TRANSITION
> > + ? cur->datum.trans->otype
> > + : cur->datum.data;
> > + buf32[items++] = cpu_to_le32(data);
> > cur->merged = 1;
> > set--;
> > }
> > @@ -256,6 +262,11 @@ static int avtab_write_item(policydb_t * p,
> > items = put_entry(buf32, sizeof(uint32_t),8,fp);
> > if (items != 8)
> > return POLICYDB_ERROR;
> > + } else if (cur->key.specified & AVTAB_TRANSITION) {
> > + buf32[0] = cpu_to_le32(cur->datum.trans->otype);
> > + items = put_entry(buf32, sizeof(uint32_t), 1, fp);
> > + if (items != 1)
> > + return POLICYDB_ERROR;
> > } else {
> > buf32[0] = cpu_to_le32(cur->datum.data);
> > items = put_entry(buf32, sizeof(uint32_t), 1, fp);
> > --
> > 2.40.0
> >
