[Kaul <mykaul@xxxxxxxxx>]

Well, I solved the problem - but I still don't get why it's working. I've copied what packet-vnc.c does:
After getting the conversation object, get the per-packet info. If none exist, I create one and copy the protocol conversation state machine to it. Then, I act upon the state *from the packet info*. Everything works beautifully afterwards (attached changed code - mainly the addition in lines 290-297 - which fetch the per packet information and use it.)

I'd still be happy to understand why this works now (also as a lesson for others).
Y.

On Wed, Jan 20, 2010 at 11:15 PM, Kaul <mykaul@xxxxxxxxx> wrote:

On Tue, Jan 19, 2010 at 1:09 AM, Guy Harris <guy@xxxxxxxxxxxx> wrote:

On Jan 16, 2010, at 10:39 AM, Kaul wrote:

> From README.developer:
> "Wireshark distinguishes between the 2 modes with the proto_tree pointer"

I'll look at rewriting that to clarify that they're not modes of operation of Wireshark, and that one must not make assumptions about when you'll be called with, or without, a protocol tree (other than "if Wireshark needs the entire tree, for whatever reason that might be, it'll pass a non-null pointer; otherwise, it might be null or it might be non-null, don't depend on either one").

> Would posting the complete code help?

Probably.

Thank you - attached.
The main dissectors starts at dissect_spice(), and relevant code in line 283 and on.

Thanks in advance,
Yaniv.
 

___________________________________________________________________________
Sent via:    Wireshark-dev mailing list <wireshark-dev@xxxxxxxxxxxxx>
Archives:    http://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev
            mailto:wireshark-dev-request@xxxxxxxxxxxxx?subject=unsubscribe

/* packet-spice.c
 * Routines for Spice protocol dissection
 * Copyright 2010, Yaniv Kaul <ykaul@xxxxxxxxxxxx>
 *
 * Wireshark - Network traffic analyzer
 * By Gerald Combs <gerald@xxxxxxxxxxxxx>
 * Copyright 1998 Gerald Combs
 *
 * This program is free software; you can spiceistribute 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.
 *
 * This code is based on the protocol specification:
 *   http://www.spice-space.org/docs/spice_protocol.pdf
 */

#ifdef HAVE_CONFIG_H
# include "config.h"
#endif

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <glib.h>

#include <epan/conversation.h>
#include <epan/dissectors/packet-tcp.h>
#include <epan/emem.h>
#include <epan/packet.h>
#include <epan/prefs.h>

#define DBG2(format, arg1, arg2)		fprintf(stderr, format, arg1, arg2)

#define SPICE_MAGIC 0x52454451 /* = "REDQ" */
#define SPICE_VERSION_MAJOR_1 1
#define SPICE_VERSION_MINOR_0 0

#define SPICE_TICKET_PUBKEY_BYTES 162

#define CHANNEL_NONE 0
#define CHANNEL_MAIN 1
#define CHANNEL_DISPLAY 2
#define CHANNEL_INPUTS 3 
#define CHANNEL_CURSOR 4
#define CHANNEL_PLAYBACK 5
#define CHANNEL_RECORD 6

typedef enum {
	LINK_CLIENT,
	LINK_SERVER,
	
	TICKET_CLIENT,
	TICKET_SERVER,
	
	DATA
} spice_session_state_e;
	
#define TCP_PORT_SPICE	5910

static dissector_handle_t spice_handle;

static const value_string channel_types_vs[] = {
	{ CHANNEL_NONE, "Invalid"   },
	{ CHANNEL_MAIN, "Main"      },
	{ CHANNEL_DISPLAY, "Display"   },
	{ CHANNEL_INPUTS, "Inputs"    },
	{ CHANNEL_CURSOR, "Cursor"    },
	{ CHANNEL_PLAYBACK, "Playback"  },
	{ CHANNEL_RECORD, "Record"    },
	{ 0,  NULL       }
};

