[PATCH 7/7] secid reconciliation-v02: Enforcement for SELinux

[PATCH 7/7] secid reconciliation-v02: Enforcement for SELinux

[Venkat Yekkirala]

This defines SELinux enforcement of the 2 new LSM hooks.

Signed-off-by: Venkat Yekkirala <vyekkirala@TrustedCS.com>
---
 security/selinux/hooks.c        |  125 ++++++++++++++++++++++++------
 security/selinux/include/xfrm.h |    5 +
 security/selinux/ss/mls.c       |    2 
 security/selinux/ss/services.c  |    2 
 security/selinux/xfrm.c         |   28 ++++++
 5 files changed, 136 insertions(+), 26 deletions(-)

diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 5a66c4c..044e452 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -3449,8 +3449,12 @@ static int selinux_sock_rcv_skb_compat(s
 
 		err = avc_has_perm(sock_sid, port_sid,
 				   sock_class, recv_perm, ad);
+		if (err)
+			goto out;
 	}
 
+	err = selinux_xfrm_sock_rcv_skb(sock_sid, skb, ad);
+
 out:
 	return err;
 }
@@ -3489,10 +3493,6 @@ static int selinux_socket_sock_rcv_skb(s
 		goto out;
 
 	err = selinux_netlbl_sock_rcv_skb(sksec, skb, &ad);
-	if (err)
-		goto out;
-
-	err = selinux_xfrm_sock_rcv_skb(sksec->sid, skb, &ad);
 out:	
 	return err;
 }
@@ -3626,13 +3626,16 @@ static int selinux_inet_conn_request(str
 		return 0;
 	}
 
-	err = selinux_xfrm_decode_session(skb, &peersid, 0);
-	BUG_ON(err);
+	if (selinux_compat_net) {
+		err = selinux_xfrm_decode_session(skb, &peersid, 0);
+		BUG_ON(err);
 
-	if (peersid == SECSID_NULL) {
-		req->secid = sksec->sid;
-		return 0;
-	}
+		if (peersid == SECSID_NULL) {
+			req->secid = sksec->sid;
+			return 0;
+		}
+	} else
+		peersid = skb->secmark;
 
 	err = security_sid_mls_copy(sksec->sid, peersid, &newsid);
 	if (err)
@@ -3662,6 +3665,78 @@ static void selinux_req_classify_flow(co
 	fl->secid = req->secid;
 }
 
