Improve SIP Packet Detection Using Heuristic Parsing (Fixes #2022) #2024
Conversation
Packet++/header/SipLayer.h
Outdated
| /// @param[in] data Pointer to the raw data buffer | ||
| /// @param[in] dataLen Length of the data buffer in bytes | ||
| /// @return True if the first line matches SIP request/response syntax, false otherwise | ||
| static bool dissectSipHeuristic(const uint8_t* data, size_t dataLen) |
There was a problem hiding this comment.
We already have SipRequestFirstLine and SipResponseFirstLine that parse the first line, maybe we could use this instead of adding more logic to parse the first line?
There was a problem hiding this comment.
I agree that we already have SipRequestFirstLine and SipResponseFirstLine for parsing the first line, but I think keeping the heuristic logic separate is still necessary because the goals are different.
The Sip*FirstLine classes assume we already decided that the payload is SIP, operate on a Sip*Layer instance, update internal state (m_IsComplete, offsets, logging, etc.), and are meant for full parsing.
In contrast, dissectSipHeuristic() is a stateless, side-effect-free check that runs directly on raw data to answer a simpler question: “does this buffer look like a SIP message at all?”. This also matches Wireshark’s design, where heuristic detection is separate from the actual SIP dissector that parses the first line and fields.
This separation is particularly important for TCP: when we inspect data per segment, the first line may be incomplete. In that case the heuristic must be able to say “need more data / undecided” without constructing SIP layers or marking anything as invalid, which is a different lifecycle than the existing first-line parsers.
In this pull request I’m not yet handling TCP segmentation or IP fragmentation — the heuristic currently assumes it sees at least one complete first line. I plan to address proper TCP stream reassembly / IP fragmentation handling in a separate follow-up PR.
There was a problem hiding this comment.
I agree that we already have
SipRequestFirstLineandSipResponseFirstLinefor parsing the first line, but I think keeping the heuristic logic separate is still necessary because the goals are different.The
Sip*FirstLineclasses assume we already decided that the payload is SIP, operate on aSip*Layerinstance, update internal state (m_IsComplete, offsets, logging, etc.), and are meant for full parsing.In contrast,
dissectSipHeuristic()is a stateless, side-effect-free check that runs directly on raw data to answer a simpler question: “does this buffer look like a SIP message at all?”. This also matches Wireshark’s design, where heuristic detection is separate from the actual SIP dissector that parses the first line and fields.
I just noticed Sip*FirstLine classes do accept a request/response pointer in their constructor, so they can't be used directly. However, they do contain static methods such as parseStatusCode(), parseVersion(), parseMethod() that can definitely be used. If we see we still have a lot of common code between these classes and the parsing logic you need we can think what's the best way to refactor them so they can be used in both scenarios.
In this pull request I’m not yet handling TCP segmentation or IP fragmentation — the heuristic currently assumes it sees at least one complete first line. I plan to address proper TCP stream reassembly / IP fragmentation handling in a separate follow-up PR.
Handling TCP segmentation or IP fragmentation is more tricky - PcapPlusPlus parses packets one by one, there is currently no built-in way to use TcpReassembly or IPReassembly and use the outcome to parse the message again as a packet
There was a problem hiding this comment.
Thanks for the suggestion!
I refactored SipLayer::dissectSipHeuristic() to use the static parsing helpers from SipResponseFirstLine and SipRequestFirstLine instead of manually tokenizing the first line.
For responses I'm now using parseVersion() and parseStatusCode(), and for requests I'm using parseMethod(), parseVersion() and parseUri(). This removes the duplicated parsing logic and keeps the heuristic in sync with the actual SIP first-line parsers.
I didn't use the Sip*FirstLine constructors themselves, as they still require a request/response pointer as you mentioned.
There was a problem hiding this comment.
You’re absolutely right that handling TCP segmentation and IP fragmentation is more complex. Since PcapPlusPlus currently processes packets one by one, in this PR I focused only on heuristic detection on the first line of a single packet. My plan is to add built-in IP fragmentation support to PcapPlusPlus itself in a separate PR, and I’m really excited to work on that.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## dev #2024 +/- ##
=======================================
Coverage 83.87% 83.88%
=======================================
Files 307 307
Lines 53952 54046 +94
Branches 11352 11335 -17
=======================================
+ Hits 45254 45334 +80
- Misses 7483 7494 +11
- Partials 1215 1218 +3
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
@sorooshm78 I just saw we already have most of the logic that you implemented: https://github.com/seladb/PcapPlusPlus/blob/master/Packet%2B%2B/src/UdpLayer.cpp#L113-L120 The 2 things you implemented on top of what we have is parsing the request version and the URI, but are they actually required? 🤔 |
2 participants
This PR improves SIP packet detection in PcapPlusPlus by introducing heuristic parsing based on Wireshark’s SIP dissector. SIP messages are now detected from the UDP payload itself, not only when using port 5060.
static bool dissectSipHeuristic(const uint8_t* data, size_t dataLen)to detect SIP requests/responses from payload content (Wireshark-style logic)UdpLayerso SIP packets on non-standard ports are correctly classifiedRelated issue: #2022
Note: I’m not yet fully familiar with PcapPlusPlus’ internal structure, so if there are better places, names, or patterns for this logic, I’m happy to adjust the PR based on your feedback