All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC PATCH 0/2] Series short description
@ 2007-09-25 20:48 Paul Moore
  2007-09-25 20:48 ` [RFC PATCH 1/2] [SELINUX] Add a functionality version number Paul Moore
                   ` (2 more replies)
  0 siblings, 3 replies; 28+ messages in thread
From: Paul Moore @ 2007-09-25 20:48 UTC (permalink / raw)
  To: selinux

This patchset has two patches in it which I would like to get some feedback
on.  The first patch adds a functionality/compatability version number to the
policy so that we can add new functionality to the kernel which would lie
dormant until the correct policy was loaded - it is not intended to replace
Eric's undefined classes/perms work but to compliment it.  The second patch
is the first step towards a single set of access checks for the different
peer labeling mechanisms, NetLabel and labeled IPsec, by providing a single
function to determine the peer label of an incoming packet.  The ideas behind
both patches have been discussed in the past, but I'd like to see if there
are any new concerns now that people can see what an implementation would
look like.  I'm particularly interested in people's take on the policy
changes.  The approach presented in this patch seems to be sane and low risk
to me but I'm hoping some of you with more experience tinkering with the
policy code could take a look and comment.

This is an RFC quality patchset which means it does boot and passes a few
simple tests but will most likely shave your cat if your turn your back to it
for more than a few minutes.

--
paul moore
linux security @ hp


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [RFC PATCH 1/2] [SELINUX] Add a functionality version number
  2007-09-25 20:48 [RFC PATCH 0/2] Series short description Paul Moore
@ 2007-09-25 20:48 ` Paul Moore
  2007-09-25 21:12   ` Eric Paris
  2007-09-25 20:48 ` [RFC PATCH 2/2] [SELINUX] Better integration between peer labeling subsystems Paul Moore
  2007-09-25 22:28 ` [RFC PATCH 0/2] Series short description James Morris
  2 siblings, 1 reply; 28+ messages in thread
From: Paul Moore @ 2007-09-25 20:48 UTC (permalink / raw)
  To: selinux

Add a functionality/compatability version number to the policy so that the
kernel can query the policy to see what level of functionality it supports.
---

 security/selinux/include/security.h |   21 +++++++++++++++++++++
 security/selinux/selinuxfs.c        |   18 ++++++++++++++++++
 security/selinux/ss/policydb.c      |   20 ++++++++++++++++++++
 security/selinux/ss/policydb.h      |    6 +++++-
 security/selinux/ss/services.c      |    2 ++
 5 files changed, 66 insertions(+), 1 deletions(-)

diff --git a/security/selinux/include/security.h b/security/selinux/include/security.h
index 83bdd4d..cd45cf0 100644
--- a/security/selinux/include/security.h
+++ b/security/selinux/include/security.h
@@ -39,6 +39,27 @@ struct netlbl_lsm_secattr;
 extern int selinux_enabled;
 extern int selinux_mls_enabled;
 
+/* Supported functionality/compatibility version */
+extern unsigned int policydb_loaded_compatver;
+#define POLICYDB_COMPATVER_BASE		0
+
+/* Range of supported functionality/compatibility we understand */
+#define POLICYDB_COMPATVER_MIN		POLICYDB_COMPATVER_BASE
+#define POLICYDB_COMPATVER_MAX		POLICYDB_COMPATVER_BASE
+
+/**
+ * security_compat_supported - Check if policy supports certain functionality
+ * @requested: requested functionality (see #define COMPATVERS_*)
+ *
+ * Description:
+ * Query the currently loaded policy, by way of the selinux_compat_vers
+ * variable to see if the requested functionality is supported.
+ *
+ */
+static inline int security_compat_supported(unsigned int requested) {
+	return (policydb_loaded_compatver >= requested);
+}
+
 int security_load_policy(void * data, size_t len);
 
 #define SEL_VEC_MAX 32
diff --git a/security/selinux/selinuxfs.c b/security/selinux/selinuxfs.c
index c9e92da..e5927b9 100644
--- a/security/selinux/selinuxfs.c
+++ b/security/selinux/selinuxfs.c
@@ -103,6 +103,7 @@ enum sel_inos {
 	SEL_MEMBER,	/* compute polyinstantiation membership decision */
 	SEL_CHECKREQPROT, /* check requested protection, not kernel-applied one */
 	SEL_COMPAT_NET,	/* whether to use old compat network packet controls */
+	SEL_COMPAT_VERS, /* functionality/compatability version */
 	SEL_INO_NEXT,	/* The next inode number to use */
 };
 
@@ -452,6 +453,22 @@ static const struct file_operations sel_compat_net_ops = {
 	.write		= sel_write_compat_net,
 };
 
+static ssize_t sel_read_compat_vers(struct file *filp,
+				      char __user *buf,
+				      size_t count,
+				      loff_t *ppos)
+{
+	char tmpbuf[TMPBUFLEN];
+	ssize_t length;
+
+	length = scnprintf(tmpbuf, TMPBUFLEN, "%d", policydb_loaded_compatver);
+	return simple_read_from_buffer(buf, count, ppos, tmpbuf, length);
+}
+
+static const struct file_operations sel_compat_vers_ops = {
+	.read		= sel_read_compat_vers,
+};
+
 /*
  * Remaining nodes use transaction based IO methods like nfsd/nfsctl.c
  */
@@ -1575,6 +1592,7 @@ static int sel_fill_super(struct super_block * sb, void * data, int silent)
 		[SEL_MEMBER] = {"member", &transaction_ops, S_IRUGO|S_IWUGO},
 		[SEL_CHECKREQPROT] = {"checkreqprot", &sel_checkreqprot_ops, S_IRUGO|S_IWUSR},
 		[SEL_COMPAT_NET] = {"compat_net", &sel_compat_net_ops, S_IRUGO|S_IWUSR},
+		[SEL_COMPAT_VERS] = {"compat_version", &sel_compat_vers_ops, S_IRUGO},
 		/* last one */ {""}
 	};
 	ret = simple_fill_super(sb, SELINUX_MAGIC, selinux_files);
diff --git a/security/selinux/ss/policydb.c b/security/selinux/ss/policydb.c
index f05f97a..6c05516 100644
--- a/security/selinux/ss/policydb.c
+++ b/security/selinux/ss/policydb.c
@@ -47,6 +47,7 @@ static char *symtab_name[SYM_NUM] = {
 #endif
 
 int selinux_mls_enabled = 0;
+unsigned int selinux_compat_version = POLICYDB_COMPATVER_BASE;
 
 static unsigned int symtab_sizes[SYM_NUM] = {
 	2,
@@ -1531,6 +1532,25 @@ int policydb_read(struct policydb *p, void *fp)
 		}
 	}
 
