Re: [Wireshark-dev] API to adjust view in Wireshark

2024-04-19 Thread John Thacker
On Fri, Apr 19, 2024 at 10:33 AM Jeff Klingler  wrote:

> Hi,
>
> I am building a log viewer where if a user clicks on a log event it can
> show the related PCAP related to that timeframe. Is there an API where I
> can send a time and date to a Wireshark API and have the viewer jump to the
> nearest time period?
>

The short answer is not one quick API call in the way that you'd like,
largely because it's not guaranteed that the frames in capture files are in
chronological order, which rules out algorithms that would make a search
take faster than linear time but also because no one has implemented it.

A slightly longer answer is that there are ways. You can call
cap_file_provider_get_frame_ts() from cfile.h searching through the frames
and compare those times to your desired time. If you can assume that the
file is in strict time order, you can make that faster with binary search
or similar. (It might be helpful to have a flag in the capture_file struct
similar to what capinfos stores for strict time order.) Once you get the
frame number, there are API calls to go to a particular frame number.

John Thacker
___
Sent via:Wireshark-dev mailing list 
Archives:https://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-dev
 mailto:wireshark-dev-requ...@wireshark.org?subject=unsubscribe


Re: [Wireshark-dev] RPM package error on Fedora-30

2024-04-05 Thread John Thacker
On Fri, Apr 5, 2024 at 8:06 PM Ben Greear  wrote:

> Hello,
>
> I cloned latest wireshark today, and tried to build an RPM package
> on an fedora-30 system.
>
> It fails with this error below..any ideas?
>
> Fedora-36 worked as expected, and F30 could build with 'make', just not
> 'make wireshark_rpm'
>
> 
> -- Build files have been written to:
> /home/greearb/btbits/x64_btbits/3plibs/wireshark/build/packaging/rpm/BUILD/wireshark-4.3.0
> + %cmake_build
> /var/tmp/rpm-tmp.IEg59q: line 87: fg: no job control
> error: Bad exit status from /var/tmp/rpm-tmp.IEg59q (%build)
>
>
> RPM build errors:
>  Bad exit status from /var/tmp/rpm-tmp.IEg59q (%build)
> make[3]: *** [CMakeFiles/wireshark_rpm.dir/build.make:78:
> CMakeFiles/wireshark_rpm] Error 1
> make[2]: *** [CMakeFiles/Makefile2:4057: CMakeFiles/wireshark_rpm.dir/all]
> Error 2
> make[1]: *** [CMakeFiles/Makefile2:4064:
> CMakeFiles/wireshark_rpm.dir/rule] Error 2
> make: *** [Makefile:297: wireshark_rpm] Error 2
>

