Wireshark mailing list archives

Previous By Date Next
Previous By Thread Next

Re: ASN.1-based dissector development for Wireshark

From: Vincent Randal <vtrandal () gmail com>
Date: Mon, 19 Apr 2021 00:27:17 -0600
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 () wireshark org> 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 () gmail com> 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 () wireshark org> 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 () wireshark org>
Archives:    https://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-dev
             mailto:wireshark-dev-request () wireshark org
?subject=unsubscribe


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

___________________________________________________________________________
Sent via:    Wireshark-dev mailing list <wireshark-dev () wireshark org>
Archives:    https://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-dev
             mailto:wireshark-dev-request () wireshark org?subject=unsubscribe
Previous By Date Next
Previous By Thread Next

Current thread: