Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WIP ICMP Extension Header bugfixes #4451

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ventaquil
Copy link

Continuing my work with ICMP Extension Header started with this issue GH-4281, I noticed that there are more bugs or missing mechanisms (like GH-4353).

I don't have enough time to do all things at once, also it would be nice to discuss some details during implementation to fulfill your expectations and met coding style.

I started with IPv4 variant, in incoming future it would be necessary to implement similar changes for IPv6.

At this moment I fixed:

  • Setting length attribute when extension header is present and field is not set.

The length attribute MUST be specified when the ICMP Extension Structure is appended to the above mentioned ICMP messages.

  • Accept payload longer than 128 octets.

When the ICMP Extension Structure is appended to an ICMP message and that ICMP message contains an "original datagram" field, the "original datagram" field MUST contain at least 128 octets.

To be done:

  • Set IPv6 length field.
  • Write tests with more scenarios and longer payloads.

RFC for reference - RFC 4884: Extended ICMP to Support Multi-Part Messages.

Copy link

codecov bot commented Jul 4, 2024

Codecov Report

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

Project coverage is 48.94%. Comparing base (a1afb9a) to head (b16c199).

❗ There is a different number of reports uploaded between BASE (a1afb9a) and HEAD (b16c199). Click for more details.

HEAD has 9 uploads less than BASE
Flag BASE (a1afb9a) HEAD (b16c199)
10 1
Additional details and impacted files
@@             Coverage Diff             @@
##           master    #4451       +/-   ##
===========================================
- Coverage   81.46%   48.94%   -32.53%     
===========================================
  Files         353      346        -7     
  Lines       84477    78591     -5886     
===========================================
- Hits        68822    38467    -30355     
- Misses      15655    40124    +24469     
Files Coverage Δ
scapy/layers/inet.py 23.86% <0.00%> (-47.92%) ⬇️

... and 262 files with indirect coverage changes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant