Wireshark · Ethereal-dev: [Ethereal-dev] [patch] stun dissector

[Jon Ringle <ml-ethereal@xxxxxxxxxx>]

Hello,

Here is a patch for packet-stun.c against svn revision 13043. It addresses the 
following:

1) If a STUN packet has no attributes, ethereal will erroneously report 
[Malformed packet].

2) Made a subtree for each attribute. This allows viewing of a list of the 
attributes without the noise of the attribute details.

Jon

Index: epan/dissectors/packet-stun.c
===================================================================
--- epan/dissectors/packet-stun.c	(revision 13043)
+++ epan/dissectors/packet-stun.c	(working copy)
@@ -89,6 +89,7 @@
 
 /* Initialize the subtree pointers */
 static gint ett_stun = -1;
+static gint ett_stun_att_type = -1;
 static gint ett_stun_att = -1;
 
 
@@ -141,7 +142,10 @@
 	proto_item *ti;
 	proto_item *ta;
 	proto_tree *stun_tree;
+	proto_tree *att_type_tree;
 	proto_tree *att_tree;
+	guint16 msg_type;
+	guint16 msg_length;
 	guint16 att_type;
 	guint16 att_length;
 	guint16 offset;
@@ -154,26 +158,26 @@
 	if (!tvb_bytes_exist(tvb, 0, STUN_HDR_LEN))
 		return 0;
 	
-	att_type = tvb_get_ntohs(tvb, 0);
+	msg_type = tvb_get_ntohs(tvb, 0);
 	
 	/* check if message type is correct */
-	if( 	(att_type != BINDING_REQUEST) &&
-		(att_type != BINDING_RESPONSE) &&
-		(att_type != BINDING_ERROR_RESPONSE) &&
-	    	(att_type != SHARED_SECRET_REQUEST) &&
-	    	(att_type != SHARED_SECRET_RESPONSE) &&
-	    	(att_type != SHARED_SECRET_ERROR_RESPONSE)
+	if( 	(msg_type != BINDING_REQUEST) &&
+		(msg_type != BINDING_RESPONSE) &&
+		(msg_type != BINDING_ERROR_RESPONSE) &&
+	    	(msg_type != SHARED_SECRET_REQUEST) &&
+	    	(msg_type != SHARED_SECRET_RESPONSE) &&
+	    	(msg_type != SHARED_SECRET_ERROR_RESPONSE)
 	  )
 		return 0;
 	
 	
-	att_length = tvb_get_ntohs(tvb, 2);
+	msg_length = tvb_get_ntohs(tvb, 2);
 	
 	/* check if payload enough */
-	if (!tvb_bytes_exist(tvb, 0, STUN_HDR_LEN+att_length))
+	if (!tvb_bytes_exist(tvb, 0, STUN_HDR_LEN+msg_length))
 		return 0;
 
-	if(tvb_bytes_exist(tvb, 0, STUN_HDR_LEN+att_length+1))
+	if(tvb_bytes_exist(tvb, 0, STUN_HDR_LEN+msg_length+1))
 		return 0;
 
 	/* The message seems to be a valid STUN message! */
@@ -185,12 +189,12 @@
 		col_clear(pinfo->cinfo, COL_INFO);
 		
 		col_add_fstr(pinfo->cinfo, COL_INFO, "Message : %s",
-				(att_type==BINDING_REQUEST)?"Binding Request":
-				(att_type==BINDING_RESPONSE)?"Binding Response":
-				(att_type==BINDING_ERROR_RESPONSE)?"Binding Error Response":
-				(att_type==SHARED_SECRET_REQUEST)?"Shared Secret Request":
-				(att_type==SHARED_SECRET_RESPONSE)?"Shared Secret Response":
-				(att_type==SHARED_SECRET_ERROR_RESPONSE)?"Shared Secret Error Response":"UNKNOWN"
+				(msg_type==BINDING_REQUEST)?"Binding Request":
+				(msg_type==BINDING_RESPONSE)?"Binding Response":
+				(msg_type==BINDING_ERROR_RESPONSE)?"Binding Error Response":
+				(msg_type==SHARED_SECRET_REQUEST)?"Shared Secret Request":
+				(msg_type==SHARED_SECRET_RESPONSE)?"Shared Secret Response":
+				(msg_type==SHARED_SECRET_ERROR_RESPONSE)?"Shared Secret Error Response":"UNKNOWN"
 			);
 			    
 	}
