[Openswan dev] [Commits] [OPENSWAN.git] Changes to ref refs/heads/master

[David McCullough]

Jivin Paul Wouters lays it down ...
...
> >> Is that not sufficient to disable NETKEY?
> >>
> >>>>> This was running on the head of git master completely unmodified.
> >>>>>
> >>>>> Obviously somewhere else in pluto is using kern_interface to make
> >>>>> decisions and getting it wrong when kern_interface==AUTO_PICK.  I
> >>>>> didn't go looking for the offender.  I actually found this use 'git
> >>>>> bisect',  decided that there was no way that
> >>>>> fdfb59b413eec432969014762ceb847ef7e5e9a4 could be the cause and then
> >>>>> went through it all by hand again,  and still didn't believe it ;-)
> >>>>
> >>>> At least one case (I had problems with) is the function delete_ipsec_sa()
> >>>> in kernel.c.
> >>>
> >>> What problem are you seeing there ?
> >>
> >> This is just one case that goes wrong if kern_interface==AUTO_PICK.
> >> The SA will not be deleted, you just get the log
> >> "Unknown kernel stack in delete_ipsec_sa".
> >>
> >> For now it works with Pauls (and your) fixes.
> >
> > Oh,  good,  I thought something was still broken :-)
> 
> There is. The new code does nothing. So I do not believe whatever crasher
> is gone. It just cannot be. I bet gcc even optimised those lines out :P

I will bet you are wrong ;-) ;-)

> Either my patch worked and testing it was done wrong. Or something fishy
> is still broken.

I couldn't believe it either,  and it took a while to see the code problem.
I also did far more testing and reviewing than the problem deserved.

Follow through the "init_kernel" function with kern_interface = AUTO_PICK,
both NETKEY_SUPPORT and KLIPS defined in the build, but no netkey support in
the runtime kernel.  Cut down,  it looks like this:

	...
	if(kern_interface == AUTO_PICK) {
		...
		// ok we go in here
		...
    #if defined(NETKEY_SUPPORT)
        if (stat("/proc/net/pfkey", &buf) == 0) // this fails, no kernel support
        {
            kern_interface = USE_NETKEY;
        }
    # elif defined(KLIPS)
		// this code will not be compiled in due to #elif
        if (stat("/proc/net/pf_key", &buf) == 0)
        {
            kern_interface = USE_KLIPS;
        }
    #endif
	}

Note the "# elif defined(KLIPS)",  this means that the code to check for
klips is not compilied in for this case, so we drop out with
kern_interface = AUTO_PICK.

I figured since none of the other cases were dealt with,  I would like to
ensure that kern_interface was always guaranteed to be set to something on
exit (ie., BSD_KAME, KLIPS_MAST, ...)

I am more than happy to rejig the code to be more obvious.  Have a look at
the attached patch (untested) and see how that grabs you.  You probably need
to apply it to see how the resulting init_kernel looks,  I think it's
better, but I'm not attached to it.  A table of interfaces with a "check"
method may be more appropriate for readability.

Cheers,
Davidm

-- 
David McCullough,      david_mccullough at mcafee.com,  Ph:+61 734352815
McAfee - SnapGear      http://www.mcafee.com         http://www.uCdot.org
-------------- next part --------------
A non-text attachment was scrubbed...
Name: init_kernel.diff
Type: text/x-diff
Size: 3862 bytes
Desc: not available
Url : http://lists.openswan.org/pipermail/dev/attachments/20100505/fcf378c6/attachment.bin