[Martin Mathieson <martin.mathieson@xxxxxxxxxxxx>]

Hi,

These patches:
- allow SDP to parse the IP address + port for the MSRP session from the 
path attribute
- setup an MSRP conversation using this address, whose data points back 
to the SDP frame
- link to the SDP setup frame while dissecting MSRP (can be switched off 
by a preference)
- I also changed sdp.media.port to be a numeric field

Best regards,
Martin

Index: epan/dissectors/packet-msrp.c
===================================================================
--- epan/dissectors/packet-msrp.c	(revision 18787)
+++ epan/dissectors/packet-msrp.c	(working copy)
@@ -39,8 +39,11 @@
 #include <epan/conversation.h>
 
 #include <epan/packet.h>
+#include <epan/emem.h>
 #include "prefs.h"
 
+#include "packet-msrp.h"
+
 #define TCP_PORT_MSRP 0
 
 #define MSRP_HDR "MSRP"
@@ -57,6 +60,7 @@
 static int ett_msrp_element			= -1;
 static int ett_msrp_data			= -1;
 static int ett_msrp_end_line		= -1;
+static int ett_msrp_setup			= -1;
 
 static int hf_msrp_response_line	= -1;
 static int hf_msrp_request_line		= -1;
@@ -67,6 +71,11 @@
 static int hf_msrp_end_line			= -1;
 static int hf_msrp_cnt_flg			= -1;
 
+/* MSRP setup fields */
+static int hf_msrp_setup        = -1;
+static int hf_msrp_setup_frame  = -1;
+static int hf_msrp_setup_method = -1;
+
 typedef struct {
         const char *name;
 } msrp_header_t;
@@ -136,6 +145,145 @@
 static int dissect_msrp(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree);
 
 