That error is because Fedora 30 doesn't have the %cmake_build macro (which
is defined in /usr/lib/rpm/macros.d/macros.cmake in most Fedora systems).
You might be able to get it to build by tweaking the spec file template in
packaging/rpm/wireshark.spec.in to be similar on what's done for earlier
branches
before we dropped support for RHEL 7 (or before RHEL 8 added the
%cmake_build macros - see this commit
https://gitlab.com/wireshark/wireshark/-/commit/92c8c2f7a09c68489f5c2272f8f6bb9ab0b0aed9
)
and using one of the other macros instead (%ninja_build if you're using
ninja, something else with make.

Fedora 30 has officially been EOL since 2020-05-26 (
https://docs.fedoraproject.org/en-US/releases/eol/ ) so most of the
assumptions in the spec file in the last development branch are that people
will be using Fedora 33 or later (which itself has been EOL for 2.5 years),
even though Fedora 30 is roughly similar to RHEL 8 and thus is able to
build, RHEL8 (in 8.4) gained support for the cmake macros and Fedora 30
didn't, being EOL at the time that RHEL 8.4 was released.

John Thacker
___
Sent via:Wireshark-dev mailing list 
Archives:https://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-dev
 mailto:wireshark-dev-requ...@wireshark.org?subject=unsubscribe


Re: [Wireshark-dev] SCTP association analysis & selection does not work correctly

2024-02-22 Thread John Thacker
On Thu, Feb 22, 2024 at 10:24 AM Cristian Constantin via Wireshark-dev <
wireshark-dev@wireshark.org> wrote:

> Hi,
> How to figure out if a fix for an issue like the one mentioned by John
> above is part of a Wireshark release? And what Wireshark release is
> part of...
>

 The Gitlab page for the commit has some entries that indicate which
branches and tags (tags would indicate releases) have a commit. This one is
only in the master branch, not a release.

https://gitlab.com/wireshark/wireshark/-/commit/922d1f621935ace31d08243c2627c149111401be

Some commits that fix bugs (as opposed to new features or performance
enhancements) are backported. There will be links added in the merge
request and the commits to other commits that were backported. That is not
going to happen for this commit.

John
___
Sent via:Wireshark-dev mailing list 
Archives:https://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-dev
 mailto:wireshark-dev-requ...@wireshark.org?subject=unsubscribe


Re: [Wireshark-dev] resolving external symbol for ASN.1 plugin issue

2024-02-20 Thread John Thacker
On Tue, Feb 20, 2024 at 6:09 PM R Massink  wrote:

> Hello,
>
> I've been active in creating a dissector for IEC61850, which is mapped on
> top of mms. There is an integrated mms dissector that I'm using as a
> template, while mapping all mms names to the IEC61850 equivalent. I have
> been able to successfully use asn2wrs.py to generate a packet dissector
> from the .cnf and .asn files, and can build a static wireshark executable
> with my dissector included.
>
> However, I'm now trying to get the same dissector to work as an external
> plugin, so it can be loaded by others into their wireshark without having
> to use my compiled version of wireshark.
>
> While the external plugin compiled fine, i'm running into the problem that
> the dissector_acse_* external symbols cannot be
> resolved(dissect_acse_EXTERNALt is the first one) during runtime. The
> dissector relies on these symbols from epan/dissectors/packet-acse.c, and
> as statically build dissector they can of course be resolved. But when the
> plugin loads externally during runtime(manual or automatic), the symbol
> cannot be resolved anymore, while I can find it in libwireshark.so, which
> is in turn loaded into memory by the wireshark process.
>
> I was not able to find other wireshark examples where external plugins
> relied on symbols from statically linked dissectors. Is what I want simply
> not possible, and should I include all acse dissector, and sub dependencies
> as well in my plugin(lots of code duplication), or am I building the plugin
> not correct or missing a linker flag?
>

The issue is not exactly in your plugin, but in the other dissector.

Example where external plugins use symbols from dissectors in the main tree:

opcua/opcua.c uses tcp_dissect_pdus from epan/dissectors/packet-tcp.h

By default symbols are not visible. To make a symbol visible, it must have
WS_DLL_PUBLIC, which is defined in include/ws_symbol_export.h

(See some comments here:
https://gitlab.com/wireshark/wireshark/-/blob/master/CMakeLists.txt#L1087 )

WS_DLL_PUBLIC void
tcp_dissect_pdus(...);

For a ASN.1 generated dissector, to cause an existing symbol to be
exported, look at this commit:

https://gitlab.com/wireshark/wireshark/-/commit/7ce05b9dd77741bf28a1efece8429f43982e7fe2

Where I altered the .cnf file for the x509af dissector so that
dissect_x509af_Certificate
got the WS_DLL_PUBLIC
attribute added to it by the asn2wrs.py process.

Cheers,
John Thacker
___
Sent via:Wireshark-dev mailing list 
Archives:https://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-dev
 mailto:wireshark-dev-requ...@wireshark.org?subject=unsubscribe


Re: [Wireshark-dev] 4GB limit for RPC dissector?

2024-01-26 Thread John Thacker
On Fri, Jan 26, 2024, 4:27 AM Linux Smiths  wrote:

>
> Can someone confirm this or if anyone has used wireshark/tshark to decode
> RPC streams greater than 4GB your confirmation will be helpful too. Btw
> I've tried all the protocol preferences and nothing helps.
>
> Thanks,
> LS
>
>
It's a known issue, sorry, that affects anything over TCP that needs
desegmentation. That's when the TCP sequence number rolls over. See here:

https://gitlab.com/wireshark/wireshark/-/issues/10503

https://gitlab.com/wireshark/wireshark/-/issues/19331

Fixing it involves having some kind of extended sequence number and
changing certain lookups for old segments. Unlike an ordinary network
stack, Wireshark (and and also tshark, even in one pass mode) can't just
discard old segments but keeps information around so that random packet
access is possible.

John Thacker
___
Sent via:Wireshark-dev mailing list 
Archives:https://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-dev
 mailto:wireshark-dev-requ...@wireshark.org?subject=unsubscribe


Re: [Wireshark-dev] SCTP association analysis & selection does not work correctly

2023-12-22 Thread John Thacker
On Thu, Dec 7, 2023 at 3:32 AM Cristian Constantin via Wireshark-dev <
wireshark-dev@wireshark.org> wrote:

> Hi Jeff,
>
> Yes, after enabling the respective protocol decoding option, SCTP
> association analysis works.
> SCTP association analysis is _quite_ slow, though. I'll check why it
> is so slow when I have some spare time.
>

If you have some time, can you see if
https://gitlab.com/wireshark/wireshark/-/merge_requests/13786
works for you and if it's faster (at least the dissection part, this
doesn't affect the tapping)? It works
on my samples, and it uses hashmaps instead of lists so it should be better
on large files.

Thanks,
John
___
Sent via:Wireshark-dev mailing list 
Archives:https://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-dev
 mailto:wireshark-dev-requ...@wireshark.org?subject=unsubscribe


Re: [Wireshark-dev] wireshark handles SCTP association indexing wrong under some circumstances -- multi-homing is wrongly reported where there is none

2023-12-20 Thread John Thacker
On Wed, Dec 20, 2023, 4:32 PM John Thacker  wrote:

> > On 6 Dec 2023, at 12:08, Ariel Burbaickij 
>> wrote:
>> >
>> > Hello all,
>> >
>> > we have a special setup here: SS7 E1 is converted to SCTP traffic with
>> the following basic schema (I cannot share capture itself, just in case):
>> > -- there are no INITs, HEARTBEATs/ACK, SACKs, just DATA chunks sent in
>> both directions as containers then for the traffic on higher layers .
>> > --each linkset, of which there are many, is represented like this:
>> >   1.1.1.1 <-> 2.2.2.2
>> >   3.3.3.3 <-> 4.4.4.4
>> >   5.5.5.5 <-> 6.6.6.6
>> >   etc.
>> > so, that one and the same IP address is never re-used for several
>> associations and <-> means bidirectional traffic. All associations use the
>> same port 2904 on both sides.
>> >
>> >
>> > vtags used per direction are last two bytes of the source IP in the
>> least significant bytes of vtag field, so for the second association it is:
>> >
>> > 0x0303 from 3.3.3.3 to 4.4.4.4
>> > and
>> > 0x0404 from 4.4.4.4 to 3.3.3.3
>> > etc.
>> >
>> > and TSNs are verified to be accurate too.
>> >
>> > Now, upon selecting the packet from, say  3.3.3.3 to 4.4.4.4 and
>> "Analyse this Association", we get multi-homed association reported with
>> always larger vtag reported as part of association, so as a matter of
>> example:
>> >
>> > Endpoint 1 is 1.1.1.1 and 3.3.3.3 (vtag 0x0303)
>> > Endpoint 2 is  2.2.2.2 and 4.4.4.4 (vtag 0x0404)
>> >
>> > so, why does analysis fail here, where it should not ?
>>
>
>
> I see you filed an issue.
> https://gitlab.com/wireshark/wireshark/-/issues/19544
>
> After fixing the issues with truncated associations, I see that the way
> the analysis is done it won't work if the same port is used on both sides,
> as you mentioned and as I see in the sample.
>
> The ports are used as keys for the association (because it accepts the
> possibility that the same association could be used on multiple IP
> addresses on one side). Also the most recently created associations are
> searched for first.
>
> In your sample file, what happens is something like:
>
> Frame 1, port 2904 vtag 0x0204 -> 2904, create association #1 with other
> side vtag unknown. (IP addresses are ignored)
> Frame 2, port 2904 vtag 0x021c -> 2904, there's an existing association
> with matching ports and an unknown vtag, assume this is the other direction
> of #1 (it is not.)
> Frame 3, port 2904 vtag 0x0202 -> 2904, doesn't match the association
> above because it matches neither of the vtags, create new association #2
> with other side vtag unknown
> Frame 4, port 2904 vtag 0x0204 -> 2904; finds association #2 first
> (searches from the end) with an empty vtag, assumes this is the other
> direction of #2, fills in other vtag as 0x204.
> ...
>
> The addresses don't come into play at all.
>
> On the second pass, Frame 1 will find association #2 before association #1
> and it will have a different association index on the second pass.
>
> The whole thing needs to be rewritten (it's quite slow anyway - it looks
> through the entire list of associations for each packet, and does so each
> dissection.)
>
> It needs to use a map for lookups, and for your case to work needs to also
> consider addresses (only decide that this is the opposite side of an
> association if an address used in the other direction matches). That can
> run into problems if the reply comes back from a different IP address than
> the original destination, as unrelatedly is allowed to happen in GTPv1,
> which causes requests and replies not to be associated. (In GTPv1 the
> Destination Address of a response has to be the Source Address of the
> request, but the Source Address of the response doesn't have to be the
> Destination Address of the Request, except for Echos, 3GPP TS 29.060
> 10.1.2.2)
>

It works better when there are INIT chunks present, but your sample file
only has DATA chunks, as you say.

John

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


Re: [Wireshark-dev] wireshark handles SCTP association indexing wrong under some circumstances -- multi-homing is wrongly reported where there is none

2023-12-20 Thread John Thacker
>
> > On 6 Dec 2023, at 12:08, Ariel Burbaickij 
> wrote:
> >
> > Hello all,
> >
> > we have a special setup here: SS7 E1 is converted to SCTP traffic with
> the following basic schema (I cannot share capture itself, just in case):
> > -- there are no INITs, HEARTBEATs/ACK, SACKs, just DATA chunks sent in
> both directions as containers then for the traffic on higher layers .
> > --each linkset, of which there are many, is represented like this:
> >   1.1.1.1 <-> 2.2.2.2
> >   3.3.3.3 <-> 4.4.4.4
> >   5.5.5.5 <-> 6.6.6.6
> >   etc.
> > so, that one and the same IP address is never re-used for several
> associations and <-> means bidirectional traffic. All associations use the
> same port 2904 on both sides.
> >
> >
> > vtags used per direction are last two bytes of the source IP in the
> least significant bytes of vtag field, so for the second association it is:
> >
> > 0x0303 from 3.3.3.3 to 4.4.4.4
> > and
> > 0x0404 from 4.4.4.4 to 3.3.3.3
> > etc.
> >
> > and TSNs are verified to be accurate too.
> >
> > Now, upon selecting the packet from, say  3.3.3.3 to 4.4.4.4 and
> "Analyse this Association", we get multi-homed association reported with
> always larger vtag reported as part of association, so as a matter of
> example:
> >
> > Endpoint 1 is 1.1.1.1 and 3.3.3.3 (vtag 0x0303)
> > Endpoint 2 is  2.2.2.2 and 4.4.4.4 (vtag 0x0404)
> >
> > so, why does analysis fail here, where it should not ?
>


I see you filed an issue.
https://gitlab.com/wireshark/wireshark/-/issues/19544

After fixing the issues with truncated associations, I see that the way the
analysis is done it won't work if the same port is used on both sides, as
you mentioned and as I see in the sample.

The ports are used as keys for the association (because it accepts the
possibility that the same association could be used on multiple IP
addresses on one side). Also the most recently created associations are
searched for first.

In your sample file, what happens is something like:

Frame 1, port 2904 vtag 0x0204 -> 2904, create association #1 with other
side vtag unknown. (IP addresses are ignored)
Frame 2, port 2904 vtag 0x021c -> 2904, there's an existing association
with matching ports and an unknown vtag, assume this is the other direction
of #1 (it is not.)
Frame 3, port 2904 vtag 0x0202 -> 2904, doesn't match the association above
because it matches neither of the vtags, create new association #2 with
other side vtag unknown
Frame 4, port 2904 vtag 0x0204 -> 2904; finds association #2 first
(searches from the end) with an empty vtag, assumes this is the other
direction of #2, fills in other vtag as 0x204.
...

The addresses don't come into play at all.

On the second pass, Frame 1 will find association #2 before association #1
and it will have a different association index on the second pass.

The whole thing needs to be rewritten (it's quite slow anyway - it looks
through the entire list of associations for each packet, and does so each
dissection.)

It needs to use a map for lookups, and for your case to work needs to also
consider addresses (only decide that this is the opposite side of an
association if an address used in the other direction matches). That can
run into problems if the reply comes back from a different IP address than
the original destination, as unrelatedly is allowed to happen in GTPv1,
which causes requests and replies not to be associated. (In GTPv1 the
Destination Address of a response has to be the Source Address of the
request, but the Source Address of the response doesn't have to be the
Destination Address of the Request, except for Echos, 3GPP TS 29.060
10.1.2.2)

John Thacker
___
Sent via:Wireshark-dev mailing list 
Archives:https://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-dev
 mailto:wireshark-dev-requ...@wireshark.org?subject=unsubscribe


Re: [Wireshark-dev] Future of Wireshark's Debian packaging scripts in the main repository

2023-11-22 Thread John Thacker
On Wed, Nov 22, 2023 at 10:37 AM John Thacker  wrote:

>
> I think moving the packaging assets to the packaging directory and telling
> people to symbolically link it to build Debian, as we've been doing, is a
> relatively minor imposition for the Debian folks, but my understanding is
> that the package builds will fail if all the Lintian stuff isn't done,
> which creates a burden on us.
>

And it looks like Balint is trying to present a compromise on the symbols
issue:

https://gitlab.com/wireshark/wireshark/-/merge_requests/13385

John
___
Sent via:Wireshark-dev mailing list 
Archives:https://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-dev
 mailto:wireshark-dev-requ...@wireshark.org?subject=unsubscribe


Re: [Wireshark-dev] Future of Wireshark's Debian packaging scripts in the main repository

2023-11-22 Thread John Thacker
On Wed, Nov 22, 2023 at 9:40 AM João Valverde  wrote:

>
> There are a myriad issues I have touched upon. To recap, in my opinion, if
> we want to provide public shared libraries (libwireshark, wiretap,
> wsutil... for what I don't know) we should do a better job of that
> collectively as a project. If we don't want to do that we should kill the
> Debian package inanity.
>
> A third alternative is just to keep the status quo and I'll try to avoid
> this subject entirely because of how much it bothers me to just ignore all
> these technical issues.
>

My understanding of the Debian packaging scripts (and similar for the RPM
package) use case is that people might be running one of those
distributions and want to upgrade Wireshark on their system using their
distribution's native package manager by taking either a git repository or
a tarball and building a package that they can upgrade their
distribution-provided package to.

That isn't necessarily to add custom dissectors and provide public shared
libraries, though it could be. Oftentimes it's as simple as "my
distribution is capable of compiling 3.6.x or later, but for stability
reasons it's still shipping 2.6.x (Debian buster/oldstable, RHEL 8 and
clones)," and someone wants to update wireshark without any of their own
changes, just without upgrading their distribution. It's handy to be able
to accommodate that if possible.

I think moving the packaging assets to the packaging directory and telling
people to symbolically link it to build Debian, as we've been doing, is a
relatively minor imposition for the Debian folks, but my understanding is
that the package builds will fail if all the Lintian stuff isn't done,
which creates a burden on us.

On RPM distributions there's an annoyance because Red Hat / Fedora decided
to change their package names around a bit, so they're no longer quite
compatible with the old names which we provide (and which still work with
other RPM-based distros -
https://gitlab.com/wireshark/wireshark/-/issues/18709 ).

John Thacker
___
Sent via:Wireshark-dev mailing list 
Archives:https://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-dev
 mailto:wireshark-dev-requ...@wireshark.org?subject=unsubscribe


Re: [Wireshark-dev] Examples where field doesn't have enough bits of value_string values

2023-09-21 Thread John Thacker
On Thu, Sep 21, 2023, 4:20 PM Martin Mathieson via Wireshark-dev <
wireshark-dev@wireshark.org> wrote:

> After https://gitlab.com/wireshark/wireshark/-/merge_requests/12195, I'm
> finding the warnings below.  I think these are valid, based upon editing a
> mask value and watching how the value was masked/shifted before being
> looked up in the value_string.
>
> I understand the RoHC example, where a newer version of the protocol
> supports the new values in a wider field, but the old one still uses the
> updated value_string, despite some of the values being impossible to match.
>
> A common mistake seems to be to add to the value_string the byte values in
> the frame, rather than the masked + shifted value of the field as
> calculated in the item before looking up the value_string.
>
> I can hopefully try to look at these as time permits, but if you are
> familiar with any of the protocols below, I'd appreciate any fixes/feedback
> you can give!
>
> Best regards,
> Martin
>
>
> Warning: epan/dissectors/packet-bittorrent.c hf_bittorrent_msg_type
> filter= bittorrent.msg.type VALS(bittorrent_messages) has max value 261
> (0x105) which doesn't fit into 8 bits ( mask is 0x0 )
>

This one is somewhat odd but intended. There's an 8 bit message type field
in the standard Bittorrent protocol, and then the dissector handles the
Azureus variant protocol (where messages are identified with a string) by
assigning Wireshark internal numbers outside 8 bit range.

John

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


Re: [Wireshark-dev] question on validation of a dissected string from a BASE_CUSTOM hf item

2023-09-18 Thread John Thacker
On Sun, Sep 17, 2023, 10:06 PM Guy Harris  wrote:

> On Sep 7, 2023, at 9:15 AM, John Dill  wrote:
>
>
> If so, perhaps what's called for is a new mechanism to provide private
> *encodings*, so that the dissectors registration routine might do something
> such as
>
> guint32 enc_frequency;
>
> ...
>
> enc_frequency =
> register_integer_encoding_uint64(my_frequency_routine);
>
> where "encode_frequency() would take a tvbuff and an offset (and possibly
> other arguments as necessary) and provides a 64-bit unsigned integer.  Then
> you could do
>
> ti = proto_tree_add_item(word_tree,
> hf_XTS_5000_APX_8000_Receive_Frequency, tvb, offset, 6, enc_frequency);
>
> with the registration routine returning an encoding value guaranteed not
> to collide with any predefined encodings for that type or with any other
> registered encoding.
> ...
>
> A custom encoding of the proposed sort, and a custom formatter, could both
> be used; the custom decoder routine could either itself add an expert info
> item, or could include both a decoding routine and a checking routine to
> add expert info.
>
> This could be combined with a custom *display* routine.
>

A custom encoding would have the advantage of working with filtering, which
custom formatting of numbers does not.

For a field the result of the encoding is the value, but the result of the
formatting is just a string to display.

For filtering, a string can be checked against a value string's outputs, as
there's usually a limited number of possibilities. (It takes the first
match and doesn't work on range strings - what would be nice would be to
somehow convert that into matching against a set of values that yield that
string.) It doesn't test all possible 32 or 64 bit values to see which ones
might yield the string you want to filter with.

If you have something which is a derived floating point number, it's almost
surely better to compute the number, add it as a generated FT_DOUBLE field
or whatever (proto_tree_add_double()) or perhaps a FT_UINT measuring KHz
(since floating point comparisons are tricky
https://gitlab.com/wireshark/wireshark/-/issues/16483) checking for illegal
values and adding expert infos then, and then perhaps have custom
formatting for that. Then you could filter with a decimal number instead of
having to filter by typing in your BCD encoding. (Also note that generating
a filter for your current field will produce a decimal version of the still
BCD encoded value, which won't be easy to read.)

John Thacker
___
Sent via:Wireshark-dev mailing list 
Archives:https://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-dev
 mailto:wireshark-dev-requ...@wireshark.org?subject=unsubscribe


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

2023-09-17 Thread John Thacker
On Thu, Sep 14, 2023 at 12:52 PM John Dill 
wrote:

> >Message: 2
> >Date: Tue, 12 Sep 2023 10:24:19 -0400
> >From: John Thacker 
> >To: Developer support list for Wireshark 
> >Subject: Re: [Wireshark-dev] question on validation of a dissected
> >string from a BASE_CUSTOM hf item
> >Message-ID:
> ><
> cap_qlgpwrlicnapbvgebihqwgfc9kzojnmacjolthybuzey...@mail.gmail.com>
> >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",
>
It's still that way on the current master, but I think there's a solid use
case for adding to that routine the ability to add an item and get what the
standard representation would be. It's just not implemented yet.

John
___
Sent via:Wireshark-dev mailing list 
Archives:https://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-dev
 mailto:wireshark-dev-requ...@wireshark.org?subject=unsubscribe


Re: [Wireshark-dev] question on validation of a dissected string from a BASE_CUSTOM hf item

2023-09-12 Thread John Thacker
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  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, );
> Receive_Frequency(label, value);
> if () {
>   expert_add_info(pinfo, ti, _);
> }
>
> 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 
> 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.
>>
>>
>> {
>>   _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 
>> Archives:https://www.wireshark.org/lists/wireshark-dev
>> Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-dev
>>  mailto:wireshark-dev-requ...@wireshark.org
>> ?subject=unsubscribe
>>
>
___
Sent via:Wireshark-dev mailing list 
Archives:https://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-dev
 mailto:wireshark-dev-requ...@wireshark.org?subject=unsubscribe


Re: [Wireshark-dev] question on validation of a dissected string from a BASE_CUSTOM hf item

2023-09-12 Thread John Thacker
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, );
Receive_Frequency(label, value);
if () {
  expert_add_info(pinfo, ti, _);
}

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 
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.
>
>
> {
>   _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 
> Archives:https://www.wireshark.org/lists/wireshark-dev
> Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-dev
>  mailto:wireshark-dev-requ...@wireshark.org
> ?subject=unsubscribe
>
___
Sent via:Wireshark-dev mailing list 
Archives:https://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-dev
 mailto:wireshark-dev-requ...@wireshark.org?subject=unsubscribe


Re: [Wireshark-dev] SCTP statistics

2023-08-28 Thread John Thacker
The statistics mentioned here?
https://gitlab.com/wireshark/wireshark/-/issues/16367

The comments there suggest that the Enable Association Indexing preference
has to be on for the SCTP stats to work.

John


On Mon, Aug 28, 2023, 10:19 AM Jaap Keuter  wrote:

> Hi,
>
> Who knows what the current status of the SCTP statistics is? I’ve tried a
> few files, but couldn’t make sense of it. It looked like information was
> missing or not filled at all.
>
> Thanks,
> Jaap
>
> Send from my iPhone
> ___
> Sent via:Wireshark-dev mailing list 
> Archives:https://www.wireshark.org/lists/wireshark-dev
> Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-dev
>  mailto:wireshark-dev-requ...@wireshark.org
> ?subject=unsubscribe
>
___
Sent via:Wireshark-dev mailing list 
Archives:https://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-dev
 mailto:wireshark-dev-requ...@wireshark.org?subject=unsubscribe


Re: [Wireshark-dev] Help regarding CI failure in gitlab

2023-07-29 Thread John Thacker
On Sat, Jul 29, 2023, 1:24 PM Jaap Keuter  wrote:

>
> - dissector_add_uint("wtap_encap", 147, base_handle); looks suspicious,
> where’s 147 coming from.
>

It appears to be from here:
https://gerrit.openbmc.org/c/openbmc/libmctp/+/46162

"As there's no formal linktype defined for MCTP or higher-level DMTF
protocols command-line switches provide the ability to specify one of the
private linktype values in the range 147-162."

This has multiple issues:

Wiretap encapsulations are not the same as libpcap link layer types. Wtap
encap 147 is defined in wiretap/wtap.h as WTAP_ENCAP_AX25_KISS, used by
packet-ax25-kiss.c

Presumably what is meant here is the wiretap encapsulation equivalent,
WTAP_ENCAP_USER0 (45), but we are never going to upstream a patch for one
of the private linktypes / encapsulations.

The correct thing to do is to follow the directions here:

https://www.tcpdump.org/linktypes.html

And request a new link layer type, which will then also result in a new
wiretap encapsulation.

John Thacker

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


Re: [Wireshark-dev] wiki.wireshark.org Sample Capture Links Broken

2023-07-26 Thread John Thacker
Someone created an issue for this:

https://gitlab.com/wireshark/wireshark/-/issues/19234

On Sat, Jul 1, 2023, 7:18 PM chuck c  wrote:

> Thank you for the analysis.
> I copied your notes over to the Discord server for internal discussion
> about infrastructure.
>
> On Thu, Jun 29, 2023 at 10:44 AM Ken Mix  wrote:
>
>> Hello,
>>
>> The links for sample captures imported from MoinMoin are currently broken
>> at https://wiki.wireshark.org/SampleCaptures. Links uploaded recently
>> look fine, and the links are not broken at
>> https://gitlab.com/wireshark/wireshark/-/wikis/SampleCaptures
>>
>> For example, the first sample capture listed has the following correct
>> link at Gitlab:
>>
>>
>> https://gitlab.com/wireshark/wireshark/-/wikis/uploads/__moin_import__/attachments/SampleCaptures/rpl-dio-mc-nsa-optional-tlv-dissector-sample.pcap.gz
>>
>> But has the following at wiki.wireshark.org:
>>
>>
>> https://wiki.wireshark.org/uploads//__moin_import_/_/attachments/SampleCaptures/rpl-dio-mc-nsa-optional-tlv-dissector-sample.pcap.gz
>>
>> I'm not sure what the deployment process is or how exactly
>> wiki.wireshark.org is served, but something appears to be adding a
>> forward slash in between the two underscores after moin_import. If I
>> manually edit the URL and delete the extraneous forward slash, I'm able to
>> download the file:
>>
>>
>> https://wiki.wireshark.org/uploads//__moin_import__/attachments/SampleCaptures/rpl-dio-mc-nsa-optional-tlv-dissector-sample.pcap.gz
>>
>> If I should file an issue on this, or if there's somewhere else I can
>> look at the deployment process, I can do that, as well.
>>
>> Regards,
>>
>> Ken Mix
>>
>> ___
>> Sent via:Wireshark-dev mailing list 
>> Archives:https://www.wireshark.org/lists/wireshark-dev
>> Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-dev
>>  mailto:wireshark-dev-requ...@wireshark.org
>> ?subject=unsubscribe
>>
> ___
> Sent via:Wireshark-dev mailing list 
> Archives:https://www.wireshark.org/lists/wireshark-dev
> Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-dev
>  mailto:wireshark-dev-requ...@wireshark.org
> ?subject=unsubscribe
>
___
Sent via:Wireshark-dev mailing list 
Archives:https://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-dev
 mailto:wireshark-dev-requ...@wireshark.org?subject=unsubscribe


Re: [Wireshark-dev] Dissecting TLS and non-TLS using the same ports

2023-07-13 Thread John Thacker
On Thu, Jul 13, 2023, 10:49 AM Markku Leiniö  wrote:

> Hi,
>
> In my Zabbix dissector I'm currently using
> dissector_add_uint_range_with_preference("tcp.port", ZABBIX_TCP_PORTS,
> zabbix_handle) to define the TCP ports (defaulting to "10050,10051").
>
> I'm now attempting to use ssl_dissector_add() to dissect also
> TLS-encrypted Zabbix protocol packets, using the same ports (that's how
> Zabbix works: some agents use TLS, some don't, and they all connect to
> the same port on the server). I see port number 0 being used in some
> dissectors (for example in packet-kafka.c), but that does not seem to
> work. From some comments I understand that it enables to use manual
> "Decode as" or something like that.
>
> So, apparently I need to use ssl_dissector_add() with all the configured
> ports. I see examples of using range_foreach() to do that, so I used it
> like this:
>
> range_t *zabbix_tcp_range;
> zabbix_tcp_range = prefs_get_range_value("zabbix", "tcp.port");
> range_foreach(zabbix_tcp_range, range_add_zabbix_tls_callback,
> NULL);
>
> It seems to work with TLS packets, but now it won't dissect non-TLS
> Zabbix packets at all.
>
> In Lua (with my previous dissector) I was able to do simply this:
>
>  DissectorTable.get("tcp.port"):add(default_settings.ports,
> zabbix_protocol)
>  DissectorTable.get("tls.port"):add(default_settings.ports,
> zabbix_protocol)
>
> and that worked fine, it dissected both TLS and non-TLS packets correctly.
>
> How do I get the same behaviour with C dissector?
>

The short answer is don't use ssl_dissector_add(), just

dissector_add_uint_range("tls.port", zabbix_tcp_range, zabbix_protocol);

There's also a version _with_preference that takes a string instead. That
will be the same as what you did in Lua.

Longer answer:
ssl_dissector_add() does two things:

1. Registers your protocol in the "tls.port" table so TLS calls it on that
port.
2. Registers TLS in the "tcp.port" table so TCP calls TLS on that port.

Only one protocol can be registered to a given port for TCP.

With protocols like Zabbix, the choices are:
1. Register the non TLS version in the TCP port table, have it reject
packets that are not Zabbix, the TLS heuristic dissector should pick it up
if all goes well and forward it along after dissecting the TLS portion.
Sounds like that worked for you on Lua, so it should work here.
2. Register the TLS version in the TCP port and a heuristic dissector for
Zabbix; if the non TLS protocol doesn't look like TLS, the TLS dissector
should reject it, and your heuristic dissector should pick it up.
3. Register some kind of helper dissector to the TCP port that can detect
whether this is straight Zabbix or TLS, calling the TLS dissector if
necessary. This can end up making `pinfo->layers` have an extra entry,
especially for the first TLS packet in the first pass.

Probably TMI, because the first should work for you.

John Thacker
___
Sent via:Wireshark-dev mailing list 
Archives:https://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-dev
 mailto:wireshark-dev-requ...@wireshark.org?subject=unsubscribe


Re: [Wireshark-dev] Ability to dynamically dissect in more detail?

2023-05-16 Thread John Thacker
On Tue, May 16, 2023 at 12:27 PM  wrote:

> I have a dissector. I dissect the content as delimited text. Sometimes the
> textual content has further meaning, but I only want to dissect it in
> further detail on a packet by packet basis and only if the user requests it
> on a specific packet.
>
>
>
> The reason is that the detailed dissection requires extra information to
> be loaded and extra dissection processing. Is there any mechanism to expand
> a section only when requested?
>

There are lots of optimizations, many automatic and some a little more
complicated to employ, that can mean not dissecting in detail fields for
packets other than the singular packet currently selected and displayed in
the Packet Details frame (with appropriate exceptions for, e.g., fields
that are specifically being filtered.) You could certainly combine that
with a preference that says "only dissect past here on a visible packet"
with no problem.

For a visible packet, however, none of the optimizations apply and the
entire dissection is performed (up to the limits implied by your
hypothetical preference). There's some rendering on the Qt side that
doesn't happen until the details are expanded, but the dissection itself is
done.

Note that during full dissection, items in a subtree can add additional
items as siblings to a parent or grandparent, or to the root of the tree
(or even do more unusual things like change the visible text of a parent or
grandparent, etc.), so a general ability to not dissect child items until a
subtree were expanded could result in oddities. The optimizations to fake
items in general deal with this correctly (an example of an exception:
https://gitlab.com/wireshark/wireshark/-/issues/17877 - the Protocol
Hierarchy stats don't deal well with protocols that change length after the
items are added.)

John Thacker
___
Sent via:Wireshark-dev mailing list 
Archives:https://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-dev
 mailto:wireshark-dev-requ...@wireshark.org?subject=unsubscribe


Re: [Wireshark-dev] lua dissector: using base.UNIT_STRING on ftypes.DOUBLE ProtoField

2023-03-20 Thread John Thacker
On Mon, Mar 20, 2023, 2:36 PM Dennis Lambe  wrote:

> On Mon, Mar 20, 2023 at 12:17 PM chuck c  wrote:
> > Have you tried defining the field using ProtoField.float or
> ProtoField.double?
>
> Yup. Same behavior. Unit shows up correctly for `tshark -G values` but
> doesn't appear in the UI.
>

The problem isn't in Lua, but in epan/proto.c

The code that handles the values displayed for doubles ignores unit strings.

Dr. Lars Völker started a MR to add this:

https://gitlab.com/wireshark/wireshark/-/merge_requests/10006

John Thacker

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


Re: [Wireshark-dev] Some items with apparently out-of-range value_string values?

2023-02-12 Thread John Thacker
The bittorrent one is fine. The message type field in a packet with the
standard protocol is a single byte. There's an Azureus dialect that clients
can switch to if they both speak it, and it has some extra message types
specified with string names only. The dissector uses the message type field
with internal Wireshark only numbers for those types.

For EAP, the MNC has a certain value, but the string to use depends on the
MCC. `proto_tree_add_uint` is used there, but probably the more complicated
`proto_tree_add_uint_format_value` call from packet-e212.c is appropriate
in order have just the MNC for the value.

John Thacker

On Sun, Feb 12, 2023, 6:14 PM Martin Mathieson via Wireshark-dev <
wireshark-dev@wireshark.org> wrote:

> Hi,
>
> I have added another check to CHECK_HF_FILTER in proto.c (extra checks
> that only get done in the 'CLANG + Code checks' pipeline build) to check
> for values in an item's value_string that could not be represented in the
> item's type (e.g. a value of > 255 for FT_UINT8).  I can eventually look
> into all of them, but if anyone recognises a filter below from a protocol
> they know well and could check it, that would be great.
>
> I understand the RoHC case, but haven't looked into many others.  One
> thing that made this check tricky was dealing with -ve numbers in the macro
> where this check is done, but hopefully few/none of these cases here are
> just because of -ve numbers cast to an unsigned type.
>
> Thanks,
> Martin
>
>
> ** (tshark:122026) 22:58:47.725497 [Epan WARNING] epan/proto.c:8499 --
> tmp_fld_check_assert(): FT_UINT8, "Message Type" filter bittorrent.msg.type
> value of 260 cannot be represented
>
>  ** (tshark:122026) 22:58:47.824725 [Epan WARNING] epan/proto.c:8499 --
> tmp_fld_check_assert(): FT_UINT8, "Tag" filter cbor.type.tag value of 22098
> cannot be represented
>
>  ** (tshark:122026) 22:58:47.825338 [Epan WARNING] epan/proto.c:8495 --
> tmp_fld_check_assert(): FT_UINT8, "Class" filter cip.class value of 272
> cannot be represented
>
>  ** (tshark:122026) 22:58:48.432044 [Epan WARNING] epan/proto.c:8495 --
> tmp_fld_check_assert(): FT_UINT8, "Class" filter devicenet.class value of
> 272 cannot be represented
>
>  ** (tshark:122026) 22:58:48.432137 [Epan WARNING] epan/proto.c:8499 --
> tmp_fld_check_assert(): FT_UINT8, "Type" filter
> dhcp.vendor.pktc.mta_cap_type value of 12609 cannot be represented
>
>  ** (tshark:122026) 22:58:48.441871 [Epan WARNING] epan/proto.c:8495 --
> tmp_fld_check_assert(): FT_UINT16, "Identity Mobile Network Code" filter
> eap.identity.mnc value of 99101 cannot be represented
>
>  ** (tshark:122026) 22:58:48.442037 [Epan WARNING] epan/proto.c:8495 --
> tmp_fld_check_assert(): FT_UINT16, "Identity Mobile Network Code" filter
> eap.identity.mnc value of 99 cannot be represented
>
>  ** (tshark:122026) 22:58:48.443270 [Epan WARNING] epan/proto.c:8495 --
> tmp_fld_check_assert(): FT_UINT8, "SDO Transfer Abort" filter
> epl.asnd.sdo.cmd.abort.code value of 134217763 cannot be represented
>
>  ** (tshark:122026) 22:58:48.449709 [Epan WARNING] epan/proto.c:8499 --
> tmp_fld_check_assert(): FT_UINT8, "Radio Resources Management Message Type"
> filter gmr1.rr.msg_type value of 318 cannot be represented
>
>  ** (tshark:122026) 22:58:48.452640 [Epan WARNING] epan/proto.c:8499 --
> tmp_fld_check_assert(): FT_UINT8, "Error" filter h450.error value of 2002
> cannot be represented
>
>  ** (tshark:122026) 22:58:48.457042 [Epan WARNING] epan/proto.c:8499 --
> tmp_fld_check_assert(): FT_UINT8, "error code" filter jdwp.errorcode value
> of 511 cannot be represented
>
>  ** (tshark:122026) 22:58:48.459379 [Epan WARNING] epan/proto.c:8499 --
> tmp_fld_check_assert(): FT_UINT8, "Network Address family" filter
> lldp.network_address.subtype value of 16396 cannot be represented
>
>  ** (tshark:122026) 22:58:48.459399 [Epan WARNING] epan/proto.c:8499 --
> tmp_fld_check_assert(): FT_UINT8, "Address Subtype" filter
> lldp.mgn.address.subtype value of 16396 cannot be represented
>
>  ** (tshark:122026) 22:58:48.467792 [Epan WARNING] epan/proto.c:8499 --
> tmp_fld_check_assert(): FT_UINT8, "R. Trigger" filter mip6.bri_r.trigger
> value of 296 cannot be represented
>
>  ** (tshark:122026) 22:58:48.477608 [Epan WARNING] epan/proto.c:8495 --
> tmp_fld_check_assert(): FT_UINT8, "Charset" filter mysql.charset value of
> 308 cannot be represented
>
>  ** (tshark:122026) 22:58:48.477719 [Epan WARNING] epan/proto.c:8495 --
> tmp_fld_check_assert(): FT_UINT8, "Charset" filter mariadb.charset value of
> 1248 cannot be represented
>
>  *

Re: [Wireshark-dev] how to decode ULP in TLS

2022-10-08 Thread John Thacker
On Sat, Oct 8, 2022, 11:45 AM Bahjat, Ehab  wrote:

> Hi
>
> I have a trace of a ULP (OMA UserPlane Location Protocol) session. It is
> encrypted in TLS, wireshark is able to decrypt the tls packets using
> pre-master secret log file which I have but I am not able to decode the
> decrypted data because I couldn’t find ULP in the protocol list for tls as
> showing below, the only way was using a painful process to remove the
> encrypted hex string and reassemble the packet again, is there away to
> decode ULP using tls port number? Thanks !
>
It is possible to decrypt it through the deprecated "RSA keys list" UAT
preference by using the protocol name, but it doesn't appear in the Decode
As list currently, as you note.

It should (because that's the approved method for configuration going
forward) and it wouldn't be that difficult to add. Would you mind filing an
issue for it?

https://gitlab.com/wireshark/wireshark/-/issues

John Thacker
___
Sent via:Wireshark-dev mailing list 
Archives:https://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-dev
 mailto:wireshark-dev-requ...@wireshark.org?subject=unsubscribe


Re: [Wireshark-dev] CARES to old for CentOS8?

2022-09-30 Thread John Thacker
 Best regards
>>>> Anders
>>>>
>>>>
>>>> Den tors 29 sep. 2022 16:32Roland Knall  skrev:
>>>>
>>>>> That library was not the only consideration. The main consideration
>>>>> was to cut-off at a certain point for 4.0 so that we can avoid having too
>>>>> many things to consider going forward. There was a message about this on
>>>>> the list a while back as well as a discussion at SF.
>>>>>
>>>>> Now, I get the argument to have compatibility for self-built versions,
>>>>> and I could see a point, where we make a switch for a certain library to
>>>>> have a compatibility mode. But I am not sure if this should be the way
>>>>> forward in this case. Much rather have the nuisance to compile a more
>>>>> recent version together with Wireshark, than have one more thing to 
>>>>> support
>>>>>
>>>>> regards
>>>>> Roland
>>>>>
>>>>> Am Do., 29. Sept. 2022 um 15:03 Uhr schrieb Jeff Morriss <
>>>>> jeff.morriss...@gmail.com>:
>>>>>
>>>>>> Also keep in mind that if RHEL decides to fix the CVE(s) in question
>>>>>> in version 8 of their OS, they would likely apply the fix for the CVE to
>>>>>> the version of CARES that they are already shipping (i.e., they'd create 
>>>>>> a
>>>>>> version like 1.13.0. rather than upgrading to 1.14.x).  They 
>>>>>> work
>>>>>> hard to avoid changing version numbers for compatibility reasons.
>>>>>>
>>>>>> On Thu, Sep 29, 2022 at 6:59 AM Anders Broman 
>>>>>> wrote:
>>>>>>
>>>>>>> Hi,
>>>>>>> Well a choice to make if we want to support CentOS8/RHEL8 or not.
>>>>>>> One could argue that CVE:s in support libraries might not be for us to
>>>>>>> decide on but rather the OS maintainers.
>>>>>>> Best regards
>>>>>>> Anders
>>>>>>>
>>>>>>> Den tors 29 sep. 2022 kl 08:19 skrev Roland Knall >>>>>> >:
>>>>>>>
>>>>>>>> The reason for 1.14 was a CVE that was fixed. I would vote strongly
>>>>>>>> against reducing the Version just to support an older version.
>>>>>>>>
>>>>>>>> Regards, Roland
>>>>>>>>
>>>>>>>> Am 28.09.2022 um 18:48 schrieb John Thacker >>>>>>> >:
>>>>>>>>
>>>>>>>> 
>>>>>>>> On Wed, Sep 28, 2022, 10:47 AM Anders Broman 
>>>>>>>> wrote:
>>>>>>>>
>>>>>>>>> Hi,
>>>>>>>>> Is there a workaround for
>>>>>>>>> CMake Error at
>>>>>>>>> /usr/share/cmake/Modules/FindPackageHandleStandardArgs.cmake:230 
>>>>>>>>> (message):
>>>>>>>>>   Could NOT find CARES: Found unsuitable version "1.13.0", but
>>>>>>>>> required is at
>>>>>>>>>   least "1.14.0" (found /usr/lib64/libcares.so)?
>>>>>>>>> I would like to build for CentOS8...
>>>>>>>>>
>>>>>>>>
>>>>>>>> It doesn't actually need anything from 1.14.0, so changing the line
>>>>>>>> in CMakeLists.txt that sets the minimum version should be fine. Look 
>>>>>>>> at the
>>>>>>>> commit below and change one line to 1.13.0
>>>>>>>>
>>>>>>>>
>>>>>>>> https://gitlab.com/wireshark/wireshark/-/commit/5991a75d78a31ba61de6c162c79c2928da19c302
>>>>>>>>
>>>>>>>> John
>>>>>>>>
>>>>>>>>>
>>>>>>>> ___
>>>>>>>> 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
>>>&g

Re: [Wireshark-dev] CARES to old for CentOS8?

2022-09-28 Thread John Thacker
On Wed, Sep 28, 2022, 10:47 AM Anders Broman  wrote:

> Hi,
> Is there a workaround for
> CMake Error at
> /usr/share/cmake/Modules/FindPackageHandleStandardArgs.cmake:230 (message):
>   Could NOT find CARES: Found unsuitable version "1.13.0", but required is
> at
>   least "1.14.0" (found /usr/lib64/libcares.so)?
> I would like to build for CentOS8...
>

It doesn't actually need anything from 1.14.0, so changing the line in
CMakeLists.txt that sets the minimum version should be fine. Look at the
commit below and change one line to 1.13.0

https://gitlab.com/wireshark/wireshark/-/commit/5991a75d78a31ba61de6c162c79c2928da19c302

John

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


Re: [Wireshark-dev] Missing text2pcap-scanner.l in repository

2022-08-23 Thread John Thacker
On Tue, Aug 23, 2022, 4:04 PM Jirka Novak  wrote:

> Hi,
>
>By chance I noticed that there is text2pcap-scanner.c which is based
> on text2pcap-scanner.l, but the file is not in repository.
>It was there, but was removed.
>
>Was removal intended?
>

Yes, ui/text_import_scanner.l now handles processing both for text2pcap and
for the "Import from Text" feature within the GUI, so that they share code
and functionality doesn't get out of alignment. There's a large set of
text2pcap changes mentioned in the release notes for 4.0

https://gitlab.com/wireshark/wireshark/-/blob/master/ui/text_import_scanner.l

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


Re: [Wireshark-dev] wslog, windows, pytest, and heap corruption

2021-12-30 Thread John Thacker
On Thu, Dec 30, 2021 at 5:55 PM Gerald Combs  wrote:

> On 12/29/21 5:15 PM, John Thacker wrote:
> > I was working on a MR for moving the text2pcap/text_import debug over to
> the ws_log features and I ran into a seemingly bizarre problem. Setting the
> log level to a non-default value causes the pytest procedures to fail with
> heap corruption on the Gitlab Windows CI.
> >
> > Some of the text2pcap pytests depend on grepping through the stderr
> output for some of the debug information. Those tests originally passed the
> -d flag to text2pcap, so I replaced it with setting the log level to
> "debug" (and later "info") with the standard "--log-level debug" argument
> read by ws_log_parse_args().
> >
> > On Windows (but not Linux or MacOS, not clang or gcc, nor with either
> using ASAN), those tests which set the log level (and only those tests)
> started failing with a return code of 0xc374, heap corruption.
> >
> > As I looked into it closer, all the debug information that those tests
> used ought to be logged at "warning" or "message," which are at the default
> log level, so I was able to remove that flag, and then it passed.
> >
> > It looks like it might be related to some of the things discussed here,
> though I'm not 100% sure because I'm not a Windows programmer:
> >
> > https://discuss.wxpython.org/t/heap-corruption-on-windows/35583 <
> https://discuss.wxpython.org/t/heap-corruption-on-windows/35583>
> > https://bugs.python.org/issue36792 <https://bugs.python.org/issue36792>
> > https://bugs.python.org/issue37945 <https://bugs.python.org/issue37945>
> >
> > There's some kind of issue seen in Python 3.8 and higher, with Windows
> 10 build 1809 (which is a long term support build that is what the CI build
> server uses), with UTF-8 locales, with log systems that get system locale
> information and print dates, the Windows 10 Universal CRT, and heap
> corruption.
> >
> > It might have something to do with the tests spawning a lot of
> subprocesses in parallel and setting the log level to a different value
> eventually calling free_log_filter() from ws_log_set_debug_filter().
>
> Is https://gitlab.com/wireshark/wireshark/-/pipelines/438735249 one of
> the pipelines that failed? If so, it looks like Wireshark is crashing and
> Python is complaining about its return code:
>
> 
> Traceback (most recent call last):
>File "C:\builds\wireshark\wireshark\test\fixtures.py", line 54, in
> wrapped
>  test_fn(self, *fixtures)
>File "C:\builds\wireshark\wireshark\test\suite_text2pcap.py", line 186,
> in test_text2pcap_ikev1_certs_pcap
>  check_text2pcap(self, 'ikev1-certs.pcap', 'pcap')
>File "C:\builds\wireshark\wireshark\test\suite_text2pcap.py", line 144,
> in check_text2pcap_real
>  self.assertRun(text2pcap_cmd, shell=True)
>File "C:\builds\wireshark\wireshark\test\subprocesstest.py", line 304,
> in assertRun
>  self.assertEqual(process.returncode, expected_return)
> AssertionError: 3221226356 != 0
> 
>
> Yes, that's one of them. 3221226356 = 0xc374 and is apparently a
special Windows return code for heap corruption.

Just a wild guess, but maybe we need to call setlocale at the beginning of
> text2pcap similar to our other executables?
>

That probably makes sense to do anyway. However, I tried another draft
merge request adding "--log-level debug" to tshark (and dumpcap)
executables, with no other changes, and saw the same issue:

https://gitlab.com/wireshark/wireshark/-/jobs/1931595222

All the tests where I added the log-level command fail with heap corruption
on Windows (and nowhere else). Since tshark already has the setlocale
command, I guess that's not it. The various bug reports seem to indicate
that it only happens on UTF-8 code pages, and was fixed in a later Windows
release.

John Thacker
___
Sent via:Wireshark-dev mailing list 
Archives:https://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-dev
 mailto:wireshark-dev-requ...@wireshark.org?subject=unsubscribe


[Wireshark-dev] wslog, windows, pytest, and heap corruption

2021-12-29 Thread John Thacker
I was working on a MR for moving the text2pcap/text_import debug over to
the ws_log features and I ran into a seemingly bizarre problem. Setting the
log level to a non-default value causes the pytest procedures to fail with
heap corruption on the Gitlab Windows CI.

Some of the text2pcap pytests depend on grepping through the stderr output
for some of the debug information. Those tests originally passed the -d
flag to text2pcap, so I replaced it with setting the log level to "debug"
(and later "info") with the standard "--log-level debug" argument read by
ws_log_parse_args().

On Windows (but not Linux or MacOS, not clang or gcc, nor with either using
ASAN), those tests which set the log level (and only those tests) started
failing with a return code of 0xc374, heap corruption.

As I looked into it closer, all the debug information that those tests used
ought to be logged at "warning" or "message," which are at the default log
level, so I was able to remove that flag, and then it passed.

It looks like it might be related to some of the things discussed here,
though I'm not 100% sure because I'm not a Windows programmer:

https://discuss.wxpython.org/t/heap-corruption-on-windows/35583
https://bugs.python.org/issue36792
https://bugs.python.org/issue37945

There's some kind of issue seen in Python 3.8 and higher, with Windows 10
build 1809 (which is a long term support build that is what the CI build
server uses), with UTF-8 locales, with log systems that get system locale
information and print dates, the Windows 10 Universal CRT, and heap
corruption.

It might have something to do with the tests spawning a lot of subprocesses
in parallel and setting the log level to a different value eventually
calling free_log_filter() from ws_log_set_debug_filter().

John Thacker
___
Sent via:Wireshark-dev mailing list 
Archives:https://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-dev
 mailto:wireshark-dev-requ...@wireshark.org?subject=unsubscribe


Re: [Wireshark-dev] ISO-8601 date support

2021-12-24 Thread John Thacker
On Sat, Dec 4, 2021 at 8:01 AM Joerg Mayer  wrote:

> On Fri, Dec 03, 2021 at 12:28:23PM +0100, Jaap Keuter wrote:
> > With commit a0173cd7 you’ve added ISO-8601 date support to text2pcap.
> > The “Import from Hex dump...” feature of Wireshark is supposed to be
> equally capable.
> > Would you be able to add this capability there as well?
>
> While I agree that this would make sense, the C++ code looks so different,
> that I don't
> know where to add this code (and it doesn't help that I don't understand
> C++ beyond simple
> C).
>
> In order for this to really behave the same, the acutual parsing
> funtionality should
> probably be in code used by both text2pcap and the GUI and be put into the
> ui/ folder,
> where we keep code common to CLI and QT. Also, it would be nice if the
> Regular Expression
> feature from 8c1b29a597764cd3e4 could be ported back to the CLI as well.
>
> So if anyone feels like refactoring these things into common code, that
> would make sense
> from my point of view, but there is really not much I can achieve while
> only spending a
> sensible amount of time.
>

I have refactored the parsing functionality used by both text2pcap and the
GUI into the ui/ folder and checked it in.

The ISO-8601 date support does work in the GUI, through the undocumented
hack of putting "ISO" for the time format string (the GUI will accept it),
same as what text2pcap accepts. Everything supported by one or the other
still works, but there's a few features that only one supports (see
https://gitlab.com/wireshark/wireshark/-/issues/16724 for an issue
tracking):

CLI missing:
1) Regex support
2) Export PDU
3) No offset, everything into one big packet (should be easy)

