Wireshark-dev: Re: [Wireshark-dev] Wireshark-dev Digest, Vol 208, Issue 2

From: John Dill <John.Dill@xxxxxxxxxxxxxxxxx>
Date: Thu, 14 Sep 2023 16:52:32 +0000

>Message: 2
>Date: Tue, 12 Sep 2023 10:24:19 -0400
>From: John Thacker <johnthacker@xxxxxxxxx>
>To: Developer support list for Wireshark <wireshark-dev@xxxxxxxxxxxxx>
>Subject: Re: [Wireshark-dev] question on validation of a dissected
>        string from a BASE_CUSTOM hf item
>Message-ID:
>        <CAP_QLgpwRLiCnaPBvgEbiHQWGFC9KZOjNmacjoLtHybUZEybiA@xxxxxxxxxxxxxx>
>Content-Type: text/plain; charset="utf-8"
>
>You may have noticed "proto_tree_add_item_ret_display_string()" and perhaps
>found that it doesn't do what you want; it produces the display string for
>a default display representation and doesn't use your custom function.
>(Perhaps it should?)


Hi John,


I did notice that proto_item_add_item_ret_display_string and friends, but the intent doesn't appear to be what I'm envisioning, since the list of FT types doesn't include any of the FT integer types, e.g. from proto.c (I'm still 3.6)


