mlsxfrm: Various fixes

PATCH In review Venkat Yekkirala 3 participants 10 messages Nov 7, 2006 – Nov 9, 2006

Subsystems (5)

Networking Network drivers SELinux security module Security (LSM) +1 more subsystems

lore

Share & tools

lore

Venkat Yekkirala

author

2006-11-07 17:17 UTC #0 lore raw

PATCH 2/3 mlsxfrm: Various fixes

Fix SO_PEERSEC for tcp sockets to return the security context of the peer (as represented by the SA from the peer) as opposed to the SA used by the local/source socket.

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

---
 include/linux/security.h        |   14 ++++++++++
 include/net/request_sock.h      |    1 
 net/ipv4/tcp_input.c            |    2 +
 security/dummy.c                |    6 ++++
 security/selinux/hooks.c        |   21 ++++++++++++---
 security/selinux/include/xfrm.h |   12 ++++-----
 security/selinux/xfrm.c         |   40 ++----------------------------
 7 files changed, 49 insertions(+), 47 deletions(-)

include/linux/security.h +14 -0

@@ -826,6 +826,8 @@ struct request_sock;
  *	Sets the openreq's sid to socket's sid with MLS portion taken from peer sid.
  * @inet_csk_clone:
  *	Sets the new child socket's sid to the openreq sid.
+ * @inet_conn_established:
+ *     Sets the connection's peersid to the secmark on skb.
  * @req_classify_flow:
  *	Sets the flow's sid to the openreq sid.
  *

