[Swan-dev] crash during testing xauth (2) when processing dpd event

Antony Antony antony at phenome.org
Tue Oct 10 21:34:59 UTC 2017


I pushed a fix for this. It will detect dangling hp in a simple case.
I am not sure about complicated cases, such as mix of CK_INSTANCE and 
CK_PERMANENT connections between same IP addresses.

On Mon, Oct 02, 2017 at 01:02:41PM -0400, Paul Wouters wrote:
> On Mon, 2 Oct 2017, Antony Antony wrote:
> 
> > > The pointer hp points at a bunch of 0xef bytes.  This cannot have been
> > > the case when it was initialized at the start of the function: it is
> > > dereferenced to initialize d in the first for loop.
> > > 
> > > c_kind is CK_INSTANCE.  So the preceding if-body was not executed.
> > > dnshostname is NULL;
> > > d is 0xef...ef
> > > host_addr is 192.1.3.209
> > > 
> > > Theory: the ??? comment is right and terminate_connection has deleted
> > > the connection (and host pair).
> > 
> > Two years later the bug surfaced. Good catch!!
> > 
> > > Who thinks that they should fix this?
> > 
> > here is my proposed fix. Any comments? I tested using xauth-pluto-17
> 
> This would skip restarting all instance connections. That would affect
> all connections with narrowing even if they do not have right=%any and
> would break any CP based connection on the initiator by ignoring
> liveness failures.
> 
> I guess a slightly less bad check would be for rekey=no or right=%any
> and skip those.
> 
> Ideally, I guess we need to somehow find a better way to mark a
> connection as been created by a server for a roadwarrior client,
> where we are not expected to initiate towards to client.
> 
> This could be done based on modecfgserver=yes ?
> 
> > I am, still, suspecious of restart code. If there are multiple connections
> > from same NAT GW it would restart all of them when one dpd fails. Probably
> > for another day. Lets fix this crash first.
> 
> Yeah, that requires updating the concept of hostpair to take into
> account the ID as well as the IP.
> 
> > Also the test is weired. The combination IKEv1 aggressive mode, xauth ,
> > %any, dpdaction=%restart. A bad combination except for testing!
> 
> Tuomo and I have talked about the restart keyword in the past. It seems
> that DPD/liveness should only do hold or clear, and other properties
> of the connection should either cause delete or re-initiate.

good if you have universal solution go for it. What would be for OE 
TEMPLATE?  In OE an instance could be responder or initiator.  When 
connection terminate what is there to restart?

May be one simple constraint during restart, ikev2 for sure, could be do not 
initiate towards peer behind NAT.
-antony


More information about the Swan-dev mailing list