#514 drouting: improper handling for address in dr_gateways

1.8.x
closed-fixed
modules (454)
7
2012-05-05
2012-04-19
Anonymous
No

The documentation specify that the address field in dr_gateways table is a SIP URI.
When a SIP URI is populated into the table, drouting fails to load.

The first thing that needs to be fixed is creating the URI that needs to be parsed in routing.c:add_dst():
@@ -491,8 +491,9 @@
pgw->type = type;

/* add address in the list */
- if(pgw->ip_str.len<5 || (strncasecmp("sip:", ip, 4)
- && strncasecmp("sips:", ip, 5)))
+ if(pgw->ip_str.len<5 ||
+ (strncasecmp("sip:",ip,4) != 0 &&
+ strncasecmp("sips:",ip,5) != 0))
{
if(pgw->ip_str.len+4>=GWABUF_MAX_SIZE) {
LM_ERR("GW address (%d) longer "

But after that, the code still expects an IP:port in pgw->ip_str (but we have a SIP URI).
When the new destination URI is created, the resulting SIP URI is bogus because the host part of the new URI is startin with "sip:"

drouting:push_gw_for_usage: adding gw [blah] as "sip:username@sip:host" in order 0

Regards,
Ovidiu Sas

Discussion

  • Ovidiu Sas
    Ovidiu Sas
    2012-04-19

    The URI parsing part is fine (the patch is a noop).
    The only issue is storing the full SIP URI as a host:port in the structure pointed by pgw .

    Regards,
    Ovidiu Sas

     
    • priority: 5 --> 7
    • assigned_to: nobody --> bogdan_iancu
    • status: open --> open-accepted
     
  • Hi Ovidiu,

    Actually the docs should be updated - the GWs are to be defined only as IP:port format, to be consistent in the entire code.

    Regards,
    Bogdan

     
  • Ovidiu Sas
    Ovidiu Sas
    2012-04-20

    In this case proper validation of the address field should be imposed.
    Maybe socket_info.h:parse_phostport() should be broken in two by creating a parse_hostport() to validate only host[:port] strings and integrate parse_hostport() into parse_phostport().

    Regards,
    Ovidiu Sas

     
    • status: open-accepted --> open-fixed
     
  • Hi Ovidiu,

    Forget what I said, I made the fix in such a way that is works now correctly as before : the GW address can be with or without the "sip" prefix.

    The fix is on SVN (trunk, 1.8, 1.7), please test it and confirm.

    Regards,
    Bogdan

     
  • It is working ok now (but there are some other issues - see bellow). I would like to propose to make the URI validation as the preferred method of populating the address field. We should keep for now the old style, but we should mark it as deprecated.

    Also, if I populate the address field with a URI with params, those params are taken into consideration (which is good).

    Now, since we have a SIP URI, a user is allowed. If I populate a user in the URI, the user will be used to send out the call, resulting an invalid URI:
    Example:
    set gw address to "sip:blah@192.168.2.30;transport=tcp"
    The mi interface reports:
    ID:: gw_t1_id30 IP=blah@192.168.2.30;transport=tcp Enabled=yes
    and the call goes out to:
    sip:[orig_uri]@blah@192.168.2.30;transport=tcp
    We have two '@' inside destination URI.

    A quick fix would be to reject/ignore URIs with a username.

    Also, the URI can have sip or sips URI scheme. Is this information taken into consideration when the request is constructed and sent out?
    I didn't get a chance to validate this.

    Regards,
    Ovidiu Sas

     
  • OK, I see your point. I will fix this.

    Regards,
    Bogdan

     
  • An extra fix is available on SVN - please test and confirm.

    Regards,
    Bogdan

     
  • Ovidiu Sas
    Ovidiu Sas
    2012-05-05

    It is working ok now, the username is properly ignored.
    This issue can be closed.

    Regards,
    Ovidiu Sas

     
    • status: open-fixed --> closed-fixed