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

[Jonas Borjesson]

Hey,

Thanks for you reply. Just some additional notes I perhaps should have
included. How the client arrives to which Contact address it has isn't
the main issue here, opensips can fix that easily (e.g.
fix_nated_address) and in fact, these clients don't actually use STUN
per se, they use the information in the Via of the response coming
back (recieved & rport) to "fix" their contact and will re-register.
However, the scenario I describe will still happen for clients behind
the very same NAT due to how the force_tcp_alias works and currently
it will grab the port off of the via, which, as you also argues, is
unsafe and you should rely on information from the received packet
itself. So, it sounds like we are arguing pretty much the same thing,
i.e., don't trust the client :-)

So, your statement

"Two end-points behind same NAT will always be different from the
server point of view as there will be different ports opened in the
NAT-ed router. Starting with wrong assumptions like the ones above
one can easily end up in the wrong places to look for solutions."

is correct except that, again, force_tcp_alias will not allow you to
map those two different connections to two different entries within
opensips. Does that make sense?

/Jonas


On Wed, Sep 23, 2015 at 6:05 PM, Adrian Georgescu <ag at ag-projects.com> wrote:
>
>> On 23 Sep 2015, at 21:34, Jonas Borjesson <jonas at twilio.com> 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.
>
> This is unreliable, you should never use STUN for this purpose. A well configured server will never use the information presented by an end-point in the Contact header of a Register, it will use the actual transport and port of the received packet instead.
>
>> * 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.
>
> Again, this is unreliable, you should never use STUN for this purpose. OpenSIPS is smart enough to determine where the end-point registered from instead of trusting the information in the Contact header which can be wrong, fake or anything in between.
>
> Two end-points behind same NAT will always be different from the server point of view as there will be different ports opened in the NAT-ed router. Starting with wrong assumptions like the ones above  one can easily end up in the wrong places to look for solutions.
>
> Below is for others to comment.
>
>> * 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
>
>
> _______________________________________________
> Users mailing list
> Users at lists.opensips.org
> http://lists.opensips.org/cgi-bin/mailman/listinfo/users