Improve SIP packet detection using heuristic parsing #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? 🤔 |
|
@seladb Upon reviewing the code in Wireshark, I concluded that further investigation is needed. The logic in Wireshark is essentially designed to search the first line and split it into three tokens. Since the format for request and response is defined in the RFC, this determination was made based on that. Specifically, the RFC defines SIP requests and responses in the following way:
However, based on your comment, I noticed that part of the logic in your parsing already includes this check, so I decided to build on that. For SIP responses, everything was straightforward and clear because of the fixed length of the version and the three-digit status code, which made detection easy. I could use the However, for Therefore, I added two static methods to the
This was necessary to ensure that the parsing of SIP requests was handled correctly without violating the existing design logic. Please let me know if you think there's a better way to approach this or if there are any concerns with my changes. |
|
@sorooshm78 I think I understand it better now: you'd like to be able to parse SIP messages that are not on port 5060. Since the parsing logic is part of the fast path and can potentially run on every captured packet, it should be as efficient as possible, which is why we need to be careful when parsing strings. That's the reason we check the port first and then try to parse the first line. To be honest, I don't see a good way to run this parsing logic on every packet in an efficient manner that would not hurt the performance. Do you know if there are standard SIP ports other than 5060? If yes, maybe we can add them to the list and be able to parse a larger variaty of SIP packets... 🤔 |
@seladb A solution I have considered plausible is a 2 pass system. Basically the ports work only as a hint to which protocol type to try to parse as first. This would allow the library to match packets from "known" protocol ports fast, while also allowing parsing capability of packets coming from "unknown" ports. Below is a flow chart on how it can work. flowchart TD
start[Inbound Packet]-->portCheck{Check Ports}
portCheck -->|Port Matches| discectHintCheck{Run dissector for hinted port}
discectHintCheck --> |No match| runAllDissectors
portCheck -->|No match| runAllDissectors{Run all dissectors}
discectHintCheck -->|Data matches| packetParsed[Parsed Packet]
runAllDissectors -->|Data matches| packetParsed
runAllDissectors--> |No match| unknownPacket[Unknown Packet]
Basically if the inbound packet is HTTP and comes on port 80, it will be fast tracked to the HTTP parser by the port 80 -> HTTP hint. If it came on port 41518, it would fail all port hints and go to the second stage where all the possible protocols are tested for signature match, eventually getting to the HTTP parser and matching. Such architecture should keep performance on the fast path, while allowing protocols on unknown ports to be parsed. The only trade-off is that completely unparsable packets need to go through more checks until they are discarded. |
|
@seladb
|
sorooshm78
changed the title
@sorooshm78 I think this aligns with @Dimi1010 's suggestion above ☝️ The main issue I see with this approach is that all of the non-classified protocols (meaning protocols PcapPlusPlus doesn't yet parse) will fall into this bucket and will be checked for SIP matching (and in the future, maybe more protocols). Maybe we can combine this with option (2) - we can have add 2 static // Parse with checking the port
static SipLayer* parseSipLayer(uint8_t* data, size_t dataLen, Layer* prevLayer, Packet* packet, uint16_t srcPort, uint16_t dstPort);
// Parse without checking the port
static SipLayer* parseSipLayer(uint8_t* data, size_t dataLen, Layer* prevLayer, Packet* packet);The first method will do roughly what the current parsing logic does: check the ports first, then check if it's a request or a response. It will be called instead of the current SIP parsing. The second method will do what option (2) suggests: check the first 3 characters: if it looks like a SIP response - continue checking the version, status code, etc. If it looks like a request - continue checking the SIP method. This method will be called last - after all the port checks. Please let me know what you think. |
3 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