Notes:
1. There are no functionality changes with this delivery
2. This change is to reduce manual copying between structs. This will make it easier to add upcoming feature changes, and fix some connection handling issues (future merge requests).
3. Combine enip_conn_val_t and cip_conn_info_t. Previously, there were 2 different structs to track information about an overall CIP Connection.
Notes:
1. There are no functionality changes with this delivery
2. cip_connID_info_t describes a one-way connection. Each CIP Connection includes 2 of these. Previously, each operation was duplicated for each direction.
3. This change is to reduce copypaste, simplify logic, and make it easier to add upcoming feature changes, and fix some connection handling issues (future merge requests)
Changes:
1. Extract Method: get_conversation_info_one_direction
2. dissect_net_param16/dissect_net_param32: Parse and set data into cip_connID_info_t
packet-enip.c:1102:19: warning: no previous prototype for ‘create_connection_id_list’ [-Wmissing-prototypes]
Change-Id: I6e0caa29f5617a5688cddaa4d8001a260228294e
Reviewed-on: https://code.wireshark.org/review/37740
Petri-Dish: Alexis La Goutte <alexis.lagoutte@gmail.com>
Tested-by: Petri Dish Buildbot
Reviewed-by: Anders Broman <a.broman58@gmail.com>
The static arrays are supposed to be arrays of const pointers to int,
not arrays of non-const pointers to const int.
Fixing that means some bugs (scribbling on what's *supposed* to be a
const array) will be caught (see packet-ieee80211-radiotap.c for
examples, the first of which inspired this change and the second of
which was discovered while testing compiles with this change), and
removes the need for some annoying casts.
Also make some of those arrays static while we're at it.
Update documentation and dissector-generator tools.
Change-Id: I789da5fc60aadc15797cefecfd9a9fbe9a130ccc
Reviewed-on: https://code.wireshark.org/review/37517
Petri-Dish: Guy Harris <gharris@sonic.net>
Tested-by: Petri Dish Buildbot
Reviewed-by: Anders Broman <a.broman58@gmail.com>
No real functional changes:
1. Remove some types that don't appear in the spec
2. Change some variable names and text descriptions to match spec
3. Pull out some logic to separate functions
4. Correct variable types to match the interface types
Change-Id: I82ae74fd709c2ec893d20c50eaa36f6c86e20634
Reviewed-on: https://code.wireshark.org/review/37162
Petri-Dish: Anders Broman <a.broman58@gmail.com>
Tested-by: Petri Dish Buildbot
Reviewed-by: Anders Broman <a.broman58@gmail.com>
For the same reason as in g89c9d909.
Change-Id: I5e344ebdf8ba05d169484aa32b409d84edc6124f
Reviewed-on: https://code.wireshark.org/review/34943
Petri-Dish: Stig Bjørlykke <stig@bjorlykke.org>
Tested-by: Petri Dish Buildbot
Reviewed-by: Anders Broman <a.broman58@gmail.com>
Change all wireshark.org URLs to use https.
Fix some broken links while we're at it.
Change-Id: I161bf8eeca43b8027605acea666032da86f5ea1c
Reviewed-on: https://code.wireshark.org/review/34089
Reviewed-by: Guy Harris <guy@alum.mit.edu>
Remove the "title" parameter from the register_decode_as_next_proto()
function. This parameter is no longer required since decode_as_t does
not have a title any more.
Change-Id: I300c755bd465453aa91703b53ce9adc954e38c00
Reviewed-on: https://code.wireshark.org/review/33579
Reviewed-by: Martin Kaiser <wireshark@kaiser.cx>
Petri-Dish: Martin Kaiser <wireshark@kaiser.cx>
Tested-by: Petri Dish Buildbot
Reviewed-by: Anders Broman <a.broman58@gmail.com>
Some offsets were not incremented after being read, but the next field
assumed it was incremented.
Change-Id: Ifb523bc37f454cfc76d077d34c3efa2663fb6b9e
Reviewed-on: https://code.wireshark.org/review/32873
Reviewed-by: Anders Broman <a.broman58@gmail.com>
Petri-Dish: Anders Broman <a.broman58@gmail.com>
Tested-by: Petri Dish Buildbot
Reviewed-by: Alexis La Goutte <alexis.lagoutte@gmail.com>
Some buffer size checking was off by 1.
Change-Id: Ib99da61f476b6f20abe40311fd2112a8693a7878
Reviewed-on: https://code.wireshark.org/review/31946
Petri-Dish: Anders Broman <a.broman58@gmail.com>
Tested-by: Petri Dish Buildbot
Reviewed-by: Anders Broman <a.broman58@gmail.com>
1. Show 3 digits of precision after the decimal place for API/RPI when
displaying in ms.
2. Remove displaying the value as hex microseconds.
Change-Id: I483739c13ff0e02bd773b5207b41a5eec6c23289
Reviewed-on: https://code.wireshark.org/review/31583
Petri-Dish: Anders Broman <a.broman58@gmail.com>
Tested-by: Petri Dish Buildbot
Reviewed-by: Anders Broman <a.broman58@gmail.com>
SNN = Safety Network Number
No functional/parsing changes.
Changes:
1. Fix some display fields and filter names that previously used the
incorrect ssn naming.
2. Changed all variable names in a similar way.
Change-Id: I7bdc52a5aef31a9c6007545d5a79c99bab6cd184
Reviewed-on: https://code.wireshark.org/review/31549
Petri-Dish: Anders Broman <a.broman58@gmail.com>
Tested-by: Petri Dish Buildbot
Reviewed-by: Anders Broman <a.broman58@gmail.com>
Make the time stamp precision a 4-bit bitfield, so, when combined with
the other bitfields, we have 32 bits. That means we put the flags at
the same structure level as the time stamp precision, so they can be
combined; that gets rid of an extra "flags." for references to the flags.
Put the two pointers next to each other, and after a multiple of 8 bytes
worth of other fields, so that there's no padding before or between them.
It's still not down to 64 bytes, which is the next lower power of 2, so
there's more work to do.
Change-Id: I6f3e9d9f6f48137bbee8f100c152d2c42adb8fbe
Reviewed-on: https://code.wireshark.org/review/31213
Petri-Dish: Guy Harris <guy@alum.mit.edu>
Tested-by: Petri Dish Buildbot
Reviewed-by: Guy Harris <guy@alum.mit.edu>
1. Added a check that the Common Packet Format Item Length < remaining
data for that layer.
2. Added a check that there should always be at least 4 bytes available
for each Common Packet Format item (Type ID + Length)
Change-Id: Ie6f2b7904d52d8699c06cfef6844cf0032293d97
Reviewed-on: https://code.wireshark.org/review/31002
Petri-Dish: Anders Broman <a.broman58@gmail.com>
Tested-by: Petri Dish Buildbot
Reviewed-by: Anders Broman <a.broman58@gmail.com>
Add cip.connection. This works just like tcp.stream, but for CIP
connections. This is added to CIP connected messages and the Forward
Open/Close messages.
Change-Id: Ib358c00dc0a4fd61065cb22b0e9b574ac43a44a4
Reviewed-on: https://code.wireshark.org/review/30984
Reviewed-by: Anders Broman <a.broman58@gmail.com>
1. For each connected data message, display generated connection
information including:
a. Connection Path from the initial connection
b. API values
c. Forward Open packet number. (This already existed, but moving it to a
consistent place in the tree)
2. Display O->T or T->O in the Info column depending on the direction of data.
3. Remove cip.conn_path_class filter. This was originally added to show
which type of data is in a given packet. But, it's not really needed
anymore because we have the generated connection path in each connected
data packet now.
4. Ensure dummy structs used for Decode As menus are zeroed out.
5. memset -> zero initialization
pcaps from the following bug reports are good examples:
Bug: 14939
Bug: 6617
Bug: 14916
Bug: 14958
Change-Id: I63885a5ca41f95e04f855a1e1dcd9ab3684f7eec
Reviewed-on: https://code.wireshark.org/review/30808
Petri-Dish: Michael Mann <mmann78@netscape.net>
Tested-by: Petri Dish Buildbot
Reviewed-by: Anders Broman <a.broman58@gmail.com>
Rename packet-ssl{,-utils}.[ch] to packet-tls{,-utils}.[ch].
Change-Id: I4732162ec131ddf0734b3dd191ccc9e48a76ce06
Reviewed-on: https://code.wireshark.org/review/29659
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>
1. CIP Generic I/O: Add a "Decode As" option for CIP Class 1
2. Combine the 2 CIP conversation filters into one. There would never have
been more than 1 selected. This should be easier for users and less
clutter in the conversation menu.
3. CIP Safety: Name the data field as cipsafety.data instead of
enip.connection_transport_data, to make it like other CIP I/O data.
4. Minor: Rename some more variables/functions from enip to cip, to
reflect which actual protocol these are.
Change-Id: Id895f412e3584a5efcb7e69175a1b2bb3d5e9627
Reviewed-on: https://code.wireshark.org/review/28610
Petri-Dish: Anders Broman <a.broman58@gmail.com>
Tested-by: Petri Dish Buildbot
Reviewed-by: Anders Broman <a.broman58@gmail.com>
1. Add new dissector table that allows for registration of CIP Class 2/3 Data
against CIP Class that was used in the Forward Open. This is similar to the
Class 0/1 I/O dissector table. The new logic is this:
a. If there is connection info and a table entry: Call the registered dissector
handle (cip_connection_message_router.pcap).
b. If there is connection info and no table entry, use the CIP implicit
dissector (cip_connection_implicit.pcap)
c. If there is no connection info: Assume Message Router (Class 0x2)
format (cip_no_connection_message_router.pcap)
2. Remove old dissector table for "enip.sud.iface". The specification states that
the Interface Handle "shall be zero" for SendUnitData, so there isn't a need
to have custom handling for different Interface Handle values. SendRRData
does not have the same restriction, so that dissector table (enip.srrd.iface)
will stay.
3. Pull out Class 2/3 data processing into separate function: dissect_cip_class23_data()
4. Remove extra unnecessary tree layer in implicit data dissector.
Bug: 14916
Change-Id: Id53a2031a6064551b3915d8954527a7b3261d222
Reviewed-on: https://code.wireshark.org/review/28496
Petri-Dish: Graham Bloice <graham.bloice@trihedral.com>
Reviewed-by: Roland Knall <rknall@gmail.com>
Tested-by: Petri Dish Buildbot
Reviewed-by: Anders Broman <a.broman58@gmail.com>
If we know the information that belongs there, we should fill it in.
Otherwise, we should just pass a null pointer, meaning "we don't know
what this information is", and we should check for the null pointer and
not check the information in question.
Bug: 14894
Change-Id: I4f5249855330db65242d8b6eb6b5bda3af3a1925
Reviewed-on: https://code.wireshark.org/review/28404
Reviewed-by: Guy Harris <guy@alum.mit.edu>
1. Add new dissector table that allows for registration of CIP Class 0/1 I/O
against CIP Class that was used in the Forward Open. CIP Safety is still
a special case that gets checked before this table. The default handling is
generic CIP Class 0/1 I/O.
2. Changed most I/O items labelled "ENIP" to "CIP I/O". ENIP is a separate
protocol/layer, and all the I/O traffic is actually CIP. It was very
confusing explaining to people they had to look at the wrong protocol
layer in Wireshark before.
3. Add the generic Class 0/1 I/O as a separate tree layer. CIP Motion and
CIP Safety I/O were already doing this.
4. Update CIP conversation filtering naming to be more accurate.
5. Clean up some offset handling
Change-Id: I1c226fe1bd8974ed0e90640c875bef21f15f3095
Reviewed-on: https://code.wireshark.org/review/28290
Petri-Dish: Anders Broman <a.broman58@gmail.com>
Tested-by: Petri Dish Buildbot
Reviewed-by: Anders Broman <a.broman58@gmail.com>
dissect_cpf was huge and too hard to read and update.
This change pulls out item parsing into individual functions to make
it easier to read, help troubleshoot a bug related to ENIP TLS
connection filtering (Still investigating), and prep for future features.
There are no functional changes.
Main changes:
1. Pulled out the following code into separate functions:
dissect_item_list_identity
dissect_item_cip_security_information
dissect_item_list_services_response
dissect_item_sockaddr_info
dissect_item_sequenced_address
dissect_item_connected_address
dissect_item_unconnected_message_over_udp
dissect_generic_io
dissect_cip_class01_io
2. More documentation. It was a little hard to follow before.
3. Corrected offset inside the while loop in dissect_cpf(). Previously,
offset pointed to 2 bytes *before* the item actually being processed.
Change-Id: I47894fd5c50b4c3d07f916f81e1b21f8890c8396
Reviewed-on: https://code.wireshark.org/review/28205
Reviewed-by: Dylan Ulis <daulis0@gmail.com>
Reviewed-by: Anders Broman <a.broman58@gmail.com>
1. Connection Serial Number, Vendor ID, and Device Serial Number are always
used as a group. Group these as a struct to make them easier to manage.
2. Pull out common code into dissect_connection_triad()
No functional changes.
Change-Id: Ide126f8d0ea6ab8e2de5abf20e12643d5a35924d
Reviewed-on: https://code.wireshark.org/review/27926
Petri-Dish: Anders Broman <a.broman58@gmail.com>
Tested-by: Petri Dish Buildbot
Reviewed-by: Anders Broman <a.broman58@gmail.com>
Minor bug fixes:
1. Preshared Keys: Minimum size can be 1, not 3. This shows malformed packets for valid data.
2. Preshared Keys: Display PSK based on PSK size, not ID size.
3. Correct name of some hf types.
Change-Id: Ib412cd109929a1f4a1e5b67b47cb4c9f0eab6512
Reviewed-on: https://code.wireshark.org/review/27929
Petri-Dish: Anders Broman <a.broman58@gmail.com>
Tested-by: Petri Dish Buildbot
Reviewed-by: Anders Broman <a.broman58@gmail.com>
1. Convert more things to proto_tree_add_bitmask
2. Remove unnecessary pathpos param everywhere
3. Replace some tvb_get_xxx with proto_tree_add_item_ret_uint
4. Remove unused variables, params
5. Remove if(tree)
There are no functional changes. In the area where there is a lot of
diff (eg: pathpos), I verified using the pcap from Bug: 12049, and
compared the PDML output before and after, just as a sanity check.
Change-Id: I9564172ccece558cf1877b667f713b584a00d73f
Reviewed-on: https://code.wireshark.org/review/27642
Petri-Dish: Anders Broman <a.broman58@gmail.com>
Tested-by: Petri Dish Buildbot
Reviewed-by: Anders Broman <a.broman58@gmail.com>
Even though these are (currently) implemented in the enip dissector,
these conversations are actually for different types of CIP connections.
This changes makes it obvious to CIP users/developers what these are.
EtherNet/IP (enip) is mainly the encapsulation layer that allows CIP to
function on Ethernet.
Change-Id: I760f832026e35aec412d51d80e85a997b341e0b4
Reviewed-on: https://code.wireshark.org/review/27086
Petri-Dish: Anders Broman <a.broman58@gmail.com>
Tested-by: Petri Dish Buildbot
Reviewed-by: Anders Broman <a.broman58@gmail.com>
The length check in dissect_enip_tcp() was previously removed but
it's necessary to filter out one byte messages that are mostly likely
TCP keep alives.
Bug: 14434
Change-Id: I44c10aaf0a2e06870ad82f87aab9d72548b77f9f
Reviewed-on: https://code.wireshark.org/review/25807
Petri-Dish: Michael Mann <mmann78@netscape.net>
Tested-by: Petri Dish Buildbot
Reviewed-by: Michael Mann <mmann78@netscape.net>
When we copy an address from pinfo into connInfo->O2T.ipaddress, a
shallow copy is not sufficient. connInfo->O2T.ipaddress is kept across
packets whereas pinfo is valid only for the current packet.
Use wmem with file scope for the copied address. This fixes a
use-after-free error when we access the address in a subsequent packet.
Bug: 14470
Change-Id: I8b74037020189485485a506af6510cb45828e3c4
Reviewed-on: https://code.wireshark.org/review/26248
Reviewed-by: Martin Kaiser <wireshark@kaiser.cx>
Petri-Dish: Martin Kaiser <wireshark@kaiser.cx>
Tested-by: Petri Dish Buildbot
Reviewed-by: Anders Broman <a.broman58@gmail.com>
build_label_func is a pointer type, pointing to a function that can be
used to generate a label for a "Decode As" entry. There's no need to
have pointers to those pointers; have register_decode_as_next_proto()
take a build_label_func, not a build_label_func *, as its last argument,
assign that value rather than dereferencing it, get rid of unnecessary
casts, and fix the comment in epan/decode_as.h for that argument.
Change-Id: Idc3cf18204c543a2191b122c8129b362562c0ecd
Reviewed-on: https://code.wireshark.org/review/25773
Reviewed-by: Guy Harris <guy@alum.mit.edu>
1. Add Decode Options for 4 Safety I/O types. Previously, you could only decode as "CIP Safety", which only showed as a generic data block (because all important things for parsing are in the FwdOpen).
2. Change some timestamp display formatting to match other related timestamp fields (now all Dec)
3. Don't create connections for Null Forward Opens.
Change-Id: Ia1031b3887739a864a453b9e566ebe6f29fa5b8b
Reviewed-on: https://code.wireshark.org/review/25664
Petri-Dish: Michael Mann <mmann78@netscape.net>
Tested-by: Petri Dish Buildbot
Reviewed-by: Michael Mann <mmann78@netscape.net>
See Volume 9, version 1.2, sections "6-2.7.1.1" and "7-1.1"
1. Pass Connection Point from FwdOpen to Motion dissector, since that is now needed to parse I/O payload.
2. Move Run/Idle Header function to CIP dissector, since it's a CIP feature, not ENIP.
3. Add a protocol so that Format Revision 3 can be dissected without the Forward Open in the capture.
4. Minor: Highlight more bytes in some EPATH parsing.
5. Minor: Renaming some things to match spec wording.
Change-Id: I93626a6492be2675206d38c04fa1c7ce534c04ca
Reviewed-on: https://code.wireshark.org/review/25570
Petri-Dish: Michael Mann <mmann78@netscape.net>
Tested-by: Petri Dish Buildbot
Reviewed-by: Michael Mann <mmann78@netscape.net>
For the moment this mirrors the port_type enumeration (PT_XXX), but the
intent is to move away from using "port types", eliminating most (if not
all)
Added conversation_pt_to_endpoint_type() so that conversations deal with the
correct enumeration. This is for dissector that use pinfo->ptype as input
to conversation APIs. Explicit use of port types are converted to using
ENDPOINT_XXX type.
Change-Id: Ia0bf553a3943b702c921f185407e03ce93ebf0ef
Reviewed-on: https://code.wireshark.org/review/24166
Petri-Dish: Michael Mann <mmann78@netscape.net>
Tested-by: Petri Dish Buildbot
Reviewed-by: Michael Mann <mmann78@netscape.net>
This makes it easier to identify the simpler/common conversations
Change-Id: I7094f23e49156ee27f5f72c8e130308470f3e462
Reviewed-on: https://code.wireshark.org/review/24145
Petri-Dish: Michael Mann <mmann78@netscape.net>
Tested-by: Petri Dish Buildbot
Reviewed-by: Michael Mann <mmann78@netscape.net>
That allows a parallel typedef of ws_in4_addr for guint32.
Change-Id: I03b230247065e0e3840eb87635315a8e523ef562
Reviewed-on: https://code.wireshark.org/review/24073
Reviewed-by: Guy Harris <guy@alum.mit.edu>
For protocols that have don't have a unique identifier for their
payload, have register_decode_as_next_proto be a one stop shop to
create Decode As functionality and a dissector table of type FT_NONE.
Change-Id: Ic1f2e9ed0aee0554a4eb8f232630b99c0604dfc0
Reviewed-on: https://code.wireshark.org/review/22575
Petri-Dish: Michael Mann <mmann78@netscape.net>
Tested-by: Petri Dish Buildbot <buildbot-no-reply@wireshark.org>
Reviewed-by: Michael Mann <mmann78@netscape.net>
Add tfs_open_closed to general tfs collection (tfs.[ch])
Change-Id: I79b22b591128c33084489880842e19e9a0d80560
Reviewed-on: https://code.wireshark.org/review/22730
Petri-Dish: Michael Mann <mmann78@netscape.net>
Tested-by: Petri Dish Buildbot <buildbot-no-reply@wireshark.org>
Reviewed-by: Anders Broman <a.broman58@gmail.com>
Many dissectors don't have an identifier to pass to a dissector table.
When using Decode As they all have a "value" function that returns 0
just so something is returned.
A first step to a cleaner refactor of the functionality is to allow
dissectors to provide a "prompt" function when registering Decode As
with register_decode_as_next_proto() so that the text exposed in
the GUI can vary, but the function that returns 0 (nothing) can be
consolidated under decode as registration functionality. This casts
a wider net for register_decode_as_next_proto() use.
Change-Id: I2995b3c251dae70f5f529b672473d25c6288ed5c
Reviewed-on: https://code.wireshark.org/review/22562
Petri-Dish: Michael Mann <mmann78@netscape.net>
Tested-by: Petri Dish Buildbot <buildbot-no-reply@wireshark.org>
Reviewed-by: Anders Broman <a.broman58@gmail.com>
If a TCP segment is small enough, dissectors that have a only a length
check determining if it's their packet or not before calling tcp_dissect_pdus
will throw out packets that are probably destined for them.
Change-Id: I78034307b56aa537943191a6887166577936a6a3
Reviewed-on: https://code.wireshark.org/review/21950
Petri-Dish: Michael Mann <mmann78@netscape.net>
Tested-by: Petri Dish Buildbot <buildbot-no-reply@wireshark.org>
Reviewed-by: Michael Mann <mmann78@netscape.net>
This was inspired by using the Decode-As UI to decode Field "SSL TCP Dissector"
Value (port) XXX as YYY. "SSL Port" makes more sense as the UI name.
Change-Id: Id6398a5dc79e32bddc4f1bfcf0a468ae1364808f
Reviewed-on: https://code.wireshark.org/review/19573
Petri-Dish: Michael Mann <mmann78@netscape.net>
Tested-by: Petri Dish Buildbot <buildbot-no-reply@wireshark.org>
Reviewed-by: Michael Mann <mmann78@netscape.net>
Change-Id: I44f7ff6980f27b1a0d4199a91f9b217aec7e4652
Reviewed-on: https://code.wireshark.org/review/20557
Petri-Dish: Michael Mann <mmann78@netscape.net>
Tested-by: Petri Dish Buildbot <buildbot-no-reply@wireshark.org>
Reviewed-by: Anders Broman <a.broman58@gmail.com>
Many of the register_init_routine/register_cleanup_routine functions
are for initializing and cleaning up a GHashtable.
wmem_map_new_autoreset can do that automatically, so convert many
of the simple cases.
Change-Id: I93e1f435845fd5a5e5286487e9f0092fae052f3e
Reviewed-on: https://code.wireshark.org/review/19912
Petri-Dish: Michael Mann <mmann78@netscape.net>
Tested-by: Petri Dish Buildbot <buildbot-no-reply@wireshark.org>
Reviewed-by: Dario Lombardo <lomato@gmail.com>
Tested-by: Dario Lombardo <lomato@gmail.com>
Reviewed-by: Michael Mann <mmann78@netscape.net>
Alexis La Goutte