@@ -210,8 +214,10 @@
 		proto_tree_add_item(stun_tree, hf_stun_length, tvb, 2, 2, FALSE);
 		proto_tree_add_item(stun_tree, hf_stun_id, tvb, 4, 16, FALSE);
 
-		ta = proto_tree_add_item(stun_tree, hf_stun_att, tvb, STUN_HDR_LEN, -1, FALSE);
-		att_tree = proto_item_add_subtree(ta, ett_stun_att);
+		if (msg_length > 0) {
+		    ta = proto_tree_add_item(stun_tree, hf_stun_att, tvb, STUN_HDR_LEN, msg_length, FALSE);
+		    att_type_tree = proto_item_add_subtree(ta, ett_stun_att_type);
+		}
 
 		offset = STUN_HDR_LEN;
 
@@ -222,6 +228,8 @@
 			att_type = tvb_get_ntohs(tvb, offset); /* Type field in attribute header */
 			att_length = tvb_get_ntohs(tvb, offset+2); /* Length field in attribute header */
 			
+			ta = proto_tree_add_item(att_type_tree, stun_att_type, tvb, offset, 2, FALSE);
+			att_tree = proto_item_add_subtree(ta, ett_stun_att);
 			
 			switch( att_type ){
 				case MAPPED_ADDRESS:
@@ -229,7 +237,6 @@
 				case SOURCE_ADDRESS:
 				case CHANGED_ADDRESS:
 				case REFLECTED_FROM:
-					proto_tree_add_item(att_tree, stun_att_type, tvb, offset, 2, FALSE);
 					proto_tree_add_item(att_tree, stun_att_length, tvb, offset+2, 2, FALSE);
 					proto_tree_add_item(att_tree, stun_att_family, tvb, offset+5, 1, FALSE);
 					proto_tree_add_item(att_tree, stun_att_port, tvb, offset+6, 2, FALSE);
@@ -240,7 +247,6 @@
 					break;
 					
 				case CHANGE_REQUEST:
-					proto_tree_add_item(att_tree, stun_att_type, tvb, offset, 2, FALSE);
 					proto_tree_add_item(att_tree, stun_att_length, tvb, offset+2, 2, FALSE);
 					proto_tree_add_item(att_tree, stun_att_change_ip, tvb, offset+4, 4, FALSE);
 					proto_tree_add_item(att_tree, stun_att_change_port, tvb, offset+4, 4, FALSE);
@@ -252,7 +258,6 @@
 				case USERNAME:
 				case PASSWORD:
 				case MESSAGE_INTEGRITY:
-					proto_tree_add_item(att_tree, stun_att_type, tvb, offset, 2, FALSE);
 					proto_tree_add_item(att_tree, stun_att_length, tvb, offset+2, 2, FALSE);
 					proto_tree_add_item(att_tree, stun_att_value, tvb, offset+4, att_length, FALSE);
 					
@@ -261,7 +266,6 @@
 					break;
 					
 				case ERROR_CODE:
-					proto_tree_add_item(att_tree, stun_att_type, tvb, offset, 2, FALSE);
 					proto_tree_add_item(att_tree, stun_att_length, tvb, offset+2, 2, FALSE);
 					
 					proto_tree_add_item(att_tree, stun_att_error_class, tvb, offset+6, 1, FALSE);
@@ -274,7 +278,6 @@
 				
 				
 				case UNKNOWN_ATTRIBUTES:
-					proto_tree_add_item(att_tree, stun_att_type, tvb, offset, 2, FALSE);
 					proto_tree_add_item(att_tree, stun_att_length, tvb, offset+2, 2, FALSE);
 
 					offset = offset + ATTR_HDR_LEN;
@@ -383,6 +386,7 @@
 /* Setup protocol subtree array */
 	static gint *ett[] = {
 		&ett_stun,
+		&ett_stun_att_type,
 		&ett_stun_att,
 	};
 
@@ -393,6 +397,8 @@
 /* Required function calls to register the header fields and subtrees used */
 	proto_register_field_array(proto_stun, hf, array_length(hf));
 	proto_register_subtree_array(ett, array_length(ett));
+
+	new_register_dissector("stun", dissect_stun, proto_stun);
 }
 
 
@@ -401,7 +407,8 @@
 {
 	dissector_handle_t stun_handle;
 
-	stun_handle = new_create_dissector_handle(dissect_stun, proto_stun);
+	stun_handle = find_dissector("stun");
+
 	dissector_add("tcp.port", TCP_PORT_STUN, stun_handle);
 	dissector_add("udp.port", UDP_PORT_STUN, stun_handle);