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
