[Swan-dev] IKEv1 AH parsing quirk

[Andrew Cagney]

On 17 August 2017 at 22:09, Paul Wouters <paul at nohats.ca> wrote:
> There was a correcting function that did a minus 1 in the past. It was wrong for newer numbers and I think I killed it

It isn't that.  Here's the text from the RFC https://tools.ietf.org/html/rfc2407

   Note: the Authentication Algorithm attribute MUST be specified to
   identify the appropriate AH protection suite.  For example, AH_MD5
   can best be thought of as a generic AH transform using MD5.  To
   request the HMAC construction with AH, one specifies the AH_MD5
   transform ID along with the Authentication Algorithm attribute set to
   HMAC-MD5.  This is shown using the "Auth(HMAC-MD5)" notation in the
   following sections.

so both must be present, but other than a cross-check sanity check,
just the auth_alg value should be used

> Sent from my iPhone
>
>> On Aug 17, 2017, at 21:51, Andrew Cagney <andrew.cagney at gmail.com> wrote:
>>
>>> On 17 August 2017 at 21:17, Paul Wouters <paul at nohats.ca> wrote:
>>> The ESP integ numbers were the same as the AH integ numbers and the same as the IKE integ numbers? Probably sloppy programming that still worked?
>>
>> They are different.  For instance:
>>
>> AH_MD5=2 - transform
>> AUTH_ALGORITHM_HMAC_MD5=1 - attribute
>>
>> perhaps specifying both is just an IKEv1 quirk
>>
>>
>>> Sent from my iPhone
>>>
>>>> On Aug 17, 2017, at 20:46, Andrew Cagney <andrew.cagney at gmail.com> wrote:
>>>>
>>>> I've been trying to figure out why the IKEv1 AH_* values are pretty
>>>> much absent from pluto's code base and instead AUTH_ALGORITHM_*
>>>> appears everywhere.  Then I noticed this (ah-pluto-01):
>>>>
>>>> | *****parse ISAKMP Transform Payload (AH):
>>>> |    next payload type: ISAKMP_NEXT_NONE (0x0)
>>>> |    length: 28 (0x1c)
>>>> |    AH transform number: 0 (0x0)
>>>> |    AH transform ID: AH_SHA (0x3)
>>>> | encryption ike_alg_lookup_by_id id: 3DES=3, found 3DES_CBC
>>>>
>>>> which is clearly wrong.  The code that handles this, and dates back to
>>>> <=2007, unconditionally assigns the AH value to  .encrypt vis:
>>>>
>>>>   if (!in_struct(trans, trans_desc, prop_pbs, trans_pbs))
>>>>       return FALSE;
>>>>  ...
>>>>   attrs->transattrs.encrypt = trans->isat_transid;
>>>>   attrs->transattrs.encrypter =
>>>> ikev1_get_kernel_encrypt_desc(trans->isat_transid); // new
>>>>
>>>> it then goes on:
>>>>
>>>> | ******parse ISAKMP IPsec DOI attribute:
>>>> |    af+type: AUTH_ALGORITHM (0x8005)
>>>> |    length/value: 2 (0x2)
>>>> |    [2 is AUTH_ALGORITHM_HMAC_SHA1]
>>>> | integrity ike_alg_lookup_by_id id: HMAC_SHA1=2, found HMAC_SHA1_96
>>>>
>>>> which is handled by:
>>>>
>>>>       case AUTH_ALGORITHM | ISAKMP_ATTR_AF_TV:
>>>>           attrs->transattrs.integ_hash = val;
>>>>           attrs->transattrs.integ = ikev1_get_kernel_integ_desc(val); // new
>>>>           break;
>>>>
>>>> which "fixes" the integrity.  From there, while most code uses
>>>> integ_hash, .encrypt does get used as well.  For instance, in
>>>> compute_proto_keymat() both fields get used:
>>>>
>>>>   case PROTO_IPSEC_AH:
>>>>       switch (pi->attrs.transattrs.encrypt) {
>>>>       case AH_MD5:
>>>>           needed_len = HMAC_MD5_KEY_LEN;
>>>>           break;
>>>> ...
>>>>       default:
>>>>           if (kernel_alg_ah_auth_ok(
>>>>               pi->attrs.transattrs.integ_hash, NULL)) {
>>>>               needed_len += kernel_alg_ah_auth_keylen(
>>>>                   pi->attrs.transattrs.integ_hash);
>>>>               break;
>>>>           }
>>>>           bad_case(pi->attrs.transattrs.encrypt);
>>>>       }
>>>>
>>>> short of me reading the IKEv1 spec, can anyone explain what is happening here?
>>>>
>>>> Andrew
>>>> _______________________________________________
>>>> Swan-dev mailing list
>>>> Swan-dev at lists.libreswan.org
>>>> https://lists.libreswan.org/mailman/listinfo/swan-dev
>>>
>