Wireshark-dev: Re: [Wireshark-dev] payload_proto_id in SCPT dissector
From: Peter Wu <peter@xxxxxxxxxxxxx>
Date: Thu, 15 Aug 2019 23:48:42 +0100
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?). [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 On Thu, Aug 15, 2019 at 12:09:33PM +0000, Anders Broman wrote: > Hi, > > It is the SCTP PPI which is defined by IANA > > Payload protocol identifier: M3UA (3) > > { &hf_data_chunk_payload_proto_id, { "Payload protocol > identifier", "sctp.data_payload_proto_id", > FT_UINT32, BASE_DEC, VALS(sctp_payload_proto_id_values), 0x0, > NULL, HFILL } }, > > > > You find it in sctpppids.h and > > static const value_string sctp_payload_proto_id_values[] = { > > { NOT_SPECIFIED_PROTOCOL_ID, "not specified" }, > > { IUA_PAYLOAD_PROTOCOL_ID, "IUA" }, > > { M2UA_PAYLOAD_PROTOCOL_ID, "M2UA" }, > > { M3UA_PAYLOAD_PROTOCOL_ID, "M3UA" }, > > { SUA_PAYLOAD_PROTOCOL_ID, "SUA" }, > > { M2PA_PAYLOAD_PROTOCOL_ID, "M2PA" }, > > { V5UA_PAYLOAD_PROTOCOL_ID, "V5UA" }, > > { H248_PAYLOAD_PROTOCOL_ID, "H.248/MEGACO" }, > > { BICC_PAYLOAD_PROTOCOL_ID, "BICC/Q.2150.3" }, > > { TALI_PAYLOAD_PROTOCOL_ID, "TALI" }, > > { DUA_PAYLOAD_PROTOCOL_ID, "DUA" }, > > { ASAP_PAYLOAD_PROTOCOL_ID, "ASAP" }, > > { ENRP_PAYLOAD_PROTOCOL_ID, "ENRP" }, > > { H323_PAYLOAD_PROTOCOL_ID, "H.323" }, > > { QIPC_PAYLOAD_PROTOCOL_ID, "Q.IPC/Q.2150.3" }, > > { SIMCO_PAYLOAD_PROTOCOL_ID, "SIMCO" }, > > { DDP_SEG_CHUNK_PROTOCOL_ID, "DDP Segment Chunk" }, > > { DDP_STREAM_SES_CTRL_PROTOCOL_ID, "DDP Stream Session > Control" }, > > { S1AP_PAYLOAD_PROTOCOL_ID, "S1 Application Protocol > (S1AP)" }, > > { RUA_PAYLOAD_PROTOCOL_ID, "RUA" }, > > { HNBAP_PAYLOAD_PROTOCOL_ID, "HNBAP" }, > > { FORCES_HP_PAYLOAD_PROTOCOL_ID, "ForCES-HP" }, > > { FORCES_MP_PAYLOAD_PROTOCOL_ID, "ForCES-MP" }, > > { FORCES_LP_PAYLOAD_PROTOCOL_ID, "ForCES-LP" }, > > { SBC_AP_PAYLOAD_PROTOCOL_ID, "SBc-AP" }, > > { NBAP_PAYLOAD_PROTOCOL_ID, "NBAP" }, > > /* Unassigned 26 */ > > { X2AP_PAYLOAD_PROTOCOL_ID, "X2AP" }, > > { IRCP_PAYLOAD_PROTOCOL_ID, "IRCP" }, > > { LCS_AP_PAYLOAD_PROTOCOL_ID, "LCS-AP" }, > > { MPICH2_PAYLOAD_PROTOCOL_ID, "MPICH2" }, > > { SABP_PAYLOAD_PROTOCOL_ID, "SABP" }, > > { FGP_PAYLOAD_PROTOCOL_ID, "Fractal Generator > Protocol" }, > > { PPP_PAYLOAD_PROTOCOL_ID, "Ping Pong Protocol" }, > > { CALCAPP_PAYLOAD_PROTOCOL_ID, "CalcApp Protocol" }, > > { SSP_PAYLOAD_PROTOCOL_ID, "Scripting Service > Protocol" }, > > { NPMP_CTRL_PAYLOAD_PROTOCOL_ID, "NetPerfMeter Control" }, > > { NPMP_DATA_PAYLOAD_PROTOCOL_ID, "NetPerfMeter Data" }, > > { ECHO_PAYLOAD_PROTOCOL_ID, "Echo" }, > > { DISCARD_PAYLOAD_PROTOCOL_ID, "Discard" }, > > { DAYTIME_PAYLOAD_PROTOCOL_ID, "Daytime" }, > > { CHARGEN_PAYLOAD_PROTOCOL_ID, "Character Generator" }, > > { PROTO_3GPP_RNA_PROTOCOL_ID, "3GPP RNA" }, > > { PROTO_3GPP_M2AP_PROTOCOL_ID, "3GPP M2AP" }, > > { PROTO_3GPP_M3AP_PROTOCOL_ID, "3GPP M3AP" }, > > { SSH_PAYLOAD_PROTOCOL_ID, "SSH" }, > > { DIAMETER_PROTOCOL_ID, "DIAMETER" }, > > { DIAMETER_DTLS_PROTOCOL_ID, "DIAMETER OVER DTLS" }, > > { R14P_BER_PROTOCOL_ID, "R14P" }, > > { WEBRTC_DCEP_PROTOCOL_ID, "WebRTC Control" }, > > { WEBRTC_STRING_PAYLOAD_PROTOCOL_ID, "WebRTC String" }, > > { WEBRTC_BINARY_PARTIAL_PAYLOAD_PROTOCOL_ID, "WebRTC Binary Partial > (Deprecated)" }, > > { WEBRTC_BINARY_PAYLOAD_PROTOCOL_ID, "WebRTC Binary" }, > > { WEBRTC_STRING_PARTIAL_PAYLOAD_PROTOCOL_ID, "WebRTC String Partial > (Deprecated)" }, > > { PROTO_3GPP_PUA_PAYLOAD_PROTOCOL_ID, "3GPP PUA" }, > > { WEBRTC_STRING_EMPTY_PAYLOAD_PROTOCOL_ID, "WebRTC String Empty" }, > > { WEBRTC_BINARY_EMPTY_PAYLOAD_PROTOCOL_ID, "WebRTC Binary Empty" }, > > { XWAP_PROTOCOL_ID, "XwAP" }, > > { XW_CONTROL_PLANE_PROTOCOL_ID, "Xw - Control Plane" }, > > { NGAP_PROTOCOL_ID, "NGAP" }, > > { XNAP_PROTOCOL_ID, "XnAP" }, > > { F1AP_PROTOCOL_ID, "F1 AP" }, > > { ELE2_PROTOCOL_ID, "ELE2 Lawful > Interception" }, > > > > { 0, NULL } }; > > > > > > Regards > > Anders > > From: Wireshark-dev <wireshark-dev-bounces@xxxxxxxxxxxxx> On Behalf Of Dario > Lombardo > Sent: den 15 augusti 2019 13:55 > To: Developer support list for Wireshark <wireshark-dev@xxxxxxxxxxxxx> > Subject: [Wireshark-dev] payload_proto_id in SCPT dissector > > > > Hi, > > I'm working on fixing a bug reported by oss-fuzz, and I got stuck. The payload > is TLS over SCPT. > > In the sctp dissector the variable payload_proto_id is read from the tlv in > dissect_data_chunk. What is it supposed to contain? Which proto_id? > > Following the stack trace, this value is passed to dissect_tls as-is. That > function handles the value as it was a char* containing the name of the > dissector to find (indeed it is passed to find_dissector). > > I can't figure out how this is supposed to work in the first place, or I am > missing some piece of the puzzle. > > Any help?
- Follow-Ups:
- Re: [Wireshark-dev] payload_proto_id in SCPT dissector
- From: João Valverde
- Re: [Wireshark-dev] payload_proto_id in SCPT dissector
- References:
- [Wireshark-dev] payload_proto_id in SCPT dissector
- From: Dario Lombardo
- Re: [Wireshark-dev] payload_proto_id in SCPT dissector
- From: Anders Broman
- [Wireshark-dev] payload_proto_id in SCPT dissector
- Prev by Date: Re: [Wireshark-dev] Wireshark Windows libraries
- Next by Date: Re: [Wireshark-dev] payload_proto_id in SCPT dissector
- Previous by thread: Re: [Wireshark-dev] payload_proto_id in SCPT dissector
- Next by thread: Re: [Wireshark-dev] payload_proto_id in SCPT dissector
- Index(es):