[Asterisk-code-review] res_pjsip_nat: Restore original contact for REGISTER responses (...asterisk[13])

[Alexei Gradinari]

Alexei Gradinari has posted comments on this change. ( https://gerrit.asterisk.org/c/asterisk/+/12824 )

Change subject: res_pjsip_nat: Restore original contact for REGISTER responses
......................................................................


Patch Set 4:

> Patch Set 4:
> 
> > Patch Set 4:
> > 
> > > Patch Set 4:
> > > 
> > > > Patch Set 4:
> > > > 
> > > > > Patch Set 4:
> > > > > 
> > > > > It's worse, it writes asterisk's ip-address to contact host
> > > > This happens when set external_signaling_address on transport.
> > > 
> > > Are you sure? It's working for me and I do have external_signalling_address set on the transport.  The code takes exactly what's in the incoming contact header.
> > 
> > 
> > Sure.
> > I even placed a debug before and after
> > pj_strdup2(tdata->pool, &uri->host, ast_sockaddr_stringify_host(&transport_state->external_signaling_address));
> > to check it.
> > 
> > If set external_signaling_address
> > res_pjsip_nat.c: Saving contact 'endpoint-private-ip:65160'
> > res_pjsip_nat.c: got contact endpoint-public-ip:65160
> > res_pjsip_nat.c: Found orig host: endpoint-private-ip:65160
> > res_pjsip_nat.c: Before Contact URI host:port endpoint-private-ip:65160
> > res_pjsip_nat.c: After Contact URI host:port external_signaling_address:65160
> > 
> > SIP 200 REGISTER
> > Contact: <sip:endpoint at external_signaling_address:65160>;expires=179
> > 
> > If not set external_signaling_address
> > res_pjsip_nat.c: Saving contact 'endpoint-private-ip:65160'
> > res_pjsip_nat.c: got contact endpoint-public-ip:65160
> > res_pjsip_nat.c: Found orig host: endpoint-private-ip:65160
> > 
> > SIP 200 REGISTER
> > Contact: <sip:endpoint at endpoint-private-ip:65160>;expires=179
> 
> Are endpoint-private-ip and endpoint-public-ip the addresses of the device or the pbx?  Are you testing from the same machine as the pbx?  I think you discovered a completely separate bug.  If you look further down in nat_on_tx_message we're setting the Contact host to the external_signalling_address but we should only be doing that for outgoing requests, not responses.


"should only be doing that for outgoing requests", but because
.on_tx_request = nat_on_tx_message,
.on_tx_response = nat_on_tx_message,
we use the same function for request and response

this bug exists with or without your patch.
But it's related to this issue "the contact host and port sent
back in the 200 response will be not the one sent by the user agent".
So I think it should be fixed in this patch


-- 
To view, visit https://gerrit.asterisk.org/c/asterisk/+/12824
To unsubscribe, or for help writing mail filters, visit https://gerrit.asterisk.org/settings

Gerrit-Project: asterisk
Gerrit-Branch: 13
Gerrit-Change-Id: Idc263ad2d2d7bd8faa047e5804d96a5fe1cd282e
Gerrit-Change-Number: 12824
Gerrit-PatchSet: 4
Gerrit-Owner: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Alexei Gradinari <alex2grad at gmail.com>
Gerrit-Reviewer: Friendly Automation
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Kevin Harwell <kharwell at digium.com>
Gerrit-Comment-Date: Fri, 13 Sep 2019 18:39:30 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20190913/2e4e68ad/attachment.html>