FreeBSD NAT-T patch integration [CFR/CFT]

[Matthew Grooms]

On Mon, Jul 21, 2008 at 10:31:10AM +0200, VANHULLEBUS Yvan wrote:
> 
> After some more testing, I found another issue: in udp4_espdecap(),
> when payload <= sizeof(uint64_t) + sizeof(struct esp), packet should
> not be discarded, but just returned for normal processing.
> 

I noticed this too. But the only situation that I could think of where 
a valid ISAKMP packet will be smaller than this is a NAT-T keep-alive. 
These are handled previously in the code path so I don't think there is 
an issue from a functional standpoint.

> And I also have doubts about a change in udp_ctloutput(), in the
> switch statement which process optval and searches for an
> UDP_ENCAP_ESPINUDP* flag.
> 
> The way you changed it forces a flags cleanup anytime.
> I don't see why someone would set both UDP_ENCAP_ESPINUDP and
> UDP_ENCAP_ESPINUDP_NON_IKE, but as I was tracking down a problem, I
> changed it again to be processed "the old way" to ensure it was not
> the source of the issue.
>

It should be disallowed as in Sams patch. Allowing them to be mixed 
would cause problems using any of the patches I have seen. There is no 
way to distinguish between a Draft 00/01 ISAKMP packet and an RFC ESP 
packet without matching the port value to a SAD NAT-T mapping. And as 
you mentioned, I also don't see why anyone would try to set them both. 
There should never be a situation where you need to evaluate a NON-ESP 
NAT-T marker on an ISAKMP socket, only NON-ISAKMP markers.

On a related note, I noticed the patch unconditionally uses a source 
port of 500 when processing outbound Draft 00/01 packets. Should this 
value be obtained from the SAD NAT-T mapping to support an IKE daemon 
bound to a non standard port?

Thanks,

-Matthew