Wireshark · Wireshark-dev: Re: [Wireshark-dev] Duplicate dissectors (anonymous) and (anonymous) for protocol xxx

[John Dill <John.Dill@xxxxxxxxxxxxxxxxx>]

>Date: Wed, 25 Oct 2017 16:56:30 -0400
>From: Michael Mann <m...@xxxxxxxxxxxx>
>To: wireshark-dev@xxxxxxxxxxxxx
>Subject: Re: [Wireshark-dev] Duplicate dissectors (anonymous) and
>        (anonymous) for protocol xxx
>
>If you create separate dissector handles with a protocol, how is a user using "Decode As" functionality supposed to know which >dissection function will be picked?  That's why the check was added.
>You can have the same protocol ID between TCP and UDP, but you can't have the same one twice in TCP.>
>
>To me there are 2 solutions
>1. Create "placeholder" protocols with proto_register_protocol().  The "protocol lD" returned can be used to differentiate the
>different dissector function handles
>2. Create a "pino" (protocol in name only) with proto_register_protocol_in_name_only.  This allows you to create "helper"
>protocols, with most of the "control" still with the "parent" protocol created with proto_register_protocol.
>
>I believe #2 is the "preferred" solution.

Can you point me to a relatively simple dissector that implements a "pino"?  I read the README.dissector
section 2.9, but it wasn't enough description for me to grok it enough to know how to implement one.

>Date: Wed, 25 Oct 2017 14:29:14 -0700
>From: Guy Harris <g...@xxxxxxxxxxxx>
>To: Developer support list for Wireshark <wireshark-dev@xxxxxxxxxxxxx>
>Subject: Re: [Wireshark-dev] Duplicate dissectors (anonymous) and
>        (anonymous) for protocol xxx
>
>On Oct 25, 2017, at 1:44 PM, John Dill <John.Dill@xxxxxxxxxxxxxxxxx> wrote:
>
>> I just happened to turn on console printing to troubleshoot a different problem and I'm getting
>>a couple of interesting messages when I change my protocol preferences.
>>
>> Duplicate dissectors (anonymous) and (anonymous) for protocol xxx in dissector table tcp.port
>
>        ...
>
>> I have a proto_reg_handoff_xxx that creates a couple of TCP port dissector handles using
>>'dissector_add_uint("tcp.port", MY_TCP_PORT, tcp_handle)',
>
>Why *two* handles?  You can register the handle tcp_handle twice with two different TCP ports.
>
>If the format of the packets is the same for both ports, you *should* use the same dissector
>handle; if *that* causes an error, that's a bug.
>
>If the format of the packets is *not* the same for both ports, then you should use different
>dissector handles - with different dissector functions - *and* ensure that the dissectors have
>different names (which means they have to *have* names, so use "register_dissector()"
>rather than "create_dissector_handle()" to create the handles), so that, as Michael Mann
>noted, the user can choose one or the other of them for "Decode As...".

I'm trying to get a handle on the dissector registration but I'm not really getting it.  I'm trying
to understand the usage of create_dissector_handle vs register_dissector, but I can't seem to
rearrange my dissector code in a way that gets rid of the error messages that are showing.
I've probably lost understanding somewhere on the way from porting 1.12 dissector code to
2.4.x.

The README.dissector sections for register_dissector, create_dissector_handle, and the
heuristic stuff is a bit too sparse for me to grok how to use it (at least enough to get rid of
these messages):

Duplicate dissectors (anonymous) and (anonymous) for protocol NDO in dissector table tcp.port
Protocol Network Data Object Protocol is already registered in "udp" table

Wireshark still seems to dissect the messages fine, but I fear it's because I'm lucky it's working
out.  And I haven't been able to discern how it works by studying different packet-xxx.c in the
epan/dissectors folder.  There's so many different dissector implementations that I can't
quite nail down a dissector that matches what I'm doing in my implementation.

Here is a snippet of the code that I'm using, maybe the issue will be obvious to someone
on this list.

\code
/* Forward declare the Wireshark TCP packet dissector interface. */
static guint get_tcp_pdu_len(packet_info *pinfo, tvbuff_t *tvb, int offset, void *data);
static gint dissect_ndo_tcp(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree, void *data);
static gint dissect_ndo_tcp_pdu(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree, void *data);

/* Forward declare the Wireshark UDP packet dissector interface. */
static gboolean dissect_ndo_udp_heur(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree, void *data);

/* Forward declare the Wireshark protocol hand-off registration. */
void proto_register_ndo(void);
void proto_reg_handoff_ndo(void);

/* Forward declare the sub-dissectors to handle each data class. */
static void ndo_dissect_tcp_idl(tvbuff_t *tvb, packet_info *pinfo, gint offset, proto_tree *tree);
static void ndo_dissect_idl(tvbuff_t *tvb, packet_info *pinfo, gint offset, proto_tree *tree);
static void ndo_dissect_a429(tvbuff_t *tvb, packet_info *pinfo, gint offset, proto_tree *tree, gboolean time_tag);

