CIP: Improve connected data information

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>

1 files changed, 107 insertions, 53 deletions

diff --git a/epan/dissectors/packet-enip.c b/epan/dissectors/packet-enip.c
index deb4c6fd4b..1715ee2064 100644
--- a/epan/dissectors/packet-enip.c
+++ b/epan/dissectors/packet-enip.c
@@ -148,6 +148,8 @@ static int hf_enip_cpf_itemcount = -1;
 static int hf_enip_cpf_typeid = -1;
 static int hf_enip_cpf_length = -1;
 static int hf_cip_sequence_count = -1;
+static int hf_cip_cm_ot_api = -1;
+static int hf_cip_cm_to_api = -1;
 static int hf_enip_cpf_cai_connid = -1;
 static int hf_enip_cpf_sai_connid = -1;
 static int hf_enip_cpf_sai_seqnum = -1;
@@ -347,6 +349,8 @@ static gint ett_eip_cert_capability_flags = -1;
 static gint ett_eip_cert_num_certs = -1;
 static gint ett_security_profiles = -1;
 static gint ett_iana_port_state_flags = -1;
+static gint ett_connection_info = -1;
+static gint ett_connection_path_info = -1;
 
 static expert_field ei_mal_tcpip_status = EI_INIT;
 static expert_field ei_mal_tcpip_config_cap = EI_INIT;
@@ -745,9 +749,9 @@ enum enip_packet_type {ENIP_REQUEST_PACKET, ENIP_RESPONSE_PACKET, ENIP_CANNOT_CL
 enum enip_packet_data_type { EPDT_UNKNOWN, EPDT_CONNECTED_TRANSPORT, EPDT_UNCONNECTED };
 
 typedef struct enip_request_key {
+   guint32 session_handle;
    enum enip_packet_type      requesttype;
    enum enip_packet_data_type type;
-   guint32 session_handle;
    guint64 sender_context;
    guint32 conversation;
    union {
@@ -1066,7 +1070,6 @@ enip_open_cip_connection( packet_info *pinfo, cip_conn_info_t* connInfo)
    conversation_t   *conversation, *conversationTO;
    enip_conv_info_t *enip_info;
    address           dest_address;
-   ws_in6_addr ipv6_zero;
 
    if (pinfo->fd->flags.visited)
       return;
@@ -1099,14 +1102,18 @@ enip_open_cip_connection( packet_info *pinfo, cip_conn_info_t* connInfo)
       conn_val->close_frame            = 0;
       conn_val->connid                 = enip_unique_connid++;
 
+      conn_val->FwdOpenPathLenBytes = connInfo->FwdOpenPathLenBytes;
+      conn_val->pFwdOpenPathData = connInfo->pFwdOpenPathData;
+      conn_val->O2Tapi = connInfo->O2T.api;
+      conn_val->T2Oapi = connInfo->T2O.api;
+
       wmem_map_insert(enip_conn_hashtable, conn_key, conn_val );
 
       /* I/O connection */
       if (((connInfo->TransportClass_trigger & CI_TRANSPORT_CLASS_MASK) == 0) ||
           ((connInfo->TransportClass_trigger & CI_TRANSPORT_CLASS_MASK) == 1))
       {
-          /* zero out the ipv6 structure for comparison */
-          memset(&ipv6_zero, 0, sizeof(ipv6_zero));
+         ws_in6_addr ipv6_zero = {0};
 
          /* default some information if not included */
          if ((connInfo->O2T.port == 0) || (connInfo->O2T.type == CONN_TYPE_MULTICAST))
@@ -1203,8 +1210,7 @@ enip_open_cip_connection( packet_info *pinfo, cip_conn_info_t* connInfo)
    }
 
    /* Save the connection info for the conversation filter */
-   if (!pinfo->fd->flags.visited)
-      p_add_proto_data(wmem_file_scope(), pinfo, proto_enip, ENIP_CONNECTION_INFO, conn_val);
+   p_add_proto_data(wmem_file_scope(), pinfo, proto_enip, ENIP_CONNECTION_INFO, conn_val);
 }
 
 void
@@ -1226,8 +1232,7 @@ enip_close_cip_connection(packet_info *pinfo, const cip_connection_triad_t* tria
       conn_val->close_frame = pinfo->num;
 
       /* Save the connection info for the conversation filter */
-      if (!pinfo->fd->flags.visited)
-         p_add_proto_data(wmem_file_scope(), pinfo, proto_enip, ENIP_CONNECTION_INFO, conn_val);
+      p_add_proto_data(wmem_file_scope(), pinfo, proto_enip, ENIP_CONNECTION_INFO, conn_val);
    }
 }
 
@@ -2294,6 +2299,52 @@ static void dissect_item_list_services_response(packet_info* pinfo, tvbuff_t* tv
       tvb_format_stringzpad(tvb, offset + 4, 16));
 }
 