+static int selinux_skb_policy_check(struct sk_buff *skb, unsigned short family)
+{
+	u32 xfrm_sid, trans_sid;
+	int err;
+
+	if (selinux_compat_net)
+		return 1;
+
+	err = selinux_xfrm_decode_session(skb, &xfrm_sid, 0);
+	BUG_ON(err);
+
+	err = avc_has_perm(xfrm_sid, skb->secmark, SECCLASS_PACKET,
+					PACKET__FLOW_IN, NULL);
+	if (err)
+		goto out;
+
+	if (xfrm_sid) {
+		err = security_transition_sid(xfrm_sid, skb->secmark,
+						SECCLASS_PACKET, &trans_sid);
+		if (err)
+			goto out;
+
+		skb->secmark = trans_sid;
+	}
+
+	/* See if CIPSO can flow in thru the current secmark here */
+
+out:
+	return err ? 0 : 1;
+};
+
+static int selinux_skb_netfilter_check(struct sk_buff *skb, u32 nf_secid)
+{
+	u32 xfrm_sid;
+	u32 trans_sid;
+	int err;
+
+	if (selinux_compat_net)
+		return 1;
+
+	if (!skb->secmark && skb->sk) {
+		struct sk_security_struct *sksec = skb->sk->sk_security;
+		skb->secmark = sksec->sid;
+	}
+
+	selinux_skb_xfrm_sid(skb, &xfrm_sid);
+
+	err = avc_has_perm(skb->secmark, xfrm_sid, SECCLASS_PACKET,
+				PACKET__FLOW_OUT, NULL);
+
+	if (err)
+		goto out;
+
+	if (xfrm_sid) {
+		err = security_transition_sid(xfrm_sid, skb->secmark,
+						SECCLASS_PACKET, &trans_sid);
+		if (err)
+			goto out;
+
+		skb->secmark = trans_sid;
+	}
+
+	err = avc_has_perm(skb->secmark, nf_secid, SECCLASS_PACKET,
+				PACKET__FLOW_OUT, NULL);
+
+out:
+	/* Signal postroute_last that we are done with this skb */
+	skb->secmark = SECSID_WILD;
+
+	return err ? 0 : 1;
+}
+
 static int selinux_nlmsg_perm(struct sock *sk, struct sk_buff *skb)
 {
 	int err = 0;
@@ -3700,7 +3775,8 @@ out:
 
 #ifdef CONFIG_NETFILTER
 
-static int selinux_ip_postroute_last_compat(struct sock *sk, struct net_device *dev,
+static int selinux_ip_postroute_last_compat(struct sock *sk, struct sk_buff *skb,
+					    struct net_device *dev,
 					    struct avc_audit_data *ad,
 					    u16 family, char *addrp, int len)
 {
@@ -3710,6 +3786,9 @@ static int selinux_ip_postroute_last_com
 	struct inode *inode;
 	struct inode_security_struct *isec;
 
+	if (!sk)
+		goto out;
+
 	sock = sk->sk_socket;
 	if (!sock)
 		goto out;
@@ -3768,7 +3847,11 @@ static int selinux_ip_postroute_last_com
 
 		err = avc_has_perm(isec->sid, port_sid, isec->sclass,
 				   send_perm, ad);
+		if (err)
+			goto out;
 	}
+
+	err = selinux_xfrm_postroute_last(isec->sid, skb, ad);
 out:
 	return err;
 }
@@ -3782,18 +3865,13 @@ static unsigned int selinux_ip_postroute
 {
 	char *addrp;
 	int len, err = 0;
-	struct sock *sk;
 	struct sk_buff *skb = *pskb;
 	struct avc_audit_data ad;
 	struct net_device *dev = (struct net_device *)out;
-	struct sk_security_struct *sksec;
 
-	sk = skb->sk;
-	if (!sk)
+	if (!selinux_compat_net && skb->secmark == SECSID_WILD)
 		goto out;
 
-	sksec = sk->sk_security;
-
 	AVC_AUDIT_DATA_INIT(&ad, NET);
 	ad.u.net.netif = dev->name;
 	ad.u.net.family = family;
@@ -3803,16 +3881,11 @@ static unsigned int selinux_ip_postroute
 		goto out;
 
 	if (selinux_compat_net)
-		err = selinux_ip_postroute_last_compat(sk, dev, &ad,
+		err = selinux_ip_postroute_last_compat(skb->sk, skb, dev, &ad,
 						       family, addrp, len);
 	else
-		err = avc_has_perm(sksec->sid, skb->secmark, SECCLASS_PACKET,
-				   PACKET__SEND, &ad);
-
-	if (err)
-		goto out;
-
-	err = selinux_xfrm_postroute_last(sksec->sid, skb, &ad);
+		err = avc_has_perm(skb->secmark, SECINITSID_UNLABELED,
+				   SECCLASS_PACKET, PACKET__FLOW_OUT, &ad);
 out:
 	return err ? NF_DROP : NF_ACCEPT;
 }
@@ -4719,6 +4792,8 @@ static struct security_operations selinu
 	.inet_conn_request =		selinux_inet_conn_request,
 	.inet_csk_clone =		selinux_inet_csk_clone,
 	.req_classify_flow =		selinux_req_classify_flow,
+	.skb_policy_check =		selinux_skb_policy_check,
+	.skb_netfilter_check =		selinux_skb_netfilter_check,
 
 #ifdef CONFIG_SECURITY_NETWORK_XFRM
 	.xfrm_policy_alloc_security =	selinux_xfrm_policy_alloc,
diff --git a/security/selinux/include/xfrm.h b/security/selinux/include/xfrm.h
index 81eb598..ed07f92 100644
--- a/security/selinux/include/xfrm.h
+++ b/security/selinux/include/xfrm.h
@@ -41,6 +41,7 @@ int selinux_xfrm_postroute_last(u32 isec
 u32 selinux_socket_getpeer_stream(struct sock *sk);
 u32 selinux_socket_getpeer_dgram(struct sk_buff *skb);
 int selinux_xfrm_decode_session(struct sk_buff *skb, u32 *sid, int ckall);
+void selinux_skb_xfrm_sid(struct sk_buff *skb, u32 *sid);
 #else
 static inline int selinux_xfrm_sock_rcv_skb(u32 isec_sid, struct sk_buff *skb,
 			struct avc_audit_data *ad)
@@ -68,6 +69,10 @@ static inline int selinux_xfrm_decode_se
 	*sid = SECSID_NULL;
 	return 0;
 }
+static inline void selinux_skb_xfrm_sid(struct sk_buff *skb, u32 *sid)
+{
+	*sid = SECSID_NULL;
+}
 #endif
 
 #endif /* _SELINUX_XFRM_H_ */
diff --git a/security/selinux/ss/mls.c b/security/selinux/ss/mls.c
index 119bd60..c551def 100644
--- a/security/selinux/ss/mls.c
+++ b/security/selinux/ss/mls.c
@@ -548,6 +548,8 @@ int mls_compute_sid(struct context *scon
 				}
 			}
 		}
