netdev - Re: [PATCH]: SAD sometimes has double SAs.

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-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <1174957467.3085.461.camel@faith.austin.ibm.com>
Date:	Mon, 26 Mar 2007 19:04:27 -0600
From:	Joy Latten <latten@...tin.ibm.com>
To:	David Miller <davem@...emloft.net>
Cc:	eparis@...hat.com, netdev@...r.kernel.org,
	herbert@...dor.apana.org.au, jmorris@...ei.org, paul.moore@...com,
	vyekkirala@...stedcs.com
Subject: Re: [PATCH]: SAD sometimes has double SAs.

On Mon, 2007-03-26 at 14:48 -0700, David Miller wrote:
> From: Eric Paris <eparis@...hat.com>
> Date: Mon, 26 Mar 2007 17:34:59 -0400
> 
> > I'm not at all able to speak on the correctness or validity of the
> > solution,
> 
> Neither am I yet :)
> 
> > but shouldn't the ipv6 case be a && not an || like the ipv4
> > case?  Isn't this going to match all sorts of things?  Did you test this
> > patch on ipv6 and see it to solve your problem?
> > 
> > I'm also not enjoying the formatting in the ipv6 part where the first
> > time you have the cast on the same time as the object but not the second
> > part where x->props.saddr.a6 is on its own little line.
> 
> Also, I want to understand what is going to tear down these
> "other direction" fake entries later on?  I think I can review
> this patch better if I understand that.
> 
I am going to refer to the other-direction-placeholder as the "fake"
entry. And the larval SA that gets created for the new
spi as result of a GETSPI message as the "real" entry. 

The fake entry gets created when the real entry does and does not have
an spi. It shares some of the same properties of a "real" larval SA
(they are created using same code) and it's state
is marked as XFRM_STATE_ACQ. The real entry has a timeout. So, should
IKE negotiation fail, take too long, etc... it will eventually timeout
and be deleted. So does the fake entry. It will timeout and should be
eventually deleted. (I will test this part tomorrow for assurance.)

When the IKE negotiations are successful, xfrm_state_add() and the
xfrm_state_update() look for larval SAs in that they look for an SA with
same src, dst, etc... and with state==XFRM_STATE_ACQUIRE. Any that are
found are deleted and new SA added. This removes the real larval SA, and
should also remove the fake entry too.     

Of course, this is all based on my assumption that IKE will install
two SAs, one for incoming and one for outgoing. 

Hopefully this answers how fake entries will be removed. 
Admittedly, I may miss something or didn't understand something
correctly as I learn the code, so please let me know. 
There may even be a better solution that I don't readily see. 

> As it stands, this looks to me like a workaround for an improperly
> implemented IPSEC daemon.  Joy states it as saying that the current
> code requires the keying daemon to manage it's SAs, and I wonder
> whether any other implementation is even valid.
> 
My big mouth. :-) But yes, I do think more SA management in 
userspace would be ideal. 

This fix will hopefully ensure kernel doesn't send any
extra acquires regardless. 

Joy
-
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