Summary:	ASTERISK-29222: chan_sip: Hold/Resume an sRTP call on a video enabled user-agent.
Reporter:	Alexander Traud (traud)	Labels:	patch
Date Opened:	2020-12-23 08:45:08.000-0600	Date Closed:	2021-01-13 07:44:28.000-0600
Priority:	Major	Regression?
Status:	Closed/Complete	Components:	Channels/chan_sip/SRTP
Versions:	16.15.0 18.1.0	Frequency of Occurrence
Related Issues:
Environment:		Attachments:	( 0) sipura_video_hold_resume.patch
Description:	On resume, the call ends. This happens likely when # the resuming callee has more media streams (like audio and video) than the caller (like audio-only) and # SDES-sRTP was negotiated between Asterisk and the callee. *Steps to Reproduce* are based on the Call Hold example from [RFC 5393 section 2.1|http://tools.ietf.org/html/rfc5359#section-2.1] with three additional constrains: Alice, (1) with her audio-only phone, calls Bob, with his (2) audio/video-enabled phone. Actually, Bob uses a Cisco IP Phone 8865 with Multiplatform Firmware (MPP, former Sipura). Asterisk is configured (3) with {code}directmedia=false videosupport=yes{code}in the configuration file {{sip.conf}}. In this call example, the problem originates after F16. *Expected Result* The call resumes with F19 from Asterisk: SIP status OK. *Actual Results* Asterisk ends the call with SIP status 488. Asterisk prints the warning ‘Rejecting secure video stream without encryption details’. With debug level 3, Asterisk prints ‘Received offer with crypto line for media stream that is not enabled’. *Fix* Although the call was established without video, because Alice did not offer video, Bob’s Cisco offers video while resuming. This is allowed and even the expected behavior as of [RFC 6337 section 5.3|https://tools.ietf.org/html/rfc6337#section-5.3]. However, within Asterisk, video was disabled for this session. Consequently, {{process_crypto(.)}} returns early because {{rtp}} is null. However, the following checks do not check whether {{p->vrtp}} is null. The attached patch adds that check. However, the call scenario showed that the very same check is missing some code lines as well. Now, after checking {{p->vsrtp}} twice, the call continues (with SDES-sSRTP, with audio, but without video). For symmetry reasons, I added the same two checks for sessions without audio. *Notes* - This is about SDES-sRTP. I did not check DTLS-sRTP and whether this is affected and/or fixed, too. - As per RFC, Asterisk has to offer the newly added video to Alice. However, this does not happen because chan_sip cannot add a new media stream mid-call, see ASTERISK-26637. Consequently, the video stays disabled even after resuming the call.
Comments:	By: Asterisk Team (asteriskteam) 2020-12-23 08:45:09.706-0600 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. Please note that log messages and other files should not be sent to the Sangoma Asterisk Team unless explicitly asked for. All files should be placed on this issue in a sanitized fashion as needed. 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. Please note that by submitting data, code, or documentation to Sangoma through JIRA, you accept the Terms of Use present at [https://www.asterisk.org/terms-of-use/|https://www.asterisk.org/terms-of-use/]. By: Alexander Traud (traud) 2021-01-09 11:18:58.428-0600 The proposed change, the attached patch, failed code review because one of the test cases of the Asterisk Test Suite failed: [tests/channels/SIP/codec_negotiation/sipp/decline_crypto.xml|https://github.com/asterisk/testsuite/commit/296ccddfc9c92438eb6ac24be9dbc4eaac8ea5d6]. The test case expects the SIP status 488 but got status 603 now. Some facts about that test case: * is from the same author as the [blamed source code|https://github.com/asterisk/asterisk/commit/32472eca7058a03703d09cf58bf2ce1ff901c42a] * includes just a single media stream, video; no audio * uses an extension configured with videosupport=no * the formats of the SDP offer and that extension match because the default of Asterisk is to allow H.263 (RTP [Payload Type 34|https://www.iana.org/assignments/rtp-parameters], see channels/chan_sip.c:[sip_set_default_format_capabilities|https://github.com/asterisk/asterisk/commit/817fc344e7fa2eb61f91742f80f83416262c9754]) * the module res_srtp.so is not loaded * calls the extension guest2 which maps to the dialplan {{exten => _.,1,NoOp()}} which returns SIP status 603 Consequently, the test case expected to get a SIP status 488 # because Asterisk declines the offered media *profile* "RTP/SAVP" # because the module res_srtp.so was not loaded. However, because the test case includes no audio but just a video stream and video is disabled, the test case might as well get a SIP status 488 because Asterisk declines the offered media *stream* "video" and therefore declines the whole dialog because no media stream is left. Tests revealed that Asterisk does not accept any media stream in that offer but still continues with the dialog. Asterisk returns a SIP status 200 with an extension which Answer() and a SIP status 603 with an extension with NoOp(). Consequently, the issue here, its correct change, and that valid test case revealed another issue: ASTERISK-29238. That, in turn, revealed another issue: ASTERISK-29237. The proposed change for the latter fixes this issue here because the state of p->vsrtp is checked earlier already. Therefore, the attached patch is not needed anymore. By: Friendly Automation (friendly-automation) 2021-01-13 07:44:28.624-0600 Change 15271 merged by Friendly Automation: chan_sip: SDP: Sidestep stream parsing when its media is disabled. [https://gerrit.asterisk.org/c/asterisk/+/15271|https://gerrit.asterisk.org/c/asterisk/+/15271] By: Friendly Automation (friendly-automation) 2021-01-13 08:31:31.270-0600 Change 15233 merged by George Joseph: chan_sip: SDP: Sidestep stream parsing when its media is disabled. [https://gerrit.asterisk.org/c/asterisk/+/15233|https://gerrit.asterisk.org/c/asterisk/+/15233] By: Friendly Automation (friendly-automation) 2021-01-13 08:31:44.008-0600 Change 15272 merged by George Joseph: chan_sip: SDP: Sidestep stream parsing when its media is disabled. [https://gerrit.asterisk.org/c/asterisk/+/15272|https://gerrit.asterisk.org/c/asterisk/+/15272]