res_pjsip_session/res_pjsip_sdp_rtp: Be more tolerant of offers
authorMatthew Jordan <mjordan@digium.com>
Fri, 17 Oct 2014 13:35:44 +0000 (13:35 +0000)
committerMatthew Jordan <mjordan@digium.com>
Fri, 17 Oct 2014 13:35:44 +0000 (13:35 +0000)
When an inbound SDP offer is received, Asterisk currently makes a few
incorrection assumptions:

(1) If the offer contains more than a single audio/video stream, Asterisk will
    reject the entire stream with a 488. This is an overly strict response;
    generally, Asterisk should accept the media streams that it can accept and
    decline the others.
(2) If the offer contains a declined media stream, Asterisk will attempt to
    process it anyway. This can result in attempting to match format
    capabilities on a declined media stream, leading to a 488. Asterisk should
    simply ignore declined media streams.
(3) Asterisk will currently attempt to handle offers with AVPF with
    use_avpf=No/AVP with use_avpf=Yes. This mismatch results in invalid SDP
    answers being sent in response. If there is a mismatch between the media
    type being offered and the configuration, Asterisk must reject the offer
    with a 488.

This patch does the following:
* Asterisk will accept SDP offers with at least one media stream that it can
  use. Some WARNING messages have been dropped to NOTICEs as a result.
* Asterisk will not accept an offer with a media type that doesn't match its
  configuration.
* Asterisk will ignore declined media streams properly.

#SIPit31

Review: https://reviewboard.asterisk.org/r/4063/

ASTERISK-24122 #close
Reported by: James Van Vleet

ASTERISK-24381 #close
Reported by: Matt Jordan
........

Merged revisions 425868 from http://svn.asterisk.org/svn/asterisk/branches/12
........

Merged revisions 425879 from http://svn.asterisk.org/svn/asterisk/branches/13

git-svn-id: https://origsvn.digium.com/svn/asterisk/trunk@425881 65c4cc65-6c06-0410-ace0-fbb531ad65f3

res/res_pjsip.c
res/res_pjsip_sdp_rtp.c
res/res_pjsip_session.c

index d93cc91..7be8ff8 100644 (file)
                                                decline media offers not using the AVPF or SAVPF profile.
                                        </para><para>
                                                If set to <literal>no</literal>, res_pjsip will use the AVP or SAVP RTP
-                                               profile for all media offers on outbound calls and media updates, but will
-                                               accept either the AVP/AVPF or SAVP/SAVPF RTP profile for all inbound
-                                               media offers.
+                                               profile for all media offers on outbound calls and media updates, and will
+                                               decline media offers not using the AVP or SAVP profile.
                                        </para></description>
                                </configOption>
                                <configOption name="force_avp" default="no">
index 2aa8acc..1f86300 100644 (file)
@@ -247,9 +247,10 @@ static int set_caps(struct ast_sip_session *session, struct ast_sip_session_medi
                struct ast_str *thembuf = ast_str_alloca(64);
 
                ast_rtp_codecs_payloads_destroy(&codecs);
-               ast_log(LOG_WARNING, "No joint capabilities between our configuration(%s) and incoming SDP(%s)\n",
-                       ast_format_cap_get_names(peer, &usbuf),
-                       ast_format_cap_get_names(caps, &thembuf));
+               ast_log(LOG_NOTICE, "No joint capabilities for '%s' media stream between our configuration(%s) and incoming SDP(%s)\n",
+                       session_media->stream_type,
+                       ast_format_cap_get_names(caps, &usbuf),
+                       ast_format_cap_get_names(peer, &thembuf));
                return -1;
        }
 
