Re: [Wireshark-dev] Failing to get my tree to show - problem solved, but I don't understand why

2010-01-21 Thread Stephen Fisher

On Jan 21, 2010, at 2:48 PM, Kaul wrote:

> 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).

I haven't looked at your code, but here is an explanation of my thinking when I 
wrote that tracking code in the VNC dissector a while back:

 - First of all, the VNC protocol messages are usually identified by a message 
type field in the packet.  However, the messages that are exchanged at the 
beginning of a VNC session are not, which is where the conversation tracking 
comes in.  It isn't perfect though, because the Wireshark capture could start 
at any packet in the startup or after the startup messages.  That's why EVERY 
packet should have a type in it :-).

 - In the VNC dissector, the if(tree) checks are almost all gone now except for 
creating the main VNC protocol subtree.  This is because so much has to be done 
on each packet, even if it isn't displayed, to keep track of the session state. 
 A while back, I started putting if(tree) all over the place and it was getting 
ugly.

 - When Wireshark loads the VNC packets from disk/network and displays each one 
on the screen, the dissector (not have any if tree checks really) tracks the 
state that it expects the packets to be in (and does some sanity checks to see 
if what is there is what was expected).  This state is tracked between packets 
with per conversation data structures.  The state information, after being 
sanity checked, is then marked in each packet's data structures.  That way, the 
user can click on any packet in any order and the VNC dissector will first 
check to see that the per packet data has been stored and use that to determine 
what state that packet belongs to.  If we didn't do that, then the conversation 
state would keep advancing every time a packet was clicked on.  This would be 
fine if the user clicked on the packets sequentially starting at the beginning 
and not skipping any.  Most users don't do that though (and it wouldn't be very 
useful anyway in the GUI at least) :)

Hope this helps.


Steve

___
Sent via:Wireshark-dev mailing list 
Archives:http://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev
 mailto:wireshark-dev-requ...@wireshark.org?subject=unsubscribe


Re: [Wireshark-dev] Failing to get my tree to show - problem solved, but I don't understand why

2010-01-21 Thread Kaul
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  wrote:

>
>
> On Tue, Jan 19, 2010 at 1:09 AM, Guy Harris  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 
>> Archives:http://www.wireshark.org/lists/wireshark-dev
>> Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev
>> mailto:wireshark-dev-requ...@wireshark.org
>> ?subject=unsubscribe
>>
>
>
/* packet-spice.c
 * Routines for Spice protocol dissection
 * Copyright 2010, Yaniv Kaul 
 *
 * Wireshark - Network traffic analyzer
 * By Gerald Combs 
 * 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 
#include 
#include 
#include 

#include 
#include 
#include 
#include 
#include 

#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

Re: [Wireshark-dev] Failing to get my tree to show

2010-01-20 Thread Kaul
On Tue, Jan 19, 2010 at 1:09 AM, Guy Harris  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 
> Archives:http://www.wireshark.org/lists/wireshark-dev
> Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev
> mailto:wireshark-dev-requ...@wireshark.org?subject=unsubscribe
>
/* packet-spice.c
 * Routines for Spice protocol dissection
 * Copyright 2010, Yaniv Kaul 
 *
 * Wireshark - Network traffic analyzer
 * By Gerald Combs 
 * 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 
#include 
#include 
#include 

#include 
#include 
#include 
#include 
#include 

#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 SPICE_CHANNEL_NONE 0
#define SPICE_CHANNEL_MAIN 1
#define SPICE_CHANNEL_DISPLAY 2
#define SPICE_CHANNEL_INPUTS 3 
#define SPICE_CHANNEL_CURSOR 4
#define SPICE_CHANNEL_PLAYBACK 5
#define SPICE_CHANNEL_RECORD 6

#define SPICE_STATE_LINK_CLIENT 0
#define SPICE_STATE_LINK_SERVER 1
#define SPICE_STATE_TICKET_CLIENT 2
#define SPICE_STATE_TICKET_SERVER 3
#define SPICE_STATE_DATA 4

#define TCP_PORT_SPICE  5910

static dissector_handle_t spice_handle;

static const value_string channel_types_vs[] = {
{ 0, "Invalid"   },
{ 1, "Main"  },
{ 2, "Display"   },
{ 3, "Inputs"},
{ 4, "Cursor"},
{ 5, "Playback"  },
{ 6, "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;
guint8 channel_type;
guint8 channel_id;
guint8 next_state;
} spice_conversation_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 (

Re: [Wireshark-dev] Failing to get my tree to show

2010-01-18 Thread Guy Harris

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.
___
Sent via:Wireshark-dev mailing list 
Archives:http://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev
 mailto:wireshark-dev-requ...@wireshark.org?subject=unsubscribe


Re: [Wireshark-dev] Failing to get my tree to show

2010-01-16 Thread Kaul
On Sat, Jan 16, 2010 at 4:30 AM, Guy Harris  wrote:

>
> On Jan 15, 2010, at 1:50 PM, Kaul wrote:
>
> > Hi,
> >
> > I'm trying to write a new dissector, and failing miserably getting my
> tree to show, because the tree I'm getting in my dissect_PROTONAME() is
> always NULL, not sure why.
>
> Null even if you click on the packet?
>

Nope.


>
> > I'm dissecting over TCP, with (regretfully) my own desegmentation:
>
> By "my own desegmentation" I assume you mean "my own code to get the TCP
> dissector to reassemble stuff, rather than using tcp_dissect_pdus() or
> req_resp_hdrs_do_reassembly()", given that you refer to setting
> pinfo->desegment_len.
>

Indeed. I cannot use either of the above mentioned methods.


>
> > packets 1-3 are syn, syn-ack, ack.
> > packet 4 is a start of a PDU, which is not enough to dissect the PDU,
> although I'm a getting its header. From the header, I'm taking the complete
> PDU length and therefore setting pinfo->desegment_len to calculated PDU
> length - length of what I got already ( with the offset = 0).
> > This looks nice and correct and indeed seems to be desegmented correctly,
> BUT:
> > packet 4 has my COL_PROTOCOL set (why?, I didn't dissect it eventually)
>
> You're setting COL_PROTOCOL before you're doing
>
> > if (len < pdu_len && redc_desegment) { /* Did not get all
> the PDU - request the full length of the PDU */
> > pinfo->desegment_offset = 0;
> > pinfo->desegment_len = pdu_len - len;
> > return len;
> >  }
>
> so you did enough dissection to set the column.
>