GUI missing:
1) IPv6 dummy headers
2) Custom IPv4 and IPv6 addresses
3) Special try extra hard to deal with hex+ASCII dumps where the ASCII
coincidentally looks like a byte
4) Writing to pcap instead of pcapng

The documentation needs to be updated, and then after that there's a few
possible enhancements that would be nice to have. (Use command line options
similar to other CLI, support other file formats, etc.)

John Thacker
___
Sent via:Wireshark-dev mailing list 
Archives:https://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-dev
 mailto:wireshark-dev-requ...@wireshark.org?subject=unsubscribe


Re: [Wireshark-dev] Exporting FTP objects

2021-12-14 Thread John Thacker
On Tue, Dec 14, 2021 at 1:36 PM Richard Sharpe 
wrote:

> On Tue, Dec 14, 2021 at 10:18 AM Moshe Kaplan 
> wrote:
> >
> > I considered using such a data structure, but the challenge there is
> that there's no guarantee of a 'file transfer complete' that could be used
> to trigger reassembly and adding to the export objects list. AFAIK, it's
> also not possible to have a function to run after all packets were
> dissected to generate the export object list entries then.
>
> I am very unfamiliar with the tap infrastructure but perhaps you could
> introduce an EOM event through the tap so you could detect the data
> connection going down and do reassembly there.
>

The SMB export objects functionality, if I read it correctly, just does its
own reassembly inside its export objects tap, and every time it gets a new
chunk it updates the existing table entry (including showing what
percentage of the entire file has been gathered):

https://gitlab.com/wireshark/wireshark/-/blob/master/epan/dissectors/packet-smb.c#L973-1571

That's probably slower because of some extra copying and work, but you
don't have to worry about doing something special at the end, I suppose.

the TFTP export objects functionality was rewritten to be less that way a
while back:
https://gitlab.com/wireshark/wireshark/-/commit/25800536388aa2b567a18874dd0312a2bb29464d

though that's probably because the TFTP export objects only works on
complete files so the extra copies were just a performance waste.

For the data structure option, I recall doing something like that once
temporarily with I believe the TFTP dissector. You can store such a data
structure in conversation_data, and then on a second pass (checking if it's
visited) export if it's the last block seen. That doesn't work for tshark
except in two pass mode.

As far as the usefulness, for text files I found it quite useful to have
even partial sparse files, which is why I did the above, though I didn't
really consider it good enough quality to submit.

John Thacker
___
Sent via:Wireshark-dev mailing list 
Archives:https://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-dev
 mailto:wireshark-dev-requ...@wireshark.org?subject=unsubscribe


Re: [Wireshark-dev] TCP reassembly fails when ethernet tunnled over TCP

2021-11-12 Thread John Thacker
Yes, this is a long standing problem:

https://gitlab.com/wireshark/wireshark/-/issues/2345
and
https://gitlab.com/wireshark/wireshark/-/issues/9782

among others, are examples of the same generic problem.

The entire packet_info [dl_|net_]{src, dst} structure doesn't work very
well for tunnelled packets, especially those containing the same protocol
in the outer layers as well inside the tunnel. The endpoints API is
supposed to help, but the TCP dissector doesn't use it, and it would still
have to be changed for multiple protocols of the same type, see Michael
Mann's comment on #2345.

John


On Fri, Nov 12, 2021 at 7:57 AM Anders Broman via Wireshark-dev <
wireshark-dev@wireshark.org> wrote:

> Hi,
>
> We have a proprietary protocol sending usually small frames mixed with
> larger tunneled ethernet frames over TCP. If we then have a TCP segment
> where the ethernet frame
>
> Spans 2 segments reassembly fails presumably because pinfo does not have
> the IP address of the TCP segment. I think we would need a way to create a
> new pinfo structure
>
> For the tunneled frame? How to do that or some other way to solve the
> problem? In our case we only have ethernet and a vlan tag then our protocol
> again so
>
> We “fixed” that by dissecting those bytes in the internal dissector. But I
> think it may be a generic problem for tunneling that may deserve a proper
> fix.
>
> tcp_dissect_pdus() is used
>
> Regards
>
> Anders
> ___
> Sent via:Wireshark-dev mailing list 
> Archives:https://www.wireshark.org/lists/wireshark-dev
> Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-dev
>  mailto:wireshark-dev-requ...@wireshark.org
> ?subject=unsubscribe
>
___
Sent via:Wireshark-dev mailing list 
Archives:https://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-dev
 mailto:wireshark-dev-requ...@wireshark.org?subject=unsubscribe


Re: [Wireshark-dev] How to test legacy (glib-compat) code

2021-10-27 Thread John Thacker
On Wed, Oct 27, 2021, 12:54 PM Gerald Combs  wrote:

> The oldest version of GLib that we build with is 2.56.1 on the CentOS 7
> builder:
>
> CentOS 7   2.56.1
> CentOS 8   2.56.4
> Debian 2.66.8
> Fedora 2.68.4
> macOS ARM  2.68.4
> macOS Intel2.58.3
> openSUSE 15.2  2.62.6
> Ubuntu 2.64.6
> Win32  2.66.4
> Win64  2.66.4
>
> Is there any reason we shouldn't increase the minimum GLib version to 2.56
> in the master and 3.6 branches? That would mean that we no longer support
> RHEL 6, but it's currently in "extended life cycle support"
>

The glib on RHEL 6 is so old (2.28) that Wireshark 3.0 doesn't even compile
on RHEL 6, and 3.6 doesn't compile on it for other reasons, including
libgcrypt and gnuTLS.

https://gitlab.com/wireshark/wireshark/-/wikis/Development/Support_library_version_tracking


(Needs some updating with the latest point releases.)

The impact of a minimum of 2.56 would be SUSE Linux Enterprise Server 12.5
(on 2.48),  and earlier point releases / service packs of SLES 15 and RHEL
7.

Of those, the biggest impact is SLES 12, which is supposed to be fully
supported until late 2024. Dropping SLES 12 would enable moving from CMake
3.5 to at least 3.7, and moving to QT 5.7, which would mean assuming C++11
by default.

John Thacker

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


Re: [Wireshark-dev] Non-core cherry pick

2021-10-24 Thread John Thacker
On Sun, Oct 24, 2021 at 9:59 PM chuck c  wrote:

>
> https://gitlab.com/wireshark/wireshark/-/commit/51e1381b235b3fad563f5ec7467ea4e001f2605b
>
> When I select cherry-pick to release-3.6, I get the message "You can only
> create or edit files when you are on a branch".
>
> What's the best way to have this added to 3.6?
>

I don't know if that's the best one to cherry-pick automatically through
the GUI. It changes the NEWS file, which has the version number in text, so
it probably needs some manual attention. However in the general case, I
believe I recall getting that message from the GUI when my personal fork on
Gitlab didn't have the release-3.6 branch mirrored, shortly after that
branch was related. I had to go and update my mirroring settings. Have you
cherry-picked other changes before?

John Thacker
___
Sent via:Wireshark-dev mailing list 
Archives:https://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-dev
 mailto:wireshark-dev-requ...@wireshark.org?subject=unsubscribe


Re: [Wireshark-dev] Is this the bug of wmem_tree_lookup32_array_le()?

2021-10-19 Thread John Thacker
Another option, which should work with the existing API, is to have a
wmem_tree with keys the upper 32 bits whose data nodes are also trees, with
keys the lower 32 bits. (You store it by doing an exact
wmem_tree_lookup32() match, adding a new tree if not found.) Then you do a
normal wmem_tree_lookup32_le() on the upper 32 bits, get the result a tree,
and depending on whether the key that matched result was equal or less than
equal to the upper 32 bits, call wmem_tree_lookup32_le() on the returned
tree with key either the lower 32 bits or on G_MAXUINT32 (the latter will
get you the highest result in the tree, for the less than equal match case.)

John

On Tue, Oct 19, 2021 at 12:21 PM qiangxiong.huang via Wireshark-dev <
wireshark-dev@wireshark.org> wrote:

> John Thacker,
>
> Thank you for your information. I may try to add xxx_insert64/lookup64().
>
> -- Original --
> *From:* "Developer support list for Wireshark" ;
> *Date:* Tue, Oct 19, 2021 10:42 PM
> *To:* "Developer support list for Wireshark";
> *Subject:* Re: [Wireshark-dev] Is this the bug of
> wmem_tree_lookup32_array_le()?
>
> On Sun, Oct 17, 2021 at 5:41 AM qiangxiong.huang via Wireshark-dev <
> wireshark-dev@wireshark.org> wrote:
>
>>
>> Who knows that the current behavior of the wmem_tree_lookup32_array_le()
>> is designed in this way or it is just a bug?
>> If I want to fix the issue #17633, should I modify this
>> wmem_tree_lookup32_array_le() directly or write a new function like
>> wmem_tree_lookup32_array_le_key_as_big_num() (or other name you recommend)?
>>
>
> While I fully understand why you would want a lookup that would be a
> lexicographical sort with the order of the 32 bit integers in the key being
> rank (in order to handle a 64 bit integer as two 32 bit integers), the
> current dissectors that use the API depend on the current behavior and
> would break, e.g.
> https://gitlab.com/wireshark/wireshark/-/blob/master/epan/dissectors/packet-dns.c#L4184-L4185
> and
> https://gitlab.com/wireshark/wireshark/-/blob/master/epan/dissectors/packet-usb.c#L3560-L3574
>
> Generally these dissectors have several independent keys, and what they
> want to match is an exact match on some keys (transaction ID, for example)
> and a less than match on others (e.g., frame number.) They do the less than
> or equal match and then check for strict equality on the keys that need
> that. Handling the lookup like a lexicographical sort would give incorrect
> results for those dissectors; the current behavior is as intended.
>
> So if you're going to do it, you need a new API function. But perhaps what
> you really want is to implement something like wmem_tree_lookup64 that
> supports 64 bit integers directly rather than expecting the callers to
> separate into the upper and lower 32 bits. It is possible to do lookups of
> other types (there's a string lookup using a string comparison function,
> which might be worth looking at since the method for 32 bit integers calls
> the GUINT_TO_POINTER macros and is guaranteed to work for 64 bit integers.)
>
> John Thacker
> ___
> Sent via:Wireshark-dev mailing list 
> Archives:https://www.wireshark.org/lists/wireshark-dev
> Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-dev
>  mailto:wireshark-dev-requ...@wireshark.org
> ?subject=unsubscribe
>
___
Sent via:Wireshark-dev mailing list 
Archives:https://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-dev
 mailto:wireshark-dev-requ...@wireshark.org?subject=unsubscribe