static const value_string error_codes_vs[] = {
	{ 0, "SPICE_ERROR_OK"                     },
	{ 1, "SPICE_ERROR_ERROR"                  },
	{ 2, "SPICE_ERROR_INVALID_MAGIC"          },
	{ 3, "SPICE_ERROR_INVALID_DATA"           },
	{ 4, "SPICE_ERROR_VERSION_MISMATCH"       },
	{ 5, "SPICE_ERROR_NEED_SECUSPICE"           },
	{ 6, "SPICE_ERROR_NEED_UNSECUSPICE"         },
	{ 7, "SPICE_ERROR_PERMISSION_DENIED"      },
	{ 8, "SPICE_ERROR_BAD_CONNECTION_ID"      },
	{ 9, "SPICE_ERROR_CHANNEL_NOT_AVAILABLE"  },
	{ 0,  NULL       }
};

/* This structure will be tied to each conversation. */
typedef struct {
	guint32 connection_id;
	guint32 num_channel_caps;
	guint32 destport;
	spice_session_state_e next_state;
	guint8 channel_type;
	guint8 channel_id;
} spice_conversation_t;

typedef struct {
	spice_session_state_e state;
} spice_packet_t;

/* desegmentation of spice protocol */
static gboolean spice_desegment = TRUE;

/* Variables for our preferences */
static guint spice_preference_alternate_port = 0;

static gint ett_spice = -1;
static gint ett_link_client = -1;
static gint ett_link_server = -1;

static int proto_spice = -1;
static int hf_spice_magic  = -1;
static int hf_major_version  = -1;
static int hf_minor_version  = -1;
static int hf_message_size  = -1;
static int hf_conn_id  = -1;
static int hf_channel_type  = -1;
static int hf_channel_id  = -1;
static int hf_num_common_caps  = -1;
static int hf_num_channel_caps  = -1;
static int hf_caps_offset  = -1;
static int hf_error_code  = -1;
static int hf_serial = -1;
static int hf_link_client = -1;
static int hf_link_server = -1;


static void
dissect_spice_link_common_header(tvbuff_t *tvb, proto_tree *tree)
{
 	if (tree) {
		/* dissect common header */
		proto_tree_add_item(tree, hf_spice_magic, tvb, 0, 4, FALSE);
		proto_tree_add_item(tree, hf_major_version, tvb, 4, 4, TRUE);
		proto_tree_add_item(tree, hf_minor_version, tvb, 8, 4, TRUE);
		proto_tree_add_item(tree, hf_message_size, tvb, 12, 4, TRUE);
	}
}

static void
set_spice_link_channel_type(packet_info *pinfo, const guint8 channel_type)
{
	switch (channel_type) {
		case CHANNEL_MAIN:
			col_append_str(pinfo->cinfo, COL_INFO, "SPICE_CHANNEL_MAIN");
		break;
		case CHANNEL_DISPLAY:
			col_append_str(pinfo->cinfo, COL_INFO, "SPICE_CHANNEL_DISPLAY");
		break;
		case CHANNEL_INPUTS:
			col_append_str(pinfo->cinfo, COL_INFO, "SPICE_CHANNEL_INPUTS");
		break;
		case CHANNEL_CURSOR:
			col_append_str(pinfo->cinfo, COL_INFO, "SPICE_CHANNEL_CURSOR");
		break;
		case CHANNEL_PLAYBACK:
			col_append_str(pinfo->cinfo, COL_INFO, "SPICE_CHANNEL_PLAYBACK");
		break;
		case CHANNEL_RECORD:
			col_append_str(pinfo->cinfo, COL_INFO, "SPICE_CHANNEL_RECORD");
		break;
		default:
			col_append_fstr(pinfo->cinfo, COL_INFO, "UNKNOWN SPICE CHANNEL: %d", channel_type);
		break;
	}
}

static void
dissect_spice_link_client_pdu(tvbuff_t *tvb, packet_info *pinfo, proto_tree *spice_tree, spice_conversation_t *spice_info)
{

	int offset;
	guint32 caps_len;
	proto_item *ti=NULL;
	proto_tree *link_tree;
	
 	DBG2("dissect_spice_link_pdu. packet: %d, length %d\r\n", pinfo->fd->num, tvb_reported_length(tvb));
 		
 	if (spice_tree) {
		ti = proto_tree_add_item(spice_tree, hf_link_client, tvb, 0, -1, FALSE);
		link_tree = proto_item_add_subtree(ti, ett_link_client);
	}
 	dissect_spice_link_common_header(tvb, link_tree);
	offset = 16;
	return;
	if (spice_tree) {
		proto_tree_add_item(spice_tree, hf_conn_id, tvb, offset, 4, TRUE);
		proto_tree_add_item(spice_tree, hf_channel_type, tvb, offset + 4, 1, TRUE);
		proto_tree_add_item(spice_tree, hf_channel_id, tvb, offset + 5, 1, TRUE);

		proto_tree_add_item(spice_tree, hf_num_common_caps, tvb, offset + 6, 4, TRUE);
		proto_tree_add_item(spice_tree, hf_num_channel_caps, tvb, offset + 10, 4, TRUE);

		proto_tree_add_item(spice_tree, hf_caps_offset, tvb, offset + 14, 4, TRUE);
	}
	
	caps_len = tvb_get_letohl(tvb, offset + 6) + tvb_get_letohl(tvb, offset + 10);

	if (spice_info->channel_type == CHANNEL_NONE) {
		spice_info->channel_type = tvb_get_guint8(tvb, offset + 4);
	}
	offset += 18; /* LinkMess size = 4 + 1 + 1 + 4 + 4 + 4 */

	set_spice_link_channel_type(pinfo, spice_info->channel_type);
	if (caps_len > 0) {
		caps_len *= 4; /* capability length is in UINT32 units, therefore multiply by 4 */
		proto_tree_add_text(spice_tree, tvb, offset, caps_len, "Channel Capabilities (%d bytes)", caps_len);
	}
}

static void
dissect_spice_link_server_pdu(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree, spice_conversation_t *spice_info)
{

	int offset;
	guint32 caps_len;
	
 	dissect_spice_link_common_header(tvb, tree);
	
	offset = 16;
	
	if (tree) {
		proto_tree_add_item(tree, hf_error_code, tvb, offset, 4, TRUE);
		proto_tree_add_text(tree, tvb, offset + 4, SPICE_TICKET_PUBKEY_BYTES, "X.509 SubjectPublicKeyInfo");

		proto_tree_add_item(tree, hf_num_common_caps, tvb, offset + 4 + SPICE_TICKET_PUBKEY_BYTES, 4, TRUE);
		proto_tree_add_item(tree, hf_num_channel_caps, tvb, offset + 8 + SPICE_TICKET_PUBKEY_BYTES, 4, TRUE);

		proto_tree_add_item(tree, hf_caps_offset, tvb, offset + 12 + SPICE_TICKET_PUBKEY_BYTES, 4, TRUE);
	}
	
	caps_len = tvb_get_letohl(tvb, offset + 4 + SPICE_TICKET_PUBKEY_BYTES) + tvb_get_letohl(tvb, offset + 8 + SPICE_TICKET_PUBKEY_BYTES); 
	offset += 16 + SPICE_TICKET_PUBKEY_BYTES;

	set_spice_link_channel_type(pinfo, spice_info->channel_type);
	col_append_str(pinfo->cinfo, COL_INFO, " - SERVER");
	if (caps_len > 0) {
		caps_len *= 4; /* capability length is in UINT32 units, therefore multiply by 4 */
		proto_tree_add_text(tree, tvb, offset, caps_len, "Channel Capabilities (%d bytes)", caps_len);
	}
}

