>Number:         36870
>Category:       kern
>Synopsis:       FAST_IPSEC cannot handle optional SA -> busy loop
>Confidential:   no
>Severity:       critical
>Priority:       high
>Responsible:    kern-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Fri Aug 31 08:10:01 +0000 2007
>Originator:     Wolfgang Stukenbrock
>Release:        NetBSD 3.1
>Organization:
Dr. Nagler & Company GmbH
	
>Environment:
	
	
System: NetBSD test-s0 3.1 NetBSD 3.1 (test-s0) #0: Tue Apr 3 11:33:43 CEST 2007 root@test-s0:/usr/src/sys/arch/i386/compile/test-s0 i386
Architecture: i386
Machine: i386
>Description:
	In netinet/ip_ouput.c IPSEC processing is done if required to each
	outgoing packet in ip_output(). After IPSEC processing, the packet
	is passed to ip_output() again in order to send it over the network.
	To avoid processing the packet again, a check is done, if there is
	a tag attached to the packet that matches the selected rule.
	If a matching tag is present no IPSEC processing will be done, because
	it has been done before.
	Now here are multiple things that are either not correct or has not
	been completly implemented.
	1. There is a lookup for a tag PACKET_TAG_IPSEC_PENDING_TDB in order
	   to see if IPSEC processing has been done before - as the comment
	   says. But this tag will never be attached to any packet in the
	   whole kernel sources.
	2. If a tag with PACKET_TAG_IPSEC_OUT_DONE or
	   PACKET_TAG_IPSEC_OUT_CRYPTO_NEEDED is found, only the first policy
	   entry is checked, but ther may be nore than one.
	   If there is no SA attached to the first policy request entry, IPSEC
	   processing is done and no check is done if the tag matches a second,
	   third or whatever policy request.
	   (Here the busy-loop happens if the first request is of type "use"
	   and no matching SA is currently installed.)

	Here we have two different approches here to detect that IPSEC
	processing has been done before.
	I'm not shure what IPSEC specs say about processing. The first check
	assumes that there may at most one IPSEC policy applied to a packet.
	The second one assumes that there may be more than one (if an SA is
	present all the time, it will work ...) and a second policy may be
	applied to a packet after the first policy is done, and so on ...

	From my point of view only the first assumption makes sence, but I may
	be wrong here.
	Reson for my oppinion:
	If I want to setup a compressed ipsec tunnel to connect different
	locations to each other it is more common to me to setup one rule to
	do it. "spdadd ... ipcomp/tunnel/.../use esp/tunnel/..../require;"
	(remark: the ipcomp must be setup as tunnel too so that the packet
	gets decompressed at the other esp-tunnel system. transport will lead
	to a compressed packet send to the final destination and that one
	should know nothing about the tunnel itself.)
	I know know way to setup something like this in several rules, because
	I do not know how to tell the system to apply a compression rule only
	if a SA is present and then not use an esp rule with the same adresses
	specified in them.

	If this assumption is correct the second check does not make sence at
	all - not only due to the fact that only the first request is visited.

	Therefore some corrections are required to the current code.
	I cannot give you a "final" patch, because I do not know in what
	direction we should go. I've added a "workaround" for IPv4 below that
	will enable the system to use a compresses tunnel.

	I would recommend to use the PACKET_TAG_IPSEC_PENDING_TDB tag in the
	final version and allow only at most one IPSEC policy to be applied to
	a packet.

	In the actual implementation - even in the last CVS-Version in the MAIN
	tree - it is impossible to use compression in a ESP-tunnel.
	And compression in the tunnel is very usefull to avoid ESP fragmentation
	and reduce the traffic volume over the WAN!
>How-To-Repeat:
	Setup an ipsec tunnel with compression like the following example:
	  spdadd -n 1.2.3.0/24 10.11.12.0/24 any -P out ipsec
	    ipcomp/tunnel/1.2.3.4-10.11.12.13/use
	    esp/tunnel/1.2.3.4-10.11.12.13/require;
	  add -n 1.2.3.4 10.11.12.13 esp 3456 -E blowfish-cbc "123451234512345";
	Remark: there is no SA for the ipcomp part.
	If you send a packet that matches this rule, the rule is applied
	again and again until the packet is droped due to the MAXPACKET limit
	in a busy loop.
>Fix:
	This fix is only a workaround for IP4.
	Something equal must be applied to the IP6 stuff in netipsec/ipsec.c
	to get that part running.
	see descript above for more information.
	The fix will comment out most of the current loop detection code and
	assumes that if a tag was found IPSEC is done.
	It works fine here - but I use a static key setup. I have no idea if it
	will work with racoon ...


*** ip_output.c	2007/04/24 17:21:13	1.2
--- ip_output.c	2007/08/31 07:26:48
***************
*** 689,694 ****
--- 689,695 ----
  			if (mtag->m_tag_id != PACKET_TAG_IPSEC_OUT_DONE &&
  			    mtag->m_tag_id != PACKET_TAG_IPSEC_OUT_CRYPTO_NEEDED)
  				continue;
+ #if 0 /* workaround for busy loop */
  			/*
  			 * Check if policy has an SA associated with it.
  			 * This can happen when an SP has yet to acquire
***************
*** 702,707 ****
--- 703,709 ----
  			    tdbi->proto == sp->req->sav->sah->saidx.proto &&
  			    bcmp(&tdbi->dst, &sp->req->sav->sah->saidx.dst,
  				 sizeof (union sockaddr_union)) == 0) {
+ #endif /* workaround for busy loop */
  				/*
  				 * No IPsec processing is needed, free
  				 * reference to SP.
***************
*** 712,718 ****
--- 714,722 ----
  				KEY_FREESP(&sp), sp = NULL;
  				splx(s);
  				goto spd_done;
+ #if 0 /* workaround for busy loop */
  			}
+ #endif /* workaround for busy loop */
  		}
  
  		/*

>Unformatted: