Wireshark · Wireshark-dev: [Wireshark-dev] Patch for bug 1092 - need review

[Joerg Mayer <jmayer@xxxxxxxxx>]

As this is the first time I have coded something with conversations I'd
like to get some review before checking it in.

The attached patch will add  handling of the blocksize option as of
rfc2348 (see bug 1092).

  Ciao
       Joerg
-- 
Joerg Mayer                                           <jmayer@xxxxxxxxx>
We are stuck with technology when what we really want is just stuff that
works. Some say that should read Microsoft instead of technology.

Index: epan/dissectors/packet-tftp.c
===================================================================
--- epan/dissectors/packet-tftp.c	(revision 19151)
+++ epan/dissectors/packet-tftp.c	(working copy)
@@ -33,9 +33,17 @@
 #endif
 
 #include <glib.h>
+#include <string.h>
 #include <epan/packet.h>
 #include <epan/conversation.h>
+#include <epan/emem.h>
 
+/* Things we may want to remember for a whole conversation */
+typedef struct _tftp_conv_info_t {
+	guint16 blocksize;
+} tftp_conv_info_t;
+
+
 static int proto_tftp = -1;
 static int hf_tftp_opcode = -1;
 static int hf_tftp_source_file = -1;
@@ -81,20 +89,22 @@
   { 0, NULL }
 };
 
-static void tftp_dissect_options(tvbuff_t *tvb, int offset, proto_tree *tree);
+static void tftp_dissect_options(tvbuff_t *tvb, int offset,
+	proto_tree *tree, conversation_t *conversation);
 
 static void
 dissect_tftp(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree)
 {
-	proto_tree	*tftp_tree = NULL;
-	proto_item	*ti;
-	conversation_t  *conversation;
-	gint		offset = 0;
-	guint16		opcode;
-	guint16		bytes;
-	guint16		blocknum;
-	guint           i1;
-	guint16         error;
+	proto_tree	 *tftp_tree = NULL;
+	proto_item	 *ti;
+	conversation_t   *conversation;
+	gint		 offset = 0;
+	guint16		 opcode;
+	guint16		 bytes, blocksize;
+	guint16		 blocknum;
+	guint            i1;
+	guint16	         error;
+	tftp_conv_info_t *tftp_info;
 
 	/*
 	 * The first TFTP packet goes to the TFTP port; the second one
@@ -122,6 +132,10 @@
 					    pinfo->srcport, 0, NO_PORT2);
             conversation_set_dissector(conversation, tftp_handle);
 	  }
+	} else {
+	  conversation = find_conversation(pinfo->fd->num, &pinfo->src, &pinfo->dst,
+		pinfo->ptype, pinfo->srcport, pinfo->destport, 0);
+	  DISSECTOR_ASSERT(conversation);
 	}
 
 	if (check_col(pinfo->cinfo, COL_PROTOCOL))
@@ -172,7 +186,7 @@
 	  offset += i1;
 
 	  if (tree)
-	    tftp_dissect_options(tvb, offset, tftp_tree);
+	    tftp_dissect_options(tvb, offset, tftp_tree, conversation);
 	  break;
 
 	case TFTP_WRQ:
@@ -199,7 +213,7 @@
 	  offset += i1;
 
 	  if (tree)
-	    tftp_dissect_options(tvb, offset, tftp_tree);
+	    tftp_dissect_options(tvb, offset, tftp_tree, conversation);
 	  break;
 
 	case TFTP_DATA:
@@ -213,9 +227,14 @@
 	  bytes = tvb_reported_length_remaining(tvb, offset);
 
 	  if (check_col(pinfo->cinfo, COL_INFO)) {
+	    blocksize = 512; /* tftp default blocksize */
+	    tftp_info = conversation_get_proto_data(conversation, proto_tftp);
+	    if (tftp_info && tftp_info->blocksize)
+		blocksize = tftp_info->blocksize;
+
 	    col_append_fstr(pinfo->cinfo, COL_INFO, ", Block: %i%s",
 		    blocknum,
-		    (bytes < 512)?" (last)":"" );
+		    (bytes < blocksize)?" (last)":"" );
 	  }
 
 	  if (bytes != 0) {
@@ -263,7 +282,7 @@
 
 	case TFTP_OACK:
 	  if (tree)
-	    tftp_dissect_options(tvb, offset, tftp_tree);
+	    tftp_dissect_options(tvb, offset, tftp_tree, conversation);
 	  break;
 
 	default:
@@ -277,20 +296,39 @@
 }
 
 static void
-tftp_dissect_options(tvbuff_t *tvb, int offset, proto_tree *tree)
+tftp_dissect_options(tvbuff_t *tvb, int offset,
+	proto_tree *tree, conversation_t *conversation)
 {
 	int option_len, value_len;
 	int value_offset;
+	guint8 *optionname;
+	guint8 *optionvalue;
 
 	while (tvb_offset_exists(tvb, offset)) {
 	  option_len = tvb_strsize(tvb, offset);	/* length of option */
 	  value_offset = offset + option_len;
 	  value_len = tvb_strsize(tvb, value_offset);	/* length of value */
+	  optionname = tvb_get_ptr(tvb, offset, option_len);
+	  optionvalue = tvb_get_ptr(tvb, value_offset, value_len);
 	  proto_tree_add_text(tree, tvb, offset, option_len+value_len,
-	          "Option: %s = %s",
-		  tvb_get_ptr(tvb, offset, option_len),
-		  tvb_get_ptr(tvb, value_offset, value_len));
+	          "Option: %s = %s", optionname, optionvalue);
 	  offset += option_len + value_len;
+
+	  /* Special code to handle individual options */
+	  if (!strcmp(optionname, "blksize")) { /* rfc 2348: TFTP Blocksize Option */
+		gint blocksize = strtol(optionvalue, NULL, 10);
+		if (blocksize < 8 || blocksize > 65464) {
+			// Illegal values
+		} else {
+			tftp_conv_info_t *tftp_info;
+			tftp_info = conversation_get_proto_data(conversation, proto_tftp);
+		        if (!tftp_info) {
+                		tftp_info = se_alloc(sizeof(tftp_conv_info_t));
+			}
+                	tftp_info->blocksize = blocksize;
+                	conversation_add_proto_data(conversation, proto_tftp, tftp_info);
+		}
+	  }
 	}
 }