@@ -1368,6 +1370,7 @@ struct security_operations {
 	int (*inet_conn_request)(struct sock *sk, struct sk_buff *skb,
 					struct request_sock *req);
 	void (*inet_csk_clone)(struct sock *newsk, const struct request_sock *req);
+	void (*inet_conn_established)(struct sock *sk, struct sk_buff *skb);
 	void (*req_classify_flow)(const struct request_sock *req, struct flowi *fl);
 #endif	/* CONFIG_SECURITY_NETWORK */

@@ -2961,6 +2964,12 @@ static inline void security_inet_csk_clo
 {
 	security_ops->inet_csk_clone(newsk, req);
 }
+
+static inline void security_inet_conn_established(struct sock *sk,
+			struct sk_buff *skb)
+{
+	security_ops->inet_conn_established(sk, skb);
+}
 #else	/* CONFIG_SECURITY_NETWORK */
 static inline int security_unix_stream_connect(struct socket * sock,
 					       struct socket * other,

@@ -3110,6 +3119,11 @@ static inline void security_inet_csk_clo
 			const struct request_sock *req)
 {
 }
+
+static inline void security_inet_conn_established(struct sock *sk,
+			struct sk_buff *skb)
+{
+}
 #endif	/* CONFIG_SECURITY_NETWORK */
 
 #ifdef CONFIG_SECURITY_NETWORK_XFRM

include/net/request_sock.h +1 -0

@@ -54,6 +54,7 @@ struct request_sock {
 	struct request_sock_ops		*rsk_ops;
 	struct sock			*sk;
 	u32				secid;
+	u32				peer_secid;
 };
 
 static inline struct request_sock *reqsk_alloc(struct request_sock_ops *ops)

net/ipv4/tcp_input.c +2 -0

@@ -4230,6 +4230,8 @@ static int tcp_rcv_synsent_state_process
 		mb();
 		tcp_set_state(sk, TCP_ESTABLISHED);
 
+		security_inet_conn_established(sk, skb);
+
 		/* Make sure socket is routed, for correct metrics.  */
 		icsk->icsk_af_ops->rebuild_header(sk);

security/dummy.c +6 -0

@@ -828,6 +828,11 @@ static inline void dummy_inet_csk_clone(
 {
 }
 
+static inline void dummy_inet_conn_established(struct sock *sk,
+			struct sk_buff *skb)
+{
+}
+
 static inline void dummy_req_classify_flow(const struct request_sock *req,
 			struct flowi *fl)
 {

@@ -1108,6 +1113,7 @@ void security_fixup_ops (struct security
 	set_to_dummy_if_null(ops, sock_graft);
 	set_to_dummy_if_null(ops, inet_conn_request);
 	set_to_dummy_if_null(ops, inet_csk_clone);
+	set_to_dummy_if_null(ops, inet_conn_established);
 	set_to_dummy_if_null(ops, req_classify_flow);
  #endif	/* CONFIG_SECURITY_NETWORK */
 #ifdef  CONFIG_SECURITY_NETWORK_XFRM

security/selinux/include/xfrm.h +6 -6

@@ -39,7 +39,6 @@ int selinux_xfrm_sock_rcv_skb(u32 sid, s
 			struct avc_audit_data *ad);
 int selinux_xfrm_postroute_last(u32 isec_sid, struct sk_buff *skb,
 			struct avc_audit_data *ad);
-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);
 #else

@@ -55,11 +54,6 @@ static inline int selinux_xfrm_postroute
 	return 0;
 }
 
-static inline int selinux_socket_getpeer_stream(struct sock *sk)
-{
-	return SECSID_NULL;
-}
-
 static inline int selinux_socket_getpeer_dgram(struct sk_buff *skb)
 {
 	return SECSID_NULL;

@@ -71,4 +65,10 @@ static inline int selinux_xfrm_decode_se
 }
 #endif
 
+static inline void selinux_skb_xfrm_sid(struct sk_buff *skb, u32 *sid)
+{
+	int err = selinux_xfrm_decode_session(skb, sid, 0);
+	BUG_ON(err);
+}
+
 #endif /* _SELINUX_XFRM_H_ */

security/selinux/hooks.c +17 -4

@@ -3528,8 +3528,10 @@ static int selinux_socket_getpeersec_str
 	}
 	else if (isec->sclass == SECCLASS_TCP_SOCKET) {
 		peer_sid = selinux_netlbl_socket_getpeersec_stream(sock);
-		if (peer_sid == SECSID_NULL)
-			peer_sid = selinux_socket_getpeer_stream(sock->sk);
+		if (peer_sid == SECSID_NULL) {
+			ssec = sock->sk->sk_security;
+			peer_sid = ssec->peer_sid;
+		}
 		if (peer_sid == SECSID_NULL) {
 			err = -ENOPROTOOPT;
 			goto out;

@@ -3640,11 +3642,11 @@ static int selinux_inet_conn_request(str
 		return 0;
 	}
 
-	err = selinux_xfrm_decode_session(skb, &peersid, 0);
-	BUG_ON(err);
+	selinux_skb_xfrm_sid(skb, &peersid);
 
 	if (peersid == SECSID_NULL) {
 		req->secid = sksec->sid;
+		req->peer_secid = 0;
 		return 0;
 	}

@@ -3653,6 +3655,7 @@ static int selinux_inet_conn_request(str
 		return err;
 
 	req->secid = newsid;
+	req->peer_secid = peersid;
 	return 0;
 }

@@ -3662,6 +3665,7 @@ static void selinux_inet_csk_clone(struc
 	struct sk_security_struct *newsksec = newsk->sk_security;
 
 	newsksec->sid = req->secid;
+	newsksec->peer_sid = req->peer_secid;
 	/* NOTE: Ideally, we should also get the isec->sid for the
 	   new socket in sync, but we don't have the isec available yet.
 	   So we will wait until sock_graft to do it, by which

@@ -3670,6 +3674,14 @@ static void selinux_inet_csk_clone(struc
 	selinux_netlbl_sk_security_init(newsksec, req->rsk_ops->family);
 }
 
+static void selinux_inet_conn_established(struct sock *sk,
+				struct sk_buff *skb)
+{
+	struct sk_security_struct *sksec = sk->sk_security;
+
+	selinux_skb_xfrm_sid(skb, &sksec->peer_sid);
+}
+
 static void selinux_req_classify_flow(const struct request_sock *req,
 				      struct flowi *fl)
 {

@@ -4732,6 +4744,7 @@ static struct security_operations selinu
 	.sock_graft =			selinux_sock_graft,
 	.inet_conn_request =		selinux_inet_conn_request,
 	.inet_csk_clone =		selinux_inet_csk_clone,
+	.inet_conn_established =	selinux_inet_conn_established,
 	.req_classify_flow =		selinux_req_classify_flow,
 
 #ifdef CONFIG_SECURITY_NETWORK_XFRM

security/selinux/xfrm.c +3 ~~-37~~

@@ -184,7 +184,8 @@ int selinux_xfrm_flow_state_match(struct
 }
 
 /*
- * LSM hook implementation that determines the sid for the session.
+ * LSM hook implementation that checks and/or returns the xfrm sid for the
+ * incoming packet.
  */
 
 int selinux_xfrm_decode_session(struct sk_buff *skb, u32 *sid, int ckall)

@@ -402,43 +403,8 @@ void selinux_xfrm_state_free(struct xfrm
 }
 
 /*
- * SELinux internal function to retrieve the context of a connected
- * (sk->sk_state == TCP_ESTABLISHED) TCP socket based on its security
- * association used to connect to the remote socket.
- *
- * Retrieve via getsockopt SO_PEERSEC.
- */
-u32 selinux_socket_getpeer_stream(struct sock *sk)
-{
-	struct dst_entry *dst, *dst_test;
-	u32 peer_sid = SECSID_NULL;
-
-	if (sk->sk_state != TCP_ESTABLISHED)
-		goto out;
-
-	dst = sk_dst_get(sk);
-	if (!dst)
-		goto out;
-
- 	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;
-			peer_sid = ctx->ctx_sid;
-			break;
-		}
-	}
-	dst_release(dst);
-
-out:
-	return peer_sid;
-}
-
-/*
  * SELinux internal function to retrieve the context of a UDP packet
- * based on its security association used to connect to the remote socket.
+ * based on its security association.
  *
  * Retrieve via setsockopt IP_PASSSEC and recvmsg with control message
  * type SCM_SECURITY.

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

maintainer

2006-11-07 20:38 UTC #1 lore raw

Quoted 3 lines · Venkat Yekkirala

Fix SO_PEERSEC for tcp sockets to return the security context of the peer (as represented by the SA from the peer) as opposed to the SA used by the local/source socket.

What about the case of a localhost TCP connection not using xfrm labeling?

Joe Nall raised this as an important requirement.

(Also, 'mlsxfrm' is MLS-specific).

- James

Signature · 7 lines

James Morris
<[email protected]>

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

Venkat Yekkirala

author

2006-11-08 14:31 UTC #2 lore raw

Quoted 8 lines

Quoted 3 lines
Fix SO_PEERSEC for tcp sockets to return the security context of the peer (as represented by the SA from the peer) as opposed to the SA used by the local/source socket.
What about the case of a localhost TCP connection not using xfrm labeling?
Joe Nall raised this as an important requirement.

Yes. We need to come up with some new ideas on this (the failed secid-recon patchset sought to do this using the secmark field on the skb).

The scope of this patchset is to strictly fix things related to labeled-xfrm.

Quoted 4 lines

(Also, 'mlsxfrm' is MLS-specific).

Will switch to "labeled-ipsec".

Paul Moore

2006-11-09 04:08 UTC #3 lore raw

Quoted 4 lines · Venkat Yekkirala

Quoted 4 lines
Quoted 3 lines
Fix SO_PEERSEC for tcp sockets to return the security context of the peer (as represented by the SA from the peer) as opposed to the SA used by the local/source socket.

Quoted 8 lines

Quoted 4 lines
What about the case of a localhost TCP connection not using xfrm labeling?
Joe Nall raised this as an important requirement.
Yes. We need to come up with some new ideas on this (the failed secid-recon patchset sought to do this using the secmark field on the skb).

You mentioned in an earlier thread that it may be possibile to enable XFRM for localhost via a sysctl variable; I would think this would make the most sense. I understand there is a performance hit due to IPsec being used, but I think this solution offers a few advantages:

1. Functionality is available right now, no additional kernel changes needed
2. No special handling for localhost, I tend to like the idea of having consistent behavior for all addresses/interfaces

Besides the performance penalty of IPsec and the untested nature of this solution is there some gotcha here which would prevent this from working?

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.

James Morris

maintainer

2006-11-09 04:38 UTC #4 lore raw

Quoted 3 lines · Paul Moore

1. Functionality is available right now, no additional kernel changes needed
2. No special handling for localhost, I tend to like the idea of having consistent behavior for all addresses/interfaces

I don't agree. SO_PEERSEC should always just work for loopback, just like with Unix sockets.

- James

Signature · 7 lines

James Morris
<[email protected]>

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

Paul Moore

2006-11-09 04:59 UTC #5 lore raw

Quoted 8 lines · James Morris

Quoted 3 lines · Paul Moore
1. Functionality is available right now, no additional kernel changes needed
2. No special handling for localhost, I tend to like the idea of having consistent behavior for all addresses/interfaces
I don't agree. SO_PEERSEC should always just work for loopback, just like with Unix sockets.

My main concern is that we would have "special" behavior for a single IP address
and that this behavior wouldn't be subject to the same labeled networking
configuration/management methods as the rest of the address space. Treating localhost like any other IP address seems consistent with the way we handle Unix sockets - we don't have any special handling depending on the path of the socket.

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.

James Morris

maintainer

2006-11-09 06:15 UTC #6 lore raw

Quoted 7 lines · Paul Moore

Quoted 6 lines · James Morris
Quoted 3 lines · Paul Moore
1. Functionality is available right now, no additional kernel changes needed
2. No special handling for localhost, I tend to like the idea of having consistent behavior for all addresses/interfaces

Quoted 6 lines

Quoted 2 lines
I don't agree. SO_PEERSEC should always just work for loopback, just like with Unix sockets.
My main concern is that we would have "special" behavior for a single IP address
and that this behavior wouldn't be subject to the same labeled networking
configuration/management methods as the rest of the address space.

It's a very special case, and loopack networking has lots of special case handling because of this. It's nearly zero cost to have this work, and then you get full SELinux control over local IP communications.

It doesn't prevent the IPsec stuff from working, if you want it to override the default. But would people really run IPsec for localhost communications?

Let's keep the simple case simple.

Quoted 3 lines

Treating localhost like any other IP address seems consistent with the
way we handle Unix sockets - we don't have any special handling depending on the path of the socket.

Unix sockets can't do both local and remote communication.

- James

Signature · 7 lines

James Morris
<[email protected]>

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

Paul Moore

2006-11-09 06:39 UTC #7 lore raw

Quoted 13 lines · James Morris

Quoted 11 lines · Paul Moore
Quoted 8 lines · James Morris
Quoted 3 lines · Paul Moore
1. Functionality is available right now, no additional kernel changes needed
2. No special handling for localhost, I tend to like the idea of having consistent behavior for all addresses/interfaces
I don't agree. SO_PEERSEC should always just work for loopback, just like with Unix sockets.

Quoted 7 lines

Quoted 3 lines
My main concern is that we would have "special" behavior for a single IP address
and that this behavior wouldn't be subject to the same labeled networking
configuration/management methods as the rest of the address space.
It's a very special case, and loopack networking has lots of special case handling because of this. It's nearly zero cost to have this work, and then you get full SELinux control over local IP communications.

It sounds like you have an idea of how you would like to see this implemented, can you give me a rough outline? Is this the partitioned SECMARK field you talked about earlier?

I'm asking because the only localhost SO_PEERSEC mechanism that I have seen that didn't require explicit packet labeling was the secid approach which I think we gave up on ...

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.

James Morris

maintainer

2006-11-09 07:02 UTC #8 lore raw

Quoted 3 lines · Paul Moore

It sounds like you have an idea of how you would like to see this implemented, can you give me a rough outline? Is this the partitioned SECMARK field you talked about earlier?

No, just the fact that you are in the same kernel address space and can readily access the security context of the peer.

- James

Signature · 7 lines

James Morris
<[email protected]>

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

Paul Moore

2006-11-09 17:26 UTC #9 lore raw

Quoted 8 lines · James Morris

Quoted 3 lines · Paul Moore
It sounds like you have an idea of how you would like to see this implemented, can you give me a rough outline? Is this the partitioned SECMARK field you talked about earlier?
No, just the fact that you are in the same kernel address space and can readily access the security context of the peer.

For a minute I got all excited thinking that you had found a solution to this :)

The problem I keep running into is that it is not obvious to me how we can determine the security context of the sending socket on the receive side by looking at the skb. I'm really hoping that it is just because I haven't looked at the code long enough, or thought about it hard enough. It is just so frustrating because you are right - all the information is there, I just don't know how to get to it when we need it without using external labeling.

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.