[Openswan dev] [PATCH] Roll back ipsec_sa changes in case of errors in ikev1_quick

[Herbert Xu]

Hi Michael:

The following patch rolls back the changes made by install_ipsec_sa and
install_inbound_ipsec_sa in the various quick_* functions when we detect
an error after the SAs have been installed.

This is needed because otherwise the system enters a consistent state
which can cause crashes elsewhere in the code.

For example, if the final quick mode processing fails because dpd_init
couldn't find a phase 1 SA, we will have a state that fails the
IS_IPSEC_SA_ESTABLISHED test because it has not yet transitioned into
the final state.

However, this state will be the eroute owner of the SPD which causes a
crash in this spot:

delete_state -> connection_discard ->
delete_connection -> release_connection -> delete_states_by_connection

Normally delete_state would have removed the eroute if the state passed
the IS_IPSEC_SA_ESTABLISHED test.

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert at gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
-------------- next part --------------
Index: programs/pluto/ikev1_quick.c
===================================================================
RCS file: /public/cvs/openswan-2/programs/pluto/ikev1_quick.c,v
retrieving revision 1.8
diff -u -r1.8 ikev1_quick.c
--- programs/pluto/ikev1_quick.c	9 Oct 2005 20:30:12 -0000	1.8
+++ programs/pluto/ikev1_quick.c	12 Oct 2005 07:05:09 -0000
@@ -1967,7 +1967,10 @@
     /* encrypt message, except for fixed part of header */
     
     if (!encrypt_message(&md->rbody, st))
+    {
+	delete_ipsec_sa(st, TRUE);
 	return STF_INTERNAL_ERROR;	/* ??? we may be partly committed */
+    }
 
     DBG(DBG_CONTROLMORE, DBG_log("finished processing quick inI1"));
     return STF_OK;
@@ -2101,7 +2104,10 @@
     /* encrypt message, except for fixed part of header */
 
     if (!encrypt_message(&md->rbody, st))
+    {
+	delete_ipsec_sa(st, FALSE);
 	return STF_INTERNAL_ERROR;	/* ??? we may be partly committed */
+    }
 
     {
       DBG(DBG_CONTROLMORE, DBG_log("inR1_outI2: instance %s[%ld], setting newest_ipsec_sa to #%ld (was #%ld) (spd.eroute=#%ld)"
@@ -2122,6 +2128,7 @@
 	on this conn, so initialize it */
     if (st->st_connection->dpd_delay && st->st_connection->dpd_timeout) {
 	if(dpd_init(st) != STF_OK) {
+	    delete_ipsec_sa(st, FALSE);
 	    return STF_FAIL;
 	}
     }
@@ -2177,6 +2184,7 @@
 	on this conn, so initialize it */
     if(st->st_connection->dpd_delay && st->st_connection->dpd_timeout) {
 	if(dpd_init(st) != STF_OK) {
+	    delete_ipsec_sa(st, FALSE);
 	    return STF_FAIL;
 	}
     }