[Gilbert Ramirez <gram@xxxxxxxxxx>]

Here's a patch that uses Guy's excellent 'prefs' routines to 
allow a user to place the decoded payload of an AH packet in
a subtree or keep it at the same level as the AH header.

Placing the payload in a subtree also prevents the summary
information from the payload from showing up in the
GtkCList (by adding a new flag in col_info, 'writable',
which allows dissectors to turn off the ability to write
to the columns. col_check() honors this flag).

Attached is a small trace of ICMP-in-AH sent to be my
Santeri Paavolainen. It was he who requested this
flexiblity (http://ethereal.zing.org/lists/ethereal-dev/200004/msg00273.html)

Because of the way the display filter routines work,
a protocol that is placed in a subtree is not searchable.
This is because the display filter routines expect protocols
to be at the "top level" of the protocol tree. I could fix this,
or I could keep this as a feature. For example, in this
sample trace, when ICMP is at the same level as AH,
the display filter "icmp" works. But when ICMP is placed in
a subtree of AH, "icmp" fails to find any packets. This could
be viewed as a feature since if you're hiding ICMP under AH
anyway, then you're probably only interested in the headers up to
AH, but not after AH.

Anyway, for those that us tunneled protocols (SOCKS, AH, PIM, etc),
let me know if this type of arrangement is good. That is, should
one preference item control *both* the placement of the decode
and the packet summary record, or should they be split into two?
Should the display filter routines act differently? etc.

--gilbert


? conv
Index: packet-ipsec.c
===================================================================
RCS file: /usr/local/cvsroot/ethereal/packet-ipsec.c,v
retrieving revision 1.17
diff -u -r1.17 packet-ipsec.c
--- packet-ipsec.c	2000/06/05 03:21:02	1.17
+++ packet-ipsec.c	2000/07/07 16:58:50
@@ -42,7 +42,11 @@
 #include "packet-ipsec.h"
 #include "packet-ip.h"
 #include "resolv.h"
+#include "prefs.h"
 
+/* Place AH payload in sub tree */
+gboolean g_ah_payload_in_subtree = FALSE;
+
 static int proto_ah = -1;
 static int hf_ah_spi = -1;
 static int hf_ah_sequence = -1;
@@ -144,7 +148,7 @@
 void
 dissect_ah(const u_char *pd, int offset, frame_data *fd, proto_tree *tree)
 {
-    proto_tree *ah_tree;
+    proto_tree *ah_tree, *next_tree = NULL;
     proto_item *ti;
     struct newah ah;
     int advance;
@@ -176,15 +180,27 @@
 			    (guint32)ntohl(ah.ah_seq));
 	proto_tree_add_text(ah_tree, NullTVB, offset + sizeof(ah), (ah.ah_len - 1) << 2,
 			    "ICV");
+
+	/* Decide where to place next protocol decode */
+	if (g_ah_payload_in_subtree) {
+		next_tree = ah_tree;
+	}
+	else {
+		next_tree = tree;
+	}
     }
 
     /* start of the new header (could be a extension header) */
     offset += advance;
 
-  /* do lookup with the subdissector table */
-  if (!dissector_try_port(ip_dissector_table, ah.ah_nxt, pd, offset, fd, tree)) {
-    dissect_data(pd, offset, fd, tree);
-  }
+    if (g_ah_payload_in_subtree && fd->cinfo) {
+	fd->cinfo->writable = FALSE;
+    }
+
+    /* do lookup with the subdissector table */
+    if (!dissector_try_port(ip_dissector_table, ah.ah_nxt, pd, offset, fd, next_tree)) {
+      dissect_data(pd, offset, fd, next_tree);
+    }
 }
 
 static void
@@ -317,6 +333,8 @@
     &ett_ipcomp,
   };
 
+  module_t *ah_module;
+
   proto_ah = proto_register_protocol("Authentication Header", "ah");
   proto_register_field_array(proto_ah, hf_ah, array_length(hf_ah));
 
@@ -327,6 +345,13 @@
   proto_register_field_array(proto_ipcomp, hf_ipcomp, array_length(hf_ipcomp));
 
   proto_register_subtree_array(ett, array_length(ett));
+
+  /* Register a configuration option for placement of AH payload dissection */
+  ah_module = prefs_register_module("ah", "AH", NULL);
+  prefs_register_bool_preference(ah_module, "place_ah_payload_in_subtree",
+	    "Place AH payload in subtree",
+"Whether the AH payload decode should be placed in a subtree",
+	    &g_ah_payload_in_subtree);
 }
 
 void