+/* Displaying conversation setup info */
+static gboolean global_msrp_show_setup_info = TRUE;
+static void show_setup_info(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree);
+
+/* Set up an MSRP conversation using the info given */
+void msrp_add_address( packet_info *pinfo,
+                       address *addr, int port,
+                       const gchar *setup_method, guint32 setup_frame_number)
+{
+	address null_addr;
+	conversation_t* p_conv;
+	struct _msrp_conversation_info *p_conv_data = NULL;
+
+	/*
+	 * If this isn't the first time this packet has been processed,
+	 * we've already done this work, so we don't need to do it
+	 * again.
+	 */
+	if (pinfo->fd->flags.visited)
+	{
+		return;
+	}
+
+	SET_ADDRESS(&null_addr, AT_NONE, 0, NULL);
+
+	/*
+	 * Check if the ip address and port combination is not
+	 * already registered as a conversation.
+	 */
+	p_conv = find_conversation( pinfo->fd->num, addr, &null_addr, PT_TCP, port, 0,
+	                            NO_ADDR_B | NO_PORT_B);
+
+	/*
+	 * If not, create a new conversation.
+	 */
+	if (!p_conv) {
+		p_conv = conversation_new( pinfo->fd->num, addr, &null_addr, PT_TCP,
+		                           (guint32)port, 0,
+		                           NO_ADDR2 | NO_PORT2);
+	}
+
+	/* Set dissector */
+	conversation_set_dissector(p_conv, msrp_handle);
+
+	/*
+	 * Check if the conversation has data associated with it.
+	 */
+	p_conv_data = conversation_get_proto_data(p_conv, proto_msrp);
+
+	/*
+	 * If not, add a new data item.
+	 */
+	if (!p_conv_data) {
+		/* Create conversation data */
+		p_conv_data = se_alloc(sizeof(struct _msrp_conversation_info));
+		if (!p_conv_data)
+		{
+			return;
+		}
+		memset(p_conv_data, 0, sizeof(struct _msrp_conversation_info));
+		conversation_add_proto_data(p_conv, proto_msrp, p_conv_data);
+	}
+
+	/*
+	 * Update the conversation data.
+	 */
+	p_conv_data->setup_method_set = TRUE;
+	strncpy(p_conv_data->setup_method, setup_method, MAX_MSRP_SETUP_METHOD_SIZE);
+	p_conv_data->setup_method[MAX_MSRP_SETUP_METHOD_SIZE] = '\0';
+	p_conv_data->setup_frame_number = setup_frame_number;
+}
+
+
+
+/* Look for conversation info and display any setup info found */
+void show_setup_info(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree)
+{
+	/* Conversation and current data */
+	conversation_t *p_conv = NULL;
+	struct _msrp_conversation_info *p_conv_data = NULL;
+
+	/* Use existing packet data if available */
+	p_conv_data = p_get_proto_data(pinfo->fd, proto_msrp);
+
+	if (!p_conv_data)
+	{
+		/* First time, get info from conversation */
+		p_conv = find_conversation(pinfo->fd->num, &pinfo->net_dst, &pinfo->net_src,
+		                           PT_TCP,
+		                           pinfo->destport, pinfo->srcport, 0);//NO_ADDR_B | NO_PORT_B);
+
+		if (p_conv)
+		{
+			/* Look for data in conversation */
+			struct _msrp_conversation_info *p_conv_packet_data;
+			p_conv_data = conversation_get_proto_data(p_conv, proto_msrp);
+
+			if (p_conv_data)
+			{
+				/* Save this conversation info into packet info */
+				p_conv_packet_data = se_alloc(sizeof(struct _msrp_conversation_info));
+				if (!p_conv_packet_data)
+				{
+					return;
+				}
+				memcpy(p_conv_packet_data, p_conv_data,
+				       sizeof(struct _msrp_conversation_info));
+
+				p_add_proto_data(pinfo->fd, proto_msrp, p_conv_packet_data);
+			}
+		}                                                           
+	}
+
+	/* Create setup info subtree with summary info. */
+	if (p_conv_data && p_conv_data->setup_method_set)
+	{
+		proto_tree *msrp_setup_tree;
+		proto_item *ti =  proto_tree_add_string_format(tree, hf_msrp_setup, tvb, 0, 0,
+		                                               "",
+		                                               "Stream setup by %s (frame %u)",
+		                                               p_conv_data->setup_method,
+		                                               p_conv_data->setup_frame_number);
+		PROTO_ITEM_SET_GENERATED(ti);
+		msrp_setup_tree = proto_item_add_subtree(ti, ett_msrp_setup);
+		if (msrp_setup_tree)
+		{
+			/* Add details into subtree */
+			proto_item* item = proto_tree_add_uint(msrp_setup_tree, hf_msrp_setup_frame,
+			                                       tvb, 0, 0, p_conv_data->setup_frame_number);
+			PROTO_ITEM_SET_GENERATED(item);
+			item = proto_tree_add_string(msrp_setup_tree, hf_msrp_setup_method,
+			                             tvb, 0, 0, p_conv_data->setup_method);
+			PROTO_ITEM_SET_GENERATED(item);
+		}
+	}
+}
+
+
+
 /* Returns index of headers */
 static gint msrp_is_known_msrp_header(tvbuff_t *tvb, int offset, guint header_len)
 {
@@ -286,8 +434,10 @@
 		 * TODO Set up conversation here
 		 */
 		if (pinfo->fd->flags.visited){
+			/* Look for existing conversation */
 			conversation = find_conversation(pinfo->fd->num, &pinfo->src, &pinfo->dst, pinfo->ptype,
 				pinfo->srcport, pinfo->destport, 0);
+			/* Create new one if not found */
 			if (conversation == NULL){
 				conversation = conversation_new(pinfo->fd->num, &pinfo->src, &pinfo->dst,
 					pinfo->ptype, pinfo->srcport, pinfo->destport, 0);
@@ -447,6 +597,12 @@
 			proto_tree_add_item(reqresp_tree,hf_msrp_method,tvb,token_3_start,token_3_len,FALSE);
 		}
 
+		/* Conversation setup info */
+		if (global_msrp_show_setup_info)
+		{
+			show_setup_info(tvb, pinfo, msrp_tree);
+		}
+
 		/* Headers */
 		msrp_headers_item = proto_tree_add_item(msrp_tree, hf_msrp_msg_hdr, tvb, offset,(end_line_offset - offset), FALSE);
 		msrp_hdr_tree = proto_item_add_subtree(msrp_headers_item, ett_msrp_hdr);
@@ -633,6 +789,7 @@
 		&ett_msrp_element,
 		&ett_msrp_data,
 		&ett_msrp_end_line,
+		&ett_msrp_setup
 	};
 
         /* Setup list of header fields */
@@ -752,6 +909,21 @@
 			FT_STRING, BASE_NONE,NULL,0x0,
 			"Authentication-Info", HFILL }
 		},
