Re: [Wireshark-dev] ASN.1-based dissector development for Wireshark
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 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 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
Re: [Wireshark-dev] ASN.1-based dissector development for Wireshark
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 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
