Wireshark · Ethereal-dev: [Ethereal-dev] GIOP patches

["W. Borgert" <debacle@xxxxxxxxxx>]

Hi,

I send three trivial corrections for the GIOP dissector:

- allow filtering GIOP exceptions, e.g. "giop.replystatus = 2"
  or "giop.exceptionid matches MyError", older patch at
  http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=314835

- show IDLs sequence<octet> more compact, not one line per
  octet, older patch at
  http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=314836

- decode _is_a requests and replies, older patch at
  http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=314871

The attached patches are against trunk of today, and not as good
tested as the old patches (against 0.10.10).

Talking of GIOP, has somebody corrected the heuristic dissection
of different GIOP based protocols?  I.e. when having two IDL
interfaces with an operation foo():

interface A { void foo(in long l); };

interface B { void foo(in string s); };

ethereal is/was totally confused and might select the wrong
foo().  I didn't check > 0.10.10, so maybe it's solved.

And a small feature request: It would be nice to have a
Statistics -> Service Response Time -> GIOP menu entry :-)

Cheers,
-- 
W. Borgert <debacle@xxxxxxxxxx>, http://people.debian.org/~debacle/

diff -uraN ethereal.upstream/epan/dissectors/packet-giop.c ethereal/epan/dissectors/packet-giop.c
--- ethereal.upstream/epan/dissectors/packet-giop.c	2005-08-09 17:58:07.622153048 +0000
+++ ethereal/epan/dissectors/packet-giop.c	2005-08-09 18:16:49.287810640 +0000
@@ -436,6 +436,9 @@
 static int hf_giop_iop_vscid = -1;
 static int hf_giop_iop_scid = -1;
 
+static int hf_giop_reply_status = -1;
+static int hf_giop_exception_id = -1;
+
 /*
  * (sub)Tree declares
  */
@@ -2877,12 +2880,6 @@
       }
       if (sequence_length != 0)
 	{
-          if (tree)
-          {
-	      proto_tree_add_text(tree, tvb, offset, sequence_length,
-			   "Exception id: %s",
-			   tvb_format_text(tvb, offset, sequence_length));
-          }
 #if 1
 
           header->exception_id = g_new0(gchar,sequence_length ); /* allocate buffer */
@@ -2891,6 +2888,11 @@
 
           tvb_get_nstringz0(tvb,offset,sequence_length, header->exception_id );
 
+          if (tree)
+          {
+	      proto_tree_add_string(tree, hf_giop_exception_id, tvb,
+			   offset, sequence_length, header->exception_id);
+          }
 
 #endif
 
@@ -3059,9 +3061,8 @@
   }
 
   if (tree) {
-    proto_tree_add_text (reply_tree, tvb, offset-4, 4,
-                         "Reply status: %s",
-                         val_to_str(reply_status, reply_status_types, "Unknown (%u)"));
+    proto_tree_add_uint(reply_tree, hf_giop_reply_status, tvb,
+                         offset-4, 4, reply_status);
 
   }
 
@@ -3133,10 +3134,8 @@
   }
 
   if (tree) {
-    proto_tree_add_text (reply_tree, tvb, offset-4, 4,
-                         "Reply status: %s",
-                         val_to_str(reply_status, reply_status_types, "Unknown (%u)"));
-
+    proto_tree_add_uint(reply_tree, hf_giop_reply_status, tvb,
+                         offset-4, 4, reply_status);
   }
 
   /*
@@ -4209,8 +4208,14 @@
   { "Request operation", "giop.request_op",
 	  FT_STRING, BASE_DEC, NULL, 0x0, "", HFILL }
   },
-	  
-
+  { &hf_giop_reply_status,
+  { "Reply status", "giop.replystatus",
+	  FT_UINT32, BASE_DEC, VALS(reply_status_types), 0x0, "", HFILL }
+  },
+  { &hf_giop_exception_id,
+  { "Exception id", "giop.exceptionid",
+	  FT_STRING, BASE_DEC, NULL, 0x0, "", HFILL }
+  },
   };
 	
 		
@@ -4464,7 +4469,7 @@
 
   guint32 i;			/* loop index */
 
-  guint8 v_major,v_minor;	/* IIOP sersion */
+  guint8 v_major,v_minor;	/* IIOP version */
   gchar *buf;
   guint32 u_octet4;		/* u long */
   guint16 u_octet2;		/* u short */

diff -uraN ethereal.upstream/epan/dissectors/packet-giop.c ethereal/epan/dissectors/packet-giop.c
--- ethereal.upstream/epan/dissectors/packet-giop.c	2005-08-09 17:58:07.622153048 +0000
+++ ethereal/epan/dissectors/packet-giop.c	2005-08-09 18:07:37.047866056 +0000
@@ -720,6 +720,7 @@
 static const char  giop_op_resolve[]           = "resolve";
 static const char  giop_op_bind_new_context[]  = "bind_new_context";
 static const char  giop_op_bind[]              = "bind";
