From d2e5bd80cbc5c29f6a33dc6678baeaa4fd358c64 Mon Sep 17 00:00:00 2001 From: Dylan Ulis Date: Wed, 19 Oct 2022 15:23:32 +0000 Subject: [PATCH] CIP: Minor cleanup/refactoring --- epan/dissectors/packet-cip.c | 51 ++++++++++++++++-------------- epan/dissectors/packet-cip.h | 5 ++- epan/dissectors/packet-cipmotion.c | 4 +-- epan/dissectors/packet-enip.c | 12 +++---- 4 files changed, 36 insertions(+), 36 deletions(-) diff --git a/epan/dissectors/packet-cip.c b/epan/dissectors/packet-cip.c index 6c4a811fe0..26365c380f 100644 --- a/epan/dissectors/packet-cip.c +++ b/epan/dissectors/packet-cip.c @@ -4615,16 +4615,16 @@ static int dissect_segment_port(tvbuff_t* tvb, int offset, gboolean generate, if (generate) { /* Add size of extended link address */ - proto_item* it = proto_tree_add_uint(path_seg_item, hf_cip_link_address_size, tvb, 0, 0, opt_link_size); + proto_item* it = proto_tree_add_uint(path_seg_tree, hf_cip_link_address_size, tvb, 0, 0, opt_link_size); proto_item_set_generated(it); /* Add extended link address */ - it = proto_tree_add_string(path_seg_item, hf_cip_link_address_string, tvb, 0, 0, tvb_format_text(wmem_packet_scope(), tvb, offset + offset_link_address, opt_link_size)); + it = proto_tree_add_string(path_seg_tree, hf_cip_link_address_string, tvb, 0, 0, tvb_format_text(wmem_packet_scope(), tvb, offset + offset_link_address, opt_link_size)); proto_item_set_generated(it); } else { - proto_tree_add_item(path_seg_item, hf_cip_link_address_size, tvb, offset + 1, 1, ENC_LITTLE_ENDIAN); - proto_tree_add_item(path_seg_item, hf_cip_link_address_string, tvb, offset + offset_link_address, opt_link_size, ENC_ASCII | ENC_NA); + proto_tree_add_item(path_seg_tree, hf_cip_link_address_size, tvb, offset + 1, 1, ENC_LITTLE_ENDIAN); + proto_tree_add_item(path_seg_tree, hf_cip_link_address_string, tvb, offset + offset_link_address, opt_link_size, ENC_ASCII | ENC_NA); } proto_item_append_text(epath_item, ", Address: %s", tvb_format_text(wmem_packet_scope(), tvb, offset + offset_link_address, opt_link_size)); @@ -4656,12 +4656,12 @@ static int dissect_segment_port(tvbuff_t* tvb, int offset, gboolean generate, if (generate) { guint8 link_address_byte = tvb_get_guint8(tvb, offset + offset_link_address); - proto_item* it = proto_tree_add_uint(path_seg_item, hf_cip_link_address_byte, tvb, 0, 0, link_address_byte); + proto_item* it = proto_tree_add_uint(path_seg_tree, hf_cip_link_address_byte, tvb, 0, 0, link_address_byte); proto_item_set_generated(it); } else { - proto_tree_add_item(path_seg_item, hf_cip_link_address_byte, tvb, offset + offset_link_address, 1, ENC_LITTLE_ENDIAN); + proto_tree_add_item(path_seg_tree, hf_cip_link_address_byte, tvb, offset + offset_link_address, 1, ENC_LITTLE_ENDIAN); } proto_item_append_text(epath_item, ", Address: %d", tvb_get_guint8(tvb, offset + offset_link_address)); @@ -4672,12 +4672,12 @@ static int dissect_segment_port(tvbuff_t* tvb, int offset, gboolean generate, if (generate) { guint16 port_extended = tvb_get_letohs(tvb, extended_port_offset); - proto_item* it = proto_tree_add_uint(path_seg_item, hf_cip_port_extended, tvb, 0, 0, port_extended); + proto_item* it = proto_tree_add_uint(path_seg_tree, hf_cip_port_extended, tvb, 0, 0, port_extended); proto_item_set_generated(it); } else { - proto_tree_add_item(path_seg_item, hf_cip_port_extended, tvb, extended_port_offset, 2, ENC_LITTLE_ENDIAN); + proto_tree_add_item(path_seg_tree, hf_cip_port_extended, tvb, extended_port_offset, 2, ENC_LITTLE_ENDIAN); } } @@ -6835,8 +6835,7 @@ dissect_cip_cm_fwd_open_req(cip_req_info_t *preq_info, proto_tree *cmd_tree, tvb preq_info->connInfo->TransportClass_trigger = TransportClass_trigger; preq_info->connInfo->timeout_multiplier = timeout_multiplier; preq_info->connInfo->safety = safety_fwdopen; - preq_info->connInfo->ClassID = connection_path.iClass; - preq_info->connInfo->ConnPoint = connection_path.iConnPoint; + preq_info->connInfo->connection_path = connection_path; preq_info->connInfo->FwdOpenPathLenBytes = conn_path_size; preq_info->connInfo->pFwdOpenPathData = wmem_alloc(wmem_file_scope(), conn_path_size); @@ -6869,24 +6868,29 @@ static void display_previous_route_connection_path(cip_req_info_t *preq_info, pr } } +gboolean cip_connection_triad_match(const cip_connection_triad_t* left, const cip_connection_triad_t* right) +{ + return (left->ConnSerialNumber == right->ConnSerialNumber) && + (left->VendorID == right->VendorID) && + (left->DeviceSerialNumber == right->DeviceSerialNumber); +} + static int dissect_cip_cm_fwd_open_rsp_success(cip_req_info_t *preq_info, proto_tree *tree, tvbuff_t *tvb, int offset, packet_info *pinfo) { int parsed_len = 26; - unsigned char app_rep_size; - guint32 O2TConnID, T2OConnID; guint16 init_rollover_value = 0, init_timestamp_value = 0; proto_tree *pid_tree, *safety_tree; cip_connection_triad_t target_triad = {0}; /* Display originator to target connection ID */ - O2TConnID = tvb_get_letohl( tvb, offset ); - proto_tree_add_item( tree, hf_cip_cm_ot_connid, tvb, offset, 4, ENC_LITTLE_ENDIAN); + guint32 O2TConnID; + proto_tree_add_item_ret_uint(tree, hf_cip_cm_ot_connid, tvb, offset, 4, ENC_LITTLE_ENDIAN, &O2TConnID); /* Display target to originator connection ID */ - T2OConnID = tvb_get_letohl( tvb, offset+4 ); - proto_tree_add_item( tree, hf_cip_cm_to_connid, tvb, offset+4, 4, ENC_LITTLE_ENDIAN); + guint32 T2OConnID; + proto_tree_add_item_ret_uint(tree, hf_cip_cm_to_connid, tvb, offset+4, 4, ENC_LITTLE_ENDIAN, &T2OConnID); // Add Connection IDs as hidden items so that it's easy to find all Connection IDs in different fields. proto_item* pi = proto_tree_add_item(tree, hf_cip_connid, tvb, offset, 4, ENC_LITTLE_ENDIAN); @@ -6900,15 +6904,15 @@ dissect_cip_cm_fwd_open_rsp_success(cip_req_info_t *preq_info, proto_tree *tree, &conn_triad); /* Display originator to target actual packet interval */ - guint32 O2TAPI = tvb_get_letohl( tvb, offset+16 ); - proto_tree_add_item(tree, hf_cip_cm_ot_api, tvb, offset + 16, 4, ENC_LITTLE_ENDIAN); + guint32 O2TAPI; + proto_tree_add_item_ret_uint(tree, hf_cip_cm_ot_api, tvb, offset + 16, 4, ENC_LITTLE_ENDIAN, &O2TAPI); /* Display originator to target actual packet interval */ - guint32 T2OAPI = tvb_get_letohl( tvb, offset+20 ); - proto_tree_add_item(tree, hf_cip_cm_to_api, tvb, offset + 20, 4, ENC_LITTLE_ENDIAN); + guint32 T2OAPI; + proto_tree_add_item_ret_uint(tree, hf_cip_cm_to_api, tvb, offset + 20, 4, ENC_LITTLE_ENDIAN, &T2OAPI); /* Display the application reply size */ - app_rep_size = tvb_get_guint8( tvb, offset+24 ) * 2; + guint16 app_rep_size = tvb_get_guint8( tvb, offset+24 ) * 2; proto_tree_add_item(tree, hf_cip_cm_app_reply_size, tvb, offset+24, 1, ENC_LITTLE_ENDIAN); /* Display the Reserved byte */ @@ -6976,9 +6980,7 @@ dissect_cip_cm_fwd_open_rsp_success(cip_req_info_t *preq_info, proto_tree *tree, if ((preq_info != NULL) && (preq_info->connInfo != NULL)) { /* Ensure the connection triad matches before updating the connection IDs */ - if ((preq_info->connInfo->triad.ConnSerialNumber == conn_triad.ConnSerialNumber) && - (preq_info->connInfo->triad.VendorID == conn_triad.VendorID) && - (preq_info->connInfo->triad.DeviceSerialNumber == conn_triad.DeviceSerialNumber)) + if (cip_connection_triad_match(&(preq_info->connInfo->triad), &conn_triad)) { /* Update the connection IDs as ForwardOpen reply is allowed to update them from the ForwardOpen request */ @@ -6992,6 +6994,7 @@ dissect_cip_cm_fwd_open_rsp_success(cip_req_info_t *preq_info, proto_tree *tree, preq_info->connInfo->safety.running_rollover_value = init_rollover_value; preq_info->connInfo->safety.running_timestamp_value = init_timestamp_value; preq_info->connInfo->safety.target_triad = target_triad; + preq_info->connInfo->safety.seen_non_zero_timestamp = FALSE; } } } diff --git a/epan/dissectors/packet-cip.h b/epan/dissectors/packet-cip.h index 5e3add2bf0..5f5690bd7d 100644 --- a/epan/dissectors/packet-cip.h +++ b/epan/dissectors/packet-cip.h @@ -530,10 +530,9 @@ typedef struct cip_conn_info { guint8 TransportClass_trigger; guint32 timeout_multiplier; cip_safety_epath_info_t safety; - guint32 ClassID; - guint32 ConnPoint; guint32 FwdOpenPathLenBytes; void* pFwdOpenPathData; + cip_simple_request_info_t connection_path; // Information about specific packet numbers. guint32 open_req_frame; @@ -632,7 +631,7 @@ extern int dissect_padded_epath_len_uint(packet_info *pinfo, proto_tree *tree, extern void load_cip_request_data(packet_info *pinfo, cip_simple_request_info_t *req_data); extern void reset_cip_request_info(cip_simple_request_info_t* req_data); extern gboolean should_dissect_cip_response(tvbuff_t *tvb, int offset, guint8 gen_status); - +extern gboolean cip_connection_triad_match(const cip_connection_triad_t* left, const cip_connection_triad_t* right); /* ** Exported variables diff --git a/epan/dissectors/packet-cipmotion.c b/epan/dissectors/packet-cipmotion.c index 8dfefc9c72..c6c4612281 100644 --- a/epan/dissectors/packet-cipmotion.c +++ b/epan/dissectors/packet-cipmotion.c @@ -2073,7 +2073,7 @@ dissect_cipmotion(tvbuff_t* tvb, packet_info* pinfo, proto_tree* tree, void* dat guint8 ConnPoint = 2; if (io_data_input && io_data_input->conn_info) { - ConnPoint = io_data_input->conn_info->ConnPoint; + ConnPoint = io_data_input->conn_info->connection_path.iConnPoint; } /* Create display subtree for the protocol by creating an item and then @@ -2188,7 +2188,7 @@ static int dissect_cipmotion3(tvbuff_t* tvb, packet_info* pinfo, proto_tree* tre { cip_conn_info_t conn_info; memset(&conn_info, 0, sizeof(conn_info)); - conn_info.ConnPoint = 3; + conn_info.connection_path.iConnPoint = 3; cip_io_data_input io_data_input; io_data_input.conn_info = &conn_info; diff --git a/epan/dissectors/packet-enip.c b/epan/dissectors/packet-enip.c index 3a7e05ed34..ddf17f3690 100644 --- a/epan/dissectors/packet-enip.c +++ b/epan/dissectors/packet-enip.c @@ -1108,9 +1108,7 @@ enip_conn_equal(gconstpointer v, gconstpointer w) const enip_conn_key_t *v1 = (const enip_conn_key_t *)v; const enip_conn_key_t *v2 = (const enip_conn_key_t *)w; - if ((v1->triad.ConnSerialNumber == v2->triad.ConnSerialNumber) && - (v1->triad.VendorID == v2->triad.VendorID) && - (v1->triad.DeviceSerialNumber == v2->triad.DeviceSerialNumber) && + if (cip_connection_triad_match(&v1->triad, &v2->triad) && ((v1->O2TConnID == 0) || (v2->O2TConnID == 0) || (v1->O2TConnID == v2->O2TConnID)) && ((v1->T2OConnID == 0) || (v2->T2OConnID == 0) || (v1->T2OConnID == v2->T2OConnID))) return TRUE; @@ -2614,7 +2612,7 @@ static void dissect_cip_class01_io(packet_info* pinfo, tvbuff_t* tvb, int offset } else { - dissector_handle_t dissector = dissector_get_uint_handle(subdissector_io_table, conn_info->ClassID); + dissector_handle_t dissector = dissector_get_uint_handle(subdissector_io_table, conn_info->connection_path.iClass); if (dissector) { call_dissector_with_data(dissector, next_tvb, pinfo, dissector_tree, &io_data_input); @@ -2660,14 +2658,14 @@ static void dissect_cip_class23_data(packet_info* pinfo, tvbuff_t* tvb, int offs if (conn_info != NULL) { - dissector_handle_t dissector = dissector_get_uint_handle(subdissector_cip_connection_table, conn_info->ClassID); + dissector_handle_t dissector = dissector_get_uint_handle(subdissector_cip_connection_table, conn_info->connection_path.iClass); if (dissector) { - call_dissector_with_data(dissector, next_tvb, pinfo, dissector_tree, GUINT_TO_POINTER(conn_info->ClassID)); + call_dissector_with_data(dissector, next_tvb, pinfo, dissector_tree, GUINT_TO_POINTER(conn_info->connection_path.iClass)); } else { - call_dissector_with_data(cip_implicit_handle, next_tvb, pinfo, dissector_tree, GUINT_TO_POINTER(conn_info->ClassID)); + call_dissector_with_data(cip_implicit_handle, next_tvb, pinfo, dissector_tree, GUINT_TO_POINTER(conn_info->connection_path.iClass)); } } else