Re: [Wireshark-dev] Is this the bug of wmem_tree_lookup32_array_le()?

2021-10-19 Thread John Thacker
On Sun, Oct 17, 2021 at 5:41 AM qiangxiong.huang via Wireshark-dev <
wireshark-dev@wireshark.org> wrote:

>
> Who knows that the current behavior of the wmem_tree_lookup32_array_le()
> is designed in this way or it is just a bug?
> If I want to fix the issue #17633, should I modify this
> wmem_tree_lookup32_array_le() directly or write a new function like
> wmem_tree_lookup32_array_le_key_as_big_num() (or other name you recommend)?
>

While I fully understand why you would want a lookup that would be a
lexicographical sort with the order of the 32 bit integers in the key being
rank (in order to handle a 64 bit integer as two 32 bit integers), the
current dissectors that use the API depend on the current behavior and
would break, e.g.
https://gitlab.com/wireshark/wireshark/-/blob/master/epan/dissectors/packet-dns.c#L4184-L4185
and
https://gitlab.com/wireshark/wireshark/-/blob/master/epan/dissectors/packet-usb.c#L3560-L3574

Generally these dissectors have several independent keys, and what they
want to match is an exact match on some keys (transaction ID, for example)
and a less than match on others (e.g., frame number.) They do the less than
or equal match and then check for strict equality on the keys that need
that. Handling the lookup like a lexicographical sort would give incorrect
results for those dissectors; the current behavior is as intended.

So if you're going to do it, you need a new API function. But perhaps what
you really want is to implement something like wmem_tree_lookup64 that
supports 64 bit integers directly rather than expecting the callers to
separate into the upper and lower 32 bits. It is possible to do lookups of
other types (there's a string lookup using a string comparison function,
which might be worth looking at since the method for 32 bit integers calls
the GUINT_TO_POINTER macros and is guaranteed to work for 64 bit integers.)

John Thacker
___
Sent via:Wireshark-dev mailing list 
Archives:https://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-dev
 mailto:wireshark-dev-requ...@wireshark.org?subject=unsubscribe


Re: [Wireshark-dev] Byte view mouse hover behaviour

2021-09-11 Thread John Thacker
On Sat, Sep 11, 2021 at 7:23 AM Jaap Keuter  wrote:

> Hi all,
>
> Often when working with captured data from some development setup I wonder
> around the packet bytes with the mouse pointer to try and follow where in
> the code under development certain parts of a packet contents is being
> created. What happens now is that the highlighting of the field where this
> particular bytes are from follows the mouse pointer. But I'm working the
> other way around, by first selecting a field in the packet details, then
> dive into the packet bytes, where I would like the selected field
> highlighting to remain, so I can navigate around while keeping my bearings
> in the packet.
>
> But I recon this will upset people. So putting it behind a preference
> would a the way to go. The the question becomes where to put that? Anyone a
> good idea?
>