+static const char  giop_op_is_a[]              = "_is_a";
 
 /*
  * Enums  for interesting local operations, that we may need to monitor
@@ -2955,6 +2956,11 @@
 	exres = try_heuristic_giop_dissector(tvb,pinfo,clnp_tree,&offset,header,entry->operation);
       }
 
+      if (!exres && !strcmp(giop_op_is_a, entry->operation) && tree) {
+	proto_tree_add_text(tree, tvb, offset - 1, 1, "Type Id%s matched",
+			    get_CDR_boolean(tvb, &offset) ? "" : " not");
+      }
+
       if(! exres) {
         gint stub_length = tvb_reported_length_remaining(tvb, offset);
 	proto_tree_add_text(tree, tvb, offset, -1,
@@ -3413,7 +3419,16 @@
   /* Only call heuristic if no explicit dissector was found */
 
   if (! exres) {
-    try_heuristic_giop_dissector(tvb,pinfo,tree,&offset,header,operation);
+    exres = try_heuristic_giop_dissector(tvb,pinfo,tree,&offset,header,operation);
+  }
+
+  if (!exres && !strcmp(giop_op_is_a, operation) && request_tree) {
+    gchar *type_id;
+    len = get_CDR_string(tvb, &type_id, &offset, stream_is_big_endian, 0);
+    proto_tree_add_text(request_tree, tvb, offset - len - 4, 4,
+			"Type Id length: %d", len);
+    proto_tree_add_text(request_tree, tvb, offset - len, len,
+			"Type Id: %s", type_id);
   }
 
   if(! exres) {
@@ -3571,7 +3586,16 @@
   /* Only call heuristic if no explicit dissector was found */
 
   if (! exres) {
-    try_heuristic_giop_dissector(tvb,pinfo,tree,&offset,header,operation);
+    exres = try_heuristic_giop_dissector(tvb,pinfo,tree,&offset,header,operation);
+  }
+
+  if (!exres && !strcmp(giop_op_is_a, operation) && request_tree) {
+    gchar *type_id;
+    len = get_CDR_string(tvb, &type_id, &offset, stream_is_big_endian, 0);
+    proto_tree_add_text(request_tree, tvb, offset - len - 4, 4,
+			"Type Id length: %d", len);
+    proto_tree_add_text(request_tree, tvb, offset - len, len,
+			"Type Id: %s", type_id);
   }
 
   if(! exres) {

diff -uraN ethereal.upstream/epan/dissectors/packet-giop.c ethereal/epan/dissectors/packet-giop.c
--- ethereal.upstream/epan/dissectors/packet-giop.c	2005-08-09 17:58:07.622153048 +0000
+++ ethereal/epan/dissectors/packet-giop.c	2005-08-09 18:21:10.922916032 +0000
@@ -1871,7 +1871,7 @@
  * Caller must free the new string.
  */
 
-static gchar * make_printable_string (gchar *in, guint32 len) {
+gchar * make_printable_string (gchar *in, guint32 len) {
   guint32 i = 0;
   gchar *print_string = NULL;
 
diff -uraN ethereal.upstream/epan/dissectors/packet-giop.h ethereal/epan/dissectors/packet-giop.h
--- ethereal.upstream/epan/dissectors/packet-giop.h	2005-08-09 17:58:07.627152288 +0000
+++ ethereal/epan/dissectors/packet-giop.h	2005-08-09 18:21:38.467728584 +0000
@@ -534,7 +534,16 @@
 			   gboolean old_stream_is_big_endian, guint32 old_boundary,
 			   gboolean *new_stream_is_big_endian_ptr, guint32 *new_boundary_ptr );
 
+/* Take in an array of char and create a new string.
+ * Replace non-printable characters with periods.
+ *
+ * The array may contain \0's so dont use strdup
+ * The string is \0 terminated, and thus longer than
+ * the initial sequence.
+ * Caller must free the new string.
+ */
 
+extern gchar * make_printable_string (gchar *in, guint32 len);
 
 /*
  * Enums for TCkind
diff -uraN ethereal.upstream/ethereal_gen.py ethereal/ethereal_gen.py
--- ethereal.upstream/ethereal_gen.py	2005-08-09 17:58:09.653844184 +0000
+++ ethereal/ethereal_gen.py	2005-08-09 18:28:32.718651848 +0000
@@ -1001,6 +1001,9 @@
             self.get_CDR_struct(type,pn)
         elif pt ==  idltype.tk_TypeCode: # will I ever get here ?
             self.get_CDR_TypeCode(pn)
+        elif pt == idltype.tk_sequence and \
+                 type.unalias().seqType().kind() == idltype.tk_octet:
+            self.get_CDR_sequence_octet(type,pn)
         elif pt == idltype.tk_sequence:
             self.get_CDR_sequence(type,pn)
         elif pt == idltype.tk_objref:
@@ -1417,6 +1420,17 @@
         self.st.out(self.template_get_CDR_sequence_loop_end)
 
 
+    #
+    # Generate code to access a sequence of octet
+    #
+
+    def get_CDR_sequence_octet(self,type, pn):
+        self.st.out(self.template_get_CDR_sequence_length, seqname=pn)
+        self.st.out(self.template_get_CDR_sequence_octet, seqname=pn)
+        self.addvar(self.c_i_lim + pn + ";")
+        self.addvar("gchar * binary_seq_" + pn + ";")
+        self.addvar("gchar * text_seq_" + pn + ";")
+
 
     #
     # Generate code to access arrays,
@@ -2085,7 +2099,18 @@
 }
 """
 
-
+    template_get_CDR_sequence_octet = """\
+if (u_octet4_loop_@seqname@ > 0 and tree) {
+    get_CDR_octet_seq(tvb, &binary_seq_@seqname@, offset,
+        u_octet4_loop_@seqname@);
+    text_seq_@seqname@ = make_printable_string(binary_seq_@seqname@,
+        u_octet4_loop_@seqname@);
+    proto_tree_add_text(tree, tvb, *offset - u_octet4_loop_@seqname@,
+        u_octet4_loop_@seqname@, \"@seqname@: %s\", text_seq_@seqname@);
+    g_free(binary_seq_@seqname@);
+    g_free(text_seq_@seqname@);
+}
+"""
 
     template_get_CDR_array_start = """\
 for (i_@aname@=0; i_@aname@ < @aval@; i_@aname@++) {