@@ -521,12 +522,11 @@ static enum ast_sip_session_media_encryption check_endpoint_media_transport(
        const struct pjmedia_sdp_media *stream)
 {
        enum ast_sip_session_media_encryption incoming_encryption;
+       char transport_end = stream->desc.transport.ptr[stream->desc.transport.slen - 1];
 
-       if (endpoint->media.rtp.use_avpf) {
-               char transport_end = stream->desc.transport.ptr[stream->desc.transport.slen - 1];
-               if (transport_end != 'F') {
-                       return AST_SIP_MEDIA_TRANSPORT_INVALID;
-               }
+       if ((transport_end == 'F' && !endpoint->media.rtp.use_avpf)
+               || (transport_end != 'F' && endpoint->media.rtp.use_avpf)) {
+               return AST_SIP_MEDIA_TRANSPORT_INVALID;
        }
 
        incoming_encryption = get_media_encryption_type(stream->desc.transport);
@@ -727,8 +727,15 @@ static int negotiate_incoming_sdp_stream(struct ast_sip_session *session, struct
        RAII_VAR(struct ast_sockaddr *, addrs, NULL, ast_free_ptr);
        enum ast_media_type media_type = stream_to_media_type(session_media->stream_type);
 
+       /* If port is 0, ignore this media stream */
+       if (!stream->desc.port) {
+               ast_debug(3, "Media stream '%s' is already declined\n", session_media->stream_type);
+               return 0;
+       }
+
        /* If no type formats have been configured reject this stream */
        if (!ast_format_cap_has_type(session->endpoint->media.codecs, media_type)) {
+               ast_debug(3, "Endpoint has no codecs for media type '%s', declining stream\n", session_media->stream_type);
                return 0;
        }
 
@@ -760,7 +767,7 @@ static int negotiate_incoming_sdp_stream(struct ast_sip_session *session, struct
        }
 
        if (set_caps(session, session_media, stream)) {
-               return -1;
+               return 0;
        }
        return 1;
 }
@@ -1061,6 +1068,10 @@ static int apply_negotiated_sdp_stream(struct ast_sip_session *session, struct a
                return 1;
        }
 
+       if (!local_stream->desc.port || !remote_stream->desc.port) {
+               return 1;
+       }
+
        /* Ensure incoming transport is compatible with the endpoint's configuration */
        if (!session->endpoint->media.rtp.use_received_transport &&
                check_endpoint_media_transport(session->endpoint, remote_stream) == AST_SIP_MEDIA_TRANSPORT_INVALID) {
@@ -1088,7 +1099,7 @@ static int apply_negotiated_sdp_stream(struct ast_sip_session *session, struct a
        ast_sockaddr_set_port(addrs, remote_stream->desc.port);
        ast_rtp_instance_set_remote_address(session_media->rtp, addrs);
        if (set_caps(session, session_media, local_stream)) {
-               return -1;
+               return 1;
        }
 
        if ((fdno = media_type_to_fdno(media_type)) < 0) {
index 5618721..d5dc98e 100644 (file)
@@ -186,160 +186,10 @@ void ast_sip_session_unregister_sdp_handler(struct ast_sip_session_sdp_handler *
        ao2_callback_data(sdp_handlers, OBJ_KEY | OBJ_UNLINK | OBJ_NODATA, remove_handler, (void *)stream_type, handler);
 }
 
-static int validate_port_hash(const void *obj, int flags)
-{
-       const int *port = obj;
-       return *port;
-}
-
-static int validate_port_cmp(void *obj, void *arg, int flags)
-{
-       int *port1 = obj;
-       int *port2 = arg;
-
-       return *port1 == *port2 ? CMP_MATCH | CMP_STOP : 0;
-}
-
-struct bundle_assoc {
-       int port;
-       char tag[1];
-};
-
-static int bundle_assoc_hash(const void *obj, int flags)
-{
-       const struct bundle_assoc *assoc = obj;
-       const char *tag = flags & OBJ_KEY ? obj : assoc->tag;
-
-       return ast_str_hash(tag);
-}
-
-static int bundle_assoc_cmp(void *obj, void *arg, int flags)
-{
-       struct bundle_assoc *assoc1 = obj;
-       struct bundle_assoc *assoc2 = arg;
-       const char *tag2 = flags & OBJ_KEY ? arg : assoc2->tag;
-
-       return strcmp(assoc1->tag, tag2) ? 0 : CMP_MATCH | CMP_STOP;
-}
-
-/* return must be ast_freed */
-static pjmedia_sdp_attr *media_get_mid(pjmedia_sdp_media *media)
-{
-       pjmedia_sdp_attr *attr = pjmedia_sdp_media_find_attr2(media, "mid", NULL);
-       if (!attr) {
-               return NULL;
-       }
-
-       return attr;
-}
-
-static int get_bundle_port(const pjmedia_sdp_session *sdp, const char *mid)
-{
-       int i;
-       for (i = 0; i < sdp->media_count; ++i) {
-               pjmedia_sdp_attr *mid_attr = media_get_mid(sdp->media[i]);
-               if (mid_attr && !pj_strcmp2(&mid_attr->value, mid)) {
-                       return sdp->media[i]->desc.port;
-               }
-       }
-
-       return -1;
-}
-
-static int validate_incoming_sdp(const pjmedia_sdp_session *sdp)
-{
-       int i;
-       RAII_VAR(struct ao2_container *, portlist, ao2_container_alloc(5, validate_port_hash, validate_port_cmp), ao2_cleanup);
-       RAII_VAR(struct ao2_container *, bundle_assoc_list, ao2_container_alloc(5, bundle_assoc_hash, bundle_assoc_cmp), ao2_cleanup);
-
-       /* check for bundles (for websocket RTP multiplexing, there can be more than one) */
-       for (i = 0; i < sdp->attr_count; ++i) {
-               char *bundle_list;
-               int bundle_port = 0;
-               if (pj_stricmp2(&sdp->attr[i]->name, "group")) {
-                       continue;
-               }
-
-               /* check to see if this group is a bundle */
-               if (7 >= sdp->attr[i]->value.slen || pj_strnicmp2(&sdp->attr[i]->value, "bundle ", 7)) {
-                       continue;
-               }
-
-               bundle_list = ast_alloca(sdp->attr[i]->value.slen - 6);
-               strncpy(bundle_list, sdp->attr[i]->value.ptr + 7, sdp->attr[i]->value.slen - 7);
-               bundle_list[sdp->attr[i]->value.slen - 7] = '\0';
-               while (bundle_list) {
-                       char *item;
-                       RAII_VAR(struct bundle_assoc *, assoc, NULL, ao2_cleanup);
-                       item = strsep(&bundle_list, " ,");
-                       if (!bundle_port) {
-                               RAII_VAR(int *, port, ao2_alloc(sizeof(int), NULL), ao2_cleanup);
-                               RAII_VAR(int *, port_match, NULL, ao2_cleanup);
-                               bundle_port = get_bundle_port(sdp, item);
-                               if (bundle_port < 0) {
-                                       return -1;
-                               }
-                               port_match = ao2_find(portlist, &bundle_port, OBJ_KEY);
-                               if (port_match) {
-                                       /* bundle port aready consumed by a different bundle */
-                                       return -1;
-                               }
-                               *port = bundle_port;
-                               ao2_link(portlist, port);
-                       }
-                       assoc = ao2_alloc(sizeof(*assoc) + strlen(item), NULL);
-                       if (!assoc) {
-                               return -1;
-                       }
-
-                       /* safe use of strcpy */
-                       strcpy(assoc->tag, item);
-                       assoc->port = bundle_port;
-                       ao2_link(bundle_assoc_list, assoc);
-               }
-       }
-
-       /* validate all streams */
-       for (i = 0; i < sdp->media_count; ++i) {
-               RAII_VAR(int *, port, ao2_alloc(sizeof(int), NULL), ao2_cleanup);
-               RAII_VAR(int *, port_match, NULL, ao2_cleanup);
-
-               *port = sdp->media[i]->desc.port;
-               port_match = ao2_find(portlist, port, OBJ_KEY);
-               if (port_match) {
-                       RAII_VAR(struct bundle_assoc *, assoc, NULL, ao2_cleanup);
-                       pjmedia_sdp_attr *mid = media_get_mid(sdp->media[i]);
-                       char *mid_val;
-
-                       if (!mid) {
-                               /* not part of a bundle */
-                               return -1;
-                       }
-
-                       mid_val = ast_alloca(mid->value.slen + 1);
-                       strncpy(mid_val, mid->value.ptr, mid->value.slen);
-                       mid_val[mid->value.slen] = '\0';
-
-                       assoc = ao2_find(bundle_assoc_list, mid_val, OBJ_KEY);
-                       if (!assoc || assoc->port != *port) {
-                               /* This port already exists elsewhere in the SDP
-                                * and is not an appropriate bundle port, fail
-                                * catastrophically */
-                               return -1;
-                       }
-               }
-               ao2_link(portlist, port);
-       }
-       return 0;
-}
-
 static int handle_incoming_sdp(struct ast_sip_session *session, const pjmedia_sdp_session *sdp)
 {
        int i;
-
-       if (validate_incoming_sdp(sdp)) {
-               return -1;
-       }
+       int handled = 0;
 
        for (i = 0; i < sdp->media_count; ++i) {
                /* See if there are registered handlers for this media stream type */
@@ -361,14 +211,22 @@ static int handle_incoming_sdp(struct ast_sip_session *session, const pjmedia_sd
 
                if (session_media->handler) {
                        handler = session_media->handler;
+                       ast_debug(1, "Negotiating incoming SDP media stream '%s' using %s SDP handler\n",
+                               session_media->stream_type,
+                               session_media->handler->id);
                        res = handler->negotiate_incoming_sdp_stream(session, session_media, sdp,
                                sdp->media[i]);
-                       if (res <= 0) {
-                               /* Catastrophic failure or ignored by assigned handler. Abort! */
+                       if (res < 0) {
+                               /* Catastrophic failure. Abort! */
                                return -1;
+                       } else if (res > 0) {
+                               ast_debug(1, "Media stream '%s' handled by %s\n",
+                                       session_media->stream_type,
+                                       session_media->handler->id);
+                               /* Handled by this handler. Move to the next stream */
+                               handled = 1;
+                               continue;
                        }
-                       /* Handled by this handler. Move to the next stream */
-                       continue;
                }
 
                handler_list = ao2_find(sdp_handlers, media, OBJ_KEY);
@@ -377,6 +235,9 @@ static int handle_incoming_sdp(struct ast_sip_session *session, const pjmedia_sd
                        continue;
                }
                AST_LIST_TRAVERSE(&handler_list->list, handler, next) {
+                       ast_debug(1, "Negotiating incoming SDP media stream '%s' using %s SDP handler\n",
+                               session_media->stream_type,
+                               handler->id);
                        res = handler->negotiate_incoming_sdp_stream(session, session_media, sdp,
                                sdp->media[i]);
                        if (res < 0) {
@@ -384,12 +245,19 @@ static int handle_incoming_sdp(struct ast_sip_session *session, const pjmedia_sd
                                return -1;
                        }
                        if (res > 0) {
+                               ast_debug(1, "Media stream '%s' handled by %s\n",
+                                       session_media->stream_type,
+                                       handler->id);
                                /* Handled by this handler. Move to the next stream */
                                session_media->handler = handler;
+                               handled = 1;
                                break;
                        }
                }
        }
+       if (!handled) {
+               return -1;
+       }
        return 0;
 }
 
@@ -429,9 +297,15 @@ static int handle_negotiated_sdp_session_media(void *obj, void *arg, int flags)
 
                handler = session_media->handler;
                if (handler) {
+                       ast_debug(1, "Applying negotiated SDP media stream '%s' using %s SDP handler\n",
+                               session_media->stream_type,
+                               handler->id);
                        res = handler->apply_negotiated_sdp_stream(session, session_media, local,
                                local->media[i], remote, remote->media[i]);
                        if (res >= 0) {
+                               ast_debug(1, "Applied negotiated SDP media stream '%s' using %s SDP handler\n",
+                                       session_media->stream_type,
+                                       handler->id);
                                return CMP_MATCH;
                        }
                        return 0;
@@ -443,6 +317,9 @@ static int handle_negotiated_sdp_session_media(void *obj, void *arg, int flags)
                        continue;
                }
                AST_LIST_TRAVERSE(&handler_list->list, handler, next) {
+                       ast_debug(1, "Applying negotiated SDP media stream '%s' using %s SDP handler\n",
+                               session_media->stream_type,
+                               handler->id);
                        res = handler->apply_negotiated_sdp_stream(session, session_media, local,
                                local->media[i], remote, remote->media[i]);
                        if (res < 0) {
@@ -450,6 +327,9 @@ static int handle_negotiated_sdp_session_media(void *obj, void *arg, int flags)
                                return 0;
                        }
                        if (res > 0) {
+                               ast_debug(1, "Applied negotiated SDP media stream '%s' using %s SDP handler\n",
+                                       session_media->stream_type,
+                                       handler->id);
                                /* Handled by this handler. Move to the next stream */
                                session_media->handler = handler;
                                return CMP_MATCH;
@@ -827,10 +707,6 @@ static int sdp_requires_deferral(struct ast_sip_session *session, const pjmedia_
 {
        int i;
 
-       if (validate_incoming_sdp(sdp)) {
-               return 0;
-       }
-
        for (i = 0; i < sdp->media_count; ++i) {
                /* See if there are registered handlers for this media stream type */
                char media[20];