secid reconciliation-v02: Enforcement for SELinux

PATCH In review Venkat Yekkirala 3 participants 4 messages Sep 8, 2006 – Sep 18, 2006

Subsystems (3)

Networking Network drivers SELinux security module

lore

Share & tools

lore

Venkat Yekkirala

author

2006-09-08 16:50 UTC #0 lore raw

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

This defines SELinux enforcement of the 2 new LSM hooks.

Signed-off-by Venkat Yekkirala <[email protected]>

---

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(-)

security/selinux/hooks.c +0 -0

index 5a66c4c..044e452 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,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,

security/selinux/include/xfrm.h +0 -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 +0 -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 +0 -0

index 27ee28c..f9cd484 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 +0 -0

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.

James Morris

2006-09-18 18:37 UTC #1 lore raw

Review

Quoted 3 lines · Venkat Yekkirala

(quoted diff) +3 -0

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

Review

Quoted 2 lines

(quoted diff) +2 -0

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

Same.

Signature · 2 lines

James Morris
<[email protected]>

James Morris

2006-09-18 18:55 UTC #2 lore raw

In reply to Venkat Yekkirala

Quoted 2 lines · Venkat Yekkirala

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

Signature · 2 lines

James Morris
<[email protected]>

Paul Moore

2006-09-18 22:26 UTC #3 lore raw

In reply to Venkat Yekkirala

Quoted 1 line · Venkat Yekkirala

This defines SELinux enforcement of the 2 new LSM hooks.

{snip}

Review

Quoted 10 lines

+static int selinux_skb_policy_check(struct sk_buff *skb, unsigned short family) +{
(quoted diff) +8 -0
+	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?

Quoted 19 lines

(quoted diff) +19 -0

+	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;
+};

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.