selinux.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2] default_range glblub implementation
@ 2019-08-28 20:43 Joshua Brindle
  2019-08-28 22:31 ` Paul Moore
  2019-08-29 20:45 ` Stephen Smalley
  0 siblings, 2 replies; 8+ messages in thread
From: Joshua Brindle @ 2019-08-28 20:43 UTC (permalink / raw)
  To: selinux; +Cc: paul, Joshua Brindle

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.

The glb (greatest lower bound) lub (lowest upper bound) of a range is calculated
as the greater of the low sensitivities and the lower of the high sensitivities
and the and of each category bitmap.

This can be used by MLS solution developers to compute a context that satisfies,
for example, the range of a network interface and the range of a user logging in.

Some examples are:

User Permitted Range | Network Device Label | Computed Label
---------------------|----------------------|----------------
S0-S1:c0.c12         | S0                   | S0
S0-S1:c0.c12         | S0-S1:c0.c1024       | S0-S1:c0.c12
S0-S4:c0.c512        | S1-S1:c0.c1024       | S1-S1:c0.c512
S0-S16:c0,c2         | S4-S6:c0.c128        | S4-S6:c0,c2
S0-S4                | S2-S6                | S2-S4
S0-S4                | S5-S8                | INVALID
S5-S8                | S0-S4                | INVALID
S6:c0,c2-S7:c4,c5    | S0:c2,c4-S6:c5.c100  | S6:c2-S6:c5

Signed-off-by: Joshua Brindle <joshua.brindle@crunchydata.com>
---
 security/selinux/include/security.h |  3 ++-
 security/selinux/ss/context.h       | 28 ++++++++++++++++++++++++++++
 security/selinux/ss/ebitmap.c       | 18 ++++++++++++++++++
 security/selinux/ss/ebitmap.h       |  1 +
 security/selinux/ss/mls.c           |  2 ++
 security/selinux/ss/policydb.c      |  5 +++++
 security/selinux/ss/policydb.h      |  1 +
 7 files changed, 57 insertions(+), 1 deletion(-)

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..0a0fbcc0d90c 100644
--- a/security/selinux/ss/context.h
+++ b/security/selinux/ss/context.h
@@ -95,6 +95,34 @@ 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)
+{
+	struct mls_range *dr = &dst->range, *r1 = &c1->range, *r2 = &c2->range;
+	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
+	dr->level[0].sens = max(r1->level[0].sens, r2->level[0].sens);
+
+        // Take the least of the high
+	dr->level[1].sens = min(r1->level[1].sens, r2->level[1].sens);
+
+	rc = ebitmap_and(&dr->level[0].cat, &r1->level[0].cat, &r2->level[0].cat);
+	if (rc)
+		goto out;
+
+	rc = ebitmap_and(&dr->level[1].cat, &r1->level[1].cat, &r2->level[1].cat);
+	if (rc)
+		goto out;
+
+out:
+	return rc;
+}
+
 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..c0a9f8ecffc3 100644
--- a/security/selinux/ss/ebitmap.c
+++ b/security/selinux/ss/ebitmap.c
@@ -77,6 +77,24 @@ int ebitmap_cpy(struct ebitmap *dst, struct ebitmap *src)
 	return 0;
 }
 