+	if ((le32_to_cpu(buf[1]) & POLICYDB_CONFIG_COMPATVER)) {
+		__le32 compat_buf;
+		rc = next_entry(&compat_buf, fp, sizeof(compat_buf));
+		if (rc < 0)
+			goto bad;
+
+		p->compat_version = le32_to_cpu(compat_buf);
+		if (p->compat_version < POLICYDB_COMPATVER_MIN ||
+		    p->compat_version > POLICYDB_COMPATVER_MAX) {
+			printk(KERN_ERR
+			       "security:  policydb compat_version %d does not"
+			       " match my version range %d-%d\n",
+			       p->compat_version,
+			       POLICYDB_COMPATVER_MIN,
+			       POLICYDB_COMPATVER_MAX);
+			goto bad;
+		}
+	}
+
 	info = policydb_lookup_compat(p->policyvers);
 	if (!info) {
 		printk(KERN_ERR "security:  unable to find policy compat info "
diff --git a/security/selinux/ss/policydb.h b/security/selinux/ss/policydb.h
index 8319d5f..59177d3 100644
--- a/security/selinux/ss/policydb.h
+++ b/security/selinux/ss/policydb.h
@@ -242,6 +242,9 @@ struct policydb {
 	struct ebitmap *type_attr_map;
 
 	unsigned int policyvers;
+
+	/* functionality/compatability version */
+	unsigned int compat_version;
 };
 
 extern void policydb_destroy(struct policydb *p);
@@ -251,7 +254,8 @@ extern int policydb_read(struct policydb *p, void *fp);
 
 #define PERM_SYMTAB_SIZE 32
 
-#define POLICYDB_CONFIG_MLS    1
+#define POLICYDB_CONFIG_MLS		0x00000001
+#define POLICYDB_CONFIG_COMPATVER	0x00000002
 
 #define OBJECT_R "object_r"
 #define OBJECT_R_VAL 1
diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c
index 6100fc0..ef557b6 100644
--- a/security/selinux/ss/services.c
+++ b/security/selinux/ss/services.c
@@ -58,6 +58,7 @@
 
 extern void selnl_notify_policyload(u32 seqno);
 unsigned int policydb_loaded_version;
+unsigned int policydb_loaded_compatver;
 
 /*
  * This is declared in avc.c
@@ -1304,6 +1305,7 @@ int security_load_policy(void *data, size_t len)
 			return -EINVAL;
 		}
 		policydb_loaded_version = policydb.policyvers;
+		policydb_loaded_compatver = policydb.compat_version;
 		ss_initialized = 1;
 		seqno = ++latest_granting;
 		LOAD_UNLOCK;


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [RFC PATCH 2/2] [SELINUX] Better integration between peer labeling subsystems
  2007-09-25 20:48 [RFC PATCH 0/2] Series short description Paul Moore
  2007-09-25 20:48 ` [RFC PATCH 1/2] [SELINUX] Add a functionality version number Paul Moore
@ 2007-09-25 20:48 ` Paul Moore
  2007-09-25 21:37   ` Eric Paris
                     ` (2 more replies)
  2007-09-25 22:28 ` [RFC PATCH 0/2] Series short description James Morris
  2 siblings, 3 replies; 28+ messages in thread
From: Paul Moore @ 2007-09-25 20:48 UTC (permalink / raw)
  To: selinux

Rename the existing selinux_skb_extlbl_sid() function to
selinux_skb_peerlbl_sid() and modify it's behavior such that it now reconciles
multiple peer/external labels and if reconciliation is not possible it returns
an error to the caller.
---

 security/selinux/hooks.c            |   70 ++++++++++++++++++++++++++---------
 security/selinux/include/security.h |    2 +
 security/selinux/ss/services.c      |   42 +++++++++++++++++++++
 3 files changed, 95 insertions(+), 19 deletions(-)

diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 0753b20..a97d1b1 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -3132,32 +3132,59 @@ static int selinux_parse_skb(struct sk_buff *skb, struct avc_audit_data *ad,
 }
 
 /**
- * selinux_skb_extlbl_sid - Determine the external label of a packet
+ * selinux_skb_peerlbl_sid - Determine the peer label of a packet
  * @skb: the packet
- * @sid: the packet's SID
+ * @sid: the packet's peer label SID
  *
  * Description:
- * Check the various different forms of external packet labeling and determine
- * the external SID for the packet.  If only one form of external labeling is
- * present then it is used, if both labeled IPsec and NetLabel labels are
- * present then the SELinux type information is taken from the labeled IPsec
- * SA and the MLS sensitivity label information is taken from the NetLabel
- * security attributes.  This bit of "magic" is done in the call to
- * selinux_netlbl_skbuff_getsid().
+ * Check the various different forms of network peer labeling and determine
+ * the peer label/SID for the packet.  If only one form of peer labeling is
+ * present then it is returned in @sid.  If multiple forms of peer labeling
+ * are present the the different peer labels are checked for consistency
+ * (i.e. all the different peer labels have the same value) before the peer
+ * label is returned to the caller in @sid.  The function returns negative
+ * values when there are multiple, inconsistent peer labels present and zero
+ * otherwise.  A table summarizing the different return values is below:
+ *
+ *                                 | function return |      @sid
+ *   ------------------------------+-----------------+-----------------
+ *   no peer labels                |        0        |    SECSID_NULL
+ *   single peer label             |        0        |    peer_label
+ *   multiple, consistent labels   |        0        |    peer_label
+ *   multiple, inconsistent labels |     -EACCES     |    SECSID_NULL
  *
  */
-static void selinux_skb_extlbl_sid(struct sk_buff *skb, u32 *sid)
+static int selinux_skb_peerlbl_sid(struct sk_buff *skb, u32 *sid)
 {
 	u32 xfrm_sid;
 	u32 nlbl_sid;
 
 	selinux_skb_xfrm_sid(skb, &xfrm_sid);
-	if (selinux_netlbl_skbuff_getsid(skb,
-					 (xfrm_sid == SECSID_NULL ?
-					  SECINITSID_NETMSG : xfrm_sid),
-					 &nlbl_sid) != 0)
-		nlbl_sid = SECSID_NULL;
-	*sid = (nlbl_sid == SECSID_NULL ? xfrm_sid : nlbl_sid);
+	selinux_netlbl_skbuff_getsid(skb, SECINITSID_NETMSG, &nlbl_sid);
+
+	if (nlbl_sid != SECSID_NULL && xfrm_sid != SECSID_NULL) {
+		if (nlbl_sid != xfrm_sid &&
+		    /* XXX - not sure if we should just compare the low end of
+		     * the range or the whole range?  probably safest to
+		     * compare the entire range ... */
+		    security_sid_mls_cmp(nlbl_sid, xfrm_sid) != 0) {
+			*sid = SECSID_NULL;
+			return -EACCES;
+		} else
+			/* at present NetLabel SIDs/labels really only carry
+			 * MLS information so if the MLS portion of the
+			 * NetLabel SID matches the MLS portion of the labeled
+			 * XFRM SID/label then pass along the XFRM SID as it
+			 * has the most peer label information */
+			*sid = xfrm_sid;
+	} else if (nlbl_sid != SECSID_NULL)
+		*sid = nlbl_sid;
+	else if (xfrm_sid != SECSID_NULL)
+		*sid = xfrm_sid;
+	else
+		*sid = SECSID_NULL;
+
+	return 0;
 }
 
 /* socket security operations */
@@ -3640,6 +3667,9 @@ static int selinux_socket_sock_rcv_skb(struct sock *sk, struct sk_buff *skb)
 	if (err)
 		goto out;
 
+	/* XXX - make use of selinux_skb_peerlbl_sid() here but only once we
+	 *       have the new peer object class in place */
+
 	err = selinux_netlbl_sock_rcv_skb(sksec, skb, &ad);
 	if (err)
 		goto out;
@@ -3701,7 +3731,7 @@ static int selinux_socket_getpeersec_dgram(struct socket *sock, struct sk_buff *
 	if (sock && sock->sk->sk_family == PF_UNIX)
 		selinux_get_inode_sid(SOCK_INODE(sock), &peer_secid);
 	else if (skb)
-		selinux_skb_extlbl_sid(skb, &peer_secid);
+		selinux_skb_peerlbl_sid(skb, &peer_secid);
 
 	if (peer_secid == SECSID_NULL)
 		err = -EINVAL;
@@ -3762,7 +3792,9 @@ static int selinux_inet_conn_request(struct sock *sk, struct sk_buff *skb,
 	u32 newsid;
 	u32 peersid;
 
-	selinux_skb_extlbl_sid(skb, &peersid);
+	err = selinux_skb_peerlbl_sid(skb, &peersid);
+	if (err)
+		return err;
 	if (peersid == SECSID_NULL) {
 		req->secid = sksec->sid;
 		req->peer_secid = SECSID_NULL;
@@ -3800,7 +3832,7 @@ static void selinux_inet_conn_established(struct sock *sk,
 {
 	struct sk_security_struct *sksec = sk->sk_security;
 
-	selinux_skb_extlbl_sid(skb, &sksec->peer_sid);
+	selinux_skb_peerlbl_sid(skb, &sksec->peer_sid);
 }
 
 static void selinux_req_classify_flow(const struct request_sock *req,
diff --git a/security/selinux/include/security.h b/security/selinux/include/security.h
index cd45cf0..4cbb6f7 100644
--- a/security/selinux/include/security.h
+++ b/security/selinux/include/security.h
@@ -109,6 +109,8 @@ int security_validate_transition(u32 oldsid, u32 newsid, u32 tasksid,
 
 int security_sid_mls_copy(u32 sid, u32 mls_sid, u32 *new_sid);
 
+int security_sid_mls_cmp(u32 sid_a, u32 sid_b);
+
 int security_get_classes(char ***classes, int *nclasses);
 int security_get_permissions(char *class, char ***perms, int *nperms);
 
diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c
index ef557b6..ea78f75 100644
--- a/security/selinux/ss/services.c
+++ b/security/selinux/ss/services.c
@@ -2009,6 +2009,48 @@ out:
 	return rc;
 }
 
+/**
+ * security_sid_mls_cmp - Compare the MLS portion of two SIDs
+ * @sid_a: SID to compare
+ * @sid_b: SID to compare
+ *
+ * Description:
+ * Compare the MLS portions of two SIDs and return zero if they are the same,
+ * otherwise return a non-zero value.
+ *
+ */
+int security_sid_mls_cmp(u32 sid_a, u32 sid_b)
+{
+	int rc;
+	struct context *ctx_a;
+	struct context *ctx_b;
+
+	if (!ss_initialized || !selinux_mls_enabled)
+		return 0;
+
+	POLICY_RDLOCK;
+
+	ctx_a = sidtab_search(&sidtab, sid_a);
+	if (!ctx_a) {
+		printk(KERN_ERR
+		       "security_sid_mls_cmp:  unrecognized SID %d\n", sid_a);
+		rc = -EINVAL;
+		goto out;
+	}
+	ctx_b = sidtab_search(&sidtab, sid_b);
+	if (!ctx_b) {
+		printk(KERN_ERR
+		       "security_sid_mls_cmp:  unrecognized SID %d\n", sid_b);
+		rc = -EINVAL;
+		goto out;
+	}
+	rc = mls_context_cmp(ctx_a, ctx_b);
+
+out:
+	POLICY_RDUNLOCK;
+	return rc;
+}
+
 static int get_classes_callback(void *k, void *d, void *args)
 {
 	struct class_datum *datum = d;


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* Re: [RFC PATCH 1/2] [SELINUX] Add a functionality version number
  2007-09-25 20:48 ` [RFC PATCH 1/2] [SELINUX] Add a functionality version number Paul Moore
@ 2007-09-25 21:12   ` Eric Paris
  2007-09-25 21:16     ` Paul Moore
  0 siblings, 1 reply; 28+ messages in thread
From: Eric Paris @ 2007-09-25 21:12 UTC (permalink / raw)
  To: Paul Moore; +Cc: selinux

On 9/25/07, Paul Moore <paul.moore@hp.com> wrote:
> Add a functionality/compatability version number to the policy so that the
> kernel can query the policy to see what level of functionality it supports.
> ---
>
>  security/selinux/include/security.h |   21 +++++++++++++++++++++
>  security/selinux/selinuxfs.c        |   18 ++++++++++++++++++
>  security/selinux/ss/policydb.c      |   20 ++++++++++++++++++++
>  security/selinux/ss/policydb.h      |    6 +++++-
>  security/selinux/ss/services.c      |    2 ++
>  5 files changed, 66 insertions(+), 1 deletions(-)

Why is this better/different than just using the policy version we
already have in a slightly different way?  We are at version what?
.21?  We already have some compat stuff in context_struct_compute_av()
based on this....

-Eric

--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [RFC PATCH 1/2] [SELINUX] Add a functionality version number
  2007-09-25 21:12   ` Eric Paris
@ 2007-09-25 21:16     ` Paul Moore
  0 siblings, 0 replies; 28+ messages in thread
From: Paul Moore @ 2007-09-25 21:16 UTC (permalink / raw)
  To: Eric Paris; +Cc: selinux

On Tuesday 25 September 2007 5:12:47 pm Eric Paris wrote:
> On 9/25/07, Paul Moore <paul.moore@hp.com> wrote:
> > Add a functionality/compatability version number to the policy so that
> > the kernel can query the policy to see what level of functionality it
> > supports. ---
> >
> >  security/selinux/include/security.h |   21 +++++++++++++++++++++
> >  security/selinux/selinuxfs.c        |   18 ++++++++++++++++++
> >  security/selinux/ss/policydb.c      |   20 ++++++++++++++++++++
> >  security/selinux/ss/policydb.h      |    6 +++++-
> >  security/selinux/ss/services.c      |    2 ++
> >  5 files changed, 66 insertions(+), 1 deletions(-)
>
> Why is this better/different than just using the policy version we
> already have in a slightly different way?

It's different but that's about it ... I did it this way because thought 
people would object to using the policy version for such a purpose.  I'm more 
than happy to use the existing policy version if that is okay.

-- 
paul moore
linux security @ hp

--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [RFC PATCH 2/2] [SELINUX] Better integration between peer labeling subsystems
  2007-09-25 20:48 ` [RFC PATCH 2/2] [SELINUX] Better integration between peer labeling subsystems Paul Moore
@ 2007-09-25 21:37   ` Eric Paris
  2007-09-25 22:01     ` Paul Moore
  2007-09-25 22:38   ` James Morris
  2007-09-26 12:41   ` Stephen Smalley
  2 siblings, 1 reply; 28+ messages in thread
From: Eric Paris @ 2007-09-25 21:37 UTC (permalink / raw)
  To: Paul Moore; +Cc: selinux

On 9/25/07, Paul Moore <paul.moore@hp.com> wrote:
> Rename the existing selinux_skb_extlbl_sid() function to
> selinux_skb_peerlbl_sid() and modify it's behavior such that it now reconciles
> multiple peer/external labels and if reconciliation is not possible it returns
> an error to the caller.

> -static void selinux_skb_extlbl_sid(struct sk_buff *skb, u32 *sid)
> +static int selinux_skb_peerlbl_sid(struct sk_buff *skb, u32 *sid)
>  {
>         u32 xfrm_sid;
>         u32 nlbl_sid;
>
>         selinux_skb_xfrm_sid(skb, &xfrm_sid);
> -       if (selinux_netlbl_skbuff_getsid(skb,
> -                                        (xfrm_sid == SECSID_NULL ?
> -                                         SECINITSID_NETMSG : xfrm_sid),
> -                                        &nlbl_sid) != 0)
> -               nlbl_sid = SECSID_NULL;
> -       *sid = (nlbl_sid == SECSID_NULL ? xfrm_sid : nlbl_sid);
> +       selinux_netlbl_skbuff_getsid(skb, SECINITSID_NETMSG, &nlbl_sid);
> +
> +       if (nlbl_sid != SECSID_NULL && xfrm_sid != SECSID_NULL) {
> +               if (nlbl_sid != xfrm_sid &&

for now nlbl_sid will NEVER == xfrm_sid since netlbl_sid cannot be of
an interesting type, correct?  So this code works.  If the mls portion
is wrong bomb, if the mls portion is right allow.  Since that's all we
are testing lets get that misleading nlbl_sid != xfrm_sid out of there
they can never be the same and it implies we actually check something
other than the mls portion.

assuming one day it does carry interesting non-mls information do we
care to fix this to work today?

Your code:
xfrm_sid = type1_t:s0
netlbl_sid = type2_t:s0

xfrm_sid != netlbl_sid but mls_cmp will be 0, so our result is going
to be type1_t:s0?   Or do we really want to do this up front and say
(horrible psudo codish thing to follow, don't throw up or gouge your
eyes out)

if (xfrm_sid == netlbl_sid)
   return xfrm_sid
else if (xfrm_sid != netlbl_sid) && (netlbl_sid.type == UNSET) && (mls_cmp == 0)
   return xfrm_sid
else
   blow up because it isn't valid.

> +                   /* XXX - not sure if we should just compare the low end of
> +                    * the range or the whole range?  probably safest to
> +                    * compare the entire range ... */
> +                   security_sid_mls_cmp(nlbl_sid, xfrm_sid) != 0) {
> +                       *sid = SECSID_NULL;
> +                       return -EACCES;
> +               } else
> +                       /* at present NetLabel SIDs/labels really only carry
> +                        * MLS information so if the MLS portion of the
> +                        * NetLabel SID matches the MLS portion of the labeled
> +                        * XFRM SID/label then pass along the XFRM SID as it
> +                        * has the most peer label information */
> +                       *sid = xfrm_sid;
> +       } else if (nlbl_sid != SECSID_NULL)
> +               *sid = nlbl_sid;
> +       else if (xfrm_sid != SECSID_NULL)
> +               *sid = xfrm_sid;
> +       else
> +               *sid = SECSID_NULL;
> +
> +       return 0;
>  }
>
>  /* socket security operations */

--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [RFC PATCH 2/2] [SELINUX] Better integration between peer labeling subsystems
  2007-09-25 21:37   ` Eric Paris
@ 2007-09-25 22:01     ` Paul Moore
  0 siblings, 0 replies; 28+ messages in thread
From: Paul Moore @ 2007-09-25 22:01 UTC (permalink / raw)
  To: Eric Paris; +Cc: selinux

On Tuesday 25 September 2007 5:37:53 pm Eric Paris wrote:
> On 9/25/07, Paul Moore <paul.moore@hp.com> wrote:
> > -static void selinux_skb_extlbl_sid(struct sk_buff *skb, u32 *sid)
> > +static int selinux_skb_peerlbl_sid(struct sk_buff *skb, u32 *sid)
> >  {
> >         u32 xfrm_sid;
> >         u32 nlbl_sid;
> >
> >         selinux_skb_xfrm_sid(skb, &xfrm_sid);
> > -       if (selinux_netlbl_skbuff_getsid(skb,
> > -                                        (xfrm_sid == SECSID_NULL ?
> > -                                         SECINITSID_NETMSG : xfrm_sid),
> > -                                        &nlbl_sid) != 0)
> > -               nlbl_sid = SECSID_NULL;
> > -       *sid = (nlbl_sid == SECSID_NULL ? xfrm_sid : nlbl_sid);
> > +       selinux_netlbl_skbuff_getsid(skb, SECINITSID_NETMSG, &nlbl_sid);
> > +
> > +       if (nlbl_sid != SECSID_NULL && xfrm_sid != SECSID_NULL) {
> > +               if (nlbl_sid != xfrm_sid &&
>
> for now nlbl_sid will NEVER == xfrm_sid since netlbl_sid cannot be of
> an interesting type, correct?  So this code works.  If the mls portion
> is wrong bomb, if the mls portion is right allow.  Since that's all we
> are testing lets get that misleading nlbl_sid != xfrm_sid out of there
> they can never be the same and it implies we actually check something
> other than the mls portion.

[NOTE: pseudo code removed to protect the eyesight of anyone who happens to 
wander onto the mailing list]

Presently you are correct that is the case, I'll yank it out and we can add it 
back in later when we need it.  How about this?

static int selinux_skb_peerlbl_sid(struct sk_buff *skb, u32 *sid)
{
	u32 xfrm_sid;
	u32 nlbl_sid;

	selinux_skb_xfrm_sid(skb, &xfrm_sid);
	selinux_netlbl_skbuff_getsid(skb, SECINITSID_NETMSG, &nlbl_sid);

	if (nlbl_sid != SECSID_NULL && xfrm_sid != SECSID_NULL)
		if (security_sid_mls_cmp(nlbl_sid, xfrm_sid) != 0) {
			*sid = SECSID_NULL;
			return -EACCES;
		} else
			/* at present NetLabel SIDs/labels really only carry
			 * MLS information so if the MLS portion of the
			 * NetLabel SID matches the MLS portion of the labeled
			 * XFRM SID/label then pass along the XFRM SID as it
			 * has the most peer label information */
			*sid = xfrm_sid;
	else if (nlbl_sid != SECSID_NULL)
		*sid = nlbl_sid;
	else if (xfrm_sid != SECSID_NULL)
		*sid = xfrm_sid;
	else
		*sid = SECSID_NULL;

	return 0;
}

--
paul moore
linux security @ hp

--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [RFC PATCH 0/2] Series short description
  2007-09-25 20:48 [RFC PATCH 0/2] Series short description Paul Moore
  2007-09-25 20:48 ` [RFC PATCH 1/2] [SELINUX] Add a functionality version number Paul Moore
  2007-09-25 20:48 ` [RFC PATCH 2/2] [SELINUX] Better integration between peer labeling subsystems Paul Moore
@ 2007-09-25 22:28 ` James Morris
  2007-09-25 22:38   ` Paul Moore
  2 siblings, 1 reply; 28+ messages in thread
From: James Morris @ 2007-09-25 22:28 UTC (permalink / raw)
  To: Paul Moore; +Cc: selinux

On Tue, 25 Sep 2007, Paul Moore wrote:

> This patchset has two patches in it which I would like to get some feedback
> on.  The first patch adds a functionality/compatability version number to the
> policy so that we can add new functionality to the kernel which would lie
> dormant until the correct policy was loaded - it is not intended to replace
> Eric's undefined classes/perms work but to compliment it.

Can you explain the expected usage scenarios for this?

We already have versioning of policy, and will have the ability to reject, 
ignore or enforce unknown elements, so why do we need this and how would 
it typically be used?

Should we instead start thinking about versioning each permission & class?  
i.e. create a fully versioned interface


-- 
James Morris
<jmorris@namei.org>

--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [RFC PATCH 2/2] [SELINUX] Better integration between peer labeling subsystems
  2007-09-25 20:48 ` [RFC PATCH 2/2] [SELINUX] Better integration between peer labeling subsystems Paul Moore
  2007-09-25 21:37   ` Eric Paris
@ 2007-09-25 22:38   ` James Morris
  2007-09-25 22:48     ` Paul Moore
  2007-09-26 12:41   ` Stephen Smalley
  2 siblings, 1 reply; 28+ messages in thread
From: James Morris @ 2007-09-25 22:38 UTC (permalink / raw)
  To: Paul Moore; +Cc: selinux

On Tue, 25 Sep 2007, Paul Moore wrote:

> +int security_sid_mls_cmp(u32 sid_a, u32 sid_b)

This looks heavyweight on TCP connection establishment (MCS is enabled in 
Fedora/RHEL).

Perhaps see it can be noticed with lmbench and apachbench (with high rates 
of connection & small files) ?


> +{
> +	int rc;
> +	struct context *ctx_a;
> +	struct context *ctx_b;
> +
> +	if (!ss_initialized || !selinux_mls_enabled)
> +		return 0;
> +
> +	POLICY_RDLOCK;
> +
> +	ctx_a = sidtab_search(&sidtab, sid_a);
> +	if (!ctx_a) {
> +		printk(KERN_ERR
> +		       "security_sid_mls_cmp:  unrecognized SID %d\n", sid_a);
> +		rc = -EINVAL;
> +		goto out;
> +	}
> +	ctx_b = sidtab_search(&sidtab, sid_b);
> +	if (!ctx_b) {
> +		printk(KERN_ERR
> +		       "security_sid_mls_cmp:  unrecognized SID %d\n", sid_b);
> +		rc = -EINVAL;
> +		goto out;
> +	}
> +	rc = mls_context_cmp(ctx_a, ctx_b);
> +
> +out:
> +	POLICY_RDUNLOCK;
> +	return rc;
> +}
> +
>  static int get_classes_callback(void *k, void *d, void *args)
>  {
>  	struct class_datum *datum = d;
> 
> 
> --
> This message was distributed to subscribers of the selinux mailing list.
> If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
> the words "unsubscribe selinux" without quotes as the message.
> 

-- 
James Morris
<jmorris@namei.org>

--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [RFC PATCH 0/2] Series short description
  2007-09-25 22:28 ` [RFC PATCH 0/2] Series short description James Morris
@ 2007-09-25 22:38   ` Paul Moore
  2007-09-26  2:19     ` Joshua Brindle
  0 siblings, 1 reply; 28+ messages in thread
From: Paul Moore @ 2007-09-25 22:38 UTC (permalink / raw)
  To: James Morris; +Cc: selinux

On Tuesday 25 September 2007 6:28:48 pm James Morris wrote:
> On Tue, 25 Sep 2007, Paul Moore wrote:
> > This patchset has two patches in it which I would like to get some
> > feedback on.  The first patch adds a functionality/compatability version
> > number to the policy so that we can add new functionality to the kernel
> > which would lie dormant until the correct policy was loaded - it is not
> > intended to replace Eric's undefined classes/perms work but to compliment
> > it.
>
> Can you explain the expected usage scenarios for this?

The one example that springs immediately to mind is the two separate checks 
for peer labels (one for NetLabel, one for labeled IPsec) in 
selinux_sock_rcv_skb().  As discussed on the list, we would like to move to a 
single check for the peer label.  Unfortunately, this runs into problems with 
backwards compatibility with people using old policy on new kernels.  The 
idea here was that the kernel would have code for both types of access 
control and depending on the policy loaded it would select which type of 
access control to use.

> We already have versioning of policy, and will have the ability to reject,
> ignore or enforce unknown elements, so why do we need this and how would
> it typically be used?

Eric suggested using the policy version to achieve the same thing; if no one 
objects to using the policy version number for this purpose I'm more than 
happy to use it.

> Should we instead start thinking about versioning each permission & class?
> i.e. create a fully versioned interface

This question nags at me, but I keep ignoring it because I fear that a fully 
versioned avc interface would be overkill in the majority of cases and only 
serve to slow down the access checks.

-- 
paul moore
linux security @ hp

--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [RFC PATCH 2/2] [SELINUX] Better integration between peer labeling subsystems
  2007-09-25 22:38   ` James Morris
@ 2007-09-25 22:48     ` Paul Moore
  0 siblings, 0 replies; 28+ messages in thread
From: Paul Moore @ 2007-09-25 22:48 UTC (permalink / raw)
  To: James Morris; +Cc: selinux

On Tuesday 25 September 2007 6:38:37 pm James Morris wrote:
> On Tue, 25 Sep 2007, Paul Moore wrote:
> > +int security_sid_mls_cmp(u32 sid_a, u32 sid_b)
>
> This looks heavyweight on TCP connection establishment (MCS is enabled in
> Fedora/RHEL).

Agreed.  That is why it is only executed when both NetLabel and labeled IPsec 
peer labels are present on the same packet/connection.  A situation which I 
expect to be extremely unlikely for a variety of reasons so I don't think 
this will be a problem in the general case.

In the case where we have two types of peer labels on a packet, the TCS folks 
expressed a concern (which makes sense to me) that we could run into a 
problem where the two peer labels might not match.  This is an attempt to 
address that problem, I'm always open to suggestions for better ways to solve 
the problem.

-- 
paul moore
linux security @ hp

--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [RFC PATCH 0/2] Series short description
  2007-09-25 22:38   ` Paul Moore
@ 2007-09-26  2:19     ` Joshua Brindle
  2007-09-26  3:12       ` Paul Moore
  0 siblings, 1 reply; 28+ messages in thread
From: Joshua Brindle @ 2007-09-26  2:19 UTC (permalink / raw)
  To: Paul Moore; +Cc: James Morris, selinux

Paul Moore wrote:
> On Tuesday 25 September 2007 6:28:48 pm James Morris wrote:
>   
>> On Tue, 25 Sep 2007, Paul Moore wrote:
>>     
>>> This patchset has two patches in it which I would like to get some
>>> feedback on.  The first patch adds a functionality/compatability version
>>> number to the policy so that we can add new functionality to the kernel
>>> which would lie dormant until the correct policy was loaded - it is not
>>> intended to replace Eric's undefined classes/perms work but to compliment
>>> it.
>>>       
>> Can you explain the expected usage scenarios for this?
>>     
>
> The one example that springs immediately to mind is the two separate checks 
> for peer labels (one for NetLabel, one for labeled IPsec) in 
> selinux_sock_rcv_skb().  As discussed on the list, we would like to move to a 
> single check for the peer label.  Unfortunately, this runs into problems with 
> backwards compatibility with people using old policy on new kernels.  The 
> idea here was that the kernel would have code for both types of access 
> control and depending on the policy loaded it would select which type of 
> access control to use.
>
>   
>> We already have versioning of policy, and will have the ability to reject,
>> ignore or enforce unknown elements, so why do we need this and how would
>> it typically be used?
>>     

It seems like we've gotten versioning of the policy wrong if this sort 
of thing is necessary (two separate, slightly related version numbers). 
A while back someone (I can't remember if it was me or not) suggested a 
bitmap of policy capabilities that would be determined at compile time 
(eg., does this policy have conditional expressions? turn that bit on. 
Does this policy use class foo? turn that bit on, etc). Why is your 
version scheme better than something like that? Granted we'd have a 
limited number of bits (or an ebitmap). I don't know how well this would 
work in practice, conditional policy, for example, still changes to 
policy binary format so the reader needs to know that and read the 
additional avtab.

It might serve to make the format a little more flexible though, instead 
of having policy format changes stack on top of each other the 
functionality bitmap would be checked to see if it should read, for 
example, the additional MLS bits. Right now even if MLS is turned off 
the mls parts of the reader are called just because the policy version 
is above POLICYDB_VERSION_MLS, and not because mls is really enabled in 
the policy. This is true for basically all the format versions we 
currently support.

This might be really hard to support previous versions of policy on 
newer kernels but I think its certainly worth thinking about.

Just for reference, the first bitmap entries would be:
conditional policy,
ipv6 support in network contexts,
separate netlink classes,
new mls support,
compressed avtab support,
new mls rangetrans support;

I think a functionality bitmap would cover those all fine, if the newer 
netlink classes are present and used that bit would flip on and the 
hooks would use them, if it's off the hooks would use the old netlink 
class. If the avtab compression bit is on the reader would read the 
reverse attr lookup tables, if not it wouldn't, etc.

> Eric suggested using the policy version to achieve the same thing; if no one 
> objects to using the policy version number for this purpose I'm more than 
> happy to use it.
>
>   

The more we use the policy version number for things like this the worse 
the reader (and compiler) get. I agree with the idea of having something 
separate, I just don't think another version number is the answer.

>> Should we instead start thinking about versioning each permission & class?
>> i.e. create a fully versioned interface
>>     
>
> This question nags at me, but I keep ignoring it because I fear that a fully 
> versioned avc interface would be overkill in the majority of cases and only 
> serve to slow down the access checks.
>   

I don't think it solves the problem, of the versions I've listed the 
only one this might help with is the netlink object classes.


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [RFC PATCH 0/2] Series short description
  2007-09-26  2:19     ` Joshua Brindle
@ 2007-09-26  3:12       ` Paul Moore
  2007-09-26 13:18         ` Joshua Brindle
  2007-09-26 13:29         ` Stephen Smalley
  0 siblings, 2 replies; 28+ messages in thread
From: Paul Moore @ 2007-09-26  3:12 UTC (permalink / raw)
  To: Joshua Brindle; +Cc: James Morris, selinux

On Tuesday 25 September 2007 10:19:50 pm Joshua Brindle wrote:
> It seems like we've gotten versioning of the policy wrong if this sort
> of thing is necessary (two separate, slightly related version numbers).

I don't think it's necessary to have these two version numbers, as Eric 
pointed out earlier the existing policy version would work.  While I don't 
have my "History of SELinux" book in front of me right now to know the 
original intent of the policy version number, it appears to have been used to 
signify policy capabilities on more than one occasion.

> A while back someone (I can't remember if it was me or not) suggested a
> bitmap of policy capabilities that would be determined at compile time
> (eg., does this policy have conditional expressions? turn that bit on.
> Does this policy use class foo? turn that bit on, etc). Why is your
> version scheme better than something like that?

Well, like I said earlier, forget the versioning scheme I proposed.  However, 
for the sake or argument one advantage that I can see to a version number or 
a capability bitmap (it would most likely need to be an ebitmap to allow for 
future growth) is that it is much easier/quicker to check in the security 
server.  Comparing two integers is much faster then checking for a single bit 
in an ebitmap.

That said, I agree that your idea of a policy capability bitmap is interesting 
and would allow a lot of flexibility.  I'm just not sure the implementation 
would be practical, but then again, you have much more experience in the 
policy compiler/toolchain than I do.

> > Eric suggested using the policy version to achieve the same thing; if no
> > one objects to using the policy version number for this purpose I'm more
> > than happy to use it.
>
> The more we use the policy version number for things like this the worse
> the reader (and compiler) get. I agree with the idea of having something
> separate, I just don't think another version number is the answer.

Ideally we would be able to speed up both the security server and the 
compiler/parser, but if I had to pick only one I would choose to speed up the 
security server.

>From what I can see the policy version number has been abused several times to 
signify new capabilities within the policy.  Using the policy version to 
signal a shift in the network access controls seems like an acceptable thing 
to me based on this history.  However, I understand your concerns that this 
is an _abuse_ of the policy version concept and continuing down this road is 
only going to cause more and more problems in the policy compiler/parser.  
The capability bitmap is initially attractive but I fear it will introduce 
too much additional overhead in the security server.

I'm not quite sure where this leaves us, but I would like to work towards 
resolving this somehow ...

-- 
paul moore
linux security @ hp


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [RFC PATCH 2/2] [SELINUX] Better integration between peer labeling subsystems
  2007-09-25 20:48 ` [RFC PATCH 2/2] [SELINUX] Better integration between peer labeling subsystems Paul Moore
  2007-09-25 21:37   ` Eric Paris
  2007-09-25 22:38   ` James Morris
@ 2007-09-26 12:41   ` Stephen Smalley
  2007-09-26 15:46     ` Paul Moore
  2 siblings, 1 reply; 28+ messages in thread
From: Stephen Smalley @ 2007-09-26 12:41 UTC (permalink / raw)
  To: Paul Moore; +Cc: selinux

On Tue, 2007-09-25 at 16:48 -0400, Paul Moore wrote:
> Rename the existing selinux_skb_extlbl_sid() function to
> selinux_skb_peerlbl_sid() and modify it's behavior such that it now reconciles
> multiple peer/external labels and if reconciliation is not possible it returns
> an error to the caller.
> ---
> 
>  security/selinux/hooks.c            |   70 ++++++++++++++++++++++++++---------
>  security/selinux/include/security.h |    2 +
>  security/selinux/ss/services.c      |   42 +++++++++++++++++++++
>  3 files changed, 95 insertions(+), 19 deletions(-)
> 
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index 0753b20..a97d1b1 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -3132,32 +3132,59 @@ static int selinux_parse_skb(struct sk_buff *skb, struct avc_audit_data *ad,
>  }
>  
>  /**
> - * selinux_skb_extlbl_sid - Determine the external label of a packet
> + * selinux_skb_peerlbl_sid - Determine the peer label of a packet
>   * @skb: the packet
> - * @sid: the packet's SID
> + * @sid: the packet's peer label SID
>   *
>   * Description:
> - * Check the various different forms of external packet labeling and determine
> - * the external SID for the packet.  If only one form of external labeling is
> - * present then it is used, if both labeled IPsec and NetLabel labels are
> - * present then the SELinux type information is taken from the labeled IPsec
> - * SA and the MLS sensitivity label information is taken from the NetLabel
> - * security attributes.  This bit of "magic" is done in the call to
> - * selinux_netlbl_skbuff_getsid().
> + * Check the various different forms of network peer labeling and determine
> + * the peer label/SID for the packet.  If only one form of peer labeling is
> + * present then it is returned in @sid.  If multiple forms of peer labeling
> + * are present the the different peer labels are checked for consistency
> + * (i.e. all the different peer labels have the same value) before the peer
> + * label is returned to the caller in @sid.  The function returns negative
> + * values when there are multiple, inconsistent peer labels present and zero
> + * otherwise.  A table summarizing the different return values is below:
> + *
> + *                                 | function return |      @sid
> + *   ------------------------------+-----------------+-----------------
> + *   no peer labels                |        0        |    SECSID_NULL
> + *   single peer label             |        0        |    peer_label
> + *   multiple, consistent labels   |        0        |    peer_label
> + *   multiple, inconsistent labels |     -EACCES     |    SECSID_NULL
>   *
>   */
> -static void selinux_skb_extlbl_sid(struct sk_buff *skb, u32 *sid)
> +static int selinux_skb_peerlbl_sid(struct sk_buff *skb, u32 *sid)
>  {
>  	u32 xfrm_sid;
>  	u32 nlbl_sid;
>  
>  	selinux_skb_xfrm_sid(skb, &xfrm_sid);
> -	if (selinux_netlbl_skbuff_getsid(skb,
> -					 (xfrm_sid == SECSID_NULL ?
> -					  SECINITSID_NETMSG : xfrm_sid),
> -					 &nlbl_sid) != 0)
> -		nlbl_sid = SECSID_NULL;
> -	*sid = (nlbl_sid == SECSID_NULL ? xfrm_sid : nlbl_sid);
> +	selinux_netlbl_skbuff_getsid(skb, SECINITSID_NETMSG, &nlbl_sid);
> +
> +	if (nlbl_sid != SECSID_NULL && xfrm_sid != SECSID_NULL) {
> +		if (nlbl_sid != xfrm_sid &&
> +		    /* XXX - not sure if we should just compare the low end of
> +		     * the range or the whole range?  probably safest to
> +		     * compare the entire range ... */
> +		    security_sid_mls_cmp(nlbl_sid, xfrm_sid) != 0) {

I know that this isn't the first instance of this, but the goal of the
Flask architecture was to encapsulate the security model completely
within the security server.  So leaking MLS specific logic out into the
hook functions (as is also done by security_sid_mls_copy) violates that
goal.

> +			*sid = SECSID_NULL;
> +			return -EACCES;
> +		} else
> +			/* at present NetLabel SIDs/labels really only carry
> +			 * MLS information so if the MLS portion of the
> +			 * NetLabel SID matches the MLS portion of the labeled
> +			 * XFRM SID/label then pass along the XFRM SID as it
> +			 * has the most peer label information */
> +			*sid = xfrm_sid;
> +	} else if (nlbl_sid != SECSID_NULL)
> +		*sid = nlbl_sid;
> +	else if (xfrm_sid != SECSID_NULL)
> +		*sid = xfrm_sid;
> +	else
> +		*sid = SECSID_NULL;
> +
> +	return 0;
>  }
>  
>  /* socket security operations */
> @@ -3640,6 +3667,9 @@ static int selinux_socket_sock_rcv_skb(struct sock *sk, struct sk_buff *skb)
>  	if (err)
>  		goto out;
>  
> +	/* XXX - make use of selinux_skb_peerlbl_sid() here but only once we
> +	 *       have the new peer object class in place */
> +
>  	err = selinux_netlbl_sock_rcv_skb(sksec, skb, &ad);
>  	if (err)
>  		goto out;
> @@ -3701,7 +3731,7 @@ static int selinux_socket_getpeersec_dgram(struct socket *sock, struct sk_buff *
>  	if (sock && sock->sk->sk_family == PF_UNIX)
>  		selinux_get_inode_sid(SOCK_INODE(sock), &peer_secid);
>  	else if (skb)
> -		selinux_skb_extlbl_sid(skb, &peer_secid);
> +		selinux_skb_peerlbl_sid(skb, &peer_secid);
>  
>  	if (peer_secid == SECSID_NULL)
>  		err = -EINVAL;
> @@ -3762,7 +3792,9 @@ static int selinux_inet_conn_request(struct sock *sk, struct sk_buff *skb,
>  	u32 newsid;
>  	u32 peersid;
>  
> -	selinux_skb_extlbl_sid(skb, &peersid);
> +	err = selinux_skb_peerlbl_sid(skb, &peersid);
> +	if (err)
> +		return err;
>  	if (peersid == SECSID_NULL) {
>  		req->secid = sksec->sid;
>  		req->peer_secid = SECSID_NULL;
> @@ -3800,7 +3832,7 @@ static void selinux_inet_conn_established(struct sock *sk,
>  {
>  	struct sk_security_struct *sksec = sk->sk_security;
>  
> -	selinux_skb_extlbl_sid(skb, &sksec->peer_sid);
> +	selinux_skb_peerlbl_sid(skb, &sksec->peer_sid);
>  }
>  
>  static void selinux_req_classify_flow(const struct request_sock *req,
> diff --git a/security/selinux/include/security.h b/security/selinux/include/security.h
> index cd45cf0..4cbb6f7 100644
> --- a/security/selinux/include/security.h
> +++ b/security/selinux/include/security.h
> @@ -109,6 +109,8 @@ int security_validate_transition(u32 oldsid, u32 newsid, u32 tasksid,
>  
>  int security_sid_mls_copy(u32 sid, u32 mls_sid, u32 *new_sid);
>  
> +int security_sid_mls_cmp(u32 sid_a, u32 sid_b);
> +
>  int security_get_classes(char ***classes, int *nclasses);
>  int security_get_permissions(char *class, char ***perms, int *nperms);
>  
> diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c
> index ef557b6..ea78f75 100644
> --- a/security/selinux/ss/services.c
> +++ b/security/selinux/ss/services.c
> @@ -2009,6 +2009,48 @@ out:
>  	return rc;
>  }
>  
> +/**
> + * security_sid_mls_cmp - Compare the MLS portion of two SIDs
> + * @sid_a: SID to compare
> + * @sid_b: SID to compare
> + *
> + * Description:
> + * Compare the MLS portions of two SIDs and return zero if they are the same,
> + * otherwise return a non-zero value.
> + *
> + */
> +int security_sid_mls_cmp(u32 sid_a, u32 sid_b)
> +{
> +	int rc;
> +	struct context *ctx_a;
> +	struct context *ctx_b;
> +
> +	if (!ss_initialized || !selinux_mls_enabled)
> +		return 0;
> +
> +	POLICY_RDLOCK;
> +
> +	ctx_a = sidtab_search(&sidtab, sid_a);
> +	if (!ctx_a) {
> +		printk(KERN_ERR
> +		       "security_sid_mls_cmp:  unrecognized SID %d\n", sid_a);
> +		rc = -EINVAL;
> +		goto out;
> +	}
> +	ctx_b = sidtab_search(&sidtab, sid_b);
> +	if (!ctx_b) {
> +		printk(KERN_ERR
> +		       "security_sid_mls_cmp:  unrecognized SID %d\n", sid_b);
> +		rc = -EINVAL;
> +		goto out;
> +	}
> +	rc = mls_context_cmp(ctx_a, ctx_b);
> +
> +out:
> +	POLICY_RDUNLOCK;
> +	return rc;
> +}
> +
>  static int get_classes_callback(void *k, void *d, void *args)
>  {
>  	struct class_datum *datum = d;
> 
> 
> --
> This message was distributed to subscribers of the selinux mailing list.
> If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
> the words "unsubscribe selinux" without quotes as the message.
-- 
Stephen Smalley
National Security Agency


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [RFC PATCH 0/2] Series short description
  2007-09-26  3:12       ` Paul Moore
@ 2007-09-26 13:18         ` Joshua Brindle
  2007-09-26 13:29         ` Stephen Smalley
  1 sibling, 0 replies; 28+ messages in thread
From: Joshua Brindle @ 2007-09-26 13:18 UTC (permalink / raw)
  To: Paul Moore; +Cc: James Morris, selinux

Paul Moore wrote:
> On Tuesday 25 September 2007 10:19:50 pm Joshua Brindle wrote:
>   
>> It seems like we've gotten versioning of the policy wrong if this sort
>> of thing is necessary (two separate, slightly related version numbers).
>>     
>
> I don't think it's necessary to have these two version numbers, as Eric 
> pointed out earlier the existing policy version would work.  While I don't 
> have my "History of SELinux" book in front of me right now to know the 
> original intent of the policy version number, it appears to have been used to 
> signify policy capabilities on more than one occasion.
>
>   

Yes.

>> A while back someone (I can't remember if it was me or not) suggested a
>> bitmap of policy capabilities that would be determined at compile time
>> (eg., does this policy have conditional expressions? turn that bit on.
>> Does this policy use class foo? turn that bit on, etc). Why is your
>> version scheme better than something like that?
>>     
>
> Well, like I said earlier, forget the versioning scheme I proposed.  However, 
> for the sake or argument one advantage that I can see to a version number or 
> a capability bitmap (it would most likely need to be an ebitmap to allow for 
> future growth) is that it is much easier/quicker to check in the security 
> server.  Comparing two integers is much faster then checking for a single bit 
> in an ebitmap.
>
>   

Meh, optimizations are easy.

int reconcile_peersids = ebitmap_get_bit(pol->capability_map, PEERSID_BIT);

do this at policy read time for anything that needs fast comparisons.

> That said, I agree that your idea of a policy capability bitmap is interesting 
> and would allow a lot of flexibility.  I'm just not sure the implementation 
> would be practical, but then again, you have much more experience in the 
> policy compiler/toolchain than I do.
>
>   

Alot more practical than the policy version comparisons that are there 
now (and once again, this strangeness with stacking of capabilities, 
ipv6 implies boolean support, fine grained netlink classes imply ipv6 
support, etc)


>>> Eric suggested using the policy version to achieve the same thing; if no
>>> one objects to using the policy version number for this purpose I'm more
>>> than happy to use it.
>>>       
>> The more we use the policy version number for things like this the worse
>> the reader (and compiler) get. I agree with the idea of having something
>> separate, I just don't think another version number is the answer.
>>     
>
> Ideally we would be able to speed up both the security server and the 
> compiler/parser, but if I had to pick only one I would choose to speed up the 
> security server.
>
> >From what I can see the policy version number has been abused several times to 
> signify new capabilities within the policy.  Using the policy version to 
> signal a shift in the network access controls seems like an acceptable thing 
> to me based on this history.  However, I understand your concerns that this 
> is an _abuse_ of the policy version concept and continuing down this road is 
> only going to cause more and more problems in the policy compiler/parser.  
> The capability bitmap is initially attractive but I fear it will introduce 
> too much additional overhead in the security server.
>
> I'm not quite sure where this leaves us, but I would like to work towards 
> resolving this somehow ...
>   

Its been abused like that many times, and it may again. One benefit of 
the bitmap is that it would allow us to turn off features we aren't 
using, even if the format supports it. For example, if I'm building a 
real time system without conditional policy wouldn't it be nice to not 
waste time checking the conditional avtab? Or if its non-mls we could 
just skip checking those constraints altogether.




--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [RFC PATCH 0/2] Series short description
  2007-09-26  3:12       ` Paul Moore
  2007-09-26 13:18         ` Joshua Brindle
@ 2007-09-26 13:29         ` Stephen Smalley
  2007-09-26 16:00           ` Paul Moore
  2007-09-26 20:36           ` Joshua Brindle
  1 sibling, 2 replies; 28+ messages in thread
From: Stephen Smalley @ 2007-09-26 13:29 UTC (permalink / raw)
  To: Paul Moore; +Cc: Joshua Brindle, James Morris, selinux

On Tue, 2007-09-25 at 23:12 -0400, Paul Moore wrote:
> On Tuesday 25 September 2007 10:19:50 pm Joshua Brindle wrote:
> > It seems like we've gotten versioning of the policy wrong if this sort
> > of thing is necessary (two separate, slightly related version numbers).
> 
> I don't think it's necessary to have these two version numbers, as Eric 
> pointed out earlier the existing policy version would work.  While I don't 
> have my "History of SELinux" book in front of me right now to know the 
> original intent of the policy version number, it appears to have been used to 
> signify policy capabilities on more than one occasion.

The policy version tells the kernel how to interpret the policy image.
It is primarily about the policy image format, not the policy content,
although it has been abused for the latter in some cases because we had
no other mechanism.

Using the policy version as an indication of the policy content today is
problematic because the policy toolchain always generates the latest
version/format it supports independent of what the policy may have
contained, and at policy load time, the policy image is automatically
downgraded (by libselinux with help from libsepol) if necessary to the
latest version/format supported by the kernel.  So the kernel may not
even see the original version number that was built.

> 
> > A while back someone (I can't remember if it was me or not) suggested a
> > bitmap of policy capabilities that would be determined at compile time
> > (eg., does this policy have conditional expressions? turn that bit on.
> > Does this policy use class foo? turn that bit on, etc). Why is your
> > version scheme better than something like that?
> 
> Well, like I said earlier, forget the versioning scheme I proposed.  However, 
> for the sake or argument one advantage that I can see to a version number or 
> a capability bitmap (it would most likely need to be an ebitmap to allow for 
> future growth) is that it is much easier/quicker to check in the security 
> server.  Comparing two integers is much faster then checking for a single bit 
> in an ebitmap.
> 
> That said, I agree that your idea of a policy capability bitmap is interesting 
> and would allow a lot of flexibility.  I'm just not sure the implementation 
> would be practical, but then again, you have much more experience in the 
> policy compiler/toolchain than I do.

I think we'd need a separate content/feature version or bitmap from the
format version.

The bitmap seems better if we want to support independent selection of
subsets.  The version seems better if we want to incrementally build
upon each new feature and always require the prior ones to exist or to
be obsoleted by the new one.

> > > Eric suggested using the policy version to achieve the same thing; if no
> > > one objects to using the policy version number for this purpose I'm more
> > > than happy to use it.
> >
> > The more we use the policy version number for things like this the worse
> > the reader (and compiler) get. I agree with the idea of having something
> > separate, I just don't think another version number is the answer.
> 
> Ideally we would be able to speed up both the security server and the 
> compiler/parser, but if I had to pick only one I would choose to speed up the 
> security server.
> 
> >From what I can see the policy version number has been abused several times to 
> signify new capabilities within the policy.  Using the policy version to 
> signal a shift in the network access controls seems like an acceptable thing 
> to me based on this history.  However, I understand your concerns that this 
> is an _abuse_ of the policy version concept and continuing down this road is 
> only going to cause more and more problems in the policy compiler/parser.  
> The capability bitmap is initially attractive but I fear it will introduce 
> too much additional overhead in the security server.
> 
> I'm not quite sure where this leaves us, but I would like to work towards 
> resolving this somehow ...

-- 
Stephen Smalley
National Security Agency


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [RFC PATCH 2/2] [SELINUX] Better integration between peer labeling subsystems
  2007-09-26 12:41   ` Stephen Smalley
@ 2007-09-26 15:46     ` Paul Moore
  2007-09-26 16:18       ` Paul Moore
  0 siblings, 1 reply; 28+ messages in thread
From: Paul Moore @ 2007-09-26 15:46 UTC (permalink / raw)
  To: Stephen Smalley; +Cc: selinux

On Wednesday 26 September 2007 8:41:36 am Stephen Smalley wrote:
> On Tue, 2007-09-25 at 16:48 -0400, Paul Moore wrote:
> > -static void selinux_skb_extlbl_sid(struct sk_buff *skb, u32 *sid)
> > +static int selinux_skb_peerlbl_sid(struct sk_buff *skb, u32 *sid)
> >  {
> >  	u32 xfrm_sid;
> >  	u32 nlbl_sid;
> >
> >  	selinux_skb_xfrm_sid(skb, &xfrm_sid);
> > -	if (selinux_netlbl_skbuff_getsid(skb,
> > -					 (xfrm_sid == SECSID_NULL ?
> > -					  SECINITSID_NETMSG : xfrm_sid),
> > -					 &nlbl_sid) != 0)
> > -		nlbl_sid = SECSID_NULL;
> > -	*sid = (nlbl_sid == SECSID_NULL ? xfrm_sid : nlbl_sid);
> > +	selinux_netlbl_skbuff_getsid(skb, SECINITSID_NETMSG, &nlbl_sid);
> > +
> > +	if (nlbl_sid != SECSID_NULL && xfrm_sid != SECSID_NULL) {
> > +		if (nlbl_sid != xfrm_sid &&
> > +		    /* XXX - not sure if we should just compare the low end of
> > +		     * the range or the whole range?  probably safest to
> > +		     * compare the entire range ... */
> > +		    security_sid_mls_cmp(nlbl_sid, xfrm_sid) != 0) {
>
> I know that this isn't the first instance of this, but the goal of the
> Flask architecture was to encapsulate the security model completely
> within the security server.  So leaking MLS specific logic out into the
> hook functions (as is also done by security_sid_mls_copy) violates that
> goal.

A reasonable request, although off the top of my head I'm not sure there is 
much we can do other than rename security_sid_mls_cmp() to something a bit 
less MLS'esque.  Perhaps security_net_peersid_cmp()?  If you have a better 
idea I'm all ears/eyes ...

Regarding security_sid_mls_copy(), as you have pointed out before, we should 
try and rework that so the newly accepted socket just takes the label of the 
peer but I think we are still a ways off from being able to do that in the 
policy.  Always something to work on ...

-- 
paul moore
linux security @ hp

--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [RFC PATCH 0/2] Series short description
  2007-09-26 13:29         ` Stephen Smalley
@ 2007-09-26 16:00           ` Paul Moore
  2007-09-26 16:43             ` Joshua Brindle
  2007-09-26 20:36           ` Joshua Brindle
  1 sibling, 1 reply; 28+ messages in thread
From: Paul Moore @ 2007-09-26 16:00 UTC (permalink / raw)
  To: Stephen Smalley, Joshua Brindle; +Cc: James Morris, selinux

On Wednesday 26 September 2007 9:29:52 am Stephen Smalley wrote:
> On Tue, 2007-09-25 at 23:12 -0400, Paul Moore wrote:
> > On Tuesday 25 September 2007 10:19:50 pm Joshua Brindle wrote:
> > > A while back someone (I can't remember if it was me or not) suggested a
> > > bitmap of policy capabilities that would be determined at compile time
> > > (eg., does this policy have conditional expressions? turn that bit on.
> > > Does this policy use class foo? turn that bit on, etc). Why is your
> > > version scheme better than something like that?
> >
> > Well, like I said earlier, forget the versioning scheme I proposed. 
> > However, for the sake or argument one advantage that I can see to a
> > version number or a capability bitmap (it would most likely need to be an
> > ebitmap to allow for future growth) is that it is much easier/quicker to
> > check in the security server.  Comparing two integers is much faster then
> > checking for a single bit in an ebitmap.
> >
> > That said, I agree that your idea of a policy capability bitmap is
> > interesting and would allow a lot of flexibility.  I'm just not sure the
> > implementation would be practical, but then again, you have much more
> > experience in the policy compiler/toolchain than I do.
>
> I think we'd need a separate content/feature version or bitmap from the
> format version.
>
> The bitmap seems better if we want to support independent selection of
> subsets.  The version seems better if we want to incrementally build
> upon each new feature and always require the prior ones to exist or to
> be obsoleted by the new one.

Okay, it looks like its probably at least worth investigating the bitmap 
concept with some RFC patches.

Josh, since you brought up the idea and you have a better grasp of the 
toolchain, can I talk/bribe/make-you-an-offer-you-can't-refuse into working 
up some patches for the toolchain if I come up with the matching kernel 
piece?

-- 
paul moore
linux security @ hp

--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [RFC PATCH 2/2] [SELINUX] Better integration between peer labeling subsystems
  2007-09-26 15:46     ` Paul Moore
@ 2007-09-26 16:18       ` Paul Moore
  0 siblings, 0 replies; 28+ messages in thread
From: Paul Moore @ 2007-09-26 16:18 UTC (permalink / raw)
  To: Stephen Smalley; +Cc: selinux

On Wednesday 26 September 2007 11:46:28 am Paul Moore wrote:
> On Wednesday 26 September 2007 8:41:36 am Stephen Smalley wrote:
> > On Tue, 2007-09-25 at 16:48 -0400, Paul Moore wrote:
> > > -static void selinux_skb_extlbl_sid(struct sk_buff *skb, u32 *sid)
> > > +static int selinux_skb_peerlbl_sid(struct sk_buff *skb, u32 *sid)
> > >  {
> > >  	u32 xfrm_sid;
> > >  	u32 nlbl_sid;
> > >
> > >  	selinux_skb_xfrm_sid(skb, &xfrm_sid);
> > > -	if (selinux_netlbl_skbuff_getsid(skb,
> > > -					 (xfrm_sid == SECSID_NULL ?
> > > -					  SECINITSID_NETMSG : xfrm_sid),
> > > -					 &nlbl_sid) != 0)
> > > -		nlbl_sid = SECSID_NULL;
> > > -	*sid = (nlbl_sid == SECSID_NULL ? xfrm_sid : nlbl_sid);
> > > +	selinux_netlbl_skbuff_getsid(skb, SECINITSID_NETMSG, &nlbl_sid);
> > > +
> > > +	if (nlbl_sid != SECSID_NULL && xfrm_sid != SECSID_NULL) {
> > > +		if (nlbl_sid != xfrm_sid &&
> > > +		    /* XXX - not sure if we should just compare the low end of
> > > +		     * the range or the whole range?  probably safest to
> > > +		     * compare the entire range ... */
> > > +		    security_sid_mls_cmp(nlbl_sid, xfrm_sid) != 0) {
> >
> > I know that this isn't the first instance of this, but the goal of the
> > Flask architecture was to encapsulate the security model completely
> > within the security server.  So leaking MLS specific logic out into the
> > hook functions (as is also done by security_sid_mls_copy) violates that
> > goal.
>
> A reasonable request, although off the top of my head I'm not sure there is
> much we can do other than rename security_sid_mls_cmp() to something a bit
> less MLS'esque.  Perhaps security_net_peersid_cmp()?  If you have a better
> idea I'm all ears/eyes ...

On second thought, it's probably better to move most of the peer label 
resolution logic into the security server so that selinux_skb_peerlbl_sid() 
looks something like the following ...

 selinux_skb_peerlbl_sid(skb, *sid)
 {
	nlbl_sid = fetch_nlbl_sid(skb);
        xfrm_sid = fetch_xfrm_sid(skb);

        *sid = security_net_peersid_resolve(nlbl_sid, xfrm_sid);
 }

-- 
paul moore
linux security @ hp

--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [RFC PATCH 0/2] Series short description
  2007-09-26 16:00           ` Paul Moore
@ 2007-09-26 16:43             ` Joshua Brindle
  2007-09-26 16:48               ` Stephen Smalley
  2007-09-26 16:54               ` Paul Moore
  0 siblings, 2 replies; 28+ messages in thread
From: Joshua Brindle @ 2007-09-26 16:43 UTC (permalink / raw)
  To: Paul Moore; +Cc: Stephen Smalley, James Morris, selinux

Paul Moore wrote:
> On Wednesday 26 September 2007 9:29:52 am Stephen Smalley wrote:
>   
>> On Tue, 2007-09-25 at 23:12 -0400, Paul Moore wrote:
>>     
>>> On Tuesday 25 September 2007 10:19:50 pm Joshua Brindle wrote:
>>>       
>>>> A while back someone (I can't remember if it was me or not) suggested a
>>>> bitmap of policy capabilities that would be determined at compile time
>>>> (eg., does this policy have conditional expressions? turn that bit on.
>>>> Does this policy use class foo? turn that bit on, etc). Why is your
>>>> version scheme better than something like that?
>>>>         
>>> Well, like I said earlier, forget the versioning scheme I proposed. 
>>> However, for the sake or argument one advantage that I can see to a
>>> version number or a capability bitmap (it would most likely need to be an
>>> ebitmap to allow for future growth) is that it is much easier/quicker to
>>> check in the security server.  Comparing two integers is much faster then
>>> checking for a single bit in an ebitmap.
>>>
>>> That said, I agree that your idea of a policy capability bitmap is
>>> interesting and would allow a lot of flexibility.  I'm just not sure the
>>> implementation would be practical, but then again, you have much more
>>> experience in the policy compiler/toolchain than I do.
>>>       
>> I think we'd need a separate content/feature version or bitmap from the
>> format version.
>>
>> The bitmap seems better if we want to support independent selection of
>> subsets.  The version seems better if we want to incrementally build
>> upon each new feature and always require the prior ones to exist or to
>> be obsoleted by the new one.
>>     
>
> Okay, it looks like its probably at least worth investigating the bitmap 
> concept with some RFC patches.
>
> Josh, since you brought up the idea and you have a better grasp of the 
> toolchain, can I talk/bribe/make-you-an-offer-you-can't-refuse into working 
> up some patches for the toolchain if I come up with the matching kernel 
> piece?
>   

I knew I should have kept my mouth shut :)

I'll try to get around to the toolchain patches this week. Some things 
need to be settled, this will require a policy version bump to 22, and 
we can either ignore all the capabilities we have now and start with 
secid reconciliation or actually factor in all the capabilities we have 
now into it. It will be easier short term to do the former, but possibly 
better in the long term to do the latter.

The latter might have some crappy code in it, for example, in the reader 
when reading the binary there would be something like:

if ((policyvers > POLICYDB_VERSION_BOOLS && policyvers < 
POLICYDB_VERSION_CAPS) || (policyvers > POLICYDB_VERSION_CAPS && 
ebitmap_get_bit(cap_bitmap, BOOL_CAP_BIT))

it gets ugly, if we just ignore what we have now it would look alot 
nicer, though I still like the idea of being able to turn off 
unnecessary checks when conditionals aren't used at all.



--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [RFC PATCH 0/2] Series short description
  2007-09-26 16:43             ` Joshua Brindle
@ 2007-09-26 16:48               ` Stephen Smalley
  2007-09-26 16:54               ` Paul Moore
  1 sibling, 0 replies; 28+ messages in thread
From: Stephen Smalley @ 2007-09-26 16:48 UTC (permalink / raw)
  To: Joshua Brindle; +Cc: Paul Moore, James Morris, selinux

On Wed, 2007-09-26 at 12:43 -0400, Joshua Brindle wrote:
> Paul Moore wrote:
> > On Wednesday 26 September 2007 9:29:52 am Stephen Smalley wrote:
> >   
> >> On Tue, 2007-09-25 at 23:12 -0400, Paul Moore wrote:
> >>     
> >>> On Tuesday 25 September 2007 10:19:50 pm Joshua Brindle wrote:
> >>>       
> >>>> A while back someone (I can't remember if it was me or not) suggested a
> >>>> bitmap of policy capabilities that would be determined at compile time
> >>>> (eg., does this policy have conditional expressions? turn that bit on.
> >>>> Does this policy use class foo? turn that bit on, etc). Why is your
> >>>> version scheme better than something like that?
> >>>>         
> >>> Well, like I said earlier, forget the versioning scheme I proposed. 
> >>> However, for the sake or argument one advantage that I can see to a
> >>> version number or a capability bitmap (it would most likely need to be an
> >>> ebitmap to allow for future growth) is that it is much easier/quicker to
> >>> check in the security server.  Comparing two integers is much faster then
> >>> checking for a single bit in an ebitmap.
> >>>
> >>> That said, I agree that your idea of a policy capability bitmap is
> >>> interesting and would allow a lot of flexibility.  I'm just not sure the
> >>> implementation would be practical, but then again, you have much more
> >>> experience in the policy compiler/toolchain than I do.
> >>>       
> >> I think we'd need a separate content/feature version or bitmap from the
> >> format version.
> >>
> >> The bitmap seems better if we want to support independent selection of
> >> subsets.  The version seems better if we want to incrementally build
> >> upon each new feature and always require the prior ones to exist or to
> >> be obsoleted by the new one.
> >>     
> >
> > Okay, it looks like its probably at least worth investigating the bitmap 
> > concept with some RFC patches.
> >
> > Josh, since you brought up the idea and you have a better grasp of the 
> > toolchain, can I talk/bribe/make-you-an-offer-you-can't-refuse into working 
> > up some patches for the toolchain if I come up with the matching kernel 
> > piece?
> >   
> 
> I knew I should have kept my mouth shut :)
> 
> I'll try to get around to the toolchain patches this week. Some things 
> need to be settled, this will require a policy version bump to 22, and 
> we can either ignore all the capabilities we have now and start with 
> secid reconciliation or actually factor in all the capabilities we have 
> now into it. It will be easier short term to do the former, but possibly 
> better in the long term to do the latter.
> 
> The latter might have some crappy code in it, for example, in the reader 
> when reading the binary there would be something like:
> 
> if ((policyvers > POLICYDB_VERSION_BOOLS && policyvers < 
> POLICYDB_VERSION_CAPS) || (policyvers > POLICYDB_VERSION_CAPS && 
> ebitmap_get_bit(cap_bitmap, BOOL_CAP_BIT))
> 
> it gets ugly, if we just ignore what we have now it would look alot 
> nicer, though I still like the idea of being able to turn off 
> unnecessary checks when conditionals aren't used at all.

I'm not clear it is worth trying to apply this retroactively to the
existing "versions"/features, and it is both potentially much more
disruptive.  In the case where there are no conditional rules, the
avtab_search() will devolve trivially.  Similarly for constraints (and
MLS constraints aren't treated specially internally - they are just
added to the per-class constraints list).  I don't think we gain much by
omitting them altogether.

-- 
Stephen Smalley
National Security Agency


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [RFC PATCH 0/2] Series short description
  2007-09-26 16:43             ` Joshua Brindle
  2007-09-26 16:48               ` Stephen Smalley
@ 2007-09-26 16:54               ` Paul Moore
  2007-09-26 16:57                 ` Joshua Brindle
  1 sibling, 1 reply; 28+ messages in thread
From: Paul Moore @ 2007-09-26 16:54 UTC (permalink / raw)
  To: Joshua Brindle; +Cc: Stephen Smalley, James Morris, selinux

On Wednesday 26 September 2007 12:43:28 pm Joshua Brindle wrote:
> Paul Moore wrote:
> > Okay, it looks like its probably at least worth investigating the bitmap
> > concept with some RFC patches.
> >
> > Josh, since you brought up the idea and you have a better grasp of the
> > toolchain, can I talk/bribe/make-you-an-offer-you-can't-refuse into
> > working up some patches for the toolchain if I come up with the matching
> > kernel piece?
>
> I knew I should have kept my mouth shut :)

Think of it as "taking one for the team" ;)

> I'll try to get around to the toolchain patches this week. Some things
> need to be settled, this will require a policy version bump to 22, and
> we can either ignore all the capabilities we have now and start with
> secid reconciliation or actually factor in all the capabilities we have
> now into it. It will be easier short term to do the former, but possibly
> better in the long term to do the latter.

I'm not familiar enough with that code to have a strong opinion either way, 
although I would think that if you are going to introduce a new policy 
version to support this you might as well go all the way and put all the 
features into the bitmap.

Also, don't worry too much about getting them done right away, there is plenty 
of work still left to do with the network bits before this issue becomes a 
roadblock.  Where would you put the capability bitmap in the policy?  After 
the version/config/table-sizes (where I put the ill-fated version number)?

-- 
paul moore
linux security @ hp

--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [RFC PATCH 0/2] Series short description
  2007-09-26 16:54               ` Paul Moore
@ 2007-09-26 16:57                 ` Joshua Brindle
  2007-09-26 17:04                   ` Paul Moore
  0 siblings, 1 reply; 28+ messages in thread
From: Joshua Brindle @ 2007-09-26 16:57 UTC (permalink / raw)
  To: Paul Moore; +Cc: Stephen Smalley, James Morris, selinux

Paul Moore wrote:
> On Wednesday 26 September 2007 12:43:28 pm Joshua Brindle wrote:
>   
>> Paul Moore wrote:
>>     
>>> Okay, it looks like its probably at least worth investigating the bitmap
>>> concept with some RFC patches.
>>>
>>> Josh, since you brought up the idea and you have a better grasp of the
>>> toolchain, can I talk/bribe/make-you-an-offer-you-can't-refuse into
>>> working up some patches for the toolchain if I come up with the matching
>>> kernel piece?
>>>       
>> I knew I should have kept my mouth shut :)
>>     
>
> Think of it as "taking one for the team" ;)
>
>   
>> I'll try to get around to the toolchain patches this week. Some things
>> need to be settled, this will require a policy version bump to 22, and
>> we can either ignore all the capabilities we have now and start with
>> secid reconciliation or actually factor in all the capabilities we have
>> now into it. It will be easier short term to do the former, but possibly
>> better in the long term to do the latter.
>>     
>
> I'm not familiar enough with that code to have a strong opinion either way, 
> although I would think that if you are going to introduce a new policy 
> version to support this you might as well go all the way and put all the 
> features into the bitmap.
>
> Also, don't worry too much about getting them done right away, there is plenty 
> of work still left to do with the network bits before this issue becomes a 
> roadblock.  Where would you put the capability bitmap in the policy?  After 
> the version/config/table-sizes (where I put the ill-fated version number)?
>   

Yes, that is probably the best place, though just sticking it somewhere 
random every time would be much more exciting...


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [RFC PATCH 0/2] Series short description
  2007-09-26 16:57                 ` Joshua Brindle
@ 2007-09-26 17:04                   ` Paul Moore
  2007-09-26 20:39                     ` Joshua Brindle
  0 siblings, 1 reply; 28+ messages in thread
From: Paul Moore @ 2007-09-26 17:04 UTC (permalink / raw)
  To: Joshua Brindle; +Cc: Stephen Smalley, James Morris, selinux

On Wednesday 26 September 2007 12:57:34 pm Joshua Brindle wrote:
> Paul Moore wrote:
> > On Wednesday 26 September 2007 12:43:28 pm Joshua Brindle wrote:
> >> Paul Moore wrote:
> >>> Okay, it looks like its probably at least worth investigating the
> >>> bitmap concept with some RFC patches.
> >>>
> >>> Josh, since you brought up the idea and you have a better grasp of the
> >>> toolchain, can I talk/bribe/make-you-an-offer-you-can't-refuse into
> >>> working up some patches for the toolchain if I come up with the
> >>> matching kernel piece?
> >>
> >> I knew I should have kept my mouth shut :)
> >
> > Think of it as "taking one for the team" ;)
> >
> >> I'll try to get around to the toolchain patches this week. Some things
> >> need to be settled, this will require a policy version bump to 22, and
> >> we can either ignore all the capabilities we have now and start with
> >> secid reconciliation or actually factor in all the capabilities we have
> >> now into it. It will be easier short term to do the former, but possibly
> >> better in the long term to do the latter.
> >
> > I'm not familiar enough with that code to have a strong opinion either
> > way, although I would think that if you are going to introduce a new
> > policy version to support this you might as well go all the way and put
> > all the features into the bitmap.
> >
> > Also, don't worry too much about getting them done right away, there is
> > plenty of work still left to do with the network bits before this issue
> > becomes a roadblock.  Where would you put the capability bitmap in the
> > policy?  After the version/config/table-sizes (where I put the ill-fated
> > version number)?
>
> Yes, that is probably the best place, though just sticking it somewhere
> random every time would be much more exciting...

Oh yeah, we could even shift the bitmap X number of bits based on the checksum 
of the policy!  So much fun!

I really need to find something else to occupy my time :)

-- 
paul moore
linux security @ hp

--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [RFC PATCH 0/2] Series short description
  2007-09-26 20:36           ` Joshua Brindle
@ 2007-09-26 20:32             ` Stephen Smalley
  0 siblings, 0 replies; 28+ messages in thread
From: Stephen Smalley @ 2007-09-26 20:32 UTC (permalink / raw)
  To: Joshua Brindle; +Cc: Paul Moore, James Morris, selinux

On Wed, 2007-09-26 at 16:36 -0400, Joshua Brindle wrote:
> Stephen Smalley wrote:
> > On Tue, 2007-09-25 at 23:12 -0400, Paul Moore wrote:
> >   
> >> On Tuesday 25 September 2007 10:19:50 pm Joshua Brindle wrote:
> >>     
> >>> It seems like we've gotten versioning of the policy wrong if this sort
> >>> of thing is necessary (two separate, slightly related version numbers).
> >>>       
> >> I don't think it's necessary to have these two version numbers, as Eric 
> >> pointed out earlier the existing policy version would work.  While I don't 
> >> have my "History of SELinux" book in front of me right now to know the 
> >> original intent of the policy version number, it appears to have been used to 
> >> signify policy capabilities on more than one occasion.
> >>     
> >
> > The policy version tells the kernel how to interpret the policy image.
> > It is primarily about the policy image format, not the policy content,
> > although it has been abused for the latter in some cases because we had
> > no other mechanism.
> >
> > Using the policy version as an indication of the policy content today is
> > problematic because the policy toolchain always generates the latest
> > version/format it supports independent of what the policy may have
> > contained, and at policy load time, the policy image is automatically
> > downgraded (by libselinux with help from libsepol) if necessary to the
> > latest version/format supported by the kernel.  So the kernel may not
> > even see the original version number that was built.
> >
> >   
> >>> A while back someone (I can't remember if it was me or not) suggested a
> >>> bitmap of policy capabilities that would be determined at compile time
> >>> (eg., does this policy have conditional expressions? turn that bit on.
> >>> Does this policy use class foo? turn that bit on, etc). Why is your
> >>> version scheme better than something like that?
> >>>       
> >> Well, like I said earlier, forget the versioning scheme I proposed.  However, 
> >> for the sake or argument one advantage that I can see to a version number or 
> >> a capability bitmap (it would most likely need to be an ebitmap to allow for 
> >> future growth) is that it is much easier/quicker to check in the security 
> >> server.  Comparing two integers is much faster then checking for a single bit 
> >> in an ebitmap.
> >>
> >> That said, I agree that your idea of a policy capability bitmap is interesting 
> >> and would allow a lot of flexibility.  I'm just not sure the implementation 
> >> would be practical, but then again, you have much more experience in the 
> >> policy compiler/toolchain than I do.
> >>     
> >
> > I think we'd need a separate content/feature version or bitmap from the
> > format version.
> >
> > The bitmap seems better if we want to support independent selection of
> > subsets.  The version seems better if we want to incrementally build
> > upon each new feature and always require the prior ones to exist or to
> > be obsoleted by the new one.
> >   
> 
> I've always wondered why we incrementally built features on and required 
> the prior ones even though they were completely unrelated. I like the 
> bitmap idea because it lets the compiler (or even the user in some cases 
> (?)) choose which features about the policy are in use and therefore 
> turned on. It would also let someone read a node in selinuxfs and see 
> what the current expected behavior is for the loaded policy. This 
> probably would have been the ideal way to handle compat_net (in fact, we 
> could go ahead and do that if it won't cause problems..)

Well, historically, as we added support for new things, new policies
took advantage of them immediately, and thus people tracked new features
monotonically as they updated their checkpolicy and policy.  Not
independent selection of them.

-- 
Stephen Smalley
National Security Agency


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [RFC PATCH 0/2] Series short description
  2007-09-26 13:29         ` Stephen Smalley
  2007-09-26 16:00           ` Paul Moore
@ 2007-09-26 20:36           ` Joshua Brindle
  2007-09-26 20:32             ` Stephen Smalley
  1 sibling, 1 reply; 28+ messages in thread
From: Joshua Brindle @ 2007-09-26 20:36 UTC (permalink / raw)
  To: Stephen Smalley; +Cc: Paul Moore, James Morris, selinux

Stephen Smalley wrote:
> On Tue, 2007-09-25 at 23:12 -0400, Paul Moore wrote:
>   
>> On Tuesday 25 September 2007 10:19:50 pm Joshua Brindle wrote:
>>     
>>> It seems like we've gotten versioning of the policy wrong if this sort
>>> of thing is necessary (two separate, slightly related version numbers).
>>>       
>> I don't think it's necessary to have these two version numbers, as Eric 
>> pointed out earlier the existing policy version would work.  While I don't 
>> have my "History of SELinux" book in front of me right now to know the 
>> original intent of the policy version number, it appears to have been used to 
>> signify policy capabilities on more than one occasion.
>>     
>
> The policy version tells the kernel how to interpret the policy image.
> It is primarily about the policy image format, not the policy content,
> although it has been abused for the latter in some cases because we had
> no other mechanism.
>
> Using the policy version as an indication of the policy content today is
> problematic because the policy toolchain always generates the latest
> version/format it supports independent of what the policy may have
> contained, and at policy load time, the policy image is automatically
> downgraded (by libselinux with help from libsepol) if necessary to the
> latest version/format supported by the kernel.  So the kernel may not
> even see the original version number that was built.
>
>   
>>> A while back someone (I can't remember if it was me or not) suggested a
>>> bitmap of policy capabilities that would be determined at compile time
>>> (eg., does this policy have conditional expressions? turn that bit on.
>>> Does this policy use class foo? turn that bit on, etc). Why is your
>>> version scheme better than something like that?
>>>       
>> Well, like I said earlier, forget the versioning scheme I proposed.  However, 
>> for the sake or argument one advantage that I can see to a version number or 
>> a capability bitmap (it would most likely need to be an ebitmap to allow for 
>> future growth) is that it is much easier/quicker to check in the security 
>> server.  Comparing two integers is much faster then checking for a single bit 
>> in an ebitmap.
>>
>> That said, I agree that your idea of a policy capability bitmap is interesting 
>> and would allow a lot of flexibility.  I'm just not sure the implementation 
>> would be practical, but then again, you have much more experience in the 
>> policy compiler/toolchain than I do.
>>     
>
> I think we'd need a separate content/feature version or bitmap from the
> format version.
>
> The bitmap seems better if we want to support independent selection of
> subsets.  The version seems better if we want to incrementally build
> upon each new feature and always require the prior ones to exist or to
> be obsoleted by the new one.
>   

I've always wondered why we incrementally built features on and required 
the prior ones even though they were completely unrelated. I like the 
bitmap idea because it lets the compiler (or even the user in some cases 
(?)) choose which features about the policy are in use and therefore 
turned on. It would also let someone read a node in selinuxfs and see 
what the current expected behavior is for the loaded policy. This 
probably would have been the ideal way to handle compat_net (in fact, we 
could go ahead and do that if it won't cause problems..)


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [RFC PATCH 0/2] Series short description
  2007-09-26 17:04                   ` Paul Moore
@ 2007-09-26 20:39                     ` Joshua Brindle
  2007-09-26 20:46                       ` Paul Moore
  0 siblings, 1 reply; 28+ messages in thread
From: Joshua Brindle @ 2007-09-26 20:39 UTC (permalink / raw)
  To: Paul Moore; +Cc: Stephen Smalley, James Morris, selinux

Paul Moore wrote:
> On Wednesday 26 September 2007 12:57:34 pm Joshua Brindle wrote:
>   
>> Paul Moore wrote:
>>     
>>> On Wednesday 26 September 2007 12:43:28 pm Joshua Brindle wrote:
>>>       
>>>> Paul Moore wrote:
>>>>         
>>>>> Okay, it looks like its probably at least worth investigating the
>>>>> bitmap concept with some RFC patches.
>>>>>
>>>>> Josh, since you brought up the idea and you have a better grasp of the
>>>>> toolchain, can I talk/bribe/make-you-an-offer-you-can't-refuse into
>>>>> working up some patches for the toolchain if I come up with the
>>>>> matching kernel piece?
>>>>>           
>>>> I knew I should have kept my mouth shut :)
>>>>         
>>> Think of it as "taking one for the team" ;)
>>>
>>>       
>>>> I'll try to get around to the toolchain patches this week. Some things
>>>> need to be settled, this will require a policy version bump to 22, and
>>>> we can either ignore all the capabilities we have now and start with
>>>> secid reconciliation or actually factor in all the capabilities we have
>>>> now into it. It will be easier short term to do the former, but possibly
>>>> better in the long term to do the latter.
>>>>         
>>> I'm not familiar enough with that code to have a strong opinion either
>>> way, although I would think that if you are going to introduce a new
>>> policy version to support this you might as well go all the way and put
>>> all the features into the bitmap.
>>>
>>> Also, don't worry too much about getting them done right away, there is
>>> plenty of work still left to do with the network bits before this issue
>>> becomes a roadblock.  Where would you put the capability bitmap in the
>>> policy?  After the version/config/table-sizes (where I put the ill-fated
>>> version number)?
>>>       
>> Yes, that is probably the best place, though just sticking it somewhere
>> random every time would be much more exciting...
>>     
>
> Oh yeah, we could even shift the bitmap X number of bits based on the checksum 
> of the policy!  So much fun!
>
> I really need to find something else to occupy my time :)
>   

So.. how are we going to determine if the policy is capable of doing 
peersid reconciliation? Is this going to be a user flag or is there an 
object class it can depend on?


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [RFC PATCH 0/2] Series short description
  2007-09-26 20:39                     ` Joshua Brindle
@ 2007-09-26 20:46                       ` Paul Moore
  0 siblings, 0 replies; 28+ messages in thread
From: Paul Moore @ 2007-09-26 20:46 UTC (permalink / raw)
  To: Joshua Brindle; +Cc: Stephen Smalley, James Morris, selinux

On Wednesday 26 September 2007 4:39:29 pm Joshua Brindle wrote:
> So.. how are we going to determine if the policy is capable of doing
> peersid reconciliation? Is this going to be a user flag or is there an
> object class it can depend on?

In this case we can probably key off the existence of the yet to be 
defined "peer" class.  Although we might want/need a more general way of 
setting the capability bitmap in the future.

-- 
paul moore
linux security @ hp

--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

^ permalink raw reply	[flat|nested] 28+ messages in thread

end of thread, other threads:[~2007-09-26 20:46 UTC | newest]

Thread overview: 28+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2007-09-25 20:48 [RFC PATCH 0/2] Series short description Paul Moore
2007-09-25 20:48 ` [RFC PATCH 1/2] [SELINUX] Add a functionality version number Paul Moore
2007-09-25 21:12   ` Eric Paris
2007-09-25 21:16     ` Paul Moore
2007-09-25 20:48 ` [RFC PATCH 2/2] [SELINUX] Better integration between peer labeling subsystems Paul Moore
2007-09-25 21:37   ` Eric Paris
2007-09-25 22:01     ` Paul Moore
2007-09-25 22:38   ` James Morris
2007-09-25 22:48     ` Paul Moore
2007-09-26 12:41   ` Stephen Smalley
2007-09-26 15:46     ` Paul Moore
2007-09-26 16:18       ` Paul Moore
2007-09-25 22:28 ` [RFC PATCH 0/2] Series short description James Morris
2007-09-25 22:38   ` Paul Moore
2007-09-26  2:19     ` Joshua Brindle
2007-09-26  3:12       ` Paul Moore
2007-09-26 13:18         ` Joshua Brindle
2007-09-26 13:29         ` Stephen Smalley
2007-09-26 16:00           ` Paul Moore
2007-09-26 16:43             ` Joshua Brindle
2007-09-26 16:48               ` Stephen Smalley
2007-09-26 16:54               ` Paul Moore
2007-09-26 16:57                 ` Joshua Brindle
2007-09-26 17:04                   ` Paul Moore
2007-09-26 20:39                     ` Joshua Brindle
2007-09-26 20:46                       ` Paul Moore
2007-09-26 20:36           ` Joshua Brindle
2007-09-26 20:32             ` Stephen Smalley

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.