Wireshark · Wireshark-bugs: [Wireshark-bugs] [Bug 12882] TCP packets sometimes are incorrectly parsed as TDS (or other corruptio

[bugzilla-daemon@xxxxxxxxxxxxx]

Comment # 12 on bug 12882 from Michael Mann

(In reply to Guy Harris from comment #11)
> (If you mean "how often does an arbitrary packet in a TCP connection contain
> the end of a multi-segment PDU and the beginning of the next PDU", the
> answer is "often enough that it's a good thing that, given the general way
> tcp_dissect_pdus() works, we happen to handle that case as long as
> reassembly is being done and we managed to dissect the beginning of the
> first of those PDUs".)

I would agree with this, which is why I thought converting the dissector to use
tcp_dissect_pdus negated most of the usefulness of conversation_set_dissector
and why I thought it could be removed as a tradeoff.

> I'd say that any protocol that 1) is a popular protocol and 2) doesn't have
> a fixed port assigned to it should have a heuristic dissector if possible;
> if the heuristic is really weak (such as the RTP heuristic), it should
> probably not be enabled *by default*, but we shouldn't require the user to
> use "Decode As..." for protocols such as ONC RPC-based protocols (to give an
> example of a heuristic that works pretty well and that is pretty useful).

This is where the definition of "heuristic" gets blurred to me.  Many protocols
don't have a strong heuristic, so they're stuck with registering on a port and
using that as the main heuristic.  And for ones that don't have an IANA port,
I'm okay with a dissector "claiming" that port and just resolving the rare case
when then overlap just so all users using/expecting that protocol don't need
the extra step of Decode As.  I think executing that heuristic is much faster
than creating a heuristic function handler that checks for that same port. 
It's also easier to override with Decode As.

> 
> What might be useful here would be
> 
>   1) a way of saying "frames XXX through YYY of this conversation should be
> decode as protocol PPP" (which might also be useful for protocols that can
> switch to TLS in the middle of a session, for example)

This seems like a lot of work.  Yes, I think I'll take shopping instead.

> 
> and
> 
>   2) having tcp_dissect_pdus() (and other wrappers that handle reassembly
> over byte-stream protocols, such as the text-based request-response protocol
> code) on the first pass *automatically* mark the conversation as "all frames
> from the current frame through the end of the capture should be decoded as
> protocol PPP" if it decides that more data needs to be added as part of
> reassembly, so that, at minimum, the next segment in that flow gets handed
> to the dissector so that the reassembly can be one, and have it only close
> out that mark when it finds a PDU that ends at the end of the segment.

I've wondered for a long while why this wasn't implemented (because it seems
like a simple one line change).   I just figured there were people smarter than
me that knew the reason and typically "negatives" aren't documented (like
"here's why we don't call set_conversation_dissector here in the TCP
dissector")

> 
> At least that way, we wouldn't be marking the *entire* conversation as TDS
> in this capture.  If we don't want *any* conversations dissected as TDS,
> either we need to strengthen the heuristic in the TDS dissector, or the user
> has to disable the TDS heuristic.  Users doing a significant amount of MSSQL
> dissection would presumably want to leave that heuristic enabled, as it
> would probably get the right answer more often than it gets the wrong
> answer.)

Regarding "bug maintenance", should this bug be left open until such time as
the heuristic can be strengthened (seems unlikely given what fields are
available)?  Or should it just be marked as "WONTFIX" because users that run
into this should just disable TDS heuristic because they probably don't need
it.

---

      You are receiving this mail because:

• You are watching all bug changes.