The patches below have more information on the problem but the issue is that
we are not correctly labeling incoming TCP connection sockets at present.  Due
to a variety of things all coming together we've gotten a bit lucky in that
the majority of labeled TCP connections are labeled correctly now but this
won't hold for long as some of the new features added recently will break this
"happy coincidence" for some configurations.

For the folks on netdev, you probably only care about the relocation of the
security_inet_conn_request() hooks but this is such a minor move I doubt there
will be much objection.  Still, I would appreciate any feedback/comments.

For the folks on the SELinux and LSM lists please take a look and let me know
if the basic approach seems reasonable.  Keep in mind I still need to make a
few more passes through the code to clean it up and make sure I haven't done
anything stupid.  I've done basic testing under both SELinux and Smack using
2.6.29-rc7 and everything looks okay so far so feel free to play with it but
more testing is still needed.

---

Paul Moore (3):
      netlabel: Label incoming TCP connections correctly in Smack
      netlabel: Label incoming TCP connections correctly in SELinux
      lsm: Relocate the IPv4 security_inet_conn_request() hooks


 include/net/cipso_ipv4.h            |   17 +++
 include/net/netlabel.h              |   17 +++
 net/ipv4/cipso_ipv4.c               |  130 ++++++++++++++++++++++--
 net/ipv4/syncookies.c               |    9 +-
 net/ipv4/tcp_ipv4.c                 |    7 +
 net/netlabel/netlabel_kapi.c        |  124 +++++++++++++++++++++--
 security/selinux/hooks.c            |   54 +++-------
 security/selinux/include/netlabel.h |   26 ++---
 security/selinux/netlabel.c         |  187 ++++++++++-------------------------
 security/smack/smack_lsm.c          |   53 +++++++---
 10 files changed, 393 insertions(+), 231 deletions(-)

--

[ message continues ]

The current placement of the security_inet_conn_request() hooks do not allow
individual LSMs to override the IP options of the connection's request_sock.
This is a problem as both SELinux and Smack have the ability to use labeled
networking protocols which make use of IP options to carry security attributes
and the inability to set the IP options at the start of the TCP handshake is
problematic.

This patch moves the IPv4 security_inet_conn_request() hooks past the code
where the request_sock's IP options are set/reset so that the LSM can safely
manipulate the IP options as needed.  This patch intentionally does not change
the related IPv6 hooks as IPv6 based labeling protocols which use IPv6 options
are not currently implemented, once they are we will have a better idea of
the correct placement for the IPv6 hooks.
---

 net/ipv4/syncookies.c |    9 +++++----
 net/ipv4/tcp_ipv4.c   |    7 ++++---
 2 files changed, 9 insertions(+), 7 deletions(-)

diff --git a/net/ipv4/syncookies.c b/net/ipv4/syncookies.c
index d346c22..b35a950 100644
--- a/net/ipv4/syncookies.c
+++ b/net/ipv4/syncookies.c
@@ -288,10 +288,6 @@ struct sock *cookie_v4_check(struct sock *sk, struct sk_buff *skb,
 	if (!req)
 		goto out;
 
-	if (security_inet_conn_request(sk, skb, req)) {
-		reqsk_free(req);
-		goto out;
-	}
 	ireq = inet_rsk(req);
 	treq = tcp_rsk(req);
 	treq->rcv_isn		= ntohl(th->seq) - 1;
@@ -322,6 +318,11 @@ struct sock *cookie_v4_check(struct sock *sk, struct sk_buff *skb,
 		}
 	}
 
+	if (security_inet_conn_request(sk, skb, req)) {
+		reqsk_free(req);
+		goto out;
+	}
+
 	req->expires	= 0UL;
 	req->retrans	= 0;
 
diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
index cf74c41..5499c28 100644
--- a/net/ipv4/tcp_ipv4.c
+++ b/net/ipv4/tcp_ipv4.c
@@ -1239,14 +1239,15 @@ int tcp_v4_conn_request(struct sock *sk, struct sk_buff *skb)
 
 	tcp_openreq_init(req, &tmp_opt, skb);
 
-	if (security_inet_conn_request(sk, skb, req))
-		goto drop_and_free;
-
 	ireq = ...
[ message continues ]

From: Paul Moore <paul.moore@hp.com>

This looks OK to me:

Acked-by: David S. Miller <davem@davemloft.net>
--

[ message continues ]

