[Swan-dev] dh and intermediate exchanges
Andrew Cagney
andrew.cagney at gmail.com
Fri Nov 13 01:52:40 UTC 2020
On Thu, 12 Nov 2020 at 16:18, Sahana Prasad <sahana.prasad07 at gmail.com> wrote:
>
>
>
> On Thu, Nov 12, 2020 at 9:00 PM Andrew Cagney <andrew.cagney at gmail.com> wrote:
>>
>> On Thu, 12 Nov 2020 at 13:16, Sahana Prasad <sahana.prasad07 at gmail.com> wrote:
>> >
>> > Hello Andrew,
>> >
>> > The author of the intermediate exchange draft (Valery), confirmed that we always use the last recalculated keys to protect the next
>> >
>> > exchange (whether it is IKE_AUTH or next IKE_INTERMEDIATE). If keys are not recalculated in an IKE_INTERMEDIATE (no KE were exchanged),
>> >
>> > then they don’t change and are used in the next exchange. Our case is the same, we don't exchange KE, so we use the same keys for both exchanges.
>>
>> I'm not sure I follow.
>>
>> When the IKE_SA_INIT response comes back two calculations are performed:
>> - the shared DH secret using the local DH secret and remote KE (aka
>> .st_gr) - see calc_dh_shared_secret()
>> - the the keying material using shared DH secret and a key derivation
>> function (KDF/PRF+) - see (the new) calc_v2_keymat()
>> these keys are then used to protect the next message - either an
>> IKE_AUTH or IKE_INTERMEDIATE request.
>>
>> I gather that, after each intermediate exchange, the keying material
>> needs to be recomputed ready for the next exchange?
>
> yes
>>
>> However, what's the calculation used? Is it a variation on the above
>> or something else specified by the RFC?
>
> It is the same as above.
> This section in the draft confirms that -
> https://tools.ietf.org/html/draft-ietf-ipsecme-ikev2-intermediate-04#section-3.3.1
In the above I noted two operations:
#1 compute the shared DH secret (aka g^ir)
#2 mash g^ir and N[ir] (all from IKE_SA_INIT) into SKEYSEED, and then
expand SKEYSEED using PRF+, and split the result into aka SK_e[i/r]
and SK_a[i/r]
Looking at 3.3.1 it cites
https://tools.ietf.org/html/rfc7296#section-2.14 which is discussing
the second step. Beyond pointing out that the very first calculation
is identical to the existing exchange, it says nothing about what the
"additional key exchange" value is and how exactly it is to be fed
into the above. Presumably as a replacement for g^ir? Who knows.
In the new code, #1 is performed by submit_dh_shared_secret() and #2
by calc_v2_keymat().
In the old code, calc_dh_v2(), which runs on a helper thread,
computes both the DH shared secret (using calc_dh_shared()) and the
key material (calc_skeyseed_v2()). A later call to finish_dh_v2()
then saves the computed keying material in the state (think of
calc_v2_keymat() as replacing finish_dh_v2(), but it isn't that
simple).
For the code path in question, finish_dh_v2() wasn't called. Hence
the results aren't saved and things leak (for the new code, I
preserved this behaviour by omitting an equivalent calc_v2_keymat()
call).
But why do things "work"? Because the inputs g^ir and N[ir] are from
the IKE_SA_INIT exchange, and those inputs haven't changed.
So I see two options:
- replace the redundant DH calculation with comments moting where to
add "additional key exchange" calculation
- add the missing calc_v2_keymat(); and document it as redundant in
most cases (re-computing a quantum proof key will be very expensive)
>>
>> > Yes we recalculate the keys the second time, but it should be the same as the first one.
>> >
>> > You are right, the recalculation is unnecessary.
>>
>> My puzzlement is more that when the IKE_INTERMEDIATE response comes
>> back the code calculates:
>>
>
> I'm puzzled too now :) The current code on main, looks very different from what I used to look at.
> It is missing a couple of things from ikev2_parent.c?
>
> https://github.com/yulia-kuz/libreswan/commit/db53cc9352fa58e9525b84f7641a8db95c3420da#diff-ed770c143a1a1ef20fac4bb54fc2cd4a14b16fe06822d65752b03e3181713647
>
> This was the code I was familiar with
> (https://github.com/yulia-kuz/libreswan/commits/ike_int)
>
>
>> - the shared DH secret using the local DH secret and remote KE (aka
>> .st_gr) - see calc_dh_shared_secret()
>>
>> but then completely ignores the result -> the re-computed shared DH
>> secret isn't used
>>
>> (I can understand the code doing some sort of NOP as a placeholder for
>> the real intermediate key derivation calculation, but only when the
>> result is used).
>>
>> >
>> > (We did it that way, because we weren't sure how to handle the key calculation.
>> >
>> > It was already being done before we knew we wanted to do intermediate, also we use common
>> >
>> > functions in state transitions for both IKE_AUTH and IKE_INT. Perhaps that should change too)
>> >
>> >
>> > Added Yulia in case I have missed any details.
>> >
>> >
>> > Thank you,
>> >
>> > Regards,
>> >
>> > Sahana Prasad
>> >
>> >
>> > On Mon, Nov 9, 2020 at 4:07 PM Andrew Cagney <andrew.cagney at gmail.com> wrote:
>> >>
>> >> thanks!
>> >>
>> >> As an aside. I'm currently cleaning up the crypto code by eliminating
>> >> the remaining pcr style crypto helpers. A likely side effect is that
>> >> the memory leak will disappear. The possibly unnecessary DH
>> >> calculation will remain, it just won't leak.
>> >>
>> >> On Wed, 4 Nov 2020 at 16:22, Sahana Prasad <sahana.prasad07 at gmail.com> wrote:
>> >> >
>> >> > Hello Andrew,
>> >> > Thanks for the analysis.
>> >> >
>> >> > I'll get back to you on this one with more details.
>> >> >
>> >> > Thank you,
>> >> > Regards,
>> >> > Sahana Prasad
>> >> >
>> >> > On Wed, 4 Nov 2020, 20:56 Andrew Cagney, <andrew.cagney at gmail.com> wrote:
>> >> >>
>> >> >> The immediate problem is that the intermediate exchange is leaking the
>> >> >> local dh secret. However there seems to be more going on - someone
>> >> >> familiar with intermediate code should probably take a look.
>> >> >>
>> >> >> - the initiator, on receipt of an IKE_SA_INIT responce calls,
>> >> >> ikev2_parent_inR1outI2() which finishes with:
>> >> >>
>> >> >> /* If we seen the intermediate AND we are configured to use intermediate */
>> >> >> /* for now, do only one Intermediate Exchange round and proceed
>> >> >> with IKE_AUTH */
>> >> >> crypto_req_cont_func (*pcrc_func) = (ike->sa.st_seen_intermediate
>> >> >> && (md->pbs[PBS_v2N_INTERMEDIATE_EXCHANGE_SUPPORTED] != NULL) &&
>> >> >> !(md->hdr.isa_xchg == ISAKMP_v2_IKE_INTERMEDIATE)) ?
>> >> >> ikev2_parent_inR1outI2_continue : ikev2_parent_inR1outI2_continue;
>> >> >> submit_v2_dh_shared_secret(st, "ikev2_inR1outI2 KE",
>> >> >> SA_INITIATOR,
>> >> >> NULL, NULL, &st->st_ike_rekey_spis,
>> >> >> pcrc_func);
>> >> >> return STF_SUSPEND;
>> >> >>
>> >> >> so it submits a request to finish the DH calculation and compute
>> >> >> dh-shared-secret#1, and then since it is doing an intermediate change
>> >> >> chooses to continue with ikev2_parent_inR1out_intermediate(). This
>> >> >> presumably will send an encrypted IKE_INTERMEDATE request
>> >> >>
>> >> >> - the initiator, on receipt of an IKE_INTERMEDIATE response, also
>> >> >> calls ikev2_parent_inR1outI2(), which submits a request to compute
>> >> >> dh-shared-secret#2, but this time chooses to continue with
>> >> >> ikev2_parent_inR1outI2_continue(). This presumably will send an
>> >> >> encrypted IKE_AUTH request.
>> >> >>
>> >> >> It's this second continue that is leaking:
>> >> >> - ikev2_parent_inR1out_intermediate() calls
>> >> >> finish_v2_dh_shared_secret() unconditionally which saves
>> >> >> dh-shared-secret#1 in the state
>> >> >> - ikev2_parent_inR1outI2_continue(), only calls
>> >> >> finish_v2_dh_shared_secret() when the response isn't IKE_INTERMEDIATE
>> >> >> (and here it is) so doesn't save dh-shared-secret#2 leaking it
>> >> >> (you'll need to trace through a few calls or sprinkle some debug lines
>> >> >> to see this)
>> >> >>
>> >> >> So, does anyone know the back story, and what should be happening? Is
>> >> >> the DH needed for instance?
>> >> >>
>> >> >> Andrew
>> >> >> _______________________________________________
>> >> >> Swan-dev mailing list
>> >> >> Swan-dev at lists.libreswan.org
>> >> >> https://lists.libreswan.org/mailman/listinfo/swan-dev
More information about the Swan-dev
mailing list