Bug #7651
opendecoder/pppoe: valid packets are getting dropped as decoder.ppp.unsup_proto
Description
After upgrading from suricata 7.0.6 to 7.0.8, we found that PPP packets were getting dropped.
The packets were getting marked as PPP_UNSUP_PROTO in DecodePPPOESession. If rules are configured as drop then the packets now get dropped.
I tracked down the change in behaviour to this commit https://github.com/OISF/suricata/commit/b23fa51e331141a2743ef7ded4ec01db951ec697 ("detect: fix decoder only events" for #7414 ).
I made a suricata-verify test using the pcap from https://www.cloudshark.org/captures/9ee1fd7ca29e with the ppp(oe) rules from suricata/rules/decoder-events.rules and all the PPPoES packets get alerted. I haven't made a pull request as I'm not sure about the licensing requirements for that file (it was first google result for pppoe pcap) - I can look into generating a pcap myself if needed. With the patch reverted, the packets do not get alerted.
I am guessing this is a result of ENGINE_SET_EVENT being called on the packet - but should that result in the packet being detected compared to ENGINE_SET_INVALID_EVENT?Perhaps DecodePPPOESession and DecodePPPUncompressedProto should not be setting the PPP_UNSUP_PROTO on various proto values including:
- PPP_IPCP
- PPP_IPV6CP
- PPP_LCP
- PPP_PAP
- PPP_CHAP
- PPP_CCP
The previously mentioned pcap also contains a proto that suricata doesn't know, 0x8207 CDPCP, that instead gets marked as PPP_WRONG_TYPE as it's not in the switch statement.
Updated by Thomas Winter 4 months ago
Locally I have made DecodePPPUncompressedProto and DecodePPPOESession just return TM_ECODE_OK for the following protocols IDs which should be expected in a PPP connection:
+ case PPP_IPCP:
+ case PPP_IPV6CP:
+ case PPP_LCP:
+ case PPP_PAP:
+ case PPP_CHAP:
+ case PPP_CCP:
+ case PPP_LQM:
+ case PPP_CBCP:
+ case PPP_COMP_DGRAM:
+ /* Valid types to be in PPP but don't inspect validity. */
+ return TM_ECODE_OK;
+
Updated by Philippe Antoine about 1 month ago
- Status changed from New to Feedback
- Label Needs Suricata-Verify test added
Could you supply a pcap to test ?
Updated by Thomas Winter 28 days ago ยท Edited
Hi, I have made suricata-verify PR that contains a pcap I captured myself https://github.com/OISF/suricata-verify/pull/2607
I'll also make a suricata PR with the changes I made there. https://github.com/OISF/suricata/pull/13623
Updated by Philippe Antoine 27 days ago
- Status changed from Feedback to In Review
- Affected Versions 8.0.0 added
- Affected Versions deleted (
8.0.0-beta1)
Updated by Philippe Antoine 27 days ago
- Label Needs backport to 7.0 added
- Label deleted (
Needs Suricata-Verify test)
Updated by Philippe Antoine 27 days ago
- Target version changed from TBD to 8.0.1
Updated by Philippe Antoine 27 days ago
- Assignee changed from OISF Dev to Thomas Winter
Updated by Victor Julien 3 days ago
- Subject changed from Regular PPPOE packets are getting dropped as decoder.ppp.unsup_proto to decoder/pppoe: valid packets are getting dropped as decoder.ppp.unsup_proto