+		{ &hf_msrp_setup,
+			{ "Stream setup", "msrp.setup",
+			FT_STRING, BASE_NONE, NULL, 0x0,
+			"Stream setup, method and frame number", HFILL}
+		},
+		{ &hf_msrp_setup_frame,
+			{ "Setup frame", "msrp.setup-frame",
+			FT_FRAMENUM, BASE_NONE, NULL, 0x0,
+			"Frame that set up this stream", HFILL}
+		},
+		{ &hf_msrp_setup_method,
+			{ "Setup Method", "msrp.setup-method",
+			FT_STRING, BASE_NONE, NULL, 0x0,
+			"Method used to set up this stream", HFILL}
+		},
 	};
 
 	module_t *msrp_module;
@@ -772,7 +944,14 @@
 		"MSRP message should be displayed "
 		"in addition to the dissection tree",
 		&global_msrp_raw_text);
-		
+
+	prefs_register_bool_preference(msrp_module, "show_setup_info",
+		"Show stream setup information",
+		"Where available, show which protocol and frame caused "
+		"this MSRP stream to be created",
+		&global_msrp_show_setup_info);
+
+
 	/*
 	 * Register the dissector by name, so other dissectors can
 	 * grab it by name rather than just referring to it directly.

Index: epan/dissectors/packet-sdp.c
===================================================================
--- epan/dissectors/packet-sdp.c	(revision 18787)
+++ epan/dissectors/packet-sdp.c	(working copy)
@@ -64,13 +64,13 @@
 #include <epan/prefs.h>
 
 #include "packet-rtcp.h"
-
 #include "packet-t38.h"
+#include "packet-msrp.h"
 
 static dissector_handle_t rtp_handle=NULL;
 static dissector_handle_t rtcp_handle=NULL;
-
 static dissector_handle_t t38_handle=NULL;
+static dissector_handle_t msrp_handle=NULL;
 
 static int sdp_tap = -1;
 
@@ -190,6 +190,13 @@
   gint8 media_count;
 } transport_info_t;
 
+
+/* MSRP transport info (as set while parsing path attribute) */
+static gboolean msrp_transport_address_set = FALSE;
+static guint32  msrp_ipaddr[4];
+static guint16  msrp_port_number;
+
+
 /* static functions */
 
 static void call_sdp_subdissector(tvbuff_t *tvb, int hf, proto_tree* ti,
@@ -232,6 +239,7 @@
   guint32     port=0;
   gboolean    is_rtp=FALSE;
   gboolean    is_t38=FALSE;
+  gboolean    is_msrp=FALSE;
   gboolean    set_rtp=FALSE;
   gboolean    is_ipv4_addr=FALSE;
   gboolean    is_ipv6_addr=FALSE;
@@ -397,14 +405,19 @@
     }
     if(transport_info.media_proto[n]!=NULL) {
       /* Check if media protocol is RTP
-	   * and stream decoding is enabled in preferences 
-	   */
-		if(global_sdp_establish_conversation){
-			is_rtp = (strcmp(transport_info.media_proto[n],"RTP/AVP")==0);
-			/* Check if media protocol is T38 */
-			is_t38 = ( (strcmp(transport_info.media_proto[n],"UDPTL")==0) || (strcmp(transport_info.media_proto[n],"udptl")==0) );
-		}
+       * and stream decoding is enabled in preferences 
+       */
+       if(global_sdp_establish_conversation){
+            /* Check if media protocol is RTP */
+            is_rtp = (strcmp(transport_info.media_proto[n],"RTP/AVP")==0);
+            /* Check if media protocol is T38 */
+            is_t38 = ( (strcmp(transport_info.media_proto[n],"UDPTL")==0) || (strcmp(transport_info.media_proto[n],"udptl")==0) );
+            /* Check if media protocol is MSRP/TCP */
+            is_msrp = (strcmp(transport_info.media_proto[n],"msrp/tcp")==0);
+       }
     }
