[Swan-dev] IKEv1 AH parsing quirk

[Paul Wouters]

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

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
>>