T.38: Support reassembly of more than two data items in a frame

Store information about more than one reassembly in the given frame.

Don't increment additional_hdlc_data_field_counter on every pass,
but use the value from the first pass in the packet data.

Store the data field item number of the first data field in a
reassembly, to identify the fragment sequence number of when in
the first frame of the reassemble.

This produces a consistent set of fragment sequence numbers that
start at 0 and increment by 1, regardless of if there are more than
two data field fragments in a frame, or if there is data starting
a new reassembly in the same frame as a signal end ending a reassembly.

This still doesn't handle the case where more than one reassembly
begins in the same frame (as opposed to one ending and another beginning
in the same frame.) That would require changing the reassembly ID to
be a value guaranteed to increment for each reassembly, instead of
using the frame number of the first fragment.

Fix #12552. Fix #5792.
This commit is contained in:
John Thacker 2023-07-13 00:11:30 -04:00
parent 4b377dd250
commit ca1a477921
5 changed files with 82 additions and 42 deletions

View File

@ -257,6 +257,7 @@ void t38_add_address(packet_info *pinfo,
p_conversation_data->src_t38_info.time_first_t4_data = 0;
p_conversation_data->src_t38_info.additional_hdlc_data_field_counter = 0;
p_conversation_data->src_t38_info.seqnum_prev_data_field = -1;
p_conversation_data->src_t38_info.next = NULL;
p_conversation_data->dst_t38_info.reass_ID = 0;
p_conversation_data->dst_t38_info.reass_start_seqnum = -1;
@ -267,6 +268,7 @@ void t38_add_address(packet_info *pinfo,
p_conversation_data->dst_t38_info.time_first_t4_data = 0;
p_conversation_data->dst_t38_info.additional_hdlc_data_field_counter = 0;
p_conversation_data->dst_t38_info.seqnum_prev_data_field = -1;
p_conversation_data->dst_t38_info.next = NULL;
}
@ -437,6 +439,7 @@ init_t38_info_conv(packet_info *pinfo)
p_t38_conv->src_t38_info.time_first_t4_data = 0;
p_t38_conv->src_t38_info.additional_hdlc_data_field_counter = 0;
p_t38_conv->src_t38_info.seqnum_prev_data_field = -1;
p_t38_conv->src_t38_info.next = NULL;
p_t38_conv->dst_t38_info.reass_ID = 0;
p_t38_conv->dst_t38_info.reass_start_seqnum = -1;
@ -447,6 +450,7 @@ init_t38_info_conv(packet_info *pinfo)
p_t38_conv->dst_t38_info.time_first_t4_data = 0;
p_t38_conv->dst_t38_info.additional_hdlc_data_field_counter = 0;
p_t38_conv->dst_t38_info.seqnum_prev_data_field = -1;
p_t38_conv->dst_t38_info.next = NULL;
conversation_add_proto_data(p_conv, proto_t38, p_t38_conv);
}

View File

