2004-09-11 19:41:52

by Paul Komkoff

[permalink] [raw]
Subject: [PATCH] [RFC] Support for wccp version 1 and 2 in ip_gre.c

Hello!

Some time ago I posted the following patch.
It indeed adds support for wccp version 1 and 2 decapsulation in
ip_gre.c. But there is an open question.
I surrounded all decapsulation stuff in if (1).
Which knob I should use instead of that 1 to make this change
acceptable into mainline kernel?

Thanks in advance.

diff -urN linux-2.6.6-1.435/net/ipv4/ip_gre.c linux-2.6.6-1.435a/net/ipv4/ip_gre.c
--- linux-2.6.6-1.435/net/ipv4/ip_gre.c 2004-05-10 06:32:54.000000000 +0400
+++ linux-2.6.6-1.435a/net/ipv4/ip_gre.c 2004-06-30 16:38:43.781838344 +0400
@@ -553,6 +553,8 @@
return INET_ECN_encapsulate(tos, inner);
}

+#define ETH_P_WCCP 0x883E
+
int ipgre_rcv(struct sk_buff *skb)
{
struct iphdr *iph;
@@ -605,13 +607,21 @@
if ((tunnel = ipgre_tunnel_lookup(iph->saddr, iph->daddr, key)) != NULL) {
secpath_reset(skb);

+ skb->protocol = *(u16*)(h + 2);
+ if (1) {
+ if ((flags == 0) && (skb->protocol == __constant_htons(ETH_P_WCCP))) {
+ skb->protocol = __constant_htons(ETH_P_IP);
+ if ((*(h + offset) & 0xF0) != 0x40)
+ offset += 4;
+ }
+ }
+
skb->mac.raw = skb->nh.raw;
skb->nh.raw = __pskb_pull(skb, offset);
memset(&(IPCB(skb)->opt), 0, sizeof(struct ip_options));
if (skb->ip_summed == CHECKSUM_HW)
skb->csum = csum_sub(skb->csum,
csum_partial(skb->mac.raw, skb->nh.raw-skb->mac.raw, 0));
- skb->protocol = *(u16*)(h + 2);
skb->pkt_type = PACKET_HOST;
#ifdef CONFIG_NET_IPGRE_BROADCAST
if (MULTICAST(iph->daddr)) {

--
Paul P 'Stingray' Komkoff Jr // http://stingr.net/key <- my pgp key
This message represents the official view of the voices in my head


2004-09-13 00:06:39

by David Miller

[permalink] [raw]
Subject: Re: [PATCH] [RFC] Support for wccp version 1 and 2 in ip_gre.c

On Sat, 11 Sep 2004 23:41:08 +0400
Paul P Komkoff Jr <[email protected]> wrote:

> Some time ago I posted the following patch.
> It indeed adds support for wccp version 1 and 2 decapsulation in
> ip_gre.c. But there is an open question.
> I surrounded all decapsulation stuff in if (1).
> Which knob I should use instead of that 1 to make this change
> acceptable into mainline kernel?

What are the rules for IP_GRE in general for when
to apply this transformation?

Please do me a favor also, and redo your patch by putting
ETH_P_WCCP into include/linux/if_ether.h where it belongs.

Thanks.

2004-09-13 05:17:14

by Paul Komkoff

[permalink] [raw]
Subject: Re: [PATCH] [RFC] Support for wccp version 1 and 2 in ip_gre.c

Replying to David S. Miller:
> What are the rules for IP_GRE in general for when
> to apply this transformation?

As you can see, I am applying it unconditionally when fits. For most
cases, this will be OK.
There can be situations when this is not wanted (for example, when
debugging something), so in general, tuning knob will be useful, but
I just don't know where to add it, maybe tunnel->parms.i_flags ...

> Please do me a favor also, and redo your patch by putting
> ETH_P_WCCP into include/linux/if_ether.h where it belongs.

Done.

> Thanks.


diff -urN /usr/src/linux-2.6.8-1.521/include/linux/if_ether.h linux-2.6.8-1.521wccp/include/linux/if_ether.h
--- /usr/src/linux-2.6.8-1.521/include/linux/if_ether.h 2004-08-14 09:37:15.000000000 +0400
+++ linux-2.6.8-1.521wccp/include/linux/if_ether.h 2004-09-13 08:51:02.707155288 +0400
@@ -59,6 +59,8 @@
#define ETH_P_8021Q 0x8100 /* 802.1Q VLAN Extended Header */
#define ETH_P_IPX 0x8137 /* IPX over DIX */
#define ETH_P_IPV6 0x86DD /* IPv6 over bluebook */
+#define ETH_P_WCCP 0x883E /* Web-cache coordination protocol
+ * defined in draft-wilson-wrec-wccp-v2-00.txt */
#define ETH_P_PPP_DISC 0x8863 /* PPPoE discovery messages */
#define ETH_P_PPP_SES 0x8864 /* PPPoE session messages */
#define ETH_P_MPLS_UC 0x8847 /* MPLS Unicast traffic */
diff -urN /usr/src/linux-2.6.8-1.521/net/ipv4/ip_gre.c linux-2.6.8-1.521wccp/net/ipv4/ip_gre.c
--- /usr/src/linux-2.6.8-1.521/net/ipv4/ip_gre.c 2004-08-14 09:37:37.000000000 +0400
+++ linux-2.6.8-1.521wccp/net/ipv4/ip_gre.c 2004-09-13 09:02:33.293170256 +0400
@@ -605,13 +605,25 @@
if ((tunnel = ipgre_tunnel_lookup(iph->saddr, iph->daddr, key)) != NULL) {
secpath_reset(skb);

+ skb->protocol = *(u16*)(h + 2);
+ /* WCCP version 1 and 2 protocol decoding.
+ * - Change protocol to IP
+ * - When dealing with WCCPv2, Skip extra 4 bytes in GRE header
+ */
+ if (1) {
+ if ((flags == 0) && (skb->protocol == __constant_htons(ETH_P_WCCP))) {
+ skb->protocol = __constant_htons(ETH_P_IP);
+ if ((*(h + offset) & 0xF0) != 0x40)
+ offset += 4;
+ }
+ }
+
skb->mac.raw = skb->nh.raw;
skb->nh.raw = __pskb_pull(skb, offset);
memset(&(IPCB(skb)->opt), 0, sizeof(struct ip_options));
if (skb->ip_summed == CHECKSUM_HW)
skb->csum = csum_sub(skb->csum,
csum_partial(skb->mac.raw, skb->nh.raw-skb->mac.raw, 0));
- skb->protocol = *(u16*)(h + 2);
skb->pkt_type = PACKET_HOST;
#ifdef CONFIG_NET_IPGRE_BROADCAST
if (MULTICAST(iph->daddr)) {

--
Paul P 'Stingray' Komkoff Jr // http://stingr.net/key <- my pgp key
This message represents the official view of the voices in my head

2004-09-13 23:21:05

by David Miller

[permalink] [raw]
Subject: Re: [PATCH] [RFC] Support for wccp version 1 and 2 in ip_gre.c

On Mon, 13 Sep 2004 09:17:06 +0400
Paul P Komkoff Jr <[email protected]> wrote:

> Replying to David S. Miller:
> > What are the rules for IP_GRE in general for when
> > to apply this transformation?
>
> As you can see, I am applying it unconditionally when fits. For most
> cases, this will be OK.
> There can be situations when this is not wanted (for example, when
> debugging something), so in general, tuning knob will be useful, but
> I just don't know where to add it, maybe tunnel->parms.i_flags ...

I don't think adding such a knob is necessary, but yes i_flags
would be the place to do it.

I will apply your patch with the "if(1)" simply removed.

Thanks.

2004-09-14 09:02:42

by Lincoln Dale

[permalink] [raw]
Subject: Re: [PATCH] [RFC] Support for wccp version 1 and 2 in ip_gre.c

At 09:19 AM 14/09/2004, David S. Miller wrote:
> > As you can see, I am applying it unconditionally when fits. For most
> > cases, this will be OK.
> > There can be situations when this is not wanted (for example, when
> > debugging something), so in general, tuning knob will be useful, but
> > I just don't know where to add it, maybe tunnel->parms.i_flags ...
>
>I don't think adding such a knob is necessary, but yes i_flags
>would be the place to do it.
>
>I will apply your patch with the "if(1)" simply removed.

the logic is correct, but it may make sense to call the appropriate
netfilter hook again with the "unwrapped" GRE packet, as otherwise
packets-inside-GRE represent a possible security hole where one can inject
packets externally and bypass firewall rules.


cheers,

lincoln.

2004-09-14 12:42:08

by Paul Komkoff

[permalink] [raw]
Subject: Re: [PATCH] [RFC] Support for wccp version 1 and 2 in ip_gre.c

Replying to Lincoln Dale:
> the logic is correct, but it may make sense to call the appropriate
> netfilter hook again with the "unwrapped" GRE packet, as otherwise
> packets-inside-GRE represent a possible security hole where one can inject
> packets externally and bypass firewall rules.

>From what I observe, netfilter hooks *are* called for unwrapped packets.
Either for usual IP packets passed from GRE tunnel, or for demangled
wccp packets.

--
Paul P 'Stingray' Komkoff Jr // http://stingr.net/key <- my pgp key
This message represents the official view of the voices in my head

2004-09-14 13:17:47

by Lincoln Dale

[permalink] [raw]
Subject: Re: [PATCH] [RFC] Support for wccp version 1 and 2 in ip_gre.c

At 10:39 PM 14/09/2004, Paul P Komkoff Jr wrote:
>Replying to Lincoln Dale:
> > the logic is correct, but it may make sense to call the appropriate
> > netfilter hook again with the "unwrapped" GRE packet, as otherwise
> > packets-inside-GRE represent a possible security hole where one can inject
> > packets externally and bypass firewall rules.
>
> From what I observe, netfilter hooks *are* called for unwrapped packets.
>Either for usual IP packets passed from GRE tunnel, or for demangled
>wccp packets.

you probably want to ensure that the order of netfilter events are:
1. [packet comes in]
2. netfilter INPUT
3. [GRE decap]
4. [addressed to us?]
Yes => netfilter INPUT
No => netfilter FORWARD

i don't think that both (2) and (4) are done.

also just a minor nit: not all WCCP needs to be GRE-encoded; on
high-performance switch/router platforms, only a layer-2 rewrite of the dst
MAC addr is used instead of a layer-3 GRE tunnel. you may want the comment
at line 609 to explicitly mention "WCCPv1 and WCCPv2 GRE Forwarding mode".


cheers,

lincoln.

2004-09-14 20:18:14

by Paul Komkoff

[permalink] [raw]
Subject: Re: [PATCH] [RFC] Support for wccp version 1 and 2 in ip_gre.c

Replying to Lincoln Dale:
> >From what I observe, netfilter hooks *are* called for unwrapped packets.
> >Either for usual IP packets passed from GRE tunnel, or for demangled
> >wccp packets.
>
> you probably want to ensure that the order of netfilter events are:

Basically all surroinding stuff was untouched. I just enabled further
processing (by kernel IP stack) of GRE-WCCP encapsulated packets.

> 1. [packet comes in]
> 2. netfilter INPUT
> 3. [GRE decap]
> 4. [addressed to us?]
> Yes => netfilter INPUT
> No => netfilter FORWARD
>
> i don't think that both (2) and (4) are done.

I think (2) is done before ipgre_rcv, and (4) is done because after
decapsulation
skb->dev = tunnel->dev;
netif_rx(skb);
this packet reenters the building from matched gre interface.

> also just a minor nit: not all WCCP needs to be GRE-encoded; on
> high-performance switch/router platforms, only a layer-2 rewrite of the dst
> MAC addr is used instead of a layer-3 GRE tunnel. you may want the comment
> at line 609 to explicitly mention "WCCPv1 and WCCPv2 GRE Forwarding mode".

Yes I know this :) L2 case do not need any extra processing, but not
all routers support it. For example, my 4500 don't.

Here is updated patch with if(1) removed and comment changed.


diff -urN /usr/src/linux-2.6.8-1.521/include/linux/if_ether.h linux-2.6.8-1.521wccp/include/linux/if_ether.h
--- /usr/src/linux-2.6.8-1.521/include/linux/if_ether.h 2004-08-14 09:37:15.000000000 +0400
+++ linux-2.6.8-1.521wccp/include/linux/if_ether.h 2004-09-13 08:51:02.000000000 +0400
@@ -59,6 +59,8 @@
#define ETH_P_8021Q 0x8100 /* 802.1Q VLAN Extended Header */
#define ETH_P_IPX 0x8137 /* IPX over DIX */
#define ETH_P_IPV6 0x86DD /* IPv6 over bluebook */
+#define ETH_P_WCCP 0x883E /* Web-cache coordination protocol
+ * defined in draft-wilson-wrec-wccp-v2-00.txt */
#define ETH_P_PPP_DISC 0x8863 /* PPPoE discovery messages */
#define ETH_P_PPP_SES 0x8864 /* PPPoE session messages */
#define ETH_P_MPLS_UC 0x8847 /* MPLS Unicast traffic */
diff -urN /usr/src/linux-2.6.8-1.521/net/ipv4/ip_gre.c linux-2.6.8-1.521wccp/net/ipv4/ip_gre.c
--- /usr/src/linux-2.6.8-1.521/net/ipv4/ip_gre.c 2004-08-14 09:37:37.000000000 +0400
+++ linux-2.6.8-1.521wccp/net/ipv4/ip_gre.c 2004-09-15 00:03:45.586135760 +0400
@@ -605,13 +605,23 @@
if ((tunnel = ipgre_tunnel_lookup(iph->saddr, iph->daddr, key)) != NULL) {
secpath_reset(skb);

+ skb->protocol = *(u16*)(h + 2);
+ /* WCCP version 1 and 2 (in GRE forwarding mode) protocol decoding.
+ * - Change protocol to IP
+ * - When dealing with WCCPv2, skip extra 4 bytes in GRE header
+ */
+ if ((flags == 0) && (skb->protocol == __constant_htons(ETH_P_WCCP))) {
+ skb->protocol = __constant_htons(ETH_P_IP);
+ if ((*(h + offset) & 0xF0) != 0x40)
+ offset += 4;
+ }
+
skb->mac.raw = skb->nh.raw;
skb->nh.raw = __pskb_pull(skb, offset);
memset(&(IPCB(skb)->opt), 0, sizeof(struct ip_options));
if (skb->ip_summed == CHECKSUM_HW)
skb->csum = csum_sub(skb->csum,
csum_partial(skb->mac.raw, skb->nh.raw-skb->mac.raw, 0));
- skb->protocol = *(u16*)(h + 2);
skb->pkt_type = PACKET_HOST;
#ifdef CONFIG_NET_IPGRE_BROADCAST
if (MULTICAST(iph->daddr)) {

--
Paul P 'Stingray' Komkoff Jr // http://stingr.net/key <- my pgp key
This message represents the official view of the voices in my head

2004-09-15 05:47:48

by David Miller

[permalink] [raw]
Subject: Re: [PATCH] [RFC] Support for wccp version 1 and 2 in ip_gre.c

On Tue, 14 Sep 2004 18:48:20 +1000
Lincoln Dale <[email protected]> wrote:

> the logic is correct, but it may make sense to call the appropriate
> netfilter hook again with the "unwrapped" GRE packet, as otherwise
> packets-inside-GRE represent a possible security hole where one can inject
> packets externally and bypass firewall rules.

This will occur when we push the packet back into the
RX path via the netif_rx() call.

Paul if you want to fix up the comment, that's fine.
But please send such a patch relative to what I put
into the tree already which is the following:

# This is a BitKeeper generated diff -Nru style patch.
#
# ChangeSet
# 2004/09/13 16:04:36-07:00 [email protected]
# [IPV4]: Add wccp v1/v2 support to ip_gre.c
#
# Signed-off-by: David S. Miller <[email protected]>
#
# net/ipv4/ip_gre.c
# 2004/09/13 16:04:06-07:00 [email protected] +12 -1
# [IPV4]: Add wccp v1/v2 support to ip_gre.c
#
# include/linux/if_ether.h
# 2004/09/13 16:04:06-07:00 [email protected] +2 -0
# [IPV4]: Add wccp v1/v2 support to ip_gre.c
#
diff -Nru a/include/linux/if_ether.h b/include/linux/if_ether.h
--- a/include/linux/if_ether.h 2004-09-14 22:27:39 -07:00
+++ b/include/linux/if_ether.h 2004-09-14 22:27:39 -07:00
@@ -59,6 +59,8 @@
#define ETH_P_8021Q 0x8100 /* 802.1Q VLAN Extended Header */
#define ETH_P_IPX 0x8137 /* IPX over DIX */
#define ETH_P_IPV6 0x86DD /* IPv6 over bluebook */
+#define ETH_P_WCCP 0x883E /* Web-cache coordination protocol
+ * defined in draft-wilson-wrec-wccp-v2-00.txt */
#define ETH_P_PPP_DISC 0x8863 /* PPPoE discovery messages */
#define ETH_P_PPP_SES 0x8864 /* PPPoE session messages */
#define ETH_P_MPLS_UC 0x8847 /* MPLS Unicast traffic */
diff -Nru a/net/ipv4/ip_gre.c b/net/ipv4/ip_gre.c
--- a/net/ipv4/ip_gre.c 2004-09-14 22:27:39 -07:00
+++ b/net/ipv4/ip_gre.c 2004-09-14 22:27:39 -07:00
@@ -603,13 +603,24 @@
if ((tunnel = ipgre_tunnel_lookup(iph->saddr, iph->daddr, key)) != NULL) {
secpath_reset(skb);

+ skb->protocol = *(u16*)(h + 2);
+ /* WCCP version 1 and 2 protocol decoding.
+ * - Change protocol to IP
+ * - When dealing with WCCPv2, Skip extra 4 bytes in GRE header
+ */
+ if (flags == 0 &&
+ skb->protocol == __constant_htons(ETH_P_WCCP)) {
+ skb->protocol = __constant_htons(ETH_P_IP);
+ if ((*(h + offset) & 0xF0) != 0x40)
+ offset += 4;
+ }
+
skb->mac.raw = skb->nh.raw;
skb->nh.raw = __pskb_pull(skb, offset);
memset(&(IPCB(skb)->opt), 0, sizeof(struct ip_options));
if (skb->ip_summed == CHECKSUM_HW)
skb->csum = csum_sub(skb->csum,
csum_partial(skb->mac.raw, skb->nh.raw-skb->mac.raw, 0));
- skb->protocol = *(u16*)(h + 2);
skb->pkt_type = PACKET_HOST;
#ifdef CONFIG_NET_IPGRE_BROADCAST
if (MULTICAST(iph->daddr)) {