I would appreciate that, I have wished for that behavior myself. While one
option is just giving it a name so that people can only access it through
Preferences->Advanced, I think the most natural place is in
Appearance->Layout. There's already a section for Packet List settings
there, that includes a preference for "Enable mouse-over colorization." (
https://gitlab.com/wireshark/wireshark/-/blob/master/epan/prefs.c#L3377-L3380)
Perhaps add a "Packet Bytes settings" group and then the new preference.

Cheers,
John Thacker
___
Sent via:Wireshark-dev mailing list 
Archives:https://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-dev
 mailto:wireshark-dev-requ...@wireshark.org?subject=unsubscribe


[Wireshark-dev] Issues to close

2021-09-10 Thread John Thacker
Here's a few more old migrated from Bugzilla bugs that have been fixed that
can be closed. Can someone with permission please do so?

https://gitlab.com/wireshark/wireshark/-/issues/15216

Zstd and LZ4 support, added by
https://gitlab.com/wireshark/wireshark/-/merge_requests/3901

https://gitlab.com/wireshark/wireshark/-/issues/14520

F5ethtrailer support, dissector was added to the tree three years ago and
issue never closed.

https://gitlab.com/wireshark/wireshark/-/issues/7683

Add reassembled_data field. This patch was applied seven years ago and the
issue never closed. I think it was left open because of immediate thoughts
about backing out the patch because perhaps the reassembled_data field and
the fragments field were redundant (although one is FT_NONE and one
FT_BYTES.) I don't know that is likely after seven years.

Thanks,
John Thacker
___
Sent via:Wireshark-dev mailing list 
Archives:https://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-dev
 mailto:wireshark-dev-requ...@wireshark.org?subject=unsubscribe


[Wireshark-dev] close issues

2021-09-04 Thread John Thacker
BACnet issues
https://gitlab.com/wireshark/wireshark/-/issues/17142
https://gitlab.com/wireshark/wireshark/-/issues/17398

were both fixed by merge
https://gitlab.com/wireshark/wireshark/-/merge_requests/3019

(Merge 3019 tried to automatically close 17142, but the commit message was
slightly off in how Gitlab automatically parses it, and it only viewed it
as "related to.")

Can someone with the proper permissions close them?

Thanks,
John Thacker
___
Sent via:Wireshark-dev mailing list 
Archives:https://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-dev
 mailto:wireshark-dev-requ...@wireshark.org?subject=unsubscribe


Re: [Wireshark-dev] Merge blocked: the source branch must be rebased onto the target branch.

2021-08-27 Thread John Thacker
On Fri, Aug 27, 2021, 12:01 PM Mora, Jorge via Wireshark-dev <
wireshark-dev@wireshark.org> wrote:

> Hello,
>
> I have been trying to push this fix but I don’t know what I am doing wrong
> and what I need to do for this fix to be reviewed. I have not received any
> e-mails that something is wrong but I did see the merge has been blocked.
>
> Can anybody tell me why am I getting this merge blocked?
>
The merge being blocked is not a concern unless there are conflicts between
other commits and your changes

Wireshark uses a restrictive merge policy that only allows fast-forward
commits. What that means is that if any other change has been committed
since you branched, your fix has to be rebased to include those commits.

You will see that message about being blocked until you rebase.

The Gitlab GUI makes this very easy if there are no merge conflicts, as
there's a button that either you or a reviewer can push to rebase. If there
are merge conflicts, then there's a little more work, as described on the
Wiki and documentation.

Reviewing is a different matter, and you will have to wait for one of the
core developers to review and approve it. If there are no conflicts, then
the same developer who reviews it will also rebase it for you when
approving the merge.

John Thacker
___
Sent via:Wireshark-dev mailing list 
Archives:https://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-dev
 mailto:wireshark-dev-requ...@wireshark.org?subject=unsubscribe


Re: [Wireshark-dev] Can the legacy HAVE_LIBGCRYPT_AEAD check be removed?

2021-07-12 Thread John Thacker
On Tue, Jul 13, 2021 at 2:26 AM Pascal Quantin  wrote:

> Hi Matthias,
>
> Le lun. 12 juil. 2021 à 21:22, Dr. Matthias St. Pierre <
> matthias.st.pie...@ncp-e.com> a écrit :
>
>> Hi all,
>>
>> in wsgcrypt.h the libgcrypt version is checked to ensure that it supports
>> AEAD ciphers:
>>
>> #if GCRYPT_VERSION_NUMBER >= 0x010600 /* 1.6.0 */
>> /* Whether to provide support for authentication in addition to
>> decryption. */
>> #define HAVE_LIBGCRYPT_AEAD
>> #endif
>>
>> (see [1])
>>
>> Given the fact that libgcrypt 1.6.0 was released in 2013-12-16 (see [2]),
>> I was wondering whether after such a long time it would be acceptable to
>> simplify
>> all the conditional code depending on HAVE_LIBGCRYPT_AEAD by making
>> GCRYPT_VERSION_NUMBER >= 0x010600 a requirement and removing  the
>> #else code parts.
>>
>> The reason why I am asking is because the packet-isakmp dissector has
>> some conditional code to emulate AES-GCM using AES-CTR in the absence
>> of HAVE_LIBGCRYPT_AEAD which I'd like to remove. I did some git history
>> research and noticed that there were some recent commits in 2020 related
>> to fixing compilation without HAVE_LIBGCRYPT_AEAD  (see [3]). However,
>> I'm not sure whether this was done deliberately to support libgcrypt < 1.6.0
>> or just out of habit because nobody considered the option of removing the
>> legacy check.
>>
>
> If some changes were done to fix the compilation, it means that some
> people still use a libgcrypt version older than 1.6.0. And as seen in our
> CMakeLists.txt file,
> the minimum version required for now is 1.5.0.
>

The minimum version being stuck at 1.5.0 is, I believe, almost entirely due
to RHEL/CentOS 7 being stuck at 1.5.3
(
https://gitlab.com/wireshark/wireshark/-/wikis/Development/Support_library_version_tracking#libgcrypt
)

It's a widely used enough distribution, still scheduled for 3 more years of
support, that I think that for now the current approach of supporting it is
reasonable. We do warn about it in strong terms.
(The RH package may have some backports of features from later versions,
but I haven't looked into it.)

If at some point we want to drop RHEL 7 (and in a similar fashion, SUSE
Enterprise Server 12), there's a number of packages whose version could be
bumped, as seen on that page.

John Thacker
___
Sent via:Wireshark-dev mailing list 
Archives:https://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-dev
 mailto:wireshark-dev-requ...@wireshark.org?subject=unsubscribe


Re: [Wireshark-dev] Calling a dissector: Type for data parameter

2021-06-21 Thread John Thacker
On Mon, Jun 21, 2021 at 9:28 PM João Valverde via Wireshark-dev <
wireshark-dev@wireshark.org> wrote:

>
>
> On 22/06/21 01:26, John Thacker wrote:
> > On Mon, Jun 21, 2021 at 2:21 PM João Valverde via Wireshark-dev
> > mailto:wireshark-dev@wireshark.org>>
> wrote:
> >
> >
> >
> > On 16/06/21 15:36, David Perry wrote:
> >  > Sorry to drag up an old topic, but I've been thinking about this:
> >  >
> >  >> Message: 5
> >  >> Date: Sat, 29 May 2021 09:32:29 +0200
> >  >> From: Anders Broman  > <mailto:a.broma...@gmail.com>>
> >  >> To: Developer support list for Wireshark
> > mailto:wireshark-dev@wireshark.org>>
> >  >> Subject: Re: [Wireshark-dev] Calling a dissector: Type for data
> >  >> parameter
> >  >> Message-ID:
> >  >>
> >   zdycm33pxuwtbctew7gttecslij-f8shw0l-863q5...@mail.gmail.com  zdycm33pxuwtbctew7gttecslij-f8shw0l-863q5...@mail.gmail.com>>
> >  >> Content-Type: text/plain; charset="utf-8"
> >  >>
> >  >> Hi,
> >  >> Yes the method is fragile. At the time of development I think it
> was
> >  >> proposed to pass a struct containing a string and the void
> > pointer where
> >  >> the string could be used as a identifier. But that was voted
> down.
> >  >> Regards
> >  >> Anders
> >  >
> >  > I wasn't around for that discussion so I don't know the reasons,
> but
> >  > how does this sound as a refined approach?:
> >  >
> >  > * Define a `dissector_data_t` that has a `guint32` identifier
> field,
> >  > and a `void *` data field.
> >  >
> >  > * Replace the `void *data` parameter to dissectors with a pointer
> > to a
> >  > `dissector_data_t`.
> >  >
> >  > * Either:
> >  >
> >  > * Easy way: maintain a static list of identifiers that map to
> >  > expected data types, or
> >  >
> >  > * Have dissector X request an identifier in its registration
> >  > function for the type of data it expects, and have dissector Y
> > (which
> >  > will call X) request, in its handoff function, the identifier of
> the
> >  > type of data it needs to pass to X.
> >  >
> >  > * Dissectors check for the right identifier in their
> >  > `dissector_data_t` parameter and don't try to use it if it's
> wrong.
> >  >
> >  > Thoughts?
> >  >
> >
> > I think what you suggest would be the most straightforward fix.
> >
> > To avoid breaking backward compatibility and changing thousands of
> > dissectors at the same time, both of which are highly problematic, it
> > can be done by adding a new dissector type (like it was done with
> > "dissector_cb_t", only using a different signature).[1]
> >
> > Also a giant static list of dissector_data_t type identifiers would
> be
> > pretty clunky. I think we could recycle the protocol registration
> > number
> > for that.
> >
> >
> > Perhaps I don't quite understand, but what would be the point if the
> > protocol registration number were used? Presumably that is the number
> > for the called protocol, based on what David outlined (the called
> > protocol registering what data it expects.) But the calling dissector
> > would always have that number (via dissector_handle_get_protocol_index()
> > ) and pass it in, which wouldn't provide any guarantee that the data
> > passed in was the correct type than what is being done now.
> >
> > The only way that I see it would make sense to pass in an identifier is
> > if a protocol registers multiple data types it might expect to be passed
> > in when called from different types (whether in one dissect_proto()
> > function or multiple ones), in which case the protocol registration
> > number couldn't be used, or if the identifier is instead related to the
> > calling protocol and controlled by it (which is perhaps for this method
> > of calling the wrong dependency direction, unlike with dissector tables
> > where the calling protocol does control the passed data type, e.g.
> > packet-ip always passes a ws_ip4* to the "ip proto" table or its
> > heuristic subdissector table.)
> >
> > That doesn't sound like wh

Re: [Wireshark-dev] Calling a dissector: Type for data parameter

2021-06-21 Thread John Thacker
On Mon, Jun 21, 2021 at 2:21 PM João Valverde via Wireshark-dev <
wireshark-dev@wireshark.org> wrote:

>
>
> On 16/06/21 15:36, David Perry wrote:
> > Sorry to drag up an old topic, but I've been thinking about this:
> >
> >> Message: 5
> >> Date: Sat, 29 May 2021 09:32:29 +0200
> >> From: Anders Broman 
> >> To: Developer support list for Wireshark 
> >> Subject: Re: [Wireshark-dev] Calling a dissector: Type for data
> >> parameter
> >> Message-ID:
> >>  >
> >> Content-Type: text/plain; charset="utf-8"
> >>
> >> Hi,
> >> Yes the method is fragile. At the time of development I think it was
> >> proposed to pass a struct containing a string and the void pointer where
> >> the string could be used as a identifier. But that was voted down.
> >> Regards
> >> Anders
> >
> > I wasn't around for that discussion so I don't know the reasons, but
> > how does this sound as a refined approach?:
> >
> > * Define a `dissector_data_t` that has a `guint32` identifier field,
> > and a `void *` data field.
> >
> > * Replace the `void *data` parameter to dissectors with a pointer to a
> > `dissector_data_t`.
> >
> > * Either:
> >
> > * Easy way: maintain a static list of identifiers that map to
> > expected data types, or
> >
> > * Have dissector X request an identifier in its registration
> > function for the type of data it expects, and have dissector Y (which
> > will call X) request, in its handoff function, the identifier of the
> > type of data it needs to pass to X.
> >
> > * Dissectors check for the right identifier in their
> > `dissector_data_t` parameter and don't try to use it if it's wrong.
> >
> > Thoughts?
> >
>
> I think what you suggest would be the most straightforward fix.
>
> To avoid breaking backward compatibility and changing thousands of
> dissectors at the same time, both of which are highly problematic, it
> can be done by adding a new dissector type (like it was done with
> "dissector_cb_t", only using a different signature).[1]
>
> Also a giant static list of dissector_data_t type identifiers would be
> pretty clunky. I think we could recycle the protocol registration number
> for that.
>

Perhaps I don't quite understand, but what would be the point if the
protocol registration number were used? Presumably that is the number for
the called protocol, based on what David outlined (the called protocol
registering what data it expects.) But the calling dissector would always
have that number (via dissector_handle_get_protocol_index() ) and pass it
in, which wouldn't provide any guarantee that the data passed in was the
correct type than what is being done now.

The only way that I see it would make sense to pass in an identifier is if
a protocol registers multiple data types it might expect to be passed in
when called from different types (whether in one dissect_proto() function
or multiple ones), in which case the protocol registration number couldn't
be used, or if the identifier is instead related to the calling protocol
and controlled by it (which is perhaps for this method of calling the wrong
dependency direction, unlike with dissector tables where the calling
protocol does control the passed data type, e.g. packet-ip always passes a
ws_ip4* to the "ip proto" table or its heuristic subdissector table.)

That doesn't sound like what's being proposed, though, so I am confused.

John Thacker
___
Sent via:Wireshark-dev mailing list 
Archives:https://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-dev
 mailto:wireshark-dev-requ...@wireshark.org?subject=unsubscribe


Re: [Wireshark-dev] Wiki editor permission request

2021-06-10 Thread John Thacker
Be warned, though, it doesn't seem like the changes to the editable wiki
have been reflected in the main public wiki for about a month:

https://gitlab.com/wireshark/wireshark/-/issues/17397

It was working before then.

John Thacker

On Wed, Jun 9, 2021, 1:46 PM Gerald Combs  wrote:

> Done!
>
> On 6/8/21 11:41 PM, Sun Lin via Wireshark-dev wrote:
> > Hi,
> >
> > I would like permission to edit the Wireshark wiki. My GitLab username
> is @lin.sun .
> > There're two pcap example files for OPUS rtp packets to upload to the
> https://gitlab.com/wireshark/wireshark/-/wikis/SampleCaptures .
> >
> > B.R.
> > Lin
> >
> ___
> > Sent via:Wireshark-dev mailing list 
> > Archives:https://www.wireshark.org/lists/wireshark-dev
> > Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-dev
> >   mailto:wireshark-dev-requ...@wireshark.org
> ?subject=unsubscribe
> >
>
> ___
> Sent via:Wireshark-dev mailing list 
> Archives:https://www.wireshark.org/lists/wireshark-dev
> Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-dev
>  mailto:wireshark-dev-requ...@wireshark.org
> ?subject=unsubscribe
>
___
Sent via:Wireshark-dev mailing list 
Archives:https://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-dev
 mailto:wireshark-dev-requ...@wireshark.org?subject=unsubscribe


[Wireshark-dev] Close bugs

2021-05-25 Thread John Thacker
Can someone with the proper permission please close #13238 (
https://gitlab.com/wireshark/wireshark/-/issues/13238)?
It was fixed by commit 81f184bc00b938807cfdee72dc6f8d49412e26c6 three years
ago, but didn't get closed (and no one really owns it after the Bugzilla
migration.)

Also please close #6365 (
https://gitlab.com/wireshark/wireshark/-/issues/6365), which was addressed
with the "Export PDUs to File" functionality and API.

Thanks,
John Thacker
___
Sent via:Wireshark-dev mailing list 
Archives:https://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-dev
 mailto:wireshark-dev-requ...@wireshark.org?subject=unsubscribe


Re: [Wireshark-dev] Question / nit / ocd trigger

2021-05-24 Thread John Thacker
On Mon, May 24, 2021 at 12:19 PM Graham Bloice 
wrote:

>
>
> On Mon, 24 May 2021 at 16:22, Jason Cohen  wrote:
>
>> One thing that has bothered me for years has been the TCP flags filters.
>> ...
>>
>> Is there history, reasoning for this?  Should there be some level of
>> consistency?  I certainly do not advocate for tcp.flags.acknowledgement or
>> tcp.flags.syncronize.  However, I think it would be reasonable for reset
>> and push to be replaced with "rst" and "psh" respectively.  Perhaps an
>> alias to allow the spelled out filters to continue to work.
>>
>>
> While consistency is good and the change seems simple, it will break many
> existing workflows and "muscle memory" and all the many guides\manuals etc.
> out there.  An alias would help going forwards but I think users may still
> become confused.
>
> I class this as the type of change that really needs a time machine to
> allow the correct implementation at the start or maybe a Neuralyzer (
> https://meninblack.fandom.com/wiki/Neuralyzer)
>

For me, from a dissector development standpoint, the all time winner in
this category is "why does packet_info use src and dst for addresses, but
srcport and destport for ports, why isn't it dstport?"

John Thacker
___
Sent via:Wireshark-dev mailing list 
Archives:https://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-dev
 mailto:wireshark-dev-requ...@wireshark.org?subject=unsubscribe


Re: [Wireshark-dev] Ethernet dissector

2021-05-23 Thread John Thacker
On Sun, May 23, 2021 at 1:10 PM Antonello Tartamo <
antonellotart...@gmail.com> wrote:

> I manually added the MAC addresses using proto_tree_add_ether().
> I thought there was a better way.
> Thanks in advance
> Regards
> Antonello
>

If the MAC addresses are just present in the normal way like an Ethernet
header. proto_tree_add_item() would be easier and better in this situation.
You should only have to use proto_tree_add_ether() when you need to
transform the bytes in the buffer in some way. With proto_tree_add_item you
won't have to fetch the bytes from the buffer first and pass them back.
___
Sent via:Wireshark-dev mailing list 
Archives:https://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-dev
 mailto:wireshark-dev-requ...@wireshark.org?subject=unsubscribe


Re: [Wireshark-dev] Ethernet dissector

2021-05-23 Thread John Thacker
On Sun, May 23, 2021 at 12:18 PM John Thacker  wrote:

> On Sun, May 23, 2021 at 11:59 AM Antonello Tartamo <
> antonellotart...@gmail.com> wrote:
>
>> The problem is that I don't have a predefined ether type as the ether
>> type field is used as length field.
>> Is there any other way to reuse the ethernet dissector ?
>> Thanks in advance
>>
>
> So if I understand correctly, you have a protocol that does not contain
> Ethernet, but has a two MAC addresses (destination and source), followed by
> a field which is two octets but *always* is a length field (like a 802.3
> Ethernet frame, not Ethernet II), even if over 1500? Or is it something
> where it's only for lengths less than 1500 bytes, like 802.3 Ethernet, but
> it's not any of the non Ethernet II frame types (raw 802.3 or 802.3
> followed by LLC, with or without SNAP)?
>
> Then it's not on Ethernet, and you need to manually add the source and
> destination addresses in your dissector and not call the Ethernet
> dissector. It's not difficult at all to add two FT_ETHER fields to your
> dissector.
>
> Are you trying to have your protocol work on capture files that claim to
> have an Ethernet link layer, with this not quite compatible link layer
> instead?
>

Note that you can do that, that's what the table you've used is for (a
non-Ethernet packet inside Ethernet framing), you just can't subsequently
call dissect_eth*() with the same tvb again without creating an infinite
loop.

It sounds like what you want is something similar to making
dissect_address_data() packet-eth.c a function in the header instead of
static, because you want the various subfields (U/L, I/G bits, etc.) broken
out like in Ethernet (see
https://gitlab.com/wireshark/wireshark/-/issues/15493), which is a little
bit of a pain.

John Thacker
___
Sent via:Wireshark-dev mailing list 
Archives:https://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-dev
 mailto:wireshark-dev-requ...@wireshark.org?subject=unsubscribe


Re: [Wireshark-dev] Ethernet dissector

2021-05-23 Thread John Thacker
On Sun, May 23, 2021 at 11:59 AM Antonello Tartamo <
antonellotart...@gmail.com> wrote:

> The problem is that I don't have a predefined ether type as the ether type
> field is used as length field.
> Is there any other way to reuse the ethernet dissector ?
> Thanks in advance
>

So if I understand correctly, you have a protocol that does not contain
Ethernet, but has a two MAC addresses (destination and source), followed by
a field which is two octets but *always* is a length field (like a 802.3
Ethernet frame, not Ethernet II), even if over 1500? Or is it something
where it's only for lengths less than 1500 bytes, like 802.3 Ethernet, but
it's not any of the non Ethernet II frame types (raw 802.3 or 802.3
followed by LLC, with or without SNAP)?

Then it's not on Ethernet, and you need to manually add the source and
destination addresses in your dissector and not call the Ethernet
dissector. It's not difficult at all to add two FT_ETHER fields to your
dissector.

Are you trying to have your protocol work on capture files that claim to
have an Ethernet link layer, with this not quite compatible link layer
instead?

John Thacker
___
Sent via:Wireshark-dev mailing list 
Archives:https://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-dev
 mailto:wireshark-dev-requ...@wireshark.org?subject=unsubscribe


Re: [Wireshark-dev] Ethernet dissector

2021-05-23 Thread John Thacker
On Sun, May 23, 2021 at 8:06 AM Antonello Tartamo <
antonellotart...@gmail.com> wrote:

> Hello everyone,
> I'm trying to create an ethernet dissector for a custom protocol working
> on L2.
>
> In proto_reg_handoff_myproto() function I've called:
> heur_dissector_add("eth", dissect_myproto, "MyProtocol", "mp", proto_mp,
> HEURISTIC_ENABLE);
> eth_handle = find_dissector("eth_withoutfcs");
>
> then in the dissect_myproto function when I call:
> tvbuff_t* next_tvb = tvb_new_subset_remaining(tvb, 0);
> int new_off = call_dissector(eth_handle, tvb, pinfo, tree);
> return new_off;
>
> I get the following two errors on the terminal:
> ** (wireshark:11483): WARNING **: 07:31:59.826: Dissector bug, protocol
> Ethernet, in packet 12: /home/osboxes/Devel/wireshark/epan/packet.c:2794:
> failed assertion "saved_layers_len < 500"
>
> ** (wireshark:11483): WARNING **: 07:31:59.826: Dissector bug, protocol
> Ethernet, in packet 12: /home/osboxes/Devel/wireshark/epan/packet.c:775:
> failed assertion "saved_layers_len < 500"
>
> I'm running the development wireshark with ./run/wireshark.
>
> I think the error is due to the fact the both the heuristic dissector and
> the "find_dissector" are ethernet based.
> Is there another way to reuse the ethernet dissector and avoid manually
> adding to the tree the src/dst mac addresses and the ethertype ?
>

The error is that the number of layers in the packet is too large (and that
variable is only 8 bit.) While it's possible to run into that assertion
legitimately with some protocols that have a disgusting amount of PDUs and
encapsulation, you have an infinite loop.
eth_handle calls dissect_eth_common, which calls dissector_try_heuristic
which eventually calls your dissect_myproto.
But dissect_myproto hands the tvb back unchanged to the Ethernet dissector,
which will call dissect_myproto, ad infinitum.

Is dissect_myproto being called in any other way? If not, then there's no
reason to call eth_handle there after you've registered it as a heuristic
dissector with Ethernet. It doesn't call the Ethernet dissector; it's
called by it. (It's also fine if it's being called by
dissector_add_uint("ethertype", ETHERTYPE_MYPROTO, myproto_handle) or
dissector_add_for_decode_as[_with_preference]("ethertype", myproto_handle)
as well.)

If it's being called by something else (whether a custom DLT or whatever),
then whatever else is calling it shouldn't call the same function as being
registered in the heuristic dissector. It should call a different function.

John Thacker
___
Sent via:Wireshark-dev mailing list 
Archives:https://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-dev
 mailto:wireshark-dev-requ...@wireshark.org?subject=unsubscribe


Re: [Wireshark-dev] How to disable QT_MULTIMEDIA_LIB during cmake

2021-04-28 Thread John Thacker
In general some features can be disabled, see CMakeOptions.txt for a list,
but Qt Multimedia Lib cannot be disabled easily.

If you look at cmakeconfig.h.in:
<https://gitlab.com/wireshark/wireshark/-/blob/master/cmakeconfig.h.in#L320>

/* Define to the version of this package. */
#cmakedefine PACKAGE_VERSION

/* Define if we have QtMultimedia */
#define QT_MULTIMEDIA_LIB 1

/* Define if we have QtMacExtras */
#cmakedefine QT_MACEXTRAS_LIB 1

You will see that QT_MULTIMEDIA_LIB is always defined.

If you *both* change that line to

#cmakedefine QT_MULTIMEDIA_LIB 1

*and*

comment out Qt5Multimedia from CMakeLists.txt
<https://gitlab.com/wireshark/wireshark/-/blob/master/CMakeLists.txt#L1179>

and then rerun cmake (possibly after removing config.h and build.ninja or
your Makefile from the build directory),
then you can build without QT_MULTIMEDIA_LIB being set. On the current
master, when doing so, I get the following build error:

In file included from
ui/qt/qtui_autogen/EWIEGA46WW/../../../../../wireshark/ui/qt/rtp_stream_dialog.h:16,
 from
ui/qt/qtui_autogen/EWIEGA46WW/../../../../../wireshark/ui/qt/main_window.h:78,
 from ui/qt/qtui_autogen/EWIEGA46WW/moc_main_window.cpp:10,
 from ui/qt/qtui_autogen/mocs_compilation.cpp:65:
ui/qt/qtui_autogen/EWIEGA46WW/../../../../../wireshark/ui/qt/rtp_player_dialog.h:27:10:
fatal error: QAudioDeviceInfo: No such file or directory
   27 | #include 
  |  ^~

Because QAudioDeviceInfo is part of Qt Multimedia but not properly
protected by that #ifdef, which is exactly what you're trying to test.

John Thacker


On Mon, Apr 26, 2021, 5:03 AM Jirka Novak  wrote:

> Hi,
>
>   I would like to test whether #ifdef QT_MULTIMEDIA_LIB are correct in
> source code so I need to disable it during cmake detection. Is there
> something like there was --nofeature in autoconfigure?
>
> Best regards,
>
> Jirka
> ___
> Sent via:Wireshark-dev mailing list 
> Archives:https://www.wireshark.org/lists/wireshark-dev
> Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-dev
>  mailto:wireshark-dev-requ...@wireshark.org
> ?subject=unsubscribe
___
Sent via:Wireshark-dev mailing list 
Archives:https://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-dev
 mailto:wireshark-dev-requ...@wireshark.org?subject=unsubscribe

Re: [Wireshark-dev] How to build the simple ASN.1 UDP-based dissector example (foo)

2021-04-13 Thread John Thacker
Depending on your build system, the other ASN.1 dissectors can be
regenerated with either

ninja asn1
  Or
make asn1

without starting the full build process.

I do it all the time, last did it a week ago when developing on Linux and
just did it two minutes ago before writing this email to check the command.
That is for the built in ones not a new custom one, but I have added a
custom dissector before and it works perfectly fine on Linux.


On Tue, Apr 13, 2021, 11:23 AM Vincent Randal  wrote:

> Before or After running cmake (in Linux) there are no files containing
> "generate_dissector" in the filename in asn1/foo. And they do not exist
> anywhere in the source tree.
>
> How did the other asn1 dissectors get generated in Linux? When was the
> last time anyone generated or updated a dissector in epan/dissectors/asn1
> using Linux? Maybe this step has been broken for a while in Linux (assuming
> it works in Windows).
>
> On Tue, Apr 13, 2021 at 9:03 AM Pascal Quantin 
> wrote:
>
>> Hi Vicent,
>>
>> Le mar. 13 avr. 2021 à 16:53, Vincent Randal  a
>> écrit :
>>
>>> I should give that a try. What version of Windows and tools are you
>>> using? I’m beat. I need to do something that works soon.
>>>
>>
>> No need to install Windows; it works the same way on Linux. Go to your
>> build folder and run:
>> ninja epan/dissectors/asn1/foo/generate_dissector-foo
>> Assuming you are using ninja, otherwise replace it by make.
>> The target should have been created iby running CMake after the addition
>> of the CMakeListsCustom.txt file.
>>
>> Best regards.
>>
>>
>>> On Tue, Apr 13, 2021 at 8:47 AM Anders Broman via Wireshark-dev <
>>> wireshark-dev@wireshark.org> wrote:
>>>
>>>> Hi,
>>>>
>>>> I don’t think they are generated what will be generated are the files
>>>> needed to *DO *the generation.
>>>>
>>>> On windows the next step is to run
>>>>
>>>> msbuild /m /p:Configuration=RelWithDebInfo
>>>> epan\dissectors\asn1\h248\generate_dissector-h248.vcxproj
>>>>
>>>> which will the generate the .c and .h files
>>>>
>>>> Regards
>>>>
>>>> Anders
>>>>
>>>>
>>>>
>>>> *From:* Wireshark-dev  *On Behalf
>>>> Of *Vincent Randal
>>>> *Sent:* den 13 april 2021 16:40
>>>> *To:* Developer support list for Wireshark >>> >
>>>> *Subject:* Re: [Wireshark-dev] How to build the simple ASN.1 UDP-based
>>>> dissector example (foo)
>>>>
>>>>
>>>>
>>>> Correct insofar as there are no generated files associated with
>>>> asn1/foo directory. Namely, packet-foo.c and packet-foo.h did not get
>>>> generated. But maybe that's not definitive proof that asn1/foo dissector
>>>> did not get built. How else can I confirm the dissector was or was not
>>>> built? Open Wireshark and attempt to apply "foo" as a display filter? It's
>>>> not there.
>>>>
>>>>
>>>>
>>>> On Tue, Apr 13, 2021 at 8:10 AM Anders Broman via Wireshark-dev <
>>>> wireshark-dev@wireshark.org> wrote:
>>>>
>>>> Hi,
>>>>
>>>> So you are saying that if you create foo dir like
>>>>
>>>> epan/dissectors/asn1/foo/
>>>>
>>>> Rename and update the custom cmake file to
>>>>
>>>> set(CUSTOM_ASN1_SRC_DIR
>>>>
>>>>foo
>>>>
>>>> )
>>>>
>>>> And place your source file and cmake.txt in the foo dir then rerun the
>>>> cmake process
>>>>
>>>> Nothing happens?
>>>>
>>>> Try to delete the build dir  before rerunning cmake again?
>>>>
>>>> I’m not sure on linux if the generate cmake file ends up under the
>>>> build dir or in the source dir.
>>>>
>>>>
>>>>
>>>> Regards
>>>>
>>>> Anders
>>>>
>>>> *From:* Wireshark-dev  *On Behalf
>>>> Of *Vincent Randal
>>>> *Sent:* den 13 april 2021 15:32
>>>> *To:* Developer support list for Wireshark >>> >
>>>> *Subject:* Re: [Wireshark-dev] How to build the simple ASN.1 UDP-based
>>>> dissector example (foo)
>>>>
>>>>
>>>>
>>>> I tried renaming ./epan/dissectors/asn1/CMa

Re: [Wireshark-dev] How to build the simple ASN.1 UDP-based dissector example (foo)

2021-04-13 Thread John Thacker
On Tue, Apr 13, 2021, 8:32 AM Vincent Randal  wrote:

> Hello everyone,
>
> I need help building the simple ASN.1 UDP-based dissector example (foo);
> specifically, I need help building the generate_dissector-*proto* target
> (Step #6 below). I'm certainly missing something here.
>
>
> (c) I created directory "foo" by extracting the attachment (foo.tgz) in
> epan/dissectors/asn1/
> (d) There is a CMakeListsCustom.txt.example file in epan/dissectors/asn1
> which already contains an entry for "foo".
> (e) Since I don't know what to do in Step #6, I build Wireshare (using
> cmake), but no build targets get updated.
>
> If the solution to this problem belongs in the Wireshark documentation, I
> would be glad to help update the documentation. Namely, I don't understand
> the usage of the 5 (five) CMakeListsCustom.txt.example files inWireshark
> source code.
>

The CMakeListsCustom.txt.example files are just that, examples. You need to
copy or rename them to CMakeListsCustom.txt (without the .example) for them
to have any effect (and edit them appropriately to add the dissector name
to the list, not commented out.)

CMake is configured to look for the files by that name.

John Thacker
___
Sent via:Wireshark-dev mailing list 
Archives:https://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-dev
 mailto:wireshark-dev-requ...@wireshark.org?subject=unsubscribe

Re: [Wireshark-dev] Fwd: ASN.1 dissector Wireshark

2021-04-10 Thread John Thacker
On Sat, Apr 10, 2021 at 11:15 PM Vincent Randal  wrote:

> Greetings,  everyone. I’ve a question regarding:
>
> https://www.wireshark.org/docs/wsdg_html_chunked/Asn1DissectorRequirements.html
>
> I take this to mean Wireshark ASN.1 support might not be up to date going
> forward. Is that the meaning? ASN.1 has a long history. It makes sense
> periodically to freeze support for specific versions of ASN.1. Is that the
> meaning here (Changing the ASN1 file is being depreciated)? It says:
>
> The compiler needs 4 input files: an ASN.1 description of a protocol, a
> .cnf file, and two template files. The ASN.1 specification may have to be
> edited to work, however work is in progress to at least read all ASN1
> specifications. Changing the ASN1 file is being depreciated as this creates
> problems when updating protocols. The H.248 Binary encoding dissector is a
> good example of a dissector with relatively small changes.
>
>
No, I take that to mean:

Many protocols (especially those associated with the ITU-T or ETSI) are
formally specified in ASN.1. Sometimes it is difficult for asn2wrs.py, the
ASN.1 to Wireshark dissector compiler, to translate the ASN.1 specification
into code that works optimally (and in rare cases, even at all) as a
Wireshark dissector. (Fairly trivial examples include protocols that define
certain fields as OCTET STRINGs of length 1, 2, 3, or 4, when in Wireshark
they would be better displayed as uint{8,16,24,32}, but much more
complicated examples exist.)

In such cases, the preferred method is to edit the .cnf file or the
template files (packet-PROTOABBREV-template.[ch]), rather than edit the
ASN.1 description directly, as the latter comes from the official protocol
specification as defined by the standards body. While the ultimate goal is
for asn2wrs.py to at least successfully read all ASN.1 specifications and
produce usable output (even if the .cnf file or template files must be
adjusted), in rare cases it may fail to do so, and then it will be
necessary to edit the ASN.1 file itself. However, editing the file is
deprecated and should be viewed as a last resort, as any such changes would
have to be reapplied when the protocol is updated to a later released
version of the specification with a new official ASN.1 definition. Problems
sometimes result, as occurs anytime forked code has to be merged.

John Thacker
___
Sent via:Wireshark-dev mailing list 
Archives:https://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-dev
 mailto:wireshark-dev-requ...@wireshark.org?subject=unsubscribe

Re: [Wireshark-dev] tvb_get_nstringz0

2021-03-27 Thread John Thacker
On Sat, Mar 27, 2021 at 2:57 PM Dario Lombardo  wrote:

> Hi John,
> thanks, your explanation helped a lot. However I still don't get why the
> code crashes. Please let me use the actual buffer sizes since the ones I
> told before were examples. The packet is 49, the local buffer is 15.
>
> When you call tvb_get_nstringz0() you pass in bufsize = 15.
>> tvb_get_nstringz0() calls _tvb_get_nstringz()
>> check_offset_len() runs to the end of the packet, setting len to 49.
>> Since len >= bufsize, it sets limit = bufsize.
>> stringlen = tvb_strnlen(tvb, abs_offset, limit - 1) looks at the first 9
>> bytes, doesn't find a NUL, returns -1
>>
>
> That's a point I don't get. This piece of code (stringlen =
> tvb_strnlen(tvb, 0, 14)) actually returns 49. Despite the fact that NULL is
> present or not, shouldn't this function fulfill the (limit - 1)? Shouldn't
> that return 14 at most?
>
>
>> stringlen is -1, tvb_memcpy copies over limit (10) bytes into buffer from
>> tvb, bytes_copies is set to 10, _tvb_get_nstringz() returns -1.
>>
>
> That's where things start to get hairy: stringlen is 49, then the actual
> copy starts against buffer, that is only 15 bytes long. Crash.
>

Huh, that's odd. I just added some lines to packet-http.c near the top of
dissect_http_tcp() like so:

guint bufsize = 19;
guint8 buffer[19];
tvb_get_nstringz0(tvb, 0, bufsize, buffer);

And stepped through with gdb after setting a breakpoint and opening a file
with a ~480 byte HTTP packet with no nulls in the HTTP layer:

3578 stringlen = tvb_strnlen(tvb, abs_offset, limit - 1);
(gdb) print limit -1
$12 = 18
(gdb) n
3580 if (stringlen == -1) {
(gdb) print stringlen
$14 = -1
(gdb) print tvb->length
$15 = 479

And all proceeds as expected.

tvb_strnlen(tvb, offset, maxlength) is supposed to return -1 if 'maxlength'
(here, 14) is reached before a NULL.

Let's see, tvb_strnlen calls tvb_find_guint8(tvb, abs_offset, maxlength=14,
0), and returns -1 if and only if tvb_find_guint8(...) returns -1. That
looks right, so let's look at tvb_find_guint8():

DISSECTOR_ASSERT(tvb && tvb->initialized);

exception = compute_offset_and_remaining(tvb, offset, _offset,
);
if (exception)
THROW(exception);

/* Only search to end of tvbuff, w/o throwing exception. */
if (maxlength >= 0 && limit > (guint) maxlength) {
/* Maximum length doesn't go past end of tvbuff; search
   to that value. */
limit = (guint) maxlength;
}

Have you tried stepping through it with a debugger? The code looks right to
me and runs correctly over here. Are you perhaps somehow hitting an
exception or a failed assertion?

John
___
Sent via:Wireshark-dev mailing list 
Archives:https://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-dev
 mailto:wireshark-dev-requ...@wireshark.org?subject=unsubscribe

Re: [Wireshark-dev] tvb_get_nstringz0

2021-03-26 Thread John Thacker
On Fri, Mar 26, 2021 at 4:22 AM Dario Lombardo  wrote:

> Hi,
> I am a bit puzzled by the use of tvb_get_nstringz0. Let's say I have a
> packet 100 bytes long, that does NOT contain NUL. I call tvb_get_nstringz0
> with a buffer 10 bytes long.
> For what I can see, the function will seek the packet for NUL, stopping at
> the end of the packet, copying the result into the buffer. But the buffer
> is too short, resulting in a crash.
> What's the error here? How is the caller sure their call won't be invalid?
> Should they always pass a long-enough buffer? Was the call to this function
> wrong in the first place?
>

Hmm, looks okay to me.

When you call tvb_get_nstringz0() you pass in bufsize = 10.
tvb_get_nstringz0() calls _tvb_get_nstringz()
check_offset_len() runs to the end of the packet, setting len to 100.
Since len >= bufsize, it sets limit = bufsize.
stringlen = tvb_strnlen(tvb, abs_offset, limit - 1) looks at the first 9
bytes, doesn't find a NUL, returns -1
stringlen is -1, tvb_memcpy copies over limit (10) bytes into buffer from
tvb, bytes_copies is set to 10, _tvb_get_nstringz() returns -1.
Since -1 was returned, tvb_get_nstringz0() sets buffer[9] to NULL, and
returns 9.

The caller assures that the call won't be invalid by passing in the size of
the buffer.

John Thacker
___
Sent via:Wireshark-dev mailing list 
Archives:https://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-dev
 mailto:wireshark-dev-requ...@wireshark.org?subject=unsubscribe

Re: [Wireshark-dev] New Protocol encapsulation as plugin

2021-01-27 Thread John Thacker
On Wed, Jan 27, 2021 at 6:16 AM Björn <
bjoern.peter...@missinglinkelectronics.com> wrote:

> Hi,
>
> we use a custom dissector to analyze custom protocol traffic. However, to
> further increase the usability, we need to add protocol analysis specific
> GUI elements. For now, we are not aware of a way to add a first level
> plugin which can be called through an encapsulation type from a pcap file.
> One other point is that we are not able to load a compiled plugin to
> wireshark, if we don’t build it from source. We can’t link against
> wireshark and cmake will not load the project if we install wireshark from
> the APT packages.
>
>1. Are implementations available to add an encapsulation type via a
>plugin?
>2. Could anybody point us to examples of similar attempts?
>3. Is there already some work in progress to provide such a plugin
>mechanism for extending the encapsulation types?
>4. We noticed that distributed packets, e.g. in Ubuntu 18.04 do not
>allow for C plugins to be loaded. Do you know if this is common practice?
>
>
The approach I generally do is to generate files with one of the USER
encapsulations (which are reserved for private use), and then call your
plugin using the DLT_USER preferences, as detailed here:

https://gitlab.com/wireshark/wireshark/-/wikis/HowToDissectAnything

You can then go on to save those DLT_USER preferences in a configuration
profile
,
and later export that configuration profile and distribute it with your
plugin so that it is installed as a globally available configuration
profile.

Is there some reason that doesn't work for you? If you're able to generate
pcaps with a custom link-layer header type, then you should be able to do
that.
Adding a new encapsulation is possible, but to do it properly it's best to
keep it in sync with the link-layer header types in libpcap files, which
means following the process in wiretap/pcap-common.c

Reusing an existing link-layer header type for a different (newly defined)
Wireshark encapsulation is strongly discouraged.

John
___
Sent via:Wireshark-dev mailing list 
Archives:https://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-dev
 mailto:wireshark-dev-requ...@wireshark.org?subject=unsubscribe

Re: [Wireshark-dev] Display of UTF-8 Characters

2020-12-12 Thread John Thacker
The problem is the sigma. The black diamond with a question mark is the
UTF-8 REPLACMENT CHARACTER, which
is being inserted twice for the two bytes that make up the character.

There was an issue with UTF-8 sigma and other Greek letters (
https://gitlab.com/wireshark/wireshark/-/issues/17070)
that was fixed in the recently released 3.2.9, 3.4.1, and master, but would
be broken in 3.3.0, where it would appear as that.

A workaround would be to use proto_tree_add_string_format_value() with the
last two parameters "%s" and your string value again,
which ends up bypassing the flawed format_text() function in that version.
Or upgrade or get the patch from that bug.

John Thacker


On Sat, Dec 12, 2020 at 1:43 PM  wrote:

> I create a GString str = “A{Dagger}B{Sigma}C”; (i.e.
> “\x41\xE2\x80\xA0\x42\xCE\xA3\x43” where \xE2\x80\xA0 is Dagger and
> \xCE\xA3 is Sigma).
>
> The Dagger is the correct UTF-8 code (
> https://www.fileformat.info/info/unicode/char/2020/index.htm)
>
> and the Sigma is the correct UTF-8 code (
> https://www.fileformat.info/info/unicode/char/03a3/index.htm).
>
>
>
> I use col_append_lstr(pinfo->cinfo, COL_INFO, str,
> COL_ADD_LSTR_TERMINATOR);
>
> The display is “A{Dagger}B{Sigma}C” where the {Dagger} and {Sigma} are the
> correct visual single characters.
>
>
>
> I use proto_string_add_string(…, str);
>
> The display is
> “A{Dagger}B{black-diamond-with-question-mark}{black-diamond-with-question-mark}C”
> where the {black-diamond-with-question-mark} is the visual single character
> of a black diamond with a question mark (and it is displayed twice).
>
>
>
> So col_append_lstr handles UTF-8 and proto_string_add_string partially
> handles UTF-8.
>
>
>
> How can I get a proto_string_* function that will display UTF-8 correctly
> like col_append_lstr does?
>
> I do not need any string function to validate my UTF-8 bytes (if I make a
> mistake, that’s my problem). I just want a consistent display.
>
>
>
> Environment:
>
> Windows 10 Enterprise (10.0.18363) x64
>
> Microsoft Visual Studio Community 2019 Version 16.7.1
>
> QT v5.15.0 using msvc2019_64
>
> Wireshark 3.3.0 with customer dissector
>
> Wireshark Font Consolas Regular 12.0
> ___
> Sent via:Wireshark-dev mailing list 
> Archives:https://www.wireshark.org/lists/wireshark-dev
> Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-dev
>  mailto:wireshark-dev-requ...@wireshark.org
> ?subject=unsubscribe
___
Sent via:Wireshark-dev mailing list 
Archives:https://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-dev
 mailto:wireshark-dev-requ...@wireshark.org?subject=unsubscribe

[Wireshark-dev] MacOS buildbot

2020-12-06 Thread John Thacker
>From what I can see from the MacOS buildbot's CMake logs (e.g.
https://buildbot.wireshark.org/wireshark-master/builders/macOS%2010.14%20x64/builds/811/steps/compile_2/logs/stdio),
it doesn't seem to have any of the support library updates for the last six
months (so nothing from
https://gitlab.com/wireshark/wireshark/-/commit/ec65f1d9e203ae0a01107941abff7e57b6b4180a
or later). E.g. GnuTLS is still 3.4.17.

Is anyone able to update the libraries by running tools/macos-setup.sh or
similar?

John Thacker
___
Sent via:Wireshark-dev mailing list 
Archives:https://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-dev
 mailto:wireshark-dev-requ...@wireshark.org?subject=unsubscribe

Re: [Wireshark-dev] Packet Diagram shows only raw bytes of a subtree instead of individual fields

2020-11-26 Thread John Thacker
On Thu, Nov 26, 2020 at 1:19 PM Maynard, Christopher via Wireshark-dev <
wireshark-dev@wireshark.org> wrote:

> Many protocols contain subtrees, such as a header with various fields that
> are part of the header, and it’s convenient/logical to group those fields
> within the header subtree.  However, doing so results in a Packet Diagram
> that only shows the raw bytes of the subtree rather than the individual
> fields contained within the subtree.
>
> So either I’m doing something wrong, in which case I welcome any
> suggestions for improving the display, or there seems to be a current
> limitation to the way the Packet Diagram behaves with respect to subtrees.
> Has anyone else noticed this?
> ...
>
> Is there a way to achieve this while still grouping the fields within a
> subtree?
>

Not in a subtree currently. If you look around line 600 of
ui/qt/packet-diagram.cpp, you'll see that it only groups the top level
fields in each protocol.

For the same reason, bitmask fields that are grouped together not in a
subtree, using proto_tree_add_bitmask_list()
(like packet-rtp.c#L2072 with octet1_fields), then they are displayed
separately (in master, post commit
https://gitlab.com/wireshark/wireshark/-/commit/7654bb260d08fdb7adeafce1877fa3c38f3465ae
), whereas
for bitmask fields that are added with a subtree with
proto_tree_add_bitmask() only the top level header
item appears.

You can see some images here:
https://gitlab.com/wireshark/wireshark/-/merge_requests/959
There you can see bitmask fields that are displayed properly because there
is no subtree.

I agree it would be a nice enhancement to travel down into the children of
items that have children, though I imagine
you'd have to take care in some cases; e.g., dissect_e164_msisdn() from
packet-e164.[ch] is used a lot in various dissectors,
and has a header that has the entire number, with child that only has the
country code (but not a child for the non country code digits).
The simplest way to descend into the subtree for a E.164 number would thus
only has an entry for the country code but leave the
other bits blank. Or you could have issues with dealing with overlaps.

John Thacker
___
Sent via:Wireshark-dev mailing list 
Archives:https://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-dev
 mailto:wireshark-dev-requ...@wireshark.org?subject=unsubscribe

Re: [Wireshark-dev] Indicate [Generated] field on Display Filter Reference page

2020-11-14 Thread John Thacker
On Sat, Nov 14, 2020, 4:14 PM Guy Harris  wrote:

>
> I.e., "generated" is a property of an instance of a field, not of a field;
> that's why the code sample you showed has a separate call to
> proto_item_set_generated().
>
> That may not be the case for *all* fields that have generated instances,
> but there might be some fields that have both generated and non-generated
> instances.
>

For an example, take a look at packet-tftp.c

TFTP packets with the read request and write request opcodes contain a
source or destination filename, respectively, and in those packets the
corresponding fields are added without set_generated()

For packets with other opcodes, conversation tracking is used to add the
source or destination filename, as appropriate, for the ongoing transfer,
and there set_generated() is used because it's not actually in those
packets.

John Thacker

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

Re: [Wireshark-dev] How to close migrated issues without a merge?

2020-10-26 Thread John Thacker
My understanding is that people have to be made members at the "Reporter"
level or higher in order to close issues (which is moving issues to the
Closed list) or otherwise manage them (label, etc.)

https://docs.gitlab.com/ee/user/project/issue_board.html#permissions
https://docs.gitlab.com/ee/user/permissions.html#project-members-permissions

Since the project doesn't have people with Reporter level membership, it's
really just the core team members that can close issues.
(Effectively, merging gates them the same way since the core team must
approve merges.)

Here's another obvious example that should be closed, regarding Bugzilla
and MIME types of files:
https://gitlab.com/wireshark/wireshark/-/issues/8476

John

On Mon, Oct 26, 2020, 8:26 PM chuck c  wrote:

> Example closed with a merge:
> https://gitlab.com/wireshark/wireshark/-/issues/16000
>  Opened 1 year ago by Wireshark GitLab MigrationGuest
> AndersBroman @AndersBroman closed via merge request !718 (merged) 15 hours
> ago
>
> No code to merge. Doing housekeeping.
> https://gitlab.com/wireshark/wireshark/-/issues/16347
> Opened 9 months ago by Wireshark GitLab MigrationGuest
>
> Added a comment showing the fix but can't close the issue.
> Is there a way to assign issues back to the person that opened it on
> bugziila?
> ___
> Sent via:Wireshark-dev mailing list 
> Archives:https://www.wireshark.org/lists/wireshark-dev
> Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-dev
>  mailto:wireshark-dev-requ...@wireshark.org
> ?subject=unsubscribe
___
Sent via:Wireshark-dev mailing list 
Archives:https://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-dev
 mailto:wireshark-dev-requ...@wireshark.org?subject=unsubscribe

[Wireshark-dev] Adventures in cherry-picking

2020-10-24 Thread John Thacker
Right now, cherry-picking via the GitLab GUI seems pretty broken. It
appends to the commit message some informative text[1] like:

"

(cherry picked from commit 273eb06390821597d632606587ad79a8e152e122)"

Which is reasonably helpful enough, but adds the two blank lines, not just
one[2], in recent cases I've tested. The extra blank line causes it to fail
the "git stripspace" check in tools/validate.commit.py and thus fail the
pipelines. There's no way around this from the GUI, since the commit
message is automatically created. Changing the merge request message
doesn't affect the commit itself; the cherry-picked branch can be pulled
and altered on the command line, but one might as well just use the CLI for
all of the cherry-picking instead of GitLab's GUI.

Checking, I see that this has hit other people as well:

https://gitlab.com/wireshark/wireshark/-/merge_requests/695

Another irritating thing I have found with cherry-picking into multiple
branches (e.g., release-3.4 and master-3.2) from the original wireshark
repository is that GitLab automatically names cherry-pick branches using
only the commit name but not the target branch.[3]

What's worse is that if I create a cherry pick branch for release-3.4, and
then immediately go back to the same commit in wireshark master and try to
create a cherry-pick branch for master-3.2, I get an error in the GUI like:

"A branch called 'cherry-pick-e42cc671' already exists. Switch to that
branch in order to make changes"

If I slightly vary things and create one cherry-pick from the main GitLab
repository's master and the other cherry-pick from my local fork of master,
then I do get two branches, one named 'cherry-pick-{hash}' and the other
named 'cherry-pick-{hash}-2'. I'm not sure where I'd turn to get a
cherry-pick into the target master-3.0. Perhaps cherry-picking a
cherry-picked commit.

Has anyone else run into this, and do you have a workaround? Our existing
workflow has been built around cherry-picking, and this makes it harder.

John Thacker

[1] - https://gitlab.com/gitlab-org/gitlab-foss/-/issues/32348
[2] - https://gitlab.com/gitlab-org/gitlab-foss/-/merge_requests/13475/diffs
- The code has been moved around this since this merge, but it's the same
behavior
[3] - https://gitlab.com/gitlab-org/gitlab/-/issues/18825
https://gitlab.com/gitlab-org/gitlab/-/issues/17462
___
Sent via:Wireshark-dev mailing list 
Archives:https://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-dev
 mailto:wireshark-dev-requ...@wireshark.org?subject=unsubscribe

Re: [Wireshark-dev] lua decoder accessing info from layers above

2020-10-12 Thread John Thacker
On Mon, Oct 12, 2020, 8:00 PM Fulko Hew  wrote:

>
> I'm trying to update/improve someone else's decoder written in Lua.
> It's for a simple UDP (and TCP) based protocol.
> But I need to be able to get access to the upper layer
> to be able to decode it easily.
>
> Can someone provide an example of how to determine if the higher layer was
> UDP or TCP?
>

If all you need to know is whether it was called from TCP or UDP, then a
typical approach is to create slightly different dissector functions, one
for TCP and one for UDP (they can then call a common function, setting a
parameter) and register the TCP dissector with TCP and the UDP dissector
with UDP. That approach for C dissectors is demonstrated, for example, here:

https://gitlab.com/wireshark/wireshark/-/blob/master/doc/README.heuristic

And it's pretty similar for Lua dissectors using some of the examples
linked from here:

https://gitlab.com/wireshark/wireshark/-/wikis/Lua

John Thacker
___
Sent via:Wireshark-dev mailing list 
Archives:https://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-dev
 mailto:wireshark-dev-requ...@wireshark.org?subject=unsubscribe

Re: [Wireshark-dev] So how are fixes cherry-picked to release branches?

2020-10-03 Thread John Thacker
On Wed, Sep 2, 2020 at 4:03 AM Guy Harris  wrote:

> Searching for "cherry" on the Wiki finds these pages:
>
>
> https://gitlab.com/wireshark/wireshark/-/wikis/Development/Backporting
>
>
> https://gitlab.com/wireshark/wireshark/-/wikis/Development/Workflow
>
>
> https://gitlab.com/wireshark/wireshark/-/wikis/Development/SubmittingPatches/Git-Review
>
> all of which contain the name "Gerrit".
>
> We should probably update those to reflect whatever the new procedure is.
>

The new procedure for cherry-picking is elucidated in:

https://gitlab.com/wireshark/wireshark/-/wikis/Development/SubmittingPatches#backporting-a-change-to-a-release-branch

The Roadmap page (
https://gitlab.com/wireshark/wireshark/-/wikis/Development/Roadmap)
previously had a link to
the Backporting page, but I moved the link to the subsection of
Development/SubmittingPatches.

The three pages you list above are, from what I can tell by searching the
Wiki, now orphen pages that nothing else in the Wiki links to. I don't
think that they contain any useful unique information now either (possibly
the "Background" section in Development/Workflow, though that could go
elsewhere), so the pages could be deleted. Someone with greater permission
than me has to do that.

John
___
Sent via:Wireshark-dev mailing list 
Archives:https://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-dev
 mailto:wireshark-dev-requ...@wireshark.org?subject=unsubscribe

Re: [Wireshark-dev] FT_STRING, FT_STRINGZPAD, and null padding

2020-09-05 Thread John Thacker
So for the problem with calling format_text() on a string that has had
replacement characters applied, I see that packet-ieee80211.c calls
format_text() on a string that has already been obtained with
tvb_get_string_enc() in order to get something printable.
This is pretty common, a lot of dissectors do this, because format_text()
expects UTF-8, tvb_format_text() doesn't take an encoding at all and just
assumes UTF-8 (and works on ASCII), and it makes sense to try to call
something that does charset conversion to UTF-8 first for any field that
might have an encoding other than ASCII or UTF-8.

However, tvb_get_string_enc() for ASCII strings replaces characters with
the high bit set with the Unicode Replacement character, which can change
the length of the string. It's not clear what length to pass into
format_text() - the original length can be wrong, but strlen() doesn't work
for the cases where there's a NUL in the middle of the string, as it won't
get the trailing characters. A similar problem would occur if
tvb_get_string_enc() actually did replacement characters for UTF-8 invalid
characters (it doesn't do that now, see the comments of
https://gitlab.com/wireshark/wireshark/-/issues/14948 ), and does occur
with all the other encodings, where the replacement character substitution
already does occur.

So I don't particularly see any easy way to handle the counted strings that
might have replacement characters substituted in but also might have a NUL
in the middle and we still want to display any trailing characters in the
field after the NUL and warn about them.

For the dissectors that only have either ASCII or UTF-8 as the encoding, a
workaround is to call tvb_format_text() instead of calling format_text() on
the output of tvb_get_string_enc(). For dissectors with any other encoding,
that's not an option and as a result it doesn't feel great as a solution.

To handle other encodings, I can only see:
1) Changing tvb_get_string_enc - and the encoding specific helper functions
it calls - to pass back the true length as a output parameter
2) Changing tvb_get_string_enc to pass back a wmem_strbuf_t type that
handles embedded nulls and has length, use that type more widely, pass it
into the format function
3) Having some kind of tvb_format_text_enc() function (and variations for
whitespace) that takes different encodings, and differs from
tvb_get_string_enc in that it produces a printable output.

3) alone seems like a bad solution, because it would probably be called in
addition to tvb_get_string_enc for the same string in the same dissector,
and that's hitting some very similar tvbuff access and conversion code
twice. It also would mean some more tricky and error prone conversions. It
could be added as syntactic sugar for the function composition of
tvb_get_string_enc() and format_text() if 1) or 2) was implemented.

John Thacker
___
Sent via:Wireshark-dev mailing list 
Archives:https://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-dev
 mailto:wireshark-dev-requ...@wireshark.org?subject=unsubscribe

Re: [Wireshark-dev] FT_STRING, FT_STRINGZPAD, and null padding

2020-09-05 Thread John Thacker
any time the UTF-8 replacement character is present
as a hint that maybe the encoding is wrong.

We can see that be doing the same thing and *not* applying the patch to
allow SSIDs to always be UTF-8. Treating it as ASCII, we see:

Tag: SSID parameter set: admini\000traci�
Tag Number: SSID parameter set (0)
Tag length: 15
SSID: admini
[Expert Info (Warning/Undecoded): Trailing stray characters]

(admini\000traci� also showed up in the FT_COLUMN). What's going on is that
"\xc3\xb3" ("ó" in UTF-8) is not recognized as ASCII and is being replaced
with "\xef\xbf\xbd\xef\xbf\bd", two copies of the 3 byte UTF-8
representation of UNREPR, so then some of the other functions that read the
string stop after the first three replacement character bytes (reaching the
length of 15 octets) and never get to the second replacement character or
to the 'n."

Conversely, if I put non ASCII characters earlier in the string, I can get
this nonsense:

Tag: SSID parameter set: ai\000
Tag Number: SSID parameter set (0)
Tag length: 15
SSID: ai

Where the parts that call tvb_format_string() stop after 15 bytes are
written, which is right at the first null but before the trailing
characters that are in the 15 octet field, the part where
proto_tree_add_item() stops after before the first NUL, and the expert info
for detect_trailing_stray_characters() doesn't show up for the same reason
that tvb_format_string() stops in time-- it reaches 15 octets in the
substituted/validated string:

That's for a field with hexdump:

 61 c3 b3 c3 b3 69 00 74 72 61 63 69 c3 b3 6e ai.t raci..n

which with the UTF-8 patch looks like a different sort of nonsense:

Tag: SSID parameter set: ai\000
Tag Number: SSID parameter set (0)
Tag length: 15
SSID: aóói
    [Expert Info (Warning/Undecoded): Trailing stray characters]

for something that looks like  aóói\0tración when interpreted as UTF-8

I do plan to file a bug or two, just wanted to check on the expected
behavior.

John Thacker
___
Sent via:Wireshark-dev mailing list 
Archives:https://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-dev
 mailto:wireshark-dev-requ...@wireshark.org?subject=unsubscribe

[Wireshark-dev] FT_STRING, FT_STRINGZPAD, and null padding

2020-09-04 Thread John Thacker
I don't understand this change, which makes me wonder if I understand
FT_STRINGZPAD vs FT_STRING in general:
https://gitlab.com/wireshark/wireshark/-/commit/a42286524a0e23f5944e988e672a07a5fb395c69


Everything in proto.c and tvbuff.c treats FT_STRING and FT_STRINGZPAD
exactly the same currently, except for checking stray characters. If the
length is given as -1, the length to the end of the tvbuff is used.
Otherwise, the specified length is checked and copied from the tvbuff, and
a NUL is added at the end regardless. The last byte ensures that they both
work even if the buffer is not null terminated.

However, if the string is null terminated somewhere in the middle, all the
bytes after that (presumably padding) are still copied into the return
value, along with the extra '\0'. This doesn't really cause any problems
other than wasting a little bit of memory and time, since the first null
ends the string.

Before that commit, there was absolutely no difference that I could tell.
Now the difference is that if the string is terminated by a NUL somewhere
in the middle before the specified field length, whether or not the bytes
in the padding after that must be null padding. It is FT_STRING that
requires that any padding be zero-padding (and adds an expert warning if
not), whereas FT_STRINGZPAD does not.

While I agree with the commit message that "there are protocols where a
string that's not the full length of the part of the packet for the string
has a null terminator but isn't guaranteed to be fully padded with nulls,"
and that there can be "a separate type for fields where we really *should*
check that the padding is all nulls," I don't see why FT_STRINGZPAD isn't
that latter type. To me, the name FT_STRINGZPAD implies that it is the one
where we really should check that the padding is zero padding.

I would reverse the handling in that commit, and have FT_STRINGZPAD be the
type that does check, and make FT_STRING be the type that does not check
for trailing characters.

The documentation in README.dissector does not really explain the
differences between the use cases, as it seems like FT_STRING can be used
for everything where FT_STRINGZPAD can:

FT_STRING   A string of characters, not necessarily
NULL-terminated, but possibly NULL-padded.This, and
the other string-of-characterstypes, are to be used
for text strings,not raw binary data.
...FT_STRINGZPAD   A NULL-padded string of characters.
   The length is given in the proto_tree_add_item()
call, but may be larger than the length ofthe string,
with extra bytes being NULL padding.This is typically
used for fixed-length fieldsthat contain a string
value that might be shorterthan the fixed length.

John Thacker
___
Sent via:Wireshark-dev mailing list 
Archives:https://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-dev
 mailto:wireshark-dev-requ...@wireshark.org?subject=unsubscribe

Re: [Wireshark-dev] Cherry-picking, gitlab permissions

2020-08-26 Thread John Thacker
On Wed, Aug 26, 2020 at 9:13 AM Graham Bloice 
wrote:

> The "Options" droplist Pascal mentions can be seen on this page:
> https://gitlab.com/wireshark/wireshark/-/commit/847d3949c977a39c3cea15a214af02671ccd21e9?merge_request_iid=26
>
> Let us know if you can't see it John.
>

OK, thanks, I see it there. I looked for that page by going to Merge
Requests, then clicking on Commits within that, and it leads to a similar
looking page but one without the Options drop list:
https://gitlab.com/wireshark/wireshark/-/merge_requests/26/diffs?commit_id=847d3949c977a39c3cea15a214af02671ccd21e9
does not have the Options drop list, whereas the page from the
Repository->Commits left sidebar that you link above does. A little
confusing, but I'll manage.

Thanks,
John
___
Sent via:Wireshark-dev mailing list 
Archives:https://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-dev
 mailto:wireshark-dev-requ...@wireshark.org?subject=unsubscribe

[Wireshark-dev] Cherry-picking, gitlab permissions

2020-08-26 Thread John Thacker
The official Gitlab instructions (along with the still being transitioned
developer's guide) mention the ability to cherry pick merges from the GUI:
https://docs.gitlab.com/ee/user/project/merge_requests/cherry_pick_changes.html#cherry-picking-a-merge-request
https://www.wireshark.org/docs/wsdg_html_chunked/ChSrcContribute.html

Those buttons don't appear for me. I do see this bug about the cherry-pick
button not appearing for projects set to fast-forward merge only, like
wireshark:
https://gitlab.com/gitlab-org/gitlab/-/issues/5990
but the workaround listed in that issue doesn't work for me either.

I suspect that it may also have to do with permission levels. Is the
current workaround to do it from the command line? Are there plans to have
people who are not core developers request get and various intermediate
levels of permission? The "Reporter" level of permission seems to include
only things that previously people could get by registering for Bugzilla,
and some other features that people used to be able to do seem at the
"Developer" level (though that also includes some higher level permissions
too.)

John Thacker
___
Sent via:Wireshark-dev mailing list 
Archives:https://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-dev
 mailto:wireshark-dev-requ...@wireshark.org?subject=unsubscribe

Re: [Wireshark-dev] tshark --export-objects : -2 assumed or required for two-pass ?

2020-08-10 Thread John Thacker
On Mon, Aug 10, 2020 at 5:32 PM chuck c  wrote:

> tshark --export-objects dicom is behaving differently than exporting
> Dicom objects in Wireshark.
>
> Is the "-2" option assumed to be set, observed if set or not used at all
> for exporting objects with tshark?
>

Having implemented Export Objects on a different custom TFTP-like protocol,
I experienced the same thing.

With tshark, -2 is observed if set, and that can result in different
behavior. Generally more accurate information is obtained with two passes,
which is equivalent to Wireshark behavior.

There are certain protocols where single pass analysis just isn't
sufficient to determine all the data, and dissectors where some state
object is set, like packet-dcm.c, are a common case.

John Thacker
___
Sent via:Wireshark-dev mailing list 
Archives:https://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-dev
 mailto:wireshark-dev-requ...@wireshark.org?subject=unsubscribe

Re: [Wireshark-dev] Some apparent type bugs

2020-07-31 Thread John Thacker
On Fri, Jul 31, 2020 at 8:13 AM Jaap Keuter  wrote:

> Hi,
>
> Don’t know, just noticed the UINT part and thought about returning 'a
> value' should be possible.
> Will have to look into this more closely to see if and what makes sense.
>
>
I created change 38006  for
these two. Both of them wanted the length in order to increment the offset.
They were
written assuming that it would return just the value in the length field,
not the total length (including the fixed width
of the length field itself).

John
___
Sent via:Wireshark-dev mailing list 
Archives:https://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-dev
 mailto:wireshark-dev-requ...@wireshark.org?subject=unsubscribe

Re: [Wireshark-dev] Some apparent type bugs

2020-07-31 Thread John Thacker
On Fri, Jul 31, 2020 at 7:51 AM Jaap Keuter  wrote:

> Hi,
>
> Shouldn’t these be supported? After all, they have a UINT.
>
> On 31 Jul 2020, at 11:28, Martin Mathieson via Wireshark-dev <
> wireshark-dev@wireshark.org> wrote:
>
> Error: proto_tree_add_item_ret_uint (.., hf_lcp_opt_id , ...) called at
> ./tools/../epan/dissectors/packet-ppp.c 2377 with type FT_UINT_BYTES
> (allowed types are {'FT_CHAR', 'FT_UINT32', 'FT_UINT24', 'FT_UINT16',
> 'FT_UINT8'} )
>
> Error: proto_tree_add_item_ret_uint (.., hf_slsk_directory_name , ...)
> called at ./tools/../epan/dissectors/packet-slsk.c 1051 with type
> FT_UINT_STRING
> (allowed types are {'FT_CHAR', 'FT_UINT32', 'FT_UINT24', 'FT_UINT16',
> 'FT_UINT8'} )
>
>
There's a UINT passed in that's the fixed length of the length field,
there's the total length (which is the value in the length field of
the variable bytes plus the passed in fixed UINT) returned by
proto_tree_add_item_ret_length(), and then there's the value of the
variable length field only.

Is the proposed change to have proto_tree_add_item_ret_uint() return the
value contained in the variable length field
instead of the total length? Is that ever necessary? Shouldn't in most
cases just _ret_length() be used instead?

John Thacker
___
Sent via:Wireshark-dev mailing list 
Archives:https://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-dev
 mailto:wireshark-dev-requ...@wireshark.org?subject=unsubscribe

Re: [Wireshark-dev] Why tvb_get_bits() assumes Big Endian?

2020-07-30 Thread John Thacker
On Thu, Jul 30, 2020 at 11:56 AM Tomasz Moń  wrote:

> On Thu, Jul 30, 2020 at 3:35 PM Filipe Laíns  wrote:
> > For this reason I decided to drop it in favor of tvb_get_bits_array
> > + pletoh*. It is simple enough to be used standalone.
>
> This looks like a workaround to me, not a proper solution.
>
> To me the bit level order within a single byte seems clear - it's up
> to the tool that provides data to Wireshark to handle it. Wireshark
> job starts at the byte (octet) level.
>

Maybe here, but consider the FDDI dissector, along with H.223 and BMC, all
of which include wsutil/bitswap.h and call bitswap_buf_inplace()
>From what I recall of FDDI, that's because FDDI had the opposite bit order
of
Ethernet. Also it used MAC addresses and allowed bridging to Ethernet with
ARPs, so the MAC addresses in the ARP were in the opposite bit order as
the same MAC addresses in the FDDI header.

Certainly there is a special place reserved for people who come up with
this,
but if you really do need to reverse the bit level order for an array of
bytes, there
is already a function for that.

John Thacker
___
Sent via:Wireshark-dev mailing list 
Archives:https://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-dev
 mailto:wireshark-dev-requ...@wireshark.org?subject=unsubscribe

Re: [Wireshark-dev] Dissect data on a bit-by-bit basis

2020-07-21 Thread John Thacker
On Tue, Jul 21, 2020 at 9:05 PM Filipe Laíns  wrote:

> Hi,
>
> I am working on the USB HID dissector and I need to dissect data on a
> bit by bit basis, instead of byte. The data structure is completely
> dynamic (described by the HID descriptor) and the basic data block is
> the bit. Any bit or sequence of bits can have a meaning, the data can
> be completely unaligned. See the following example which shows
> different fields distributed in a 2 byte packet.
>
>
> What is the best way to dissect this data? I feel like I am going to
> have to basically write a complicated internal wrapper around the tvb
> API, and that won't even work properly in some cases.
>
> Is modifying the tvb API to allow data control on a bit level
> reasonable?


The API already allows fields which are bits, including unaligned ones, see
proto_tree_add_bits_item() described in proto.[c,h]
It even allows fields made of non-contiguous bits assembled into a single
field, see proto_tree_add_split_bits_*()

If you have the field types defined (say that you know the possible types
of data fields, just not their structure or how many
ahead of time), you can dynamically pass in the values for the bit_offset,
no_of_bits, and/or
the crumb_spec to those functions. If you need to change the formatting,
you can use the
proto_tree_add_[u]int[64]_bits_format_value() functions. (Though I don't
think that there is a *bits_format() function
that would let you change the name of the field for your Button A, Button B
types.) You can also do the
_add_bitmask* functions and dynamically change the pointer of fields passed
in.

John Thacker
___
Sent via:Wireshark-dev mailing list 
Archives:https://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-dev
 mailto:wireshark-dev-requ...@wireshark.org?subject=unsubscribe

Re: [Wireshark-dev] Version of Qt required?

2020-07-07 Thread John Thacker
On Tue, Jul 7, 2020 at 4:08 PM John Thacker  wrote:

> On Tue, Jul 7, 2020, 1:11 PM Pascal Quantin  wrote:
>
>> Hi John,
>>
>> Le mar. 7 juil. 2020 à 19:09, John Thacker  a
>> écrit :
>>
>>> On Tue, Jul 7, 2020 at 10:48 AM Graham Bloice <
>>> graham.blo...@trihedral.com> wrote:
>>>
>>>>
>>>> The table of Qt versions shows which versions have been used in
>>>> Wireshark installers for Windows and macOS. Determining the versions
>>>> available on specific Linux distributions is a bit more of an artform but
>>>> can be gleaned from the tables further down.
>>>>
>>>
>>> Ah, I see, yes I misread the table and interpreted the bold versions as
>>> minimum requirements. I just took a look and  updated the RHEL/CentOS and
>>> SLES information. Looking at the rest of the tables, Debian Jesse just went
>>> end of support, so Ubuntu xenial (16.04LTS, end of support April 2021)
>>> being at 5.5.x is I think the oldest QT in a still officially supported
>>> distribution on that page. After that there's several at 5.6.x.
>>>
>>> I reckon that QT 5.9 is fine for every distro's most recent LTS release,
>>> and 5.6 is fine for every distro's N-1 LTS release. Personally I'd be fine
>>> with moving master to 5.9 (which also settles requiring C+11), but I could
>>> understand 5.6.
>>>
>>
>> Can't this code be made conditional to the Qt version used for compiling?
>>
>
> It can be, but I think I would just as well rewrite it to use code from QT
> 5.3 and earlier. In this case I don't think there's from later than 5.3
> that's absolutely necessary for the functionality, more just syntactic
> sugar that can be done a different way.
>

Well, I guess foreach was officially deprecated in QT 5.7, so I suppose I
could check and do the loop with foreach for QT_VERSION < 5.7 and the
qAsConst way with 5.7 and higher.
___
Sent via:Wireshark-dev mailing list 
Archives:https://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-dev
 mailto:wireshark-dev-requ...@wireshark.org?subject=unsubscribe

Re: [Wireshark-dev] Version of Qt required?

2020-07-07 Thread John Thacker
On Tue, Jul 7, 2020, 1:11 PM Pascal Quantin  wrote:

> Hi John,
>
> Le mar. 7 juil. 2020 à 19:09, John Thacker  a
> écrit :
>
>> On Tue, Jul 7, 2020 at 10:48 AM Graham Bloice <
>> graham.blo...@trihedral.com> wrote:
>>
>>>
>>> The table of Qt versions shows which versions have been used in
>>> Wireshark installers for Windows and macOS. Determining the versions
>>> available on specific Linux distributions is a bit more of an artform but
>>> can be gleaned from the tables further down.
>>>
>>
>> Ah, I see, yes I misread the table and interpreted the bold versions as
>> minimum requirements. I just took a look and  updated the RHEL/CentOS and
>> SLES information. Looking at the rest of the tables, Debian Jesse just went
>> end of support, so Ubuntu xenial (16.04LTS, end of support April 2021)
>> being at 5.5.x is I think the oldest QT in a still officially supported
>> distribution on that page. After that there's several at 5.6.x.
>>
>> I reckon that QT 5.9 is fine for every distro's most recent LTS release,
>> and 5.6 is fine for every distro's N-1 LTS release. Personally I'd be fine
>> with moving master to 5.9 (which also settles requiring C+11), but I could
>> understand 5.6.
>>
>
> Can't this code be made conditional to the Qt version used for compiling?
>

It can be, but I think I would just as well rewrite it to use code from QT
5.3 and earlier. In this case I don't think there's from later than 5.3
that's absolutely necessary for the functionality, more just syntactic
sugar that can be done a different way.

John
___
Sent via:Wireshark-dev mailing list 
Archives:https://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-dev
 mailto:wireshark-dev-requ...@wireshark.org?subject=unsubscribe

Re: [Wireshark-dev] Version of Qt required?

2020-07-07 Thread John Thacker
On Tue, Jul 7, 2020 at 10:48 AM Graham Bloice 
wrote:

>
> The table of Qt versions shows which versions have been used in Wireshark
> installers for Windows and macOS. Determining the versions available on
> specific Linux distributions is a bit more of an artform but can be gleaned
> from the tables further down.
>

Ah, I see, yes I misread the table and interpreted the bold versions as
minimum requirements. I just took a look and  updated the RHEL/CentOS and
SLES information. Looking at the rest of the tables, Debian Jesse just went
end of support, so Ubuntu xenial (16.04LTS, end of support April 2021)
being at 5.5.x is I think the oldest QT in a still officially supported
distribution on that page. After that there's several at 5.6.x.

I reckon that QT 5.9 is fine for every distro's most recent LTS release,
and 5.6 is fine for every distro's N-1 LTS release. Personally I'd be fine
with moving master to 5.9 (which also settles requiring C+11), but I could
understand 5.6.

John Thacker
___
Sent via:Wireshark-dev mailing list 
Archives:https://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-dev
 mailto:wireshark-dev-requ...@wireshark.org?subject=unsubscribe

Re: [Wireshark-dev] Version of Qt required?

2020-07-07 Thread John Thacker
Ah, sorry, that is the result of my recent change. I had checked here

https://wiki.wireshark.org/Development/Support_library_version_tracking

which says that 5.9 and 5.12 had already been required by Wireshark 3.0 and
3.2 so I went ahead and used it.

Does it actually compile and work on 5.6 except for that change?

QT 5.6 is a long term support release and there are some distributions
stuck on it. I can change my recent commit if we don't want to bump the
required version.

John

On Tue, Jul 7, 2020, 9:15 AM Martin Mathieson via Wireshark-dev <
wireshark-dev@wireshark.org> wrote:

> In a VM where I build Wireshark, I am getting this error on master:
>
> /home/martin/wireshark/ui/qt/follow_stream_dialog.cpp: In member function
> ‘void FollowStreamDialog::addCodecs(const QMap&)’:
> /home/martin/wireshark/ui/qt/follow_stream_dialog.cpp:164:47: error:
> ‘qAsConst’ was not declared in this scope
>  for (const auto  : qAsConst(codecMap)) {
>^
> ui/qt/CMakeFiles/qtui.dir/build.make:1015: recipe for target
> 'ui/qt/CMakeFiles/qtui.dir/follow_stream_dialog.cpp.o' failed
> make[2]: *** [ui/qt/CMakeFiles/qtui.dir/follow_stream_dialog.cpp.o] Error 1
> make[2]: *** Waiting for unfinished jobs
>
> My installed version of Qt appears to be 5.6.2.
>
> Looking at CMakeLists.txt, there are various checks for Qt5Widgets, but
> the earliest supported version looks like 5.3?
>
> I think  https://doc.qt.io/qt-5/qtglobal.html#qAsConst  was introduced in
> 5.7.
> Do we want to bump the minimum versions?
>
> Martin
>
>
> ___
> Sent via:Wireshark-dev mailing list 
> Archives:https://www.wireshark.org/lists/wireshark-dev
> Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-dev
>  mailto:wireshark-dev-requ...@wireshark.org
> ?subject=unsubscribe
___
Sent via:Wireshark-dev mailing list 
Archives:https://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-dev
 mailto:wireshark-dev-requ...@wireshark.org?subject=unsubscribe

Re: [Wireshark-dev] Issues in synphasor dissector

2020-07-06 Thread John Thacker
Well, there are two other options:
1) Adding the ability to do BASE_CUSTOM for FT_FLOAT and FT_DOUBLE
2) Declaring as FT_[U]INT32 with BASE_CUSTOM, and do the type punning in
the custom function, as done in
tvb_get_nttohieee_float() in tvbuff.c for when FT_FLOAT is used anyway. For
bonus points, implement (or export
and make no longer static) get_ieee_float() in the same that exists for
VAXen and other non IEEE754 floating point
machines where simple type punning won't work.

