Add support for HMAC based on sha256 (128b keys).

[kjopek]

Add support for HMAC based on SHA256 with 128 bits keys.

It can be used in the following form: ./tcpdump -nr ~/ipsec_dump.pcap -s0 -E "[email protected] aes256-cbc-hmac-sha256-128:[KEY1],[email protected] aes256-cbc-hmac-sha256-128:[KEY2]"

Sponsored by: Digital Fingerprints

[fxlb]

And please, rebase on top of the-tcpdump-group:master.

[infrastation]

In addition to the previous comment, which still stands (merging from 3 years ago is not rebasing), you might want to read the file CONTRIBUTING.md (especially step 4), so you have a compiler feedback loop in your working copy.

[kjopek]

Thank you very much for all your comments and I am very sorry for breaking contributors' rules.

From what I see now, after 3+ years, this patch was wrong from the very beginning and here is my explanation why I think so.

The original patch was kind-of quick and therefore dirty hack to allow IPSEC debugging under certain configuration. Particularly, to make sure the encryption works as expected and data is correctly encrypted/decrypted. In my humble opinion, the pull request should have been splitted into at least two smaller ones:

1. First one to introduce strendswith function and use it where needed.
2. Second one to introduce additional primitive for packet authentication.

While the first one is rather obvious, the second one is tricky as the whole function relies on authlen variable which is used only for keeping length of hmac code and is required only to calculate correct offsets in the packet to decrypt it correctly. I see no other usage of authentication-related code in print-esp.c file. sa_list structure maintains authsecret and authsecret_len fields which are unpopulated and therefore does not play any role in printing ESP packets. In my humble opinion, tcpdump lacks hmac verification and this functionality could be added separately.

Summing up, my original intention was to enable ESP decryption for different ENC+AUTH scenario, but from my current perspective this pull request should be closed and I should create another one only to implement strendswith function. The remaining part (lack of auth support) should be discussed and added separately. I am leaving this PR open to allow further discussion.

[infrastation]

Thank you for the detailed comment. If you think it would make more sense to split the change into two commits, split it. If the first commit is useful on its own, it can be merged on its own. Would you like to take more time to state the decryption problem properly?

[ZsBT]

+1 for this (and keeping the issue open)