Wireshark · Wireshark-dev: Re: [Wireshark-dev] [PATCH] draft-ietf-behave-rfc3489bis-07

[Marc Petit-Huguenin <marc@xxxxxxxxxxxxxxxxxx>]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Jeff Morriss wrote:
> Marc Petit-Huguenin wrote:
>> -----BEGIN PGP SIGNED MESSAGE-----
>> Hash: SHA1
>>
>> Hi,
>>
>> This is an update to the STUN2 dissector using the last draft,
>> draft-ietf-behave-rfc3489bis-07.
> 
> It doesn't compile as is:
> 
>>  gcc -DHAVE_CONFIG_H -I. -I. -I../.. -I./../.. -I./.. -I/usr/include -I/usr/local/include -Werror -DINET6 "-D_U_=__attribute__((unused))" -g -O2 -Wall -W -Wdeclaration-after-statement -Wendif-labels -Wpointer-arith -I/usr/local/include -I/usr/include/gtk-1.2 -I/usr/include/glib-1.2 -I/usr/lib64/glib/include -I/usr/X11R6/include -MT libcleandissectors_la-packet-stun2.lo -MD -MP -MF .deps/libcleandissectors_la-packet-stun2.Tpo -c packet-stun2.c  -fPIC -DPIC -o .libs/libcleandissectors_la-packet-stun2.o
>> packet-stun2.c: In function `dissect_stun2_tcp':
>> packet-stun2.c:398: warning: implicit declaration of function `tcp_dissect_pdus'
>> packet-stun2.c: In function `proto_reg_handoff_stun2':
>> packet-stun2.c:548: warning: passing arg 1 of `create_dissector_handle' from incompatible pointer type
>> packet-stun2.c:549: warning: passing arg 1 of `create_dissector_handle' from incompatible pointer type
>> packet-stun2.c: At top level:
>> packet-stun2.c:146: warning: unused parameter 'pinfo'
>> packet-stun2.c:60: warning: 'stun2_att_password' defined but not used
>> packet-stun2.c:73: warning: 'stun2_att_refresh_interval' defined but not used
>> make[1]: *** [libcleandissectors_la-packet-stun2.lo] Error 1
> 
> Probably due to mixing new-style (returns int) and old style (returns 
> nothing) dissectors.  For example:
> 
>> static int
>> dissect_stun2_tcp(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree)
>> {
>>         tcp_dissect_pdus(tvb, pinfo, tree, TRUE, STUN2_HDR_LEN,
>>                         get_stun2_message_len, dissect_stun2_message);
>> }
> 
> will (eventually) generate a warning that the function's not calling return.
> 
> Sorry, I don't really have time to clean it up myself now.

Sorry for the mistakes.

Please find attached a new version of the patch.


- --
Marc Petit-Huguenin           [                                 ]
Home: marc@xxxxxxxxxxxxxxxxxx [RFC1855-compliant space for rent ]
Work: marc@xxxxxxx            [                                 ]
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.6 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFGoTTW9RoMZyVa61cRAqdHAJ9H1jdXK/AZ1J1MJWCZbfG7+Fs7jgCgj3q5
RSogAiGnPp2NXVZb0XfDloU=
=Q23u
-----END PGP SIGNATURE-----

Index: epan/dissectors/packet-stun2.c
===================================================================
--- epan/dissectors/packet-stun2.c	(revision 22365)
+++ epan/dissectors/packet-stun2.c	(working copy)
@@ -1,7 +1,8 @@
 /* packet-stun2.c
- * Routines for Simple Traversal Underneath NAT dissection
+ * Routines for Session Traversal Utilities for NAT (STUN) dissection
  * Copyright 2003, Shiang-Ming Huang <smhuang@xxxxxxxxxxxxxxxxxxxx>
  * Copyright 2006, Marc Petit-Huguenin <marc@xxxxxxxxxxxxxxxxxx>
+ * Copyright 2007, 8x8 Inc. <petithug@xxxxxxx>
  *
  * $Id$
  *
@@ -23,7 +24,7 @@
  * along with this program; if not, write to the Free Software
  * Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA.
  *
- * Please refer to draft-ietf-behave-rfc3489bis-05 for protocol detail.
+ * Please refer to draft-ietf-behave-rfc3489bis-07 for protocol detail.
  */
 
 #ifdef HAVE_CONFIG_H
@@ -37,6 +38,7 @@
 #include <glib.h>
 
 #include <epan/packet.h>
+#include "packet-tcp.h"
 
 /* Initialize the protocol and registered fields */
 static int proto_stun2 = -1;
@@ -56,7 +58,6 @@
 static int stun2_att_port = -1;
 static int stun2_att_username = -1;
 static int stun2_att_padding = -1;
-static int stun2_att_password = -1;
 static int stun2_att_hmac = -1;
 static int stun2_att_crc32 = -1;
 static int stun2_att_error_class = -1;
@@ -69,7 +70,6 @@
 static int stun2_att_xor_ipv6 = -1;
 static int stun2_att_xor_port = -1;
 static int stun2_att_server = -1;
-static int stun2_att_refresh_interval = -1;
 static int stun2_att_value = -1;
 
 /* Message classes */
@@ -77,27 +77,24 @@
 #define REQUEST		0x0000
 #define INDICATION	0x0001
 #define RESPONSE	0x0010
-#define STUN2_ERROR	0x0011  /* use prefix to prevent redefinition from wingdi.h */
+#define ERROR_RESPONSE	0x0011
 
-/* Message methods */
-#define METHOD_MASK	0xCEEF
-#define BINDING		0x0001
-#define SHARED_SECRET	0x0002
+/* Request/Response Transactions */
+#define METHOD_MASK		0xCEEF
+#define BINDING			0x0001	/* draft-ietf-behave-rfc3489bis-07 */
 
 /* Attribute Types */
-#define MAPPED_ADDRESS		0x0001
-#define USERNAME		0x0006
-#define PASSWORD		0x0007
-#define MESSAGE_INTEGRITY	0x0008
-#define ERROR_CODE		0x0009
-#define UNKNOWN_ATTRIBUTES	0x000a
-#define REALM			0x0014
-#define NONCE			0x0015
-#define XOR_MAPPED_ADDRESS	0x0020
-#define SERVER			0x8022
-#define ALTERNATE_SERVER	0x8023
-#define REFRESH_INTERVAL	0x8024
-#define FINGERPRINT		0x8025
+#define MAPPED_ADDRESS		0x0001	/* draft-ietf-behave-rfc3489bis-07 */
+#define USERNAME		0x0006	/* draft-ietf-behave-rfc3489bis-07 */
+#define MESSAGE_INTEGRITY	0x0008	/* draft-ietf-behave-rfc3489bis-07 */
+#define ERROR_CODE		0x0009	/* draft-ietf-behave-rfc3489bis-07 */
+#define UNKNOWN_ATTRIBUTES	0x000a	/* draft-ietf-behave-rfc3489bis-07 */
+#define REALM			0x0014	/* draft-ietf-behave-rfc3489bis-07 */
+#define NONCE			0x0015	/* draft-ietf-behave-rfc3489bis-07 */
+#define XOR_MAPPED_ADDRESS	0x0020	/* draft-ietf-behave-rfc3489bis-07 */
+#define SERVER			0x8022	/* draft-ietf-behave-rfc3489bis-07 */
+#define ALTERNATE_SERVER	0x8023	/* draft-ietf-behave-rfc3489bis-07 */
+#define FINGERPRINT		0x8028	/* draft-ietf-behave-rfc3489bis-07 */
 
 /* Initialize the subtree pointers */
 static gint ett_stun2 = -1;
@@ -114,21 +111,19 @@
 static const value_string classes[] = {
 	{REQUEST, "Request"},
 	{INDICATION, "Indication"},
-	{RESPONSE, "Response"},
-	{STUN2_ERROR, "Error Response"},
+	{RESPONSE, "Success Response"},
+	{ERROR_RESPONSE, "Error Response"},
 	{0x00, NULL}
 };
 
 static const value_string methods[] = {
 	{BINDING, "Binding"},
-	{SHARED_SECRET, "Shared Secret"},
 	{0x00, NULL}
 };
 
 static const value_string attributes[] = {
 	{MAPPED_ADDRESS, "MAPPED-ADDRESS"},
 	{USERNAME, "USERNAME"},
-	{PASSWORD, "PASSWORD"},
 	{MESSAGE_INTEGRITY, "MESSAGE-INTEGRITY"},
 	{ERROR_CODE, "ERROR-CODE"},
 	{UNKNOWN_ATTRIBUTES, "UNKNOWN-ATTRIBUTES"},
@@ -137,7 +132,6 @@
 	{XOR_MAPPED_ADDRESS, "XOR-MAPPED-ADDRESS"},
 	{SERVER, "SERVER"},
 	{ALTERNATE_SERVER, "ALTERNATE-SERVER"},
-	{REFRESH_INTERVAL, "REFRESH-INTERVAL"},
 	{FINGERPRINT, "FINGERPRINT"},
 	{0x00, NULL}
 };
@@ -148,9 +142,14 @@
 	{0x00, NULL}
 };
 
-static int
-dissect_stun2(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree)
+static guint get_stun2_message_len(packet_info *pinfo _U_, tvbuff_t *tvb, int offset)
 {
+	return (guint)tvb_get_ntohs(tvb, offset+2) + 20;
+}
+
+static void
+dissect_stun2_message(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree)
+{
 	proto_item *ti;
 	proto_item *ta;
 	proto_tree *stun2_tree;
@@ -171,22 +170,22 @@
 
 	/* First, make sure we have enough data to do the check. */
 	if (!tvb_bytes_exist(tvb, 0, STUN2_HDR_LEN))
-		return 0;
+		return;
 
 	msg_type = tvb_get_ntohs(tvb, 0);
 	msg_length = tvb_get_ntohs(tvb, 2);
 
 	/* Check if it is really a STUN2 message */
 	if (msg_type & 0xC000 || tvb_get_ntohl(tvb, 4) != 0x2112a442)
-		return 0;
+		return;
 
 	/* check if payload enough */
 	if (!tvb_bytes_exist(tvb, 0, STUN2_HDR_LEN+msg_length))
-		return 0;
+		return;
 
 	/* Check if too much payload */
 	if (tvb_bytes_exist(tvb, 0, STUN2_HDR_LEN+msg_length+1))
-		return 0;
+		return;
 
 	/* The message seems to be a valid STUN2 message! */
 
@@ -231,7 +230,7 @@
 				ta = proto_tree_add_text(att_type_tree, tvb, offset,
 					ATTR_HDR_LEN+att_length,
 					"Attribute: %s",
-					val_to_str(att_type, attributes, "Unknown (0x%04x)"));
+					val_to_str(att_type, attributes, att_type & 0x8000 ? "Unknown (0x%4x) - Comprehension-optional" : "Unknown (0x%04x)- Comprehension-required"));
 				att_tree = proto_item_add_subtree(ta, ett_stun2_att);
 
 				proto_tree_add_uint(att_tree, stun2_att_type, tvb,
@@ -278,36 +277,12 @@
 							proto_tree_add_uint(att_tree, stun2_att_padding, tvb, offset+att_length, 4-(att_length % 4), 4-(att_length % 4));
 						break;
 
-					case PASSWORD:
-						proto_tree_add_item(att_tree, stun2_att_password, tvb, offset, att_length, FALSE);
-						if (att_length % 4 != 0)
-							proto_tree_add_uint(att_tree, stun2_att_padding, tvb, offset+att_length, 4-(att_length % 4), 4-(att_length % 4));
-						break;
-
-					case NONCE:
-						proto_tree_add_item(att_tree, stun2_att_nonce, tvb, offset, att_length, FALSE);
-						if (att_length % 4 != 0)
-							proto_tree_add_uint(att_tree, stun2_att_padding, tvb, offset+att_length, 4-(att_length % 4), 4-(att_length % 4));
-						break;
-
-					case REALM:
-						proto_tree_add_item(att_tree, stun2_att_realm, tvb, offset, att_length, FALSE);
-						if (att_length % 4 != 0)
-							proto_tree_add_uint(att_tree, stun2_att_padding, tvb, offset+att_length, 4-(att_length % 4), 4-(att_length % 4));
-						break;
-
 					case MESSAGE_INTEGRITY:
 						if (att_length < 20)
 							break;
 						proto_tree_add_item(att_tree, stun2_att_hmac, tvb, offset, att_length, FALSE);
 						break;
 
-					case FINGERPRINT:
-						if (att_length < 4)
-							break;
-						proto_tree_add_item(att_tree, stun2_att_crc32, tvb, offset, att_length, FALSE);
-						break;
-
 					case ERROR_CODE:
 						if (att_length < 3)
 							break;
@@ -329,12 +304,18 @@
 							proto_tree_add_uint(att_tree, stun2_att_padding, tvb, offset+att_length, 4-(att_length % 4), 4-(att_length % 4));
 						break;
 
-					case SERVER:
-						proto_tree_add_item(att_tree, stun2_att_server, tvb, offset, att_length, FALSE);
+					case REALM:
+						proto_tree_add_item(att_tree, stun2_att_realm, tvb, offset, att_length, FALSE);
 						if (att_length % 4 != 0)
 							proto_tree_add_uint(att_tree, stun2_att_padding, tvb, offset+att_length, 4-(att_length % 4), 4-(att_length % 4));
 						break;
 
+					case NONCE:
+						proto_tree_add_item(att_tree, stun2_att_nonce, tvb, offset, att_length, FALSE);
+						if (att_length % 4 != 0)
+							proto_tree_add_uint(att_tree, stun2_att_padding, tvb, offset+att_length, 4-(att_length % 4), 4-(att_length % 4));
+						break;
+
 					case XOR_MAPPED_ADDRESS:
 						if (att_length < 2)
 							break;
@@ -377,10 +358,16 @@
 							}
 						break;
 
-					case REFRESH_INTERVAL:
+					case SERVER:
+						proto_tree_add_item(att_tree, stun2_att_server, tvb, offset, att_length, FALSE);
+						if (att_length % 4 != 0)
+							proto_tree_add_uint(att_tree, stun2_att_padding, tvb, offset+att_length, 4-(att_length % 4), 4-(att_length % 4));
+						break;
+
+					case FINGERPRINT:
 						if (att_length < 4)
 							break;
-						proto_tree_add_item(att_tree, stun2_att_refresh_interval, tvb, offset, 4, FALSE);
+						proto_tree_add_item(att_tree, stun2_att_crc32, tvb, offset, att_length, FALSE);
 						break;
 
 					default:
@@ -395,16 +382,46 @@
 		    }
 		}
 	}
-	return tvb_length(tvb);
 }
 
+static void
+dissect_stun2_udp(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree)
+{
+	dissect_stun2_message(tvb, pinfo, tree);
+}
 
+static void
+dissect_stun2_tcp(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree)
+{
+	tcp_dissect_pdus(tvb, pinfo, tree, TRUE, STUN2_HDR_LEN,
+			get_stun2_message_len, dissect_stun2_message);
+}
+
 static gboolean
 dissect_stun2_heur(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree)
 {
-	if (dissect_stun2(tvb, pinfo, tree) == 0)
+	guint16 msg_type;
+	guint16 msg_length;
+
+	/* First, make sure we have enough data to do the check. */
+	if (!tvb_bytes_exist(tvb, 0, STUN2_HDR_LEN))
 		return FALSE;
 
+	msg_type = tvb_get_ntohs(tvb, 0);
+	msg_length = tvb_get_ntohs(tvb, 2);
+
+	/* Check if it is really a STUN2 message */
+	if (msg_type & 0xC000 || tvb_get_ntohl(tvb, 4) != 0x2112a442)
+		return FALSE;
+
+	/* check if payload enough */
+	if (!tvb_bytes_exist(tvb, 0, STUN2_HDR_LEN+msg_length))
+		return FALSE;
+
+	/* Check if too much payload */
+	if (tvb_bytes_exist(tvb, 0, STUN2_HDR_LEN+msg_length+1))
+		return FALSE;
+	dissect_stun2_message(tvb, pinfo, tree);
 	return TRUE;
 }
 
@@ -422,7 +439,7 @@
 		},
 		{ &hf_stun2_length,
 			{ "Message Length",	"stun2.length",	FT_UINT16,
-			BASE_HEX,	NULL,	0x0, 	"",	HFILL }
+			BASE_DEC,	NULL,	0x0, 	"",	HFILL }
 		},
 		{ &hf_stun2_cookie,
 			{ "Message Cookie",	"stun2.cookie",	FT_BYTES,
@@ -469,10 +486,6 @@
 			{ "Padding",	"stun2.att.padding",	FT_UINT16,
 			BASE_DEC,	NULL,	0x0, 	"",	HFILL }
 		},
-		{ &stun2_att_password,
-			{ "Password",	"stun2.att.password",	FT_STRING,
-			BASE_NONE,	NULL,	0x0, 	"",	HFILL }
-		},
 		{ &stun2_att_hmac,
 			{ "HMAC-SHA1",	"stun2.att.hmac",	FT_BYTES,
 			BASE_HEX,	NULL,	0x0, 	"",	HFILL }
@@ -490,7 +503,7 @@
 			BASE_DEC, 	NULL,	0x0,	"",	HFILL}
 		},
 		{ &stun2_att_error_reason,
-			{ "Error Reason Phase","stun2.att.error.reason",	FT_STRING,
+			{ "Error Reason Phrase","stun2.att.error.reason",	FT_STRING,
 			BASE_NONE, 	NULL,	0x0,	"",	HFILL}
 		},
 		{ &stun2_att_realm,
@@ -521,10 +534,6 @@
 			{ "Server software","stun2.att.server",	FT_STRING,
 			BASE_NONE, 	NULL,	0x0,	"",	HFILL}
  		},
-		{ &stun2_att_refresh_interval,
-			{ "Refresh Interval","stun2.att.refresh-interval",	FT_UINT16,
-			BASE_DEC, 	NULL,	0x0,	"",	HFILL}
- 		},
 		{ &stun2_att_value,
 			{ "Value",	"stun2.value",	FT_BYTES,
 			BASE_HEX,	NULL,	0x0, 	"",	HFILL }
@@ -539,26 +548,25 @@
 	};
 
 /* Register the protocol name and description */
-	proto_stun2 = proto_register_protocol("Simple Traversal Underneath NAT",
+	proto_stun2 = proto_register_protocol("Session Traversal Utilities for NAT",
 	    "STUN2", "stun2");
 
 /* Required function calls to register the header fields and subtrees used */
 	proto_register_field_array(proto_stun2, hf, array_length(hf));
 	proto_register_subtree_array(ett, array_length(ett));
-
-	new_register_dissector("stun2", dissect_stun2, proto_stun2);
 }
 
-
 void
 proto_reg_handoff_stun2(void)
 {
-	dissector_handle_t stun2_handle;
+	dissector_handle_t stun2_tcp_handle;
+	dissector_handle_t stun2_udp_handle;
 
-	stun2_handle = find_dissector("stun2");
+	stun2_tcp_handle = create_dissector_handle(dissect_stun2_tcp, proto_stun2);
+	stun2_udp_handle = create_dissector_handle(dissect_stun2_udp, proto_stun2);
 
-	dissector_add("tcp.port", TCP_PORT_STUN2, stun2_handle);
-	dissector_add("udp.port", UDP_PORT_STUN2, stun2_handle);
+	dissector_add("tcp.port", TCP_PORT_STUN2, stun2_tcp_handle);
+	dissector_add("udp.port", UDP_PORT_STUN2, stun2_udp_handle);
 
 	heur_dissector_add("udp", dissect_stun2_heur, proto_stun2);
 	heur_dissector_add("tcp", dissect_stun2_heur, proto_stun2);