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

Add support for S1G beacon to the dot11 layer. (#2) #4458

Open
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

rkinder2023
Copy link
Contributor

Add support for 802.11ah (S1G) beacon parsing.

  • Includes unit test parsing and confirming an S1G beacon
  • Includes support for the new Frame Control format for type=3, subtype=1 (S1G beacon)
  • Includes changes to support addressing used in S1G beacon.

All dot11 unit tests pass with this change.

Fixes #4439

 - Includes unit test parsing and confirming an S1G beacon
 - Includes support for the new Frame Control format for type=3,
   subtype=1 (S1G beacon)
 - Includes changes to support addressing used in S1G beacon.

All dot11 unit tests pass with this change.
Copy link

codecov bot commented Jul 12, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 81.67%. Comparing base (afd859a) to head (fc06630).

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #4458   +/-   ##
=======================================
  Coverage   81.67%   81.67%           
=======================================
  Files         355      355           
  Lines       84830    84835    +5     
=======================================
+ Hits        69287    69292    +5     
  Misses      15543    15543           
Files Coverage Δ
scapy/layers/dot11.py 91.52% <100.00%> (+0.07%) ⬆️

... and 5 files with indirect coverage changes

@rkinder2023
Copy link
Contributor Author

I'm really not sure what to do here as the commit into my master branch shows coverage at the correct level (80.73%). I don't know why coverage is reported so low here.

https://app.codecov.io/github/rkinder2023/scapy-s1g/commit/d377b137fa67845896442fd01615abcd883bc590

Any hints from the maintainers?

* Add support for S1G beacon to the dot11 layer.
 - Includes unit test parsing and confirming an S1G beacon
 - Includes support for the new Frame Control format for type=3,
   subtype=1 (S1G beacon)
 - Includes changes to support addressing used in S1G beacon.

All dot11 unit tests pass with this change.
@rkinder2023
Copy link
Contributor Author

G'day folks, can I please get approval for the workflow (review would be great too :)? @gpotter2, @guedou, @p-l- ?

scapy/layers/dot11.py Outdated Show resolved Hide resolved
scapy/layers/dot11.py Outdated Show resolved Hide resolved
scapy/layers/dot11.py Outdated Show resolved Hide resolved
* Fix review comments from @p-l-:
 - Use set for 'type in' checks for extension frame type 3, subtype 1
   (S1G beacon).

* Update to FCfield to split into three parts for type 3, subtype 1 (S1G
beacon frame control field). This allows use of BitField for the 'bw',
and FlagsField for the remaining bits.
@rkinder2023
Copy link
Contributor Author

Could I please get another review? I have modified to use a set on the 'type in' as per @p-l-, and updated the frame control field to split into three sections for type 3, subtype 1 (@p-l-, @gpotter2 suggestions). This seems the best solution as a change to the underlying frame control to use BitField for all types and subtypes will introduce significant back-compat issues.

Let me know if there is any more testing/unit testing you'd like on this change? I'd like to push this one in as I have more changes to support the S1G IEs within the beacon queued up.

Thanks guys!

@rkinder2023
Copy link
Contributor Author

Hi folks, any further review or comments? @p-l- @gpotter2 @guedou?

@gpotter2
Copy link
Member

gpotter2 commented Aug 9, 2024

Yeah sorry for the delay. I wanted to test your PR, and still couldn't get around to do it.
Would you mind sharing a pcap that contains a few of the packets you are adding?

@rkinder2023
Copy link
Contributor Author

@gpotter2, PCAP parsing for the trace I have doesn't work, I'm working on an acceptable solution to this. I managed to hack something in place in order to extract this beacon frame, but don't yet have something ready for review.

@rkinder2023
Copy link
Contributor Author

rkinder2023 commented Aug 13, 2024

S1G-beacon.pcap.gz

This is the S1G beacon I used for developing the patch. The hack I used to get the pcap to load is here:

rkinder2023@fda0bab

I'm sure there's a better way to make TLVs work properly, but I'm not sure what it is.

@rkinder2023
Copy link
Contributor Author

@gpotter2, any thoughts on this? Anything else you need me to provide?

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.

Add support for S1G beacon to dot11
3 participants