aboutsummaryrefslogtreecommitdiff
path: root/net/xfrm
diff options
context:
space:
mode:
authorJames Morris <jmorris@namei.org>2006-10-05 15:42:27 -0500
committerDavid S. Miller <davem@sunset.davemloft.net>2006-10-11 23:59:34 -0700
commit134b0fc544ba062498451611cb6f3e4454221b3d (patch)
tree84120e405d2bc7112b971fc82b718ae23991351b /net/xfrm
parent388b24057f90ba109d4bf855006a8809c383eb76 (diff)
IPsec: propagate security module errors up from flow_cache_lookup
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). Signed-off-by: James Morris <jmorris@namei.org>
Diffstat (limited to 'net/xfrm')
-rw-r--r--net/xfrm/xfrm_policy.c68
1 files changed, 53 insertions, 15 deletions
diff --git a/net/xfrm/xfrm_policy.c b/net/xfrm/xfrm_policy.c
index 2a7861661f1..fffdd34f3ba 100644
--- a/net/xfrm/xfrm_policy.c
+++ b/net/xfrm/xfrm_policy.c
@@ -883,30 +883,32 @@ out:
}
EXPORT_SYMBOL(xfrm_policy_walk);
-/* Find policy to apply to this flow. */
-
+/*
+ * Find policy to apply to this flow.
+ *
+ * Returns 0 if policy found, else an -errno.
+ */
static int xfrm_policy_match(struct xfrm_policy *pol, struct flowi *fl,
u8 type, u16 family, int dir)
{
struct xfrm_selector *sel = &pol->selector;
- int match;
+ int match, ret = -ESRCH;
if (pol->family != family ||
pol->type != type)
- return 0;
+ return ret;
match = xfrm_selector_match(sel, fl, family);
- if (match) {
- if (!security_xfrm_policy_lookup(pol, fl->secid, dir))
- return 1;
- }
+ if (match)
+ ret = security_xfrm_policy_lookup(pol, fl->secid, dir);
- return 0;
+ return ret;
}
static struct xfrm_policy *xfrm_policy_lookup_bytype(u8 type, struct flowi *fl,
u16 family, u8 dir)
{
+ int err;
struct xfrm_policy *pol, *ret;
xfrm_address_t *daddr, *saddr;
struct hlist_node *entry;
@@ -922,7 +924,15 @@ static struct xfrm_policy *xfrm_policy_lookup_bytype(u8 type, struct flowi *fl,
chain = policy_hash_direct(daddr, saddr, family, dir);
ret = NULL;
hlist_for_each_entry(pol, entry, chain, bydst) {
- if (xfrm_policy_match(pol, fl, type, family, dir)) {
+ err = xfrm_policy_match(pol, fl, type, family, dir);
+ if (err) {
+ if (err == -ESRCH)
+ continue;
+ else {
+ ret = ERR_PTR(err);
+ goto fail;
+ }
+ } else {
ret = pol;
priority = ret->priority;
break;
@@ -930,36 +940,53 @@ static struct xfrm_policy *xfrm_policy_lookup_bytype(u8 type, struct flowi *fl,
}
chain = &xfrm_policy_inexact[dir];
hlist_for_each_entry(pol, entry, chain, bydst) {
- if (xfrm_policy_match(pol, fl, type, family, dir) &&
- pol->priority < priority) {
+ err = xfrm_policy_match(pol, fl, type, family, dir);
+ if (err) {
+ if (err == -ESRCH)
+ continue;
+ else {
+ ret = ERR_PTR(err);
+ goto fail;
+ }
+ } else if (pol->priority < priority) {
ret = pol;
break;
}
}
if (ret)
xfrm_pol_hold(ret);
+fail:
read_unlock_bh(&xfrm_policy_lock);
return ret;
}
-static void xfrm_policy_lookup(struct flowi *fl, u16 family, u8 dir,
+static int xfrm_policy_lookup(struct flowi *fl, u16 family, u8 dir,
void **objp, atomic_t **obj_refp)
{
struct xfrm_policy *pol;
+ int err = 0;
#ifdef CONFIG_XFRM_SUB_POLICY
pol = xfrm_policy_lookup_bytype(XFRM_POLICY_TYPE_SUB, fl, family, dir);
- if (pol)
+ if (IS_ERR(pol)) {
+ err = PTR_ERR(pol);
+ pol = NULL;
+ }
+ if (pol || err)
goto end;
#endif
pol = xfrm_policy_lookup_bytype(XFRM_POLICY_TYPE_MAIN, fl, family, dir);
-
+ if (IS_ERR(pol)) {
+ err = PTR_ERR(pol);
+ pol = NULL;
+ }
#ifdef CONFIG_XFRM_SUB_POLICY
end:
#endif
if ((*objp = (void *) pol) != NULL)
*obj_refp = &pol->refcnt;
+ return err;
}
static inline int policy_to_flow_dir(int dir)
@@ -1297,6 +1324,8 @@ restart:
policy = flow_cache_lookup(fl, dst_orig->ops->family,
dir, xfrm_policy_lookup);
+ if (IS_ERR(policy))
+ return PTR_ERR(policy);
}
if (!policy)
@@ -1343,6 +1372,10 @@ restart:
fl, family,
XFRM_POLICY_OUT);
if (pols[1]) {
+ if (IS_ERR(pols[1])) {
+ err = PTR_ERR(pols[1]);
+ goto error;
+ }
if (pols[1]->action == XFRM_POLICY_BLOCK) {
err = -EPERM;
goto error;
@@ -1581,6 +1614,9 @@ int __xfrm_policy_check(struct sock *sk, int dir, struct sk_buff *skb,
pol = flow_cache_lookup(&fl, family, fl_dir,
xfrm_policy_lookup);
+ if (IS_ERR(pol))
+ return 0;
+
if (!pol) {
if (skb->sp && secpath_has_nontransport(skb->sp, 0, &xerr_idx)) {
xfrm_secpath_reject(xerr_idx, skb, &fl);
@@ -1599,6 +1635,8 @@ int __xfrm_policy_check(struct sock *sk, int dir, struct sk_buff *skb,
&fl, family,
XFRM_POLICY_IN);
if (pols[1]) {
+ if (IS_ERR(pols[1]))
+ return 0;
pols[1]->curlft.use_time = (unsigned long)xtime.tv_sec;
npols ++;
}