+    
+
     if(transport_info.connection_address!=NULL) {
       if(transport_info.connection_type!=NULL) {
         if (strcmp(transport_info.connection_type,"IP4")==0) {
@@ -438,16 +451,29 @@
         port++;
         rtcp_add_address(pinfo, &src_addr, port, 0, "SDP", pinfo->fd->num);
       }
-    } 
-      
+    }
+
     /* Add t38 conversation, if available and only if no rtp */
     if((!pinfo->fd->flags.visited) && port!=0 && !set_rtp && is_t38 && is_ipv4_addr){
       src_addr.data=(char *)&ipaddr;
       if(t38_handle){
         t38_add_address(pinfo, &src_addr, port, 0, "SDP", pinfo->fd->num);
-      }  
+      }
     }
 
+    /* Add MSRP conversation.  Uses addresses discovered in attribute
+       rather than connection information of media session line */
+    if (is_msrp ){
+        if ((!pinfo->fd->flags.visited) && msrp_transport_address_set){
+            if(msrp_handle){
+                src_addr.type=AT_IPv4;
+                src_addr.len=4;
+                src_addr.data=(char *)&msrp_ipaddr;
+                msrp_add_address(pinfo, &src_addr, msrp_port_number, "SDP", pinfo->fd->num);
+            }
+        }
+    }
+
     /* Create the RTP summary str for the Voip Call analysis */
     for (i = 0; i < transport_info.media[n].pt_count; i++)
     {
@@ -879,6 +905,9 @@
   next_offset = 0;
   tokenlen = 0;
 
+  /* Re-initialise for a new media description */
+  msrp_transport_address_set = FALSE;
+
   sdp_media_tree = proto_item_add_subtree(ti,ett_sdp_media);
 
   next_offset = tvb_find_guint8(tvb,offset, -1, ' ');
@@ -904,8 +933,8 @@
     /* Save port info */
     transport_info->media_port[transport_info->media_count] = tvb_get_ephemeral_string(tvb, offset, tokenlen);
 
-    proto_tree_add_item(sdp_media_tree, hf_media_port, tvb, offset, tokenlen,
-                        FALSE);
+    proto_tree_add_uint(sdp_media_tree, hf_media_port, tvb, offset, tokenlen,
+                        atoi(tvb_get_string(tvb, offset, tokenlen)));
     offset = next_offset + 1;
     next_offset = tvb_find_guint8(tvb,offset, -1, ' ');
     if(next_offset == -1)
@@ -924,8 +953,8 @@
     transport_info->media_port[transport_info->media_count] = tvb_get_ephemeral_string(tvb, offset, tokenlen);
 
     /* XXX Remember Port */
-    proto_tree_add_item(sdp_media_tree, hf_media_port, tvb, offset, tokenlen,
-                        FALSE);
+    proto_tree_add_uint(sdp_media_tree, hf_media_port, tvb, offset, tokenlen,
+                        atoi(tvb_get_string(tvb, offset, tokenlen)));
     offset = next_offset + 1;
   }
 
@@ -1132,30 +1161,39 @@
   gint offset, next_offset, tokenlen, n;
   guint8 *field_name;
   guint8 *payload_type;
+  guint8 *attribute_value;
   gint   *key;
 
   offset = 0;
   next_offset = 0;
   tokenlen = 0;
 
+  /* Create attribute tree */
   sdp_media_attribute_tree = proto_item_add_subtree(ti,
                                                     ett_sdp_media_attribute);
-
+  /* Find end of field */
   next_offset = tvb_find_guint8(tvb,offset,-1,':');
 
   if(next_offset == -1)
     return;
 
+  /* Attribute field name is token before ':' */
   tokenlen = next_offset - offset;
-
   proto_tree_add_item(sdp_media_attribute_tree,
                       hf_media_attribute_field,
                       tvb, offset, tokenlen, FALSE);
-
   field_name = tvb_get_ephemeral_string(tvb, offset, tokenlen);
 
+  /* Skip colon */
   offset = next_offset + 1;
 
+  /* Value is the remainder of the line */
+  attribute_value = tvb_get_string(tvb, offset, tvb_length_remaining(tvb, offset));
+
+
+  /*********************************************/
+  /* Special parsing for some field name types */
+  
   /* decode the rtpmap to see if it is DynamicPayload to dissect them automatic */
   if (strcmp(field_name, "rtpmap") == 0) {
 
@@ -1234,6 +1272,7 @@
 
         return;
   }
