This dissector was iniitally a plug-in and was converted to a built-in
on Change: https://code.wireshark.org/review/#/c/26428/
There is still a fair bit of clean up necessary. I'm attempting some of
that here. Detailed below are steps of each of the consecutive patch
sets as I work through it. They are in a top (oldest) down (newest)
order. If there is another way I should do this please let me know.
f5ethtrailer: Fix indiscriminate consumption of trailers
Dissector was not discriminating in consuming trailers. Rewrote
the heuristic to look for specific signatures and return if they
are not found. Setting heuristic back to enabled since it now only
consumes trailers that it should be able to dissect.
f5ehtrailer: remove unneeded () from around return values
f5ethtrailer: Remove macros
Macros were left over from using the same same code base to compile the
plugin against differed API versions (versions of Wireshark). Now that
the code is part of the core repository, it only needs to apply to the
branch it exists in.
f5ethtrailer: Correct FILEINFO mappings
f5ethtrailer: Eliminate compile time option to exclude POP_OTHER_FIELDS
This is now a runtime pref that is disabled by defualt.
Remove unnecessary NULL check
f5ethtrailer: Comment clean up
Clean up a few comments, make sure every function has a doc comment
f5ethtrailer: Trailer detection improved
Skip over leading zeros
Add prefrence to walk remaining data looking for trailer.
Disabled (default) Only look for trailers at first non-zero byte
Enabled start at first non-zero byte looking for trailers. Walk the
remaining data byte-by-byte looking for a matching trailer.
Change-Id: I6499ed6c6a760b668efe86632011cd07a7e447b2
Reviewed-on: https://code.wireshark.org/review/36012
Reviewed-by: Jason Cohen <kryojenik2@gmail.com>
Petri-Dish: Alexis La Goutte <alexis.lagoutte@gmail.com>
Tested-by: Petri Dish Buildbot
Reviewed-by: Anders Broman <a.broman58@gmail.com>
Trivial, mostly just redundant assignments or
format specifiers.
Change-Id: Iaf33f24d2af5a48a5e1b797e582bf936914c8daa
Reviewed-on: https://code.wireshark.org/review/36154
Petri-Dish: Martin Mathieson <martin.r.mathieson@googlemail.com>
Tested-by: Petri Dish Buildbot
Reviewed-by: Martin Mathieson <martin.r.mathieson@googlemail.com>
Update existing tests to the new smb2_seskey_list syntax and add new
tests for decrypting using different combinations of provided keys.
Change-Id: I86fda351ff736cae6029ec2321c45a02c1917226
Reviewed-on: https://code.wireshark.org/review/36137
Reviewed-by: Peter Wu <peter@lekensteyn.nl>
Petri-Dish: Peter Wu <peter@lekensteyn.nl>
Tested-by: Petri Dish Buildbot
Reviewed-by: Alexis La Goutte <alexis.lagoutte@gmail.com>
When dissecting a capture made in the middle of an existing encrypted
session we cannot decrypt the traffic because we don't know:
* what SMB dialect and encryption algorithm was picked during the
session establishment
* which host is the server and which host is the client
Since we know the decrypted payload always starts with a valid header
we use this as an heuristic and try all possible decryption settings.
Change-Id: I1daa297ced98e62cf361b9022871c668e56f8f4b
Reviewed-on: https://code.wireshark.org/review/36136
Reviewed-by: Peter Wu <peter@lekensteyn.nl>
Petri-Dish: Peter Wu <peter@lekensteyn.nl>
Tested-by: Petri Dish Buildbot
Reviewed-by: Alexis La Goutte <alexis.lagoutte@gmail.com>
Previously users could only give a session key via
uat:smb2_seskey_list:<id>,<seskey>
which was used to generate the decryption keys, as long as the trace
contained the session establishement.
Users have often asked about how to decrypt traffic captured in the
middle of an existing session but this wasn't possible.
This commit extends uat:smb2_seskey_list with 2 extra columns to store
decryption keys so that traffic can be decrypted at any point of the
session.
This has the side effect of changing the current syntax from:
... -o uat:smb2_seskey_list:<id>,<seskey>
To:
... -o 'uat:smb2_seskey_list:<id>,<seskey>,"",""'
(make sure the quoting is right)
Change-Id: I810d464b6f3e749de39b4428d73e0d6be29f3152
Reviewed-on: https://code.wireshark.org/review/36135
Reviewed-by: Peter Wu <peter@lekensteyn.nl>
Petri-Dish: Peter Wu <peter@lekensteyn.nl>
Tested-by: Petri Dish Buildbot
Reviewed-by: Alexis La Goutte <alexis.lagoutte@gmail.com>
when expanding QUERY_NETWORK_INTERFACE_INFO responses completely IPv4
addresses show up backwards. Fix this by setting the right
endianess (BE).
Change-Id: I94897290f4052bc1e2471bc26d72dce8012b3e3a
Reviewed-on: https://code.wireshark.org/review/36144
Petri-Dish: Alexis La Goutte <alexis.lagoutte@gmail.com>
Tested-by: Petri Dish Buildbot
Reviewed-by: Alexis La Goutte <alexis.lagoutte@gmail.com>
This codepoint was not registered in RFC 7858, but requested later by
Jon Reed at 2019-12-12, in "[dns-privacy] ALPN protocol ID for DoT":
Tne primary use case we have is supporting both DoT and DoH on port
443, when port 853 is blocked between clients and the servers (this
is by mutual agreement, as discussed in RFC 7858 § 3.1).
Change-Id: Ic993023eedf6f40565a208033703aa1575710c17
Reviewed-on: https://code.wireshark.org/review/36151
Petri-Dish: Peter Wu <peter@lekensteyn.nl>
Tested-by: Petri Dish Buildbot
Reviewed-by: Alexis La Goutte <alexis.lagoutte@gmail.com>
A UDP payload of 543 bytes can never be a valid Handshake Initiation
message for example. Reject such messages to avoid user confusion.
Bug: 16394
Change-Id: Ia40ae24f8ff8abaf2bead54cbf091db907b66373
Reviewed-on: https://code.wireshark.org/review/36149
Petri-Dish: Peter Wu <peter@lekensteyn.nl>
Tested-by: Petri Dish Buildbot
Reviewed-by: Alexis La Goutte <alexis.lagoutte@gmail.com>
It was assumed that the WireGuard dissector is always called on the
first pass before the second pass. That might not be the case when the
heuristics dissector sets a conversation dissector later in the stream.
Be prepared to handle this case. Do not simply abort, the previous
packets may be valid data messages.
Bug: 16394
Change-Id: Id5bf38c07f4d1bffd4b372e92d9a8784e094829a
Reviewed-on: https://code.wireshark.org/review/36148
Petri-Dish: Peter Wu <peter@lekensteyn.nl>
Tested-by: Petri Dish Buildbot
Reviewed-by: Alexis La Goutte <alexis.lagoutte@gmail.com>
As I said in
https://ask.wireshark.org/question/10361/how-to-properly-use-heuristic-dissector-for-tcp/?answer=10363#post-id-10363
"Should" has multiple meanings; there's "Used to indicate obligation,
duty, or correctness, typically when criticizing someone's actions.", as
in "I think we should trust our people more", and there's "Used to
indicate what is probable.", as in "the bus should arrive in a few
minutes". You're reading it in the latter sense; it was intended in the
former sense.
That sentence should probably be changed to "Wireshark must be then set
up..." to avoid the ambiguity.
Make it so (over half a year later, sigh), and change another case where
"should" is meant in the first sense while we're at it.
Change-Id: I90198d1616619c75802deeeb703ceee0c8bac1bf
Reviewed-on: https://code.wireshark.org/review/36155
Reviewed-by: Guy Harris <guy@alum.mit.edu>
Use heuristics to distinguish TP encoding of older drafts from draft 27.
Tested with a draft-24 and draft-27 capture.
Bug: 13881
Change-Id: I0426f2b3afeab974104f0363b25dcf6387101d1f
Reviewed-on: https://code.wireshark.org/review/36150
Petri-Dish: Peter Wu <peter@lekensteyn.nl>
Tested-by: Petri Dish Buildbot
Reviewed-by: Alexis La Goutte <alexis.lagoutte@gmail.com>
Add debug macros to help development. These macros are disabled by
default.
Change-Id: I9abdfdf58bbfb47f1d9145b6f4156708bee01512
Reviewed-on: https://code.wireshark.org/review/36134
Petri-Dish: Alexis La Goutte <alexis.lagoutte@gmail.com>
Tested-by: Petri Dish Buildbot
Reviewed-by: Peter Wu <peter@lekensteyn.nl>
This is required for WireGuard decryption.
Change-Id: I8d27ac198a8bac161c1675e87c3685c8d73c9246
Reviewed-on: https://code.wireshark.org/review/36129
Reviewed-by: Gerald Combs <gerald@wireshark.org>
Fix dead store (Dead assignement/Dead increment) Warning found by Clang
Change-Id: I4f07d18cef039d671ca3bbade8ca956be1341b56
Reviewed-on: https://code.wireshark.org/review/36082
Reviewed-by: Anders Broman <a.broman58@gmail.com>
Requires less lookups, and allows overriding the registration in plugins.
Change-Id: I8fe78bf69d992737d9363ac79ed865b1d6490cda
Reviewed-on: https://code.wireshark.org/review/36124
Petri-Dish: Anders Broman <a.broman58@gmail.com>
Tested-by: Petri Dish Buildbot
Reviewed-by: Anders Broman <a.broman58@gmail.com>
Some CSN.1 definitions may contain so-called unions that usually
combine two or more choices. The exact element to be chosen is
determined by the value encoded in one or more bits preceeding
it. Here is an example of an identity union:
{ 0 < Global TFI : < Global TFI IE > >
| 10 < TLLI / G-RNTI : bit (32) >
| 110 < TQI : bit (16) > }
So if a given bitstream starts with '0'B, the Global TFI IE follows.
Otherwise either TLLI / G-RNTI or TQI is to be chosen. But what if
neither of the choice items matches? For example, what if a given
bitstream starts with '111'B?
Most likely we should treat the bitstream as malformed, stop further
decoding and report an error. And that's how Pycrate's [1] CSN.1
decoder [2] behaves. Hovewer, as it turns out, Wireshark would
simply skip the whole choice element and start decoding the next
one from the same bit position.
Here is an example of a malformed packet:
GSM RLC/MAC: PACKET_POLLING_REQUEST (4) (Downlink)
01.. .... = Payload Type (DL): RLC/MAC block contains an RLC/MAC control block
that does not include the optional octets of
the RLC/MAC control header (1)
..00 .... = RRBP: Reserved Block: (N+13) mod 2715648 (0)
.... 1... = S/P: RRBP field is valid
.... .001 = USF: 1
PACKET_POLLING_REQUEST (4) (downlink)
0001 00.. = MESSAGE_TYPE (DL): PACKET_POLLING_REQUEST (4)
.... ..11 = PAGE_MODE: Same as before (3)
---! ID <--- This is wrong! '111'B is unknown
1... .... = CONTROL_ACK_TYPE: PACKET CONTROL ACKNOWLEDGEMENT
message format shall be an RLC/MAC control block
Padding Bits
.110 0000 0000 1000 0101 0000 1000 1000 = Padding: 1611157640
0100 0000 0001 0011 1010 1000 0000 0100 = Padding: 1075030020
1000 1011 0010 1011 0010 1011 0010 1011 = Padding: 2334862123
0010 1011 0010 1011 0010 1011 0010 1011 = Padding: 724249387
0010 1011 0010 1011 0010 1011 0010 1011 = Padding: 724249387
0010 1011 = Padding: 43
Let's fix this, so after this patch we get:
GSM RLC/MAC: PACKET_POLLING_REQUEST (4) (Downlink)
01.. .... = Payload Type (DL): RLC/MAC block contains an RLC/MAC control block
that does not include the optional octets of
the RLC/MAC control header (1)
..00 .... = RRBP: Reserved Block: (N+13) mod 2715648 (0)
.... 1... = S/P: RRBP field is valid
.... .001 = USF: 1
PACKET_POLLING_REQUEST (4) (downlink)
0001 00.. = MESSAGE_TYPE (DL): PACKET_POLLING_REQUEST (4)
.... ..11 = PAGE_MODE: Same as before (3)
ID
STREAM NOT SUPPORTED (PacketPollingID)
[Expert Info (Warning/Protocol): STREAM NOT SUPPORTED (PacketPollingID)]
[STREAM NOT SUPPORTED (PacketPollingID)]
[Severity level: Warning]
[Group: Protocol]
[1] https://github.com/P1sec/pycrate
[2] https://github.com/P1sec/pycrate/wiki/Using-the-pycrate-csn1-translator-and-runtime
Change-Id: I7096c294e0d04d6afb3414874d3404cbb637fdae
Reviewed-on: https://code.wireshark.org/review/36077
Reviewed-by: Pau Espin Pedrol <pespin@sysmocom.de>
Petri-Dish: Alexis La Goutte <alexis.lagoutte@gmail.com>
Tested-by: Petri Dish Buildbot
Reviewed-by: Anders Broman <a.broman58@gmail.com>
RFC6824bis-18 (MPTCP version 1) removes the IP version field and
replaces it with an Echo bit that provides a reliability
mechanism for the ADD_ADDR option. This change allows either
v0 or v1 ADD_ADDR options to be displayed correctly.
Change-Id: I375bcf6e54c07f88ca8877a2c4b4220cf4157a64
Reviewed-on: https://code.wireshark.org/review/36095
Petri-Dish: Anders Broman <a.broman58@gmail.com>
Tested-by: Petri Dish Buildbot
Reviewed-by: Anders Broman <a.broman58@gmail.com>
Wrong value used for the value string map.
Change-Id: I320f1d0bfc967beed84770efa75dee98c5f68e70
Reviewed-on: https://code.wireshark.org/review/36123
Petri-Dish: Anders Broman <a.broman58@gmail.com>
Tested-by: Petri Dish Buildbot
Reviewed-by: Anders Broman <a.broman58@gmail.com>
Gisle Vanem reported a build issue:
In file included from epan/dissectors/packet-acdr.c:25:
In file included from ./epan/dissectors/packet-acdr.h:13:
f:/Programfiler/Gtk-Plus/Gtk3-3.6.4/include/glib-2.0\glib/gtypes.h(28,2): error: "Only <glib.h> can be included directly."
glib.h is already included elsewhere, so it can be safely dropped.
Change-Id: I943ffb58099253048dba3d46b520b2338c99443a
Reviewed-on: https://code.wireshark.org/review/36121
Petri-Dish: Peter Wu <peter@lekensteyn.nl>
Tested-by: Petri Dish Buildbot
Reviewed-by: Anders Broman <a.broman58@gmail.com>
MainWindow::captureFileReadStarted() is called inside event handlers.
There isn't any actual processing after the captureFileReadStarted() is
called so in most cases the code will quickly return to the event loop.
In case of cf_read() callbacks, there is dedicated "slow processing"
detection implemented that eventually leads to processEvent() call in
update_progress_dlg().
Change-Id: Icfefa0ba7bf1bec43014e30756d0eec4078d389c
Reviewed-on: https://code.wireshark.org/review/36113
Petri-Dish: Tomasz Moń <desowin@gmail.com>
Tested-by: Petri Dish Buildbot
Reviewed-by: Peter Wu <peter@lekensteyn.nl>
Only the change to packet-imap.c really represents a bug.
Change-Id: Ie270f97f3d94c338ea3c84a712f8f4d43ffd36f4
Reviewed-on: https://code.wireshark.org/review/36115
Petri-Dish: Martin Mathieson <martin.r.mathieson@googlemail.com>
Tested-by: Petri Dish Buildbot
Reviewed-by: Anders Broman <a.broman58@gmail.com>
Following commit c397adda8a there was some missing change
* Some `cur_offset += name_len` instead of `cur_offset += used_bytes`
* Some missing format_text
I took a look at the code after observing a bug with RRSIG record.
The signature in RRSIG was dissecting with some strange offset.
You can easily generate some pcap with those commands
delv @1.1.1.1 A www.cloudflare.com
and/or
dig @1.1.1.1 +dnssec www.cloudflare.com
Change-Id: Ibd6a6248b7497b8409d7797dc320035c8c2d1ed8
Reviewed-on: https://code.wireshark.org/review/36080
Petri-Dish: Alexis La Goutte <alexis.lagoutte@gmail.com>
Tested-by: Petri Dish Buildbot
Reviewed-by: Dario Lombardo <lomato@gmail.com>
Reviewed-by: Anders Broman <a.broman58@gmail.com>
If mustexist property is absent or set to false, allow the user to
specify the filename.
Add Clear button next to file selection. Previously cancelling file
selection when mustexist was false would clear the entry. However,
if mustexist was true, there was no easy way to clear the entry.
Change-Id: I367756fb868b4040a7203f1eb8c92b6bfaf29901
Reviewed-on: https://code.wireshark.org/review/35643
Petri-Dish: Tomasz Moń <desowin@gmail.com>
Tested-by: Petri Dish Buildbot
Reviewed-by: Roland Knall <rknall@gmail.com>
Add software_update_info() to the software update module, which returns
the name of our update library if we have one. Use it to add automatic
update information to the compiled information in `wireshark --version`.
Add a "release" test suite, which contains a test for automatic updates.
Ping-Bug: 16381
Change-Id: I867a96bdcfde8be541eca2dc0e84b5000276e7dd
Reviewed-on: https://code.wireshark.org/review/36107
Reviewed-by: Gerald Combs <gerald@wireshark.org>
Petri-Dish: Gerald Combs <gerald@wireshark.org>
Tested-by: Petri Dish Buildbot
Reviewed-by: Anders Broman <a.broman58@gmail.com>
The processor brand string returned by CPUID is left-padded in some
cases. For example, adding
g_warning("==>%s<==\n", CPUBrandString);
to get_cpu_info() on a test machine here returns
** (tshark.exe:3808): WARNING **: ==> Intel(R) Core(TM) i5-3320M CPU @ 2.60GHz<==
Make sure it's stripped before we add it to our version information.
Change-Id: Idf9f9574477043a5e7fe4ff1ecb7890d6da90e0a
Reviewed-on: https://code.wireshark.org/review/36108
Reviewed-by: Gerald Combs <gerald@wireshark.org>
Petri-Dish: Gerald Combs <gerald@wireshark.org>
Tested-by: Petri Dish Buildbot
Reviewed-by: Anders Broman <a.broman58@gmail.com>
When verification fails, it is reported, but when verification
succeeded, it was not entirely sure whether this happened. Change it:
...
Retry Token: a1146aad02d817fec13d6cb95e48b0e3a4d8bd7eb1029588ac9dc55434381cea9c5cec6b…
Retry Integrity Tag: 0b299146c79957dff224ecec33d8b2fc [verified]
Change-Id: I7b99e74d091c28677be91cc6544a0e2cdc1d9ae1
Ping-Bug: 13881
Reviewed-on: https://code.wireshark.org/review/36111
Petri-Dish: Peter Wu <peter@lekensteyn.nl>
Tested-by: Petri Dish Buildbot
Reviewed-by: Anders Broman <a.broman58@gmail.com>
Allow unconsumed octets to be passed back to the caller so that possible
trailer dissctors can be ran.
Bug: 16387
Change-Id: I289b4b077d40581d5d0f884e30c2f882d58fccf1
Reviewed-on: https://code.wireshark.org/review/36097
Petri-Dish: Alexis La Goutte <alexis.lagoutte@gmail.com>
Tested-by: Petri Dish Buildbot
Reviewed-by: Jason Cohen <kryojenik2@gmail.com>
Reviewed-by: Anders Broman <a.broman58@gmail.com>
Allow unconsumed octets to be passed back to the caller so that possible
trailer dissctors can be ran.
Bug: 16388
Change-Id: I022fb2e714a687390259037ac2885751d24619f7
Reviewed-on: https://code.wireshark.org/review/36096
Petri-Dish: Alexis La Goutte <alexis.lagoutte@gmail.com>
Tested-by: Petri Dish Buildbot
Reviewed-by: Jason Cohen <kryojenik2@gmail.com>
Reviewed-by: Anders Broman <a.broman58@gmail.com>
My previous patch was broken and did not handle the new Anti-Clogging Token
container. It was broken because I did not realise that Table 9-42 specified
the order of elements in the SAE Fixed Field. Table 9-43 specifies when
elements will be in which type of SAE request. However, 9-42 specifies the
order.
This has been tested with captures from WFA and Jouni Malinen.
Change-Id: Icbaa53560036c421299c74867ec04d9a28ea8aa0
Reviewed-on: https://code.wireshark.org/review/36098
Petri-Dish: Richard Sharpe <realrichardsharpe@gmail.com>
Tested-by: Petri Dish Buildbot
Reviewed-by: Anders Broman <a.broman58@gmail.com>
All I/O Graph instances share the same configuration. The code was
accessing the UAT underlying number of items variable (num_io_graphs_)
directly but the actual rows were accessed indirectly via UAT interface.
This could lead to UAT missynchronization and in turn an out of range
index access in IOGraphDialog::createIOGraph().
Fix the issue by not using the num_io_graphs_ directly.
Bug: 16373
Change-Id: Ifbc0fddb619d23f31f32aa46c4ae613954a8b780
Reviewed-on: https://code.wireshark.org/review/36106
Petri-Dish: Tomasz Moń <desowin@gmail.com>
Tested-by: Petri Dish Buildbot
Reviewed-by: Anders Broman <a.broman58@gmail.com>
From RFC7170
Bug: 16379
Change-Id: I1698e87c78ce3cdc3e322cfb112fd99e8d23e3ec
Reviewed-on: https://code.wireshark.org/review/36056
Reviewed-by: Peter Wu <peter@lekensteyn.nl>
Petri-Dish: Peter Wu <peter@lekensteyn.nl>
Tested-by: Petri Dish Buildbot
Reviewed-by: Anders Broman <a.broman58@gmail.com>
ACDR is a protocol over UDP that is used by AudioCodes devices for
recording traffic to and from the device.
It adds a header to each packet that contains extra data about the packet.
For some packet types (like SIP), it also appends the IP and UDP/TCP
headers of the sent/received packet.
The dissector unwraps the ACDR header, and displays the packets with the
original type (and when available, with the original addresses).
Bug: 16275
Change-Id: I19ad90053a2ef73da80881dc5e94aa362de23ea3
Reviewed-on: https://code.wireshark.org/review/35417
Petri-Dish: Alexis La Goutte <alexis.lagoutte@gmail.com>
Tested-by: Petri Dish Buildbot
Reviewed-by: Peter Wu <peter@lekensteyn.nl>
Fix decryption of the Initial Packet for Facebook mvfst, based on IETF
QUIC draft -22.
Bug: 16378
Change-Id: I023738f792a68fe020d780e0caee7c6046fe5ca8
Reviewed-on: https://code.wireshark.org/review/36089
Petri-Dish: Peter Wu <peter@lekensteyn.nl>
Tested-by: Petri Dish Buildbot
Reviewed-by: Alexis La Goutte <alexis.lagoutte@gmail.com>
GREASE (Generate Random Extensions And Sustain Extensibility) is no
longer a draft. Changed references from 'draft' to RFC 8701.
https://tools.ietf.org/html/rfc8701
Change-Id: I9c56098d0c18f1bee1a45ca8ef609b07ea3c0487
Reviewed-on: https://code.wireshark.org/review/36087
Petri-Dish: Peter Wu <peter@lekensteyn.nl>
Tested-by: Petri Dish Buildbot
Reviewed-by: Peter Wu <peter@lekensteyn.nl>