On Thu, 12 Nov 2020 at 13:16, Sahana Prasad <[email protected]> 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? However, what's the calculation used? Is it a variation on the above or something else specified by the RFC? > 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: - 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 <[email protected]> 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 <[email protected]> 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, <[email protected]> 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 >> >> [email protected] >> >> https://lists.libreswan.org/mailman/listinfo/swan-dev _______________________________________________ Swan-dev mailing list [email protected] https://lists.libreswan.org/mailman/listinfo/swan-dev
