Re: UDP_ENCAP_ESPINUDP_NON_IKE

[Chuck Zmudzinski <frchuckz%gmail.com@localhost>]

After testing my environment without UDP_ENCAP_ESPINUDP_NON_IKE 
available on NetBSD, I would like to see UDP_ENCAP_ESPINUDP_NON_IKE 
removed from NetBSD so I filed a problem report [1]. I think it would 
greatly improve NetBSD's support for IPsec through NAT devices.

Chuck

[1] https://gnats.netbsd.org/cgi-bin/query-pr-single.pl?number=53334


On 05/26/2018 03:10 PM, Chuck Zmudzinski wrote:
> I have written a patch to remove UDP_ENCAP_ESPINUDP_NON_IKE from 
> NetBSD, which is attached to this message. The patch is for the 
> netbsd-8 branch.
>
> I have tested the kernel produced by this patch with my NetBSD 7.1 
> system and it seems to work as designed.
>
> Chuck
>
>
> On 05/17/2018 02:16 AM, Maxime Villard wrote:
> > I think it would be good to retire UDP_ENCAP_ESPINUDP_NON_IKE. It is not
> > part of any RFC, it was just introduced in a draft, and then 
> > subsequently
> > removed from that draft.
> >
> > RFC3948 makes authority, and it documents only the Non-ESP marker - that
> > is to say, UDP_ENCAP_ESPINUDP.
> >
> > I think we are not supposed to support undocumented options.
> >
> > FreeBSD did the same, see [1]. OpenBSD has never had that, it seems.
> >
> > The reason I'm bringing this, is because there appears to be a bug with
> > UDP_ENCAP_ESPINUDP_NON_IKE, in udp_usrreq.c:
> >
> > 1317         skip = sizeof(struct udphdr) + 2 * sizeof(uint32_t);
> > 1318     }
> > 1319
> > 1320     /*
> > 1321      * Get the UDP ports. They are handled in network order
> > 1322      * everywhere in the IPSEC_NAT_T code.
> > 1323      */
> > 1324     udphdr = (struct udphdr *)((char *)data - skip);
> >
> > Here we have:
> >
> >     data = mtod(m) + IP_header_len + sizeof(struct udphdr)
> >
> > So it is wrong to substract 'skip', because then 'udphdr' will point to
> > some data in the IP header. I'm not sure if it means the code has never
> > worked, or if I'm just completely misreading it.
> >
> > What do you think? I asked Ryota and Kengo, they don't know. One 
> > concern is
> > raccoon, because it supports each draft that led to the RFC [2]. To 
> > me this
> > is wrong too, there shouldn't be support for temporary specs, that 
> > have no
> > meaning once the RFC is out.
> >
> > Don't know.
> >
> > Maxime
> >
> > [1] http://freshbsd.org/commit/freebsd/r309808
> > [2] 
> > https://nxr.netbsd.org/xref/src/crypto/dist/ipsec-tools/src/racoon/vendorid.c#67