Subject: Re: AH + GRE works; ESP + GRE doesn't
To: Jun-ichiro itojun Hagino <itojun@iijlab.net>
From: Curt Sampson <cjs@cynic.net>
List: tech-net
Date: 04/21/2003 17:00:08
So I added this patch to a 1.6.1 system, but a) it didn't fix the
problem of incoming packets not being seen on the gre interface if ESP
is in use, and b) it seems to have introduced a problem that, when
they do come in on the gre interface, they are not forwarded properly
after that. I don't know where they go, but after being received on
gre they do not appear to be dropped by ipfilter (I log all my block
statements) and they are not routed out the appropriate interface.
(Packets originating at the host do seem to be ok.)

I checked the routing table with netstat -rv, and the "use" count on
that route for that host (it's directly connected to a local interface)
does not go up when the ICMP echo response comes back and I see it on
the gre interface. Any idea where the packet could be going?

cjs
-- 
Curt Sampson  <cjs@cynic.net>   +81 90 7737 2974   http://www.netbsd.org
    Don't you know, in this new Dark Age, we're all light.  --XTC

On Mon, 21 Apr 2003, Jun-ichiro itojun Hagino wrote:

> >> 	sys/netinet/in_gre.c:gre_mobile_input() assumes that GRE header
> >> 	and IP header are continuous on a single mbuf, but the assumption
> >> 	does not hold for ESP case i guess.  i guess we need to fix
> >> 	gre_mobile_input() to perform m_pullup().
> >Can you suggest a patch for the 1.6 branch?
>
> 	against -current.
>
> itojun
>
>
> Index: ip_gre.c
> ===================================================================
> RCS file: /cvsroot/src/sys/netinet/ip_gre.c,v
> retrieving revision 1.23
> diff -u -r1.23 ip_gre.c
> --- ip_gre.c	2002/11/25 23:37:08	1.23
> +++ ip_gre.c	2003/04/21 06:33:19
> @@ -151,7 +151,7 @@
>  int
>  gre_input2(struct mbuf *m ,int hlen, u_char proto)
>  {
> -	struct greip *gip = mtod(m, struct greip *);
> +	struct greip *gip;
>  	int s;
>  	struct ifqueue *ifq;
>  	struct gre_softc *sc;
> @@ -162,12 +162,19 @@
>  		return (0);
>  	}
>
> +	if (m->m_len < sizeof(*gip)) {
> +		m = m_pullup(m, sizeof(*gip));
> +		if (m == NULL)
> +			return (ENOBUFS);
> +	}
> +	gip = mtod(m, struct greip *);
> +
>  	sc->sc_if.if_ipackets++;
>  	sc->sc_if.if_ibytes += m->m_pkthdr.len;
>
>  	switch (proto) {
>  	case IPPROTO_GRE:
> -		hlen += sizeof (struct gre_h);
> +		hlen += sizeof(struct gre_h);
>
>  		/* process GRE flags as packet can be of variable len */
>  		flags = ntohs(gip->gi_flags);
> @@ -181,7 +188,7 @@
>  		if (flags & GRE_KP)
>  			hlen += 4;
>  		if (flags & GRE_SP)
> -			hlen +=4;
> +			hlen += 4;
>
>  		switch (ntohs(gip->gi_ptype)) { /* ethertypes */
>  		case ETHERTYPE_IP: /* shouldn't need a schednetisr(), as */
> @@ -210,8 +217,11 @@
>  		return (0);
>  	}
>
> -	m->m_data += hlen;
> -	m->m_len -= hlen;
> +	if (hlen > m->m_pkthdr.len) {
> +		m_freem(m);
> +		return (EINVAL);
> +	}
> +	m_adj(m, hlen);
>  	m->m_pkthdr.len -= hlen;
>
>  #if NBPFILTER > 0
> @@ -224,7 +234,7 @@
>  		m0.m_data = (char *)&af;
>
>  		bpf_mtap(sc->sc_if.if_bpf, &m0);
> -		}
> +	}
>  #endif /*NBPFILTER > 0*/
>
>  	m->m_pkthdr.rcvif = &sc->sc_if;
> @@ -257,8 +267,8 @@
>          va_dcl
>  #endif
>  {
> -	struct ip *ip = mtod(m, struct ip *);
> -	struct mobip_h *mip = mtod(m, struct mobip_h *);
> +	struct ip *ip;
> +	struct mobip_h *mip;
>  	struct ifqueue *ifq;
>  	struct gre_softc *sc;
>  	int hlen,s;
> @@ -275,19 +285,36 @@
>  		return;
>  	}
>
> +	if (m->m_len < sizeof(*mip)) {
> +		m = m_pullup(m, sizeof(*mip));
> +		if (m == NULL)
> +			return;
> +	}
> +	ip = mtod(m, struct ip *)
> +	mip = mtod(m, struct mobip_h *);
> +
>  	sc->sc_if.if_ipackets++;
>  	sc->sc_if.if_ibytes += m->m_pkthdr.len;
>
> -	if(ntohs(mip->mh.proto) & MOB_H_SBIT) {
> +	if (ntohs(mip->mh.proto) & MOB_H_SBIT) {
>  		msiz = MOB_H_SIZ_L;
>  		mip->mi.ip_src.s_addr = mip->mh.osrc;
>  	} else {
>  		msiz = MOB_H_SIZ_S;
>  	}
> +
> +	if (m->m_len < msiz) {
> +		m = m_pullup(m, msiz);
> +		if (m == NULL)
> +			return;
> +		ip = mtod(m, struct ip *)
> +		mip = mtod(m, struct mobip_h *);
> +	}
> +
>  	mip->mi.ip_dst.s_addr = mip->mh.odst;
>  	mip->mi.ip_p = (ntohs(mip->mh.proto) >> 8);
>
> -	if (gre_in_cksum((u_short*)&mip->mh,msiz) != 0) {
> +	if (gre_in_cksum((u_short*)&mip->mh, msiz) != 0) {
>  		m_freem(m);
>  		return;
>  	}
> @@ -311,7 +338,7 @@
>  		m0.m_data = (char *)&af;
>
>  		bpf_mtap(sc->sc_if.if_bpf, &m0);
> -		}
> +	}
>  #endif /*NBPFILTER > 0*/
>
>  	ifq = &ipintrq;
>