On Thu, Nov 12, 2020 at 9:00 PM Andrew Cagney <[email protected]> wrote:
> 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? > 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 > > 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 <[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
