[Swan] SHA2 support for ESP in KLIPS?

Paul Wouters pwouters at redhat.com
Sat Jun 29 00:08:12 EEST 2013


On Fri, 28 Jun 2013, D. Hugh Redelmeier wrote:

> | From: Paul Wouters <pwouters at redhat.com>
>
> | err_t kernel_alg_esp_auth_ok(int auth,
> |                              struct alg_info_esp *alg_info __attribute__(
> |                                      (unused)))
>
> | The function (misleadingly) returns NULL on success, and non-NULL on
> | failure.
>
> No, that's not misleading.  That's what err_t means: an error message,
> or NULL if not an error.  This convention is widely used in Pluto.

Hence me putting (misleading) in braces. Perhaps I should have used
quotes....

> #ifdef KERNEL_ALG
>                         if (kernel_alg_esp_auth_ok(pi->attrs.transattrs.
>                                                    integ_hash, NULL) == NULL) {
>                                 needed_len += kernel_alg_esp_auth_keylen(
>                                         pi->attrs.transattrs.integ_hash);
>                                 break;
>                         }
> #endif
>                 case AUTH_ALGORITHM_DES_MAC:
>                         bad_case(pi->attrs.transattrs.integ_hash);
>                         break;
>
>                 }
>
> Th ifdef KERNEL_ALG code handles the success case (notice the ==
> NULL).

But I only yesterday added the == NULL to actually make that clear, to
avoid more "fixes" to add a "!" in front of it because people did not
realise it returned err_t.

> If KERNEL_ALG isn't defined, or kernel_alg_esp_auth_ok fails,
> the code falls through to case AUTH_ALGORITHM_DES_MAC.

I changed that too, now KERNEL_ALG is no longer there and the code is
always enabled. I did move the case to make it more obvious:

So the entire chunk now looks:

                 switch (pi->attrs.transattrs.integ_hash) {
                 case AUTH_ALGORITHM_NONE:
                         break;
                 case AUTH_ALGORITHM_HMAC_MD5:
                         needed_len += HMAC_MD5_KEY_LEN;
                         break;
                 case AUTH_ALGORITHM_HMAC_SHA1:
                         needed_len += HMAC_SHA1_KEY_LEN;
                         break;
                 case AUTH_ALGORITHM_DES_MAC:
                         bad_case(pi->attrs.transattrs.integ_hash);
                         break;
                 default:
                         if (kernel_alg_esp_auth_ok(pi->attrs.transattrs.
                                                    integ_hash, NULL) == NULL) {
                                 needed_len += kernel_alg_esp_auth_keylen(
                                         pi->attrs.transattrs.integ_hash);
                                 break;
                         }
                         bad_case(pi->attrs.transattrs.integ_hash);
                 }

> But it results in an assertion failure.  So it appears to depend on
> some earlier code to prevent this happening.

That was because Ellison made another (wrong) change.

Paul


More information about the Swan mailing list