+static void display_fwd_open_connection_path(enip_conn_val_t* conn_info, proto_tree* tree, tvbuff_t* tvb, packet_info* pinfo)
+{
+   if (!conn_info->pFwdOpenPathData)
+   {
+      return;
+   }
+
+   tvbuff_t* tvbIOI = tvb_new_real_data((const guint8*)conn_info->pFwdOpenPathData, conn_info->FwdOpenPathLenBytes, conn_info->FwdOpenPathLenBytes);
+   if (tvbIOI)
+   {
+      proto_item* pi = NULL;
+      proto_tree* epath_tree = proto_tree_add_subtree(tree, tvb, 0, 0, ett_connection_path_info, &pi, "Connection Path: ");
+      PROTO_ITEM_SET_GENERATED(pi);
+
+      dissect_epath(tvbIOI, pinfo, epath_tree, pi, 0, conn_info->FwdOpenPathLenBytes, TRUE, FALSE, NULL, NULL, NO_DISPLAY, NULL, FALSE);
+      tvb_free(tvbIOI);
+   }
+}
+
+static void display_connection_information(packet_info* pinfo, tvbuff_t* tvb, proto_tree* tree, enip_conn_val_t* conn_info, enum enip_connid_type connid_type)
+{
+   proto_item* pi = NULL;
+   proto_tree* conn_info_tree = proto_tree_add_subtree(tree, tvb, 0, 0, ett_connection_info, &pi, "Connection Information");
+   PROTO_ITEM_SET_GENERATED(pi);
+
+   if (connid_type == ECIDT_O2T)
+   {
+       proto_item_append_text(pi, ": O->T");
+   }
+   else if (connid_type == ECIDT_T2O)
+   {
+       proto_item_append_text(pi, ": T->O");
+   }
+
+   display_fwd_open_connection_path(conn_info, conn_info_tree, tvb, pinfo);
+
+   pi = proto_tree_add_uint_format_value(conn_info_tree, hf_cip_cm_ot_api, tvb, 0, 0, conn_info->O2Tapi, "%dms (0x%08X)", conn_info->O2Tapi / 1000, conn_info->O2Tapi);
+   PROTO_ITEM_SET_GENERATED(pi);
+
+   pi = proto_tree_add_uint_format_value(conn_info_tree, hf_cip_cm_to_api, tvb, 0, 0, conn_info->T2Oapi, "%dms (0x%08X)", conn_info->T2Oapi / 1000, conn_info->T2Oapi);
+   PROTO_ITEM_SET_GENERATED(pi);
+
+   pi = proto_tree_add_uint(conn_info_tree, hf_enip_fwd_open_in, tvb, 0, 0, conn_info->open_frame);
+   PROTO_ITEM_SET_GENERATED(pi);
+}
+
 // This dissects Class 0 or Class 1 I/O.
 // offset - Starts at the field after the Item Length field.
 static int dissect_cip_io_generic(tvbuff_t *tvb, packet_info *pinfo _U_, proto_tree *tree, void* data)
