feat: added hsrpv2 support layer#5032
Open
pengsies wants to merge 6 commits into
Open
Conversation
Contributor
|
Thanks for the PR! |
AI-Assisted: no
…er for ipv4/ipv6) AI-Assisted: yes (OpenAI Codex)
AI-Assisted: no
AI-Assisted: yes (OpenAI Codex)
AI-Assisted: no
AI-Assisted: yes (OpenAI Codex)
34d5336 to
00566d3
Compare
Author
|
oops no worries! thanks :D |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #5032 +/- ##
==========================================
- Coverage 80.21% 80.21% -0.01%
==========================================
Files 387 387
Lines 96405 96505 +100
==========================================
+ Hits 77331 77409 +78
- Misses 19074 19096 +22
🚀 New features to boost your workflow:
|
Contributor
|
Please fix the code health checks. |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adds native TLV-based HSRPv2 parsing/building support to Scapy’s existing HSRP implementation, while aiming to preserve current HSRPv1 behavior (notably on UDP/1985 for IPv4).
Changes:
- Added new
HSRPv2+ TLV container/dispatch logic, including Group State / Interface State / Text Auth / MD5 Auth TLVs and unknown TLV preservation. - Updated UDP binding/dispatch to distinguish HSRPv1 vs HSRPv2 on IPv4 UDP/1985 and bind IPv6 HSRPv2 on UDP/2029 with
ff02::66. - Added regression tests covering HSRPv1 compatibility and HSRPv2 TLV build/dissection behavior.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
scapy/layers/hsrp.py |
Introduces the HSRPv2/TLV packet model and adjusts dispatch/binding logic for UDP/1985 (IPv4) and UDP/2029 (IPv6). |
test/scapy/layers/hsrp.uts |
Adds tests for HSRPv2 TLV build/dissection, multi-TLV chaining, unknown TLVs, and IPv4/IPv6 port behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+302
to
+306
| """ | ||
| since port 1954 (UDP) would be shared by both hsrpv1 and hsprv2 (ipv4), | ||
| users building hsrpv2 (ipv4) packets should set IP(dst="224.0.0.102") explicitly to avoid using wrong multicast destination | ||
| """ | ||
| # DestIPField.bind_addr(UDP, "224.0.0.102", dport=1985) |
Comment on lines
+65
to
+66
| if tlvtype not in _HSRP_V2_TLV_TYPES: | ||
| return False |
Comment on lines
+234
to
+238
| class HSRPv2MD5AuthTLV(_HSRPv2TLVPayload): | ||
| name = "HSRPv2 MD5 Authentication TLV" | ||
| fields_desc = [ | ||
| ByteEnumField("type", 4, _HSRP_V2_TLV_TYPES), | ||
| ByteField("len", None), |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
context
hi! This is my first pull request to an open-source project, so please let me know if there is anything I should adjust.
so while working with HSRP packet building for a poc in my uni's hsrp network security project, i noticed that scapy’s existing hsrp layer supports the classic hsrpv1 packet format, but not the tlv-based format of hsrpv2. of course, because Scapy does not currently expose a native hsrpv2 layer, this means that packets of that nature would have to be constructed manually as raw udp payloads which was what i did. for example, the group state tlv and text authentication tlv need to be packed manually before being attached as
Raw(load=...)..but in any case, i thought that it would be neat if ____, so i ____.thought it would be useful for Scapy to support hsrpv2 directly, which resulted in this small contribution :D
tldr; PR adds a hsrpv2 support layer without changing any existing hsrpv1 packet model(s).
description
PR adds native hsrpv2 tlv support, and includes:
notes
previous behavior is intended to remain unchanged.
for ipv4 hsrpv2, UDP:1985 is shared with hsrpv1. because of that, i thought that Scapy should not automatically change the ipv4 destination address default to
224.0.0.102in case it was meant for hsrpv1. this would mean that when building ipv4 hsrpv2 packets, users should explicitly set the multicast destination as:instead of relying scapy to infer the destination automatically usually as shown below
however, for IPv6 HSRPv2, there would be no issue, as this change would automatically binds hsrpv2 to UDP:2029 and
ff02::66.MD5 Authentication tlvs are currently preserved as unknown tlvs rather than fully dissected, pending verified packet captures or specification confirmation for the exact ivp4/ipv6 layout.
tests
on top of existing hsrpv1 tests, i checked for:
closing remarks!
thanks for reviewing! please do let me know if there are any remarks or changes that i might need to make