Fix STUN-over-SSL/TLS/DTLS.

Different dissectors are required for protocols running atop SSL/TLS and
protocols running atop DTLS - SSL/TLS provides a byte-stream service, so
there's no guarantee that there's a correspondence between SSL/TLS
application data record boundaries and packet boundaries, but DTLS
provides a datagram service, with packet boundaries corresponding to
application data record boundaries.

This is similar to the difference between dissectors for protocols
running atop TCP and protocols running atop protocols such as UDP.

So have two separate tables mapping Application-Layer Protocol
Negotiation (ALPN) Protocol IDs to dissector names - one for SSL/TLS and
one for DTLS.

There are both "over a byte-stream protocol" and "over a packet-oriented
protocol" dissectors for STUN and TURN ChannelData packets.  Register
the "over a byte-stream protocol" ones by name, and use the appropriate
ones in the appropriate tables.  (There is not one named "stun", so the
STUN dissector wouldn't have been called at all.)

Change-Id: I054e169f6ae3291abdc7eb58918ef65a17c90a63
Reviewed-on: https://code.wireshark.org/review/27822
Reviewed-by: Guy Harris <guy@alum.mit.edu>

diff --git a/epan/dissectors/packet-ssl-utils.c b/epan/dissectors/packet-ssl-utils.c
index 5bdb8c7e80a2f1a8b1b3be7499460afa3df0c13d..a693a709a2e6e8c80493a1c66e70d8220072c14c 100644 (file)
--- a/epan/dissectors/packet-ssl-utils.c
+++ b/epan/dissectors/packet-ssl-utils.c
@@ -1401,6 +1401,9 @@ static const bytes_string ct_logids[] = {
     { NULL, 0, NULL }
 };
 
+/*
+ * http://www.iana.org/assignments/tls-extensiontype-values/tls-extensiontype-values.xhtml#alpn-protocol-ids
+ */
 /* string_string is inappropriate as it compares strings while
  * "byte strings MUST NOT be truncated" (RFC 7301) */
 typedef struct ssl_alpn_protocol {
@@ -1408,19 +1411,32 @@ typedef struct ssl_alpn_protocol {
     gboolean         match_exact;
     const char      *dissector_name;
 } ssl_alpn_protocol_t;
-/* http://www.iana.org/assignments/tls-extensiontype-values/tls-extensiontype-values.xhtml#alpn-protocol-ids */
+
+/*
+ * For SSL/TLS; the dissectors should handle running atop a byte-stream
+ * protocol such as TCP.
+ */
 static const ssl_alpn_protocol_t ssl_alpn_protocols[] = {
     { "http/1.1",           TRUE,   "http" },
     /* SPDY moves so fast, just 1, 2 and 3 are registered with IANA but there
      * already exists 3.1 as of this writing... match the prefix. */
     { "spdy/",              FALSE,  "spdy" },
-    { "stun.turn",          TRUE,   "turnchannel" },
-    { "stun.nat-discovery", TRUE,   "stun" },
+    { "stun.turn",          TRUE,   "turnchannel-tcp" }, /* RFC 7443 */
+    { "stun.nat-discovery", TRUE,   "stun-tcp" },        /* RFC 7443 */
     /* draft-ietf-httpbis-http2-16 */
     { "h2-",                FALSE,  "http2" }, /* draft versions */
     { "h2",                 TRUE,   "http2" }, /* final version */
 };
 
+/*
+ * For DTLS; the dissectors should handle running atop a datagram
+ * protocol such as UDP.
+ */
+static const ssl_alpn_protocol_t dtls_alpn_protocols[] = {
+    { "stun.turn",          TRUE,   "turnchannel" }, /* RFC 7443 */
+    { "stun.nat-discovery", TRUE,   "stun-udp" },    /* RFC 7443 */
+};
+
 const value_string quic_transport_parameter_id[] = {
     { SSL_HND_QUIC_TP_INITIAL_MAX_STREAM_DATA, "initial_max_stream_data" },
     { SSL_HND_QUIC_TP_INITIAL_MAX_DATA, "initial_max_data" },
@@ -5863,7 +5879,8 @@ static gint
 ssl_dissect_hnd_hello_ext_alpn(ssl_common_dissect_t *hf, tvbuff_t *tvb,
                                packet_info *pinfo, proto_tree *tree,
                                guint32 offset, guint32 offset_end,
-                               guint8 hnd_type, SslSession *session)
+                               guint8 hnd_type, SslSession *session,
+                               gboolean is_dtls)
 {
 
     /* https://tools.ietf.org/html/rfc7301#section-3.1
@@ -5877,6 +5894,8 @@ ssl_dissect_hnd_hello_ext_alpn(ssl_common_dissect_t *hf, tvbuff_t *tvb,
     guint32     next_offset, alpn_length, name_length;
     guint8     *proto_name = NULL;
     guint32     proto_name_length = 0;
+    const ssl_alpn_protocol_t *alpn_protocols;
+    size_t      n_alpn_protocols;
 
     /* ProtocolName protocol_name_list<2..2^16-1> */
     if (!ssl_add_vector(hf, tvb, pinfo, tree, offset, offset_end, &alpn_length,
@@ -5914,9 +5933,11 @@ ssl_dissect_hnd_hello_ext_alpn(ssl_common_dissect_t *hf, tvbuff_t *tvb,
     /* If ALPN is given in ServerHello, then ProtocolNameList MUST contain
      * exactly one "ProtocolName". */
     if (proto_name) {
+        alpn_protocols = is_dtls ? dtls_alpn_protocols : ssl_alpn_protocols;
+        n_alpn_protocols = is_dtls ? G_N_ELEMENTS(dtls_alpn_protocols) : G_N_ELEMENTS(ssl_alpn_protocols);
         /* '\0'-terminated string for prefix/full string comparison purposes. */
-        for (size_t i = 0; i < G_N_ELEMENTS(ssl_alpn_protocols); i++) {
-            const ssl_alpn_protocol_t *alpn_proto = &ssl_alpn_protocols[i];
+        for (size_t i = 0; i < n_alpn_protocols; i++) {
+            const ssl_alpn_protocol_t *alpn_proto = &alpn_protocols[i];
 
             if ((alpn_proto->match_exact &&
                         proto_name_length == strlen(alpn_proto->proto_name) &&
@@ -8148,7 +8169,7 @@ ssl_dissect_hnd_extension(ssl_common_dissect_t *hf, tvbuff_t *tvb, proto_tree *t
             offset++;
             break;
         case SSL_HND_HELLO_EXT_ALPN:
-            offset = ssl_dissect_hnd_hello_ext_alpn(hf, tvb, pinfo, ext_tree, offset, next_offset, hnd_type, session);
+            offset = ssl_dissect_hnd_hello_ext_alpn(hf, tvb, pinfo, ext_tree, offset, next_offset, hnd_type, session, is_dtls);
             break;
         case SSL_HND_HELLO_EXT_STATUS_REQUEST_V2:
             if (hnd_type == SSL_HND_CLIENT_HELLO)

diff --git a/epan/dissectors/packet-stun.c b/epan/dissectors/packet-stun.c
index 9ee5a8ea7bfe5c38effc50446d0ac41f13fed5c8..46e5a382343ec0eac62f4ad6f833b7f2da72c7c1 100644 (file)
--- a/epan/dissectors/packet-stun.c
+++ b/epan/dissectors/packet-stun.c
@@ -1717,6 +1717,7 @@ proto_register_stun(void)
     /* heuristic subdissectors (used for the DATA field) */
     heur_subdissector_list = register_heur_dissector_list("stun", proto_stun);
 
+    register_dissector("stun-tcp", dissect_stun_tcp, proto_stun);
     register_dissector("stun-udp", dissect_stun_udp, proto_stun);
     register_dissector("stun-heur", dissect_stun_heur, proto_stun);
 }
@@ -1724,8 +1725,8 @@ proto_register_stun(void)
 void
 proto_reg_handoff_stun(void)
 {
-    stun_tcp_handle = create_dissector_handle(dissect_stun_tcp, proto_stun);
-    stun_udp_handle = create_dissector_handle(dissect_stun_udp, proto_stun);
+    stun_tcp_handle = find_dissector("stun-tcp");
+    stun_udp_handle = find_dissector("stun-udp");
 
     dissector_add_uint_with_preference("tcp.port", TCP_PORT_STUN, stun_tcp_handle);
     dissector_add_uint_with_preference("udp.port", UDP_PORT_STUN, stun_udp_handle);

diff --git a/epan/dissectors/packet-turnchannel.c b/epan/dissectors/packet-turnchannel.c
index 552f24d9a9e202fa73bb1a0e14ac8bb98d2b4ca1..339db978f33175f7f2979c5ef3322ae27914a1f2 100644 (file)
--- a/epan/dissectors/packet-turnchannel.c
+++ b/epan/dissectors/packet-turnchannel.c
@@ -49,6 +49,7 @@ static int hf_turnchannel_len = -1;
 /* Initialize the subtree pointers */
 static gint ett_turnchannel = -1;
 
+static dissector_handle_t turnchannel_tcp_handle;
 static dissector_handle_t turnchannel_udp_handle;
 
 static int
@@ -181,6 +182,7 @@ proto_register_turnchannel(void)
        proto_turnchannel = proto_register_protocol("TURN Channel",
            "TURNCHANNEL", "turnchannel");
 
+       turnchannel_tcp_handle = register_dissector("turnchannel-tcp", dissect_turnchannel_tcp, proto_turnchannel);
        turnchannel_udp_handle = register_dissector("turnchannel", dissect_turnchannel_message, proto_turnchannel);
 
 /* subdissectors */
@@ -196,10 +198,6 @@ proto_register_turnchannel(void)
 void
 proto_reg_handoff_turnchannel(void)
 {
-       dissector_handle_t turnchannel_tcp_handle;
-
-       turnchannel_tcp_handle = create_dissector_handle(dissect_turnchannel_tcp, proto_turnchannel);
-
        /* Register for "Decode As" in case STUN negotiation isn't captured */
        dissector_add_for_decode_as_with_preference("tcp.port", turnchannel_tcp_handle);
        dissector_add_for_decode_as_with_preference("udp.port", turnchannel_udp_handle);