@@ -2305,7 +2356,8 @@ static int dissect_cip_io_generic(tvbuff_t *tvb, packet_info *pinfo _U_, proto_t
    proto_item* ti = proto_tree_add_item(tree, proto_cipio, tvb, 0, -1, ENC_NA);
    proto_tree* io_tree = proto_item_add_subtree(ti, ett_cip_io_generic);
 
-   if (io_data_input != NULL) {
+   if (io_data_input != NULL)
+   {
       if ((io_data_input->conn_info->TransportClass_trigger & CI_TRANSPORT_CLASS_MASK) == 1)
       {
          proto_tree_add_item(io_tree, hf_cip_sequence_count, tvb, offset, 2, ENC_LITTLE_ENDIAN);
@@ -2368,10 +2420,6 @@ static void dissect_cip_class01_io(packet_info* pinfo, tvbuff_t* tvb, int offset
             call_dissector_with_data(cip_io_generic_handle, next_tvb, pinfo, dissector_tree, &io_data_input);
          }
       }
-
-      /* Save the connection info for the conversation filter */
-      if (!pinfo->fd->flags.visited)
-         p_add_proto_data(wmem_file_scope(), pinfo, proto_enip, ENIP_CONNECTION_INFO, conn_info);
    }
    else
    {
@@ -2398,10 +2446,6 @@ static void dissect_cip_class23_data(packet_info* pinfo, tvbuff_t* tvb, int offs
       request_info = enip_match_request(pinfo, tree, request_key);
    }
 
-   /* Save the connection info for the conversation filter */
-   if ((!pinfo->fd->flags.visited) && (conn_info != NULL))
-      p_add_proto_data(wmem_file_scope(), pinfo, proto_enip, ENIP_CONNECTION_INFO, conn_info);
-
    /* Add sequence count ( Transport Class 2,3 ) */
    proto_tree_add_item(item_tree, hf_cip_sequence_count, tvb, offset, 2, ENC_LITTLE_ENDIAN);
 
@@ -2475,29 +2519,31 @@ static void dissect_item_sockaddr_info(packet_info *pinfo, tvbuff_t* tvb, int of
 // offset - Starts at the Connection ID
 // Returns: connid_type, conn_info
 static void dissect_item_sequenced_address(packet_info* pinfo, tvbuff_t* tvb, int offset,
-   proto_tree* item_tree, proto_tree* tree,
-   enum enip_connid_type* connid_type, enip_conn_val_t** conn_info)
+   proto_tree* item_tree, enum enip_connid_type* connid_type, enip_conn_val_t** conn_info)
 {
-   *conn_info = enip_get_io_connid(pinfo, tvb_get_letohl(tvb, offset), connid_type);
-   proto_tree_add_item(item_tree, hf_enip_cpf_sai_connid, tvb, offset, 4, ENC_LITTLE_ENDIAN);
-   proto_tree_add_item(item_tree, hf_enip_cpf_sai_seqnum, tvb, offset + 4, 4, ENC_LITTLE_ENDIAN);
+   guint32 connection_id;
+   proto_tree_add_item_ret_uint(item_tree, hf_enip_cpf_sai_connid, tvb, offset, 4, ENC_LITTLE_ENDIAN, &connection_id);
+
+   guint32 sequence_num;
+   proto_tree_add_item_ret_uint(item_tree, hf_enip_cpf_sai_seqnum, tvb, offset + 4, 4, ENC_LITTLE_ENDIAN, &sequence_num);
 
-   if (*conn_info)
+   *conn_info = enip_get_io_connid(pinfo, connection_id, connid_type);
+
+   col_add_fstr(pinfo->cinfo, COL_INFO, "Connection: ID=0x%08X, SEQ=%010d", connection_id, sequence_num);
+   if (*connid_type == ECIDT_O2T)
    {
-      proto_item* it = proto_tree_add_uint(tree, hf_enip_fwd_open_in, tvb, 0, 0, (*conn_info)->open_frame);
-      PROTO_ITEM_SET_GENERATED(it);
+       col_append_str(pinfo->cinfo, COL_INFO, ", O->T");
+   }
+   else if (*connid_type == ECIDT_T2O)
+   {
+       col_append_str(pinfo->cinfo, COL_INFO, ", T->O");
    }
-
-   /* Add info to column */
-   col_add_fstr(pinfo->cinfo, COL_INFO, "Connection:  ID=0x%08X, SEQ=%010d",
-      tvb_get_letohl(tvb, offset),
-      tvb_get_letohl(tvb, offset + 4));
 }
 
 // offset - Starts at the Connection ID
 // Returns: conn_info
 static void dissect_item_connected_address(packet_info* pinfo, tvbuff_t* tvb, int offset,
-   proto_tree* item_tree, proto_tree* tree, proto_item* enip_item,
+   proto_tree* item_tree, proto_item* enip_item,
    enip_request_key_t* request_key, enip_conn_val_t** conn_info)
 {
    guint32 connection_id;
@@ -2511,18 +2557,12 @@ static void dissect_item_connected_address(packet_info* pinfo, tvbuff_t* tvb, in
    }
 
    /* Add Connection ID to Info col and tree */
-   col_append_fstr(pinfo->cinfo, COL_INFO, ", CONID: 0x%08X", connection_id);
+   col_append_fstr(pinfo->cinfo, COL_INFO, ", Connection: ID=0x%08X", connection_id);
 
    if (enip_item)
    {
       proto_item_append_text(enip_item, ", Connection ID: 0x%08X", connection_id);
    }
-
-   if (*conn_info)
-   {
-      proto_item* it = proto_tree_add_uint(tree, hf_enip_fwd_open_in, tvb, 0, 0, (*conn_info)->open_frame);
-      PROTO_ITEM_SET_GENERATED(it);
-   }
 }
 
 // offset - Starts at Unconn Msg Type
@@ -2570,7 +2610,7 @@ static void dissect_item_unconnected_message_over_udp(packet_info* pinfo, tvbuff
 /* Dissect Common Packet Format */
 static void
 dissect_cpf(enip_request_key_t *request_key, int command, tvbuff_t *tvb,
-            packet_info *pinfo, proto_tree *tree, proto_tree *dissector_tree,
+            packet_info *pinfo, proto_tree *tree, proto_tree *dissector_tree, proto_tree *enip_layer_tree,
             proto_item *enip_item, int offset, guint32 ifacehndl)
 {
    proto_item            *count_item;
@@ -2626,7 +2666,7 @@ dissect_cpf(enip_request_key_t *request_key, int command, tvbuff_t *tvb,
           {
             case CONNECTION_BASED:  // 1st Item for: Class 3 Connected Messages
                conn_info = NULL;
-               dissect_item_connected_address(pinfo, tvb, offset, item_tree, tree, enip_item, request_key, &conn_info);
+               dissect_item_connected_address(pinfo, tvb, offset, item_tree, enip_item, request_key, &conn_info);
                break;
 
             case UNCONNECTED_MSG_DTLS:  // Only item for: Unconnected messages over DTLS
@@ -2687,6 +2727,12 @@ dissect_cpf(enip_request_key_t *request_key, int command, tvbuff_t *tvb,
             }
 
             case CONNECTION_TRANSPORT:  // 2nd item for: Connected messages (both Class 0/1 and Class 3)
+               // Save the connection info for the conversation filter
+               if (!pinfo->fd->flags.visited && conn_info)
+               {
+                  p_add_proto_data(wmem_file_scope(), pinfo, proto_enip, ENIP_CONNECTION_INFO, conn_info);
+               }
+
                if (command == SEND_UNIT_DATA)  // Class 2/3 over TCP.
                {
                   dissect_cip_class23_data(pinfo, tvb, offset, tree, item_tree, item_length, request_key, conn_info, dissector_tree);
@@ -2696,6 +2742,11 @@ dissect_cpf(enip_request_key_t *request_key, int command, tvbuff_t *tvb,
                   dissect_cip_class01_io(pinfo, tvb, offset, item_length, conn_info, connid_type, dissector_tree);
                }
 
+               if (conn_info)
+               {
+                  display_connection_information(pinfo, tvb, enip_layer_tree, conn_info, connid_type);
+               }
+
                break;
 
             case LIST_IDENTITY_RESP:
@@ -2716,7 +2767,7 @@ dissect_cpf(enip_request_key_t *request_key, int command, tvbuff_t *tvb,
 
             case SEQ_ADDRESS:  // 1st item for: Class 0/1 connected data
                conn_info = NULL;
-               dissect_item_sequenced_address(pinfo, tvb, offset, item_tree, tree, &connid_type, &conn_info);
+               dissect_item_sequenced_address(pinfo, tvb, offset, item_tree, &connid_type, &conn_info);
                break;
 
             case LIST_SERVICES_RESP:
@@ -2799,7 +2850,6 @@ dissect_enip_pdu(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree, void* data
    guint16             encap_cmd, encap_data_length;
    const char         *pkt_type_str = "";
    guint32             ifacehndl;
-   enip_request_key_t  request_key;
    conversation_t     *conversation;
 
    /* Set up structures needed to add the protocol subtree and manage it */
@@ -2844,7 +2894,7 @@ dissect_enip_pdu(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree, void* data
     * Attach that information to the conversation, and add
     * it to the list of information structures later before dissection.
     */
-   memset( &request_key, 0, sizeof(enip_request_key_t) );
+   enip_request_key_t  request_key = {0};
    request_key.requesttype    = packet_type;
    request_key.type           = EPDT_UNKNOWN;
    request_key.session_handle = tvb_get_letohl( tvb, 4 );
@@ -2909,15 +2959,15 @@ dissect_enip_pdu(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree, void* data
             break;
 
          case LIST_SERVICES:
-            dissect_cpf( &request_key, encap_cmd, tvb, pinfo, csftree, tree, NULL, 24, 0 );
+            dissect_cpf( &request_key, encap_cmd, tvb, pinfo, csftree, tree, enip_tree, NULL, 24, 0 );
             break;
 
          case LIST_IDENTITY:
-            dissect_cpf( &request_key, encap_cmd, tvb, pinfo, csftree, tree, NULL, 24, 0 );
+            dissect_cpf( &request_key, encap_cmd, tvb, pinfo, csftree, tree, enip_tree, NULL, 24, 0 );
             break;
 
          case LIST_INTERFACES:
-            dissect_cpf( &request_key, encap_cmd, tvb, pinfo, csftree, tree, NULL, 24, 0 );
+            dissect_cpf( &request_key, encap_cmd, tvb, pinfo, csftree, tree, enip_tree, NULL, 24, 0 );
             break;
 
          case REGISTER_SESSION:
@@ -2933,7 +2983,7 @@ dissect_enip_pdu(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree, void* data
             proto_tree_add_item( csftree, hf_enip_timeout,        tvb, 28, 2, ENC_LITTLE_ENDIAN );
 
             ifacehndl = tvb_get_letohl( tvb, 24 );
-            dissect_cpf( &request_key, encap_cmd, tvb, pinfo, csftree, tree, NULL, 30, ifacehndl );
+            dissect_cpf( &request_key, encap_cmd, tvb, pinfo, csftree, tree, enip_tree, NULL, 30, ifacehndl );
             break;
 
          case SEND_UNIT_DATA:
@@ -2941,7 +2991,7 @@ dissect_enip_pdu(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree, void* data
             proto_tree_add_item( csftree, hf_enip_timeout,        tvb, 28, 2, ENC_LITTLE_ENDIAN );
 
             ifacehndl = tvb_get_letohl( tvb, 24 );
-            dissect_cpf( &request_key, encap_cmd, tvb, pinfo, csftree, tree, ti, 30, ifacehndl );
+            dissect_cpf( &request_key, encap_cmd, tvb, pinfo, csftree, tree, enip_tree, ti, 30, ifacehndl );
 
             break;
 
@@ -3004,10 +3054,10 @@ dissect_cipio(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree, void* data _U
 
    enip_tree = proto_item_add_subtree(ti, ett_enip);
 
-   dissect_cpf( NULL, 0xFFFF, tvb, pinfo, enip_tree, tree, NULL, 0, 0 );
+   dissect_cpf( NULL, 0xFFFF, tvb, pinfo, enip_tree, tree, enip_tree, NULL, 0, 0 );
 
    return tvb_captured_length(tvb);
-} /* end of dissect_enipio() */
+}
 
 
 static gboolean
@@ -3152,7 +3202,7 @@ dissect_dlr(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree, void *data _U_)
 
 static int dissect_cip_class1(tvbuff_t* tvb, packet_info* pinfo, proto_tree* tree, void* data _U_)
 {
-   enip_conn_val_t conn_info;
+   enip_conn_val_t conn_info = {0};
    conn_info.TransportClass_trigger = 1;
 
    cip_io_data_input io_data_input;
@@ -3492,6 +3542,9 @@ proto_register_enip(void)
         { "Forward Open Request In", "enip.fwd_open_in",
         FT_FRAMENUM, BASE_NONE, NULL, 0, NULL, HFILL } },
 
+      // Generated API data.
+      { &hf_cip_cm_ot_api, { "O->T API", "cip.cm.otapi", FT_UINT32, BASE_DEC, NULL, 0, NULL, HFILL } },
+      { &hf_cip_cm_to_api, { "T->O API", "cip.cm.toapi", FT_UINT32, BASE_DEC, NULL, 0, NULL, HFILL } },
       { &hf_cip_io_data,
         { "Data", "cipio.data",
           FT_BYTES, BASE_NONE|BASE_ALLOW_ZERO, NULL, 0x0,
@@ -4291,7 +4344,9 @@ proto_register_enip(void)
       &ett_eip_cert_capability_flags,
       &ett_eip_cert_num_certs,
       &ett_security_profiles,
-      &ett_iana_port_state_flags
+      &ett_iana_port_state_flags,
+      &ett_connection_info,
+      &ett_connection_path_info
    };
 
    static ei_register_info ei[] = {
@@ -4555,7 +4610,6 @@ proto_register_enip(void)
    enip_tcp_handle = register_dissector("enip", dissect_enip_tcp, proto_enip);
    cipio_handle = register_dissector("cipio", dissect_cipio, proto_cipio);
    cip_class1_handle = register_dissector("cipio_class1", dissect_cip_class1, proto_cip_class1);
-
    cip_io_generic_handle = register_dissector("cipgenericio", dissect_cip_io_generic, proto_cipio);
 
    /* Required function calls to register the header fields and subtrees used */