On 15/08/19 23:48, Peter Wu wrote:
The problem was introduced with v3.1.1rc0-144-gede7be3440 ("TLS: allow
dissectors to set the appdata protocol via the data param"). Since that
commit, the "data" parameter of TCP is interpreted as a string.
The problem is that the SCTP dissector can also call the TLS dissector
with a non-NULL data parameter:
dissector_try_uint_new(sctp_port_dissector_table, high_port, payload_tvb, pinfo, tree, TRUE, GUINT_TO_POINTER(ppi)))
This dissector table registration happened in ssl_association_add:
dissector_add_uint("sctp.port", port, main_handle);
The data parameter is badly overloaded, all I wanted to is to directly
pass data from the EAP dissector to the TLS dissector via
call_dissector_with_data(tls_handle, ...);
Instead we have several things that can go wrong:
- sctp.port - sctp dissector passes an integer
- tcp.port - TCP passes a "struct tcpinfo" structure
- udp.port - UDP passes NULL (ok).
So far I only considered the case where the Lua dissector passes NULL, I
did not think about the above dissector table cases... Meh.
There are at least two ways to fix this:
- Add an explicit check to ignore the data parameter when invoked
through the TCP or SCTP dissectors. Disadvantage: any other user that
adds TLS to their dissector table with non-NULL data will have
exactly the same issue.
- Apply my initial approach: do not use the data parameter and instead
introduce a new function similar to ssl_starttls
(tls_set_appdata_dissector). That does not reuse existing dissector
APIs however and is indirect which is why I considered the data
parameter instead.
João's proposed patch to allow sub-dissectors to pass data via a
hashtable[1] would have a similar affect to the second option, except
that it would require additional code in the TLS dissector to actually
look up the data. Such approaches also do not work if you have nested
TLS traffic for some reason (maybe a VPN tunnel in TLS?).
I would like to understand your concern with encapsulation/nested
traffic and [1]. I think the point your are missing, correct me if
I'm wrong, is that encapsulation already does not work (for your
definition of "not working") with the void data pointer dissector
argument and my patch is orthogonal to that issue.
Using a hash table is an indirect method of passing data. A void
pointer function argument is a direct method of passing data. So why
would the former present problems with nested TLS traffic and the
latter not? Any limitations present in one would be present in the
other and vice-versa. What am I missing?
What I would like to do is implement a consumer pull model of data
passing between dissectors instead of a producer push model. And one
that would be Lua friendly too. This seems like a difficult and very
time consuming task. One that would require breaking all sorts of
compatibility, optimizations and assumptions I suspect.
[1]: https://code.wireshark.org/review/34049
For now I will consider the first option, but I am open to other
suggestions.
Kind regards,
Peter
|
