Fix bug 9021: 'RTP not decoded inside the conversation in v.1.10.1'
The behavior for SIP/SDP handling of RTP conversation tracking changed in v1.10, with some unintended consequences. The bugs did not show up at the time because wireshark makes 2 passes of the packet list, and so the problems auto-corrected themselves in most cases. Unfortunately, a change in r53641 modified how UDP behaves, making it always create conversations for UDP packets, and that exposed the bugs inherent in the SIP/SDP code changes. This commit reverts the behavior of SIP/SDP to its pre-1.10 model, but creates a new preference setting for "Delay SDP changes for tracking media", which if enabled, will turn on the new (but buggy) model introduced in 1.10. This preference is *disabled* by default, since for a majority of cases the new behavior is worse than the previous behavior. The preference, and this commit's fix, is not intended to last long. I intend to re-write the SIP/SDP/RTP interaction model for release 1.11 - I think it's too big a change for 1.10, however, which is why I submitted this commit. Change-Id: Ic5601749d6c2344e952ced8206dd9296bfdc4b90 Reviewed-on: https://code.wireshark.org/review/543 Reviewed-by: Evan Huus <eapache@gmail.com>
This commit is contained in:
parent
70ff7be1e6
commit
c4c8350284
|
@ -1684,7 +1684,8 @@ convert_disposable_media(transport_info_t* transport_info, disposable_media_info
|
|||
}
|
||||
|
||||
void
|
||||
setup_sdp_transport(tvbuff_t *tvb, packet_info *pinfo, enum sdp_exchange_type exchange_type, int request_frame)
|
||||
setup_sdp_transport(tvbuff_t *tvb, packet_info *pinfo, enum sdp_exchange_type exchange_type,
|
||||
int request_frame, const gboolean delay)
|
||||
{
|
||||
gint offset = 0, next_offset, n;
|
||||
int linelen;
|
||||
|
@ -1736,6 +1737,18 @@ setup_sdp_transport(tvbuff_t *tvb, packet_info *pinfo, enum sdp_exchange_type ex
|
|||
if (transport_info->media_count > 0)
|
||||
start_transport_info_count = transport_info->media_count;
|
||||
|
||||
/* if we don't delay, and this is an answer after a previous offer, then
|
||||
we free'd the unused media rtp_dyn_payload last time while processing
|
||||
the offer, so we need to re-create them this time in case we need them.
|
||||
If they don't get used they'll get free'd again later */
|
||||
if (!delay && (exchange_type == SDP_EXCHANGE_ANSWER_ACCEPT) &&
|
||||
(transport_info->sdp_status == SDP_EXCHANGE_OFFER)) {
|
||||
for (n = start_transport_info_count; n < SDP_MAX_RTP_CHANNELS; n++) {
|
||||
if (!transport_info->media[n].rtp_dyn_payload)
|
||||
transport_info->media[n].rtp_dyn_payload = g_hash_table_new(g_int_hash, g_int_equal);
|
||||
}
|
||||
}
|
||||
|
||||
/*
|
||||
* Show the SDP message a line at a time.
|
||||
*/
|
||||
|
@ -1815,8 +1828,8 @@ setup_sdp_transport(tvbuff_t *tvb, packet_info *pinfo, enum sdp_exchange_type ex
|
|||
convert_disposable_media(transport_info, &media_info, start_transport_info_count);
|
||||
|
||||
/* We have a successful negotiation, apply data to their respective protocols */
|
||||
if ((exchange_type == SDP_EXCHANGE_ANSWER_ACCEPT) &&
|
||||
(transport_info->sdp_status == SDP_EXCHANGE_OFFER)) {
|
||||
if (!delay || ((exchange_type == SDP_EXCHANGE_ANSWER_ACCEPT) &&
|
||||
(transport_info->sdp_status == SDP_EXCHANGE_OFFER))) {
|
||||
|
||||
for (n = 0; n <= transport_info->media_count; n++) {
|
||||
guint32 current_rtp_port = 0;
|
||||
|
@ -1920,7 +1933,7 @@ setup_sdp_transport(tvbuff_t *tvb, packet_info *pinfo, enum sdp_exchange_type ex
|
|||
}
|
||||
}
|
||||
}
|
||||
transport_info->sdp_status = SDP_EXCHANGE_ANSWER_ACCEPT;
|
||||
transport_info->sdp_status = exchange_type;
|
||||
|
||||
} else if ((exchange_type == SDP_EXCHANGE_ANSWER_REJECT) &&
|
||||
(transport_info->sdp_status != SDP_EXCHANGE_ANSWER_REJECT)) {
|
||||
|
|
|
@ -35,7 +35,7 @@ enum sdp_exchange_type
|
|||
SDP_EXCHANGE_ANSWER_REJECT
|
||||
};
|
||||
|
||||
extern void setup_sdp_transport(tvbuff_t *tvb, packet_info *pinfo, enum sdp_exchange_type type, int request_frame);
|
||||
extern void setup_sdp_transport(tvbuff_t *tvb, packet_info *pinfo, enum sdp_exchange_type type, int request_frame, const gboolean delay);
|
||||
/* Handles duplicate OFFER packets so they don't end up processed by dissect_sdp(). This can probably
|
||||
* be removed when all higher layer dissectors properly handle SDP themselves with setup_sdp_transport()
|
||||
*/
|
||||
|
|
|
@ -57,7 +57,6 @@
|
|||
|
||||
#include "packet-sdp.h" /* SDP needs a transport layer to determine request/response */
|
||||
|
||||
|
||||
#define TCP_PORT_SIP 5060
|
||||
#define UDP_PORT_SIP 5060
|
||||
#define TLS_PORT_SIP 5061
|
||||
|
@ -811,6 +810,9 @@ static gboolean sip_desegment_body = TRUE;
|
|||
*/
|
||||
static gboolean sip_retrans_the_same_sport = TRUE;
|
||||
|
||||
/* whether we hold off tracking RTP conversations until an SDP answer is received */
|
||||
static gboolean sip_delay_sdp_changes = FALSE;
|
||||
|
||||
/* Extension header subdissectors */
|
||||
static dissector_table_t ext_hdr_subdissector_table;
|
||||
|
||||
|
@ -3506,15 +3508,15 @@ dissect_sip_common(tvbuff_t *tvb, int offset, packet_info *pinfo, proto_tree *tr
|
|||
/* Resends don't count */
|
||||
if (resend_for_packet == 0) {
|
||||
if (line_type == REQUEST_LINE) {
|
||||
setup_sdp_transport(next_tvb, pinfo, SDP_EXCHANGE_OFFER, pinfo->fd->num);
|
||||
setup_sdp_transport(next_tvb, pinfo, SDP_EXCHANGE_OFFER, pinfo->fd->num, sip_delay_sdp_changes);
|
||||
} else if (line_type == STATUS_LINE) {
|
||||
if (stat_info->response_code >= 400) {
|
||||
/* SIP client request failed, so SDP offer should fail */
|
||||
setup_sdp_transport(next_tvb, pinfo, SDP_EXCHANGE_ANSWER_REJECT, request_for_response);
|
||||
setup_sdp_transport(next_tvb, pinfo, SDP_EXCHANGE_ANSWER_REJECT, request_for_response, sip_delay_sdp_changes);
|
||||
}
|
||||
else if ((stat_info->response_code >= 200) && (stat_info->response_code <= 299)) {
|
||||
/* SIP success request, so SDP offer should be accepted */
|
||||
setup_sdp_transport(next_tvb, pinfo, SDP_EXCHANGE_ANSWER_ACCEPT, request_for_response);
|
||||
setup_sdp_transport(next_tvb, pinfo, SDP_EXCHANGE_ANSWER_ACCEPT, request_for_response, sip_delay_sdp_changes);
|
||||
}
|
||||
}
|
||||
} else {
|
||||
|
@ -5576,6 +5578,13 @@ void proto_register_sip(void)
|
|||
"Retransmissions always use the same source port",
|
||||
"Whether retransmissions are detected coming from the same source port only.",
|
||||
&sip_retrans_the_same_sport);
|
||||
prefs_register_bool_preference(sip_module, "delay_sdp_changes",
|
||||
"Delay SDP changes for tracking media",
|
||||
"Whether SIP should delay tracking the media (e.g., RTP/RTCP) until an SDP offer "
|
||||
"is answered. If enabled, mid-dialog changes to SDP and media state only take "
|
||||
"effect if and when an SDP offer is successfully answered; however enabling this "
|
||||
"prevents tracking media in early-media call scenarios",
|
||||
&sip_delay_sdp_changes);
|
||||
|
||||
prefs_register_obsolete_preference(sip_module, "tcp.port");
|
||||
|
||||
|
|
Loading…
Reference in New Issue