[OpenSIPS-Users] force_tcp_alias default behavior seems wrong – should not use port from Via-header

[Jonas Borjesson]

Hi Bogdan,

Just wanted to check in again to see what you thought about this
topic. I will most likely go ahead and fix it according to my
suggested solution #1 on the branch I have since this is a serious
issue for us. Any alternative approaches and/or suggestions are
greatly appreciated. If you decide that any of the two approaches are
desirable for others as well, I will create a bug and issue a pull
request with a fix so others can benefit as well.

Thanks,

/Jonas

On Thu, Sep 24, 2015 at 7:52 AM, Jonas Borjesson <jonas at twilio.com> wrote:
> Hi,
>
> The exact details is as follows:
>
> The client will establish a TLS connection, which is terminated by
> opensips. The client will then send an OPTIONS and from the 200
> response will learn about its external, to opensips, IP and port
> through the received and rport from the Via-header. It will then issue
> a REGISTER using the same connection and populate that REGISTER with
> the information it learned from the OPTIONS. I agree that this is not
> really per any spec but in general, this is really the very same as
> issuing an initial REGISTER (which it will get wrong), examine the Via
> in that response and re-register with the correct Contact based off of
> that info. Now, this particular client has currently two issues, which
> I'm having them fix:
>
> 1. They “forgot” to use the rport value in the Via from the response
> to the OPTIONS as the port in their Contact of the REGISTER. Hence,
> they essentially will register with Contact: <public_ip>:5061 but this
> can be easily fixed by opensips fix_nated_contact and, I'm arguing, is
> a separate issue from connection management.
>
> 2. The client doesn't end up using the rport in the new Via, which
> then effectively becomes 5061 and that is in fact how I found this
> issue. In general though, I think the Via in this case actually
> shouldn't be the external facing IP and port but rather the actual
> private ip and port of where the connection is “attached” to on
> Alice's client since because of “symmetric response routing” this is
> not an issue (the use of rport etc) and in general, you can't really
> trust the information in there anyway (which is why we have received
> and rport in the first place).
>
> However, even though the client has some issues I think that the
> connection management of opensips is a separate problem but of course,
> if the Client gets the Contact wrong, and it isn't being fixed by
> opensips through e.g. fix_nated_contact then the “key” used to lookup
> the connection for any future requests going to that client will of
> course be wrong and the request will fail. Anyway, I believe that
> since we are dealing with connections, the key for that connection
> must be calculated off of the information representing that
> connection, which a layer 7 protocol, such as SIP, does not. Hence,
> they key should be local IP + local port + remote IP + remote port +
> transport and this is irrespective of what it says in the SIP message
> itself. Based on that philosophy, I changed the force_tcp_alias to
> grab the port from the packet instead (port=msg->rcv.src_port) and
> that will correctly store the connection under the correct key, which
> then truly represents that connection and this will address the issue.
> Again though, the Contact of the REGISTER must of course match that
> but at least now, all connections will be stored correctly in the
> internal map of opensips.
>
> Does that make sense?
>
> /Jonas
>
>
> On Thu, Sep 24, 2015 at 12:51 AM, Bogdan-Andrei Iancu
> <bogdan at opensips.org> wrote:
>> Hi Jonas,
>>
>> A quick question first about the Contact hdr used by Alice and Bob devices
>> during registration. From your explanation, I understand that STUN correctly
>> discover the IP address of the NAT, but you haven't mentioned anything on
>> the port. So, I guess the contact populated in the Register is a mixing of
>> the public NAT address and the port of the local box. Is this correct ?
>>
>> I'm asking, as before exploring the issue about the "force_tcp_alias"
>> (which, I agree, has some problems), I want to understand how the end-user
>> (bob or alice) are to be addressed on the OpenSIPS proxy - like INVITE to
>> sip:bob at NAT_IP:local_port ?
>>
>> Regards,
>>
>> Bogdan-Andrei Iancu
>> OpenSIPS Founder and Developer
>> http://www.opensips-solutions.com
>>
>>
>> On 24.09.2015 03:34, Jonas Borjesson wrote:
>>>
>>> Hi all,
>>>
>>> I have the following problem (sorry for somewhat long explanation,
>>> want to get it right on the first try :-):
>>>
>>> * Alice is behind a NAT and registers with whatever.com and has
>>> through STUN figured out her public ip, which will go into the Contact
>>> of the REGISTER.
>>> * Bob is behind the same NAT as Alice and registers with whatever.com
>>> and has through STUN figured out his public ip, which will go into the
>>> Contact of the REGISTER.
>>> * Carol is another user that calls bob at whatever.com.
>>> * opensips is acting as a pure transaction stateful proxy for all SIP
>>> traffic (including REGISTER so I'm not using opensips as a registrar)
>>> * opensips config is using force_tcp_alias() so that the connection
>>> can be re-used at a later point.
>>> * Alice's and Bob's clients are sending keep-alive traffic (double
>>> crlf) to keep the connection up.
>>>
>>> In the above scenario, because of the way force_tcp_alias works, Bob
>>> will NOT get the call but rather Alice for the following reason:
>>>
>>> When Alice's client registers and the force_tcp_alias is executed, a
>>> mapping between Alice's public ip + the port found in the top-most
>>> Via-header and her incoming TCP/TLS connection will be created. When
>>> Bob registers, which he does after Alice, he too will create mapping
>>> between his connection and the public_ip + port in Via. Of course,
>>> since Alice and Bob are behind the same NAT they will have the same
>>> IP, hence “half” the key is shared between Alice & Bob at all times.
>>> If Alice & Bob also puts the same port as each other in the top-most
>>> Via-header, they now share the exact same key for the connection
>>> whereby opensips will complain with the following message:
>>> “tcpconn_add_alias: possible port hijack attempt”. Hence, when Carol
>>> later on calls Bob, which then will be “resolved” to
>>> bob at public_ip:some_port (by a location aware proxy behind opensips)
>>> and subsequently proxied to Bob via the opensips node, it will find a
>>> live connection and re-use that, which ends up at Alice.
>>>
>>> So, looking at the code (action.c):
>>>
>>> case FORCE_TCP_ALIAS_T:
>>>      …
>>>      if (a->elem[0].type==NOSUBTYPE) {
>>>          port=msg->via1->port;
>>>      ...
>>>
>>> which clearly grabs the port out of the Via. By doing this there is a
>>> chance that clients will happen to have the same local port and you
>>> will run into the scenario above. Even worse, for those clients that
>>> do not set the port on the Via to the actual port of the connection
>>> (which clients do) they will end up with the default for the
>>> transport, which in my case was 5061 for TLS. Malicious users behind
>>> the same corporate NAT could take advantage of this by setting up many
>>> connections to effectively “steal” other peoples phone calls, granted,
>>> they may have to setup several thousands of connections to be sure so
>>> perhaps not practical.
>>>
>>> So, suggested solutions:
>>>
>>> Suggestion 1: I do not see any value with using the port from the via
>>> but rather always use the port from the src packet itself since that
>>> is what you really want anyway. That will avoid everything above.
>>>
>>> Suggestion 2: Allow for script variables to force_tcp_alias so you
>>> could pass whatever you want, which in my case always would be the
>>> source port of the incoming packet. Currently, the config-file grammar
>>> only allows for number but could be easily extended to allow for other
>>> types as well.
>>>
>>> Both solutions are fairly trivial where solution 1 seems to be the
>>> correct default behavior but solution 2 has the most flexibility and
>>> also wouldn't mess with any existing deployments in the wild, even
>>> though I'm guessing they suffer from the same problem as described but
>>> may not have been discovered yet.
>>>
>>> Comments/thoughts? If people agree, I will issue a pull request
>>> against latest 1.11. Also, the behavior is the same for at least
>>> versions 1.8 and 1.11. I am assuming it’s also the same for all
>>> versions in between as well, and possibly earlier versions but I
>>> haven't checked.
>>>
>>> Thanks,
>>>
>>> /Jonas
>>>
>>> _______________________________________________
>>> Users mailing list
>>> Users at lists.opensips.org
>>> http://lists.opensips.org/cgi-bin/mailman/listinfo/users
>>
>>