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