John Thacker



On Mon, Jul 6, 2020 at 5:51 PM Jaap Keuter  wrote:

> Hi,
>
> So, you want to keep this function call:
>
> proto_tree_add_item(wgs84_tree, hf_conf_pmu_lat, tvb, offset, 4,
> ENC_BIG_ENDIAN);
>
> with this header field definition:
>
> { _conf_pmu_lat, { "PMU Latitude", "synphasor.conf.pmu_latitude",
> FT_FLOAT,
> BASE_NONE | BASE_UNIT_STRING, _degree_degrees, 0x0, NULL, HFILL
> }}
>
> but have it output "*Unspecified Location*" in case the value is +/-
> infinity.
>
> Unfortunately the use of BASE_CUSTOM is not available for FT_FLOAT, so I
> see no other way than this:
>
> if (isinf(pmu_lat)) {
> proto_tree_add_float_format_value(wgs84_tree, hf_conf_pmu_lat_unknown,
> tvb, offset, 4, INFINITY, unspecified_location);
> } else {
> proto_tree_add_item(wgs84_tree, hf_conf_pmu_lat, tvb, offset, 4,
> ENC_BIG_ENDIAN);
> }
>
> and these header field definitions:
>
> { _conf_pmu_lat_unknown, { "PMU Latitude",
> "synphasor.conf.pmu_latitude", FT_FLOAT,
> BASE_NONE, NULL, 0x0, NULL, HFILL }}
> { _conf_pmu_lat, { "PMU Latitude", "synphasor.conf.pmu_latitude",
> FT_FLOAT,
> BASE_NONE | BASE_UNIT_STRING, _degree_degrees, 0x0, NULL, HFILL
> }}
>
> Now at least the field abbrev "synphasor.conf.pmu_latitude" has a
> constant type FT_FLOAT.
>
> I hope I got this right.
> Jaap
>
>
> On 7/6/20 10:12 PM, Елисеев Дмитрий Алексеевич wrote:
>
> Hello, Jaap Keuter!
> I made some fast cheks and need some help.
> I need some function like:
>
>
> *proto_tree_add_item(wgs84_tree, hf_conf_pmu_lat, tvb, offset, 4,
> ENC_BIG_ENDIAN); { _conf_pmu_lat, { "PMU Latitude",
> "synphasor.conf.pmu_latitude", FT_FLOAT, *
> *  BASE_NONE | BASE_UNIT_STRING, _degree_degrees, 0x0, NULL,
> HFILL }}*
>
> witch replace Infinity with text: "*Unspecified Location*"
>
> С уважением,
> Дмитрий Елисеев
> *инженер*
> сектора испытаний систем управления и автоматики
> АО «НТЦ ЕЭС»
> т: 29-29-499 доб. 214
> a: ул. Курчатова, д. 1, лит.А , Санкт-Петербург, Россия, 194223
> e: elis...@ntcees.ru
>
>
> * От: * Jaap Keuter  
> * Кому: * Dmitriy Eliseev  ,
> Developer support list for Wireshark 
> 
> * Отправлено: * 06.07.2020 19:10
> * Тема: * Issues in synphasor dissector
>
> Hi Dmitriy,
>
> There are still some issues with the synphasor dissector. There are a
> number of fields with the same field abbreviation, but incompatible type.
> These are lines from the 'conflict check' run by the buildbot:
>
> 'synphasor.conf.chnam_len' exists multiple times with NOT compatible
> types: FT_STRING and FT_UINT8
> 'synphasor.conf.phasor_flags' exists multiple times with NOT compatible
> types: FT_UINT16 and FT_BOOLEAN
> 'synphasor.conf.pmu_latitude' exists multiple times with NOT compatible
> types: FT_STRING and FT_FLOAT
> 'synphasor.conf.pmu_longitude' exists multiple times with NOT compatible
> types: FT_STRING and FT_FLOAT
> 'synphasor.conf.pmu_elevation' exists multiple times with NOT compatible
> types: FT_STRING and FT_FLOAT
>
>
> https://buildbot.wireshark.org/petri-dish/builders/Ubuntu%20Petri%20Dish%20x64/builds/12010/steps/conflict%20check/logs/stdio
>
> You seem to be knowledgeable about this protocol and dissector. Would you
> be able to address these reported issues?
>
> Thanks,
> Jaap
>
>
> ___
> Sent via:Wireshark-dev mailing list 
> Archives:https://www.wireshark.org/lists/wireshark-dev
> Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-dev
>  mailto:wireshark-dev-requ...@wireshark.org
> ?subject=unsubscribe
___
Sent via:Wireshark-dev mailing list 
Archives:https://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-dev
 mailto:wireshark-dev-requ...@wireshark.org?subject=unsubscribe

