Skip to contentSkip to conversation

secid reconciliation-v03: Enforcement for SELinux

PATCH In review Venkat Yekkirala 5 participants 11 messages Sep 29, 2006

Subsystems (3)
lore
Share & tools
lore
#0 lore raw

PATCH 7/7 secid reconciliation-v03: Enforcement for SELinux

This defines SELinux enforcement of the 2 new LSM hooks as well as related changes elsewhere in the SELinux code.

---
 security/selinux/hooks.c        |  129 +++++++++++++++++++++++-------
 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, 139 insertions(+), 27 deletions(-)
security/selinux/hooks.c +103 -26
index 5a66c4c..143b4b8 100644
@@ -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,69 @@ static void selinux_req_classify_flow(co
 	fl->secid = req->secid;
 }
 
+static int selinux_skb_flow_in(struct sk_buff *skb, unsigned short family)
+{
+	u32 xfrm_sid, trans_sid;
+	int err;
+
+	if (selinux_compat_net)
+		return 1;
+
+	/* xfrm/cipso inapplicable for loopback traffic */
+	if (skb->dev == &loopback_dev)
+		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_flow_out(struct sk_buff *skb, u32 nf_secid)
+{
+	u32 trans_sid;
+	int err;
+
+	if (selinux_compat_net)
+		return 1;
+
+	if (!skb->secmark) {
+		u32 xfrm_sid;
+
+		selinux_skb_xfrm_sid(skb, &xfrm_sid);
+
+		if (xfrm_sid)
+			skb->secmark = xfrm_sid;
+		else if (skb->sk) {
+			struct sk_security_struct *sksec = skb->sk->sk_security;
+			skb->secmark = sksec->sid;
+		}
+	}
+
+	err = avc_has_perm(skb->secmark, nf_secid, SECCLASS_PACKET,
+				PACKET__FLOW_OUT, NULL);
+
+out:
+	return err ? 0 : 1;
+}
+
 static int selinux_nlmsg_perm(struct sock *sk, struct sk_buff *skb)
 {
 	int err = 0;
@@ -3700,7 +3766,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 +3777,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 +3838,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,17 +3856,9 @@ 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)
-		goto out;
-
-	sksec = sk->sk_security;
 
 	AVC_AUDIT_DATA_INIT(&ad, NET);
 	ad.u.net.netif = dev->name;
@@ -3803,16 +3869,25 @@ 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);
+	else {
+		if (!skb->secmark) {
+			u32 xfrm_sid;
 
-	if (err)
-		goto out;
+			selinux_skb_xfrm_sid(skb, &xfrm_sid);
 
-	err = selinux_xfrm_postroute_last(sksec->sid, skb, &ad);
+			if (xfrm_sid)
+				skb->secmark = xfrm_sid;
+			else if (skb->sk) {
+				struct sk_security_struct *sksec =
+						skb->sk->sk_security;
+				skb->secmark = sksec->sid;
+			}
+		}
+		err = avc_has_perm(skb->secmark, SECINITSID_UNLABELED,
+				   SECCLASS_PACKET, PACKET__FLOW_OUT, &ad);
+	}
 out:
 	return err ? NF_DROP : NF_ACCEPT;
 }
@@ -4719,6 +4794,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_flow_in =			selinux_skb_flow_in,
+	.skb_flow_out =			selinux_skb_flow_out,
 
 #ifdef CONFIG_SECURITY_NETWORK_XFRM
 	.xfrm_policy_alloc_security =	selinux_xfrm_policy_alloc,
security/selinux/include/xfrm.h +5 -0
index 81eb598..ed07f92 100644
@@ -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_ */
security/selinux/ss/mls.c +2 -0
index 119bd60..c551def 100644
@@ -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)
security/selinux/ss/services.c +2 -0
index 22ed17c..26176c2 100644
@@ -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;
security/selinux/xfrm.c +27 -1
index 3e742b8..3a68723 100644
@@ -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
  */

--
This message was distributed to subscribers of the selinux mailing list. If you no longer wish to subscribe, send mail to [email protected] with the words "unsubscribe selinux" without quotes as the message.

#1 lore raw
Quoted 4 lines · Venkat Yekkirala

<snip>

