Project

General

Profile

Actions

Bug #7651

open

decoder/pppoe: valid packets are getting dropped as decoder.ppp.unsup_proto

Added by Thomas Winter 4 months ago. Updated 3 days ago.

Status:
In Review
Priority:
Normal
Assignee:
Target version:
Affected Versions:
Effort:
Difficulty:
Label:

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.


Subtasks 1 (1 open0 closed)

Bug #7826: Regular PPPOE packets are getting dropped as decoder.ppp.unsup_proto (7.0.x backport)AssignedOISF DevActions
Actions #1

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;
+

Actions #2

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 ?

Actions #3

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

Actions #4

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)
Actions #5

Updated by Philippe Antoine 27 days ago

  • Label Needs backport to 7.0 added
  • Label deleted (Needs Suricata-Verify test)
Actions #6

Updated by Philippe Antoine 27 days ago

  • Target version changed from TBD to 8.0.1
Actions #7

Updated by OISF Ticketbot 27 days ago

  • Subtask #7826 added
Actions #8

Updated by OISF Ticketbot 27 days ago

  • Label deleted (Needs backport to 7.0)
Actions #9

Updated by Philippe Antoine 27 days ago

  • Assignee changed from OISF Dev to Thomas Winter
Actions #10

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
Actions

Also available in: Atom PDF