Great, thanks for taking a look.

-- 
paul moore
linux @ hp

--

[ message continues ]

This patch labels incoming TCP connections correctly in a manner very similar
to SELinux using the security_inet_conn_request() to label the request_sock.
---

 include/net/netlabel.h       |    5 ++++
 net/netlabel/netlabel_kapi.c |   13 +++++++++++
 security/smack/smack_lsm.c   |   51 ++++++++++++++++++++++++++++++++----------
 3 files changed, 57 insertions(+), 12 deletions(-)

diff --git a/include/net/netlabel.h b/include/net/netlabel.h
index bdb10e5..60ebbc1 100644
--- a/include/net/netlabel.h
+++ b/include/net/netlabel.h
@@ -417,6 +417,7 @@ int netlbl_conn_setattr(struct sock *sk,
 			const struct netlbl_lsm_secattr *secattr);
 int netlbl_req_setattr(struct request_sock *req,
 		       const struct netlbl_lsm_secattr *secattr);
+void netlbl_req_delattr(struct request_sock *req);
 int netlbl_skbuff_setattr(struct sk_buff *skb,
 			  u16 family,
 			  const struct netlbl_lsm_secattr *secattr);
@@ -547,6 +548,10 @@ static inline int netlbl_req_setattr(struct request_sock *req,
 {
 	return -ENOSYS;
 }
+static inline void netlbl_req_delattr(struct request_sock *req)
+{
+	return;
+}
 static inline int netlbl_skbuff_setattr(struct sk_buff *skb,
 				      u16 family,
 				      const struct netlbl_lsm_secattr *secattr)
diff --git a/net/netlabel/netlabel_kapi.c b/net/netlabel/netlabel_kapi.c
index d2e6b5a..ff43eda 100644
--- a/net/netlabel/netlabel_kapi.c
+++ b/net/netlabel/netlabel_kapi.c
@@ -844,6 +844,19 @@ req_setattr_return:
 }
 
 /**
+* netlbl_req_delattr - Delete all the NetLabel labels on a socket
+* @req: the socket
+*
+* Description:
+* Remove all the NetLabel labeling from @req.
+*
+*/
+void netlbl_req_delattr(struct request_sock *req)
+{
+	cipso_v4_req_delattr(req);
+}
+
+/**
  * netlbl_skbuff_setattr - Label a packet using the correct protocol
  * @skb: the packet
  * @family: protocol family
diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
index 25e953f..b648ac2 100644
--- a/security/smack/smack_lsm.c
+++ ...
[ message continues ]

The current NetLabel/SELinux behavior for incoming TCP connections works but
only through a series of happy coincidences that rely on the limited nature of
standard CIPSO (only able to convey MLS attributes) and the write equality
imposed by the SELinux MLS constraints.  The problem is that network sockets
created as the result of an incoming TCP connection were not on-the-wire
labeled based on the security attributes of the parent socket but rather based
on the wire label of the remote peer.  The issue had to do with how IP options
were managed as part of the network stack and where the LSM hooks were in
relation to the code which set the IP options on these newly created child
sockets.  While NetLabel/SELinux did correctly set the socket's on-the-wire
label it was promptly cleared by the network stack and reset based on the IP
options of the remote peer.

This patch, in conjunction with a prior patch that adjusted the LSM hook
locations, works to set the correct on-the-wire label format for new incoming
connections through the security_inet_conn_request() hook.  Besides the
correct behavior there are many advantages to this change, the most significant
is that all of the NetLabel socket labeling code in SELinux now lives in hooks
which can return error codes to the core stack which allows us to finally get
ride of the selinux_netlbl_inode_permission() logic which greatly simplfies
the NetLabel/SELinux glue code.
---

 include/net/cipso_ipv4.h            |   17 +++
 include/net/netlabel.h              |   12 ++
 net/ipv4/cipso_ipv4.c               |  130 ++++++++++++++++++++++--
 net/netlabel/netlabel_kapi.c        |  111 +++++++++++++++++++--
 security/selinux/hooks.c            |   54 +++-------
 security/selinux/include/netlabel.h |   26 ++---
 security/selinux/netlabel.c         |  187 ++++++++++-------------------------
 security/smack/smack_lsm.c          |    2 
 8 files changed, 327 insertions(+), 212 deletions(-)

diff --git a/include/net/cipso_ipv4.h ...
[ message continues ]