REPORT_DISSECTOR_BUG("field %s is not of type FT_STRING, FT_STRINGZ, FT_UINT_STRING, FT_STRINGZPAD, FT_STRINGZTRUNC, FT_BYTES, or FT_UINT_BYTES",


I parse 1553 data over ethernet and often times the encodings for certain data items can be... creative.  I would certainly prefer to mod a way in Wireshark to use the custom function once for the display and return the dissected display string as part of adding the item to the proto_tree for expert_info validation (an example expert_info is when something demarcated as spare in the expected data is non-zero, indicating some kind of data definition drift to equipment over time).


My use case is kind of an amalgam of proto_tree_add_item_ret_display_string_and_length, but the header field represents a 16-bit up to 64-bit integer that is set to BASE_CUSTOM with a function that returns the resulting dissected string from the custom function pointer.  Putting some equivalent into Wireshark proto.h/c pushes more custom code out of an already large plugin (160 kloc at this point in time, and I'm not anywhere near done).


This is kind of a side endeavor, so I'm not in a rush on this.  Thanks for your code sample.  I think I got my answer as far as I'm not missing an obvious method to accomplish this.


Thanks,

John D.



From: Wireshark-dev <wireshark-dev-bounces@xxxxxxxxxxxxx> on behalf of wireshark-dev-request@xxxxxxxxxxxxx <wireshark-dev-request@xxxxxxxxxxxxx>
Sent: Tuesday, September 12, 2023 10:24:37 AM
To: wireshark-dev@xxxxxxxxxxxxx
Subject: Wireshark-dev Digest, Vol 208, Issue 2
 
Send Wireshark-dev mailing list submissions to
        wireshark-dev@xxxxxxxxxxxxx

To subscribe or unsubscribe via the World Wide Web, visit
        https://www.wireshark.org/mailman/listinfo/wireshark-dev
or, via email, send a message with subject or body 'help' to
        wireshark-dev-request@xxxxxxxxxxxxx

You can reach the person managing the list at
        wireshark-dev-owner@xxxxxxxxxxxxx

When replying, please edit your Subject line so it is more specific
than "Re: Contents of Wireshark-dev digest..."


Today's Topics:

   1. Re: question on validation of a dissected string from a
      BASE_CUSTOM hf item (John Thacker)
   2. Re: question on validation of a dissected string from a
      BASE_CUSTOM hf item (John Thacker)


----------------------------------------------------------------------

Message: 1
Date: Tue, 12 Sep 2023 10:05:44 -0400
From: John Thacker <johnthacker@xxxxxxxxx>
To: Developer support list for Wireshark <wireshark-dev@xxxxxxxxxxxxx>
Subject: Re: [Wireshark-dev] question on validation of a dissected
        string from a BASE_CUSTOM hf item
Message-ID:
        <CAP_QLgr9wpT28KyRqd3kTkWo+7h0CaHMqGbz0dkk2cMY4BZkOg@xxxxxxxxxxxxxx>
Content-Type: text/plain; charset="utf-8"

In general you don't want to do that, if your goal is to create an expert
info. The string would not exist and thus the Expert Info would not show in
the "Analyze -> Expert Information" dialog, or in other situations where
the tree was not visible and the item was faked; you might have to
specifically filter for the item. (See
https://gitlab.com/wireshark/wireshark/-/issues/18955 for a similar
situation. Also somewhat relatedly
https://gitlab.com/wireshark/wireshark/-/issues/19216 ) For optimization
purposes, strings and display labels are not filled in unless necessary.

What you could do is allocate your own string buffer, and pass it and the
uint64_t to your Receive_Frequency function. You can simplify that by
replacing your proto_tree_add_item() call with:

uint64_t value;
char[ITEM_LABEL_LENGTH] label;
ti = proto_tree_add_item_ret_uint64(tree, hf_, tvb, start, length,
encoding, &value);
Receive_Frequency(label, value);
if (....) {
  expert_add_info(pinfo, ti, &ei_);
}

However, since it's processor intensive and error prone to convert to a
string only to parse the string back to floating point, you'd probably be
better off retrieving the uint64_t value and passing it to a function that
converts it directly to the floating point you need, separate from your
custom display function.

Cheers,
John Thacker


On Thu, Sep 7, 2023 at 12:15 PM John Dill <John.Dill@xxxxxxxxxxxxxxxxx>
wrote:

> I have a question whether I can get the dissected string of the
> BASE_CUSTOM header field so that I can do analysis on it and convert it to
> floating point to do range analysis so I can issue an expert info if the
> value is valid but out of range.
>
>
>     {
>       &hf_Receive_Frequency,
>       {
>         "Receive Frequency",
>         "Receive_Frequency",
>         FT_UINT48,
>         BASE_CUSTOM,
>         CF_FUNC(Receive_Frequency),
>         0x0,
>         NULL,
>         HFILL
>       }
>     },
>
> ...
>
>     ti = proto_tree_add_item(word_tree,
> hf_XTS_5000_APX_8000_Receive_Frequency, tvb, offset, 6, ENC_BIG_ENDIAN);
>
> ...
>
> static void Receive_Frequency(gchar *result, guint64 value)
> {
>   guint16 w_Base_Frequency;
>   guint16 w_1_MHz_BCD;
>   guint16 w_100_KHz_BCD;
>   guint16 w_10_KHz_BCD;
>   guint16 w_Rx_KHz_Frequency_Field;
>
>   guint32 MHz_Frequency;
>   guint32 KHz_Frequency;
>
> ... processing ...
>
>   switch (lut_Base_Frequency[w_Base_Frequency])
>   {
>     case -1:
>       g_snprintf(result, ITEM_LABEL_LENGTH, "Illegal");
>       break;
>
>     case 0:
>       g_snprintf(result, ITEM_LABEL_LENGTH, "Invalid");
>       break;
>
>     default:
>       /* Frequency is typically displayed in MHz with 4 significant digits
> */
>       g_snprintf(result, ITEM_LABEL_LENGTH, "%" G_GUINT32_FORMAT ".%"
> G_GUINT32_FORMAT " MHz", MHz_Frequency, KHz_Frequency / 100);
>       break;
>   }
> }
>
> Is there a way for me to peel the dissected result string from "ti" after
> the proto_tree_add_item call so that I can do validation and range checking
> for expert info?
>
> Can I try to hijack proto_item_fill_display_label to somehow push "tmp"
> out of the interface into a proto_tree_add_*_ret_processed_string?
>
> \code
> if (FIELD_DISPLAY(hfinfo->display) == BASE_CUSTOM) {
> gchar tmp[ITEM_LABEL_LENGTH];
> custom_fmt_func_t fmtfunc = (custom_fmt_func_t)hfinfo->strings;
>
> DISSECTOR_ASSERT(fmtfunc);
> fmtfunc(tmp, number);
>
> label_len = protoo_strlcpy(display_label_str, tmp, label_str_size);
> \endcode
>
> This 'tmp' from fmtfunc is the string I want to grab for post-analysis,
> but it seems non-obvious how to get at it.
>
> I might be completely overlooking an easy way to do this.
>
> Any suggestions?  I already maintain a fork, so having a custom mod to
> pull this out is on the table for me.
>
> Thanks,
> John D.
>
> ___________________________________________________________________________
> 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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://www.wireshark.org/lists/wireshark-dev/attachments/20230912/67685ee5/attachment.htm>

------------------------------

Message: 2
Date: Tue, 12 Sep 2023 10:24:19 -0400
From: John Thacker <johnthacker@xxxxxxxxx>
To: Developer support list for Wireshark <wireshark-dev@xxxxxxxxxxxxx>
Subject: Re: [Wireshark-dev] question on validation of a dissected
        string from a BASE_CUSTOM hf item
Message-ID:
        <CAP_QLgpwRLiCnaPBvgEbiHQWGFC9KZOjNmacjoLtHybUZEybiA@xxxxxxxxxxxxxx>
Content-Type: text/plain; charset="utf-8"

You may have noticed "proto_tree_add_item_ret_display_string()" and perhaps
found that it doesn't do what you want; it produces the display string for
a default display representation and doesn't use your custom function.
(Perhaps it should?)

On Tue, Sep 12, 2023, 10:05 AM John Thacker <johnthacker@xxxxxxxxx> wrote:

> In general you don't want to do that, if your goal is to create an expert
> info. The string would not exist and thus the Expert Info would not show in
> the "Analyze -> Expert Information" dialog, or in other situations where
> the tree was not visible and the item was faked; you might have to
> specifically filter for the item. (See
> https://gitlab.com/wireshark/wireshark/-/issues/18955 for a similar
> situation. Also somewhat relatedly
> https://gitlab.com/wireshark/wireshark/-/issues/19216 ) For optimization
> purposes, strings and display labels are not filled in unless necessary.
>
> What you could do is allocate your own string buffer, and pass it and the
> uint64_t to your Receive_Frequency function. You can simplify that by
> replacing your proto_tree_add_item() call with:
>
> uint64_t value;
> char[ITEM_LABEL_LENGTH] label;
> ti = proto_tree_add_item_ret_uint64(tree, hf_, tvb, start, length,
> encoding, &value);
> Receive_Frequency(label, value);
> if (....) {
>   expert_add_info(pinfo, ti, &ei_);
> }
>
> However, since it's processor intensive and error prone to convert to a
> string only to parse the string back to floating point, you'd probably be
> better off retrieving the uint64_t value and passing it to a function that
> converts it directly to the floating point you need, separate from your
> custom display function.
>
> Cheers,
> John Thacker
>
>
> On Thu, Sep 7, 2023 at 12:15 PM John Dill <John.Dill@xxxxxxxxxxxxxxxxx>
> wrote:
>
>> I have a question whether I can get the dissected string of the
>> BASE_CUSTOM header field so that I can do analysis on it and convert it to
>> floating point to do range analysis so I can issue an expert info if the
>> value is valid but out of range.
>>
>>
>>     {
>>       &hf_Receive_Frequency,
>>       {
>>         "Receive Frequency",
>>         "Receive_Frequency",
>>         FT_UINT48,
>>         BASE_CUSTOM,
>>         CF_FUNC(Receive_Frequency),
>>         0x0,
>>         NULL,
>>         HFILL
>>       }
>>     },
>>
>> ...
>>
>>     ti = proto_tree_add_item(word_tree,
>> hf_XTS_5000_APX_8000_Receive_Frequency, tvb, offset, 6, ENC_BIG_ENDIAN);
>>
>> ...
>>
>> static void Receive_Frequency(gchar *result, guint64 value)
>> {
>>   guint16 w_Base_Frequency;
>>   guint16 w_1_MHz_BCD;
>>   guint16 w_100_KHz_BCD;
>>   guint16 w_10_KHz_BCD;
>>   guint16 w_Rx_KHz_Frequency_Field;
>>
>>   guint32 MHz_Frequency;
>>   guint32 KHz_Frequency;
>>
>> ... processing ...
>>
>>   switch (lut_Base_Frequency[w_Base_Frequency])
>>   {
>>     case -1:
>>       g_snprintf(result, ITEM_LABEL_LENGTH, "Illegal");
>>       break;
>>
>>     case 0:
>>       g_snprintf(result, ITEM_LABEL_LENGTH, "Invalid");
>>       break;
>>
>>     default:
>>       /* Frequency is typically displayed in MHz with 4 significant
>> digits */
>>       g_snprintf(result, ITEM_LABEL_LENGTH, "%" G_GUINT32_FORMAT ".%"
>> G_GUINT32_FORMAT " MHz", MHz_Frequency, KHz_Frequency / 100);
>>       break;
>>   }
>> }
>>
>> Is there a way for me to peel the dissected result string from "ti" after
>> the proto_tree_add_item call so that I can do validation and range checking
>> for expert info?
>>
>> Can I try to hijack proto_item_fill_display_label to somehow push "tmp"
>> out of the interface into a proto_tree_add_*_ret_processed_string?
>>
>> \code
>> if (FIELD_DISPLAY(hfinfo->display) == BASE_CUSTOM) {
>> gchar tmp[ITEM_LABEL_LENGTH];
>> custom_fmt_func_t fmtfunc = (custom_fmt_func_t)hfinfo->strings;
>>
>> DISSECTOR_ASSERT(fmtfunc);
>> fmtfunc(tmp, number);
>>
>> label_len = protoo_strlcpy(display_label_str, tmp, label_str_size);
>> \endcode
>>
>> This 'tmp' from fmtfunc is the string I want to grab for post-analysis,
>> but it seems non-obvious how to get at it.
>>
>> I might be completely overlooking an easy way to do this.
>>
>> Any suggestions?  I already maintain a fork, so having a custom mod to
>> pull this out is on the table for me.
>>
>> Thanks,
>> John D.
>>
>>
>> ___________________________________________________________________________
>> 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
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://www.wireshark.org/lists/wireshark-dev/attachments/20230912/1726e9e8/attachment.htm>

------------------------------

Subject: Digest Footer

_______________________________________________
Wireshark-dev mailing list
Wireshark-dev@xxxxxxxxxxxxx
https://www.wireshark.org/mailman/listinfo/wireshark-dev


------------------------------

End of Wireshark-dev Digest, Vol 208, Issue 2
*********************************************