On Wed, Jul 12, 2023 at 3:17 PM Christian Göttsche
<cgzones@xxxxxxxxxxxxxx> wrote:
>
> On Wed, 12 Jul 2023 at 19:34, James Carter <jwcart2@xxxxxxxxx> wrote:
> >
> > 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
>
> If I remember correctly the kernel patches have not been ack'ed yet.
>
Yeah, you're right. I was thinking of the initial sid patches. I might
have jumped the gun a bit here.
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
> > > >
