Wireshark · Wireshark-dev: Re: [Wireshark-dev] ASN.1-based dissector development for Wireshark

[Vincent Randal <vtrandal@xxxxxxxxx>]

Hi Pascal,

I sincerely appreciate the code changes you provided. If I applied it correctly, it did not fix the problem for me in my Ubuntu 18.04 VM, so I will try it in my host operating system, Debian 10. I’m slow trying this until I get version control working with git so I can diff the code changes properly. I will reply again soon with results for Debian and maybe even Windows 10 and/or Fedora. 

Vincent

On Fri, Apr 16, 2021 at 1:51 AM Pascal Quantin <pascal@xxxxxxxxxxxxx> wrote:

Hi Vincent,

the truncated ASCII bytes pane seems like a Qt UI bug not related to the dissector code itself. It seems like you are suffering from https://gitlab.com/wireshark/wireshark/-/issues/17087 that got fixed in https://gitlab.com/wireshark/wireshark/-/merge_requests/1902 but not backported in release-3.4 branch (which is probably a mistake). Could you apply it locally and confirm it fixes the issue for you?

Indeed the simple ASN.1 based dissector code sample is outdated. Could you should replace:

static void
dissect_foo(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree)
{
        proto_item      *foo_item = NULL;
        proto_tree      *foo_tree = NULL;
        int                     offset = 0;

        /* make entry in the Protocol column on summary display */
        if (check_col(pinfo->cinfo, COL_PROTOCOL))
                col_set_str(pinfo->cinfo, COL_PROTOCOL, PNAME);

    /* create the foo protocol tree */
    if (tree) {
        foo_item = proto_tree_add_item(tree, proto_foo, tvb, 0, -1, FALSE);
        foo_tree = proto_item_add_subtree(foo_item, ett_foo);

        dissect_FOO_MESSAGE_PDU(tvb, pinfo, foo_tree);
    }
}

by

static int
dissect_foo(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree, void *data _U_)
{
    proto_item      *foo_item;
    proto_tree      *foo_tree;

    /* make entry in the Protocol column on summary display */
    col_set_str(pinfo->cinfo, COL_PROTOCOL, PNAME);

    /* create the foo protocol tree */
    foo_item = proto_tree_add_item(tree, proto_foo, tvb, 0, -1, FALSE);
    foo_tree = proto_item_add_subtree(foo_item, ett_foo);

    return dissect_FOO_MESSAGE_PDU(tvb, pinfo, foo_tree);
}

It should remove the call to the data dissector after the ASN.1 PDU dissection (that happens because the FOO dissector does not indicate to have consumed 

Best regards,
Pascal.

Le jeu. 15 avr. 2021 à 23:59, Vincent Randal <vtrandal@xxxxxxxxx> a écrit :

Hi Anders (and interested persons). Good idea. Thank you. I expanded Wireshark to fullscreen. And I've made two new screenshots to compare behavior of myfoo dissector with the icmp dissector. I realize now I might be comparing apples with oranges insofar as myfoo is UDP-based while icmp is TCP-based. Here goes.

Screenshots relevant to this email are in myfoo.bug.tgz (attached)

(myfoo.png) Screenshot for myfoo dissector. This dissector is essentially the simple ASN.1-based dissector example (foo) from the Wireshark documentation.

(icmp.png) Screenshot for imcp dissector which I believe to be TCP-based and is one of many many dissectors that come with Wireshark.

The two screenshots show only the left half of the fullscreen Wireshark UI window (the omitted right halves are blank). My focus is on how highlighting in the bottom window (raw data) responds to highlighting MYFOO Protocol [Internet Control Message Protocol (for icmp)] in the middle window (expanded tree). There's at least two things to notice: (1) For myfoo the 24 byte UDP-payload comprises the entire message; the messageId and flowId are incorrectly highlighted along with the two 10 octet strings that follow (in the messageData). Whereas for icmp the 48 data bytes are correctly preceded by 16 bytes of information (type, code, checksum, etc). (2) Both myfoo and icmp have a problem with the ASCII area to the right of the raw data in the bottom window. For myfoo the problem is more severe insofar as the ASCII area is only 8 characters wide. For icmp the ASCII area is a full 16 characters wide for part of the raw data.

My conclusions regarding the above observations:

(1) I've introduced a bug in the myfoo dissector when I made changes to packet-myfoo-template.c and packet-myfoo-template.h in an attempt to get them build. I can review those changes by diffing myfoo with foo from the simple ASN.1-based dissector example online. Specifically, the bug in myfoo is causing the messageId and flowId to be included in (highlighted along with) the two 10 octet strings that follow them. See the attached myfoo.asn file for details or refer to foo.asn from the example here: https://www.wireshark.org/docs/wsdg_html_chunked/SimpleASN1BasedDissector.html

(2) It could be that as Wireshark loads my dissector the "ASCII area" in the bottom window is adversely affected for all dissectors as indicated in the screenshot for icmp. Maybe that's not possible. Otherwise, there's a problem with the width of the ASCII area not being a full 16 characters wide to correspond to the raw data on the left.

I'm confident this write up is a review for many of you that work on dissectors in which case I'm hoping someone can comment on my conclusions and speculations. I would be pleasantly surprised if fixing myfoo bug also fixes the width of the ASCII area for icmp. I can actually test that theory by downloading and installing wireshark-3.4.4 from wireshark.org website.

It's been 14 years since I looked at dissector code and then only briefly. I sincerely appreciate when people make the effort to read and make sense of my sometime bewildered observations. Thank you. I hope this write up is of some benefit to someone else.

Vincent Randal

On Thu, Apr 15, 2021 at 12:10 PM Anders Broman via Wireshark-dev <wireshark-dev@xxxxxxxxxxxxx> wrote:

Hi,

Looks like your window is masking off part of the ascii display try to expand the left side…

/Anders

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

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