+		else if (tclass == SECCLASS_PACKET)
+			return mls_copy_context(newcontext, scontext);
 		/* Fallthrough */
 	case AVTAB_CHANGE:
 		if (tclass == SECCLASS_PROCESS)
diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c
index 27ee28c..f9cd484 100644
--- a/security/selinux/ss/services.c
+++ b/security/selinux/ss/services.c
@@ -832,6 +832,7 @@ static int security_compute_sid(u32 ssid
 
 	if (!ss_initialized) {
 		switch (tclass) {
+		case SECCLASS_PACKET:
 		case SECCLASS_PROCESS:
 			*out_sid = ssid;
 			break;
@@ -876,6 +877,7 @@ static int security_compute_sid(u32 ssid
 
 	/* Set the role and type to default values. */
 	switch (tclass) {
+	case SECCLASS_PACKET:
 	case SECCLASS_PROCESS:
 		/* Use the current role and type of process. */
 		newcontext.role = scontext->role;
diff --git a/security/selinux/xfrm.c b/security/selinux/xfrm.c
index 3e742b8..3a68723 100644
--- a/security/selinux/xfrm.c
+++ b/security/selinux/xfrm.c
@@ -155,7 +155,7 @@ int selinux_xfrm_flow_state_match(struct
 }
 
 /*
- * LSM hook implementation that determines the sid for the session.
+ * LSM hook implementation that checks/returns the xfrm sid for the incoming packet.
  */
 
 int selinux_xfrm_decode_session(struct sk_buff *skb, u32 *sid, int ckall)
@@ -193,6 +193,32 @@ int selinux_xfrm_decode_session(struct s
 }
 
 /*
+ * LSM hook implementation that returns the xfrm sid for the outgoing packet.
+ */
+
+void selinux_skb_xfrm_sid(struct sk_buff *skb, u32 *sid)
+{
+	struct dst_entry *dst;
+
+	*sid = SECSID_NULL;
+
+	dst = skb->dst;
+	if (dst) {
+		struct dst_entry *dst_test;
+		for (dst_test = dst; dst_test != 0;
+			dst_test = dst_test->child) {
+			struct xfrm_state *x = dst_test->xfrm;
+
+			if (x && selinux_authorizable_xfrm(x)) {
+				struct xfrm_sec_ctx *ctx = x->security;
+				*sid = ctx->ctx_sid;
+				break;
+			}
+		}
+	}
+}
+
+/*
  * Security blob allocation for xfrm_policy and xfrm_state
  * CTX does not have a meaningful value on input
  */

Re: [PATCH 7/7] secid reconciliation-v02: Enforcement for SELinux

[James Morris]

On Fri, 8 Sep 2006, Venkat Yekkirala wrote:

> +	if (selinux_compat_net) {
> +		err = selinux_xfrm_decode_session(skb, &peersid, 0);
> +		BUG_ON(err);

I'm pretty sure this should not be a BUG_ON.  IIUC, you want to panic the 
kernel because one of the nested SAs has a different security context.

> +	err = selinux_xfrm_decode_session(skb, &xfrm_sid, 0);
> +	BUG_ON(err);

Same.


-- 
James Morris
<jmorris@namei.org>

Re: [PATCH 7/7] secid reconciliation-v02: Enforcement for SELinux

[James Morris]

On Fri, 8 Sep 2006, Venkat Yekkirala wrote:

> This defines SELinux enforcement of the 2 new LSM hooks.
> 

I think this looks ok in general (I have a couple more technical issues), 
athough I believe that Stephen has some question about policy 
construction.

Please rename these hooks:

	+ * @skb_policy_check:
	+ * @skb_netfilter_check

to:

	skb_flow_in
	skb_flow_out



- James
-- 
James Morris
<jmorris@namei.org>

RE: [PATCH 7/7] secid reconciliation-v02: Enforcement for SELinux

[Venkat Yekkirala]

> On Fri, 8 Sep 2006, Venkat Yekkirala wrote:
> 
> > +	if (selinux_compat_net) {
> > +		err = selinux_xfrm_decode_session(skb, &peersid, 0);
> > +		BUG_ON(err);
> 
> I'm pretty sure this should not be a BUG_ON.  IIUC, you want 
> to panic the 
> kernel because one of the nested SAs has a different security context.

No, we are sending in 0 for the ckall param by which we are telling
the function NOT to do any checks, but to simply set the return param
peersid to the secid on the first xfrm if any and succeed by returning 0.
Must not fail.

Re: [PATCH 7/7] secid reconciliation-v02: Enforcement for SELinux

[Paul Moore]

On Friday 08 September 2006 12:50 pm, Venkat Yekkirala wrote:
> This defines SELinux enforcement of the 2 new LSM hooks.

{snip}

> +static int selinux_skb_policy_check(struct sk_buff *skb, unsigned short
> family) +{
> +	u32 xfrm_sid, trans_sid;
> +	int err;
> +
> +	if (selinux_compat_net)
> +		return 1;
> +
> +	err = selinux_xfrm_decode_session(skb, &xfrm_sid, 0);
> +	BUG_ON(err);

First, any reason against including the "struct sock *" in the LSM hook?  At a 
quick glance it looks like it is available at each place 
security_skb_policy_check() is invoked?  If there are no objections I would 
like to see it included in the hook.

Second, I wonder if it would be better to do a NetLabel/CIPSO query here using 
the xfrm_sid as the NetLabel "base_sid" instead of at the end of the function 
(see your comment)?  This way we wouldn't have to duplicate the 
avc_has_perm() and security_transition_sid() calls for both xfrm and 
NetLabel.  It just seems to be more inline with the whole secid 
reconciliation concept.

I don't feel too strongly either way, I just thought it was worth exploring - 
thoughts?

> +	err = avc_has_perm(xfrm_sid, skb->secmark, SECCLASS_PACKET,
> +					PACKET__FLOW_IN, NULL);
> +	if (err)
> +		goto out;
> +
> +	if (xfrm_sid) {
> +		err = security_transition_sid(xfrm_sid, skb->secmark,
> +						SECCLASS_PACKET, &trans_sid);
> +		if (err)
> +			goto out;
> +
> +		skb->secmark = trans_sid;
> +	}
> +
> +	/* See if CIPSO can flow in thru the current secmark here */
> +
> +out:
> +	return err ? 0 : 1;
> +};

-- 
paul moore
linux security @ hp

RE: [PATCH 7/7] secid reconciliation-v02: Enforcement for SELinux

[Venkat Yekkirala]

> > +static int selinux_skb_policy_check(struct sk_buff *skb, 
> unsigned short
> > family) +{
> > +	u32 xfrm_sid, trans_sid;
> > +	int err;
> > +
> > +	if (selinux_compat_net)
> > +		return 1;
> > +
> > +	err = selinux_xfrm_decode_session(skb, &xfrm_sid, 0);
> > +	BUG_ON(err);
> 
> First, any reason against including the "struct sock *" in 
> the LSM hook?  At a 
> quick glance it looks like it is available at each place 
> security_skb_policy_check() is invoked?  If there are no 
> objections I would 
> like to see it included in the hook.

There's no sock available (NULL) for forward, no-sock, time-wait cases, etc.

What you are trying to accomplish with the sock here anyway?

> 
> Second, I wonder if it would be better to do a NetLabel/CIPSO 
> query here using 
> the xfrm_sid as the NetLabel "base_sid" instead of at the end 
> of the function 
> (see your comment)?  This way we wouldn't have to duplicate the 
> avc_has_perm() and security_transition_sid() calls for both xfrm and 
> NetLabel.

There's a need for an additional avc_has_perm check anyway between
the cipso label and the ipsec/transition label, to check to make sure
the cipso level falls within the range on the IPSec/transition SA.

No need for a new transition between ipsec/transition label and the cipso
label since the cipso label would be sharing the TE portion with the
ipsec/transition label (this could change in the future, when you get round
to doing entire SELinux contexts over the wire). For now, you would just
set the secmark to the cipso label if the label could come thru
(i.e. if the avc_has_perm succeeds).

Re: [PATCH 7/7] secid reconciliation-v02: Enforcement for SELinux

[Paul Moore]

Venkat Yekkirala wrote:
>>>+static int selinux_skb_policy_check(struct sk_buff *skb, 
>>
>>unsigned short
>>
>>>family) +{
>>>+	u32 xfrm_sid, trans_sid;
>>>+	int err;
>>>+
>>>+	if (selinux_compat_net)
>>>+		return 1;
>>>+
>>>+	err = selinux_xfrm_decode_session(skb, &xfrm_sid, 0);
>>>+	BUG_ON(err);
>>
>>First, any reason against including the "struct sock *" in 
>>the LSM hook?  At a 
>>quick glance it looks like it is available at each place 
>>security_skb_policy_check() is invoked?  If there are no 
>>objections I would 
>>like to see it included in the hook.
>  
> There's no sock available (NULL) for forward, no-sock, time-wait cases, etc.

... which would be why I should have taken a closer look :)

> What you are trying to accomplish with the sock here anyway?

Actually this is no longer an issue because of something else - you can
ignore this now.

-- 
paul moore
linux security @ hp