Re: [PATCH v2 1/8] checkpolicy,libsepol: move transition to separate structure in avtab

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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
> > > >




[Index of Archives]     [Selinux Refpolicy]     [Linux SGX]     [Fedora Users]     [Fedora Desktop]     [Yosemite Photos]     [Yosemite Camping]     [Yosemite Campsites]     [KDE Users]     [Gnome Users]

  Powered by Linux