[Swan] SHA2 support for ESP in KLIPS?

[Paul Wouters]

On Tue, 25 Jun 2013, David McCullough wrote:

> Paul Wouters wrote the following:
>> On Mon, 24 Jun 2013, Elison Niven wrote:
>>
>>> # ipsec auto --status shows SHA2 for ESP
>>> algorithm ESP auth attr: id=5, name=AUTH_ALGORITHM_HMAC_SHA2_256,
>>> keysizemin=256, keysizemax=256
>>> algorithm ESP auth attr: id=6, name=AUTH_ALGORITHM_HMAC_SHA2_384,
>>> keysizemin=384, keysizemax=384
>>> algorithm ESP auth attr: id=7, name=AUTH_ALGORITHM_HMAC_SHA2_512,
>>> keysizemin=512, keysizemax=512
>>>
>>> When I establish a connection using esp=aes128-sha2_256, pluto quits saying:
>>>
>>> "SHA2" #2: responding to Quick Mode {msgid:98d24ff3}
>>> "SHA2" #2: ASSERTION FAILED at ikev1_quick.c:266: case 5 unexpected
>>
>> Ugh. Normally those are caught with the default cause. I guess
>> KERNEL_ALG is not defined when you compile with OCF?
>>
>> David: should the following code always be compiled in, and not
>> #ifdef'ed for KERNEL_ALG?
>
> Beats me,  I thought that pluto looked in /proc/net/pf_key_registered or
> /proc/net/pf_key_supported to see what was ok for klips.

I'll check.

>> In fact, why are AUTH_ALGORITHM_HMAC_MD5 and AUTH_ALGORITHM_HMAC_SHA1
>> specifically named? Shouldn't the "default" case catch those too?
>
> This is probably due to the fact that for cryptoapi,  klips still uses
> its own SHA1/MD5.

Hmm. Maybe. We should phase that out too if we can. Would it interfere
with OCF?

> I'm not so sure I even like KERNEL_ALG appearing in
> userspace,  whatever differences we have here should be handled in
> kernel_*.c shouldn't they ?

Didn't you introduce KERNEL_ALG when OCF support was added, and you
needed some thing undefined when using OCF? Or was that CONFIG_KLIPS_ALG?

I see in programs/pluto/Makefile.options:

         -DIKE_ALG -DKERNEL_ALG \

So I'm confused how Elison even got KERNEL_ALG unset?

Looking at IKE_ALG, it seems that we always need it and the #ifdef
should just be killed. The same for KERNEL_ALG too?

In fact, this one even seems to have mixed up the two:

#ifdef KERNEL_ALG
                 alg_info_addref(IKETOINFO(c->alg_info_ike));
#endif


So I'd like to suggest we clean these defines up and remove them, as the
code seems to be always required.

> Yeah,  that means its probably not handling the larger hash correctly,  I
> was expecting this may be the case,  but hoped it would be ok :-)
>
> I'll try and have a look at it this week,

Ok, thanks.

Paul