dhcp: support option 52 overload#15348
Conversation
catenacyber
left a comment
catenacyber
left a comment
There was a problem hiding this comment.
Thanks for this work : a quick remark : The pcap file does not belong to Suricata repository
Per RFC 2132 the BOOTP sname and file fields can hold extra DHCP options when option 52 is present, but the parser ignored them. After parsing the main options we now look up option 52 and walk sname or file as additional option streams, appending what we find to the same options vector so the logger and detection keywords see the overloaded values too. Bug: OISF#8538.
Document that DHCP options carried in the overloaded BOOTP sname or file fields are now merged into the EVE log option set alongside the main options area. Bug: OISF#8538.
ssam18
force-pushed
the
dhcp-option-52-overload-v2
branch
from
20f9899 to
566d06a
Compare
|
@catenacyber done, force pushed the branch with sname_overload.pcap removed along with the one unit test that loaded it via include_bytes!. The seven synthetic byte-buffer tests still cover the three overload values plus malformed-continuation and no-overload regression cases, and the real pcap is kept in the suricata-verify side at OISF/suricata-verify#3074. |
|
NOTE: This PR may contain new authors. |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #15348 +/- ##
==========================================
- Coverage 82.66% 82.64% -0.03%
==========================================
Files 995 996 +1
Lines 271046 271260 +214
==========================================
+ Hits 224069 224172 +103
- Misses 46977 47088 +111
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
|
@lukashino please assign the issue to me |
|
Done! |
3 participants
Make sure these boxes are checked accordingly before submitting your Pull Request -- thank you.
Contribution style:
https://docs.suricata.io/en/latest/devguide/contributing/contribution-process.html
Our Contribution agreements:
https://suricata.io/about/contribution-agreement/ (note: this is only required once)
Changes (if applicable):
(including schema descriptions)
https://redmine.openinfosecfoundation.org/projects/suricata/issues
Link to ticket: https://redmine.openinfosecfoundation.org/issues/8538
Per RFC 2132 the BOOTP sname and file fields can hold extra DHCP options when the standard options area carries Option Overload (52), but the parser ignored those continuation areas. This closes the visibility gap reported in 8538 where attacker controlled dns_servers, routers or domain values placed inside sname stayed invisible to EVE logging and detection. End to end testing on the reporter pcap shows the overloaded OFFER and ACK now surfacing dns_servers 10.100.0.2 and routers 10.100.0.2 in eve.json, while the parallel benign flow in the same capture continues to report its inline values unchanged.
Replaces #15340.
Describe changes:
Provide values to any of the below to override the defaults.
link to the pull request in the respective
_BRANCHvariable.SV_REPO=
SV_BRANCH=OISF/suricata-verify#3074
SU_REPO=
SU_BRANCH=