Ethereal-dev: Re: [Ethereal-dev] MEGACO plugin seriously flawed?

Note: This archive is from the project's previous web site, ethereal.com. This list is no longer active.

From: Ruud Linders <moztest@xxxxxxxxxxxxxxxxxxxx>
Date: Sat, 25 Sep 2004 12:53:49 +0200

Ariel et al,

I didn't know about SCTP NOT requiring a TPKT header but TCP indeed
DOES require a TPKT encapsulation.

Several months ago I made a quick-and-dirty patch to allow for TPKT
encapsulation and de-segmentation which is working fine for me.
However, it needs some cleaning up
- adding preference to be able to turn off/on de-segmentation,
  the global var is already there.
- correct check in the dissect_megaco_tpkt routine for the MEGACO
  string (for whatever reason I only checked for 'M' 'E' (probably
  lazines at the time!)
- proper skip/check for whitespace in same check
- registration/hook for sctp support

Attached the patch against 0.10.6, feel free to use it.

Regards,
	Ruud

Ariel Burbaickij wrote:
Hello dear mailing list participants,

following code exceprt from MEGACO plugin:
  /*
     * Check to see whether we're really dealing with MEGACO by looking
     * for the MEGACO string.  This needs to be improved when supporting
     * binary encodings.
     */
    if(!tvb_get_nstringz0(tvb,0,6,word)) return;
    if (strncasecmp(word, "MEGACO", 6) != 0) return;

This is incorrect as was found out today. IIUC what is going one this code
omits the possbility of TPKT header between TCP and MEGACO which
is reuqired as per RFC 3015 Addendum D. TPKT (RFC 1006) caters for message
segmentation. So either documentation needs to be updated stating
that MEGACO works only with SCTP where there is no such thing
as message segmentation or code needs to be corrected. Would
be glad to hear your opinion.

With Best Regards
Ariel Burbaickij

_______________________________________________
Ethereal-dev mailing list
Ethereal-dev@xxxxxxxxxxxx
http://www.ethereal.com/mailman/listinfo/ethereal-dev
--- packet-megaco.c.ORIG	Fri Aug 13 00:41:51 2004
+++ packet-megaco.c	Sat Sep 25 12:38:24 2004
@@ -11,6 +11,10 @@
 * Christoph Wiest,		2003/06/28
 * Modified 2003 by		Christoph Wiest
 *						<ch.wiest@xxxxxxxxxxxxx>
+*
+* Ruud Linders  2004-05-30	ruud.at.lucent.com
+* added TPKT support as per RFC3015
+*
 * Ethereal - Network traffic analyzer
 * By Gerald Combs <gerald@xxxxxxxxxxxx>
 * Copyright 1999 Gerald Combs
@@ -147,6 +151,9 @@
 #endif
 static gboolean global_megaco_raw_text = TRUE;
 static gboolean global_megaco_dissect_tree = TRUE;
+static gboolean megaco_desegment = TRUE;
+static dissector_handle_t megaco_text_handle_tpkt;
+static dissector_handle_t megaco_text_handle;
 
 /*
 * Variables to allow for proper deletion of dissector registration when
@@ -211,6 +218,85 @@
 static dissector_handle_t h245_handle;
 static proto_tree *top_tree;
 
+static int
+dissect_megaco_tpkt(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree)
+{
+        int lv_tpkt_len;
+        int skipws;
+        /*
+         * Check whether this looks like a TPKT-encapsulated
+         * h.248 packet.
+         *
+         * The minimum length of a MEGACO message is 6:
+         * 1 byte for the protocol discriminator,
+         * 1 for the call_reference length,
+         * and one for the message type.
+         */
+        lv_tpkt_len = is_tpkt(tvb, 6);
+        if (lv_tpkt_len == -1) {
+                /*
+                 * It's not a TPKT packet; reject it.
+                 */
+                return FALSE;
+        }
+
+        /*
+         * If this segment is *exactly* the length of a TPKT header,
+         * we assume that, as it looks like a TPKT header, it
+         * is one, and that the code put a TPKT header in one
+         * segment and the rest of the PDU in another.
+         */
+        if (tvb_length(tvb) == 4) {
+                /*
+                 * It is - call the "dissect TPKT over a TCP stream"
+                 * routine.
+                 */
+                dissect_tpkt_encap(tvb, pinfo, tree, megaco_desegment,
+                                   megaco_text_handle);
+                return TRUE;
+        }
+
+        /*
+         * Well, we have more data than just the TPKT header;
+         * check whether it looks like the beginning of a
+         * h.248 message.
+         *
+         * The minimum length of a h.248 message is 3, as per the
+         * above.
+         *
+         * Check that we have that many bytes past the TPKT header in
+         * the tvbuff; we already know that the TPKT header says we
+         * have that many bytes (as we passed 6 as the "min_len" argument
+         * to "is_tpkt()").
+         */
+        if (!tvb_bytes_exist(tvb, 4, 6))
+                return FALSE;
+
+        /* check for MEGACO??  Check the protocol discriminator */
+        if (tvb_get_guint8(tvb, 4) == 0x0a) {
+                skipws= 1;
+        } else {
+                skipws= 0;
+        }
+        if (tvb_get_guint8(tvb, 4+skipws) != 'M') {
+                /* Doesn't look like h.248 inside TPKT */
+                return FALSE;
+        }
+        if (tvb_get_guint8(tvb, 5+skipws) != 'E') {
+                /* Doesn't look like h.248 inside TPKT */
+                return FALSE;
+        }
+
+        /*
+         * OK, it looks like h.248-over-TPKT.
+         * Call the "dissect TPKT over a TCP stream" routine.
+         */
+        dissect_tpkt_encap(tvb, pinfo, tree, megaco_desegment,
+            megaco_text_handle);
+
+        return TRUE;
+}
+
 /*
 * dissect_megaco_text - The dissector for the MEGACO Protocol, using
 * text encoding.
@@ -2608,19 +2694,20 @@
 proto_reg_handoff_megaco(void)
 {
 	static int megaco_prefs_initialized = FALSE;
-	static dissector_handle_t megaco_text_handle;
 	
 	sdp_handle = find_dissector("sdp");
 	h245_handle = find_dissector("h245dg");
 	
 	if (!megaco_prefs_initialized) {
-		megaco_text_handle = create_dissector_handle(dissect_megaco_text,
-			proto_megaco);
+	  megaco_text_handle_tpkt = create_dissector_handle(dissect_megaco_tpkt,
+                                                     proto_megaco);
+	  megaco_text_handle = create_dissector_handle(dissect_megaco_text,
+                                                     proto_megaco);
 		megaco_prefs_initialized = TRUE;
 	}
 	else {
-		dissector_delete("tcp.port", txt_tcp_port, megaco_text_handle);
-		dissector_delete("udp.port", txt_udp_port, megaco_text_handle);
+		dissector_delete("tcp.port", txt_tcp_port, megaco_text_handle_tpkt);
+		dissector_delete("udp.port", txt_udp_port, megaco_text_handle_tpkt);
 #if 0
 		dissector_delete("tcp.port", bin_tcp_port, megaco_bin_handle);
 		dissector_delete("udp.port", bin_udp_port, megaco_bin_handle);
@@ -2637,8 +2724,8 @@
 	bin_udp_port = global_megaco_bin_udp_port;
 #endif
 	
-	dissector_add("tcp.port", global_megaco_txt_tcp_port, megaco_text_handle);
-	dissector_add("udp.port", global_megaco_txt_udp_port, megaco_text_handle);
+	dissector_add("tcp.port", global_megaco_txt_tcp_port, megaco_text_handle_tpkt);
+	dissector_add("udp.port", global_megaco_txt_udp_port, megaco_text_handle_tpkt);
 #if 0
 	dissector_add("tcp.port", global_megaco_bin_tcp_port, megaco_bin_handle);
 	dissector_add("udp.port", global_megaco_bin_udp_port, megaco_bin_handle);