The two functions have a basic but subtle difference - everything is reversed. For instance: - on the way out its encryption then prf; on the way in it is prf then encryption - on the way out it's aead(true); on the way in it's aead(false) (and at some point that was wrong) - on the way out SA initiator selects *i* keys; but on the way in the SA initiator selects *r* keys So, looking at the last point, having the keys grouped into a sub structure of struct state would be a nice tweak (going by the names, can you tell which field I added :-), but I'd be wary of trying to merge the functions as a whole.
The more interesting issue is that the intermediate exchange code doesn't follow this model. Both the incoming and outgoing code paths put that code last. This might help explain <<??? do these need to be copies?>> (but answering this means means reading several RFCs ...). And I'm curious as to what happens when, in encrypt_v2SK*(), intermediate auth is combined with a non-aead algorithm? On Tue, 13 Oct 2020 at 01:26, D. Hugh Redelmeier <[email protected]> wrote: > > (This isn't a term that rolls off my tongue, but it is a term of art.) > > There is a big chunk of code that is almost identical in > encrypt_v2SK_payload and ikev2_verify_and_decrypt_sk_payload. > > It would be good if this code be shared (likely in a function). > > This would mean that whenever a change is needed, it will only be needed > one place. For example, I've just fixed a bug that appeared in each. If > they used common code, this would have been just one bug. This isn't the > first pair of bugs that I fixed in this code. > > I don't have the time to find the extent of the duplication, but it > includes at least the last IF in each function. > > The duplication itself isn't a bug but it is something to be avoided. > That's what the term "smell" so crudely suggests. > > Where something "smells" in a piece of code, there is often more than one > thing wrong. > _______________________________________________ > 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
