[PATCH v4 1/2] NetLabel: secid reconciliation support

[PATCH v4 1/2] NetLabel: secid reconciliation support

[paul.moore]

[-- Attachment #1: netlabel-secid_support --]
[-- Type: text/plain, Size: 13804 bytes --]

This patch provides the missing NetLabel support to the secid reconciliation
patchset.

Signed-off-by: Paul Moore <paul.moore@hp.com>
---
 security/selinux/hooks.c                    |  104 +++++++++++++++++----------
 security/selinux/include/objsec.h           |    1 
 security/selinux/include/selinux_netlabel.h |   28 +++----
 security/selinux/ss/services.c              |  106 ++++++++++------------------
 4 files changed, 121 insertions(+), 118 deletions(-)

Index: net-2.6/security/selinux/hooks.c
===================================================================
--- net-2.6.orig/security/selinux/hooks.c
+++ net-2.6/security/selinux/hooks.c
@@ -3465,6 +3465,10 @@ static int selinux_sock_rcv_skb_compat(s
 			goto out;
 	}
 
+	err = selinux_netlbl_sock_rcv_skb(sock_sid, sock_class, skb, ad);
+	if (err)
+		goto out;
+
 	err = selinux_xfrm_sock_rcv_skb(sock_sid, skb, ad);
 
 out:
@@ -3501,10 +3505,7 @@ static int selinux_socket_sock_rcv_skb(s
 	else
 		err = avc_has_perm(sksec->sid, skb->secmark, SECCLASS_PACKET,
 				   PACKET__RECV, &ad);
-	if (err)
-		goto out;
 
-	err = selinux_netlbl_sock_rcv_skb(sksec, skb, &ad);
 out:	
 	return err;
 }
@@ -3527,11 +3528,8 @@ static int selinux_socket_getpeersec_str
 		peer_sid = ssec->peer_sid;
 	}
 	else if (isec->sclass == SECCLASS_TCP_SOCKET) {
-		peer_sid = selinux_netlbl_socket_getpeersec_stream(sock);
-		if (peer_sid == SECSID_NULL) {
-			ssec = sock->sk->sk_security;
-			peer_sid = ssec->peer_sid;
-		}
+		ssec = sock->sk->sk_security;
+		peer_sid = ssec->peer_sid;
 		if (peer_sid == SECSID_NULL) {
 			err = -ENOPROTOOPT;
 			goto out;
@@ -3573,13 +3571,13 @@ static int selinux_socket_getpeersec_dgr
 	if (sock && (sock->sk->sk_family == PF_UNIX))
 		selinux_get_inode_sid(SOCK_INODE(sock), &peer_secid);
 	else if (skb) {
-		peer_secid = selinux_netlbl_socket_getpeersec_dgram(skb);
-		if (peer_secid == SECSID_NULL) {
-			if (selinux_compat_net)
+		if (selinux_compat_net) {
+			peer_secid = selinux_netlbl_socket_getpeersec_dgram(
+				skb);
+			if (peer_secid == SECSID_NULL)
 				peer_secid = selinux_socket_getpeer_dgram(skb);
-			else
-				peer_secid = skb->secmark;
-		}
+		} else
+			peer_secid = skb->secmark;
 	}
 
 	if (peer_secid == SECSID_NULL)
@@ -3641,13 +3639,11 @@ static int selinux_inet_conn_request(str
 	u32 newsid;
 	u32 peersid;
 
-	newsid = selinux_netlbl_inet_conn_request(skb, sksec->sid);
-	if (newsid != SECSID_NULL) {
-		req->secid = newsid;
-		return 0;
-	}
-
 	if (selinux_compat_net) {
+		err = selinux_netlbl_skb_sid(skb, sksec->sid, &peersid);
+		if (err == 0 && peersid != SECSID_NULL)
+			goto out;
+
 		err = selinux_xfrm_decode_session(skb, &peersid, 0);
 		BUG_ON(err);
 
@@ -3659,6 +3655,7 @@ static int selinux_inet_conn_request(str
 	} else
 		peersid = skb->secmark;
 
+out:
 	err = security_sid_mls_copy(sksec->sid, peersid, &newsid);
 	if (err)
 		return err;
@@ -3700,6 +3697,9 @@ static void selinux_req_classify_flow(co
 static int selinux_skb_flow_in(struct sk_buff *skb, unsigned short family)
 {
 	u32 xfrm_sid;
+	u32 nlbl_sid;
+	u32 ext_sid;
+	u32 loc_sid;
 	int err;
 
 	if (selinux_compat_net)
@@ -3714,19 +3714,34 @@ static int selinux_skb_flow_in(struct sk
 	if (skb->dev == &loopback_dev)
 		return 1;
 
+	if (skb->secmark)
+		loc_sid = skb->secmark;
+	else
+		loc_sid = SECINITSID_NETMSG;
+
 	err = selinux_xfrm_decode_session(skb, &xfrm_sid, 0);
 	BUG_ON(err);
-
-	err = avc_has_perm(xfrm_sid, skb->secmark? : SECINITSID_NETMSG,
-					SECCLASS_PACKET,
-					PACKET__FLOW_IN, NULL);
+	err = selinux_netlbl_skb_sid(skb,
+				     xfrm_sid ? xfrm_sid : loc_sid,
+				     &nlbl_sid);
 	if (err)
 		goto out;
 
-	if (xfrm_sid)
-		skb->secmark = xfrm_sid;
+	if (nlbl_sid)
+		ext_sid = nlbl_sid;
+	else
+		ext_sid = xfrm_sid;
+
+	err = avc_has_perm(ext_sid,
+			   loc_sid,
+			   SECCLASS_PACKET,
+			   PACKET__FLOW_IN,
+			   NULL);
+	if (err)
+		goto out;
 
-	/* See if NetLabel can flow in thru the current secmark here */
+	if (ext_sid)
+		skb->secmark = ext_sid;
 
 out:
 	return err ? 0 : 1;
@@ -3740,21 +3755,29 @@ static int selinux_skb_flow_out(struct s
 		return 1;
 
 	if (!skb->secmark) {
+		struct sk_security_struct *sksec = skb->sk->sk_security;
 		u32 xfrm_sid;
+		u32 nlbl_sid;
 
 		selinux_skb_xfrm_sid(skb, &xfrm_sid);
+		err = selinux_netlbl_skb_sid(skb,
+					     xfrm_sid ? xfrm_sid : sksec->sid,
+					     &nlbl_sid);
+		if (err)
+			goto out;
 
-		if (xfrm_sid)
+		if (nlbl_sid)
+			skb->secmark = nlbl_sid;
+		else if (xfrm_sid)
 			skb->secmark = xfrm_sid;
-		else if (skb->sk) {
-			struct sk_security_struct *sksec = skb->sk->sk_security;
+		else if (skb->sk)
 			skb->secmark = sksec->sid;
-		}
 	}
 
 	err = avc_has_perm(skb->secmark, nf_secid, SECCLASS_PACKET,
 				PACKET__FLOW_OUT, NULL);
 
+out:
 	return err ? 0 : 1;
 }
 
@@ -3903,17 +3926,24 @@ static unsigned int selinux_ip_postroute
 						       family, addrp, len);
 	else {
 		if (!skb->secmark) {
+			struct sk_security_struct *sksec =
+				skb->sk->sk_security;
 			u32 xfrm_sid;
+			u32 nlbl_sid;
 
 			selinux_skb_xfrm_sid(skb, &xfrm_sid);
-
-			if (xfrm_sid)
+			err = selinux_netlbl_skb_sid(skb,
+					      xfrm_sid ? xfrm_sid : sksec->sid,
+					      &nlbl_sid);
+			if (err)
+				goto out;
+
+			if (nlbl_sid)
+				skb->secmark = nlbl_sid;
+			else if (xfrm_sid)
 				skb->secmark = xfrm_sid;
-			else if (skb->sk) {
-				struct sk_security_struct *sksec =
-						skb->sk->sk_security;
+			else if (skb->sk)
 				skb->secmark = sksec->sid;
-			}
 		}
 		err = avc_has_perm(skb->secmark, SECINITSID_NETMSG,
 				   SECCLASS_PACKET, PACKET__FLOW_OUT, &ad);
Index: net-2.6/security/selinux/include/objsec.h
===================================================================
--- net-2.6.orig/security/selinux/include/objsec.h
+++ net-2.6/security/selinux/include/objsec.h
@@ -102,7 +102,6 @@ struct sk_security_struct {
 	u32 sid;			/* SID of this object */
 	u32 peer_sid;			/* SID of peer */
 #ifdef CONFIG_NETLABEL
-	u16 sclass;			/* sock security class */
 	enum {				/* NetLabel state */
 		NLBL_UNSET = 0,
 		NLBL_REQUIRE,
Index: net-2.6/security/selinux/include/selinux_netlabel.h
===================================================================
--- net-2.6.orig/security/selinux/include/selinux_netlabel.h
+++ net-2.6/security/selinux/include/selinux_netlabel.h
@@ -42,17 +42,17 @@ int selinux_netlbl_socket_post_create(st
 				      int sock_family,
 				      u32 sid);
 void selinux_netlbl_sock_graft(struct sock *sk, struct socket *sock);
-u32 selinux_netlbl_inet_conn_request(struct sk_buff *skb, u32 sock_sid);
-int selinux_netlbl_sock_rcv_skb(struct sk_security_struct *sksec,
+int selinux_netlbl_sock_rcv_skb(u32 sock_sid,
+				u16 sock_class,
 				struct sk_buff *skb,
 				struct avc_audit_data *ad);
-u32 selinux_netlbl_socket_getpeersec_stream(struct socket *sock);
 u32 selinux_netlbl_socket_getpeersec_dgram(struct sk_buff *skb);
 void selinux_netlbl_sk_security_init(struct sk_security_struct *ssec,
 				     int family);
 void selinux_netlbl_sk_clone_security(struct sk_security_struct *ssec,
 				      struct sk_security_struct *newssec);
 int selinux_netlbl_inode_permission(struct inode *inode, int mask);
+int selinux_netlbl_skb_sid(struct sk_buff *skb, u32 base_sid, u32 *sid);
 #else
 static inline void selinux_netlbl_cache_invalidate(void)
 {
@@ -72,24 +72,14 @@ static inline void selinux_netlbl_sock_g
 	return;
 }
 
-static inline u32 selinux_netlbl_inet_conn_request(struct sk_buff *skb,
-						   u32 sock_sid)
-{
-	return SECSID_NULL;
-}
-
-static inline int selinux_netlbl_sock_rcv_skb(struct sk_security_struct *sksec,
+static inline int selinux_netlbl_sock_rcv_skb(u32 sock_sid,
+					      u16 sock_class,
 					      struct sk_buff *skb,
 					      struct avc_audit_data *ad)
 {
 	return 0;
 }
 
-static inline u32 selinux_netlbl_socket_getpeersec_stream(struct socket *sock)
-{
-	return SECSID_NULL;
-}
-
 static inline u32 selinux_netlbl_socket_getpeersec_dgram(struct sk_buff *skb)
 {
 	return SECSID_NULL;
@@ -114,6 +104,14 @@ static inline int selinux_netlbl_inode_p
 {
 	return 0;
 }
+
+static inline int selinux_netlbl_skb_sid(struct sk_buff *skb,
+					 u32 base_sid,
+					 u32 *sid)
+{
+	*sid = SECSID_NULL;
+	return 0;
+}
 #endif /* CONFIG_NETLABEL */
 
 #endif
Index: net-2.6/security/selinux/ss/services.c
===================================================================
--- net-2.6.orig/security/selinux/ss/services.c
+++ net-2.6/security/selinux/ss/services.c
@@ -51,6 +51,7 @@
 #include "selinux_netlabel.h"
 
 extern void selnl_notify_policyload(u32 seqno);
+extern int selinux_compat_net;
 unsigned int policydb_loaded_version;
 
 static DEFINE_RWLOCK(policy_rwlock);
@@ -2454,7 +2455,6 @@ void selinux_netlbl_sk_security_init(str
 void selinux_netlbl_sk_clone_security(struct sk_security_struct *ssec,
 				      struct sk_security_struct *newssec)
 {
-	newssec->sclass = ssec->sclass;
 	if (ssec->nlbl_state != NLBL_UNSET)
 		newssec->nlbl_state = NLBL_REQUIRE;
 	else
@@ -2476,11 +2476,8 @@ int selinux_netlbl_socket_post_create(st
 				      int sock_family,
 				      u32 sid)
 {
-	struct inode_security_struct *isec = SOCK_INODE(sock)->i_security;
 	struct sk_security_struct *sksec = sock->sk->sk_security;
 
-	sksec->sclass = isec->sclass;
-
 	if (sock_family != PF_INET)
 		return 0;
 
@@ -2500,24 +2497,23 @@ int selinux_netlbl_socket_post_create(st
  */
 void selinux_netlbl_sock_graft(struct sock *sk, struct socket *sock)
 {
-	struct inode_security_struct *isec = SOCK_INODE(sock)->i_security;
 	struct sk_security_struct *sksec = sk->sk_security;
 	struct netlbl_lsm_secattr secattr;
 	u32 nlbl_peer_sid;
 
-	sksec->sclass = isec->sclass;
-
 	if (sk->sk_family != PF_INET)
 		return;
 
-	netlbl_secattr_init(&secattr);
-	if (netlbl_sock_getattr(sk, &secattr) == 0 &&
-	    selinux_netlbl_secattr_to_sid(NULL,
-					  &secattr,
-					  sksec->sid,
-					  &nlbl_peer_sid) == 0)
-		sksec->peer_sid = nlbl_peer_sid;
-	netlbl_secattr_destroy(&secattr, 0);
+	if (selinux_compat_net) {
+		netlbl_secattr_init(&secattr);
+		if (netlbl_sock_getattr(sk, &secattr) == 0 &&
+		    selinux_netlbl_secattr_to_sid(NULL,
+						  &secattr,
+						  sksec->sid,
+						  &nlbl_peer_sid) == 0)
+			sksec->peer_sid = nlbl_peer_sid;
+		netlbl_secattr_destroy(&secattr, 0);
+	}
 
 	sksec->nlbl_state = NLBL_REQUIRE;
 
@@ -2528,32 +2524,6 @@ void selinux_netlbl_sock_graft(struct so
 }
 
 /**
- * selinux_netlbl_inet_conn_request - Handle a new connection request
- * @skb: the packet
- * @sock_sid: the SID of the parent socket
- *
- * Description:
- * If present, use the security attributes of the packet in @skb and the
- * parent sock's SID to arrive at a SID for the new child sock.  Returns the
- * SID of the connection or SECSID_NULL on failure.
- *
- */
-u32 selinux_netlbl_inet_conn_request(struct sk_buff *skb, u32 sock_sid)
-{
-	int rc;
-	u32 peer_sid;
-
-	rc = selinux_netlbl_skbuff_getsid(skb, sock_sid, &peer_sid);
-	if (rc != 0)
-		return SECSID_NULL;
-
-	if (peer_sid == SECINITSID_UNLABELED)
-		return SECSID_NULL;
-
-	return peer_sid;
-}
-
-/**
  * selinux_netlbl_inode_permission - Verify the socket is NetLabel labeled
  * @inode: the file descriptor's inode
  * @mask: the permission mask
@@ -2593,7 +2563,8 @@ int selinux_netlbl_inode_permission(stru
 
 /**
  * selinux_netlbl_sock_rcv_skb - Do an inbound access check using NetLabel
- * @sksec: the sock's sk_security_struct
+ * @sock_sid: the socket's SID
+ * @sock_class: the socket's class
  * @skb: the packet
  * @ad: the audit data
  *
@@ -2603,7 +2574,8 @@ int selinux_netlbl_inode_permission(stru
  * error.
  *
  */
-int selinux_netlbl_sock_rcv_skb(struct sk_security_struct *sksec,
+int selinux_netlbl_sock_rcv_skb(u32 sock_sid,
+				u16 sock_class,
 				struct sk_buff *skb,
 				struct avc_audit_data *ad)
 {
@@ -2618,7 +2590,7 @@ int selinux_netlbl_sock_rcv_skb(struct s
 	if (netlbl_sid == SECINITSID_UNLABELED)
 		return 0;
 
-	switch (sksec->sclass) {
+	switch (sock_class) {
 	case SECCLASS_UDP_SOCKET:
 		recv_perm = UDP_SOCKET__RECVFROM;
 		break;
@@ -2629,9 +2601,9 @@ int selinux_netlbl_sock_rcv_skb(struct s
 		recv_perm = RAWIP_SOCKET__RECVFROM;
 	}
 
-	rc = avc_has_perm(sksec->sid,
+	rc = avc_has_perm(sock_sid,
 			  netlbl_sid,
-			  sksec->sclass,
+			  sock_class,
 			  recv_perm,
 			  ad);
 	if (rc == 0)
@@ -2642,25 +2614,6 @@ int selinux_netlbl_sock_rcv_skb(struct s
 }
 
 /**
- * selinux_netlbl_socket_getpeersec_stream - Return the connected peer's SID
- * @sock: the socket
- *
- * Description:
- * Examine @sock to find the connected peer's SID.  Returns the SID on success
- * or SECSID_NULL on error.
- *
- */
-u32 selinux_netlbl_socket_getpeersec_stream(struct socket *sock)
-{
-	struct sk_security_struct *sksec = sock->sk->sk_security;
-
-	if (sksec->peer_sid == SECINITSID_UNLABELED)
-		return SECSID_NULL;
-
-	return sksec->peer_sid;
-}
-
-/**
  * selinux_netlbl_socket_getpeersec_dgram - Return the SID of a NetLabel packet
  * @skb: the packet
  *
@@ -2686,4 +2639,27 @@ u32 selinux_netlbl_socket_getpeersec_dgr
 
 	return peer_sid;
 }
+
+/**
+ * selinux_netlbl_skb_sid - Calculate the NetLabel SID of a packet
+ * @skb: the packet
+ * @base_sid: the SELinux SID to use as a context for MLS only attributes
+ * @sid: the SID
+ *
+ * Description:
+ * Call the NetLabel mechanism to get the security attributes of the given
+ * packet and use those attributes to determine the correct context/SID to
+ * assign to the packet.  Returns zero on success, negative values on failure.
+ *
+ */
+int selinux_netlbl_skb_sid(struct sk_buff *skb, u32 base_sid, u32 *sid)
+{
+	int rc;
+
+	rc = selinux_netlbl_skbuff_getsid(skb, base_sid, sid);
+	if (rc == 0 && *sid == SECINITSID_UNLABELED)
+		*sid = SECSID_NULL;
+
+	return rc;
+}
 #endif /* CONFIG_NETLABEL */

--
paul moore
linux security @ hp

RE: [PATCH v4 1/2] NetLabel: secid reconciliation support

[Venkat Yekkirala]

> @@ -3714,19 +3714,34 @@ static int selinux_skb_flow_in(struct sk
>  	if (skb->dev == &loopback_dev)
>  		return 1;
>  
> +	if (skb->secmark)
> +		loc_sid = skb->secmark;
> +	else
> +		loc_sid = SECINITSID_NETMSG;
> +
>  	err = selinux_xfrm_decode_session(skb, &xfrm_sid, 0);
>  	BUG_ON(err);
> -
> -	err = avc_has_perm(xfrm_sid, skb->secmark? : SECINITSID_NETMSG,
> -					SECCLASS_PACKET,
> -					PACKET__FLOW_IN, NULL);
> +	err = selinux_netlbl_skb_sid(skb,
> +				     xfrm_sid ? xfrm_sid : loc_sid,
> +				     &nlbl_sid);
>  	if (err)
>  		goto out;
>  
> -	if (xfrm_sid)
> -		skb->secmark = xfrm_sid;
> +	if (nlbl_sid)
> +		ext_sid = nlbl_sid;
> +	else
> +		ext_sid = xfrm_sid;

There's a problem here in that it would require 2 different policies
depending on whether one is using netlabel or xfrm. Specifically, in
the absence of matching iptables contexts (secmark), the skb here
will get:

- 0 (xfrm case)
- network_t (netlabel)

This has implications for getpeercon() where it would

- fail with ENOPROTOOPT (xfrm case)
- returns network_t (netlabel)

I would still argue that the nature of the domain being carried by
the packet is still unlabeled_t as implied by the null secmark. While
I view secmark/point as specifying BOTH a flow control point and a
default domain (incidentally using the same label more because of
implementation constrainst), I view network_t as purely a flow control
point.

But I also realize there can be equally forceful arguments for what this
patch does.

What does the community think? We need to resolve it one way or the
other unless the above differences in behavior are desired or somehow
accounted for in policy and apps.

> +
> +	err = avc_has_perm(ext_sid,
> +			   loc_sid,
> +			   SECCLASS_PACKET,
> +			   PACKET__FLOW_IN,
> +			   NULL);
> +	if (err)
> +		goto out;
>  
> -	/* See if NetLabel can flow in thru the current secmark here */
> +	if (ext_sid)
> +		skb->secmark = ext_sid;
>  
>  out:
>  	return err ? 0 : 1;
> 

RE: [PATCH v4 1/2] NetLabel: secid reconciliation support

[Venkat Yekkirala]

> > @@ -3714,19 +3714,34 @@ static int selinux_skb_flow_in(struct sk
> >  	if (skb->dev == &loopback_dev)
> >  		return 1;
> >  
> > +	if (skb->secmark)
> > +		loc_sid = skb->secmark;
> > +	else
> > +		loc_sid = SECINITSID_NETMSG;
> > +
> >  	err = selinux_xfrm_decode_session(skb, &xfrm_sid, 0);
> >  	BUG_ON(err);
> > -
> > -	err = avc_has_perm(xfrm_sid, skb->secmark? : SECINITSID_NETMSG,
> > -					SECCLASS_PACKET,
> > -					PACKET__FLOW_IN, NULL);
> > +	err = selinux_netlbl_skb_sid(skb,
> > +				     xfrm_sid ? xfrm_sid : loc_sid,
> > +				     &nlbl_sid);
> >  	if (err)
> >  		goto out;
> >  
> > -	if (xfrm_sid)
> > -		skb->secmark = xfrm_sid;
> > +	if (nlbl_sid)
> > +		ext_sid = nlbl_sid;
> > +	else
> > +		ext_sid = xfrm_sid;
> 
> There's a problem here in that it would require 2 different policies
> depending on whether one is using netlabel or xfrm. Specifically, in
> the absence of matching iptables contexts (secmark),

as well as any external labeling via ipsec/NetLabel,

> the skb here
> will get:
> 
> - 0 (xfrm case)
> - network_t (netlabel)
> 
> This has implications for getpeercon() where it would
> 
> - fail with ENOPROTOOPT (xfrm case)
> - returns network_t (netlabel)
> 
> I would still argue that the nature of the domain being carried by
> the packet is still unlabeled_t as implied by the null secmark. While
> I view secmark/point as specifying BOTH a flow control point and a
> default domain (incidentally using the same label more because of
> implementation constrainst), I view network_t as purely a flow control
> point.
> 
> But I also realize there can be equally forceful arguments 
> for what this
> patch does.
> 
> What does the community think? We need to resolve it one way or the
> other unless the above differences in behavior are desired or somehow
> accounted for in policy and apps.
> 
> > +
> > +	err = avc_has_perm(ext_sid,
> > +			   loc_sid,
> > +			   SECCLASS_PACKET,
> > +			   PACKET__FLOW_IN,
> > +			   NULL);
> > +	if (err)
> > +		goto out;
> >  
> > -	/* See if NetLabel can flow in thru the current secmark here */
> > +	if (ext_sid)
> > +		skb->secmark = ext_sid;
> >  
> >  out:
> >  	return err ? 0 : 1;
> > 
> 

Re: [PATCH v4 1/2] NetLabel: secid reconciliation support

[Paul Moore]

Venkat Yekkirala wrote:
>>@@ -3714,19 +3714,34 @@ static int selinux_skb_flow_in(struct sk
>> 	if (skb->dev == &loopback_dev)
>> 		return 1;
>> 
>>+	if (skb->secmark)
>>+		loc_sid = skb->secmark;
>>+	else
>>+		loc_sid = SECINITSID_NETMSG;
>>+
>> 	err = selinux_xfrm_decode_session(skb, &xfrm_sid, 0);
>> 	BUG_ON(err);
>>-
>>-	err = avc_has_perm(xfrm_sid, skb->secmark? : SECINITSID_NETMSG,
>>-					SECCLASS_PACKET,
>>-					PACKET__FLOW_IN, NULL);
>>+	err = selinux_netlbl_skb_sid(skb,
>>+				     xfrm_sid ? xfrm_sid : loc_sid,
>>+				     &nlbl_sid);
>> 	if (err)
>> 		goto out;
>> 
>>-	if (xfrm_sid)
>>-		skb->secmark = xfrm_sid;
>>+	if (nlbl_sid)
>>+		ext_sid = nlbl_sid;
>>+	else
>>+		ext_sid = xfrm_sid;
> 
> 
> There's a problem here in that it would require 2 different policies
> depending on whether one is using netlabel or xfrm. Specifically, in
> the absence of matching iptables contexts (secmark), the skb here
> will get:
> 
> - 0 (xfrm case)
> - network_t (netlabel)

Perhaps I'm reading it the wrong way, but I didn't quite follow your
comment.  In an effort to bring some clarity to all of this here are all
of the possibile combinations that I can see using the code above (feel
free to correct me if I made a mistake):

 * SECMARK, XFRM, NetLabel present

   xfrm_sid = <full context from xfrm>
   loc_sid = <full context from secmark>
   nlbl_sid = <xfrm_sid te ctx, netlabel mls ctx>
   ext_sid = nlbl_sid
   final skb->secmark = avc_ok : ext_sid ? unchanged

 * SECMARK, XFRM present

   xfrm_sid = <full context from xfrm>
   loc_sid = <full context from secmark>
   nlbl_sid = SECSID_NULL/0
   ext_sid = xfrm_sid
   final skb->secmark = avc_ok : ext_sid ? unchanged

 * SECMARK, NetLabel present

   xfrm_sid = SECSID_NULL/0
   loc_sid = <full context from secmark>
   nlbl_sid = <secmark te ctx, netlabel mls ctx>
   ext_sid = nlbl_sid
   final skb->secmark = avc_ok : ext_sid ? unchanged

 * XFRM, NetLabel present

   xfrm_sid = <full context from xfrm>
   loc_sid = SECINITSID_NETMSG
   nlbl_sid = <xfrm_sid te ctx, netlabel mls ctx>
   ext_sid = nlbl_sid
   final skb->secmark = avc_ok : ext_sid ? unchanged

 * XFRM present

   xfrm_sid = <full context from xfrm>
   loc_sid = SECINITSID_NETMSG
   nlbl_sid = SECSID_NULL/0
   ext_sid = xfrm_sid
   final skb->secmark = avc_ok : ext_sid ? unchanged

 * NetLabel present

   xfrm_sid = SECSID_NULL/0
   loc_sid = SECSID_NULL/0
   nlbl_sid = <SECINITSID_NETMSG te ctx, netlabel mls ctx>
   ext_sid = nlbl_sid
   final skb->secmark = avc_ok : ext_sid ? unchanged

 * Nothing

   xfrm_sid = SECSID_NULL/0
   loc_sid = SECSID_NULL/0
   nlbl_sid = SECSID_NULL/0
   ext_sid = xfrm_sid
   final skb->secmark = avc_ok : ext_sid ? unchanged

> This has implications for getpeercon() where it would
> 
> - fail with ENOPROTOOPT (xfrm case)
> - returns network_t (netlabel)
> 
> I would still argue that the nature of the domain being carried by
> the packet is still unlabeled_t as implied by the null secmark. While
> I view secmark/point as specifying BOTH a flow control point and a
> default domain (incidentally using the same label more because of
> implementation constrainst), I view network_t as purely a flow control
> point.
> 
> But I also realize there can be equally forceful arguments for what this
> patch does.

I know the two of us have talked about this before on the mail lists,
but I don't believe anyone else has ever made a comment in this regard.
 I tend to feel rather strongly that when using just NetLabel on a
connection you shouldn't see an "unlabeled_t" type as I feel that the
connotation associated with this type is misleading, even though from a
TE point of view there may be an argument made that it is appropriate.

> What does the community think? We need to resolve it one way or the
> other unless the above differences in behavior are desired or somehow
> accounted for in policy and apps.

I agree - I'd like to hear what others (namely Stephen Smalley, James
Morris and all of the Tresys folks <past and present>) have to say on
this issue.

-- 
paul moore
linux security @ hp

RE: [PATCH v4 1/2] NetLabel: secid reconciliation support

[Venkat Yekkirala]

>  * XFRM present
> 
>    xfrm_sid = <full context from xfrm>
>    loc_sid = SECINITSID_NETMSG
>    nlbl_sid = SECSID_NULL/0
>    ext_sid = xfrm_sid
>    final skb->secmark = avc_ok : ext_sid ? unchanged
> 
>  * NetLabel present
> 
>    xfrm_sid = SECSID_NULL/0
>    loc_sid = SECSID_NULL/0
>    nlbl_sid = <SECINITSID_NETMSG te ctx, netlabel mls ctx>
>    ext_sid = nlbl_sid
>    final skb->secmark = avc_ok : ext_sid ? unchanged

I was referring to the differences in what getpeercon would
return in the above 2 scenarios.

In the xfrm case:     final skb->secmark would be 0 resulting in getpeercon
to return EPROTONOOPT
In the NetLabel case: final skb->secmark would be network_t resulting in
getpeercon to return network_t

> 
>  * Nothing
> 
>    xfrm_sid = SECSID_NULL/0
>    loc_sid = SECSID_NULL/0
>    nlbl_sid = SECSID_NULL/0
>    ext_sid = xfrm_sid
>    final skb->secmark = avc_ok : ext_sid ? unchanged
> 
> > This has implications for getpeercon() where it would
> > 
> > - fail with ENOPROTOOPT (xfrm case)
> > - returns network_t (netlabel)
> > 
> > I would still argue that the nature of the domain being carried by
> > the packet is still unlabeled_t as implied by the null 
> secmark. While
> > I view secmark/point as specifying BOTH a flow control point and a
> > default domain (incidentally using the same label more because of
> > implementation constrainst), I view network_t as purely a 
> flow control
> > point.
> > 
> > But I also realize there can be equally forceful arguments 
> for what this
> > patch does.
> 
> I know the two of us have talked about this before on the mail lists,
> but I don't believe anyone else has ever made a comment in 
> this regard.
>  I tend to feel rather strongly that when using just NetLabel on a
> connection you shouldn't see an "unlabeled_t" type as I feel that the
> connotation associated with this type is misleading, even 
> though from a
> TE point of view there may be an argument made that it is appropriate.
> 
> > What does the community think? We need to resolve it one way or the
> > other unless the above differences in behavior are desired 
> or somehow
> > accounted for in policy and apps.
> 
> I agree - I'd like to hear what others (namely Stephen Smalley, James
> Morris and all of the Tresys folks <past and present>) have to say on
> this issue.
> 
> -- 
> paul moore
> linux security @ hp
> 

Re: [PATCH v4 1/2] NetLabel: secid reconciliation support

[Paul Moore]

Venkat Yekkirala wrote:
>> * XFRM present
>>
>>   xfrm_sid = <full context from xfrm>
>>   loc_sid = SECINITSID_NETMSG
>>   nlbl_sid = SECSID_NULL/0
>>   ext_sid = xfrm_sid
>>   final skb->secmark = avc_ok : ext_sid ? unchanged
>>
>> * NetLabel present
>>
>>   xfrm_sid = SECSID_NULL/0
>>   loc_sid = SECSID_NULL/0
>>   nlbl_sid = <SECINITSID_NETMSG te ctx, netlabel mls ctx>
>>   ext_sid = nlbl_sid
>>   final skb->secmark = avc_ok : ext_sid ? unchanged
> 
> I was referring to the differences in what getpeercon would
> return in the above 2 scenarios.
> 
> In the xfrm case:     final skb->secmark would be 0 resulting in getpeercon
> to return EPROTONOOPT

In the "XFRM present" case above if the access is allowed (avc_ok is
true) then the final skb->secmark value is going to be set to the value
of ext_sid which is the xfrm_sid.  Any calls made to getpeercon() would
return success with the context matching xfrm_sid.

I have a hunch we are still on different pages here ...

> In the NetLabel case: final skb->secmark would be network_t resulting in
> getpeercon to return network_t

Yep, and I understand you would like to see it as unlabeled_t.  I think
we both have valid arguments for either case and we are just waiting to
hear from others.

-- 
paul moore
linux security @ hp

RE: [PATCH v4 1/2] NetLabel: secid reconciliation support

[Venkat Yekkirala]

> >> * XFRM present
> >>
> >>   xfrm_sid = <full context from xfrm>
> >>   loc_sid = SECINITSID_NETMSG
> >>   nlbl_sid = SECSID_NULL/0
> >>   ext_sid = xfrm_sid
> >>   final skb->secmark = avc_ok : ext_sid ? unchanged

Actually, I meant to cite the following instead of the above:

 * Nothing

   xfrm_sid = SECSID_NULL/0
   loc_sid = SECSID_NULL/0
   nlbl_sid = SECSID_NULL/0
   ext_sid = xfrm_sid
   final skb->secmark = avc_ok : ext_sid ? unchanged

> >>
> >> * NetLabel present
> >>
> >>   xfrm_sid = SECSID_NULL/0
> >>   loc_sid = SECSID_NULL/0
> >>   nlbl_sid = <SECINITSID_NETMSG te ctx, netlabel mls ctx>
> >>   ext_sid = nlbl_sid
> >>   final skb->secmark = avc_ok : ext_sid ? unchanged
> > 
> > I was referring to the differences in what getpeercon would
> > return in the above 2 scenarios.
> > 
> > In the xfrm case:     final skb->secmark would be 0 
> resulting in getpeercon
> > to return EPROTONOOPT
> 
> In the "XFRM present" case above if the access is allowed (avc_ok is
> true) then the final skb->secmark value is going to be set to 
> the value
> of ext_sid which is the xfrm_sid.  Any calls made to 
> getpeercon() would
> return success with the context matching xfrm_sid.

You are right, but I was actually referring to the "Nothing"
case above.

> 
> I have a hunch we are still on different pages here ...
> 
> > In the NetLabel case: final skb->secmark would be network_t 
> resulting in
> > getpeercon to return network_t
> 
> Yep, and I understand you would like to see it as 
> unlabeled_t.  I think
> we both have valid arguments for either case and we are just 
> waiting to
> hear from others.
> 
> -- 
> paul moore
> linux security @ hp
> 

Re: [PATCH v4 1/2] NetLabel: secid reconciliation support

[Stephen Smalley]

On Wed, 2006-10-04 at 15:27 -0400, Paul Moore wrote:
> Venkat Yekkirala wrote:
> >> * XFRM present
> >>
> >>   xfrm_sid = <full context from xfrm>
> >>   loc_sid = SECINITSID_NETMSG
> >>   nlbl_sid = SECSID_NULL/0
> >>   ext_sid = xfrm_sid
> >>   final skb->secmark = avc_ok : ext_sid ? unchanged
> >>
> >> * NetLabel present
> >>
> >>   xfrm_sid = SECSID_NULL/0
> >>   loc_sid = SECSID_NULL/0
> >>   nlbl_sid = <SECINITSID_NETMSG te ctx, netlabel mls ctx>
> >>   ext_sid = nlbl_sid
> >>   final skb->secmark = avc_ok : ext_sid ? unchanged
> > 
> > I was referring to the differences in what getpeercon would
> > return in the above 2 scenarios.
> > 
> > In the xfrm case:     final skb->secmark would be 0 resulting in getpeercon
> > to return EPROTONOOPT
> 
> In the "XFRM present" case above if the access is allowed (avc_ok is
> true) then the final skb->secmark value is going to be set to the value
> of ext_sid which is the xfrm_sid.  Any calls made to getpeercon() would
> return success with the context matching xfrm_sid.
> 
> I have a hunch we are still on different pages here ...
> 
> > In the NetLabel case: final skb->secmark would be network_t resulting in
> > getpeercon to return network_t
> 
> Yep, and I understand you would like to see it as unlabeled_t.  I think
> we both have valid arguments for either case and we are just waiting to
> hear from others.

I don't understand the argument for network_t, and it seems to violate
our goals of 1) having consistent policy regardless of network labeling
mechanism, and 2) having getpeercon() always return a subject label that
can be used as a basis for avc_has_perm() and setexeccon() calls.

-- 
Stephen Smalley
National Security Agency

Re: [PATCH v4 1/2] NetLabel: secid reconciliation support

[Paul Moore]

Venkat Yekkirala wrote:
>>>>* XFRM present
>>>>
>>>>  xfrm_sid = <full context from xfrm>
>>>>  loc_sid = SECINITSID_NETMSG
>>>>  nlbl_sid = SECSID_NULL/0
>>>>  ext_sid = xfrm_sid
>>>>  final skb->secmark = avc_ok : ext_sid ? unchanged
> 
> Actually, I meant to cite the following instead of the above:
> 
>  * Nothing
> 
>    xfrm_sid = SECSID_NULL/0
>    loc_sid = SECSID_NULL/0
>    nlbl_sid = SECSID_NULL/0
>    ext_sid = xfrm_sid
>    final skb->secmark = avc_ok : ext_sid ? unchanged

Okay, thanks, I think I understand your point now.  Let me try to
restate just to make sure.

Take two cases: the first being no labeling at all, the second being
only NetLabel ...

 * Nothing

   xfrm_sid = SECSID_NULL/0
   loc_sid = SECSID_NULL/0
   nlbl_sid = SECSID_NULL/0
   ext_sid = xfrm_sid
   final skb->secmark = avc_ok : ext_sid ? unchanged

 * NetLabel

  xfrm_sid = SECSID_NULL/0
  loc_sid = SECSID_NULL/0
  nlbl_sid = <SECINITSID_NETMSG te ctx, netlabel mls ctx>
  ext_sid = nlbl_sid
  final skb->secmark = avc_ok : ext_sid ? unchanged

In the "Nothing" case the final skb->secmark is set to SECSID_NULL/0;
getpeercon() should return an error. In the "NetLabel" case the final
skb->secmark is set using the TE portions of SECINITSID_NETMSG and the
MLS label of the NetLabel packet; getpeercon() will succeed.

I understand in the "NetLabel" case above you think the TE portion of
the final skb->secmark value should come from
SECSID_NULL/SECINITSID_UNLABELED/0, but do you also want getpeercon() to
return an error in the "NetLabel" case?  I think that's a bad idea as it
will made it *extremely* difficult for applications to determine the MLS
label of a connection using NetLabel.

>>>>* NetLabel present
>>>>
>>>>  xfrm_sid = SECSID_NULL/0
>>>>  loc_sid = SECSID_NULL/0
>>>>  nlbl_sid = <SECINITSID_NETMSG te ctx, netlabel mls ctx>
>>>>  ext_sid = nlbl_sid
>>>>  final skb->secmark = avc_ok : ext_sid ? unchanged
>>>
>>>I was referring to the differences in what getpeercon would
>>>return in the above 2 scenarios.
>>>
>>>In the xfrm case:     final skb->secmark would be 0 
>>
>>resulting in getpeercon
>>
>>>to return EPROTONOOPT
>>
>>In the "XFRM present" case above if the access is allowed (avc_ok is
>>true) then the final skb->secmark value is going to be set to 
>>the value
>>of ext_sid which is the xfrm_sid.  Any calls made to 
>>getpeercon() would
>>return success with the context matching xfrm_sid.
> 
> 
> You are right, but I was actually referring to the "Nothing"
> case above.
> 
> 
>>I have a hunch we are still on different pages here ...
>>
>>
>>>In the NetLabel case: final skb->secmark would be network_t 
>>
>>resulting in
>>
>>>getpeercon to return network_t
>>
>>Yep, and I understand you would like to see it as 
>>unlabeled_t.  I think
>>we both have valid arguments for either case and we are just 
>>waiting to
>>hear from others.
>>
>>-- 
>>paul moore
>>linux security @ hp
>>


-- 
paul moore
linux security @ hp

RE: [PATCH v4 1/2] NetLabel: secid reconciliation support

[Venkat Yekkirala]

> On Wed, 2006-10-04 at 15:27 -0400, Paul Moore wrote:
> > Venkat Yekkirala wrote:
> > >> * XFRM present
> > >>
> > >>   xfrm_sid = <full context from xfrm>
> > >>   loc_sid = SECINITSID_NETMSG
> > >>   nlbl_sid = SECSID_NULL/0
> > >>   ext_sid = xfrm_sid
> > >>   final skb->secmark = avc_ok : ext_sid ? unchanged

As noted subsequently, I actually meant to refer to the below
instead of the XFRM case above:

 * Nothing

   xfrm_sid = SECSID_NULL/0
   loc_sid = SECSID_NULL/0
   nlbl_sid = SECSID_NULL/0
   ext_sid = xfrm_sid
   final skb->secmark = avc_ok : ext_sid ? unchanged

> > >>
> > >> * NetLabel present
> > >>
> > >>   xfrm_sid = SECSID_NULL/0
> > >>   loc_sid = SECSID_NULL/0
> > >>   nlbl_sid = <SECINITSID_NETMSG te ctx, netlabel mls ctx>
> > >>   ext_sid = nlbl_sid
> > >>   final skb->secmark = avc_ok : ext_sid ? unchanged
> > > 
> > > I was referring to the differences in what getpeercon would
> > > return in the above 2 scenarios.
> > > 
> > > In the xfrm case:     final skb->secmark would be 0 
> resulting in getpeercon
> > > to return EPROTONOOPT
> > 
> > In the "XFRM present" case above if the access is allowed (avc_ok is
> > true) then the final skb->secmark value is going to be set 
> to the value
> > of ext_sid which is the xfrm_sid.  Any calls made to 
> getpeercon() would
> > return success with the context matching xfrm_sid.
> > 
> > I have a hunch we are still on different pages here ...
> > 
> > > In the NetLabel case: final skb->secmark would be 
> network_t resulting in
> > > getpeercon to return network_t
> > 
> > Yep, and I understand you would like to see it as 
> unlabeled_t.  I think
> > we both have valid arguments for either case and we are 
> just waiting to
> > hear from others.
> 
> I don't understand the argument for network_t, and it seems to violate
> our goals of 1) having consistent policy regardless of 
> network labeling
> mechanism, and 2) having getpeercon() always return a subject 
> label that
> can be used as a basis for avc_has_perm() and setexeccon() calls.

But in the case where there's no domain info, but a cipso label,
getpeercon will fail (EPROTONOOPT). Do I rememember that Paul was
trying to use a special SID to use as a base sid in this case?

Re: [PATCH v4 1/2] NetLabel: secid reconciliation support

[Paul Moore]

Venkat Yekkirala wrote:
>>On Wed, 2006-10-04 at 15:27 -0400, Paul Moore wrote:
>>
>>>Venkat Yekkirala wrote:
>>>
>>>>>* XFRM present
>>>>>
>>>>>  xfrm_sid = <full context from xfrm>
>>>>>  loc_sid = SECINITSID_NETMSG
>>>>>  nlbl_sid = SECSID_NULL/0
>>>>>  ext_sid = xfrm_sid
>>>>>  final skb->secmark = avc_ok : ext_sid ? unchanged
> 
> 
> As noted subsequently, I actually meant to refer to the below
> instead of the XFRM case above:
> 
>  * Nothing
> 
>    xfrm_sid = SECSID_NULL/0
>    loc_sid = SECSID_NULL/0
>    nlbl_sid = SECSID_NULL/0
>    ext_sid = xfrm_sid
>    final skb->secmark = avc_ok : ext_sid ? unchanged
> 
> 
>>>>>* NetLabel present
>>>>>
>>>>>  xfrm_sid = SECSID_NULL/0
>>>>>  loc_sid = SECSID_NULL/0
>>>>>  nlbl_sid = <SECINITSID_NETMSG te ctx, netlabel mls ctx>
>>>>>  ext_sid = nlbl_sid
>>>>>  final skb->secmark = avc_ok : ext_sid ? unchanged
>>>>
>>>>I was referring to the differences in what getpeercon would
>>>>return in the above 2 scenarios.
>>>>
>>>>In the xfrm case:     final skb->secmark would be 0 
>>
>>resulting in getpeercon
>>
>>>>to return EPROTONOOPT
>>>
>>>In the "XFRM present" case above if the access is allowed (avc_ok is
>>>true) then the final skb->secmark value is going to be set 
>>
>>to the value
>>
>>>of ext_sid which is the xfrm_sid.  Any calls made to 
>>
>>getpeercon() would
>>
>>>return success with the context matching xfrm_sid.
>>>
>>>I have a hunch we are still on different pages here ...
>>>
>>>
>>>>In the NetLabel case: final skb->secmark would be 
>>
>>network_t resulting in
>>
>>>>getpeercon to return network_t
>>>
>>>Yep, and I understand you would like to see it as 
>>
>>unlabeled_t.  I think
>>
>>>we both have valid arguments for either case and we are 
>>
>>just waiting to
>>
>>>hear from others.
>>
>>I don't understand the argument for network_t, and it seems to violate
>>our goals of 1) having consistent policy regardless of 
>>network labeling
>>mechanism, and 2) having getpeercon() always return a subject 
>>label that
>>can be used as a basis for avc_has_perm() and setexeccon() calls.
> 
> But in the case where there's no domain info, but a cipso label,
> getpeercon will fail (EPROTONOOPT). Do I rememember that Paul was
> trying to use a special SID to use as a base sid in this case?

[I'm replying to both Stephen and Venkat here as the email is coming in
faster than I can type :)]

In the current non-secid world, if the connection is NetLabel labeled,
the sock_rcv_skb() LSM hook uses SECINITSID_NETMSG as the base_sid and
the recvfrom permission to control access.  Calls to getpeercon() on a
NetLabel connection use the socket's SID as the base_sid and the network
traffic's MLS label.

In the latest secid world, when there is only NetLabel labeling present
on the connection SECINITSID_NETMSG is used as the base_sid for the
final skb->secmark value as determined by the flow_in() LSM hook with
the MLS label being set to match the NetLabel on the packet.  Calls to
getpeercon() will use the secmark value of the packet when the
connection is establishes (in *all* cases) so getpeercon() in the
NetLabel only case will succeed and return the value determined above.

-- 
paul moore
linux security @ hp