This is a static archive of our old Q&A Site. Please post any new questions and answers at ask.wireshark.org.

Custom dissector and info column

0

Hi,

I must admit I am very new to wireshark, but I have read a lot of material of the last few days and this problem has me stumped.

I have written a custom dissector plugin to handle an XCP protocol. This consists of PDUs on TCP and I am using the tcp_dissect_pdus function to handle the reassembly of TCP packets to allow successful dissecting of my PDUs.

I want to add to the info (COL_INFO) column in the GUI the packets contained in the TCP packet (e.g. Packets: 0x1234 - 0x1240). This works as expected except when the TCP packet is reassembled and then I just get the first PDU packet number. What is really strange is that the other PDUs are correctly dissected and added to the tree view at the bottom. I've tried all manner of things to work out what is going on, but figure I need some expert help! ;-)

static void dissect_xcp(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree)
{
     if (check_col(pinfo->cinfo, COL_PROTOCOL))
         col_set_str(pinfo->cinfo, COL_PROTOCOL, PROTO_TAG_XCP);

 /* Clear out stuff in the info column */
 if(check_col(pinfo->cinfo,COL_INFO)){
     col_clear(pinfo->cinfo,COL_INFO);
 }

 if (check_col(pinfo->cinfo, COL_INFO)) {
    col_add_fstr(pinfo->cinfo, COL_INFO, "%d > %d Packets:",
    pinfo->srcport, pinfo->destport);
 }

      tcp_dissect_pdus(tvb, pinfo, tree, TRUE, FRAME_HEADER_LEN,
             get_xcp_message_len, dissect_xcp_message);



}


static void dissect_xcp_message(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree)
{
proto_item *xcp_item = NULL;
proto_item *xcp_sub_item = NULL;
proto_tree *xcp_tree = NULL;
proto_tree *xcp_header_tree = NULL;
guint16 count = 0;
guint16 status = 0;
guint16 length = 0;
// This is not a good way of dissecting packets.  The tvb length should
// be sanity checked so we aren't going past the actual size of the buffer.
tvb_memcpy(tvb, (guint8 *)&length, offset, 2);


xcpcount = tvb_get_guint8( tvb, 2 ); // Get the xcp ccounter
xcpcount += (tvb_get_guint8( tvb, 3 ) * 256);


if (check_col(pinfo->cinfo, COL_INFO)) {
col_append_fstr(pinfo->cinfo, COL_INFO, "[0x%X] ",
xcpcount);
}


if (tree)
{
// If there is a "tree" requested, we handle that request.
guint16 counter = 0;
xcp_item = proto_tree_add_item(tree, proto_xcp, tvb, 0, -1, FALSE);
xcp_tree = proto_item_add_subtree(xcp_item, ett_xcp);
xcp_header_tree = proto_item_add_subtree(xcp_item, ett_xcp);


xcp_sub_item = proto_tree_add_item( xcp_tree, hf_xcp_header, tvb, offset, -1, FALSE );
xcp_header_tree = proto_item_add_subtree(xcp_sub_item, ett_xcp);

// Copy length data
tvb_memcpy(tvb, (guint8 *)&length, offset, 2);
proto_tree_add_uint(xcp_header_tree, hf_xcp_length, tvb, 0, 0, length);

tvb_memcpy(tvb, (guint8 *)&counter, offset, 2);
proto_tree_add_uint(xcp_header_tree, hf_xcp_counter, tvb, 0, 0, counter);



}

}

Any suggestions greatly appreciated!

I also want to add a PDU sequence check (i.e. packet numbers increase monotonically). This is what lead me to this problem in the first place, since I was getting failed sequence check on the reassembled TCP packets.

Thanks

asked 01 Aug ‘11, 14:34

Tupperware's gravatar image

Tupperware
1●1●1●3
accept rate: 0%

edited 01 Aug ‘11, 16:05

helloworld's gravatar image

helloworld
3.1k●4●20●41

4 Answers:

1

TBH your code in dissect_xcp_message() is a bit of a mess. You are unnecessarily calling tvb_memcpy() rather than using tvb_get_*(), and not really using the power of proto_tree_add_item() to do all the work for you. You should read the section of the developers guide regarding basic dissection here.

As to your problem, I can't see a declaration of offset. That should be declared and initialised in dissect_xcp_message(), so that dissection of each message starts from the same point in the tvb. You are also missing an increment of offset as you extract items from the tvb.

answered 01 Aug '11, 14:55

grahamb's gravatar image

grahamb ♦
19.8k●3●30●206
accept rate: 22%

And you don't need check_col() anymore.

