[Swan-dev] code smell
andrew.cagney at gmail.com
Tue Oct 13 15:44:53 UTC 2020
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
- 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 <hugh at mimosa.com> 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
> Swan-dev at lists.libreswan.org
More information about the Swan-dev