| Message ID | 20190826142008.2198-1-joshua.brindle@crunchydata.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | default_range glblub implementation | expand |
On Mon, Aug 26, 2019 at 10:20 AM Joshua Brindle <joshua.brindle@crunchydata.com> wrote: > A policy developer can now specify glblub as a default_range default and > the computed transition will be the intersection of the mls range of > the two contexts. > > Signed-off-by: Joshua Brindle <joshua.brindle@crunchydata.com> > --- > security/selinux/include/security.h | 3 ++- > security/selinux/ss/context.h | 6 ++++++ > security/selinux/ss/ebitmap.c | 15 +++++++++++++++ > security/selinux/ss/ebitmap.h | 1 + > security/selinux/ss/mls.c | 2 ++ > security/selinux/ss/mls_types.h | 28 ++++++++++++++++++++++++++++ > security/selinux/ss/policydb.c | 5 +++++ > security/selinux/ss/policydb.h | 1 + > 8 files changed, 60 insertions(+), 1 deletion(-) Independent from the comments below I think we need to additional things for this patch: * A much better description. At the very least I would like you to explain the MLS bounding and how one might use this (think sample code); and don't worry about your description being too long ;) * Tests (see the selinux-testsuite). > diff --git a/security/selinux/ss/context.h b/security/selinux/ss/context.h > index 2260c44a568c..cecb84d8b26c 100644 > --- a/security/selinux/ss/context.h > +++ b/security/selinux/ss/context.h > @@ -95,6 +95,12 @@ static inline int mls_context_cpy_high(struct context *dst, struct context *src) > return rc; > } > > + > +static inline int mls_context_glblub(struct context *dst, struct context *c1, struct context *c2) > +{ > + return mls_range_glblub(&dst->range, &c1->range, &c2->range); > +} Considering the other functions that are already defined in context.h, it seems like you could get rid of mls_range_glblub() and put the guts in mls_context_glblub(), yes? Unless I missed something mls_range_glblub() is only ever called from mls_context_glblub() which makes one of these functions pointless. > static inline int mls_context_cmp(struct context *c1, struct context *c2) > { > return ((c1->range.level[0].sens == c2->range.level[0].sens) && > diff --git a/security/selinux/ss/ebitmap.c b/security/selinux/ss/ebitmap.c > index 09929fc5ab47..2042729b81f8 100644 > --- a/security/selinux/ss/ebitmap.c > +++ b/security/selinux/ss/ebitmap.c > @@ -77,6 +77,21 @@ int ebitmap_cpy(struct ebitmap *dst, struct ebitmap *src) > return 0; > } > > +int ebitmap_and(struct ebitmap *dst, struct ebitmap *e1, struct ebitmap *e2) > +{ > + unsigned int i, length = min(ebitmap_length(e1), ebitmap_length(e2)); A line of vertical whitespace here would be nice. > + ebitmap_init(dst); > + for (i=0; i < length; i++) { > + if (ebitmap_get_bit(e1, i) && ebitmap_get_bit(e2, i)) { > + int rc = ebitmap_set_bit(dst, i, 1); > + if (rc < 0) > + return rc; > + } > + } Same as above, a line of vertical whitespace would be nice. Beyond that, since this is an AND operation, could we make better use of things like find_first_bit()/ebitmap_start_positive()/ebitmap_next_positive() to skip along one of the bitmaps instead of needing to call ebitmap_get_bit() for each bit? I imagine it would be quicker that way. > + return 0; > +} ... > diff --git a/security/selinux/ss/mls_types.h b/security/selinux/ss/mls_types.h > index 068e0d7809db..e2a20eb0e87c 100644 > --- a/security/selinux/ss/mls_types.h > +++ b/security/selinux/ss/mls_types.h > @@ -39,6 +39,34 @@ static inline int mls_level_dom(struct mls_level *l1, struct mls_level *l2) > ebitmap_contains(&l1->cat, &l2->cat, 0)); > } > > +static inline int mls_range_glblub(struct mls_range *dst, struct mls_range *r1, struct mls_range *r2) > +{ > + int rc = 0; > + > + if (r1->level[1].sens < r2->level[0].sens || r2->level[1].sens < r1->level[0].sens) { > + { These braces aren't necessary, take them from the patch and give them to a child in need. > + // These ranges have no common sensitivities Please use the old C style comments instead of the C++ style comments (this applies to the whole patch). I know you don't track kernel development very closely, but we just had a discussion about this on-list earlier this summer. > + return -1; > + } > + > + // Take the greatest of the low > + dst->level[0].sens = max(r1->level[0].sens, r2->level[0].sens); > + > + // Take the least of the high > + dst->level[1].sens = min(r1->level[1].sens, r2->level[1].sens); > + > + rc = ebitmap_and(&dst->level[0].cat, &r1->level[0].cat, &r2->level[0].cat); > + if (rc) > + goto out; > + > + rc = ebitmap_and(&dst->level[1].cat, &r1->level[1].cat, &r2->level[1].cat); > + if (rc) > + goto out; > + > +out: > + return rc; > +}
diff --git a/security/selinux/include/security.h b/security/selinux/include/security.h index 111121281c47..ae840634e3c7 100644 --- a/security/selinux/include/security.h +++ b/security/selinux/include/security.h @@ -40,10 +40,11 @@ #define POLICYDB_VERSION_CONSTRAINT_NAMES 29 #define POLICYDB_VERSION_XPERMS_IOCTL 30 #define POLICYDB_VERSION_INFINIBAND 31 +#define POLICYDB_VERSION_GLBLUB 32 /* Range of policy versions we understand*/ #define POLICYDB_VERSION_MIN POLICYDB_VERSION_BASE -#define POLICYDB_VERSION_MAX POLICYDB_VERSION_INFINIBAND +#define POLICYDB_VERSION_MAX POLICYDB_VERSION_GLBLUB /* Mask for just the mount related flags */ #define SE_MNTMASK 0x0f diff --git a/security/selinux/ss/context.h b/security/selinux/ss/context.h index 2260c44a568c..cecb84d8b26c 100644 --- a/security/selinux/ss/context.h +++ b/security/selinux/ss/context.h @@ -95,6 +95,12 @@ static inline int mls_context_cpy_high(struct context *dst, struct context *src) return rc; } + +static inline int mls_context_glblub(struct context *dst, struct context *c1, struct context *c2) +{ + return mls_range_glblub(&dst->range, &c1->range, &c2->range); +} + static inline int mls_context_cmp(struct context *c1, struct context *c2) { return ((c1->range.level[0].sens == c2->range.level[0].sens) && diff --git a/security/selinux/ss/ebitmap.c b/security/selinux/ss/ebitmap.c index 09929fc5ab47..2042729b81f8 100644 --- a/security/selinux/ss/ebitmap.c +++ b/security/selinux/ss/ebitmap.c @@ -77,6 +77,21 @@ int ebitmap_cpy(struct ebitmap *dst, struct ebitmap *src) return 0; } +int ebitmap_and(struct ebitmap *dst, struct ebitmap *e1, struct ebitmap *e2) +{ + unsigned int i, length = min(ebitmap_length(e1), ebitmap_length(e2)); + ebitmap_init(dst); + for (i=0; i < length; i++) { + if (ebitmap_get_bit(e1, i) && ebitmap_get_bit(e2, i)) { + int rc = ebitmap_set_bit(dst, i, 1); + if (rc < 0) + return rc; + } + } + return 0; +} + + #ifdef CONFIG_NETLABEL /** * ebitmap_netlbl_export - Export an ebitmap into a NetLabel category bitmap diff --git a/security/selinux/ss/ebitmap.h b/security/selinux/ss/ebitmap.h index 6aa7cf6a2197..9a23b81b8832 100644 --- a/security/selinux/ss/ebitmap.h +++ b/security/selinux/ss/ebitmap.h @@ -124,6 +124,7 @@ static inline void ebitmap_node_clr_bit(struct ebitmap_node *n, int ebitmap_cmp(struct ebitmap *e1, struct ebitmap *e2); int ebitmap_cpy(struct ebitmap *dst, struct ebitmap *src); +int ebitmap_and(struct ebitmap *dst, struct ebitmap *e1, struct ebitmap *e2); int ebitmap_contains(struct ebitmap *e1, struct ebitmap *e2, u32 last_e2bit); int ebitmap_get_bit(struct ebitmap *e, unsigned long bit); int ebitmap_set_bit(struct ebitmap *e, unsigned long bit, int value); diff --git a/security/selinux/ss/mls.c b/security/selinux/ss/mls.c index 5e05f5b902d7..76c8ad014ac9 100644 --- a/security/selinux/ss/mls.c +++ b/security/selinux/ss/mls.c @@ -529,6 +529,8 @@ int mls_compute_sid(struct policydb *p, return mls_context_cpy_high(newcontext, tcontext); case DEFAULT_TARGET_LOW_HIGH: return mls_context_cpy(newcontext, tcontext); + case DEFAULT_GLBLUB: + return mls_context_glblub(newcontext, scontext, tcontext); } /* Fallthrough */ diff --git a/security/selinux/ss/mls_types.h b/security/selinux/ss/mls_types.h index 068e0d7809db..e2a20eb0e87c 100644 --- a/security/selinux/ss/mls_types.h +++ b/security/selinux/ss/mls_types.h @@ -39,6 +39,34 @@ static inline int mls_level_dom(struct mls_level *l1, struct mls_level *l2) ebitmap_contains(&l1->cat, &l2->cat, 0)); } +static inline int mls_range_glblub(struct mls_range *dst, struct mls_range *r1, struct mls_range *r2) +{ + int rc = 0; + + if (r1->level[1].sens < r2->level[0].sens || r2->level[1].sens < r1->level[0].sens) { + { + // These ranges have no common sensitivities + return -1; + } + + // Take the greatest of the low + dst->level[0].sens = max(r1->level[0].sens, r2->level[0].sens); + + // Take the least of the high + dst->level[1].sens = min(r1->level[1].sens, r2->level[1].sens); + + rc = ebitmap_and(&dst->level[0].cat, &r1->level[0].cat, &r2->level[0].cat); + if (rc) + goto out; + + rc = ebitmap_and(&dst->level[1].cat, &r1->level[1].cat, &r2->level[1].cat); + if (rc) + goto out; + +out: + return rc; +} + #define mls_level_incomp(l1, l2) \ (!mls_level_dom((l1), (l2)) && !mls_level_dom((l2), (l1))) diff --git a/security/selinux/ss/policydb.c b/security/selinux/ss/policydb.c index f8efaa9f647c..1b59f72effbb 100644 --- a/security/selinux/ss/policydb.c +++ b/security/selinux/ss/policydb.c @@ -160,6 +160,11 @@ static struct policydb_compat_info policydb_compat[] = { .sym_num = SYM_NUM, .ocon_num = OCON_NUM, }, + { + .version = POLICYDB_VERSION_GLBLUB, + .sym_num = SYM_NUM, + .ocon_num = OCON_NUM, + }, }; static struct policydb_compat_info *policydb_lookup_compat(int version) diff --git a/security/selinux/ss/policydb.h b/security/selinux/ss/policydb.h index fcc6366b447f..0c41d0b4da96 100644 --- a/security/selinux/ss/policydb.h +++ b/security/selinux/ss/policydb.h @@ -69,6 +69,7 @@ struct class_datum { #define DEFAULT_TARGET_LOW 4 #define DEFAULT_TARGET_HIGH 5 #define DEFAULT_TARGET_LOW_HIGH 6 +#define DEFAULT_GLBLUB 7 char default_range; };
A policy developer can now specify glblub as a default_range default and the computed transition will be the intersection of the mls range of the two contexts. Signed-off-by: Joshua Brindle <joshua.brindle@crunchydata.com> --- security/selinux/include/security.h | 3 ++- security/selinux/ss/context.h | 6 ++++++ security/selinux/ss/ebitmap.c | 15 +++++++++++++++ security/selinux/ss/ebitmap.h | 1 + security/selinux/ss/mls.c | 2 ++ security/selinux/ss/mls_types.h | 28 ++++++++++++++++++++++++++++ security/selinux/ss/policydb.c | 5 +++++ security/selinux/ss/policydb.h | 1 + 8 files changed, 60 insertions(+), 1 deletion(-)