static int
dissect_spice(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree)
{

	conversation_t *conversation;
	spice_conversation_t *spice_info;
	spice_packet_t *per_packet_info;
	guint offset, pdu_len, len;
	
	/* Set up structures needed to add the protocol subtree and manage it */
	proto_item *ti=NULL;
	proto_tree *spice_tree=NULL;
	pdu_len = 0;
	

	conversation = find_conversation(pinfo->fd->num, &pinfo->src, &pinfo->dst,
									pinfo->ptype, pinfo->srcport,
									pinfo->destport, 0);
	if (!conversation) {
		conversation = conversation_new(pinfo->fd->num, &pinfo->src, &pinfo->dst,
										pinfo->ptype, pinfo->srcport,
										pinfo->destport, 0);
	}
	
	spice_info = (spice_conversation_t*)conversation_get_proto_data(conversation, proto_spice);
	if(!spice_info) {
		/* We don't yet have a conversation, so create one. */
		spice_info = se_alloc0(sizeof(spice_conversation_t));
		spice_info->destport = pinfo->destport;
		spice_info->channel_type = CHANNEL_NONE;
		spice_info->next_state = LINK_CLIENT;
		conversation_add_proto_data(conversation, proto_spice, spice_info);
	}

	/* Make entries in Protocol column and Info column on summary display */


	per_packet_info = p_get_proto_data(pinfo->fd, proto_spice);

	if(!per_packet_info) {
		per_packet_info = se_alloc(sizeof(spice_packet_t));

		per_packet_info->state = spice_info->next_state;

		p_add_proto_data(pinfo->fd, proto_spice, per_packet_info);
	} 
	switch (per_packet_info->state) {
		case LINK_CLIENT:
				len = tvb_reported_length(tvb);
				if (len < 16 && spice_desegment) { /* the header is at least 16 bytes long */
					DBG2("dissect_spice() SPICE_STATE_LINK_CLIENT - packet %d, len %d - HEADER TOO SHORT\r\n", pinfo->fd->num, len);
					pinfo->desegment_offset = 0;
					pinfo->desegment_len = 16 - len;
					return 0;
				}				
				pdu_len = tvb_get_letohl(tvb, 12) + 16;
				if (len < pdu_len && spice_desegment) { /* Did not get all the PDU - request the full length of the PDU */
					DBG2("dissect_spice() SPICE_STATE_LINK_CLIENT - packet %d, len %d - PDU incomplete\r\n", pinfo->fd->num, len);			
					pinfo->desegment_offset = 0;
					pinfo->desegment_len = pdu_len - len;
					return 0;
 				}
				col_set_str(pinfo->cinfo, COL_PROTOCOL, "Spice");
				col_set_str(pinfo->cinfo, COL_INFO, "SPICE_STATE_LINK_CLIENT");
				if (tree) {
					ti = proto_tree_add_item(tree, proto_spice, tvb, 0, -1, FALSE);
					spice_tree = proto_item_add_subtree(ti, ett_spice);
				}

				dissect_spice_link_client_pdu(tvb, pinfo, spice_tree, spice_info);
				spice_info->next_state = LINK_SERVER;
				break;
		case LINK_SERVER:
				len = tvb_reported_length(tvb);
				return len;
				DBG2("dissect_spice() SPICE_STATE_LINK_SERVER - packet %d, len %d\r\n", pinfo->fd->num, len);
				if (len < 16) {
					pinfo->desegment_offset = 0;
					pinfo->desegment_len = 16 - len;
					return len;
				}				
				pdu_len = tvb_get_letohl(tvb, offset + 12) + 16;
				if (tvb_reported_length(tvb) < pdu_len) {
					pinfo->desegment_offset = 0;
					pinfo->desegment_len = pdu_len - len;
					return pdu_len;
 				}
 				dissect_spice_link_server_pdu(tvb, pinfo, spice_tree, spice_info);
				spice_info->next_state = TICKET_CLIENT;
				break;
		default:
				DBG2("Got an unknown state in packet %d, state %d", pinfo->fd->num, spice_info->next_state);
				break;
	}
	return tvb_reported_length(tvb);
}

void
proto_reg_handoff_spice(void)
{
	static gboolean initialized = FALSE;
	static int currentPort = 0;

	if (!initialized) {
		spice_handle = new_create_dissector_handle(dissect_spice, proto_spice);
		dissector_add("tcp.port", TCP_PORT_SPICE, spice_handle);
		initialized = TRUE;
	}
	else {
		dissector_delete("tcp.port", currentPort, spice_handle);
	}

	currentPort = spice_preference_alternate_port;
	dissector_add("tcp.port", currentPort, spice_handle);
}

