Review/testing of ESP decryption changes ?

[Martin Mathieson <martin.r.mathieson () googlemail com>]

I had a look through the history of packet-ipsec.c, looking for specific
people to invite to review my patch  (
https://code.wireshark.org/review/#/c/1421/), but so many people have been
involved I thought I'd just post to wireshark-dev.  The description for the
change was as follows:

"Add an API for programmatically adding ESP SAs (most likely from a private

dissector).

Also, speed up ESP decryption in several ways:
- store gcrypt_cipher_hd in the SA struct, rather than continually
open, setkey and close for each PDU
- don't convert the key string from ascii to hex each time - do it upon
during update callback and keep
- do the decryption in-place, avoiding the need to allocate, memcpy and
free a separate buffer for encrypted data
- when matching addresses, avoid doing a strlen until after we check
whether or not we're matching against "*""


I use ESP in quite a particular way, so it would be good to get some more
confidence in the changes before pushing them through.  I haven't done any
before/after performance measurements, but I seem to remember  that keeping
the gcry_cipher_hd_t around rather than continually opening, setting the
key and closing it in particular can make a big difference.  I don't have
any ESP CTR mode captures but I did check in a test program that calling
gcry_cipher_setctr() (followed by gcry_cipher_decrypt() multiple times)
seems to work OK.

There are some obvious follow-up changes that could be made that I wanted
to leave for now to keep the patch at a manageable size:
- do something more dynamic with the extra SAs that get added by the new
function.  For now I just added an array of 4 (in static memory)
- move the #defines of the defines for the algorithm and authentication to
the header file so calling dissectors could use them
- look at speeding up authentication by similarly keeping the gcry_md_hd_t
around for the lifetime of the entry.  So far I haven't used authentication
checking at all.

Thanks,
Martin

___________________________________________________________________________
Sent via:    Wireshark-dev mailing list <wireshark-dev () wireshark org>
Archives:    http://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev
             mailto:wireshark-dev-request () wireshark org?subject=unsubscribe