[Samuel GROOT <groot.samuel@xxxxxxxxx>]

Sorry for the late answer, I was working on something else these days, 
and didn't have time to write a proper reply.

> I believe what you want is:
> 1. A dissector for each vendor specific object
> 2. A (single) heuristic dissector for all vendor specific objects that 
> use common services.
> I thought I could simplify the existing CIP dissector and only use one 
> heuristic dissector (and just use a switch
> statement for the object IDs), but the "built in" objects have a way to 
> list their attributes so they can be dissected
> by the common service dissection routines (GAA, GAS, GAL, etc).

Your mean the static attribute_info_t cip_attribute_vals[] = {...} right?

Ideally I would just add to this structure all my objects' attributes, 
and {GAA|GAS|GAL|.} would take care of most of them, I would just need 
special functions to handle complex types like structures.


Btw, why handle complex attributes (ASAIK they're only structures) with 
custom function everytime (dissect_id_revision, dissect_id_status, 
dissect_time_sync_port_state_info, etc), there should be a (relatively) 
easy way to send it to a generic function along with a struct containing 
the layout of the dissected attribute structure?

This would greatly reduce the number of functions in the cip dissector 
and would make the whole thing easier to maintain.

I have in mind something like (in pseudo-code):

struct attribute_layout_t attribute_id_revision [] = {
  {"Major Revision", CIP_UINT8, ...},
  {"Minor Revision", CIP_UINT8}
};
/* To be sent to a generic attribute dissector instead of
   dissect_id_revision */

and

struct attribute_layout_t attribute_optional_service_list [] = {
  {"Number of Services", CIP_UINT16|CIP_INDEX_FOR_ARRAY, ...},
  {"Services Codes, CIP_UINT16|ARRAY}
}
/* To be sent to a generic attribute dissector instead of
   dissect_optional_service_list */

With this, we handle most of the common structures (struct, generic 
types, arrays, strings), and maybe use just specific functions for 
really complex types.

This is a completely separated thing to what comes next, so we could 
make a separate thread to keep things clean and readable.

  I think
> part of the reason I never added an
> "external" interface for the attribute lists was I wasn't sure how to 
> make it generic enough to fit into existing
> Wireshark APIs.

Maybe we can brainstorm together on ways to use Wireshark's API to 
integrate vendor-specific objects/attribute/services (some ideas below).

> I hope you can simplify some of the dissection with the single heuristic 
> function and a switch statement.
> But yes, you would have to rewrite (copy/paste) the value_strings used 
> in the dissector.
> There is also the option of writing a C plugin (which I've done before 
> and I know others have), but that is
> typically tied to a Wireshark release version (because APIs aren't 
> guaranteed to stay the same between
> major releases)
> In general, I would discourage adding vendor specific object code to the 
> Wireshark source base because
> of the high probability of collision (with another vendor's objects).  

A C plugin is not an option for the reasons you mentioned and many 
others, I'll stick to the lua plugins but reviewing first the different 
options available before developping anything.


The current way the CIP dissector works is (correct me if I'm wrong or 
misunderstood something):
1)   First dissect_cip(), that creates the tree for CIP and checks for
     enip info. In any cases, it calls dissect_cip_data()
2.1) dissect_cip_data() dissects the service (part A of the packet, see
     below) *before any calls to a heuristic/dissector table* (so it
     will output "Class Unknow (0x1234)" in the tree.
2.2) It then dissects the Status (part B of the packet, see below).
2.3) If it is a request, it dissects the EPATH.
2.4) Finally, it calls the heuristic.
2.5) If the heuristic returned False, it calls either a class-specific
     dissector, or a generic one.

CIP packet:
[ Service | Status | EPATH (request-only) | Payload ]
    (A)      (B)             (C)              (D)

The main issue here is that before calling any heuristic/dissector, most 
of the dissecting is already done (service/EPATH), and the .


The way I would do it:
1)   First dissect_cip(), that creates the tree for CIP and checks for
     enip info. In any cases, it calls dissect_cip_data()
2.1) dissect_cip_data() uses the heuristic table on the Service Code (A)
     (whether it is generic service code or not).
     If the heuristic returns True, the heuristic dissects the Service
     Code. For this, the heuristic can read the EPATH from the buffer to
     discover the class code if needed, or read from the previous packet
     if it's a response.
2.2) At this point, the Service code (A) has been dissected. Let's now
     dissect the Status (B), this one should relatively standard (if
     not, put a heuristic here).
2.3) If it is a request, call a new heuristic on the EPATH (C), if it
     returns False then dissect it ourselves.
2.4) Finally, call a heuristic on the payload (D), if there's one.


This way I think we can handle each part of the CIP packet individually, 
keeping the standard Wireshark API.

What do you think about it?

Regards,
Sam