[Swan-dev] clanger: ikev2_crypto_start

[Andrew Cagney]

On Sun, 10 Jun 2018 at 17:23, D. Hugh Redelmeier <hugh at mimosa.com> wrote:
>
> In front of this function, the following comment appears:
>
> /*
>  * We need an md because the crypto continuation mechanism requires one
>  * but we don't have one because we are not responding to an
>  * incoming packet.
>  * Solution: build a fake one.  How much do we need to fake?
>  * Note: almost identical code appears at the end of aggr_outI1.
>  *
>  * XXX: This code does a crypto continue using an indirect dispatch
>  * through the FSM.  Beyond making the code flow confusing is this
>  * useful?  For instance, since SA_INIT has only one code path, it can
>  * directly request ke and nonce with its dedicated continue function
>  * - no need to jump through all these hoops.
>  */
>
> And yet, two cases look into the md to get a v2KE (key exchange
> payload).  This seems contradictory.
>
> Which is right?  Can someone improve the comments to reflect the
> resolution.

You mean the first or second comment?  The first comment is referring
to the initiator case (and it may well have originally been attached
to a function that only dealt with initiating).  The two cases below
are for a responder where there is an MD.

Anyway, I'd go with the second comment:  ikev2_crypto_start() can be
be inlined at the call sites; and  ikev2_crypto_continue() eliminated.
This, with certain irony, makes the code more like IKEv1.



>         case STATE_V2_CREATE_R:
>                 /*
>                  * ??? if we don't have an md (see above) why are we referencing it?
>                  * ??? clang 6.0.0 warns md might be NULL
>                  */
>                 if (md->chain[ISAKMP_NEXT_v2KE] != NULL) {
>                         request_ke_and_nonce("Child Responder KE and nonce nr",
>                                              st, st->st_oakley.ta_dh,
>                                              ikev2_crypto_continue);
>                 } else {
>                         request_nonce("Child Responder nonce nr",
>                                       st, ikev2_crypto_continue);
>                 }
>                 return STF_SUSPEND;
>
>         case STATE_V2_REKEY_CHILD_R:
>                 /*
>                  * ??? if we don't have an md (see above) why are we referencing it?
>                  * ??? clang 6.0.0 warns md might be NULL
>                  */
>                 if (md->chain[ISAKMP_NEXT_v2KE] != NULL) {
>                         request_ke_and_nonce("Child Rekey Responder KE and nonce nr",
>                                              st, st->st_oakley.ta_dh,
>                                              ikev2_crypto_continue);
>                 } else {
>                         request_nonce("Child Rekey Responder nonce nr",
>                                       st, ikev2_crypto_continue);
>                 }
>                 return STF_SUSPEND;
> _______________________________________________
> Swan-dev mailing list
> Swan-dev at lists.libreswan.org
> https://lists.libreswan.org/mailman/listinfo/swan-dev