Hi:
Here is my proposed solution to the tunnel security issues that I raised
previously. Please do not apply this patch as it is only meant to be
an illustration of what it should do.
I have changed policy_check to drop any packets containing an SA in
their security path that is not specified in the template. Optional
SAs in the template will continue to work.
IPCOMP tunnels have been dealt with by including the IPIP state in the
security path. It has the special property that it will match any IPCOMP
tunnel SA with the same outer source/destination address.
Please let me know of your opinion on this.
Cheers,
--
Debian GNU/Linux 3.0 is out! ( http://www.debian.org/ )
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
Index: kernel-source-2.5/net/ipv4/xfrm4_tunnel.c
===================================================================
RCS file: /home/gondolin/herbert/src/CVS/debian/kernel-source-2.5/net/ipv4/xfrm4_tunnel.c,v
retrieving revision 1.1.1.3
diff -u -r1.1.1.3 xfrm4_tunnel.c
--- kernel-source-2.5/net/ipv4/xfrm4_tunnel.c 17 Jun 2003 04:19:40 -0000 1.1.1.3
+++ kernel-source-2.5/net/ipv4/xfrm4_tunnel.c 12 Jul 2003 02:16:33 -0000
@@ -83,31 +83,8 @@
return err;
}
-static inline void ipip_ecn_decapsulate(struct iphdr *outer_iph, struct sk_buff *skb)
-{
- struct iphdr *inner_iph = skb->nh.iph;
-
- if (INET_ECN_is_ce(outer_iph->tos) &&
- INET_ECN_is_not_ce(inner_iph->tos))
- IP_ECN_set_ce(inner_iph);
-}
-
static int ipip_xfrm_rcv(struct xfrm_state *x, struct xfrm_decap_state *decap, struct sk_buff *skb)
{
- struct iphdr *outer_iph = skb->nh.iph;
-
- if (!pskb_may_pull(skb, sizeof(struct iphdr)))
- return -EINVAL;
- skb->mac.raw = skb->nh.raw;
- skb->nh.raw = skb->data;
- memset(&(IPCB(skb)->opt), 0, sizeof(struct ip_options));
- dst_release(skb->dst);
- skb->dst = NULL;
- skb->protocol = htons(ETH_P_IP);
- skb->pkt_type = PACKET_HOST;
- ipip_ecn_decapsulate(outer_iph, skb);
- netif_rx(skb);
-
return 0;
}
@@ -149,46 +126,12 @@
static int ipip_rcv(struct sk_buff *skb)
{
struct xfrm_tunnel *handler = ipip_handler;
- struct xfrm_state *x = NULL;
- int err;
/* Tunnel devices take precedence. */
- if (handler) {
- err = handler->handler(skb);
- if (!err)
- goto out;
- }
-
- x = xfrm_state_lookup((xfrm_address_t *)&skb->nh.iph->daddr,
- skb->nh.iph->saddr,
- IPPROTO_IPIP, AF_INET);
-
- if (!x)
- goto drop;
-
- spin_lock(&x->lock);
-
- if (unlikely(x->km.state != XFRM_STATE_VALID))
- goto drop_unlock;
-
- err = ipip_xfrm_rcv(x, NULL, skb);
- if (err)
- goto drop_unlock;
-
- x->curlft.bytes += skb->len;
- x->curlft.packets++;
- spin_unlock(&x->lock);
- xfrm_state_put(x);
-out:
- return err;
-
-drop_unlock:
- spin_unlock(&x->lock);
- xfrm_state_put(x);
-drop:
- err = NET_RX_DROP;
- kfree_skb(skb);
- goto out;
+ if (handler && handler->handler(skb) == 0)
+ return 0;
+
+ return xfrm4_rcv(skb);
}
static void ipip_err(struct sk_buff *skb, u32 info)
Index: kernel-source-2.5/net/xfrm/xfrm_input.c
===================================================================
RCS file: /home/gondolin/herbert/src/CVS/debian/kernel-source-2.5/net/xfrm/xfrm_input.c,v
retrieving revision 1.1.1.3
diff -u -r1.1.1.3 xfrm_input.c
--- kernel-source-2.5/net/xfrm/xfrm_input.c 2 Jul 2003 20:40:16 -0000 1.1.1.3
+++ kernel-source-2.5/net/xfrm/xfrm_input.c 12 Jul 2003 00:24:15 -0000
@@ -39,6 +39,12 @@
*spi = ntohl(ntohs(*(u16*)(skb->h.raw + 2)));
*seq = 0;
return 0;
+ case IPPROTO_IPIP:
+ if (!pskb_may_pull(skb, sizeof(struct iphdr)))
+ return -EINVAL;
+ *spi = skb->nh.iph->saddr;
+ *seq = 0;
+ return 0;
default:
return 1;
}
Index: kernel-source-2.5/net/xfrm/xfrm_policy.c
===================================================================
RCS file: /home/gondolin/herbert/src/CVS/debian/kernel-source-2.5/net/xfrm/xfrm_policy.c,v
retrieving revision 1.21
diff -u -r1.21 xfrm_policy.c
--- kernel-source-2.5/net/xfrm/xfrm_policy.c 12 Jul 2003 00:06:12 -0000 1.21
+++ kernel-source-2.5/net/xfrm/xfrm_policy.c 12 Jul 2003 02:30:42 -0000
@@ -858,6 +858,8 @@
xfrm_state_ok(struct xfrm_tmpl *tmpl, struct xfrm_state *x,
unsigned short family)
{
+ if (x->id.proto == IPPROTO_IPIP && tmpl->id.proto == IPPROTO_COMP)
+ return tmpl->mode && xfrm_state_addr_cmp(tmpl, x, family);
return x->id.proto == tmpl->id.proto &&
(x->id.spi == tmpl->id.spi || !tmpl->id.spi) &&
x->props.mode == tmpl->mode &&
@@ -865,17 +867,6 @@
!(x->props.mode && xfrm_state_addr_cmp(tmpl, x, family));
}
-static inline int
-xfrm_policy_ok(struct xfrm_tmpl *tmpl, struct sec_path *sp, int idx,
- unsigned short family)
-{
- for (; idx < sp->len; idx++) {
- if (xfrm_state_ok(tmpl, sp->x[idx].xvec, family))
- return ++idx;
- }
- return -1;
-}
-
static int
_decode_session(struct sk_buff *skb, struct flowi *fl, unsigned short family)
{
@@ -926,32 +917,38 @@
xfrm_policy_lookup);
if (!pol)
- return 1;
+ return !skb->sp;
pol->curlft.use_time = (unsigned long)xtime.tv_sec;
if (pol->action == XFRM_POLICY_ALLOW) {
- if (pol->xfrm_nr != 0) {
- struct sec_path *sp;
- static struct sec_path dummy;
- int i, k;
-
- if ((sp = skb->sp) == NULL)
- sp = &dummy;
-
- /* For each tmpl search corresponding xfrm.
- * Order is _important_. Later we will implement
- * some barriers, but at the moment barriers
- * are implied between each two transformations.
- */
- for (i = pol->xfrm_nr-1, k = 0; i >= 0; i--) {
+ struct sec_path *sp;
+ static struct sec_path dummy;
+ int i, k;
+
+ if ((sp = skb->sp) == NULL)
+ sp = &dummy;
+
+ /* For each tmpl search corresponding xfrm.
+ * Order is _important_. Later we will implement
+ * some barriers, but at the moment barriers
+ * are implied between each two transformations.
+ */
+ for (i = pol->xfrm_nr-1, k = 0; i >= 0; i--) {
+ if (k >= sp->len)
+ goto reject;
+ if (!xfrm_state_ok(pol->xfrm_vec+i,
+ sp->x[k].xvec, family)) {
if (pol->xfrm_vec[i].optional)
continue;
- k = xfrm_policy_ok(pol->xfrm_vec+i, sp, k, family);
- if (k < 0)
- goto reject;
+ goto reject;
}
+ k++;
}
+
+ if (k < sp->len)
+ goto reject;
+
xfrm_pol_put(pol);
return 1;
}