security/selinux/hooks.c +2 -0
+
+	err = avc_has_perm(xfrm_sid, skb->secmark, SECCLASS_PACKET,

security/selinux/hooks.c static void selinux_req_classify_flow(co · line 3693

Quoted 10 lines
security/selinux/hooks.c +10 -0
+					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;
+

I thought we weren't doing transitions to label packets anymore per the conference call?

--
This message was distributed to subscribers of the selinux mailing list. If you no longer wish to subscribe, send mail to [email protected] with the words "unsubscribe selinux" without quotes as the message.

#2 lore raw
Quoted 17 lines · Joshua Brindle
Quoted 4 lines · Venkat Yekkirala

<snip>

security/selinux/hooks.c +2 -0
+
+	err = avc_has_perm(xfrm_sid, skb->secmark, SECCLASS_PACKET,

security/selinux/hooks.c static void selinux_req_classify_flow(co · line 3693

Quoted 10 lines
security/selinux/hooks.c +10 -0
+					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;
+

I thought we weren't doing transitions to label packets anymore per the conference call?

No, transitions are still part of the reconciliation process. By default, this just means that we end up with the xfrm_sid (which is what you want). But it allows us the freedom to define transitions on the secmark label if desired, and those transitions can still yield subject labels.

Signature · 2 lines
Stephen Smalley
National Security Agency
#3 lore raw
Quoted 19 lines · Stephen Smalley
Quoted 17 lines · Joshua Brindle
Quoted 4 lines · Venkat Yekkirala

<snip>

security/selinux/hooks.c +2 -0
+
+	err = avc_has_perm(xfrm_sid, skb->secmark, SECCLASS_PACKET,

security/selinux/hooks.c static void selinux_req_classify_flow(co · line 3693

Quoted 10 lines
security/selinux/hooks.c +10 -0
+					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;
+

I thought we weren't doing transitions to label packets anymore per the conference call?

Quoted 6 lines

No, transitions are still part of the reconciliation process. By default, this just means that we end up with the xfrm_sid (which is what you want). But it allows us the freedom to define transitions on the secmark label if desired, and those transitions can still yield subject labels.

This is not consistent with my perception of the decision made in the conference call. I thought that the secid was either going to be 1) the secmark label if no external labeling is present or 2) the external label if it is present. The flow_in permission would be checked between the external label and the secmark label in either case (unlabeled in the case of #1)

How is this different from the implementation before the call?

--
This message was distributed to subscribers of the selinux mailing list. If you no longer wish to subscribe, send mail to [email protected] with the words "unsubscribe selinux" without quotes as the message.

#4 lore raw
Quoted 27 lines · Joshua Brindle
Quoted 19 lines · Stephen Smalley
Quoted 17 lines · Joshua Brindle
Quoted 4 lines · Venkat Yekkirala

<snip>

security/selinux/hooks.c +2 -0
+
+	err = avc_has_perm(xfrm_sid, skb->secmark, SECCLASS_PACKET,

security/selinux/hooks.c static void selinux_req_classify_flow(co · line 3693

Quoted 10 lines
security/selinux/hooks.c +10 -0
+					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;
+

I thought we weren't doing transitions to label packets anymore per the conference call?

Quoted 6 lines

No, transitions are still part of the reconciliation process. By default, this just means that we end up with the xfrm_sid (which is what you want). But it allows us the freedom to define transitions on the secmark label if desired, and those transitions can still yield subject labels.

Quoted 6 lines

This is not consistent with my perception of the decision made in the conference call. I thought that the secid was either going to be 1) the secmark label if no external labeling is present or 2) the external label if it is present. The flow_in permission would be checked between the external label and the secmark label in either case (unlabeled in the case of #1)

The behavior you describe is precisely what will happen in the absence of any type_transition rules on packet class in the policy, given the way in which he has defined security_transition_sid on packet class. The only question is whether there is any value in allowing a transition to be configured in policy (and if such a transition is configured, whether the resulting SID requires its own separate permission check, which is what is usually done - the transition doesn't implicitly authorize anything). I don't recall a particular decision on the transition issue during the call nor do I see it in the posted notes. However, since the transition was removed in the flow_out case, it would be logical to remove it from the flow_in case as well, and that would have the side benefit of less overhead.

Signature · 2 lines
Stephen Smalley
National Security Agency
#5 lore raw
Quoted 3 lines · Stephen Smalley

However, since the transition was removed in the flow_out case, it would be logical to remove it from the flow_in case as well, and that would have the side benefit of less overhead.

How about adding secmark transitions later, if needed, perhaps with an /selinux config control ?

It does keep things simpler for now, in terms of getting this code merged, deployed into distros and likely certified.

- James

Signature · 2 lines
James Morris
<[email protected]>
#6 lore raw
Quoted 11 lines · James Morris
Quoted 3 lines · Stephen Smalley

However, since the transition was removed in the flow_out case, it would be logical to remove it from the flow_in case as well, and that would have the side benefit of less overhead.

How about adding secmark transitions later, if needed, perhaps with an /selinux config control ?

It does keep things simpler for now, in terms of getting this code merged, deployed into distros and likely certified.

Fine with me, unless Venkat has an immediate use case for such transitions in the flow_in case (but I think this is mostly my fault for suggesting transitions a while ago).

Signature · 2 lines
Stephen Smalley
National Security Agency
#7 lore raw
Quoted 12 lines · Stephen Smalley
Quoted 10 lines · James Morris
Quoted 3 lines · Stephen Smalley

However, since the transition was removed in the flow_out case, it would be logical to remove it from the flow_in case as well, and that would have the side benefit of less overhead.

How about adding secmark transitions later, if needed, perhaps with an /selinux config control ?

Quoted 7 lines
Quoted 2 lines

It does keep things simpler for now, in terms of getting this code merged, deployed into distros and likely certified.

Fine with me, unless Venkat has an immediate use case for such transitions in the flow_in case (but I think this is mostly my fault for suggesting transitions a while ago).

Unless I'm confusing something, there still may be a need for transitions if we want to support both IPsec and NetLabel labeling on the same connection.
If we don't support transitions and allow both labeling methods on the same connection we'll need to decide how to handle resolving the two - maybe use a transition is this one case?

Signature · 7 lines
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 [email protected] with
the words "unsubscribe selinux" without quotes as the message.
#8 lore raw
Quoted 3 lines · Paul Moore

Unless I'm confusing something, there still may be a need for transitions if we want to support both IPsec and NetLabel labeling on the same connection.

I'd prefer not to support this, as it's too complicated, and CIPSO is a legacy protocol.

Normal IPsec protection applied to CIPSO: yes, but not IPsec labeling and CIPSO labeling on the same connection.

- James

Signature · 2 lines
James Morris
<[email protected]>
#9 lore raw
Quoted 7 lines · James Morris
Quoted 3 lines · Paul Moore

Unless I'm confusing something, there still may be a need for transitions if we want to support both IPsec and NetLabel labeling on the same connection.

Quoted 5 lines

I'd prefer not to support this, as it's too complicated, and CIPSO is a legacy protocol.

Normal IPsec protection applied to CIPSO: yes, but not IPsec labeling and CIPSO labeling on the same connection.

I tend to agree, I just can't see it being all that useful in the real world. However, each time it comes up (including the conference call earlier this week) it seems that people would prefer to use both at the same time.

The good news is that it sounds like there is a reasonable solution (see the last email exchance between Venkat and myself).

Signature · 7 lines
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 [email protected] with
the words "unsubscribe selinux" without quotes as the message.
#10 lore raw
In reply to Venkat Yekkirala
Quoted 2 lines · Venkat Yekkirala

This defines SELinux enforcement of the 2 new LSM hooks as well as related changes elsewhere in the SELinux code.

As soon as I hit send on this mail I'll start working on the related patch to provide the missing NetLabel support.

Additional comments are below.

Quoted 65 lines
---
 security/selinux/hooks.c        |  129 +++++++++++++++++++++++-------
 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, 139 insertions(+), 27 deletions(-)
security/selinux/hooks.c +14 -10
index 5a66c4c..143b4b8 100644
@@ -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,69 @@ static void selinux_req_classify_flow(co
 	fl->secid = req->secid;
 }
 
+static int selinux_skb_flow_in(struct sk_buff *skb, unsigned short family)

security/selinux/hooks.c static void selinux_req_classify_flow(co · line 3678

Quoted 10 lines
security/selinux/hooks.c +10 -0
+{
+	u32 xfrm_sid, trans_sid;
+	int err;
+
+	if (selinux_compat_net)
+		return 1;
+
+	/* xfrm/cipso inapplicable for loopback traffic */
+	if (skb->dev == &loopback_dev)
+		return 1;

Just to clarify (I believe this is the case from your remarks as well as the code, but better safe than sorry) the secmark for loopback traffic is set by the originating socket, yes?

Beware: a bit of a nit follows :)

While I understand that NetLabel currently only supports CIPSO, it is a framework that can support multiple labeling procotols (i.e. RIPSO support is planned). Please change the comment from cipso/CIPSO to netlabel/NetLabel so it is consistent with the rest of the code which makes use of NetLabel.

Quoted 7 lines
security/selinux/hooks.c +7 -0
+	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;

security/selinux/hooks.c static void selinux_req_classify_flow(co · line 3697

Quoted 10 lines
security/selinux/hooks.c +10 -0
+
+	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 */

Same nit applies about the use of "CIPSO" vs "NetLabel".

Quoted 17 lines
security/selinux/hooks.c +17 -0
+out:
+	return err ? 0 : 1;
+};
+
+static int selinux_skb_flow_out(struct sk_buff *skb, u32 nf_secid)
+{
+	u32 trans_sid;
+	int err;
+
+	if (selinux_compat_net)
+		return 1;
+
+	if (!skb->secmark) {
+		u32 xfrm_sid;
+
+		selinux_skb_xfrm_sid(skb, &xfrm_sid);
+

security/selinux/hooks.c static void selinux_req_classify_flow(co · line 3725

Quoted 10 lines
security/selinux/hooks.c +10 -0
+		if (xfrm_sid)
+			skb->secmark = xfrm_sid;
+		else if (skb->sk) {
+			struct sk_security_struct *sksec = skb->sk->sk_security;
+			skb->secmark = sksec->sid;
+		}
+	}
+
+	err = avc_has_perm(skb->secmark, nf_secid, SECCLASS_PACKET,
+				PACKET__FLOW_OUT, NULL);

While I don't see any explicit mention of it in the documentation or your comments, I assume we would want a flow_out check for NetLabel here as well?

Quoted 77 lines
security/selinux/hooks.c +19 -16
+out:
+	return err ? 0 : 1;
+}
+
 static int selinux_nlmsg_perm(struct sock *sk, struct sk_buff *skb)
 {
 	int err = 0;
@@ -3700,7 +3766,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 +3777,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 +3838,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,17 +3856,9 @@ 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)
-		goto out;
-
-	sksec = sk->sk_security;
 
 	AVC_AUDIT_DATA_INIT(&ad, NET);
 	ad.u.net.netif = dev->name;
@@ -3803,16 +3869,25 @@ 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);
+	else {
+		if (!skb->secmark) {
+			u32 xfrm_sid;
 
-	if (err)
-		goto out;
+			selinux_skb_xfrm_sid(skb, &xfrm_sid);
 
-	err = selinux_xfrm_postroute_last(sksec->sid, skb, &ad);
+			if (xfrm_sid)

security/selinux/hooks.c static unsigned int selinux_ip_postroute · line 3890

Quoted 10 lines
security/selinux/hooks.c +10 -0
+				skb->secmark = xfrm_sid;
+			else if (skb->sk) {
+				struct sk_security_struct *sksec =
+						skb->sk->sk_security;
+				skb->secmark = sksec->sid;
+			}
+		}
+		err = avc_has_perm(skb->secmark, SECINITSID_UNLABELED,
+				   SECCLASS_PACKET, PACKET__FLOW_OUT, &ad);
+	}

This looks nearly identical to selinux_skb_flow_out() as implemented above, the only real differences being two of the args to the avc_has_perm() call. Any reason you don't abstract out the common parts to a separate function?

Also, the same comment/question about NetLabel support in selinux_skb_flow_out() applies here as well I suspect.

Quoted 119 lines
security/selinux/hooks.c +2 -0
 out:
 	return err ? NF_DROP : NF_ACCEPT;
 }
@@ -4719,6 +4794,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_flow_in =			selinux_skb_flow_in,
+	.skb_flow_out =			selinux_skb_flow_out,
 
 #ifdef CONFIG_SECURITY_NETWORK_XFRM
 	.xfrm_policy_alloc_security =	selinux_xfrm_policy_alloc,
security/selinux/include/xfrm.h +5 -0
index 81eb598..ed07f92 100644
@@ -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_ */
security/selinux/ss/mls.c +2 -0
index 119bd60..c551def 100644
@@ -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)
security/selinux/ss/services.c +2 -0
index 22ed17c..26176c2 100644
@@ -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;
security/selinux/xfrm.c +27 -2
index 3e742b8..3a68723 100644
@@ -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
  */
 
--

This message was distributed to subscribers of the selinux mailing list. If you no longer wish to subscribe, send mail to [email protected] with the words "unsubscribe selinux" without quotes as the message.

Signature · 7 lines
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 [email protected] with
the words "unsubscribe selinux" without quotes as the message.