Fix for IPsec leakage with SELinux enabled - V.03

From: James Morris <[email protected]>

When a security module is loaded (in this case, SELinux), the security_xfrm_policy_lookup() hook can return an access denied permission (or other error). We were not handling that correctly, and in fact inverting the return logic and propagating a false "ok" back up to xfrm_lookup(), which then allowed packets to pass as if they were not associated with an xfrm policy.

The way I was seeing the problem was when connecting via IPsec to a confined service on an SELinux box (vsftpd), which did not have the appropriate SELinux policy permissions to send packets via IPsec.

The first SYNACK would be blocked, because of an uncached lookup via flow_cache_lookup(), which would fail to resolve an xfrm policy because the SELinux policy is checked at that point via the resolver.

However, retransmitted SYNACKs would then find a cached flow entry when calling into flow_cache_lookup() with a null xfrm policy, which is interpreted by xfrm_lookup() as the packet not having any associated policy and similarly to the first case, allowing it to pass without transformation.

The solution presented here is to first ensure that errno values are correctly propagated all the way back up through the various call chains from security_xfrm_policy_lookup(), and handled correctly.

Then, flow_cache_lookup() is modified, so that if the policy resolver fails (typically a permission denied via the security module), the flow cache entry is killed rather than having a null policy assigned (which indicates that the packet can pass freely). This also forces any future lookups for the same flow to consult the security module (e.g. SELinux) for current security policy (rather than, say, caching the error on the flow cache entry).

---
 include/net/flow.h     |    2 -
 net/core/flow.c        |   42 ++++++++++++++++--------
 net/xfrm/xfrm_policy.c |   68 ++++++++++++++++++++++++++++++---------
 3 files changed, 82 insertions(+), 30 deletions(-)

diff -purN -X dontdiff net-2.6.o/include/net/flow.h net-2.6.w/include/net/flow.h
@@ -97,7 +97,7 @@ struct flowi {
 #define FLOW_DIR_FWD	2
 
 struct sock;
-typedef void (*flow_resolve_t)(struct flowi *key, u16 family, u8 dir,
+typedef int (*flow_resolve_t)(struct flowi *key, u16 family, u8 dir,
 			       void **objp, atomic_t **obj_refp);
 
 extern void *flow_cache_lookup(struct flowi *key, u16 family, u8 dir,

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

From: James Morris <[email protected]>
Date: Thu, 5 Oct 2006 16:54:38 -0400 (EDT)

We're trying to fill the flow cache here. In the case where we'd have a match in both the sub-policy and main table, I think the sub-policy is supposed to take precedence, and if you fail to get this sub-policy you should fail the entire lookup.

The way the sub-policied entries work is that you find the sub-policy as the primary object in the flow cache, and once you notice you have a sub-policy you do an explicit lookup in the main table to put the whole thing together.