(01 Aug '11, 19:19) cmaynard ♦♦

1

With reassembly, your dissector isn't going to be called until TCP has enough data to hand off to your dissector as informed by get_xcp_message_len(). This means all the TCP fragments (except the last one) are just that - TCP fragments and they are not handed off to your dissector until reassembly is complete. You don't want to do any column manipulation in dissect_xcp(); save it for dissect_xcp_message().

You may want to re-read README.developer, particularly section 2.7, again. You may also want to examine some of the dissectors that utilize tcp_dissect_pdus(), such as the one mentioned in README.developer, namely packet-dns.c. Notice the function that calls tcp_dissect_pdus() and how simple it is - it ONLY calls tcp_dissect_pdus() and nothing more.

answered 01 Aug '11, 19:40

cmaynard's gravatar image

cmaynard ♦♦
9.4k●10●38●142
accept rate: 20%

0

Thanks for your responses, much appreciated. I should have mentioned that the code has been hacked about a good few times to test out various theories which is why is looked a mess.

I have re-read the manula items you suggested, but I think maybe I need to clarify the issue a bit further. The reassembly of the packets is working as expected, but it is the calling of the dissect function that I don't quite follow.

Here is my code simplified a bit.

static void dissect_xcp(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree)
{
 tcp_dissect_pdus(tvb, pinfo, tree, TRUE, FRAME_HEADER_LEN,
                 get_xcp_message_len, dissect_xcp_message);

}

static guint get_xcp_message_len(packet_info *pinfo, tvbuff_t *tvb, int offset){
guint16 length = 0;
tvb_memcpy(tvb, (guint8 *)&length, offset, 2);

return (guint)(length + 4);  // length must include header as well as payload data length}


static void dissect_xcp_message(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree){
proto_item *xcp_item = NULL;
proto_item *xcp_sub_item = NULL;
proto_tree *xcp_tree = NULL;
proto_tree *xcp_header_tree = NULL;
guint16 count = 0;
guint16 length = 0;


if (check_col(pinfo->cinfo, COL_PROTOCOL))
col_set_str(pinfo->cinfo, COL_PROTOCOL, PROTO_TAG_XCP);


xcpcount = tvb_get_guint8( tvb, 2 ); // Get the xcp ccounter
xcpcount += (tvb_get_guint8( tvb, 3 ) * 256);


if (check_col(pinfo->cinfo, COL_INFO)) {
col_append_fstr(pinfo->cinfo, COL_INFO, "[0x%X] ",
xcpcount);
}


if (tree)
{
// If there is a "tree" requested, we handle that request.
guint16 counter = 0;
xcp_item = proto_tree_add_item(tree, proto_xcp, tvb, 0, -1, FALSE);
xcp_tree = proto_item_add_subtree(xcp_item, ett_xcp);
xcp_header_tree = proto_item_add_subtree(xcp_item, ett_xcp);


xcp_sub_item = proto_tree_add_item( xcp_tree, hf_xcp_header, tvb, 0, -1, FALSE );
xcp_header_tree = proto_item_add_subtree(xcp_sub_item, ett_xcp);

// Copy length data
tvb_memcpy(tvb, (guint8 *)&length, 0, 2);
proto_tree_add_uint(xcp_header_tree, hf_xcp_length, tvb, 0, 0, length);

tvb_memcpy(tvb, (guint8 *)&counter, 2, 2);
proto_tree_add_uint(xcp_header_tree, hf_xcp_counter, tvb, 0, 0, counter);



}

}

So this bit of code shows the packet numbers in the info column e.g. [0x1234] [0x1235] [0x1236] [0x1237] and in the detailed information I get a breakdown of each XCP packet (4 of them) arranged as XCP Protocol->Header->(Packet Length and Package Counter). This is exactly as I expected for some of the TCP packets.

However, on a packet which indicated [2 Reassembled TCP Segments xxxx] I only get the first packet number e.g. [0x1234]. In the detailed view at the bottom I get the 4 packets I expected.

I could probably live without the packet display in the info column, but when I tried to implement a packet counter check I saw the same issue when checking packet counters across TCP segments in that the packet counter I was checking against was the first one in the reassembled TCP segment rather than the last one.

Thanks

EDIT: I thought a picture might be useful to see the problem.

http://postimage.org/image/uih7wfc4/

answered 02 Aug '11, 02:37

Tupperware's gravatar image

Tupperware
1●1●1●3
accept rate: 0%

edited 02 Aug '11, 04:56

First off, as grahamb already pointed out, stop using tvb_memcpy(). Use proto_tree_add_item() instead or where you really need to grab data, use the tvb endian-friendly accessors. Is XCP Big-Endian (BE) or Little-Endian (LE)? I assume BE. Try coding get_xcp_message_len() like this instead:

static guint get_xcp_message_len(packet_info pinfo, tvbuff_t tvb, int offset){return 4 + tvb_get_ntohs(tvb, offset);}

And if XCP is BE, xcpcount is being assigned wrong. Try this instead: xcpcount = tvb_get_ntohs(tvb, 2);

If it's LE, then use tvb_get_letohs() consistently throughout instead.

(02 Aug '11, 08:08) cmaynard ♦♦

Also, get rid of C++ comments and get rid of check_col().
After all that's cleaned up, we can look into the column problem.

(02 Aug '11, 08:08) cmaynard ♦♦

0

Hi cmaynard,

Thanks for your support. I've tidied up the code (hopefully) to your liking and removed any unecessary variables. The output is the same as described above and shown in the screenshot.

static void dissect_xcp(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree)
{
    tcp_dissect_pdus(tvb, pinfo, tree, TRUE, FRAME_HEADER_LEN,
                 get_xcp_message_len, dissect_xcp_message);
}

static guint get_xcp_message_len(packet_info *pinfo, tvbuff_t tvb, int offset)
{ 

/ Get length data and extend to cover header info */
return 4 + tvb_get_letohs(tvb, offset);
}


/* This method dissects fully reassembled messages */
static void dissect_xcp_message(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree)
{
proto_item *xcp_item = NULL;
proto_item *xcp_sub_item = NULL;
proto_tree *xcp_tree = NULL;
proto_tree *xcp_header_tree = NULL;


/* Identify XCP protocol */
col_set_str(pinfo->cinfo, COL_PROTOCOL, PROTO_TAG_XCP);

/* Extract XCP counter value */
xcpcount = tvb_get_letohs(tvb, 2);

/* Add packet counter to the info column */
col_append_fstr(pinfo->cinfo, COL_INFO, "[0x%X] ",xcpcount);

if (tree)
{
    /* If there is a "tree" requested, we handle that request. */
    xcp_item = proto_tree_add_item(tree, proto_xcp, tvb, 0, -1, FALSE);
    xcp_tree = proto_item_add_subtree(xcp_item, ett_xcp);
    xcp_header_tree = proto_item_add_subtree(xcp_item, ett_xcp);
    xcp_sub_item = proto_tree_add_item( xcp_tree, hf_xcp_header, tvb, 0, -1, FALSE );
    xcp_header_tree = proto_item_add_subtree(xcp_sub_item, ett_xcp);

    /* Show length data in detail view */
    proto_tree_add_uint(xcp_header_tree, hf_xcp_length, tvb, 0, 0, tvb_get_letohs(tvb, 0));
    /* Show count data in detail view */
    proto_tree_add_uint(xcp_header_tree, hf_xcp_counter, tvb, 0, 0, xcpcount);
}



}

answered 02 Aug ‘11, 08:54

Tupperware's gravatar image

Tupperware
1●1●1●3
accept rate: 0%

OK, so it’s LE then. When you add items to the tree, be sure to use ENC_LITTLE_ENDIAN for the proto_tree_add_xyz() endian argument instead of FALSE as FALSE is actually synonymous with ENC_BIG_ENDIAN. For items where endian-ness is irrelevant, some folks prefer to use ENC_NA.

What is the value of FRAME_HEADER_LEN? Are you calling col_clear() anywhere?

(02 Aug ‘11, 09:02) cmaynard ♦♦

FRAME_HEADER_LEN is 2

I removed col_clear in this example to ensure I never lost any data from the info column.

Question is, how do the proto_tree_* calls get executed whilst col_append_fstr does not for the reassembled TCP packets? TCP packets which were not reassembled work as expected.

(02 Aug ‘11, 09:51) Tupperware

At this point, it would probably be better to move this over to the -dev mailing list. There, if you could supply your code and a sample capture file, I think it would be a lot easier to help you.

More critiques … these are wrong: proto_tree_add_uint(xcp_header_tree, hf_xcp_length, tvb, 0, 0, tvb_get_letohs(tvb, 0));

proto_tree_add_uint(xcp_header_tree, hf_xcp_counter, tvb, 0, 0, xcpcount);

The length field should be 2, not 0 and the offset for xcpcount should be 2, not 0. But don’t use proto_tree_add_uint() -> use proto_tree_add_item() instead.

(02 Aug ‘11, 10:10) cmaynard ♦♦

Added it to the list :-)

(03 Aug ‘11, 03:48) Tupperware

And for anyone following this question, the thread is here.

(03 Aug ‘11, 08:56) cmaynard ♦♦