Index: packet.c
===================================================================
RCS file: /usr/local/cvsroot/ethereal/packet.c,v
retrieving revision 1.95
diff -u -r1.95 packet.c
--- packet.c	2000/06/27 04:35:45	1.95
+++ packet.c	2000/07/07 16:58:50
@@ -700,7 +700,7 @@
 check_col(frame_data *fd, gint el) {
   int i;
 
-  if (fd->cinfo) {
+  if (fd->cinfo && fd->cinfo->writable) {
     for (i = 0; i < fd->cinfo->num_cols; i++) {
       if (fd->cinfo->fmt_matx[i][el])
         return TRUE;
@@ -715,14 +715,15 @@
   va_list ap;
   int     i;
   size_t  max_len;
+
+  if (el == COL_INFO)
+	max_len = COL_MAX_INFO_LEN;
+  else
+	max_len = COL_MAX_LEN;
   
   va_start(ap, format);
   for (i = 0; i < fd->cinfo->num_cols; i++) {
     if (fd->cinfo->fmt_matx[i][el]) {
-      if (el == COL_INFO)
-	max_len = COL_MAX_INFO_LEN;
-      else
-	max_len = COL_MAX_LEN;
       vsnprintf(fd->cinfo->col_data[i], max_len, format, ap);
     }
   }
@@ -733,12 +734,13 @@
   int    i;
   size_t max_len;
 
-  for (i = 0; i < fd->cinfo->num_cols; i++) {
-    if (fd->cinfo->fmt_matx[i][el]) {
-      if (el == COL_INFO)
+  if (el == COL_INFO)
 	max_len = COL_MAX_INFO_LEN;
-      else
+  else
 	max_len = COL_MAX_LEN;
+  
+  for (i = 0; i < fd->cinfo->num_cols; i++) {
+    if (fd->cinfo->fmt_matx[i][el]) {
       strncpy(fd->cinfo->col_data[i], str, max_len);
       fd->cinfo->col_data[i][max_len - 1] = 0;
     }
@@ -752,14 +754,15 @@
   int     i;
   size_t  len, max_len;
   
+  if (el == COL_INFO)
+	max_len = COL_MAX_INFO_LEN;
+  else
+	max_len = COL_MAX_LEN;
+  
   va_start(ap, format);
   for (i = 0; i < fd->cinfo->num_cols; i++) {
     if (fd->cinfo->fmt_matx[i][el]) {
       len = strlen(fd->cinfo->col_data[i]);
-      if (el == COL_INFO)
-	max_len = COL_MAX_INFO_LEN;
-      else
-	max_len = COL_MAX_LEN;
       vsnprintf(&fd->cinfo->col_data[i][len], max_len - len, format, ap);
     }
   }
@@ -770,13 +773,14 @@
   int    i;
   size_t len, max_len;
 
+  if (el == COL_INFO)
+	max_len = COL_MAX_INFO_LEN;
+  else
+	max_len = COL_MAX_LEN;
+  
   for (i = 0; i < fd->cinfo->num_cols; i++) {
     if (fd->cinfo->fmt_matx[i][el]) {
       len = strlen(fd->cinfo->col_data[i]);
-      if (el == COL_INFO)
-	max_len = COL_MAX_LEN;
-      else
-	max_len = COL_MAX_INFO_LEN;
       strncat(fd->cinfo->col_data[i], str, max_len - len);
       fd->cinfo->col_data[i][max_len - 1] = 0;
     }
@@ -1172,6 +1176,10 @@
 	pi.fd = fd;
 	pi.compat_top_tvb = tvb;
 	pi.pseudo_header = pseudo_header;
+
+	if (fd->cinfo) {
+		fd->cinfo->writable = TRUE;
+	}
 
 	TRY {
 		switch (fd->lnk_t) {
Index: packet.h
===================================================================
RCS file: /usr/local/cvsroot/ethereal/packet.h,v
retrieving revision 1.189
diff -u -r1.189 packet.h
--- packet.h	2000/05/26 22:08:16	1.189
+++ packet.h	2000/07/07 16:58:51
@@ -94,6 +94,7 @@
   gint      *col_width; /* Column widths to use during a "-S" capture */
   gchar    **col_title; /* Column titles */
   gchar    **col_data;  /* Column data */
+  gboolean   writable;  /* Are we stil writing to the columns? */
 } column_info;
 
 #define COL_MAX_LEN 256

Attachment: ah-ipip-ping.dat
Description: Binary data