Fix STARTTLS handling in various dissectors

This patch lets a dissector hand over control to the SSL dissector which
simplifies dissector code ("TCP | App | SSL | App" becomes
"TCP | SSL | App").

After this patch, all of the affected dissectors will now be dissected
as SSL with its Application Data being treated as the protocol before
STARTTLS. This was previously not the case because the port was not
registered for dissection via ssl_dissector_add.

The desegmentation issue within the MySQL dissector is now also gone.
Convert some tvb_length[_remaining] users in pop and smtp as well.

Tested against mysql-ssl.pcapng and mysql-ssl-larger.pcapng(*1),
Tested against pop-ssl.pcapng (note: only first stream is decrypted,
either the key after negotiation is wrong or there is a bug),
Tested against smtp-ssl.pcapng and smtp2525-ssl.pcapng (with Decode As)
and smtp-ssl.pcapng with filter "tcp.len>0",
Tested against xmpp-ssl.pcapng,
http://wiki.wireshark.org/SampleCaptures#SSL_with_decryption_keys

 *1) mysql-ssl-larger has MySQL dissector errors for the fragmented
     SSL packet, but reassembly seems to work. Needs further
     investigation.

Bug: 9515
Change-Id: I408ef8ff30d9edc8954dab9b3615900666dfa932
Reviewed-on: https://code.wireshark.org/review/6981
Reviewed-by: Michael Mann <mmann78@netscape.net>

6 files changed, 28 insertions, 113 deletions

diff --git a/epan/dissectors/packet-mysql.c b/epan/dissectors/packet-mysql.c
index 959ace714c..7ce25ccf36 100644
--- a/epan/dissectors/packet-mysql.c
+++ b/epan/dissectors/packet-mysql.c
@@ -41,6 +41,7 @@
 #include <epan/prefs.h>
 #include <epan/expert.h>
 #include "packet-tcp.h"
+#include "packet-ssl-utils.h"
 
 void proto_register_mysql(void);
 void proto_reg_handoff_mysql(void);
@@ -615,6 +616,7 @@ static int hf_mysql_auth_switch_request_name = -1;
 static int hf_mysql_auth_switch_request_data = -1;
 static int hf_mysql_auth_switch_response_data = -1;
 
+static dissector_handle_t mysql_handle;
 static dissector_handle_t ssl_handle;
 
 static expert_field ei_mysql_eof = EI_INIT;