+
   if (strcmp(field_name, "fmtp") == 0) {
     proto_item *fmtp_item, *media_format_item;
     proto_tree *fmtp_tree;
@@ -1259,8 +1298,8 @@
     offset = next_offset + 1;
 
     /* There may be 2 parameters given
-	 * TODO: Handle arbitarry number of parameters.
-	 */
+     * TODO: Handle arbitary number of parameters.
+     */
     next_offset = tvb_find_guint8(tvb,offset,-1,';');
 
     if(next_offset != -1){
@@ -1292,6 +1331,33 @@
     return;
   }
 
+  /* msrp attributes that contain address needed for conversation */
+  if (strcmp(field_name, "path") == 0) {
+    const char *msrp_res = "msrp://";
+    if (strncmp(attribute_value, msrp_res, strlen(msrp_res)) == 0){
+      int address_offset, port_offset, port_end_offset;
+
+      /* Address starts here */
+      address_offset = offset + strlen(msrp_res);
+
+      /* Port is after next ':' */
+      port_offset = tvb_find_guint8(tvb, address_offset, -1, ':');
+
+      /* Port ends with '/' */
+      port_end_offset = tvb_find_guint8(tvb, port_offset, -1, '/');
+      
+      /* Attempt to convert address */
+      if (inet_pton(AF_INET, tvb_get_ephemeral_string(tvb, address_offset, port_offset-address_offset), &msrp_ipaddr) > 0) {
+        /* Get port number */
+        msrp_port_number = atoi(tvb_get_ephemeral_string(tvb, port_offset+1, port_end_offset-port_offset-1));
+
+        /* Set flag so this info can be used */
+        msrp_transport_address_set = TRUE;
+      }
+    }
+  }
+
+
   /* No special treatment for values of this attribute type, just add as one item. */
   proto_tree_add_item(sdp_media_attribute_tree, hf_media_attribute_value,
                       tvb, offset, -1, FALSE);
@@ -1479,7 +1545,7 @@
         "Media Type", HFILL }},
     { &hf_media_port,
       { "Media Port",
-        "sdp.media.port",FT_STRING, BASE_NONE, NULL, 0x0,
+        "sdp.media.port",FT_UINT16, BASE_DEC, NULL, 0x0,
         "Media Port", HFILL }},
     { &hf_media_portcount,
       { "Media Port Count",
@@ -1549,8 +1615,8 @@
    */
    sdp_module = prefs_register_protocol(proto_sdp, NULL);
    prefs_register_bool_preference(sdp_module, "establish_conversation",
-       "Establish RTP Conversation",
-       "Specifies that RTP stream is decoded based "
+       "Establish Media Conversation",
+       "Specifies that RTP/RTCP/T.38/MSRP/etc streams are decoded based "
        "upon port numbers found in SIP/SDP payload",
        &global_sdp_establish_conversation);
  
@@ -1571,7 +1637,7 @@
 
   rtp_handle = find_dissector("rtp");
   rtcp_handle = find_dissector("rtcp");
-
+  msrp_handle = find_dissector("msrp");
   t38_handle = find_dissector("t38");
 
   sdp_handle = find_dissector("sdp");

/* packet-msrp.h
 *
 * $Id: packet-rtcp.h 18196 2006-05-21 04:49:01Z sahlberg $
 *
 * Wireshark - Network traffic analyzer
 * By Gerald Combs <gerald@xxxxxxxxxxxxx>
 * Copyright 1998 Gerald Combs
 *
 * This program is free software; you can redistribute it and/or
 * modify it under the terms of the GNU General Public License
 * as published by the Free Software Foundation; either version 2
 * of the License, or (at your option) any later version.
 *
 * This program is distributed in the hope that it will be useful,
 * but WITHOUT ANY WARRANTY; without even the implied warranty of
 * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
 * GNU General Public License for more details.
 *
 * You should have received a copy of the GNU General Public License
 * along with this program; if not, write to the Free Software
 * Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA  02111-1307, USA.
 */

/* Info to save in MSRP conversation / packet-info. */
#define MAX_MSRP_SETUP_METHOD_SIZE 7
struct _msrp_conversation_info
{
    guchar  setup_method_set;
    gchar   setup_method[MAX_MSRP_SETUP_METHOD_SIZE + 1];
    guint32 setup_frame_number;
};


/* Add an MSRP conversation with the given details */
void msrp_add_address(packet_info *pinfo,
                      address *addr, int port,
                      const gchar *setup_method, guint32 setup_frame_number);