Summary:	ASTERISK-28973: Malformed IP address in SDP of 2nd SIP timer triggered INVITE when NAT is active (UDP transport with external_media_address)
Reporter:	Michael Neuhauser (mneuhauser)	Labels:	patch
Date Opened:	2020-06-30 10:14:53	Date Closed:	2020-08-10 06:25:20
Priority:	Major	Regression?
Status:	Closed/Complete	Components:	pjproject/pjsip
Versions:	16.11.0	Frequency of Occurrence	Constant
Related Issues:	is duplicated by ASTERISK-29107 IP address in SDP written as hex is related to ASTERISK-27079 PJSIP puts invalid data in SDP when using external_media_address
Environment:		Attachments:	( 0) 0060-clone-sdp-for-sip-timer-refresh-invite.patch
Description:	PJSIP, UDP transport with external_media_address and session timers enabled. Connected to SIP server that is not in local net. Asterisk initiated the connection and is refreshing the session after 150s (timeout 300s). The 2nd refresh-INVITE triggered by the pjsip timer has a malformed IP address in its SDP (garbage string). This only happens when the SDP is modified by the nat-code to replace the local IP address with the configured external_media_address.
Comments:	By: Asterisk Team (asteriskteam) 2020-06-30 10:14:53.921-0500 Thanks for creating a report! The issue has entered the triage process. That means the issue will wait in this status until a Bug Marshal has an opportunity to review the issue. Once the issue has been reviewed you will receive comments regarding the next steps towards resolution. A good first step is for you to review the [Asterisk Issue Guidelines|https://wiki.asterisk.org/wiki/display/AST/Asterisk+Issue+Guidelines] if you haven't already. The guidelines detail what is expected from an Asterisk issue report. Then, if you are submitting a patch, please review the [Patch Contribution Process|https://wiki.asterisk.org/wiki/display/AST/Patch+Contribution+Process]. Please note that once your issue enters an open state it has been accepted. As Asterisk is an open source project there is no guarantee or timeframe on when your issue will be looked into. If you need expedient resolution you will need to find and pay a suitable developer. Asking for an update on your issue will not yield any progress on it and will not result in a response. All updates are posted to the issue when they occur. By: Michael Neuhauser (mneuhauser) 2020-06-30 10:18:45.387-0500 Analysis: the code to modify the SDP (in res_pjsip_session.c:session_outgoing_nat_hook() and also (redundantly?) in res_pjsip_sdp_rtp.c:change_outgoing_sdp_stream_media_address()) uses the tdata->pool to allocate the replacement string. But the *same* pjmedia_sdp_stream that was modified for the 1st refresh-INVITE is also used for the 2nd refresh-INVITE (because it is stored in pjmedia's pjmedia_sdp_neg structure). The problem is, that at that moment, the tdata->pool that holds the stringified external_media_address from the 1. refresh-INVITE has long been reused for something else. Solution: in pjproject/source/pjsip/src/pjsip-ua/sip_timer.c:timer_cb(), modify a cloned copy of the pjmedia_sdp_stream, not the stored original to protect it from changes (as is done in pjproject/source/pjsip/src/pjsip-ua/sip_inv.c when calling pjsip_create_sdp_body()). See attached patch. By: Joshua C. Colp (jcolp) 2020-06-30 10:23:44.154-0500 Are you able to place this change up for review on Gerrit and the PJSIP Github? By: Michael Neuhauser (mneuhauser) 2020-06-30 10:30:33.313-0500 Yes, will do it. By: Michael Neuhauser (mneuhauser) 2020-06-30 14:17:47.408-0500 https://github.com/pjsip/pjproject/issues/2475 By: Michael Neuhauser (mneuhauser) 2020-07-03 09:54:07.049-0500 Gerrit review was updated with "official" fix from pjproject problem report/pull request. By: Friendly Automation (friendly-automation) 2020-08-10 06:25:22.261-0500 Change 14717 merged by Friendly Automation: pjproject: clone sdp to protect against (nat) modifications [https://gerrit.asterisk.org/c/asterisk/+/14717|https://gerrit.asterisk.org/c/asterisk/+/14717] By: Friendly Automation (friendly-automation) 2020-08-10 06:26:27.632-0500 Change 14621 merged by Friendly Automation: pjproject: clone sdp to protect against (nat) modifications [https://gerrit.asterisk.org/c/asterisk/+/14621|https://gerrit.asterisk.org/c/asterisk/+/14621] By: Friendly Automation (friendly-automation) 2020-08-10 06:32:19.915-0500 Change 14718 merged by Friendly Automation: pjproject: clone sdp to protect against (nat) modifications [https://gerrit.asterisk.org/c/asterisk/+/14718|https://gerrit.asterisk.org/c/asterisk/+/14718] By: Friendly Automation (friendly-automation) 2020-08-10 06:34:41.088-0500 Change 14719 merged by Joshua Colp: pjproject: clone sdp to protect against (nat) modifications [https://gerrit.asterisk.org/c/asterisk/+/14719|https://gerrit.asterisk.org/c/asterisk/+/14719] By: Friendly Automation (friendly-automation) 2020-08-10 06:34:56.129-0500 Change 14720 merged by Joshua Colp: pjproject: clone sdp to protect against (nat) modifications [https://gerrit.asterisk.org/c/asterisk/+/14720|https://gerrit.asterisk.org/c/asterisk/+/14720]