netdev - [PATCH]: double SAs are created when using AH and ESP together

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-Id: <200703092314.l29NEsre023870@faith.austin.ibm.com>
Date:	Fri, 9 Mar 2007 17:14:54 -0600
From:	Joy Latten <latten@...tin.ibm.com>
To:	netdev@...r.kernel.org
Cc:	davem@...emloft.net, eparis@...hat.com,
	herbert@...dor.apana.org.au, jmorris@...ei.org, paul.moore@...com,
	vyekkirala@...stedCS.com
Subject: [PATCH]: double SAs are created when using AH and ESP together

On Tue, 2007-03-06 at 14:40 -0500, James Morris wrote:
On Tue, 6 Mar 2007, Joy Latten wrote:
> 
> > > I saw something similar to this some time ago when testing various 
> > > failure modes, and discused it with Herbert.
> > > 
> > > IIRC, there's a larval SA which is not torn down properly by Racoon once 
> > > the full SA is established, and the larval SA keeps resending until it 
> > > times out.
> > > 
> > Ok, good to know. 
> > I thought a bit more about this last night but am not
> > sure best way to fix it. Perhaps a way to keep larval
> > SA around until all SAs resulting from xfrm_vec[xfrm_nr]
> > are established... oh well, just thinking out loud... :-) 
> 
> I think the solution, if this actually the problem, is for the userland 
> code to maintain the SAs.
 
Gotta agree. :-)

I noticed that in xfrm_state_add we look for the larval SA in a few
places without checking for protocol match. So when using both 
AH and ESP, whichever one gets added first, deletes the larval SA. 
It seems AH always gets added first and ESP is always the larval 
SA's protocol since the xfrm->tmpl has it first. Thus causing the
additional km_query()

Adding the check eliminates the double SA creation. 
I know this may not seem like a complete solution and I will 
continue to test and be on the lookout, but isn't having the
check a good thing? So far I have tested SAs with just ESP, just AH
and with both and all seems ok. 

Please let me know if this patch is ok. 
My kernel was 2.6.20-rc3-git3.

Joy 

Signed-off-by: Joy Latten <latten@...tin.ibm.com>

diff -urpN linux-2.6.20.orig/net/xfrm/xfrm_state.c linux-2.6.20.patch/net/xfrm/xfrm_state.c
--- linux-2.6.20.orig/net/xfrm/xfrm_state.c	2007-03-08 17:39:14.000000000 -0600
+++ linux-2.6.20.patch/net/xfrm/xfrm_state.c	2007-03-09 11:03:25.000000000 -0600
@@ -704,7 +704,8 @@ static struct xfrm_state *__find_acq_cor
 		    x->props.mode   != mode ||
 		    x->props.family != family ||
 		    x->km.state     != XFRM_STATE_ACQ ||
-		    x->id.spi       != 0)
+		    x->id.spi       != 0 ||
+		    x->id.proto	    != proto)
 			continue;
 
 		switch (family) {
@@ -801,7 +802,8 @@ int xfrm_state_add(struct xfrm_state *x)
 
 	if (use_spi && x->km.seq) {
 		x1 = __xfrm_find_acq_byseq(x->km.seq);
-		if (x1 && xfrm_addr_cmp(&x1->id.daddr, &x->id.daddr, family)) {
+		if (x1 && ((x1->id.proto != x->id.proto) ||
+		    xfrm_addr_cmp(&x1->id.daddr, &x->id.daddr, family))) {
 			xfrm_state_put(x1);
 			x1 = NULL;
 		}
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html