running PFIL_HOOKS on decapsulated IPsec packets, too [was: Re: reverse processing order: NAT, IPsec ?]

[Hubert Feyrer <hubert%feyrer.de@localhost>]

On Thu, 25 Jun 2009, Greg A. Woods wrote:
> After seeing the ultimately simple fix Hubert posted to re-enable PFIL
> hooks for IPsec de-encapsulated packets I had a deja vu moment and I
> think I can say this silliness has caused problems in other contexts as
> well.

I don't understand - do you mean it's silly to run PFIL hooks on 
de-encapsulated packets?


> I guess the question would be if you're using non-tunnel-mode IPsec then
> does passing the de-encapsulated packet through the PFIL hooks again
> actually cause any other problems or confusions?  I suspect it would.

I'll test things here, and see what happens. So far I haven't met 
problems.

FWIW, looking at our two IPsec implementations (IPSEC vs. FAST_IPSEC), 
they seem to use different mbuf tags to indicate that the packet is still 
encapsulated (IPSEC: PACKET_TAG_ESP) and that it has been processed by 
IPSEC specifically (FAST_IPSEC: PACKET_TAG_IPSEC_IN_DONE).

I think thad adding a kernel option like 
{FAST_,}IPSEC_ALSO_RUN_PFIL_HOOKS_ON_DECAPSULATED_PACKET 
would be a first stab at my problem (see patch below). The option would be 
disabled by default, to be compatible with the existing behaviour.

Comments? Anyone familiar with the code? :)


 - Hubert



Index: ip_input.c
===================================================================
RCS file: /cvsroot/src/sys/netinet/ip_input.c,v
retrieving revision 1.275.4.1
diff -u -r1.275.4.1 ip_input.c
--- ip_input.c  25 Nov 2008 04:04:38 -0000      1.275.4.1
+++ ip_input.c  26 Jun 2009 07:37:34 -0000
@@ -650,9 +650,9 @@
         * let ipfilter look at packet on the wire,
         * not the decapsulated packet.
         */
-#ifdef IPSEC
+#if defined(IPSEC) && 
!defined(IPSEC_ALSO_RUN_PFIL_HOOKS_ON_DECAPSULATED_PACKET)
        if (!ipsec_getnhist(m))
-#elif defined(FAST_IPSEC)
+#elif defined(FAST_IPSEC) && 
!defined(FAST_IPSEC_ALSO_RUN_PFIL_HOOKS_ON_DECAPSULATED_PACKET)
        if (!ipsec_indone(m))
 #else
        if (1)