Even if I'm not setting right away (others do it as well - I've taken it
from packet-vnc.c), then it's even worse - all I see is the TCP.


>
> The BGP dissector works the same way; one could argue that it shouldn't.
>
> > packet 5 doesn't (correct, I've asked for more than it has - it just a
> TCP segment)
> > packet 6 has my COL_PROTOCOL set (good) - but the packet isn't dissected
> there, although now I have the complete data (and TCP desgmentation shows
> the data is indeed taken from packets 4, 5 ,6 correctly.
>
> OK, presumably you know it has COL_PROTOCOL set because row 6 of the packet
> list has "Spice" in the Protocol column; do you know that it's not dissected
> because, when you click on that row, you don't see a protocol tree for your
> protocol?
>

Exactly. It's not that it's not dissected - all the right functions are
called. But with tree = null, not much is seen.
I can see that TCP correctly shows the 3 segments in packet 6 (that it
reassembled from packets 4,5,6).



>
> > I do know wireshark has two modes, one of which it goes over packets
> without the tree set, but I don't get when and where.
>
> I wouldn't describe them as "modes"; it's more like "Wireshark generates a
> protocol tree if it knows it will need one, and in some cases, perhaps,
> where it doesn't, but doesn't really know that it doesn't".  There are no
> "modes" such that a guarantee is made that, in certain cases, you will be
> handed a tree and, in other cases, you won't, only that if the tree is going
> to be displayed or printed you will be handed a tree (if not, that's
> obviously a bug).
>

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


>
> There's also a bit of a hack that attempts to prevent the entire protocol
> tree from being generated when it's *not* going to be displayed or printed
> and where some fields *aren't* needed, and some dissectors that, for
> example, use proto_item_append_string() have to work around - see change
> r31460, for example.
>

Again, I think my main issue is that my main dissector function is already
called with a null tree. That makes everything afterwards pretty much fail
to display.
I did notice it doesn't happen without the desegmentation stuff - so I'm
pretty sure it's related to that. Just have no idea why.
Would posting the complete code help?
TIA,
Y.

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

Re: [Wireshark-dev] Failing to get my tree to show

2010-01-15 Thread Guy Harris

On Jan 15, 2010, at 1:50 PM, Kaul wrote:

> Hi,
> 
> I'm trying to write a new dissector, and failing miserably getting my tree to 
> show, because the tree I'm getting in my dissect_PROTONAME() is always NULL, 
> not sure why.

Null even if you click on the packet?

> I'm dissecting over TCP, with (regretfully) my own desegmentation:

By "my own desegmentation" I assume you mean "my own code to get the TCP 
dissector to reassemble stuff, rather than using tcp_dissect_pdus() or 
req_resp_hdrs_do_reassembly()", given that you refer to setting 
pinfo->desegment_len.

> packets 1-3 are syn, syn-ack, ack.
> packet 4 is a start of a PDU, which is not enough to dissect the PDU, 
> although I'm a getting its header. From the header, I'm taking the complete 
> PDU length and therefore setting pinfo->desegment_len to calculated PDU 
> length - length of what I got already ( with the offset = 0).
> This looks nice and correct and indeed seems to be desegmented correctly, BUT:
> packet 4 has my COL_PROTOCOL set (why?, I didn't dissect it eventually)

You're setting COL_PROTOCOL before you're doing

> if (len < pdu_len && redc_desegment) { /* Did not get all the 
> PDU - request the full length of the PDU */
> pinfo->desegment_offset = 0;
> pinfo->desegment_len = pdu_len - len;
> return len;
>  }

so you did enough dissection to set the column.

The BGP dissector works the same way; one could argue that it shouldn't.

> packet 5 doesn't (correct, I've asked for more than it has - it just a TCP 
> segment)
> packet 6 has my COL_PROTOCOL set (good) - but the packet isn't dissected 
> there, although now I have the complete data (and TCP desgmentation shows the 
> data is indeed taken from packets 4, 5 ,6 correctly.

OK, presumably you know it has COL_PROTOCOL set because row 6 of the packet 
list has "Spice" in the Protocol column; do you know that it's not dissected 
because, when you click on that row, you don't see a protocol tree for your 
protocol?

> I do know wireshark has two modes, one of which it goes over packets without 
> the tree set, but I don't get when and where.

I wouldn't describe them as "modes"; it's more like "Wireshark generates a 
protocol tree if it knows it will need one, and in some cases, perhaps, where 
it doesn't, but doesn't really know that it doesn't".  There are no "modes" 
such that a guarantee is made that, in certain cases, you will be handed a tree 
and, in other cases, you won't, only that if the tree is going to be displayed 
or printed you will be handed a tree (if not, that's obviously a bug).

There's also a bit of a hack that attempts to prevent the entire protocol tree 
from being generated when it's *not* going to be displayed or printed and where 
some fields *aren't* needed, and some dissectors that, for example, use 
proto_item_append_string() have to work around - see change r31460, for example.
___
Sent via:Wireshark-dev mailing list 
Archives:http://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev
 mailto:wireshark-dev-requ...@wireshark.org?subject=unsubscribe


[Wireshark-dev] Failing to get my tree to show

2010-01-15 Thread Kaul
Hi,

I'm trying to write a new dissector, and failing miserably getting my tree
to show, because the tree I'm getting in my dissect_PROTONAME() is always
NULL, not sure why.
I'm dissecting over TCP, with (regretfully) my own desegmentation:
packets 1-3 are syn, syn-ack, ack.
packet 4 is a start of a PDU, which is not enough to dissect the PDU,
although I'm a getting its header. From the header, I'm taking the complete
PDU length and therefore setting pinfo->desegment_len to calculated PDU
length - length of what I got already ( with the offset = 0).
This looks nice and correct and indeed seems to be desegmented correctly,
BUT:
packet 4 has my COL_PROTOCOL set (why?, I didn't dissect it eventually)
packet 5 doesn't (correct, I've asked for more than it has - it just a TCP
segment)
packet 6 has my COL_PROTOCOL set (good) - but the packet isn't dissected
there, although now I have the complete data (and TCP desgmentation shows
the data is indeed taken from packets 4, 5 ,6 correctly.

This is part of my dissection:

...< get conversation data and state of the protocol>

col_set_str(pinfo->cinfo, COL_PROTOCOL, "Spice");

col_clear(pinfo->cinfo, COL_INFO);

if (tree) { /* WHY IS TREE ALWAYS NULL HERE?! */
ti = proto_tree_add_item(tree, proto_spice, tvb, 0, -1, FALSE);
spice_tree = proto_item_add_subtree(ti, ett_spice);
}

switch (spice_info->next_state) {
case RED_STATE_LINK_CLIENT:
len = tvb_reported_length(tvb);
if (len < 16 && redc_desegment) { /* the header is at least
16 bytes long */
pinfo->desegment_offset = 0;
pinfo->desegment_len = 16 - len;
return len;
}
pdu_len = tvb_get_letohl(tvb, 12) + 16;
if (len < pdu_len && redc_desegment) { /* Did not get all
the PDU - request the full length of the PDU */
pinfo->desegment_offset = 0;
pinfo->desegment_len = pdu_len - len;
return len;
 }
col_set_str(pinfo->cinfo, COL_INFO,
"RED_STATE_LINK_CLIENT");
dissect_spice_link_client_pdu(tvb, pinfo, spice_tree,
spice_info);
spice_info->next_state = RED_STATE_LINK_SERVER;
break;
...


I must be missing something obvious here. I just fail to understand what it
is. I do know wireshark has two modes, one of which it goes over packets
without the tree set, but I don't get when and where.
I've looked at other dissectors and they seem to be doing identical/similar
dissection (wrongfully setting the protocol to their own even on segments,
btw?).
Thanks in advance,
Yaniv.
___
Sent via:Wireshark-dev mailing list 
Archives:http://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev
 mailto:wireshark-dev-requ...@wireshark.org?subject=unsubscribe