Allow gratuitous ARP requests. #1684
Conversation
… / reply. - NB: Removed zeroing of MAC address if the ArpLayer contains a opcode ARP_REQUEST. - Added new structs with only required data to construct a certain packet.
Dimi1010
force-pushed
the
refactor/gratuitous-arp-request
branch
from
679bdbd to
d0a507a
Compare
- Added a new private constructor to handle populating the header. The old constructor is forwarded to this one with the caveat that the target MAC address is ignored and zeroed if a request is to be made.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## dev #1684 +/- ##
==========================================
+ Coverage 82.54% 83.18% +0.63%
==========================================
Files 277 277
Lines 48081 48294 +213
Branches 9341 9923 +582
==========================================
+ Hits 39688 40171 +483
+ Misses 7491 7228 -263
+ Partials 902 895 -7
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
|
@Dimi1010 the main issue I see with this approach is that it's quite strict and limits users to specific options to create ARP messages. Creating a non-standard ARP message is not possible unless they use the deprecated c'tor. My suggestion here keeps the existing c'tor which gives the most freedom while creating another c'tor for gratuitous ARP requests which AFAIK are quite rarely used (no one requested it until now, even though ARP exists for many years in PcapPlusPlus) |
Deprecating the current ctor is not really a requirement, I deprecated it because people might have been relying on the zeroing the target MAC functionality. The new private ctor can also be exposed to allow ppl to construct whatever packets they want, which would probably work better, as the MAC addr wont be zeroed out under their feet if they dont want it. |
When I think about it, maybe we can just remove zeroing out We probably need to update some tests and examples that use ARP (like Arping) which assume an ARP request always has a |
I can make the Also a standard ARP request should have the target mac zeroed. It is in the spec. |
You convinced me 🙂 |
…e type of ARP message.
There was a problem hiding this comment.
There are also ARP layers generated in 3 places in ArpSpoofing that we can modify: https://github.com/seladb/PcapPlusPlus/blob/feba0b7124a4536ec2a0a947bc96c146ada099d7/Examples/ArpSpoofing/main.cpp
| * @remarks No validation is done on the input parameters. The caller must ensure that the input creates a valid | ||
| * header. | ||
| */ | ||
| ArpLayer(ArpOpcode opCode, const MacAddress& senderMacAddr, const IPv4Address& senderIpAddr, |
There was a problem hiding this comment.
Why do we need a new c'tor with the same params as the other c'tor but in a different order?
There was a problem hiding this comment.
The new ctor is because the old one still zeroes the targetMacAddress to keep people who are expecting that from forming invalid arp requests. It just does it in the ctor instead of calculateFields.
The new constructor is there because:
- The targetMacAddr is no longer zeroed. All parameters are directly written in the header.
- Parameters are reordered because you cant have two constructors with identical signature. Also they are now ordered in the way they appear in the header.
| } | ||
|
|
||
| { | ||
| // TODO: Add an actual packet to test against. |
There was a problem hiding this comment.
You can use ArpResponsePacket.dat
This PR is a potential implementation of #1682.
Specific header constructors
ArpLayerhas been expanded with constructors accepting the following structures that build a respective ARP header inside the layer.