Re: [Wireshark-dev] Release lifetime and version number changes?

2019-04-12 Thread John Thacker
On Thu, Apr 11, 2019 at 7:55 PM Gerald Combs  wrote:

> We currently have three active release branches: 3.0, 2.6, and 2.4. This
> is because we support each release branch for a set amount of time
> (typically 24 months after the initial .0 release) and our last three .0
> releases were less than 12 months apart. However, having many active
> branches can sometimes cause confusion[1] and far fewer people download the
> "Old Old Stable" release than the "Old Stable" or "Stable" releases. Would
> it make sense to have only two release branches active at any given time,
> e.g. by adjusting our release branch lifetimes to "24 months or whenever we
> have two newer active branches, whichever comes first"?
>

I think two active release branches makes sense, but I'm not sure that it
always makes sense to have them be the two newest stable releases. When
people decide what release to download (or when Linux distributions build a
package, since we want to consider not the direct download stats but also
what gets bundled with distributions) the primary consideration is the
necessary libraries, not the features. Some stable release branches add a
lot of Wireshark feature but don't add a lot of library requirements. For
example, I think all that 2.6 added over 2.4 was requiring a version of
CMake that all distributions still in long term support already had. Since
it's difficult to find a system that could install 2.4 that couldn't also
install 2.6, it's really not necessary to support 2.6 and 2.4
simultaneously (and that would explain the lack of 2.4 downloads.)

