[Kamailio-Devel] [ openser-Bugs-2416621 ] [nat_traversal] Should match source and *local* IP:port

[SourceForge.net]

Bugs item #2416621, was opened at 2008-12-11 10:59
Message generated for change (Comment added) made by henningw
You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=743020&aid=2416621&group_id=139143

Please note that this message will contain a full copy of the comment thread,
including the initial issue submission, for this request,
not just the latest update.
Category: modules
Group: ver devel
>Status: Closed
>Resolution: Fixed
Priority: 5
Private: No
Submitted By: Iñaki Baz Castillo (ibc_sf)
>Assigned to: Henning Westerholt (henningw)
Summary: [nat_traversal] Should match source and *local* IP:port

Initial Comment:
I've realized of a limitation in nat_traversal keepalive mechanism. Note the following case:
 
- Kamailio listening in two address: ADDR1, ADDR2 (can be different IP and/or port).
 
- A multi-account phone (Twinkle) that uses the same internal port for all the accounts (5060).
 
- The phone behind NAT.
 
- Phone account 1: uses ADDR1 as outbound proxy / registrar.
 
- Phone account 2: uses ADDR2 as outbound proxy / registrar.
 
 
When account 1 is registered nat_traversal creates a keepalive:
 
 sip:PHONE_PUBLIC_ADDR udp:ADDR1
 
When the register from account 2 arrives nat_traversal *doesn't* create a  new keepalive since the source address (IP and port) matches an existing one, but this is a bug since account 2 is using ADDR2 as socket, so when Kamailio tries to route a request to account 2 it will fail since there is NO keepalive between Kamailio ADDR2 and PHONE_PUBLIC_ADDR.
 
 
I assume the problem is in the function "HashTable_search":
 
--------
static NAT_Contact*
HashTable_search(HashTable *table, char *uri, unsigned slot)
{
 NAT_Contact *contact;
 
 contact = table->slots[slot].head;
 
 while (contact) {
   if (NAT_Contact_match(contact, uri))
     break;
   contact = contact->next;
 }
 
 return contact;
}
--------
 
When it's called from "keepalive_registration" it just creates a new keepalive if a contact wan't found:
 
--------
static void
keepalive_registration(struct sip_msg *request, time_t expire)
{
 NAT_Contact *contact;
 unsigned h;
 char *uri;
 
 uri = get_source_uri(request);
 
 h = HASH(nat_table, uri);
 lock_get(&nat_table->slots[h].lock);
 
 contact = HashTable_search(nat_table, uri, h);
 if (contact) {
   SIP_Registration_update(contact, expire);
 } else {
   contact = NAT_Contact_new(uri,request->rcv.bind_address);
   if (contact) {
     SIP_Registration_update(contact, expire);
     contact->next = nat_table->slots[h].head;
     nat_table->slots[h].head = contact;
   } else {
     LM_ERR("cannot allocate shared memory for new NAT contact\n");
   }
 }
 
 lock_release(&nat_table->slots[h].lock);
}
--------
 
 
So IMHO the function "HashTable_search" should consider the client contact and *also* the local socket used to communicate with the client.

----------------------------------------------------------------------

>Comment By: Henning Westerholt (henningw)
Date: 2009-04-16 12:13

Message:
Hi Inaki, 

thanks for the confirmation. I've ported the respective change to 1.5
branch and (git) master. 

Henning

----------------------------------------------------------------------

Comment By: Iñaki Baz Castillo (ibc_sf)
Date: 2009-04-14 18:45

Message:
Hi Henningw, sure it's the same report since I did that report in OpenSIPS
:)

----------------------------------------------------------------------

Comment By: Henning Westerholt (henningw)
Date: 2009-04-14 15:31

Message:
Hi Inaki,

is this perhaps identical to the issue that Dan fixed in opensips:

r5357 | dan_pascu | 2009-02-25 10:44:29 +0100 (Mi, 25 Feb 2009) | 1 line
Do not fix contact, only if both IP and port are the same

If yes, then i'll port this to our tree as well.

----------------------------------------------------------------------

You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=743020&aid=2416621&group_id=139143