RFC3261 8.1.1.2 states that To header "may or may not be the ultimate recipient of the request", and 8.1.1.3 states that From header is "possibly the user's address-of-record". So either of them cannot be fully trusted.
This was already noted and fixed partially in commit 1ef4587612806a94 ("modules/sca: reconcile Contact and From URIs in ACK callback").
But similar issues happen when using ENUM and/or calling using local alias (numbers only, or local forward) and the destination is in another SIP domain. To-header contains the initial domain, and not the AoR domain. When handling response to such INVITE a sane way to determine correct AoR is to inspect the Call-Info header's Application Server URI.
Thus this changes AoR canonicalization to happens as follow: 1. User portion is taken from Contact header if present, otherwise from the applicable From / To header. 2. Domain/port part is taken from Call-Info header if present, and otherwise from the applicable From / To header. --- This has not fully tested, but this seems to be the root case why SCA does not on certain inter-domain calls that I get. I'd like to get review comments, and possible other fix ideas. I'll let you know how the testing goes, and if looks OK to you can push this to master and relevant stable branches.
modules/sca/sca_util.c | 51 +++++++++++++++++++++++++++----------------------- 1 file changed, 28 insertions(+), 23 deletions(-)
diff --git a/modules/sca/sca_util.c b/modules/sca/sca_util.c index 143ac47..e209cc0 100644 --- a/modules/sca/sca_util.c +++ b/modules/sca/sca_util.c @@ -26,6 +26,7 @@ #include <assert.h>
#include "sca_util.h" +#include "sca_call_info.h"
#include "../../parser/sdp/sdp.h"
@@ -343,9 +344,13 @@ sca_aor_create_from_info( str *aor, uri_type type, str *user, str *domain, sca_create_canonical_aor_for_ua( sip_msg_t *msg, str *c_aor, int ua_opts ) { struct to_body *tf = NULL; - sip_uri_t c_uri; - str tf_aor = STR_NULL; + str user_portion; + str domain_portion; + str port_portion = STR_NULL; str contact_uri = STR_NULL; + sip_uri_t c_uri; + sca_call_info call_info; + hdr_field_t *call_info_hdr; int rc = -1;
assert( msg != NULL ); @@ -373,12 +378,6 @@ sca_create_canonical_aor_for_ua( sip_msg_t *msg, str *c_aor, int ua_opts ) } }
- if ( sca_uri_extract_aor( &tf->uri, &tf_aor ) < 0 ) { - LM_ERR( "sca_create_canonical_aor: failed to extract AoR from " - "URI <%.*s>", STR_FMT( &tf->uri )); - goto done; - } - memset( &c_uri, 0, sizeof( sip_uri_t )); if (( rc = sca_get_msg_contact_uri( msg, &contact_uri )) < 0 ) { LM_ERR( "sca_create_canonical_aor: failed to get contact URI from " @@ -394,22 +393,28 @@ sca_create_canonical_aor_for_ua( sip_msg_t *msg, str *c_aor, int ua_opts ) } }
- if ( SCA_STR_EMPTY( &c_uri.user ) || - SCA_STR_EQ( &c_uri.user, &tf->parsed_uri.user )) { - /* empty contact header or Contact user matches To/From AoR */ - c_aor->s = (char *)pkg_malloc( tf_aor.len ); - c_aor->len = tf_aor.len; - memcpy( c_aor->s, tf_aor.s, tf_aor.len ); + /* Prefer Contact header user, fallback to To/From */ + if ( SCA_STR_EMPTY( &c_uri.user ) ) + user_portion = tf->parsed_uri.user; + else + user_portion = c_uri.user; + + memset( &call_info, 0, sizeof( sca_call_info )); + call_info_hdr = sca_call_info_header_find( msg->headers ); + if ( !SCA_HEADER_EMPTY( call_info_hdr ) && + sca_call_info_body_parse( &call_info_hdr->body, &call_info ) >= 0 ) { + /* Call-Info present, use the server name as AoR domain */ + domain_portion = call_info.sca_uri; } else { - /* Contact user and To/From user mismatch */ - if ( sca_aor_create_from_info( c_aor, c_uri.type, - &c_uri.user, &tf->parsed_uri.host, - &tf->parsed_uri.port ) < 0 ) { - LM_ERR( "sca_create_canonical_aor: failed to create AoR from " - "Contact <%.*s> and URI <%.*s>", - STR_FMT( &contact_uri ), STR_FMT( &tf_aor )); - goto done; - } + /* Use To/From domain */ + domain_portion = tf->parsed_uri.host; + port_portion = tf->parsed_uri.port; + } + + if ( sca_aor_create_from_info( c_aor, c_uri.type, &user_portion, &domain_portion, &port_portion ) < 0 ) { + LM_ERR( "sca_create_canonical_aor: failed to create canonical AoR " + "user: <%.*s>, domain: <%.*s>", STR_FMT( &user_portion ), STR_FMT( &domain_portion )); + goto done; }
rc = 1;
Hi Timo. Thanks, this looks good. As long as your testing goes well, I'm happy to see it committed.
andrew
On Oct 11, 2013, at 6:30 AM, Timo Teräs timo.teras@iki.fi wrote:
RFC3261 8.1.1.2 states that To header "may or may not be the ultimate recipient of the request", and 8.1.1.3 states that From header is "possibly the user's address-of-record". So either of them cannot be fully trusted.
This was already noted and fixed partially in commit 1ef4587612806a94 ("modules/sca: reconcile Contact and From URIs in ACK callback").
But similar issues happen when using ENUM and/or calling using local alias (numbers only, or local forward) and the destination is in another SIP domain. To-header contains the initial domain, and not the AoR domain. When handling response to such INVITE a sane way to determine correct AoR is to inspect the Call-Info header's Application Server URI.
Thus this changes AoR canonicalization to happens as follow:
- User portion is taken from Contact header if present, otherwise
from the applicable From / To header. 2. Domain/port part is taken from Call-Info header if present, and otherwise from the applicable From / To header.
This has not fully tested, but this seems to be the root case why SCA does not on certain inter-domain calls that I get. I'd like to get review comments, and possible other fix ideas. I'll let you know how the testing goes, and if looks OK to you can push this to master and relevant stable branches.
modules/sca/sca_util.c | 51 +++++++++++++++++++++++++++----------------------- 1 file changed, 28 insertions(+), 23 deletions(-)
diff --git a/modules/sca/sca_util.c b/modules/sca/sca_util.c index 143ac47..e209cc0 100644 --- a/modules/sca/sca_util.c +++ b/modules/sca/sca_util.c @@ -26,6 +26,7 @@ #include <assert.h>
#include "sca_util.h" +#include "sca_call_info.h"
#include "../../parser/sdp/sdp.h"
@@ -343,9 +344,13 @@ sca_aor_create_from_info( str *aor, uri_type type, str *user, str *domain, sca_create_canonical_aor_for_ua( sip_msg_t *msg, str *c_aor, int ua_opts ) { struct to_body *tf = NULL;
- sip_uri_t c_uri;
- str tf_aor = STR_NULL;
str user_portion;
str domain_portion;
str port_portion = STR_NULL; str contact_uri = STR_NULL;
sip_uri_t c_uri;
sca_call_info call_info;
hdr_field_t *call_info_hdr; int rc = -1;
assert( msg != NULL );
@@ -373,12 +378,6 @@ sca_create_canonical_aor_for_ua( sip_msg_t *msg, str *c_aor, int ua_opts ) } }
- if ( sca_uri_extract_aor( &tf->uri, &tf_aor ) < 0 ) {
- LM_ERR( "sca_create_canonical_aor: failed to extract AoR from "
"URI <%.*s>", STR_FMT( &tf->uri ));
- goto done;
- }
- memset( &c_uri, 0, sizeof( sip_uri_t )); if (( rc = sca_get_msg_contact_uri( msg, &contact_uri )) < 0 ) { LM_ERR( "sca_create_canonical_aor: failed to get contact URI from "
@@ -394,22 +393,28 @@ sca_create_canonical_aor_for_ua( sip_msg_t *msg, str *c_aor, int ua_opts ) } }
- if ( SCA_STR_EMPTY( &c_uri.user ) ||
SCA_STR_EQ( &c_uri.user, &tf->parsed_uri.user )) {
- /* empty contact header or Contact user matches To/From AoR */
- c_aor->s = (char *)pkg_malloc( tf_aor.len );
- c_aor->len = tf_aor.len;
- memcpy( c_aor->s, tf_aor.s, tf_aor.len );
- /* Prefer Contact header user, fallback to To/From */
- if ( SCA_STR_EMPTY( &c_uri.user ) )
- user_portion = tf->parsed_uri.user;
- else
- user_portion = c_uri.user;
- memset( &call_info, 0, sizeof( sca_call_info ));
- call_info_hdr = sca_call_info_header_find( msg->headers );
- if ( !SCA_HEADER_EMPTY( call_info_hdr ) &&
sca_call_info_body_parse( &call_info_hdr->body, &call_info ) >= 0 ) {
- /* Call-Info present, use the server name as AoR domain */
- domain_portion = call_info.sca_uri; } else {
- /* Contact user and To/From user mismatch */
- if ( sca_aor_create_from_info( c_aor, c_uri.type,
&c_uri.user, &tf->parsed_uri.host,
&tf->parsed_uri.port ) < 0 ) {
LM_ERR( "sca_create_canonical_aor: failed to create AoR from "
"Contact <%.*s> and URI <%.*s>",
STR_FMT( &contact_uri ), STR_FMT( &tf_aor ));
goto done;
- }
/* Use To/From domain */
domain_portion = tf->parsed_uri.host;
port_portion = tf->parsed_uri.port;
}
if ( sca_aor_create_from_info( c_aor, c_uri.type, &user_portion, &domain_portion, &port_portion ) < 0 ) {
LM_ERR( "sca_create_canonical_aor: failed to create canonical AoR "
"user: <%.*s>, domain: <%.*s>", STR_FMT( &user_portion ), STR_FMT( &domain_portion ));
goto done; }
rc = 1;
-- 1.8.4
On Fri, 11 Oct 2013 13:30:24 +0300 Timo Teräs timo.teras@iki.fi wrote:
This has not fully tested, but this seems to be the root case why SCA does not on certain inter-domain calls that I get. I'd like to get review comments, and possible other fix ideas. I'll let you know how the testing goes, and if looks OK to you can push this to master and relevant stable branches.
We've done some testing finally, and found out it was basically unusable. The sca_uri was unparsed, but we really want only the host part of it.
The updated patch that I'm currently testing with, is as follows:
From: =?UTF-8?q?Timo=20Ter=C3=A4s?= timo.teras@iki.fi Subject: [PATCH] modules/sca: use Call-Info for canonical AoR
RFC3261 8.1.1.2 states that To header "may or may not be the ultimate recipient of the request", and 8.1.1.3 states that From header is "possibly the user's address-of-record". So either of them cannot be fully trusted.
This was already noted and fixed partially in commit 1ef4587612806a94 ("modules/sca: reconcile Contact and From URIs in ACK callback").
But similar issues happen when using ENUM and/or calling using local alias (numbers only, or local forward) and the destination is in another SIP domain. To-header contains the initial domain, and not the AoR domain. When handling response to such INVITE a sane way to determine correct AoR is to inspect the Call-Info header's Application Server URI.
Thus this changes AoR canonicalization to happens as follow: 1. User portion is taken from Contact header if present, otherwise from the applicable From / To header. 2. Domain/port part is taken from Call-Info header if present, and otherwise from the applicable From / To header.
sca_call_info.sca_uri is updated to exclude '<' and '>' like other structs with uri members so we can call parse_uri directly on it. --- modules/sca/sca_call_info.c | 4 +-- modules/sca/sca_util.c | 67 +++++++++++++++++++++++++++------------------ 2 files changed, 43 insertions(+), 28 deletions(-)
diff --git a/modules/sca/sca_call_info.c b/modules/sca/sca_call_info.c index b6d6b38..4adc859 100644 --- a/modules/sca/sca_call_info.c +++ b/modules/sca/sca_call_info.c @@ -383,8 +383,8 @@ sca_call_info_body_parse( str *hdr_body, sca_call_info *call_info ) return( -1 ); }
- call_info->sca_uri.s = p; - call_info->sca_uri.len = semi - p; + call_info->sca_uri.s = p + 1; + call_info->sca_uri.len = semi - p - 2;
p = semi; p++; diff --git a/modules/sca/sca_util.c b/modules/sca/sca_util.c index 143ac47..378e6c0 100644 --- a/modules/sca/sca_util.c +++ b/modules/sca/sca_util.c @@ -26,6 +26,7 @@ #include <assert.h>
#include "sca_util.h" +#include "sca_call_info.h"
#include "../../parser/sdp/sdp.h"
@@ -343,9 +344,14 @@ sca_aor_create_from_info( str *aor, uri_type type, str *user, str *domain, sca_create_canonical_aor_for_ua( sip_msg_t *msg, str *c_aor, int ua_opts ) { struct to_body *tf = NULL; - sip_uri_t c_uri; - str tf_aor = STR_NULL; + str user_portion; + str domain_portion; + str port_portion = STR_NULL; str contact_uri = STR_NULL; + uri_type uritype; + sip_uri_t puri; + sca_call_info call_info; + hdr_field_t *call_info_hdr; int rc = -1;
assert( msg != NULL ); @@ -373,43 +379,52 @@ sca_create_canonical_aor_for_ua( sip_msg_t *msg, str *c_aor, int ua_opts ) } }
- if ( sca_uri_extract_aor( &tf->uri, &tf_aor ) < 0 ) { - LM_ERR( "sca_create_canonical_aor: failed to extract AoR from " - "URI <%.*s>", STR_FMT( &tf->uri )); - goto done; - } - - memset( &c_uri, 0, sizeof( sip_uri_t )); - if (( rc = sca_get_msg_contact_uri( msg, &contact_uri )) < 0 ) { + rc = sca_get_msg_contact_uri( msg, &contact_uri ); + if ( rc < 0 ) { LM_ERR( "sca_create_canonical_aor: failed to get contact URI from " "Contact <%.*s>", STR_FMT( &msg->contact->body )); goto done; } + if ( rc > 0 ) { - if ( parse_uri( contact_uri.s, contact_uri.len, &c_uri ) < 0 ) { + /* Prefer Contact header user */ + if ( parse_uri( contact_uri.s, contact_uri.len, &puri ) < 0 ) { LM_ERR( "sca_create_canonical_aor: failed to parse Contact URI " "<%.*s>", STR_FMT( &contact_uri )); rc = -1; goto done; } - } - - if ( SCA_STR_EMPTY( &c_uri.user ) || - SCA_STR_EQ( &c_uri.user, &tf->parsed_uri.user )) { - /* empty contact header or Contact user matches To/From AoR */ - c_aor->s = (char *)pkg_malloc( tf_aor.len ); - c_aor->len = tf_aor.len; - memcpy( c_aor->s, tf_aor.s, tf_aor.len ); + uritype = puri.type; + user_portion = puri.user; } else { - /* Contact user and To/From user mismatch */ - if ( sca_aor_create_from_info( c_aor, c_uri.type, - &c_uri.user, &tf->parsed_uri.host, - &tf->parsed_uri.port ) < 0 ) { - LM_ERR( "sca_create_canonical_aor: failed to create AoR from " - "Contact <%.*s> and URI <%.*s>", - STR_FMT( &contact_uri ), STR_FMT( &tf_aor )); + /* Fallback to To/From user if Contact is not found */ + uritype = tf->parsed_uri.type; + user_portion = tf->parsed_uri.user; + } + + memset( &call_info, 0, sizeof( sca_call_info )); + call_info_hdr = sca_call_info_header_find( msg->headers ); + if ( !SCA_HEADER_EMPTY( call_info_hdr ) && + sca_call_info_body_parse( &call_info_hdr->body, &call_info ) >= 0 ) { + /* Call-Info present, use the server name as AoR domain */ + if ( parse_uri( call_info.sca_uri.s, call_info.sca_uri.len, &puri ) < 0 ) { + LM_ERR( "sca_create_canonical_aor: failed to parse Call-Info URI " + "<%.*s>", STR_FMT( &call_info.sca_uri )); + rc = -1; goto done; } + domain_portion = puri.host; + port_portion = puri.port; + } else { + /* Use To/From domain */ + domain_portion = tf->parsed_uri.host; + port_portion = tf->parsed_uri.port; + } + + if ( sca_aor_create_from_info( c_aor, uritype, &user_portion, &domain_portion, &port_portion ) < 0 ) { + LM_ERR( "sca_create_canonical_aor: failed to create canonical AoR " + "user: <%.*s>, domain: <%.*s>", STR_FMT( &user_portion ), STR_FMT( &domain_portion )); + goto done; }
rc = 1;
On Tue, 22 Apr 2014 10:41:02 +0300 Timo Teras timo.teras@iki.fi wrote:
On Fri, 11 Oct 2013 13:30:24 +0300 Timo Teräs timo.teras@iki.fi wrote:
We've done some testing finally, and found out it was basically unusable. The sca_uri was unparsed, but we really want only the host part of it.
The updated patch that I'm currently testing with, is as follows:
This might have an issue still.
- /* Prefer Contact header user */
- if ( parse_uri( contact_uri.s, contact_uri.len, &puri ) < 0
) { LM_ERR( "sca_create_canonical_aor: failed to parse Contact URI " "<%.*s>", STR_FMT( &contact_uri )); rc = -1; goto done; }
- }
Using Contact unconditionally will be wrong if the function is called with ua_opts that do not match the direction of the message. Is this ever the case? Or is the ua_opts argument redundant? Could we just remove the ua_opts and always use 'automatic' way of figuring out the direction?
- Timo