@@ -1001,6 +1003,7 @@ mysql_dissect_login(tvbuff_t *tvb, packet_info *pinfo, int offset,
 	{
 		col_set_str(pinfo->cinfo, COL_INFO, "Response: SSL Handshake");
 		conn_data->frame_start_ssl = pinfo->fd->num;
+		ssl_starttls_ack(ssl_handle, pinfo, mysql_handle);
 	}
 	if (conn_data->clnt_caps & MYSQL_CAPS_CU) /* 4.1 protocol */
 	{
@@ -2291,26 +2294,6 @@ dissect_mysql_pdu(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree, void* dat
 static int
 dissect_mysql(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree, void* data)
 {
-	gboolean is_ssl;
-	conversation_t  *conversation;
-	mysql_conn_data_t  *conn_data;
-
-	is_ssl = proto_is_frame_protocol(pinfo->layers, "ssl");
-
-	/* Check there is already a conversation */
-	conversation = find_or_create_conversation(pinfo);
-	conn_data = (mysql_conn_data_t *)conversation_get_proto_data(conversation, proto_mysql);
-
-	if(conn_data){
-		/* Check if flag (frame_start_ssl) is > to actual packet number and no already call SSL dissector */
-		if(conn_data->frame_start_ssl && conn_data->frame_start_ssl < pinfo->fd->num && !(is_ssl)) {
-			/* Call SSL dissector */
-			call_dissector(ssl_handle, tvb, pinfo, tree);
-			return tvb_reported_length(tvb);
-		}
-
-	}
-
 	tcp_dissect_pdus(tvb, pinfo, tree, mysql_desegment, 3,
 			 get_mysql_pdu_len, dissect_mysql_pdu, data);
 
@@ -3216,7 +3199,6 @@ void proto_register_mysql(void)
 /* dissector registration */
 void proto_reg_handoff_mysql(void)
 {
-	dissector_handle_t mysql_handle;
 	ssl_handle = find_dissector("ssl");
 	mysql_handle = new_create_dissector_handle(dissect_mysql, proto_mysql);
 	dissector_add_uint("tcp.port", TCP_PORT_MySQL, mysql_handle);
diff --git a/epan/dissectors/packet-pop.c b/epan/dissectors/packet-pop.c
index a2d9c9f3e2..0da2760141 100644
--- a/epan/dissectors/packet-pop.c
+++ b/epan/dissectors/packet-pop.c
@@ -34,6 +34,7 @@
 #include <epan/prefs.h>
 #include <epan/reassemble.h>
 #include "packet-ssl.h"
+#include "packet-ssl-utils.h"
 
 void proto_register_pop(void);
 void proto_reg_handoff_pop(void);
@@ -67,9 +68,10 @@ static gint ett_pop_reqresp = -1;
 static gint ett_pop_data_fragment = -1;
 static gint ett_pop_data_fragments = -1;
 
+static dissector_handle_t pop_handle;
 static dissector_handle_t data_handle;
-static dissector_handle_t imf_handle = NULL;
-static dissector_handle_t ssl_handle = NULL;
+static dissector_handle_t imf_handle;
+static dissector_handle_t ssl_handle;
 
 #define TCP_PORT_POP            110
 #define TCP_PORT_SSL_POP        995
@@ -112,7 +114,6 @@ struct pop_data_val {
   guint32 msg_read_len;  /* Length of RETR message read so far */
   guint32 msg_tot_len;   /* Total length of RETR message */
   gboolean stls_request;  /* Received STLS request */
-  guint32  last_nontls_frame; /* last non-TLS frame; 0 if not known or no TLS */
 };
 
 
@@ -155,26 +156,6 @@ dissect_pop(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree)
      conversation_add_proto_data(conversation, proto_pop, data_val);
   }
 
-  /* Are we doing TLS? */
-  if (data_val->last_nontls_frame != 0 && pinfo->fd->num > data_val->last_nontls_frame) {
-    guint16 save_can_desegment;
-    guint32 save_last_nontls_frame;
-
-    /* This is TLS, not raw POP/IMF. TLS can desegment */
-    save_can_desegment = pinfo->can_desegment;
-    pinfo->can_desegment = pinfo->saved_can_desegment;
-
-    /* Make sure the SSL dissector will not be called again after decryption */
-    save_last_nontls_frame = data_val->last_nontls_frame;
-    data_val->last_nontls_frame = 0;
-
-    call_dissector(ssl_handle, tvb, pinfo, tree);
-
-    pinfo->can_desegment = save_can_desegment;
-    data_val->last_nontls_frame = save_last_nontls_frame;
-    return;
-  }
-
   /*
    * Find the end of the first line.
    *
@@ -200,7 +181,7 @@ dissect_pop(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree)
    * Otherwise, just call it a continuation.
    */
   if (is_continuation) {
-    length_remaining = tvb_length_remaining(tvb, offset);
+    length_remaining = tvb_reported_length_remaining(tvb, offset);
     col_add_fstr(pinfo->cinfo, COL_INFO, "S: DATA fragment, %d byte%s",
                    length_remaining, plurality (length_remaining, "", "s"));
   }
@@ -217,7 +198,7 @@ dissect_pop(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree)
 
       if (!frame_data_p) {
 
-        data_val->msg_read_len += tvb_length(tvb);
+        data_val->msg_read_len += tvb_reported_length(tvb);
 
         frame_data_p = wmem_new(wmem_file_scope(), struct pop_proto_data);
 
@@ -231,7 +212,7 @@ dissect_pop(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree)
                                        pinfo,
                                        frame_data_p->conversation_id,
                                        NULL,
-                                       tvb_length(tvb),
+                                       tvb_reported_length(tvb),
                                        frame_data_p->more_frags);
 
       next_tvb = process_reassembled_data(tvb, offset, pinfo,
@@ -317,7 +298,7 @@ dissect_pop(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree)
         if (data_val->stls_request) {
           if (g_ascii_strncasecmp(line, "+OK ", 4) == 0) {
               /* This is the last non-TLS frame. */
-              data_val->last_nontls_frame = pinfo->fd->num;
+              ssl_starttls_ack(ssl_handle, pinfo, pop_handle);
           }
           data_val->stls_request = FALSE;
         }
@@ -478,8 +459,6 @@ proto_register_pop(void)
 void
 proto_reg_handoff_pop(void)
 {
-  dissector_handle_t pop_handle;
-
   pop_handle = find_dissector("pop");
   dissector_add_uint("tcp.port", TCP_PORT_POP, pop_handle);
   ssl_dissector_add(TCP_PORT_SSL_POP, "pop", TRUE);
diff --git a/epan/dissectors/packet-smtp.c b/epan/dissectors/packet-smtp.c
index 5cd734d321..3ebbd7f8a6 100644
--- a/epan/dissectors/packet-smtp.c
+++ b/epan/dissectors/packet-smtp.c
@@ -36,6 +36,7 @@
 #include <epan/reassemble.h>
 #include <wsutil/base64.h>
 #include "packet-ssl.h"
+#include "packet-ssl-utils.h"
 
 /* RFC 2821 */
 #define TCP_PORT_SMTP      25
@@ -109,6 +110,7 @@ static const fragment_items smtp_data_frag_items = {
   "DATA fragments"
 };
 
+static  dissector_handle_t smtp_handle;
 static  dissector_handle_t ssl_handle;
 static  dissector_handle_t imf_handle;
 static  dissector_handle_t ntlmssp_handle;
@@ -167,7 +169,6 @@ struct smtp_session_state {
   guint32  msg_read_len;        /* Length of BDAT message read so far */
   guint32  msg_tot_len;         /* Total length of BDAT message */
   gboolean msg_last;            /* Is this the last BDAT chunk */
-  guint32  last_nontls_frame;   /* last non-TLS frame; 0 if not known or no TLS */
   guint32  username_cmd_frame;  /* AUTH command contains username */
   guint32  user_pass_cmd_frame; /* AUTH command contains username and password */
   guint32  user_pass_frame;     /* Frame contains username and password */
@@ -412,29 +413,6 @@ dissect_smtp(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree)
     conversation_add_proto_data(conversation, proto_smtp, session_state);
   }
 
-  /* Are we doing TLS?
-   * FIXME In my understanding of RFC 2487 client and server can send SMTP cmds
-   * after a rejected TLS negotiation
-   */
-  if (session_state->last_nontls_frame != 0 && pinfo->fd->num > session_state->last_nontls_frame) {
-    guint16 save_can_desegment;
-    guint32 save_last_nontls_frame;
-
-    /* This is TLS, not raw SMTP. TLS can desegment */
-    save_can_desegment   = pinfo->can_desegment;
-    pinfo->can_desegment = pinfo->saved_can_desegment;
-
-    /* Make sure the SSL dissector will not be called again after decryption */
-    save_last_nontls_frame = session_state->last_nontls_frame;
-    session_state->last_nontls_frame = 0;
-
-    call_dissector(ssl_handle, tvb, pinfo, tree);
-
-    pinfo->can_desegment = save_can_desegment;
-    session_state->last_nontls_frame = save_last_nontls_frame;
-    return;
-  }
-
   /* Is this a request or a response? */
   request = pinfo->destport == pinfo->match_uint;
 
@@ -486,7 +464,7 @@ dissect_smtp(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree)
           pinfo->desegment_len = DESEGMENT_ONE_MORE_SEGMENT;
           return;
         } else {
-          linelen = tvb_length_remaining(tvb, loffset);
+          linelen = tvb_reported_length_remaining(tvb, loffset);
           next_offset = loffset + linelen;
         }
       }
@@ -507,7 +485,7 @@ dissect_smtp(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree)
             tvb_strneql(tvb, loffset, "\r\n.\r\n", 5) == 0)
           eom_seen = TRUE;
 
-        length_remaining = tvb_length_remaining(tvb, loffset);
+        length_remaining = tvb_captured_length_remaining(tvb, loffset);
         if (length_remaining == tvb_reported_length_remaining(tvb, loffset) &&
             tvb_strneql(tvb, loffset + length_remaining - 2, "\r\n", 2) == 0)
           session_state->crlf_seen = TRUE;
@@ -543,7 +521,7 @@ dissect_smtp(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree)
                * We are handling a BDAT message.
                * Check if we have reached end of the data chunk.
                */
-              session_state->msg_read_len += tvb_length_remaining(tvb, loffset);
+              session_state->msg_read_len += tvb_reported_length_remaining(tvb, loffset);
 
               if (session_state->msg_read_len == session_state->msg_tot_len) {
                 /*
@@ -762,7 +740,7 @@ dissect_smtp(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree)
 
     case SMTP_PDU_MESSAGE:
       /* Column Info */
-      length_remaining = tvb_length_remaining(tvb, offset);
+      length_remaining = tvb_reported_length_remaining(tvb, offset);
       col_set_str(pinfo->cinfo, COL_INFO, smtp_data_desegment ? "C: DATA fragment" : "C: Message Body");
       col_append_fstr(pinfo->cinfo, COL_INFO, ", %d byte%s", length_remaining,
                         plurality (length_remaining, "", "s"));
@@ -770,7 +748,8 @@ dissect_smtp(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree)
       if (smtp_data_desegment) {
         frag_msg = fragment_add_seq_next(&smtp_data_reassembly_table, tvb, 0,
                                          pinfo, spd_frame_data->conversation_id, NULL,
-                                         tvb_length(tvb), spd_frame_data->more_frags);
+                                         tvb_reported_length(tvb),
+                                         spd_frame_data->more_frags);
       } else {
         /*
          * Message body.
@@ -1061,7 +1040,7 @@ dissect_smtp(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree)
             if (session_state->smtp_state == SMTP_STATE_AWAITING_STARTTLS_RESPONSE) {
               if (code == 220) {
                 /* This is the last non-TLS frame. */
-                session_state->last_nontls_frame = pinfo->fd->num;
+                ssl_starttls_ack(ssl_handle, pinfo, smtp_handle);
               }
               session_state->smtp_state =  SMTP_STATE_READING_CMDS;
             }
@@ -1309,8 +1288,6 @@ proto_register_smtp(void)
 void
 proto_reg_handoff_smtp(void)
 {
-  dissector_handle_t smtp_handle;
-
   smtp_handle = find_dissector("smtp");
   dissector_add_uint("tcp.port", TCP_PORT_SMTP, smtp_handle);
   ssl_dissector_add(TCP_PORT_SSL_SMTP, "smtp", TRUE);
diff --git a/epan/dissectors/packet-xmpp-core.c b/epan/dissectors/packet-xmpp-core.c
index 5466cf2884..99d24c560f 100644
--- a/epan/dissectors/packet-xmpp-core.c
+++ b/epan/dissectors/packet-xmpp-core.c
@@ -33,6 +33,7 @@
 #include "packet-xmpp-other.h"
 #include "packet-xmpp-gtalk.h"
 #include "packet-xmpp-conference.h"
+#include "packet-ssl-utils.h"
 
 tvbparse_wanted_t *want_ignore;
 tvbparse_wanted_t *want_stream_end_tag;
@@ -712,6 +713,7 @@ xmpp_proceed(proto_tree *tree, tvbuff_t *tvb, packet_info *pinfo,
 {
     proto_item *proceed_item;
     proto_tree *proceed_tree;
+    guint32 ssl_proceed;
 
     xmpp_attr_info attrs_info [] = {
         {"xmlns", &hf_xmpp_xmlns, TRUE, TRUE, NULL, NULL},
@@ -726,11 +728,11 @@ xmpp_proceed(proto_tree *tree, tvbuff_t *tvb, packet_info *pinfo,
         expert_add_info(pinfo, proceed_item, &ei_xmpp_starttls_missing);
     }
 
-    if (xmpp_info->ssl_proceed && xmpp_info->ssl_proceed != pinfo->fd->num) {
-        expert_add_info_format(pinfo, proceed_item, &ei_xmpp_proceed_already_in_frame, "Already saw PROCEED in frame %u", xmpp_info->ssl_proceed);
-    }
-    else {
-        xmpp_info->ssl_proceed = pinfo->fd->num;
+    ssl_proceed =
+        ssl_starttls_ack(find_dissector("ssl"), pinfo, find_dissector("xmpp"));
+    if (ssl_proceed > 0 && ssl_proceed != pinfo->fd->num) {
+        expert_add_info_format(pinfo, proceed_item, &ei_xmpp_proceed_already_in_frame,
+                               "Already saw PROCEED in frame %u", ssl_proceed);
     }
 
     xmpp_display_attrs(proceed_tree, packet, pinfo, tvb, attrs_info, array_length(attrs_info));
diff --git a/epan/dissectors/packet-xmpp-utils.h b/epan/dissectors/packet-xmpp-utils.h
index 24907c0bee..20de98b1c1 100644
--- a/epan/dissectors/packet-xmpp-utils.h
+++ b/epan/dissectors/packet-xmpp-utils.h
@@ -118,7 +118,6 @@ typedef struct _xmpp_conv_info_t {
     wmem_tree_t *ibb_sessions;
     wmem_tree_t *gtalk_sessions;
     guint32      ssl_start;
-    guint32      ssl_proceed;
 } xmpp_conv_info_t;
 
 /** Struct conatins frame numbers (request frame(IQ set/get) and
diff --git a/epan/dissectors/packet-xmpp.c b/epan/dissectors/packet-xmpp.c
index f4a2122e10..b8b4d25711 100644
--- a/epan/dissectors/packet-xmpp.c
+++ b/epan/dissectors/packet-xmpp.c
@@ -362,7 +362,6 @@ expert_field ei_xmpp_required_attribute = EI_INIT;
 
 static dissector_handle_t xmpp_handle;
 
-static dissector_handle_t ssl_handle;
 static dissector_handle_t xml_handle;
 
 static void
@@ -392,7 +391,7 @@ dissect_xmpp(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree) {
     conversation = find_or_create_conversation(pinfo);
     xmpp_info = (xmpp_conv_info_t *)conversation_get_proto_data(conversation, proto_xmpp);
 
-    if ((!xmpp_info || !xmpp_info->ssl_proceed) && xmpp_desegment)
+    if (!xmpp_info && xmpp_desegment)
     {
         indx = tvb_reported_length(tvb) - 1;
         if (indx >= 0)
@@ -417,27 +416,6 @@ dissect_xmpp(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree) {
 
     col_clear(pinfo->cinfo, COL_INFO);
 
-    if (xmpp_info && xmpp_info->ssl_proceed &&
-            xmpp_info->ssl_proceed < pinfo->fd->num)
-    {
-        guint16 save_can_desegment;
-        guint32 save_ssl_proceed;
-
-        /* Make sure SSL/TLS can desegment */
-        save_can_desegment = pinfo->can_desegment;
-        pinfo->can_desegment = pinfo->saved_can_desegment;
-
-        /* Make sure the SSL dissector will not be called again after decryption */
-        save_ssl_proceed = xmpp_info->ssl_proceed;
-        xmpp_info->ssl_proceed = 0;
-
-        call_dissector(ssl_handle, tvb, pinfo, tree);
-
-        pinfo->can_desegment = save_can_desegment;
-        xmpp_info->ssl_proceed = save_ssl_proceed;
-        return;
-    }
-
     /*if tree == NULL then xmpp_item and xmpp_tree will also NULL*/
     xmpp_item = proto_tree_add_item(tree, proto_xmpp, tvb, 0, -1, ENC_NA);
     xmpp_tree = proto_item_add_subtree(xmpp_item, ett_xmpp);
@@ -477,7 +455,6 @@ dissect_xmpp(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree) {
         xmpp_info->ibb_sessions    = wmem_tree_new(wmem_file_scope());
         xmpp_info->gtalk_sessions  = wmem_tree_new(wmem_file_scope());
         xmpp_info->ssl_start   = 0;
-        xmpp_info->ssl_proceed = 0;
         conversation_add_proto_data(conversation, proto_xmpp, (void *) xmpp_info);
     }
 
@@ -1443,7 +1420,6 @@ proto_register_xmpp(void) {
 
 void
 proto_reg_handoff_xmpp(void) {
-    ssl_handle = find_dissector("ssl");
     xml_handle  = find_dissector("xml");
 
     dissector_add_uint("tcp.port", XMPP_PORT, xmpp_handle);