On Tue, Mar 31, 2020 at 4:30 PM James Carter <jwcart2@xxxxxxxxx> wrote:
>
> On Sat, Mar 28, 2020 at 8:46 AM Ondrej Mosnacek <omosnace@xxxxxxxxxx> wrote:
> >
> > Implement a new, more space-efficient form of storing filename
> > transitions in the binary policy. The internal structures have already
> > been converted to this new representation; this patch just implements
> > reading/writing an equivalent representation from/to the binary policy.
> >
> > This new format reduces the size of Fedora policy from 7.6 MB to only
> > 3.3 MB (with policy optimization enabled in both cases). With the
> > unconfined module disabled, the size is reduced from 3.3 MB to 2.4 MB.
> >
> > Signed-off-by: Ondrej Mosnacek <omosnace@xxxxxxxxxx>
>
> Acked-by: James Carter <jwcart2@xxxxxxxxx>
>
Sorry, it has been a while.
I was going to merge these today, but upon further testing, I noticed
a couple of problems.
First, creating a policy.conf from a binary like this:
~/local/usr/bin/checkpolicy -F -M -b -o policy.31.conf policy.31
give the following error:
libsepol.avtab_read_item: invalid item count
Second, creating a binary from a policy.conf and then trying to create
a policy.conf from that binary:
~/local/usr/bin/checkpolicy -o policy.bin policy.conf
~/local/usr/bin/checkpolicy -F -b -o policy.bin.conf policy.bin
gives the following error:
security: ebitmap: high bit (32) is not a multiple of the map size (64)
I don't know if this is related to the problems you had on the kernel
side or not. I'll keep looking into this, but, for now, I'll have to
remove my Acked-by.
Jim
> > ---
> > libsepol/include/sepol/policydb/policydb.h | 3 +-
> > libsepol/src/policydb.c | 218 +++++++++++++++++----
> > libsepol/src/write.c | 72 ++++++-
> > 3 files changed, 242 insertions(+), 51 deletions(-)
> >
> > diff --git a/libsepol/include/sepol/policydb/policydb.h b/libsepol/include/sepol/policydb/policydb.h
> > index c3180c61..9ef43abc 100644
> > --- a/libsepol/include/sepol/policydb/policydb.h
> > +++ b/libsepol/include/sepol/policydb/policydb.h
> > @@ -755,10 +755,11 @@ extern int policydb_set_target_platform(policydb_t *p, int platform);
> > #define POLICYDB_VERSION_XPERMS_IOCTL 30 /* Linux-specific */
> > #define POLICYDB_VERSION_INFINIBAND 31 /* Linux-specific */
> > #define POLICYDB_VERSION_GLBLUB 32
> > +#define POLICYDB_VERSION_COMP_FTRANS 33 /* compressed filename transitions */
> >
> > /* Range of policy versions we understand*/
> > #define POLICYDB_VERSION_MIN POLICYDB_VERSION_BASE
> > -#define POLICYDB_VERSION_MAX POLICYDB_VERSION_GLBLUB
> > +#define POLICYDB_VERSION_MAX POLICYDB_VERSION_COMP_FTRANS
> >
> > /* Module versions and specific changes*/
> > #define MOD_POLICYDB_VERSION_BASE 4
> > diff --git a/libsepol/src/policydb.c b/libsepol/src/policydb.c
> > index 6b121d66..18ce1912 100644
> > --- a/libsepol/src/policydb.c
> > +++ b/libsepol/src/policydb.c
> > @@ -200,6 +200,13 @@ static struct policydb_compat_info policydb_compat[] = {
> > .ocon_num = OCON_IBENDPORT + 1,
> > .target_platform = SEPOL_TARGET_SELINUX,
> > },
> > + {
> > + .type = POLICY_KERN,
> > + .version = POLICYDB_VERSION_COMP_FTRANS,
> > + .sym_num = SYM_NUM,
> > + .ocon_num = OCON_IBENDPORT + 1,
> > + .target_platform = SEPOL_TARGET_SELINUX,
> > + },
> > {
> > .type = POLICY_BASE,
> > .version = MOD_POLICYDB_VERSION_BASE,
> > @@ -2661,73 +2668,202 @@ int policydb_filetrans_insert(policydb_t *p, uint32_t stype, uint32_t ttype,
> > return ebitmap_set_bit(&datum->stypes, stype - 1, 1);
> > }
> >
> > -int filename_trans_read(policydb_t *p, struct policy_file *fp)
> > +static int filename_trans_read_one_old(policydb_t *p, struct policy_file *fp)
> > {
> > - unsigned int i;
> > - uint32_t buf[4], nel, len, stype, ttype, tclass, otype;
> > + uint32_t buf[4], len, stype, ttype, tclass, otype;
> > + char *name = NULL;
> > int rc;
> > - char *name;
> >
> > rc = next_entry(buf, fp, sizeof(uint32_t));
> > if (rc < 0)
> > return -1;
> > - nel = le32_to_cpu(buf[0]);
> > + len = le32_to_cpu(buf[0]);
> > + if (zero_or_saturated(len))
> > + return -1;
> >
> > - for (i = 0; i < nel; i++) {
> > - name = NULL;
> > + name = calloc(len + 1, sizeof(*name));
> > + if (!name)
> > + return -1;
> >
> > - rc = next_entry(buf, fp, sizeof(uint32_t));
> > - if (rc < 0)
> > - goto err;
> > - len = le32_to_cpu(buf[0]);
> > - if (zero_or_saturated(len))
> > + rc = next_entry(name, fp, len);
> > + if (rc < 0)
> > + goto err;
> > +
> > + rc = next_entry(buf, fp, sizeof(uint32_t) * 4);
> > + if (rc < 0)
> > + goto err;
> > +
> > + stype = le32_to_cpu(buf[0]);
> > + ttype = le32_to_cpu(buf[1]);
> > + tclass = le32_to_cpu(buf[2]);
> > + otype = le32_to_cpu(buf[3]);
> > +
> > + rc = policydb_filetrans_insert(p, stype, ttype, tclass, name, &name,
> > + otype, NULL);
> > + if (rc) {
> > + if (rc != SEPOL_EEXIST)
> > goto err;
> > + /*
> > + * Some old policies were wrongly generated with
> > + * duplicate filename transition rules. For backward
> > + * compatibility, do not reject such policies, just
> > + * issue a warning and ignore the duplicate.
> > + */
> > + WARN(fp->handle,
> > + "Duplicate name-based type_transition %s %s:%s \"%s\": %s, ignoring",
> > + p->p_type_val_to_name[stype - 1],
> > + p->p_type_val_to_name[ttype - 1],
> > + p->p_class_val_to_name[tclass - 1],
> > + name,
> > + p->p_type_val_to_name[otype - 1]);
> > + /* continue, ignoring this one */
> > + }
> > + free(name);
> > + return 0;
> > +err:
> > + free(name);
> > + return -1;
> > +}
> > +
> > +static int filename_trans_check_datum(filename_trans_datum_t *datum)
> > +{
> > + ebitmap_t stypes, otypes;
> > +
> > + ebitmap_init(&stypes);
> > + ebitmap_init(&otypes);
> > +
> > + while (datum) {
> > + if (ebitmap_get_bit(&otypes, datum->otype))
> > + return -1;
> > +
> > + if (ebitmap_set_bit(&otypes, datum->otype, 1))
> > + return -1;
> >
> > - name = calloc(len + 1, sizeof(*name));
> > - if (!name)
> > + if (ebitmap_match_any(&stypes, &datum->stypes))
> > + return -1;
> > +
> > + if (ebitmap_union(&stypes, &datum->stypes))
> > + return -1;
> > + }
> > + return 0;
> > +}
> > +
> > +static int filename_trans_read_one_new(policydb_t *p, struct policy_file *fp)
> > +{
> > + filename_trans_key_t *ft = NULL;
> > + filename_trans_datum_t **dst, *datum, *first = NULL;
> > + unsigned int i;
> > + uint32_t buf[3], len, ttype, tclass, ndatum;
> > + char *name = NULL;
> > + int rc;
> > +
> > + rc = next_entry(buf, fp, sizeof(uint32_t));
> > + if (rc < 0)
> > + return -1;
> > + len = le32_to_cpu(buf[0]);
> > + if (zero_or_saturated(len))
> > + return -1;
> > +
> > + name = calloc(len + 1, sizeof(*name));
> > + if (!name)
> > + return -1;
> > +
> > + rc = next_entry(name, fp, len);
> > + if (rc < 0)
> > + goto err;
> > +
> > + rc = next_entry(buf, fp, sizeof(uint32_t) * 3);
> > + if (rc < 0)
> > + goto err;
> > +
> > + ttype = le32_to_cpu(buf[0]);
> > + tclass = le32_to_cpu(buf[1]);
> > + ndatum = le32_to_cpu(buf[2]);
> > + if (ndatum == 0)
> > + goto err;
> > +
> > + dst = &first;
> > + for (i = 0; i < ndatum; i++) {
> > + datum = malloc(sizeof(*datum));
> > + if (!datum)
> > goto err;
> >
> > - rc = next_entry(name, fp, len);
> > + *dst = datum;
> > +
> > + /* ebitmap_read() will at least init the bitmap */
> > + rc = ebitmap_read(&datum->stypes, fp);
> > if (rc < 0)
> > goto err;
> >
> > - rc = next_entry(buf, fp, sizeof(uint32_t) * 4);
> > + rc = next_entry(buf, fp, sizeof(uint32_t));
> > if (rc < 0)
> > goto err;
> >
> > - stype = le32_to_cpu(buf[0]);
> > - ttype = le32_to_cpu(buf[1]);
> > - tclass = le32_to_cpu(buf[2]);
> > - otype = le32_to_cpu(buf[3]);
> > + datum->otype = le32_to_cpu(buf[0]);
> >
> > - rc = policydb_filetrans_insert(p, stype, ttype, tclass, name,
> > - &name, otype, NULL);
> > - if (rc) {
> > - if (rc != SEPOL_EEXIST)
> > - goto err;
> > - /*
> > - * Some old policies were wrongly generated with
> > - * duplicate filename transition rules. For backward
> > - * compatibility, do not reject such policies, just
> > - * issue a warning and ignore the duplicate.
> > - */
> > - WARN(fp->handle,
> > - "Duplicate name-based type_transition %s %s:%s \"%s\": %s, ignoring",
> > - p->p_type_val_to_name[stype - 1],
> > - p->p_type_val_to_name[ttype - 1],
> > - p->p_class_val_to_name[tclass - 1],
> > - name,
> > - p->p_type_val_to_name[otype - 1]);
> > - /* continue, ignoring this one */
> > - }
> > - free(name);
> > + p->filename_trans_count += ebitmap_cardinality(&datum->stypes);
> > +
> > + dst = &datum->next;
> > }
> > + *dst = NULL;
> > +
> > + if (ndatum > 1 && filename_trans_check_datum(first))
> > + goto err;
> > +
> > + ft = malloc(sizeof(*ft));
> > + if (!ft)
> > + goto err;
> > +
> > + ft->ttype = ttype;
> > + ft->tclass = tclass;
> > + ft->name = name;
> > +
> > + rc = hashtab_insert(p->filename_trans, (hashtab_key_t)ft,
> > + (hashtab_datum_t)first);
> > + if (rc)
> > + goto err;
> > +
> > return 0;
> > err:
> > + free(ft);
> > free(name);
> > + while (first) {
> > + datum = first;
> > + first = first->next;
> > +
> > + ebitmap_destroy(&datum->stypes);
> > + free(datum);
> > + }
> > return -1;
> > }
> >
> > +int filename_trans_read(policydb_t *p, struct policy_file *fp)
> > +{
> > + unsigned int i;
> > + uint32_t buf[1], nel;
> > + int rc;
> > +
> > + rc = next_entry(buf, fp, sizeof(uint32_t));
> > + if (rc < 0)
> > + return -1;
> > + nel = le32_to_cpu(buf[0]);
> > +
> > + if (p->policyvers < POLICYDB_VERSION_COMP_FTRANS) {
> > + for (i = 0; i < nel; i++) {
> > + rc = filename_trans_read_one_old(p, fp);
> > + if (rc < 0)
> > + return -1;
> > + }
> > + } else {
> > + for (i = 0; i < nel; i++) {
> > + rc = filename_trans_read_one_new(p, fp);
> > + if (rc < 0)
> > + return -1;
> > + }
> > + }
> > + return 0;
> > +}
> > +
> > static int ocontext_read_xen(struct policydb_compat_info *info,
> > policydb_t *p, struct policy_file *fp)
> > {
> > diff --git a/libsepol/src/write.c b/libsepol/src/write.c
> > index d3aee8d5..9c74d9f5 100644
> > --- a/libsepol/src/write.c
> > +++ b/libsepol/src/write.c
> > @@ -569,7 +569,7 @@ static int role_allow_write(role_allow_t * r, struct policy_file *fp)
> > return POLICYDB_SUCCESS;
> > }
> >
> > -static int filename_write_helper(hashtab_key_t key, void *data, void *ptr)
> > +static int filename_write_one_old(hashtab_key_t key, void *data, void *ptr)
> > {
> > uint32_t bit, buf[4];
> > size_t items, len;
> > @@ -605,6 +605,54 @@ static int filename_write_helper(hashtab_key_t key, void *data, void *ptr)
> > return 0;
> > }
> >
> > +static int filename_write_one_new(hashtab_key_t key, void *data, void *ptr)
> > +{
> > + uint32_t buf[3];
> > + size_t items, len, ndatum;
> > + filename_trans_key_t *ft = (filename_trans_key_t *)key;
> > + filename_trans_datum_t *datum;
> > + void *fp = ptr;
> > +
> > + len = strlen(ft->name);
> > + buf[0] = cpu_to_le32(len);
> > + items = put_entry(buf, sizeof(uint32_t), 1, fp);
> > + if (items != 1)
> > + return POLICYDB_ERROR;
> > +
> > + items = put_entry(ft->name, sizeof(char), len, fp);
> > + if (items != len)
> > + return POLICYDB_ERROR;
> > +
> > + ndatum = 0;
> > + datum = data;
> > + do {
> > + ndatum++;
> > + datum = datum->next;
> > + } while (datum);
> > +
> > + buf[0] = cpu_to_le32(ft->ttype);
> > + buf[1] = cpu_to_le32(ft->tclass);
> > + buf[2] = cpu_to_le32(ndatum);
> > + items = put_entry(buf, sizeof(uint32_t), 3, fp);
> > + if (items != 3)
> > + return POLICYDB_ERROR;
> > +
> > + datum = data;
> > + do {
> > + if (ebitmap_write(&datum->stypes, fp))
> > + return POLICYDB_ERROR;
> > +
> > + buf[0] = cpu_to_le32(datum->otype);
> > + items = put_entry(buf, sizeof(uint32_t), 1, fp);
> > + if (items != 1)
> > + return POLICYDB_ERROR;
> > +
> > + datum = datum->next;
> > + } while (datum);
> > +
> > + return 0;
> > +}
> > +
> > static int filename_trans_write(struct policydb *p, void *fp)
> > {
> > size_t items;
> > @@ -614,16 +662,22 @@ static int filename_trans_write(struct policydb *p, void *fp)
> > if (p->policyvers < POLICYDB_VERSION_FILENAME_TRANS)
> > return 0;
> >
> > - buf[0] = cpu_to_le32(p->filename_trans_count);
> > - items = put_entry(buf, sizeof(uint32_t), 1, fp);
> > - if (items != 1)
> > - return POLICYDB_ERROR;
> > + if (p->policyvers < POLICYDB_VERSION_COMP_FTRANS) {
> > + buf[0] = cpu_to_le32(p->filename_trans_count);
> > + items = put_entry(buf, sizeof(uint32_t), 1, fp);
> > + if (items != 1)
> > + return POLICYDB_ERROR;
> >
> > - rc = hashtab_map(p->filename_trans, filename_write_helper, fp);
> > - if (rc)
> > - return rc;
> > + rc = hashtab_map(p->filename_trans, filename_write_one_old, fp);
> > + } else {
> > + buf[0] = cpu_to_le32(p->filename_trans->nel);
> > + items = put_entry(buf, sizeof(uint32_t), 1, fp);
> > + if (items != 1)
> > + return POLICYDB_ERROR;
> >
> > - return 0;
> > + rc = hashtab_map(p->filename_trans, filename_write_one_new, fp);
> > + }
> > + return rc;
> > }
> >
> > static int role_set_write(role_set_t * x, struct policy_file *fp)
> > --
> > 2.25.1
> >
