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

pcapng enhancements (idb,epb) and some fixes #4342

Merged
merged 7 commits into from Apr 27, 2024

Conversation

jcpvdm
Copy link
Contributor

@jcpvdm jcpvdm commented Mar 29, 2024

Checklist:

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

Based on draft-ietf-opsawg-pcapng-latest, 5 March 2024, I made following changes:

  • map packet.sniffed_on to unique IDB id. When writing, if_name option and linktype is populated based on the first packet seen with a unique sniffed_on string.
  • map packet.direction to EPB flags inbound/outbound direction bits. Remaining flag bits not implemented.
  • simplified RawPcapNgReader._read_options() and moved (code,value) treatment to the caller since codes of same type can have different meaning for different type of blocks.
  • Fix RawPcapNgReader._read_block_shb() and _write_block_shb() to be according to the draft
  • Added new tests and adapted others to the changes.

Note: I couldn't figure the meaning of packet.direction on Scapy API reference or source code, but since it's marked as integer I assumed it follows the pcapng epb_flags outbound/inbound field.

Why I did this changes:
I was using scapy to convert packet hex dump from a text file to pcap. The text file included information about interface name and direction (outbound/inbound) which I would like to to have included in the pcapng file.

Based on draft-ietf-opsawg-pcapng-latest, 5 March 2024.
   -map packet.sniffed_on to unique IDB id. When writing, if_name option and linktype is populated based on the first packet seen with a unique sniffed_on string.
   -map packet.direction to EPB flags inbound/outbound direction bits. Remaining flag bits not implemented.
   -simplified RawPcapNgReader._read_options() and moved (code,value) treatment to the caller since codes of same type can have different meaning for different type of blocks.
   -Fix RawPcapNgReader._read_block_shb() and _write_block_shb().

*added new tests and adapted others to the changes
test/regression.uts Outdated Show resolved Hide resolved
test/regression.uts Outdated Show resolved Hide resolved
scapy/utils.py Show resolved Hide resolved
scapy/utils.py Outdated Show resolved Hide resolved
scapy/utils.py Outdated Show resolved Hide resolved
scapy/utils.py Show resolved Hide resolved
scapy/utils.py Outdated Show resolved Hide resolved
Co-authored-by: Guillaume Valadon <guillaume@valadon.net>
Copy link

codecov bot commented Mar 30, 2024

Codecov Report

Attention: Patch coverage is 90.10989% with 9 lines in your changes are missing coverage. Please review.

Project coverage is 82.12%. Comparing base (f919a6a) to head (b90ff7b).
Report is 19 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #4342      +/-   ##
==========================================
+ Coverage   81.67%   82.12%   +0.44%     
==========================================
  Files         350      350              
  Lines       82884    83195     +311     
==========================================
+ Hits        67699    68326     +627     
+ Misses      15185    14869     -316     
Files Coverage Δ
scapy/packet.py 84.15% <100.00%> (+0.18%) ⬆️
scapy/utils.py 73.40% <89.65%> (+0.30%) ⬆️

... and 41 files with indirect coverage changes

scapy/utils.py Outdated Show resolved Hide resolved
@guedou
Copy link
Member

guedou commented Mar 31, 2024 via email

@jcpvdm
Copy link
Contributor Author

jcpvdm commented Mar 31, 2024

@guedou, I have done some changes as per your review suggestion. Additionally added a length check for the tsresol option when reading IDB.

guedou
guedou previously approved these changes Apr 6, 2024
Copy link
Member

@guedou guedou left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. @gpotter2 can you do a second review?

- changes around self.linktype, so that attribute
is always int (when exists)
- other minor type hint fixes
Copy link
Member

@gpotter2 gpotter2 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR ! This looks good

@gpotter2 gpotter2 merged commit 56b4fa4 into secdev:master Apr 27, 2024
23 checks passed
@jcpvdm jcpvdm deleted the pcapng-enhancements branch April 27, 2024 21:51
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.

None yet

3 participants