@ -35,11 +35,13 @@ typedef struct _t38_packet_info {
/* Info to save the State to reassemble Data (e.g. HDLC) and the Setup (e.g. SDP) in T38 conversations */
typedef struct _t38_conv_info
{
typedef struct _t38_conv_info t38_conv_info;
struct _t38_conv_info {
guint32 reass_ID;
int reass_start_seqnum;
guint32 reass_start_data_field;
guint32 reass_data_type;
gint32 last_seqnum; /* used to avoid duplicated seq num shown in the Graph Analysis */
guint32 packet_lost;
@ -47,8 +49,9 @@ typedef struct _t38_conv_info
double time_first_t4_data;
guint32 additional_hdlc_data_field_counter;
gint32 seqnum_prev_data_field;
t38_conv_info *next;
} t38_conv_info;
};
/* Info to save the State to reassemble Data (e.g. HDLC) and the Setup (e.g. SDP) in T38 conversations */
typedef struct _t38_conv

View File

@ -63,11 +63,7 @@ VAL_PTR=&Data_Field_field_type_value
val_to_str(Data_Field_field_type_value,t38_T_field_type_vals,"<unknown>"));
}
/* We only reassmeble packets in the Primary part and in the first two Items. */
/* There maybe be t38 packets with more than two Items, but reassemble those packets is not easy */
/* using the current ressaemble functions. */
/* TODO: reassemble all the Items in one frame */
if (primary_part && (Data_Field_item_num<2)) {
if (primary_part) {
if (Data_Field_field_type_value == 2 || Data_Field_field_type_value == 4 || Data_Field_field_type_value == 7) {/* hdlc-fcs-OK or hdlc-fcs-OK-sig-end or t4-non-ecm-sig-end*/
fragment_head *frag_msg = NULL;
tvbuff_t* new_tvb = NULL;
@ -77,11 +73,17 @@ VAL_PTR=&Data_Field_field_type_value
/* if reass_start_seqnum=-1 it means we have received the end of the fragmente, without received any fragment data */
if (p_t38_packet_conv_info->reass_start_seqnum != -1) {
guint32 frag_seq_num;
if (seq_number == (guint32)p_t38_packet_conv_info->reass_start_seqnum) {
frag_seq_num = (guint32)p_t38_packet_conv_info->additional_hdlc_data_field_counter + Data_Field_item_num - p_t38_packet_conv_info->reass_start_data_field;
} else {
frag_seq_num = seq_number - (guint32)p_t38_packet_conv_info->reass_start_seqnum + (guint32)p_t38_packet_conv_info->additional_hdlc_data_field_counter + Data_Field_item_num;
}
frag_msg = fragment_add_seq(&data_reassembly_table, /* reassembly table */
tvb, offset, actx->pinfo,
p_t38_packet_conv_info->reass_ID, /* ID for fragments belonging together */
NULL,
seq_number + Data_Field_item_num - (guint32)p_t38_packet_conv_info->reass_start_seqnum + (guint32)p_t38_packet_conv_info->additional_hdlc_data_field_counter, /* fragment sequence number */
frag_seq_num, /* fragment sequence number */
/*0,*/
0, /* fragment length */
FALSE, /* More fragments */
@ -148,6 +150,13 @@ VAL_PTR=&Data_Field_field_type_value
p_t38_conv_info->reass_start_seqnum = -1;
p_t38_conv_info->additional_hdlc_data_field_counter = 0;
p_t38_conv_info->seqnum_prev_data_field = -1;
if (p_t38_packet_conv_info->next == NULL) {
p_t38_packet_conv_info->next = wmem_new(wmem_file_scope(), t38_conv_info);
p_t38_packet_conv_info = p_t38_packet_conv_info->next;
memcpy(p_t38_packet_conv_info, p_t38_conv_info, sizeof(t38_conv_info));
}
}
t38_info->Data_Field_field_type_value = Data_Field_field_type_value;
}
@ -172,11 +181,7 @@ VAL_PTR=&Data_Field_field_type_value
}
}
/* We only reassmeble packets in the Primary part and in the first two Items. */
/* There maybe be t38 packets with more than two Items, but reassemble those packets is not easy */
/* using the current ressaemble functions. */
/* TODO: reassemble all the Items in one frame */
if (primary_part && (Data_Field_item_num<2)) {
if (primary_part) {
fragment_head *frag_msg = NULL;
/* HDLC Data or t4-non-ecm-data */
@ -186,30 +191,40 @@ VAL_PTR=&Data_Field_field_type_value
actx->pinfo->fragmented = TRUE;
/* if we have not reassembled this packet and it is the first fragment, reset the reassemble ID and the start seq number*/
if (p_t38_packet_conv && p_t38_conv && (p_t38_packet_conv_info->reass_ID == 0)) {
if (p_t38_packet_conv && p_t38_conv && (p_t38_packet_conv_info->reass_start_seqnum == -1)) {
/* we use the first fragment's frame_number as fragment ID because the protocol doesn't provide it */
/* XXX: We'd be better off assigning our own IDs using a one-up
* counter, if it's possible for more than one reassembly to
* begin in the same frame.
*/
p_t38_conv_info->reass_ID = actx->pinfo->num;
p_t38_conv_info->reass_start_seqnum = seq_number;
p_t38_conv_info->time_first_t4_data = nstime_to_sec(&actx->pinfo->rel_ts);
p_t38_conv_info->additional_hdlc_data_field_counter = 0;
p_t38_packet_conv_info->reass_ID = p_t38_conv_info->reass_ID;
p_t38_packet_conv_info->reass_start_seqnum = p_t38_conv_info->reass_start_seqnum;
p_t38_packet_conv_info->reass_start_data_field = Data_Field_item_num;
p_t38_packet_conv_info->seqnum_prev_data_field = p_t38_conv_info->seqnum_prev_data_field;
p_t38_packet_conv_info->additional_hdlc_data_field_counter = p_t38_conv_info->additional_hdlc_data_field_counter;
p_t38_packet_conv_info->time_first_t4_data = p_t38_conv_info->time_first_t4_data;
}
if (seq_number == (guint32)p_t38_packet_conv_info->seqnum_prev_data_field){
p_t38_packet_conv_info->additional_hdlc_data_field_counter ++;
if(p_t38_conv){
p_t38_conv_info->additional_hdlc_data_field_counter = p_t38_packet_conv_info->additional_hdlc_data_field_counter;
p_t38_conv_info->additional_hdlc_data_field_counter++;
}
}
guint32 frag_seq_num;
if (seq_number == (guint32)p_t38_packet_conv_info->reass_start_seqnum) {
frag_seq_num = (guint32)p_t38_packet_conv_info->additional_hdlc_data_field_counter + Data_Field_item_num - (guint32)p_t38_packet_conv_info->reass_start_data_field;
} else {
frag_seq_num = seq_number - (guint32)p_t38_packet_conv_info->reass_start_seqnum + (guint32)p_t38_packet_conv_info->additional_hdlc_data_field_counter + Data_Field_item_num;
}
frag_msg = fragment_add_seq(&data_reassembly_table,
value_tvb, 0,
actx->pinfo,
p_t38_packet_conv_info->reass_ID, /* ID for fragments belonging together */
NULL,
seq_number - (guint32)p_t38_packet_conv_info->reass_start_seqnum + (guint32)p_t38_packet_conv_info->additional_hdlc_data_field_counter, /* fragment sequence number */
frag_seq_num, /* fragment sequence number */
value_len, /* fragment length */
TRUE, /* More fragments */
0);
@ -219,9 +234,7 @@ VAL_PTR=&Data_Field_field_type_value
if (!frag_msg) { /* Not last packet of reassembled */
if (Data_Field_field_type_value == 0) {
col_append_fstr(actx->pinfo->cinfo, COL_INFO," (HDLC fragment %u)",
seq_number + (guint32)p_t38_packet_conv_info->additional_hdlc_data_field_counter
- (guint32)p_t38_packet_conv_info->reass_start_seqnum);
col_append_fstr(actx->pinfo->cinfo, COL_INFO," (HDLC fragment %u)", frag_seq_num);
} else {
col_append_fstr(actx->pinfo->cinfo, COL_INFO," (t4-data fragment %u)", seq_number - (guint32)p_t38_packet_conv_info->reass_start_seqnum);
}

View File

@ -287,6 +287,7 @@ void t38_add_address(packet_info *pinfo,
p_conversation_data->src_t38_info.time_first_t4_data = 0;
p_conversation_data->src_t38_info.additional_hdlc_data_field_counter = 0;
p_conversation_data->src_t38_info.seqnum_prev_data_field = -1;
p_conversation_data->src_t38_info.next = NULL;
p_conversation_data->dst_t38_info.reass_ID = 0;
p_conversation_data->dst_t38_info.reass_start_seqnum = -1;
@ -297,6 +298,7 @@ void t38_add_address(packet_info *pinfo,
p_conversation_data->dst_t38_info.time_first_t4_data = 0;
p_conversation_data->dst_t38_info.additional_hdlc_data_field_counter = 0;
p_conversation_data->dst_t38_info.seqnum_prev_data_field = -1;
p_conversation_data->dst_t38_info.next = NULL;
}
@ -534,11 +536,7 @@ dissect_t38_T_field_type(tvbuff_t *tvb _U_, int offset _U_, asn1_ctx_t *actx _U_
val_to_str(Data_Field_field_type_value,t38_T_field_type_vals,"<unknown>"));
}
/* We only reassmeble packets in the Primary part and in the first two Items. */
/* There maybe be t38 packets with more than two Items, but reassemble those packets is not easy */
/* using the current ressaemble functions. */
/* TODO: reassemble all the Items in one frame */
if (primary_part && (Data_Field_item_num<2)) {
if (primary_part) {
if (Data_Field_field_type_value == 2 || Data_Field_field_type_value == 4 || Data_Field_field_type_value == 7) {/* hdlc-fcs-OK or hdlc-fcs-OK-sig-end or t4-non-ecm-sig-end*/
fragment_head *frag_msg = NULL;
tvbuff_t* new_tvb = NULL;
@ -548,11 +546,17 @@ dissect_t38_T_field_type(tvbuff_t *tvb _U_, int offset _U_, asn1_ctx_t *actx _U_
/* if reass_start_seqnum=-1 it means we have received the end of the fragmente, without received any fragment data */
if (p_t38_packet_conv_info->reass_start_seqnum != -1) {
guint32 frag_seq_num;
if (seq_number == (guint32)p_t38_packet_conv_info->reass_start_seqnum) {
frag_seq_num = (guint32)p_t38_packet_conv_info->additional_hdlc_data_field_counter + Data_Field_item_num - p_t38_packet_conv_info->reass_start_data_field;
} else {
frag_seq_num = seq_number - (guint32)p_t38_packet_conv_info->reass_start_seqnum + (guint32)p_t38_packet_conv_info->additional_hdlc_data_field_counter + Data_Field_item_num;
}
frag_msg = fragment_add_seq(&data_reassembly_table, /* reassembly table */
tvb, offset, actx->pinfo,
p_t38_packet_conv_info->reass_ID, /* ID for fragments belonging together */
NULL,
seq_number + Data_Field_item_num - (guint32)p_t38_packet_conv_info->reass_start_seqnum + (guint32)p_t38_packet_conv_info->additional_hdlc_data_field_counter, /* fragment sequence number */
frag_seq_num, /* fragment sequence number */
/*0,*/
0, /* fragment length */
FALSE, /* More fragments */
@ -619,6 +623,13 @@ dissect_t38_T_field_type(tvbuff_t *tvb _U_, int offset _U_, asn1_ctx_t *actx _U_
p_t38_conv_info->reass_start_seqnum = -1;
p_t38_conv_info->additional_hdlc_data_field_counter = 0;
p_t38_conv_info->seqnum_prev_data_field = -1;
if (p_t38_packet_conv_info->next == NULL) {
p_t38_packet_conv_info->next = wmem_new(wmem_file_scope(), t38_conv_info);
p_t38_packet_conv_info = p_t38_packet_conv_info->next;
memcpy(p_t38_packet_conv_info, p_t38_conv_info, sizeof(t38_conv_info));
}
}
t38_info->Data_Field_field_type_value = Data_Field_field_type_value;
}
@ -649,11 +660,7 @@ dissect_t38_T_field_data(tvbuff_t *tvb _U_, int offset _U_, asn1_ctx_t *actx _U_
}
}
/* We only reassmeble packets in the Primary part and in the first two Items. */
/* There maybe be t38 packets with more than two Items, but reassemble those packets is not easy */
/* using the current ressaemble functions. */
/* TODO: reassemble all the Items in one frame */
if (primary_part && (Data_Field_item_num<2)) {
if (primary_part) {
fragment_head *frag_msg = NULL;
/* HDLC Data or t4-non-ecm-data */
@ -663,30 +670,40 @@ dissect_t38_T_field_data(tvbuff_t *tvb _U_, int offset _U_, asn1_ctx_t *actx _U_
actx->pinfo->fragmented = TRUE;
/* if we have not reassembled this packet and it is the first fragment, reset the reassemble ID and the start seq number*/
if (p_t38_packet_conv && p_t38_conv && (p_t38_packet_conv_info->reass_ID == 0)) {
if (p_t38_packet_conv && p_t38_conv && (p_t38_packet_conv_info->reass_start_seqnum == -1)) {
/* we use the first fragment's frame_number as fragment ID because the protocol doesn't provide it */
/* XXX: We'd be better off assigning our own IDs using a one-up
* counter, if it's possible for more than one reassembly to
* begin in the same frame.
*/
p_t38_conv_info->reass_ID = actx->pinfo->num;
p_t38_conv_info->reass_start_seqnum = seq_number;
p_t38_conv_info->time_first_t4_data = nstime_to_sec(&actx->pinfo->rel_ts);
p_t38_conv_info->additional_hdlc_data_field_counter = 0;
p_t38_packet_conv_info->reass_ID = p_t38_conv_info->reass_ID;
p_t38_packet_conv_info->reass_start_seqnum = p_t38_conv_info->reass_start_seqnum;
p_t38_packet_conv_info->reass_start_data_field = Data_Field_item_num;
p_t38_packet_conv_info->seqnum_prev_data_field = p_t38_conv_info->seqnum_prev_data_field;
p_t38_packet_conv_info->additional_hdlc_data_field_counter = p_t38_conv_info->additional_hdlc_data_field_counter;
p_t38_packet_conv_info->time_first_t4_data = p_t38_conv_info->time_first_t4_data;
}
if (seq_number == (guint32)p_t38_packet_conv_info->seqnum_prev_data_field){
p_t38_packet_conv_info->additional_hdlc_data_field_counter ++;
if(p_t38_conv){
p_t38_conv_info->additional_hdlc_data_field_counter = p_t38_packet_conv_info->additional_hdlc_data_field_counter;
p_t38_conv_info->additional_hdlc_data_field_counter++;
}
}
guint32 frag_seq_num;
if (seq_number == (guint32)p_t38_packet_conv_info->reass_start_seqnum) {
frag_seq_num = (guint32)p_t38_packet_conv_info->additional_hdlc_data_field_counter + Data_Field_item_num - (guint32)p_t38_packet_conv_info->reass_start_data_field;
} else {
frag_seq_num = seq_number - (guint32)p_t38_packet_conv_info->reass_start_seqnum + (guint32)p_t38_packet_conv_info->additional_hdlc_data_field_counter + Data_Field_item_num;
}
frag_msg = fragment_add_seq(&data_reassembly_table,
value_tvb, 0,
actx->pinfo,
p_t38_packet_conv_info->reass_ID, /* ID for fragments belonging together */
NULL,
seq_number - (guint32)p_t38_packet_conv_info->reass_start_seqnum + (guint32)p_t38_packet_conv_info->additional_hdlc_data_field_counter, /* fragment sequence number */
frag_seq_num, /* fragment sequence number */
value_len, /* fragment length */
TRUE, /* More fragments */
0);
@ -696,9 +713,7 @@ dissect_t38_T_field_data(tvbuff_t *tvb _U_, int offset _U_, asn1_ctx_t *actx _U_
if (!frag_msg) { /* Not last packet of reassembled */
if (Data_Field_field_type_value == 0) {
col_append_fstr(actx->pinfo->cinfo, COL_INFO," (HDLC fragment %u)",
seq_number + (guint32)p_t38_packet_conv_info->additional_hdlc_data_field_counter
- (guint32)p_t38_packet_conv_info->reass_start_seqnum);
col_append_fstr(actx->pinfo->cinfo, COL_INFO," (HDLC fragment %u)", frag_seq_num);
} else {
col_append_fstr(actx->pinfo->cinfo, COL_INFO," (t4-data fragment %u)", seq_number - (guint32)p_t38_packet_conv_info->reass_start_seqnum);
}
@ -985,6 +1000,7 @@ init_t38_info_conv(packet_info *pinfo)
p_t38_conv->src_t38_info.time_first_t4_data = 0;
p_t38_conv->src_t38_info.additional_hdlc_data_field_counter = 0;
p_t38_conv->src_t38_info.seqnum_prev_data_field = -1;
p_t38_conv->src_t38_info.next = NULL;
p_t38_conv->dst_t38_info.reass_ID = 0;
p_t38_conv->dst_t38_info.reass_start_seqnum = -1;
@ -995,6 +1011,7 @@ init_t38_info_conv(packet_info *pinfo)
p_t38_conv->dst_t38_info.time_first_t4_data = 0;
p_t38_conv->dst_t38_info.additional_hdlc_data_field_counter = 0;
p_t38_conv->dst_t38_info.seqnum_prev_data_field = -1;
p_t38_conv->dst_t38_info.next = NULL;
conversation_add_proto_data(p_conv, proto_t38, p_t38_conv);
}

View File

@ -40,11 +40,13 @@ typedef struct _t38_packet_info {
/* Info to save the State to reassemble Data (e.g. HDLC) and the Setup (e.g. SDP) in T38 conversations */
typedef struct _t38_conv_info
{
typedef struct _t38_conv_info t38_conv_info;
struct _t38_conv_info {
guint32 reass_ID;
int reass_start_seqnum;
guint32 reass_start_data_field;
guint32 reass_data_type;
gint32 last_seqnum; /* used to avoid duplicated seq num shown in the Graph Analysis */
guint32 packet_lost;
@ -52,8 +54,9 @@ typedef struct _t38_conv_info
double time_first_t4_data;
guint32 additional_hdlc_data_field_counter;
gint32 seqnum_prev_data_field;
t38_conv_info *next;
} t38_conv_info;
};
/* Info to save the State to reassemble Data (e.g. HDLC) and the Setup (e.g. SDP) in T38 conversations */
typedef struct _t38_conv