Re: [Ipsec-tools-devel] minor fixes about to be committed

[VANHULLEBUS Y. <va...@fr...>]

On Tue, Sep 02, 2008 at 02:00:34PM +0300, Timo Ter?s wrote:
> Hi all,
> 
> I'll commit the following clean-ups and bugfixes unless someone objects:
> 
> 1. A workaround to linux kernel bug. It sends the spdflush
> message with uninitialized satype. The strict check makes racoon
> internal SPD cache go out of sync when SPD is reloaded.
[...]

I can't check that and don't know that problem, but if you tested it,
it's ok for me, even if kernel bugs should be fixed in kernels....


> 2. Quickmode clean up / optimization. When do not need to search
> for remoteconf by address, since we can deduce the remote conf
> from the phase1 context used to negotiate the phase2.
[....]

As this is not a "fix", please just commit that one on HEAD.


> 3. Make SPD reload work during config-reload: explicitly call
> pfkey_handler() from pfkey_reload(). Otherwise SPD dump reply
> messages might get ignored in pfkey_dump_sadb() which might
> be called when some SA is deleted due to configuration changes.
> 
> Index: src/racoon/pfkey.c
> ===================================================================
> RCS file: /cvsroot/src/crypto/dist/ipsec-tools/src/racoon/pfkey.c,v
> retrieving revision 1.29
> diff -u -p -r1.29 pfkey.c
> --- src/racoon/pfkey.c	14 Jul 2008 05:45:15 -0000	1.29
> +++ src/racoon/pfkey.c	2 Sep 2008 10:52:11 -0000
> @@ -270,7 +270,7 @@ pfkey_handler()
>  	if ((pkrecvf[msg->sadb_msg_type])(mhp) < 0)
>  		goto end;
>  
> -	error = 0;
> +	error = 1;
>  end:
>  	if (msg)
>  		racoon_free(msg);

Comment at the beginning of the function tells 0: success, first
return statements is not clear for me (but it's late, I'll see again
tomorrow :-), and you changes the code to return 1 if it works,
there's a problem somewhere, even if the only existing call to that
function (before your patch) just doesn't care about it's return code.



> 4. Similar optimization as #2. Remote conf can be deduced from
> phase1 used to negotiate phase2.

I'm quite sure I commited the original code (related to
GENERATE_POLICY_UNIQUE), but I don't remember if I did that because we
found cases where iph2->ph1->rmconf is NULL, or if I did that for "bad
reasons".

As for #2, please commit that one only on HEAD.



> 5. The code uses extract_port(rmconf->remote). Anonymous remote confs
> have AF_UNSPEC and the extract_port would report spurious error. Handle
> AF_UNSPEC silently.

Looks like a bug for me, so it's a good candidate for a backport to
0.7 branch.



Yvan.