[redhat-lspp] Updated NetLabel patch

Stephen Smalley sds at tycho.nsa.gov
Wed Jun 14 13:38:21 UTC 2006


On Tue, 2006-06-13 at 13:19 -0400, Paul Moore wrote:
> Attached is an updated NetLabel patch from June 13th (today) against the
> lspp.35 sources.  As before it has been quickly tested on x86, x86_64,
> targeted/enforcing, and mls/permissive although not all hw/policy
> combinations have been tested.  If you wish to configure NetLabel to use
> CIPSO please grab the June 13th release of netlabel_tools which can be
> found here:
> 
>  * http://free.linux.hp.com/~pmoore/projects/linux_cipso
> 
> The big changes since the last posting on June 6th are:
> 
>  * Demonstrated interop between TSOL v 8 (big thanks to Ted)
>  * Relabeling of sockets on accept()
>  * The addition of the "pass through" CIPSO mapping
>  * Better NetLabel netlink error reporting to userspace
>  * Verified CIPSO option is recognized as immutable by AH
>    (not yet tested)
> 
> The patch stats:
> 
>  CREDITS                                                   |    7
>  Documentation/00-INDEX                                    |    2
>  Documentation/netlabel/00-INDEX                           |   10
>  Documentation/netlabel/cipso_ipv4.txt                     |   48
>  Documentation/netlabel/draft-ietf-cipso-ipsecurity-01.txt |  791 ++++
>  Documentation/netlabel/introduction.txt                   |   44
>  Documentation/netlabel/lsm_interface.txt                  |   47
>  include/linux/ip.h                                        |    1
>  include/linux/netlink.h                                   |    1
>  include/net/cipso_ipv4.h                                  |  159
>  include/net/inet_sock.h                                   |    2
>  include/net/netlabel.h                                    |  354 ++
>  net/Kconfig                                               |    2
>  net/Makefile                                              |    1
>  net/ipv4/Makefile                                         |    1
>  net/ipv4/ah4.c                                            |    2
>  net/ipv4/cipso_ipv4.c                                     | 1749 ++++++
>  net/ipv4/ip_options.c                                     |   19
>  net/netlabel/Kconfig                                      |   47
>  net/netlabel/Makefile                                     |   15
>  net/netlabel/netlabel_cipso_v4.c                          |  580 +++
>  net/netlabel/netlabel_cipso_v4.h                          |  201 +
>  net/netlabel/netlabel_domainhash.c                        |  629 +++
>  net/netlabel/netlabel_domainhash.h                        |   64
>  net/netlabel/netlabel_kapi.c                              |  420 ++
>  net/netlabel/netlabel_mgmt.c                              |  677 +++
>  net/netlabel/netlabel_mgmt.h                              |  248 +
>  net/netlabel/netlabel_unlabeled.c                         |  285 +
>  net/netlabel/netlabel_unlabeled.h                         |   83
>  net/netlabel/netlabel_user.c                              |  174
>  net/netlabel/netlabel_user.h                              |   42
>  security/selinux/hooks.c                                  |   81
>  security/selinux/include/av_inherit.h                     |    1
>  security/selinux/include/av_perm_to_string.h              |    2
>  security/selinux/include/av_permissions.h                 |    1
>  security/selinux/include/flask.h                          |    1
>  security/selinux/include/security.h                       |    9
>  security/selinux/nlmsgtab.c                               |  159
>  security/selinux/ss/ebitmap.c                             |  155
>  security/selinux/ss/ebitmap.h                             |    6
>  security/selinux/ss/mls.c                                 |  160
>  security/selinux/ss/mls.h                                 |   25
>  security/selinux/ss/services.c                            |  415 ++
>  security/selinux/xfrm.c                                   |   22
>  44 files changed, 7652 insertions(+), 90 deletions(-)
> 
> I'll be posting a more "reviewer friendly" patchset in a week or so once
> this has been out for a few days and I have had a chance to work on the
> patch a bit more (discussed on Monday's concall).

diff -purN linux-2.6.16.i686/security/selinux/hooks.c linux-2.6.16.i686-netlabel_06132006/security/selinux/hooks.c
--- linux-2.6.16.i686/security/selinux/hooks.c	2006-06-13 10:47:10.000000000 -0400
+++ linux-2.6.16.i686-netlabel_06132006/security/selinux/hooks.c	2006-06-13 11:19:59.000000000 -0400
@@ -2935,6 +2938,16 @@ static void selinux_socket_post_create(s
 	isec->sid = kern ? SECINITSID_KERNEL : tsec->sid;
 	isec->initialized = 1;
 
+#ifdef CONFIG_NETLABEL        
+	if (family == PF_INET)
+		/* PM - this will throw errors unless netlabel is configured
+		   so if you enable netlabel you must make sure you configure
+		   it early in your init scripts (i.e. before you bring up
+		   networking) ... or should we just drop the BUG_ON() and
+		   audit the failure? */
+		BUG_ON(security_netlbl_socket_setsid(sock, isec->sid));
+#endif /* CONFIG_NETLABEL */
+

This needs to be changed to either propagate an error to the caller and
cleanup (per your separate posting about socket_post_create) or to audit
the issue.  It looks like this can fail for reasons other than an actual
kernel-internal bug.

Naturally, the usual guidance about #ifdefs applies, make sure you've
followed Documentation/SubmittingPatches, CodingStyle, and now
SubmitChecklist.  You also need to re-base against an upstream kernel if
you are truly targeting 2.6.18.  It would be nice if the audit tree were
fully flushed into -mm.

@@ -3113,6 +3126,19 @@ static int selinux_socket_accept(struct 
 	return 0;
 }
 
+#ifdef CONFIG_NETLABEL
+static void selinux_socket_post_accept(struct socket *sock,
+				       struct socket *newsock)
+{
+	if (newsock->sk != NULL && newsock->sk->sk_family == PF_INET)
+		/* PM - see my comments in the socket_post_create() hook, the
+		   problem is not quite as bad here since you have already
+		   created at least one pf_inet socket and gotten traffic on
+		   it but the core question still remains: BUG() or audit? */
+		BUG_ON(security_netlbl_socket_accept(sock, newsock));
+}
+#endif /* CONFIG_NETLABEL */

Likewise here, but post_accept occurs too late, so audit may be your
only option here.

@@ -3221,6 +3247,9 @@ static int selinux_socket_sock_rcv_skb(s
 	struct socket *sock;
 	struct net_device *dev;
 	struct avc_audit_data ad;
+#ifdef CONFIG_NETLABEL
+	u32 netlbl_sid;
+#endif
 
 	family = sk->sk_family;
 	if (family != PF_INET && family != PF_INET6)

The old sock_rcv_skb processing has been split by the secmark patches
(already in the net-2.6.18 tree), with the old checks going into
selinux_sock_rcv_skb_compat, which is only called if the compatibility
mode is enabled.  So your changes here need to be re-based.

@@ -3270,6 +3299,7 @@ static int selinux_socket_sock_rcv_skb(s
 	default:
 		netif_perm = NETIF__RAWIP_RECV;
 		node_perm = NODE__RAWIP_RECV;
+		recv_perm = RAWIP_SOCKET__RECV_MSG;
 		break;
 	}
 
@@ -3294,7 +3324,7 @@ static int selinux_socket_sock_rcv_skb(s
 	if (err)
 		goto out;
 
-	if (recv_perm) {
+	if (recv_perm != 0 && recv_perm != RAWIP_SOCKET__RECV_MSG) {
 		u32 port_sid;
 
 		/* Fixme: make this more efficient */
@@ -3306,10 +3336,46 @@ static int selinux_socket_sock_rcv_skb(s
 
 		err = avc_has_perm(sock_sid, port_sid,
 				   sock_class, recv_perm, &ad);
+		if (err)
+			goto out;
 	}
 
-	if (!err)
-		err = selinux_xfrm_sock_rcv_skb(sock_sid, skb);
+	err = selinux_xfrm_sock_rcv_skb(sock_sid, skb);
+#ifdef CONFIG_NETLABEL
+	if (err == 0)
+		goto out;

So if IPSEC-labeled, ignore CIPSO option?  What if you wanted to use
CIPSO to individually label different packets/streams that use the same
SA?  Also seems to need reconcialiation with Venkat's patches for MLS
xfrm support.

+
+	err = security_netlbl_skbuff_getsid(skb, sock_sid, &netlbl_sid);
+	if (err)
+		goto out;

What overhead does this add when CIPSO isn't being used at all?

+	if (netlbl_sid == SECINITSID_UNLABELED)
+		/* PM - this may look strange (it is) but it is here to
+		   preserve backward compatability */
+		err = avc_has_perm(sock_sid,
+				   SECINITSID_UNLABELED,
+				   SECCLASS_ASSOCIATION,
+				   ASSOCIATION__RECVFROM,
+				   NULL);
+	else
+		err = avc_has_perm(sock_sid, 
+				   netlbl_sid, 
+				   sock_class, 
+				   recv_perm,
+				   &ad);

I think we need to restructure this to retain encapsulation of xfrm and
netlabel.
	
diff -purN linux-2.6.16.i686/security/selinux/include/flask.h linux-2.6.16.i686-netlabel_06132006/security/selinux/include/flask.h
--- linux-2.6.16.i686/security/selinux/include/flask.h	2006-03-20 00:53:29.000000000 -0500
+++ linux-2.6.16.i686-netlabel_06132006/security/selinux/include/flask.h	2006-06-13 11:20:00.000000000 -0400
@@ -60,6 +60,7 @@
 #define SECCLASS_NSCD                                    53
 #define SECCLASS_ASSOCIATION                             54
 #define SECCLASS_NETLINK_KOBJECT_UEVENT_SOCKET           55
+#define SECCLASS_NETLINK_NETLABEL_SOCKET                 56

Already claimed upstream for other recently added classes, so you have to re-base.

diff -purN linux-2.6.16.i686/security/selinux/nlmsgtab.c linux-2.6.16.i686-netlabel_06132006/security/selinux/nlmsgtab.c
--- linux-2.6.16.i686/security/selinux/nlmsgtab.c	2006-06-13 10:47:09.000000000 -0400
+++ linux-2.6.16.i686-netlabel_06132006/security/selinux/nlmsgtab.c	2006-06-13 11:20:00.000000000 -0400
@@ -31,92 +32,101 @@ struct nlmsg_perm
 
 static struct nlmsg_perm nlmsg_route_perms[] =
 {
-	{ RTM_NEWLINK,		NETLINK_ROUTE_SOCKET__NLMSG_WRITE },
-	{ RTM_DELLINK,		NETLINK_ROUTE_SOCKET__NLMSG_WRITE },
-	{ RTM_GETLINK,		NETLINK_ROUTE_SOCKET__NLMSG_READ  },

Try to avoid extraneous diffs unrelated to your changes - if you want to
do whitespace cleanups, do them via separate patch.

@@ -169,6 +179,19 @@ int selinux_nlmsg_lookup(u16 sclass, u16
 		}
 		break;
 
+	case SECCLASS_NETLINK_NETLABEL_SOCKET:
+		/* PM - This is really a poor fit right now as NetLabel uses
+		   the nlmsg_type value to specify the "class" of NetLabel
+		   message (i.e. management, cipso, etc.).  One way to fix
+		   this would be to refactor the NetLabel netlink API so 
+		   that it uses the nlmsg_type in a way that more closely
+		   resembles the other netlink consumers, however, that is
+		   going to require a non-trivial rework at this point so lets
+		   just leave it as is right now unless somebody complains. */
+		err = nlmsg_perm(nlmsg_type, perm, nlmsg_netlabel_perms,
+				 sizeof(nlmsg_netlabel_perms));
+		break;
+

Let me be the first to complain ;)  Also, note that there is ongoing
discussion about how to deal with netlink more generally as part of
addressing recent additions that aren't yet specifically controlled
(defaulting to generic netlink class) by SELinux.

diff -purN linux-2.6.16.i686/security/selinux/ss/services.c linux-2.6.16.i686-netlabel_06132006/security/selinux/ss/services.c
--- linux-2.6.16.i686/security/selinux/ss/services.c	2006-06-13 10:47:10.000000000 -0400
+++ linux-2.6.16.i686-netlabel_06132006/security/selinux/ss/services.c	2006-06-13 11:20:00.000000000 -0400
<snip>
+/**
+ * security_netlbl_socket_accept - Handle the labeling of an accept()ed socket
+ * @sock: the original socket
+ * @newsock: the new accept()ed socket
+ *
+ * Description:
+ * Attempt to label a socket using the NetLabel mechanism based on the packets
+ * in the queue and the original socket's SID.  Returns zero values on success,
+ * negative values on failure.
+ *
+ */
+int security_netlbl_socket_accept(struct socket *sock, struct socket *newsock)
+{
+	int ret_val;
+	struct inode_security_struct *isec = SOCK_INODE(sock)->i_security;
+	struct netlbl_lsm_secattr secattr;
+	u32 newsock_sid;
+	u16 sock_class;
+	u32 relabelto_perm;
+
+	if (!ss_initialized)
+		return 0;
+	netlbl_secattr_init(&secattr);
+
+	ret_val = netlbl_socket_getattr(newsock, &secattr);
+	if (ret_val != 0)
+		goto netlbl_socket_accept_return;
+	ret_val = security_netlbl_secattr_to_sid(NULL,
+						 &secattr,
+						 isec->sid,
+						 &newsock_sid);
+	if (ret_val != 0 || newsock_sid == SECINITSID_UNLABELED)
+		goto netlbl_socket_accept_return;
+
+	sock_class = isec->sclass;
+	switch (sock_class) {
+	case SECCLASS_UDP_SOCKET:
+		relabelto_perm = UDP_SOCKET__RELABELTO;
+		break;
+	case SECCLASS_TCP_SOCKET:
+		relabelto_perm = TCP_SOCKET__RELABELTO;
+		break;
+	default:
+		relabelto_perm = RAWIP_SOCKET__RELABELTO;
+	}
+
+	/* PM - should we have a "RELABELFROM" check too? */
+	/* PM - i suspect we should audit this socket relabel */
+	ret_val = avc_has_perm(isec->sid, 
+			       newsock_sid,
+			       sock_class,
+			       relabelto_perm,
+			       NULL);
+	if (ret_val != 0)
+		goto netlbl_socket_accept_return;
+
+	isec = SOCK_INODE(newsock)->i_security;
+	isec->sid = newsock_sid;

NAK.  Relabeling of the socket based on the packet label is almost
certainly not the right thing to do, and it violates non-tranquility.
What are you really trying to do here?

+netlbl_socket_accept_return:
+	secattr.set_cache = 0;
+	netlbl_secattr_destroy(&secattr);
+
+	return security_netlbl_socket_setsid(newsock, isec->sid);
+}
+#endif /* CONFIG_NETLABEL */
+
 struct selinux_audit_rule {
 	u32 au_seqno;
 	struct context au_ctxt;
diff -purN linux-2.6.16.i686/security/selinux/xfrm.c linux-2.6.16.i686-netlabel_06132006/security/selinux/xfrm.c
--- linux-2.6.16.i686/security/selinux/xfrm.c	2006-06-13 10:46:59.000000000 -0400
+++ linux-2.6.16.i686-netlabel_06132006/security/selinux/xfrm.c	2006-06-13 11:20:00.000000000 -0400
@@ -295,13 +295,13 @@ u32 selinux_socket_getpeer_dgram(struct 
 /*
  * LSM hook that controls access to unlabelled packets.  If
  * a xfrm_state is authorizable (defined by macro) then it was
- * already authorized by the IPSec process.  If not, then
- * we need to check for unlabelled access since this may not have
- * gone thru the IPSec process.
+ * already authorized by the IPsec process.  Return zero when the
+ * packet has been approved by the IPsec process, negative values
+ * otherwise.
  */
 int selinux_xfrm_sock_rcv_skb(u32 isec_sid, struct sk_buff *skb)
 {
-	int i, rc = 0;
+	int i;
 	struct sec_path *sp;
 
 	sp = skb->sp;
@@ -317,21 +317,11 @@ int selinux_xfrm_sock_rcv_skb(u32 isec_s
 			struct xfrm_state *x = sp->xvec[i];
 
 			if (x && selinux_authorizable_xfrm(x))
-				goto accept;
+				return 0;
 		}
 	}
 
-	/* check SELinux sock for unlabelled access */
-	rc = avc_has_perm(isec_sid, SECINITSID_UNLABELED, SECCLASS_ASSOCIATION,
-			  ASSOCIATION__RECVFROM, NULL);
-	if (rc)
-		goto drop;
-
-accept:
-	return 0;
-
-drop:
-	return rc;
+	return -ENOMSG;
 }

This overlaps with Venkat's changes for MLS xfrm support.
 
 /*

-- 
Stephen Smalley
National Security Agency




More information about the redhat-lspp mailing list