+int ebitmap_and(struct ebitmap *dst, struct ebitmap *e1, struct ebitmap *e2)
+{
+	struct ebitmap_node *n;
+	int bit;
+
+	ebitmap_init(dst);
+
+	ebitmap_for_each_positive_bit(e1, n, bit) {
+		if (ebitmap_get_bit(e2, bit)) {
+			int rc = ebitmap_set_bit(dst, bit, 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/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;
 };
 
-- 
2.21.0


^ permalink raw reply related	[flat|nested] 8+ messages in thread
* Re: [PATCH v2] default_range glblub implementation
  2019-08-28 20:43 [PATCH v2] default_range glblub implementation Joshua Brindle
@ 2019-08-28 22:31 ` Paul Moore
  2019-08-29 13:29   ` Joshua Brindle
  2019-08-29 20:45 ` Stephen Smalley
  1 sibling, 1 reply; 8+ messages in thread
From: Paul Moore @ 2019-08-28 22:31 UTC (permalink / raw)
  To: Joshua Brindle; +Cc: selinux

On Wed, Aug 28, 2019 at 4:43 PM 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.
>
> The glb (greatest lower bound) lub (lowest upper bound) of a range is calculated
> as the greater of the low sensitivities and the lower of the high sensitivities
> and the and of each category bitmap.
>
> This can be used by MLS solution developers to compute a context that satisfies,
> for example, the range of a network interface and the range of a user logging in.
>
> Some examples are:
>
> User Permitted Range | Network Device Label | Computed Label
> ---------------------|----------------------|----------------
> S0-S1:c0.c12         | S0                   | S0
> S0-S1:c0.c12         | S0-S1:c0.c1024       | S0-S1:c0.c12
> S0-S4:c0.c512        | S1-S1:c0.c1024       | S1-S1:c0.c512
> S0-S16:c0,c2         | S4-S6:c0.c128        | S4-S6:c0,c2
> S0-S4                | S2-S6                | S2-S4
> S0-S4                | S5-S8                | INVALID
> S5-S8                | S0-S4                | INVALID
> S6:c0,c2-S7:c4,c5    | S0:c2,c4-S6:c5.c100  | S6:c2-S6:c5
>
> Signed-off-by: Joshua Brindle <joshua.brindle@crunchydata.com>
> ---
>  security/selinux/include/security.h |  3 ++-
>  security/selinux/ss/context.h       | 28 ++++++++++++++++++++++++++++
>  security/selinux/ss/ebitmap.c       | 18 ++++++++++++++++++
>  security/selinux/ss/ebitmap.h       |  1 +
>  security/selinux/ss/mls.c           |  2 ++
>  security/selinux/ss/policydb.c      |  5 +++++
>  security/selinux/ss/policydb.h      |  1 +
>  7 files changed, 57 insertions(+), 1 deletion(-)

You incorporated some feedback from the v1 patch, but you ignored
some, can you explain why?

For reference, here are my comments on your first patch:
* https://lore.kernel.org/selinux/CAHC9VhRXyRDjj3KJDHvA4ruJg6H+1kzFPzfA-PLZ-NqBicsLrw@mail.gmail.com/

-- 
paul moore
www.paul-moore.com

^ permalink raw reply	[flat|nested] 8+ messages in thread
* Re: [PATCH v2] default_range glblub implementation
  2019-08-28 22:31 ` Paul Moore
@ 2019-08-29 13:29   ` Joshua Brindle
  2019-08-29 16:58     ` Paul Moore
  0 siblings, 1 reply; 8+ messages in thread
From: Joshua Brindle @ 2019-08-29 13:29 UTC (permalink / raw)
  To: Paul Moore; +Cc: selinux

On Wed, Aug 28, 2019 at 6:31 PM Paul Moore <paul@paul-moore.com> wrote:
>
> On Wed, Aug 28, 2019 at 4:43 PM 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.
> >
> > The glb (greatest lower bound) lub (lowest upper bound) of a range is calculated
> > as the greater of the low sensitivities and the lower of the high sensitivities
> > and the and of each category bitmap.
> >
> > This can be used by MLS solution developers to compute a context that satisfies,
> > for example, the range of a network interface and the range of a user logging in.
> >
> > Some examples are:
> >
> > User Permitted Range | Network Device Label | Computed Label
> > ---------------------|----------------------|----------------
> > S0-S1:c0.c12         | S0                   | S0
> > S0-S1:c0.c12         | S0-S1:c0.c1024       | S0-S1:c0.c12
> > S0-S4:c0.c512        | S1-S1:c0.c1024       | S1-S1:c0.c512
> > S0-S16:c0,c2         | S4-S6:c0.c128        | S4-S6:c0,c2
> > S0-S4                | S2-S6                | S2-S4
> > S0-S4                | S5-S8                | INVALID
> > S5-S8                | S0-S4                | INVALID
> > S6:c0,c2-S7:c4,c5    | S0:c2,c4-S6:c5.c100  | S6:c2-S6:c5
> >
> > Signed-off-by: Joshua Brindle <joshua.brindle@crunchydata.com>
> > ---
> >  security/selinux/include/security.h |  3 ++-
> >  security/selinux/ss/context.h       | 28 ++++++++++++++++++++++++++++
> >  security/selinux/ss/ebitmap.c       | 18 ++++++++++++++++++
> >  security/selinux/ss/ebitmap.h       |  1 +
> >  security/selinux/ss/mls.c           |  2 ++
> >  security/selinux/ss/policydb.c      |  5 +++++
> >  security/selinux/ss/policydb.h      |  1 +
> >  7 files changed, 57 insertions(+), 1 deletion(-)
>
> You incorporated some feedback from the v1 patch, but you ignored
> some, can you explain why?
>

I apologize, I missed a couple C++ style comments, I'll fix those and
resend, was there anything else? I thought I addressed all of the
technical concerns.

> For reference, here are my comments on your first patch:
> * https://lore.kernel.org/selinux/CAHC9VhRXyRDjj3KJDHvA4ruJg6H+1kzFPzfA-PLZ-NqBicsLrw@mail.gmail.com/
>
> --
> paul moore
> www.paul-moore.com

^ permalink raw reply	[flat|nested] 8+ messages in thread
* Re: [PATCH v2] default_range glblub implementation
  2019-08-29 13:29   ` Joshua Brindle
@ 2019-08-29 16:58     ` Paul Moore
  2019-08-29 17:11       ` Joshua Brindle
  0 siblings, 1 reply; 8+ messages in thread
From: Paul Moore @ 2019-08-29 16:58 UTC (permalink / raw)
  To: Joshua Brindle; +Cc: selinux

On Thu, Aug 29, 2019 at 9:29 AM Joshua Brindle
<joshua.brindle@crunchydata.com> wrote:
> On Wed, Aug 28, 2019 at 6:31 PM Paul Moore <paul@paul-moore.com> wrote:
> > On Wed, Aug 28, 2019 at 4:43 PM 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.
> > >
> > > The glb (greatest lower bound) lub (lowest upper bound) of a range is calculated
> > > as the greater of the low sensitivities and the lower of the high sensitivities
> > > and the and of each category bitmap.
> > >
> > > This can be used by MLS solution developers to compute a context that satisfies,
> > > for example, the range of a network interface and the range of a user logging in.
> > >
> > > Some examples are:
> > >
> > > User Permitted Range | Network Device Label | Computed Label
> > > ---------------------|----------------------|----------------
> > > S0-S1:c0.c12         | S0                   | S0
> > > S0-S1:c0.c12         | S0-S1:c0.c1024       | S0-S1:c0.c12
> > > S0-S4:c0.c512        | S1-S1:c0.c1024       | S1-S1:c0.c512
> > > S0-S16:c0,c2         | S4-S6:c0.c128        | S4-S6:c0,c2
> > > S0-S4                | S2-S6                | S2-S4
> > > S0-S4                | S5-S8                | INVALID
> > > S5-S8                | S0-S4                | INVALID
> > > S6:c0,c2-S7:c4,c5    | S0:c2,c4-S6:c5.c100  | S6:c2-S6:c5
> > >
> > > Signed-off-by: Joshua Brindle <joshua.brindle@crunchydata.com>
> > > ---
> > >  security/selinux/include/security.h |  3 ++-
> > >  security/selinux/ss/context.h       | 28 ++++++++++++++++++++++++++++
> > >  security/selinux/ss/ebitmap.c       | 18 ++++++++++++++++++
> > >  security/selinux/ss/ebitmap.h       |  1 +
> > >  security/selinux/ss/mls.c           |  2 ++
> > >  security/selinux/ss/policydb.c      |  5 +++++
> > >  security/selinux/ss/policydb.h      |  1 +
> > >  7 files changed, 57 insertions(+), 1 deletion(-)
> >
> > You incorporated some feedback from the v1 patch, but you ignored
> > some, can you explain why?
> >
>
> I apologize, I missed a couple C++ style comments, I'll fix those and
> resend, was there anything else? I thought I addressed all of the
> technical concerns.

My biggest concern wasn't really the style nits (although please do
fix those), but rather the guts of ebitmap_and() and the use of
ebitmap_get_bit() instead of something a bit more efficient.  Here is
my original comment:

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

> > For reference, here are my comments on your first patch:
> > * https://lore.kernel.org/selinux/CAHC9VhRXyRDjj3KJDHvA4ruJg6H+1kzFPzfA-PLZ-NqBicsLrw@mail.gmail.com/

-- 
paul moore
www.paul-moore.com

^ permalink raw reply	[flat|nested] 8+ messages in thread
* Re: [PATCH v2] default_range glblub implementation
  2019-08-29 16:58     ` Paul Moore
@ 2019-08-29 17:11       ` Joshua Brindle
  2019-08-30 21:38         ` Paul Moore
  0 siblings, 1 reply; 8+ messages in thread
From: Joshua Brindle @ 2019-08-29 17:11 UTC (permalink / raw)
  To: Paul Moore; +Cc: selinux

On Thu, Aug 29, 2019 at 12:59 PM Paul Moore <paul@paul-moore.com> wrote:
>
> On Thu, Aug 29, 2019 at 9:29 AM Joshua Brindle
> <joshua.brindle@crunchydata.com> wrote:
> > On Wed, Aug 28, 2019 at 6:31 PM Paul Moore <paul@paul-moore.com> wrote:
> > > On Wed, Aug 28, 2019 at 4:43 PM 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.
> > > >
> > > > The glb (greatest lower bound) lub (lowest upper bound) of a range is calculated
> > > > as the greater of the low sensitivities and the lower of the high sensitivities
> > > > and the and of each category bitmap.
> > > >
> > > > This can be used by MLS solution developers to compute a context that satisfies,
> > > > for example, the range of a network interface and the range of a user logging in.
> > > >
> > > > Some examples are:
> > > >
> > > > User Permitted Range | Network Device Label | Computed Label
> > > > ---------------------|----------------------|----------------
> > > > S0-S1:c0.c12         | S0                   | S0
> > > > S0-S1:c0.c12         | S0-S1:c0.c1024       | S0-S1:c0.c12
> > > > S0-S4:c0.c512        | S1-S1:c0.c1024       | S1-S1:c0.c512
> > > > S0-S16:c0,c2         | S4-S6:c0.c128        | S4-S6:c0,c2
> > > > S0-S4                | S2-S6                | S2-S4
> > > > S0-S4                | S5-S8                | INVALID
> > > > S5-S8                | S0-S4                | INVALID
> > > > S6:c0,c2-S7:c4,c5    | S0:c2,c4-S6:c5.c100  | S6:c2-S6:c5
> > > >
> > > > Signed-off-by: Joshua Brindle <joshua.brindle@crunchydata.com>
> > > > ---
> > > >  security/selinux/include/security.h |  3 ++-
> > > >  security/selinux/ss/context.h       | 28 ++++++++++++++++++++++++++++
> > > >  security/selinux/ss/ebitmap.c       | 18 ++++++++++++++++++
> > > >  security/selinux/ss/ebitmap.h       |  1 +
> > > >  security/selinux/ss/mls.c           |  2 ++
> > > >  security/selinux/ss/policydb.c      |  5 +++++
> > > >  security/selinux/ss/policydb.h      |  1 +
> > > >  7 files changed, 57 insertions(+), 1 deletion(-)
> > >
> > > You incorporated some feedback from the v1 patch, but you ignored
> > > some, can you explain why?
> > >
> >
> > I apologize, I missed a couple C++ style comments, I'll fix those and
> > resend, was there anything else? I thought I addressed all of the
> > technical concerns.
>
> My biggest concern wasn't really the style nits (although please do
> fix those), but rather the guts of ebitmap_and() and the use of
> ebitmap_get_bit() instead of something a bit more efficient.  Here is
> my original comment:
>
>   "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."
>

I used ebitmap_for_each_positive_bit() for the outer loop which uses
ebitmap_start_positive() and ebitmap_next_positive().

I suppose I could try to track both lists at the same time and AND the
bitmaps when the startbit is the same but I don't expect this to
really be any kind of bottleneck.

> > > For reference, here are my comments on your first patch:
> > > * https://lore.kernel.org/selinux/CAHC9VhRXyRDjj3KJDHvA4ruJg6H+1kzFPzfA-PLZ-NqBicsLrw@mail.gmail.com/
>
> --
> paul moore
> www.paul-moore.com

^ permalink raw reply	[flat|nested] 8+ messages in thread
* Re: [PATCH v2] default_range glblub implementation
  2019-08-28 20:43 [PATCH v2] default_range glblub implementation Joshua Brindle
  2019-08-28 22:31 ` Paul Moore
@ 2019-08-29 20:45 ` Stephen Smalley
  2019-08-29 21:10   ` Joshua Brindle
  1 sibling, 1 reply; 8+ messages in thread
From: Stephen Smalley @ 2019-08-29 20:45 UTC (permalink / raw)
  To: Joshua Brindle, selinux; +Cc: paul, Chad Hanson

On 8/28/19 4:43 PM, Joshua Brindle 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.
> 
> The glb (greatest lower bound) lub (lowest upper bound) of a range is calculated
> as the greater of the low sensitivities and the lower of the high sensitivities
> and the and of each category bitmap.
> 
> This can be used by MLS solution developers to compute a context that satisfies,
> for example, the range of a network interface and the range of a user logging in.
> 
> Some examples are:
> 
> User Permitted Range | Network Device Label | Computed Label
> ---------------------|----------------------|----------------
> S0-S1:c0.c12         | S0                   | S0
> S0-S1:c0.c12         | S0-S1:c0.c1024       | S0-S1:c0.c12
> S0-S4:c0.c512        | S1-S1:c0.c1024       | S1-S1:c0.c512
> S0-S16:c0,c2         | S4-S6:c0.c128        | S4-S6:c0,c2
> S0-S4                | S2-S6                | S2-S4
> S0-S4                | S5-S8                | INVALID
> S5-S8                | S0-S4                | INVALID
> S6:c0,c2-S7:c4,c5    | S0:c2,c4-S6:c5.c100  | S6:c2-S6:c5

I don't quite understand why it is an AND / intersection of the 
categories on both levels.  Normally lub would be an OR / union. 
Possibly it is just a terminology issue.  Is this "glblub" relation 
defined by anyone else, or used in any other MLS system?

Also, I'm left wondering whether certain cases of differing category 
sets ought to yield an INVALID state.  What if the user's permitted 
range was s0:c0,c5-s15:c0,c5 and the network device label was s0:c13? Or 
take your last example - is it really correct to let the user login w/o c0?

> 
> Signed-off-by: Joshua Brindle <joshua.brindle@crunchydata.com>
> ---
>   security/selinux/include/security.h |  3 ++-
>   security/selinux/ss/context.h       | 28 ++++++++++++++++++++++++++++
>   security/selinux/ss/ebitmap.c       | 18 ++++++++++++++++++
>   security/selinux/ss/ebitmap.h       |  1 +
>   security/selinux/ss/mls.c           |  2 ++
>   security/selinux/ss/policydb.c      |  5 +++++
>   security/selinux/ss/policydb.h      |  1 +
>   7 files changed, 57 insertions(+), 1 deletion(-)
> 
> 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..0a0fbcc0d90c 100644
> --- a/security/selinux/ss/context.h
> +++ b/security/selinux/ss/context.h
> @@ -95,6 +95,34 @@ 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)
> +{
> +	struct mls_range *dr = &dst->range, *r1 = &c1->range, *r2 = &c2->range;
> +	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;

This error will get propagate to userspace as an errno? So it should be 
-EINVAL?

> +
> +	// Take the greatest of the low
> +	dr->level[0].sens = max(r1->level[0].sens, r2->level[0].sens);
> +
> +        // Take the least of the high
> +	dr->level[1].sens = min(r1->level[1].sens, r2->level[1].sens);
> +
> +	rc = ebitmap_and(&dr->level[0].cat, &r1->level[0].cat, &r2->level[0].cat);
> +	if (rc)
> +		goto out;
> +
> +	rc = ebitmap_and(&dr->level[1].cat, &r1->level[1].cat, &r2->level[1].cat);
> +	if (rc)
> +		goto out;
> +
> +out:
> +	return rc;
> +}
> +
>   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..c0a9f8ecffc3 100644
> --- a/security/selinux/ss/ebitmap.c
> +++ b/security/selinux/ss/ebitmap.c
> @@ -77,6 +77,24 @@ int ebitmap_cpy(struct ebitmap *dst, struct ebitmap *src)
>   	return 0;
>   }
>   
> +int ebitmap_and(struct ebitmap *dst, struct ebitmap *e1, struct ebitmap *e2)
> +{
> +	struct ebitmap_node *n;
> +	int bit;
> +
> +	ebitmap_init(dst);
> +
> +	ebitmap_for_each_positive_bit(e1, n, bit) {
> +		if (ebitmap_get_bit(e2, bit)) {
> +			int rc = ebitmap_set_bit(dst, bit, 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/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;
>   };
>   
> 


^ permalink raw reply	[flat|nested] 8+ messages in thread
* Re: [PATCH v2] default_range glblub implementation
  2019-08-29 20:45 ` Stephen Smalley
@ 2019-08-29 21:10   ` Joshua Brindle
  0 siblings, 0 replies; 8+ messages in thread
From: Joshua Brindle @ 2019-08-29 21:10 UTC (permalink / raw)
  To: Stephen Smalley; +Cc: selinux, Paul Moore, Chad Hanson

On Thu, Aug 29, 2019 at 4:46 PM Stephen Smalley <sds@tycho.nsa.gov> wrote:
>
> On 8/28/19 4:43 PM, Joshua Brindle 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.
> >
> > The glb (greatest lower bound) lub (lowest upper bound) of a range is calculated
> > as the greater of the low sensitivities and the lower of the high sensitivities
> > and the and of each category bitmap.
> >
> > This can be used by MLS solution developers to compute a context that satisfies,
> > for example, the range of a network interface and the range of a user logging in.
> >
> > Some examples are:
> >
> > User Permitted Range | Network Device Label | Computed Label
> > ---------------------|----------------------|----------------
> > S0-S1:c0.c12         | S0                   | S0
> > S0-S1:c0.c12         | S0-S1:c0.c1024       | S0-S1:c0.c12
> > S0-S4:c0.c512        | S1-S1:c0.c1024       | S1-S1:c0.c512
> > S0-S16:c0,c2         | S4-S6:c0.c128        | S4-S6:c0,c2
> > S0-S4                | S2-S6                | S2-S4
> > S0-S4                | S5-S8                | INVALID
> > S5-S8                | S0-S4                | INVALID
> > S6:c0,c2-S7:c4,c5    | S0:c2,c4-S6:c5.c100  | S6:c2-S6:c5
>
> I don't quite understand why it is an AND / intersection of the
> categories on both levels.  Normally lub would be an OR / union.
> Possibly it is just a terminology issue.  Is this "glblub" relation
> defined by anyone else, or used in any other MLS system?

I talked to the other MLS people about it during LSS. I may be
conflating things by combining the GLB and LUB into a single
transition but conceptually I think it is what we need.

The network interface range will always be what clearance and
categories that network is approved for, the user has their own set of
clearances and categories assigned to them. The idea is that you can
only access something that both the user has clearance to see and the
network is approved for.

>
> Also, I'm left wondering whether certain cases of differing category
> sets ought to yield an INVALID state.  What if the user's permitted
> range was s0:c0,c5-s15:c0,c5 and the network device label was s0:c13? Or
> take your last example - is it really correct to let the user login w/o c0?
>

In this example the user may be read into the c0 and c5 programs, but
the network is only authorized for the c13 program. This would
degenerate into only s0 information is available, which I believe is
correct since the user isn't read into c13 and the network is only
approved for that.

BTW this gets even more complicated for inverse bits, the network
would be connected to/approved for coalition partner A so that bit is
turned off in the inverse bit section so a US person logging in over
that network could only see data releasable to coalition partner A,
even though they are US, which also seems correct since otherwise
there would be potential to leak non-releasable info.

Another MLS person encouraged me to do this in userspace because the
kernel wouldn't know what a "valid" range is for a deployed system.
While mcstransd does some bit math on the ranges to me the kernel owns
this information. The resulting label can be passed onto mcstransd and
if it violates a constraint there we can deny the entire operation.

> >
> > Signed-off-by: Joshua Brindle <joshua.brindle@crunchydata.com>
> > ---
> >   security/selinux/include/security.h |  3 ++-
> >   security/selinux/ss/context.h       | 28 ++++++++++++++++++++++++++++
> >   security/selinux/ss/ebitmap.c       | 18 ++++++++++++++++++
> >   security/selinux/ss/ebitmap.h       |  1 +
> >   security/selinux/ss/mls.c           |  2 ++
> >   security/selinux/ss/policydb.c      |  5 +++++
> >   security/selinux/ss/policydb.h      |  1 +
> >   7 files changed, 57 insertions(+), 1 deletion(-)
> >
> > 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..0a0fbcc0d90c 100644
> > --- a/security/selinux/ss/context.h
> > +++ b/security/selinux/ss/context.h
> > @@ -95,6 +95,34 @@ 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)
> > +{
> > +     struct mls_range *dr = &dst->range, *r1 = &c1->range, *r2 = &c2->range;
> > +     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;
>
> This error will get propagate to userspace as an errno? So it should be
> -EINVAL?
>
> > +
> > +     // Take the greatest of the low
> > +     dr->level[0].sens = max(r1->level[0].sens, r2->level[0].sens);
> > +
> > +        // Take the least of the high
> > +     dr->level[1].sens = min(r1->level[1].sens, r2->level[1].sens);
> > +
> > +     rc = ebitmap_and(&dr->level[0].cat, &r1->level[0].cat, &r2->level[0].cat);
> > +     if (rc)
> > +             goto out;
> > +
> > +     rc = ebitmap_and(&dr->level[1].cat, &r1->level[1].cat, &r2->level[1].cat);
> > +     if (rc)
> > +             goto out;
> > +
> > +out:
> > +     return rc;
> > +}
> > +
> >   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..c0a9f8ecffc3 100644
> > --- a/security/selinux/ss/ebitmap.c
> > +++ b/security/selinux/ss/ebitmap.c
> > @@ -77,6 +77,24 @@ int ebitmap_cpy(struct ebitmap *dst, struct ebitmap *src)
> >       return 0;
> >   }
> >
> > +int ebitmap_and(struct ebitmap *dst, struct ebitmap *e1, struct ebitmap *e2)
> > +{
> > +     struct ebitmap_node *n;
> > +     int bit;
> > +
> > +     ebitmap_init(dst);
> > +
> > +     ebitmap_for_each_positive_bit(e1, n, bit) {
> > +             if (ebitmap_get_bit(e2, bit)) {
> > +                     int rc = ebitmap_set_bit(dst, bit, 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/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;
> >   };
> >
> >
>

^ permalink raw reply	[flat|nested] 8+ messages in thread
* Re: [PATCH v2] default_range glblub implementation
  2019-08-29 17:11       ` Joshua Brindle
@ 2019-08-30 21:38         ` Paul Moore
  0 siblings, 0 replies; 8+ messages in thread
From: Paul Moore @ 2019-08-30 21:38 UTC (permalink / raw)
  To: Joshua Brindle; +Cc: selinux

On Thu, Aug 29, 2019 at 1:11 PM Joshua Brindle
<joshua.brindle@crunchydata.com> wrote:
> On Thu, Aug 29, 2019 at 12:59 PM Paul Moore <paul@paul-moore.com> wrote:
> > On Thu, Aug 29, 2019 at 9:29 AM Joshua Brindle
> > <joshua.brindle@crunchydata.com> wrote:
> > > On Wed, Aug 28, 2019 at 6:31 PM Paul Moore <paul@paul-moore.com> wrote:
> > > > On Wed, Aug 28, 2019 at 4:43 PM 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.
> > > > >
> > > > > The glb (greatest lower bound) lub (lowest upper bound) of a range is calculated
> > > > > as the greater of the low sensitivities and the lower of the high sensitivities
> > > > > and the and of each category bitmap.
> > > > >
> > > > > This can be used by MLS solution developers to compute a context that satisfies,
> > > > > for example, the range of a network interface and the range of a user logging in.
> > > > >
> > > > > Some examples are:
> > > > >
> > > > > User Permitted Range | Network Device Label | Computed Label
> > > > > ---------------------|----------------------|----------------
> > > > > S0-S1:c0.c12         | S0                   | S0
> > > > > S0-S1:c0.c12         | S0-S1:c0.c1024       | S0-S1:c0.c12
> > > > > S0-S4:c0.c512        | S1-S1:c0.c1024       | S1-S1:c0.c512
> > > > > S0-S16:c0,c2         | S4-S6:c0.c128        | S4-S6:c0,c2
> > > > > S0-S4                | S2-S6                | S2-S4
> > > > > S0-S4                | S5-S8                | INVALID
> > > > > S5-S8                | S0-S4                | INVALID
> > > > > S6:c0,c2-S7:c4,c5    | S0:c2,c4-S6:c5.c100  | S6:c2-S6:c5
> > > > >
> > > > > Signed-off-by: Joshua Brindle <joshua.brindle@crunchydata.com>
> > > > > ---
> > > > >  security/selinux/include/security.h |  3 ++-
> > > > >  security/selinux/ss/context.h       | 28 ++++++++++++++++++++++++++++
> > > > >  security/selinux/ss/ebitmap.c       | 18 ++++++++++++++++++
> > > > >  security/selinux/ss/ebitmap.h       |  1 +
> > > > >  security/selinux/ss/mls.c           |  2 ++
> > > > >  security/selinux/ss/policydb.c      |  5 +++++
> > > > >  security/selinux/ss/policydb.h      |  1 +
> > > > >  7 files changed, 57 insertions(+), 1 deletion(-)
> > > >
> > > > You incorporated some feedback from the v1 patch, but you ignored
> > > > some, can you explain why?
> > > >
> > >
> > > I apologize, I missed a couple C++ style comments, I'll fix those and
> > > resend, was there anything else? I thought I addressed all of the
> > > technical concerns.
> >
> > My biggest concern wasn't really the style nits (although please do
> > fix those), but rather the guts of ebitmap_and() and the use of
> > ebitmap_get_bit() instead of something a bit more efficient.  Here is
> > my original comment:
> >
> >   "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."
> >
>
> I used ebitmap_for_each_positive_bit() for the outer loop which uses
> ebitmap_start_positive() and ebitmap_next_positive().

Sorry, my mistake.  Your implementation wasn't quite what I was
expecting and on quick glance it looked similar to your previous
effort so I assumed it hadn't changed.  The v2 variant should be fine.

> I suppose I could try to track both lists at the same time and AND the
> bitmaps when the startbit is the same but I don't expect this to
> really be any kind of bottleneck.

-- 
paul moore
www.paul-moore.com

^ permalink raw reply	[flat|nested] 8+ messages in thread
end of thread, other threads:[~2019-08-30 21:38 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-28 20:43 [PATCH v2] default_range glblub implementation Joshua Brindle
2019-08-28 22:31 ` Paul Moore
2019-08-29 13:29   ` Joshua Brindle
2019-08-29 16:58     ` Paul Moore
2019-08-29 17:11       ` Joshua Brindle
2019-08-30 21:38         ` Paul Moore
2019-08-29 20:45 ` Stephen Smalley
2019-08-29 21:10   ` Joshua Brindle

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).