OTOH, 3.0 bumps a lot of library versions, so if 3.2 (or whatever it's
called) is relatively minor in requirements changes (but heavy enough in
features to justify a new branch), I could see it making sense to keep 3.2
and 2.6 around, and drop 3.0 support first.

Cheers,
John Thacker
___
Sent via:Wireshark-dev mailing list 
Archives:https://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-dev
 mailto:wireshark-dev-requ...@wireshark.org?subject=unsubscribe

Re: [Wireshark-dev] ZigBee APS re-assemble with re-used sequence number

2018-08-02 Thread John Thacker
I had to deal with this problem when I added support for protocols like PPP
MP with a single sequence number that increments each fragment instead of
separate sequence numbers and fragment numbers. (See Appendix A of RFC 4623
(PWE3 Fragmentation and Reassembly) for a list of protocols that use this
style, including PPP MP (RFC 1990), PWE3 MPLS (RFC 4385), L2TPv2 (RFC
2661), L2TPv3 (RFC 3931), ATM, and Frame Relay.)

The problem happened in some long captures I had with some dropped packets
and where the "short sequence number" variant was used for MLPPP, which is
12 bits (so a little bit bigger than the 8 bits with ZigBee, but not having
separate fragment sequence numbers effectively costs a couple bits.) See in
particular fragment_add_seq_single_aging(), and all the
fragment_add_seq_single_* functions in general. My approach, which may not
have been optimal, used a REASSEMBLE_FLAGS_AGING flag and a max_age
parameter to discard partially reassembled fragments. When wireshark gets a
new fragment and looks in the table of partially reassembled fragments to
add it to, if the partially reassembled fragment comes from a frame number
that is very old, that partially reassembled fragment is discarded. I ended
up using the previously unused frame field of the partially reassembled
fragment head for this (fh->frame), storing the frame number of most
recently added fragment in this as well.

Take a look at those functions, as it may provide some help for your
approach. It worked for me on those files that I had.

John Thacker

On Thu, Aug 2, 2018 at 2:04 PM Jaap Keuter  wrote:

> Hi,
>
> Not burdened by any ZigBee domain knowledge I would say that a seq#
> rollover would require a clearing of the non-reassembled fragments. But not
> all of them because we could still be in the process of reassembling the
> part of the stream with the not-yet rolled over seq#. A sliding window of
> non-reassembled fragments, of about half the seq# range, moved forward by
> the next received seq#, could be sufficient. All in all this would be an
> extension of the generic reassembly routenes, assuming they are used...
>
> Thanks,
> Jaap
>
> On 2 Aug 2018, at 12:17, Kenneth Soerensen  wrote:
>
> Hi
>
> I have filled bug
> https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=15021
>
> Any idea how we can fix this?
>
> The packet re-assembler is confused by ZigBee APS re-using sequence
> numbers, which makes it hard to distinguish what fragments belong to
> specific re-assembled packets.
>
> /Kenneth
>
>
> ___
> Sent via:Wireshark-dev mailing list 
> Archives:https://www.wireshark.org/lists/wireshark-dev
> Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-dev
>  mailto:wireshark-dev-requ...@wireshark.org
> ?subject=unsubscribe
___
Sent via:Wireshark-dev mailing list 
Archives:https://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-dev
 mailto:wireshark-dev-requ...@wireshark.org?subject=unsubscribe

Re: [Wireshark-dev] Dissector for link layer to run before ethernet one

2017-07-20 Thread John Thacker
On Thu, Jul 20, 2017 at 7:47 AM, Mihai Cîrîc via Wireshark-dev <
wireshark-dev@wireshark.org> wrote:

> Hello all,
>
> I have some capture files with packets encapsulated under ethernet. But
> these packets have a short header before the mac addresses and I am
> trying to write a dissector that would run before the ethernet one,
> parse the header and then call the ethernet dissector to continue parsing
> the rest of the packet.
>
> I was not able to find any example of this being done and I guess it would
> involve changing the entry in the wtap_encap table to replace the eth
> dissector.
>
> Any ideas on how this could be done?
>

The quickest way is to change the encapsulation of the files to one of the
DLT_USER types (say with editcap) and then follow the procedure outlined
here:

https://wiki.wireshark.org/HowToDissectAnything

For starters, you can simply skip over your initial header with
header_size, and then after you've written your dissector you can call it
directly with the header_proto option.

John Thacker
___
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-requ...@wireshark.org?subject=unsubscribe

Re: [Wireshark-dev] [Help_Wireshark] difference between fragmentation reassembly functions

2017-07-05 Thread John Thacker
On Wed, Jul 5, 2017 at 4:41 AM, Pascal Quantin <pascal.quan...@gmail.com>
wrote:

>
>
> 2017-07-05 8:32 GMT+02:00 hhw hhw <hhw.h...@gmail.com>:
>
>> my packets are like:
>>sequence idsequence number  message type
>>16   00
>> Begin
>>16   11
>> Continue
>>16   22
>> End (more_frag=FALSE)
>>
>> PDUs  with sequence number =0-2 should be reassembled here.
>> PDUs with sequence number =3 -10 : Dont Fragmented
>> --
>>
>>  5   11   0
>> Begin
>>  5   12   1
>> Continue
>>  5   13   2
>> End  (more_frag=FALSE)
>>
>> PDUs  with sequence number =11-13 should be reassembled here.
>> PDUs with sequence number =13 -19 : Dont Fragmented
>> ---
>>
>>  16  20   0
>> Begin
>>  16  21   2
>> End (more_frag=FALSE)
>>
>> PDUs  with sequence number =20-21 should be reassembled here.
>>
>>
>> Now. How can i reassemble fragmented PDUs? which reassembly function?
>> fragment_add_seq_check or fragment_add_seq or fragment_add_check ?
>>
>
> It's a shame your protocol does not have an explicit fragment number. I
> wonder how this is supposed to work if you have a UDP datagram loss... If
> this can be changed, better do it and it would solve your reassembly API
> usage headache at the same time.
>

There's a number of protocols that work in a similar fashion without
explicit fragment numbers. PPP MP (RFC 1990 / 2686) is one such. Instead of
getting "sequence_number,fragment_number, last" as in other protocols, PPP
MP provides a single sequence number that is effectively "seqnum +
fragnum", and it provides flags for both the first and last fragment of a
reassembly. For things that are not fragmented, both the First and Last
(Begin and End) flags are set.

That seems like a similar approach to this protocol. See Appendix A of RFC
4623 (PWE3 Fragmentation and Reassembly) for a list of protocols that use
this style, including PPP MP (RFC 1990), PWE3 MPLS (RFC 4385), L2TPv2 (RFC
2661), L2TPv3 (RFC 3931), ATM, and Frame Relay. It's not ideal because when
there's datagram loss or out of order delivery it's unclear what
reassembled packet fragments belong to. E.g., if you receive sequence
number 12 and 13 above before receiving 2-11, then you don't know if 12 and
13 are the twelfth and thirteenth fragments in a large packet beginning
with 0, or whether the packet beginning with 0 ends sooner. You have to do
some juggling.

Anyway, I implemented such an approach for PPP MP that works for captures I
have. Take a look at fragment_add_seq_single and
fragment_add_seq_single_aging. The latter adds aging that assumes that
fragments shouldn't be reassembled together with other fragments that
arrived much later. This is necessary for cases with some packet loss
combined with the sequence number wrapping around; e.g., if above your
packet 21 was lost so the reassembly starting with 20 was never completed,
but then the sequence number wrapped around and you got another packet much
later in the capture with sequence number 21 incorrect reassembly could
occur.

It's not perfect, because it doesn't properly handle when a packet is
fragmented across the boundary of where the sequence number wraps around.
That wasn't a big deal with PPP MP (24 or 12 bit sequence numbers) as it
only affected a small percentage of packets, but I do recall someone on the
list wanting to use it with a protocol with much shorter sequence numbers
where wraparound was frequent. Since it's not perfect and I didn't have
examples of the other protocols, I didn't change it so that the others in
that list use the new API function.

John Thacker
___
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-requ...@wireshark.org?subject=unsubscribe

Re: [Wireshark-dev] Using /var/tmp instead of /tmp

2017-06-29 Thread John Thacker
On Thu, Jun 29, 2017 at 1:21 PM, Guy Harris <g...@alum.mit.edu> wrote:

> On Jun 29, 2017, at 3:48 AM, John Thacker <johnthac...@gmail.com> wrote:
>
> > I believe the main reason is that Fedora defaults /tmp to tmpfs (
> https://fedoraproject.org/wiki/Features/tmp-on-tmpfs) which thus limits
> it to the size of RAM (half that, at default).
>
> Does Linux not have a *virtual* memory-based tmpfs, such as the one that
> Sun developed back in the late 1980's, allowing pages from temporary files
> either to be in memory or swap space?
>
> Ah, to be correct, it is possible to set it to more than the amount of
physical RAM, and it will use swap at that point (see
https://www.kernel.org/doc/Documentation/filesystems/tmpfs.txt as well as
someone actually testing it
https://unix.stackexchange.com/questions/237030/how-safe-is-it-to-increase-tmpfs-to-more-than-physical-memory)
but it's set to a defined maximum at mount time and has to be remounted to
change. I've never actually set it to larger than my amount of RAM. I
suspect that Fedora simply found it easier to leave the default and then
change large files to use /var/tmp

The Sun version was the inspiration; the old Linux ramfs that tmpfs
replaced was limited to the size of RAM, and thus not as flexible.

John
___
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-requ...@wireshark.org?subject=unsubscribe

Re: [Wireshark-dev] Using /var/tmp instead of /tmp

2017-06-29 Thread John Thacker
I believe the main reason is that Fedora defaults /tmp to tmpfs (
https://fedoraproject.org/wiki/Features/tmp-on-tmpfs) which thus limits it
to the size of RAM (half that, at default). Temporary captures might use up
that capacity (depending on the system), so hence the patch to use
/var/tmp. From the feature page:

"We might need to patch a couple of packages not to store big files and
files needing boot persistance in /tmp, but rather in /var/tmp."

John

On Thu, Jun 29, 2017 at 6:30 AM, Roland Knall  wrote:

> Not sure why, because the two temp directories are different.
>
> Judging by https://en.wikipedia.org/wiki/Filesystem_Hierarchy_Standard
>
> /tmp is usually purged during a reboot (at least you cannot expect to
> survive it)
> /var/tmp is not purged during a reboot
>
> The Qt::Dir() related stuff is actually a bug and should default to the
> wsutil function or at least the same g_get_tmp_dir, as Gtk and Qt might
> have different methods of getting the tmp directory and by using a Qt
> specific version this may lead to file polution.
>
> I think using /var/tmp was a decision by the package developer for the
> Fedora package. I honestly see no reasons behind that decision, except to
> wanting to access the tmp files and making sure they stay around. But this
> leads to an overly crowded /var/tmp after some time in my book, so I think
> it actually should change back to /tmp
>
> cheers
> Roland
>
> On Thu, Jun 29, 2017 at 11:25 AM, Martin Sehnoutka 
> wrote:
>
>> What are the pros and cons of using /var/tmp instead of /tmp for
>> Wireshark?
>>
>> In Fedora, there is this downstream patch, which makes Wireshark use
>> /var/tmp:
>> http://pkgs.fedoraproject.org/cgit/rpms/wireshark.git/tree/w
>> ireshark-0008-move-default-temporary-directory-to-var-tmp.patch
>> so I'm just wondering why is it Fedora specific.
>>
>> --
>> Martin Sehnoutka | Associate Software Engineer
>> PGP: 5FD64AF5
>> UTC+1 (CET)
>> RED HAT | TRIED. TESTED. TRUSTED.
>>
>>
>>
>> 
>> ___
>> Sent via:Wireshark-dev mailing list 
>> Archives:https://www.wireshark.org/lists/wireshark-dev
>> Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-dev
>>  mailto:wireshark-dev-requ...@wireshark.org?subject=unsubscr
>> ibe
>>
>
>
> 
> ___
> Sent via:Wireshark-dev mailing list 
> Archives:https://www.wireshark.org/lists/wireshark-dev
> Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-dev
>  mailto:wireshark-dev-requ...@wireshark.org?subject=
> unsubscribe
>
___
Sent via:Wireshark-dev mailing list 
Archives:https://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-dev
 mailto:wireshark-dev-requ...@wireshark.org?subject=unsubscribe

Re: [Wireshark-dev] Different reassembly needs

2017-01-19 Thread John Thacker
Hi Paul,

On Thu, Jan 19, 2017 at 12:20 PM, Paul Williamson <p...@mustbeart.com>
wrote:

>
> I want to add fragmentation reassembly to the dissector I'm improving for
> DVB-S2-BB. To get things exactly right, I'll need to reassemble in a way
> that doesn't seem to be supported by the existing code in
> epan/reassemble.[ch]. What approach is considered most preferable?
>
> 1. Modify epan/reassemble.[ch] to do what I need...
>
> If you're interested in the details, here are some. The protocol I'm
> dissecting is an encapsulation protocol running encapsulated in UDP.
> Fragments are tagged as first, last, or in between, and arrive in order
> without a sequence number, so the existing function fragment_add_seq_next()
> is close to what I need. I also need to check whether a first fragment has
> already been received before adding a last or in-between fragment, and I
> think I can do that with fragment_get().
>
> However, I also need to age out old fragments. There isn't currently a
> function like fragment_add_seq_next_aging() in the code, but perhaps it
> would be easy enough to add. Unfortunately, the existing aging mechanism in
> epan/reassemble.[ch] is based on pinfo->num, and to comply with the spec I
> need it to be based on a count of frames of my specific protocol.This is
> where I think I'd need to modify epan/reassemble.[ch].
>

Hi Paul,

When I recently wrote the add_seq_single* functions because of PPP
Multilink Protocol (though a number of other protocols use a similar method
that just isn't handled now), I asked a similar question before starting,
and got the recommendation on this list to modify epan/reassemble.[ch] to
add necessary functions. After all, it's quite possible that other
protocols will come along that will need the same functions.

Regarding aging, I wrote the _single_aging function because it was
necessary to make things work especially with the short sequence number
option causing overflow. It's not defined in the spec, so I picked
pinfo->num simply because that was the easiest thing to implement without
changing the existing structure. Looking specifically at the packet count
for the corresponding protocol only would almost surely be better, just a
little more work. For that reason, if you implemented it well I would be up
for changing the existing function, especially since there's only the one
dissector that uses it yet last I looked. That said, the dissector will
probably work in most practical cases without doing so, but not be quite
compliant with the spec.

John Thacker
___
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-requ...@wireshark.org?subject=unsubscribe

[Wireshark-dev] Wiki editing privileges

2016-12-19 Thread John Thacker
Hi,

I would like for my user account JohnThacker to become a memory of the
EditGroup for the Wireshark wiki.

Thanks,
John Thacker
___
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-requ...@wireshark.org?subject=unsubscribe

Re: [Wireshark-dev] fragment reassembly

2016-07-07 Thread John Thacker
On Fri, Jun 24, 2016 at 4:54 PM, Guy Harris <g...@alum.mit.edu> wrote:

> On Jun 24, 2016, at 1:14 PM, John Thacker <johnthac...@gmail.com> wrote:
>
> > I am implementing fragment reassembly of PPP Multilink (RFC 1990) and
> also implementing multiclass extension (RFC 2686). (
> https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=12548
> https://code.wireshark.org/review/#/c/16044/)
> > The protocol is unlike other protocols that do reassembly. Most
> protocols contain a sequence number that corresponds to the reassembled
> PDU, plus generally fragment numbers that indicate the position of the
> fragment within the reassembly and a flag that indicates when there are no
> more fragments expected. (There are some exceptions without fragment
> numbers; those have to assume that fragments are received in order and use
> fragment_add_seq_next().)
> >
> > PPP Multilink only has a single sequence number, plus flags that
> indicate whether something is the first or last (or both or neither)
> fragment of a PDU. There's no indicate of what the fragment number is
> within the sequence, other than "First" or "not first."
>
> I.e., it's more like TCP, where the sequence number is a sequence number
> within a "connection".
>
> I seem to remember that there are some other protocols that work that
> way.  There should probably be an additional set of reassembly APIs to
> handle that case.
>

Yes, I found a big list of protocols in Appendix A of RFC 4623 (PWE3
Fragmentation and Reassembly) that work this way, in addition to PPP MP.
PWE3 MPLS (RFC 4385), L2TPv2 (RFC 2661), L2TPv3 (RFC 3931), ATM, and Frame
Relay are all claimed to use this style of fragmentation. It looks like,
just as with PPP MP, Wireshark reads the flags and sequence number but
doesn't attempt reassembly.

I submitted a new changeset taking a an approach of adding reassembly APIs.
It's fairly large, so I've been testing for a while on various corner cases
(missing packets, strange order of arrival, etc.)
___
Sent via:Wireshark-dev mailing list <wireshark-dev@wireshark.org>
Archives:https://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev
 mailto:wireshark-dev-requ...@wireshark.org?subject=unsubscribe

[Wireshark-dev] fragment reassembly

2016-06-24 Thread John Thacker
I am implementing fragment reassembly of PPP Multilink (RFC 1990) and also
implementing multiclass extension (RFC 2686). (
https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=12548
https://code.wireshark.org/review/#/c/16044/)
The protocol is unlike other protocols that do reassembly. Most protocols
contain a sequence number that corresponds to the reassembled PDU, plus
generally fragment numbers that indicate the position of the fragment
within the reassembly and a flag that indicates when there are no more
fragments expected. (There are some exceptions without fragment numbers;
those have to assume that fragments are received in order and use
fragment_add_seq_next().)

PPP Multilink only has a single sequence number, plus flags that indicate
whether something is the first or last (or both or neither) fragment of a
PDU. There's no indicate of what the fragment number is within the
sequence, other than "First" or "not first." The ordinary fragment_add_seq*
functions would prefer two values, a sequence number, and a fragment
number; effectively we have the sum of these two. Thus an in order sequence
of packets might look like:

0, First, Last (0, 0)
1, First (1, 0)
2, Last, (1, 1)
3, First (3, 0)
4, (3, 1)
5, Last (3, 2)

The value in parentheses is how these would naturally be treated under the
standard (seqnum, fragnum) schema.

If we receive packet 4 or 5 before packet 2 or 3, then it is impossible to
tell if packet 4 or 5 are fragments 3 or 4 of a packet beginning at 1, or
fragments 1 and 2 of the packet beginning at packet 3. (If we haven't
received packet 1 as well, then there are even more possibilities.) The
approach I use is to tentatively add them to both, then clean things up as
necessary. (The approach could be improved somewhat; I already decrement
the sequence number and increment the fragment number and stop if we finish
a reassembly. I could also stop as soon as we hit a partially reassembly
for which a recemt First fragment (fragnum 0) has been seen.)

Cleaning them up involves two things: using fragment_delete either
immediately or later, when that sequence number has cycled around, in order
to remove entries in the in-progress reassembly table that are completely
wrong. Another necessary method to clean things up is removing extra
fragments added to the end. In the example above, if we receive packet 4
and 5 before 2 and 3, but then receive packet 2, then since packet 2 is
marked as the Last packet in a reassembly, we remove any tentative packets
after 2 (such as 4 and 5) from tentative in-progress reassemblies. That
involves some linked list manipulation and use of tvb_free and
g_slice_free. That's pretty hairy; at the same time, so is the use of
fragment_delete as, even though it's added to reassemble.h, no other
dissector currently uses it.

The sequence number itself can be short (12 bit) or long (24 bit), either
of which can loop around in a single capture. Because of this, if a
sequence number is encountered and there is a packet currently being
reassembled whose most recent packet is quite old (set by a configuration
value), the old reassembly is discarded with fragment_delete. This use of
fragment_delete is unneeded if all packets are present in the capture and
reassembly is successful (as that removes it from the in-progress
reassembly table), but that may not always be the case.

This all seems to work and I've tested it a lot, but I'm somewhat unsure
about it. It's not like any other fragmented protocol (though the aging
stuff could be helpful), so I had to use the defragmentation API quite
differently. Perhaps these things should be added into reassemble.h and
reassemble.c, since they're pretty low level and it's possible that other
protocols in the future would need them. The other API calls tend to assume
that things are simply in error if, as a result of calls, multiple tails
end up getting added, or a tail gets added that is before other data,
whereas in this case that's an expected outcome but one that needs to be
cleaned up.

A more thorough rewrite of how reassembly works could introduce new API
calls that are designed to work for sequence numbers as implemented here,
"front end" sequence numbers that increment with each fragment instead of
end result PDU sequence numbers. I haven't fully thought of how to do it.

Any thoughts or suggestions?

John Thacker
___
Sent via:Wireshark-dev mailing list <wireshark-dev@wireshark.org>
Archives:https://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev
 mailto:wireshark-dev-requ...@wireshark.org?subject=unsubscribe