(discussion) Build AH without SecurityAssociation

[Shu-xueyuan]

Fix UDP packet unable to calculate validity when carrying AH extension header

Checklist:

• [x] If you are new to Scapy: I have checked CONTRIBUTING.md (esp. section submitting-pull-requests)
• [x] I squashed commits belonging together
• [x] I added unit tests or explained why they are not relevant
• [x] I executed the regression tests (using cd test && ./run_tests or tox)
• [x] If the PR is still not finished, please create a Draft Pull Request

fixes #xxx

[codecov[bot]]

Codecov Report

Attention: Patch coverage is 0% with 16 lines in your changes missing coverage. Please review.

Project coverage is 49.29%. Comparing base (d71014a) to head (45aee45). Report is 152 commits behind head on master.

:exclamation: There is a different number of reports uploaded between BASE (d71014a) and HEAD (45aee45). Click for more details.

HEAD has 10 uploads less than BASE

| Flag | BASE (d71014a) | HEAD (45aee45) | |------|------|------| ||11|1|

Additional details and impacted files

@@             Coverage Diff             @@
##           master    #4227       +/-   ##
===========================================
- Coverage   81.77%   49.29%   -32.48%     
===========================================
  Files         331      343       +12     
  Lines       76721    77435      +714     
===========================================
- Hits        62736    38175    -24561     
- Misses      13985    39260    +25275     

Files	Coverage Δ
scapy/layers/inet.py	23.74% <0.00%> (-46.88%)	:arrow_down:

... and 273 files with indirect coverage changes

[guedou]

@Shu-xueyuan thanks for your PR. Could you share an example that triggers the issue that you are fixing?

[Shu-xueyuan]

@Shu-xueyuan thanks for your PR. Could you share an example that triggers the issue that you are fixing?

@guedou I'm very glad that you can reply to me. When I use the official scapy library to construct a UDP message carrying Authentication Header, the Checksum of the message is illegal. Using my modified scapy library to construct the same message will not have this problem. The following is my process of calling scapy: originalPacket = Ether(**ETH2_dicts) / IPv6(**IPV6_dicts) / AH(**Ah_dicts) / UDP(**UDP_dicts).

[Shu-xueyuan]

@Shu-xueyuan thanks for your PR. Could you share an example that triggers the issue that you are fixing?

@guedou This is the encapsulation format of AH in transport mode： Looking forward to your reply.

[gpotter2]

Hi. This looks good, but you need to add tests in https://github.com/secdev/scapy/blob/master/test/scapy/layers/inet.uts to make sure that this does not regress. Thanks

[Shu-xueyuan]

@gpotter2 Hello, the test case has been submitted

[Shu-xueyuan]

@gpotter2 Hello, when will my changes be reviewed?

[polybassa]

LGTM

[polybassa]

Please fix the flake8 issues.

[gpotter2]

I noted I still need to check when this PR really applies, and compare to what's already implemented in ipsec.py. I will review this more in depth eventually, sorry for the delay. There were some higher priority issues blocking for 2.6.0

[Shu-xueyuan]

Thanks for your reply, I totally understand. I'll be happy to wait until you have time to review my edits. If there is anything you need further explanation or assistance from me, please feel free to let me know. I fully understand the high-priority issues encountered in version 2.6.0 and hope that the issues can be resolved smoothly. Thanks!

[Shu-xueyuan]

Please fix the flake8 issues.

@polybassa Thank you for your review, the issue has been resolved.

[gpotter2]

Hi, sorry for the delay.

So I think that generally you should consider that AH is supported through the SecurityAssociation class only. I'm not sure if we want to support stacking AH() packets in the normal Scapy way, as I don't really see how those packets would be valid without the crypto that comes with it. What do you think @guedou @p-l- ?

This currently works fine for your case, and builds valid packets with the proper checksum:

>>> sa = SecurityAssociation(AH, spi=1)
>>> sa.encrypt(IP()/UDP()/b"aaa")

Crafting AH or ESP packets without using SecurityAssociation probably means using only 'null' ciphers, and I'm not sure if that's worth the extra code to support it in inet.py.

[guedou]

@Shu-xueyuan could you describe the use case that you have in mind for these changes ?