From patchwork Wed Mar 31 22:58:23 2010 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: jamal X-Patchwork-Id: 49180 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 4C0C6B7D06 for ; Thu, 1 Apr 2010 09:58:34 +1100 (EST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1758411Ab0CaW63 (ORCPT ); Wed, 31 Mar 2010 18:58:29 -0400 Received: from mail-qy0-f179.google.com ([209.85.221.179]:51600 "EHLO mail-qy0-f179.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1758362Ab0CaW62 (ORCPT ); Wed, 31 Mar 2010 18:58:28 -0400 Received: by qyk9 with SMTP id 9so636267qyk.1 for ; Wed, 31 Mar 2010 15:58:27 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:received:received:sender:subject:from:reply-to :to:cc:in-reply-to:references:content-type:date:message-id :mime-version:x-mailer; bh=H8O18TI3+zCnsQJwCnffJ598M9XB+lf9K50g965B7JU=; b=S3YJhqpMqKuHaclzIHDfObNwypL/3owrnLbV2Qjyv9QY/mh9oNqOJH/AxrMwP8B3jb PI8OZ4ERg4LWMIKEraCjF0C8ePPAhgWQcNqjSnaIyWCRm0SY92z29BGiuZ16llHJokxu pIXlq0qAFRnrDJC5plhyjGL3u5As+W53CTwH4= DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=sender:subject:from:reply-to:to:cc:in-reply-to:references :content-type:date:message-id:mime-version:x-mailer; b=j634lUhiNROio7W+PvuuPE65zThQF2AQLaNagm3ThhfjemSL7mknT+RGs0jED+JI1P ULOUwHap3MTb0A0MXCTftc31WPep/6i/sLvPrXmYaZBq0+pprt29pljRbpTgMYGcWQT3 Z9xpFCLaM+BbPq+nzrquzVrLT3P6PJ3nAbRTk= Received: by 10.224.105.80 with SMTP id s16mr2781406qao.337.1270076307591; Wed, 31 Mar 2010 15:58:27 -0700 (PDT) Received: from [10.0.0.26] (CPE0030ab124d2f-CM001bd7a7f1a0.cpe.net.cable.rogers.com [99.240.66.42]) by mx.google.com with ESMTPS id 4sm12964721qwe.32.2010.03.31.15.58.25 (version=SSLv3 cipher=RC4-MD5); Wed, 31 Mar 2010 15:58:26 -0700 (PDT) Subject: Re: [RFC] SPD basic actions per netdev From: jamal Reply-To: hadi@cyberus.ca To: Herbert Xu Cc: Timo Teras , "David S. Miller" , Patrick McHardy , netdev@vger.kernel.org In-Reply-To: <1270053478.26743.111.camel@bigi> References: <1270053478.26743.111.camel@bigi> Date: Wed, 31 Mar 2010 18:58:23 -0400 Message-Id: <1270076303.26743.119.camel@bigi> Mime-Version: 1.0 X-Mailer: Evolution 2.26.1 Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org And here's something i just tested on net-next that fixes this for me. cheers, jamal On Wed, 2010-03-31 at 12:38 -0400, jamal wrote: > This may be oversight in current implementation and possibly > nobody has needed it before - hence it is not functional. > > I want to have a drop-all policy on a per-interface level > for incoming packets and add exceptions as i need them. > [using the flow table is cheap if you have xfrm built in]. > i.e something along the lines of: > > #eth0, wild-card drop all > ip xfrm policy add src 0.0.0.0/0 dst 0.0.0.0/0 dev eth0 \ > dir in ptype main action block priority $SOME-HIGH-value > #eth0, exception > ip xfrm policy add blah blah dev eth0 \ > dir in ptype main action allow priority $SOME-small-value > #eth1, wild-card drop all > ip xfrm policy add src 0.0.0.0/0 dst 0.0.0.0/0 dev eth1 \ > dir in ptype main action block priority $SOME-HIGH-value > #eth1 exception ... > > The problem is this works as long as i dont specify an interface. > i.e, this would work in the in-direction: > > ip xfrm policy add src 0.0.0.0/0 dst 0.0.0.0/0 \ > dir in ptype main action block priority $SOME-HIGH-value > > This would not work: > ip xfrm policy add src 0.0.0.0/0 dst 0.0.0.0/0 dev eth0 \ > dir in ptype main action block priority $SOME-HIGH-value > > > The checks in the selector matching is the culprit, example for v4: > > __xfrm4_selector_match(struct xfrm_selector *sel, struct flowi *fl) > { > return .... && > .... && > (fl->oif == sel->ifindex || !sel->ifindex); > } > > i.e in the second case i have a non-zero sel->ifindex but > a zero fl->oif; so it wont match. > > One approach to fix this is to pass the direction then i can do > in the function call, then i can do something along the lines of > matching if: > (fl_dir == FLOW_DIR_IN && (fl->iif == sel->ifindex || !sel->ifindex) || > (fl->oif == sel->ifindex || !sel->ifindex); > > Is there any reason the selector matching only assumes fl->oif? > Are there any unforeseen issues/breakages if i added a check for the > above. > > cheers, > jamal diff --git a/include/net/xfrm.h b/include/net/xfrm.h index d74e080..ce18464 100644 --- a/include/net/xfrm.h +++ b/include/net/xfrm.h @@ -838,7 +838,7 @@ __be16 xfrm_flowi_dport(struct flowi *fl) } extern int xfrm_selector_match(struct xfrm_selector *sel, struct flowi *fl, - unsigned short family); + unsigned short family, int use_if); #ifdef CONFIG_SECURITY_NETWORK_XFRM /* If neither has a context --> match diff --git a/net/xfrm/xfrm_policy.c b/net/xfrm/xfrm_policy.c index 843e066..e7e230b 100644 --- a/net/xfrm/xfrm_policy.c +++ b/net/xfrm/xfrm_policy.c @@ -55,35 +55,37 @@ static struct xfrm_policy *__xfrm_policy_unlink(struct xfrm_policy *pol, int dir); static inline int -__xfrm4_selector_match(struct xfrm_selector *sel, struct flowi *fl) +__xfrm4_selector_match(struct xfrm_selector *sel, struct flowi *fl, int uif) { + int use_if = uif ? uif : fl->oif; return addr_match(&fl->fl4_dst, &sel->daddr, sel->prefixlen_d) && addr_match(&fl->fl4_src, &sel->saddr, sel->prefixlen_s) && !((xfrm_flowi_dport(fl) ^ sel->dport) & sel->dport_mask) && !((xfrm_flowi_sport(fl) ^ sel->sport) & sel->sport_mask) && (fl->proto == sel->proto || !sel->proto) && - (fl->oif == sel->ifindex || !sel->ifindex); + (use_if == sel->ifindex || !sel->ifindex); } static inline int -__xfrm6_selector_match(struct xfrm_selector *sel, struct flowi *fl) +__xfrm6_selector_match(struct xfrm_selector *sel, struct flowi *fl, int uif) { + int use_if = uif ? uif : fl->oif; return addr_match(&fl->fl6_dst, &sel->daddr, sel->prefixlen_d) && addr_match(&fl->fl6_src, &sel->saddr, sel->prefixlen_s) && !((xfrm_flowi_dport(fl) ^ sel->dport) & sel->dport_mask) && !((xfrm_flowi_sport(fl) ^ sel->sport) & sel->sport_mask) && (fl->proto == sel->proto || !sel->proto) && - (fl->oif == sel->ifindex || !sel->ifindex); + (use_if == sel->ifindex || !sel->ifindex); } int xfrm_selector_match(struct xfrm_selector *sel, struct flowi *fl, - unsigned short family) + unsigned short family, int ifindex) { switch (family) { case AF_INET: - return __xfrm4_selector_match(sel, fl); + return __xfrm4_selector_match(sel, fl, ifindex); case AF_INET6: - return __xfrm6_selector_match(sel, fl); + return __xfrm6_selector_match(sel, fl, ifindex); } return 0; } @@ -917,14 +919,17 @@ 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, ret = -ESRCH; + int use_if = fl->oif, match, ret = -ESRCH; if (pol->family != family || (fl->mark & pol->mark.m) != pol->mark.v || pol->type != type) return ret; - match = xfrm_selector_match(sel, fl, family); + if (dir == FLOW_DIR_IN) + use_if = fl->iif; + + match = xfrm_selector_match(sel, fl, family, use_if); if (match) ret = security_xfrm_policy_lookup(pol->security, fl->secid, dir); @@ -1041,7 +1046,7 @@ static struct xfrm_policy *xfrm_sk_policy_lookup(struct sock *sk, int dir, struc read_lock_bh(&xfrm_policy_lock); if ((pol = sk->sk_policy[dir]) != NULL) { int match = xfrm_selector_match(&pol->selector, fl, - sk->sk_family); + sk->sk_family, 0); int err = 0; if (match) { @@ -1918,6 +1923,7 @@ int __xfrm_policy_check(struct sock *sk, int dir, struct sk_buff *skb, struct flowi fl; u8 fl_dir; int xerr_idx = -1; + int use_if = 0; reverse = dir & ~XFRM_POLICY_MASK; dir &= XFRM_POLICY_MASK; @@ -1928,6 +1934,9 @@ int __xfrm_policy_check(struct sock *sk, int dir, struct sk_buff *skb, return 0; } + if (fl_dir == FLOW_DIR_IN) + use_if = fl.iif = skb->skb_iif; + nf_nat_decode_session(skb, &fl, family); /* First, check used SA against their selectors. */ @@ -1936,7 +1945,7 @@ int __xfrm_policy_check(struct sock *sk, int dir, struct sk_buff *skb, for (i=skb->sp->len-1; i>=0; i--) { struct xfrm_state *x = skb->sp->xvec[i]; - if (!xfrm_selector_match(&x->sel, &fl, family)) { + if (!xfrm_selector_match(&x->sel, &fl, family, use_if)) { XFRM_INC_STATS(net, LINUX_MIB_XFRMINSTATEMISMATCH); return 0; } @@ -1956,6 +1965,7 @@ int __xfrm_policy_check(struct sock *sk, int dir, struct sk_buff *skb, pol = flow_cache_lookup(net, &fl, family, fl_dir, xfrm_policy_lookup); + if (IS_ERR(pol)) { XFRM_INC_STATS(net, LINUX_MIB_XFRMINPOLERROR); return 0; @@ -2243,7 +2253,7 @@ int xfrm_bundle_ok(struct xfrm_policy *pol, struct xfrm_dst *first, if (first->origin && !flow_cache_uli_match(first->origin, fl)) return 0; if (first->partner && - !xfrm_selector_match(first->partner, fl, family)) + !xfrm_selector_match(first->partner, fl, family, 0)) return 0; } #endif @@ -2253,7 +2263,7 @@ int xfrm_bundle_ok(struct xfrm_policy *pol, struct xfrm_dst *first, do { struct xfrm_dst *xdst = (struct xfrm_dst *)dst; - if (fl && !xfrm_selector_match(&dst->xfrm->sel, fl, family)) + if (fl && !xfrm_selector_match(&dst->xfrm->sel, fl, family, 0)) return 0; if (fl && pol && !security_xfrm_state_pol_flow_match(dst->xfrm, pol, fl)) diff --git a/net/xfrm/xfrm_state.c b/net/xfrm/xfrm_state.c index 17d5b96..f47c90b 100644 --- a/net/xfrm/xfrm_state.c +++ b/net/xfrm/xfrm_state.c @@ -756,7 +756,7 @@ static void xfrm_state_look_at(struct xfrm_policy *pol, struct xfrm_state *x, */ if (x->km.state == XFRM_STATE_VALID) { if ((x->sel.family && - !xfrm_selector_match(&x->sel, fl, x->sel.family)) || + !xfrm_selector_match(&x->sel, fl, x->sel.family, 0)) || !security_xfrm_state_pol_flow_match(x, pol, fl)) return; @@ -769,7 +769,7 @@ static void xfrm_state_look_at(struct xfrm_policy *pol, struct xfrm_state *x, *acq_in_progress = 1; } else if (x->km.state == XFRM_STATE_ERROR || x->km.state == XFRM_STATE_EXPIRED) { - if (xfrm_selector_match(&x->sel, fl, x->sel.family) && + if (xfrm_selector_match(&x->sel, fl, x->sel.family, 0) && security_xfrm_state_pol_flow_match(x, pol, fl)) *error = -ESRCH; }