Re: kern/48473: IPSEC tunnel doesn't send ICMP fragmentation needed for packets that are too large

[Arnaud Degroote <arnaud.degroote%laas.fr@localhost>]

The following reply was made to PR kern/48473; it has been noted by GNATS.

From: Arnaud Degroote <arnaud.degroote%laas.fr@localhost>
To: gnats-bugs%NetBSD.org@localhost
Cc: kern-bug-people%NetBSD.org@localhost, gnats-admin%NetBSD.org@localhost, 
netbsd-bugs%NetBSD.org@localhost
Subject: Re: kern/48473: IPSEC tunnel doesn't send ICMP fragmentation needed
 for packets that are too large
Date: Tue, 24 Dec 2013 14:26:46 +0100

 --GvXjxJ+pjyke8COw
 Content-Type: text/plain; charset=us-ascii
 Content-Disposition: inline
 Content-Transfer-Encoding: quoted-printable
 
 On 23/Dec - 07:10, khym%azeotrope.org@localhost wrote:
 > >Number:         48473
 > >Category:       kern
 > >Synopsis:       IPSEC tunnel doesn't send ICMP fragmentation needed for =
 packets that are too large
 > >Confidential:   no
 > >Severity:       serious
 > >Priority:       medium
 > >Responsible:    kern-bug-people
 > >State:          open
 > >Class:          sw-bug
 > >Submitter-Id:   net
 > >Arrival-Date:   Mon Dec 23 07:10:00 +0000 2013
 > >Originator:     Dave Huang
 > >Release:        NetBSD 6.99.28 (-current from Dec 20 2013)
 > >Organization:
 > Name: Dave Huang         |  Mammal, mammal / their names are called /
 > INet: khym%azeotrope.org@localhost |  they raise a paw / the bat, the cat /
 > FurryMUCK: Dahan         |  dolphin and dog / koala bear and hog -- TMBG
 > Dahan: Hani G Y+C 38 Y++ L+++ W- C++ T++ A+ E+ S++ V++ F- Q+++ P+ B+ PA+ =
 PL++
 > >Environment:
 > =09
 > =09
 > System: NetBSD foxy.azeotrope.org 6.99.28 NetBSD 6.99.28 (FOXY) #20: Fri =
 Dec 20 00:04:49 CST 2013  
khym%vmbsd.azeotrope.org@localhost:/usr/obj.i386/sys/arch/i=
 386/compile/FOXY i386
 > Architecture: i386
 > Machine: i386
 > >Description:
 >      An IPSEC tunnel will have a smaller MTU than the interface it
 > runs on due to encapsulation overhead. However, (FAST_)IPSEC doesn't
 > send ICMP fragmentation needed when it gets a Don't Fragment packet
 > that needs to be fragmented because of encapsulation overhead. This
 > breaks PMTU discovery. Beverly Schwartz posted an analysis of the
 > problem last year
 > <http://mail-index.netbsd.org/tech-net/2012/10/16/msg003674.html>, but
 > nobody said anything :(
 >=20
 > >How-To-Repeat:
 > Set up an IPSEC tunnel between two networks with NetBSD as the local
 > endpoint. Then on a host on this side of the tunnel, send a large
 > packet with DF set to a host on the other side of the tunnel. E.g.,
 > % ping -Ds1450 10.2.1.2
 >=20
 > Note the lack of any response--no echo replies, but also no ICMP
 > fragmentation needed.
 > >Fix:
 > Here's a patch based on the changes Beverly posted about in last
 > year's thread. I don't think it's exactly correct though--it uses the
 > MTU from the inner packet's interface, rather than the outer packet's.
 > Those are often the same, so this is better than nothing, but it'd be
 > nice if someone came up with a proper fix :)
 >=20
 > Index: ipsec.c
 > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
 =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
 =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
 > RCS file: /cvsroot/src/sys/netipsec/ipsec.c,v
 > retrieving revision 1.60
 > diff -u -r1.60 ipsec.c
 > --- ipsec.c  8 Jun 2013 13:50:22 -0000       1.60
 > +++ ipsec.c  23 Dec 2013 06:43:49 -0000
 > @@ -806,6 +806,17 @@
 >      }
 > =20
 >      /*
 > +     * Check that MTU is sufficient.
 > +     */
 > +    if (ntohs(ip->ip_off) & IP_DF) {
 > +            size_t ipsec_hdrlen =3D ipsec_hdrsiz(sp);
 > +            if (ntohs(ip->ip_len) + ipsec_hdrlen > *mtu) {
 > +                    splx(s);
 > +                    return EMSGSIZE;
 > +            }
 > +    }
 > +
 > +    /*
 >       * Do delayed checksums now because we send before
 >       * this is done in the normal processing path.
 >       */
 > @@ -912,11 +923,12 @@
 >              return EINVAL;
 >      }
 > =20
 > -    /* Count IPsec header size. */
 > -    ipsechdr =3D ipsec4_hdrsiz(m, IPSEC_DIR_OUTBOUND, NULL);
 > +    /* Find the correct route for outer IPv4 header */
 > +    ipsechdr =3D roundup2(ipsec4_hdrsiz(m, IPSEC_DIR_OUTBOUND, NULL), 4);
 > =20
 >      /*
 >       * Find the correct route for outer IPv4 header, compute tunnel MTU.
 > +     * XXX doesn't seem to work; sp->req->sav is NULL
 >       */
 >      if (sp->req && sp->req->sav && sp->req->sav->sah) {
 >              struct route *ro;
 > @@ -927,9 +939,10 @@
 >              if (rt && rt->rt_ifp) {
 >                      *destmtu =3D rt->rt_rmx.rmx_mtu ?
 >                          rt->rt_rmx.rmx_mtu : rt->rt_ifp->if_mtu;
 > -                    *destmtu -=3D ipsechdr;
 >              }
 >      }
 > +    *destmtu -=3D ipsechdr;
 > +
 >      KEY_FREESP(&sp);
 >      return 0;
 >  }
 >=20
 > >Unformatted:
 
 
 The main issue of this patch for me (if I don't miss something) is that
 it does not consider that the size of the packet may decrease after
 IPSEC processing (i.e. if IPCOMP is configured for this packet), and so
 may pass properly in the tunnel.
 
 --GvXjxJ+pjyke8COw
 Content-Type: application/pgp-signature; name="signature.asc"
 Content-Description: Digital signature
 
 -----BEGIN PGP SIGNATURE-----
 Version: GnuPG v1.4.14 (GNU/Linux)
 
 iQEcBAEBAgAGBQJSuYuWAAoJEAeVGKMo0YKejm4H/A8oHqZjsQC/GR0H4/51MaoB
 V2bmnwLlOPsrHOkm6A27lyTpHo4CjpYWxTBqmNoTSp8ZCCXbdaqDNA6pnzv71YSu
 /djlAmyQbX1P5zGmojXfEKxRuXlO6FxWWJNo/fd/7oeKOH478zBNlqVYzzikv+Qn
 90LDzykCoCmlZMSYruONAf2Zr9ImekEBhy5brdOUv0VazofuHr7bThklAth/qVrQ
 DJ24HXtUEIi6Zp1osyelvZAVrjQiRG65UeRT9ZqEm5SJV7HDjWXDBHwWHZWWItVG
 Rx91TesAkkmKHGzd+NMWD4LRV/zY50WSB2u2G1xwmBs/M4dVqQB7yqkyNmxSvuI=
 =2Kzq
 -----END PGP SIGNATURE-----
 
 --GvXjxJ+pjyke8COw--