Re: [Wireshark-dev] Failing to get my tree to show - problem solved, but I don't understand why
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
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
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
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
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
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
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