/* Register the protocol with Wireshark */
void
proto_register_spice(void)
{
  module_t *spice_module; /* To handle our preferences */

  /* Setup list of header fields */
  static hf_register_info hf[] = {
    { &hf_link_client,
      { "LINK client", "spice.link_client",
	FT_NONE, BASE_NONE, NULL, 0x0,
	"Spice link client", HFILL }
      },
    { &hf_link_server,
      { "LINK server", "spice.link_server",
	FT_NONE, BASE_NONE, NULL, 0x0,
	"Spice link server", HFILL }
      },
    { &hf_spice_magic,
      { "spice MAGIC", "spice.magic",
	FT_STRING, BASE_NONE, NULL, 0x0,
	"spice magic", HFILL }
      },
    { &hf_major_version,
      { "Protocol major version", "spice.major_version",
	FT_UINT32, BASE_DEC, NULL, 0x0,
	"spice protocol major version", HFILL }
      },
    { &hf_minor_version,
      { "Protocol minor version", "spice.minor_version",
	FT_UINT32, BASE_DEC, NULL, 0x0,
	"spice protocol minor version", HFILL }
      },
    { &hf_message_size,
      { "Message size", "spice.message_size",
	FT_UINT32, BASE_DEC, NULL, 0x0,
	"Message size", HFILL }
      },
    { &hf_conn_id,
      { "Session ID", "spice.conn_id",
	FT_UINT32, BASE_HEX, NULL, 0x0,
	"Session ID", HFILL }
      },
    { &hf_channel_type,
      { "Channel type", "spice.channel_type",
	FT_UINT8, BASE_DEC, NULL, 0x0,
	"spice channel type", HFILL }
      },
    { &hf_channel_id,
      { "Channel ID", "spice.channel_id",
	FT_UINT8, BASE_DEC, NULL, 0x0,
	"spice channel ID", HFILL }
      },
    { &hf_num_common_caps,
      { "spice NUM COMMON CAPS", "spice.num_common_caps",
	FT_UINT32, BASE_DEC, NULL, 0x0,
	"spice number of common caps", HFILL }
      },
    { &hf_num_channel_caps,
      { "spice NUM CHANNEL CAPS", "spice.num_channel_caps",
	FT_UINT32, BASE_DEC, NULL, 0x0,
	"spice number of channel caps", HFILL }
      },
    { &hf_caps_offset,
      { "spice CAPS OFFSET", "spice.caps_offset",
	FT_UINT32, BASE_DEC, NULL, 0x0,
	"spice CAPS offset", HFILL }
      },
    { &hf_error_code,
      { "spice ERROR", "spice.error_code",
	FT_UINT32, BASE_DEC, NULL, 0x0,
	"spice error code", HFILL }
      },
    { &hf_serial,
      { "spice SERIAL", "spice.serial",
	FT_UINT64, BASE_HEX, NULL, 0x0,
	"spice message serial number", HFILL }
      },
    };

  /* Setup protocol subtree arrays */
  static gint *ett[] = {
    &ett_spice,
    &ett_link_client,
    &ett_link_server
  };

  /* Register the protocol name and description */
  proto_spice = proto_register_protocol("Spice protocol",
                                       "Spice", "spice");

  /* Requispice function calls to register the header fields and subtrees */
  proto_register_field_array(proto_spice, hf, array_length(hf));
  proto_register_subtree_array(ett, array_length(ett));

  /* Register our preferences module */
  spice_module = prefs_register_protocol(proto_spice, proto_reg_handoff_spice);

  prefs_register_bool_preference(spice_module, "desegment", "Reassemble spice messages spanning multiple TCP segments.", "Whether the spice dissector should reassemble messages spanning multiple TCP segments.  To use this option, you must also enable \"Allow subdissectors to reassemble TCP streams\" in the TCP protocol settings.", &spice_desegment);

  /*prefs_register_uint_preference(spice_module, "alternate_port", "Alternate TCP port", "Decode this port's traffic as spice in addition to the default ports (5980)", 10, &spice_preference_alternate_port);*/
}