/* Forward declare the individual NDO message sub-dissectors to handle each NDO ID. */
static void dissect_ndo_xxx(tvbuff_t *tvb, gint offset, proto_tree *tree);
... long list of individual NDO message dissectors ...

void proto_register_ndo(void)
{
  /*
    * The header field 'hf' registration info declares an array of
    * structures to describe field data and how to convert it into
    * text for display in wireshark.
    */
  static hf_register_info hf[] =
  {
    {
      &hf_tcp_payload_len,          /* p_id */
      {
        "Payload Length",           /* name */
        "ndo.tcp_payload_len",      /* abbrev */
        FT_UINT32,                  /* type */
        BASE_DEC,                   /* display */
        NULL,                       /* strings */
        0x0,                        /* bitmask */
        NULL,                       /* blurb */
        HFILL
      }
    },
     ... lots of kloc of header field definitions ...
  };

  /* Every expansion tree state defined above should be declared
     in this protocol subtree array. */
  static gint *ett[] =
  {
    &ett_ndo,
    &ett_ndo_d,
    &ett_ndo_p,

   ... long list of ett ...
  };

  /* Register the NDO protocol. */

  /* proto_register_protocol(name, short_name, abbreviation) */
  proto_ndo = proto_register_protocol("Network Data Object Protocol",
                                      "NDO",
                                      "ndo");

  /* The Wireshark defined macro 'array_length' calculates the number
     of elements of a fixed-size array at the compile phase. */
  proto_register_field_array(proto_ndo, hf, array_length(hf));
  proto_register_subtree_array(ett, array_length(ett));

  /* Register configuration options for the NDO protocol dissector. */
  {
    module_t *ndo_module;
    ndo_module = prefs_register_protocol(proto_ndo, proto_reg_handoff_ndo);

    prefs_register_bool_preference(ndo_module,
      "display_arinc_429_bitfields",
      "Display ARINC 429 bitfields in Packet Details pane",
      "Display the bit location of individual data elements in the "
      "payload of an ARINC 429 message.",
      &display_arinc_429_bitfields);

    prefs_register_bool_preference(ndo_module,
      "desegment_ndo_messages",
      "Reassemble NDO messages spanning multiple TCP segments",
      "Whether the NDO dissector should reassemble NDO messages "
      "spanning multiple TCP segments.  To use this option, you "
      "must also enable \"Allow sub-dissectors to reassemble TCP streams\" "
      "in the TCP protocol settings.",
      &desegment_ndo_messages);

    prefs_register_uint_preference(ndo_module,
      "udp.port",
      "Time in microseconds for each ARINC 429 time tag tick",
      "Set the number of microseconds for each time tag tick",
      10,
      &arinc_429_timetag_tick);
  }
}

void
proto_reg_handoff_ndo(void)
{
  /*
   * To use the tcp_dissect_pdus API function to reassemble TCP
   * segments into a single packet, each TCP port needs to register
   * a dissector function with the TCP port that the NDO messages
   * are transmitted on.
   *
   * Dissector handles are also useful for creating conversations,
   * but no support is implemented for them at this time.
   */
  dissector_handle_t ndo_tcp_handle;

  /* new_create_dissector_handle is for dissectors with the 'void *data' parameter. */
  ndo_tcp_handle = create_dissector_handle(dissect_ndo_tcp, proto_ndo);

  dissector_add_uint("tcp.port", FLIGHT_PLAN_TCP_PORT, ndo_tcp_handle);
  dissector_add_uint("tcp.port", CDU_DISPLAY_TCP_PORT, ndo_tcp_handle);

  heur_dissector_add("udp", dissect_ndo_udp_heur, "Network Data Object", "ndo", proto_ndo, HEURISTIC_ENABLE);
}
\endcode

I guess some questions I'm trying to work out are the following.

1. I don't know when to use static dissector_handle_t vs locally defined variables in
proto_reg_handoff_xxx.
2. I don't get the use cases of when to prefer create_dissector_handle or register_dissector,
and how they interact with the callback tables.
3. I don't know when to use tvb_reported_length vs tvb_captured_length.
4. I currently use a heuristic for all UDP NDO messages.  It parses the NDO header, figures
out the NDO ID, and passes it to the payload dissector function.  If I were to restructure my
dissector so that each UDP port had it's own dissector (there's about 25 UDP ports that carry
NDO messages), is that more recommended than a single heuristic?

Sorry if I'm being a bit dense.  The code seems to work, though I don't like not knowing why
it still works when I get these error messages.  I don't expect you to answer everything, but
if there's links to some docs/wiki/specific dissector that may help, that would be great though.

Best regards,
John D.