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
___________________________________________________________________________ Sent via: Wireshark-dev mailing list <wireshark-dev@wireshark.org> Archives: https://www.wireshark.org/lists/wireshark-dev Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-dev mailto:wireshark-dev-requ...@wireshark.org?subject=unsubscribe