[Asterisk-code-review] res pjsip/chan sip: Advertise 'ws' in the SIP URI transport ... (asterisk[13])

[Joshua Colp]

Joshua Colp has submitted this change and it was merged. ( https://gerrit.asterisk.org/4509 )

Change subject: res_pjsip/chan_sip: Advertise 'ws' in the SIP URI transport parameter
......................................................................


res_pjsip/chan_sip: Advertise 'ws' in the SIP URI transport parameter

Per RFC 7118 5.2, the SIP URI 'transport' parameter should advertise
'ws' when WebSockets are to be used as the transport. This applies to
both secure and insecure WebSockets.

There were two bugs in Asterisk with respect to this:

(1) The most egregious occurs in res_pjsip. There, we advertise 'ws' for
    insecure websockets and 'wss' for secure websockets. While this
    would seem to make sense - since 'WS' and 'WSS' are used for the Via
    Transport parameter - this is not the case for the SIP URI. This
    patch corrects that by registering the secure websockets with
    pjproject using the shorthand 'WS', and by returning 'ws' when asked
    for the transport parameter. Note that in pjproject, it is perfectly
    valid to have multiple transports use the same shorthand.

(2) In chan_sip, we return an upper-case version of the transport 'WS'
    instead of 'ws'. Since we should be strict in what we send and
    liberal in what we accept (within reason), this patch lower-cases
    the transport before appending it to the parameter.

ASTERISK-24330 #close
Reported by: cervajs, Inaki Baz Castillo

Change-Id: Iff77b645f8cc3b7cd35168a6676c26b147f22f42
---
M channels/chan_sip.c
M res/res_pjsip_transport_websocket.c
2 files changed, 4 insertions(+), 4 deletions(-)

Approvals:
  George Joseph: Looks good to me, approved
  Anonymous Coward #1000019: Verified
  Joshua Colp: Looks good to me, but someone else must approve



diff --git a/channels/chan_sip.c b/channels/chan_sip.c
index b661f0d..5fbca42 100644
--- a/channels/chan_sip.c
+++ b/channels/chan_sip.c
@@ -14138,6 +14138,7 @@
 	char tmp[SIPBUFSIZE];
 	char *user = ast_uri_encode(p->exten, tmp, sizeof(tmp), ast_uri_sip_user);
 	int use_sips;
+	char *transport = ast_strdupa(sip_get_transport(p->socket.type));
 
 	if (incoming) {
 		use_sips = uas_sips_contact(req);
@@ -14152,7 +14153,7 @@
 	} else {
 		ast_string_field_build(p, our_contact, "<%s:%s%s%s;transport=%s>",
 			use_sips ? "sips" : "sip", user, ast_strlen_zero(user) ? "" : "@",
-			ast_sockaddr_stringify_remote(&p->ourip), sip_get_transport(p->socket.type));
+			ast_sockaddr_stringify_remote(&p->ourip), ast_str_to_lower(transport));
 	}
 }
 
diff --git a/res/res_pjsip_transport_websocket.c b/res/res_pjsip_transport_websocket.c
index 4181828..82ade56 100644
--- a/res/res_pjsip_transport_websocket.c
+++ b/res/res_pjsip_transport_websocket.c
@@ -378,7 +378,6 @@
 static pj_bool_t websocket_on_rx_msg(pjsip_rx_data *rdata)
 {
 	static const pj_str_t STR_WS = { "ws", 2 };
-	static const pj_str_t STR_WSS = { "wss", 3 };
 	pjsip_contact_hdr *contact;
 
 	long type = rdata->tp_info.transport->key.type;
@@ -395,7 +394,7 @@
 		uri->port = rdata->pkt_info.src_port;
 		ast_debug(4, "Re-wrote Contact URI host/port to %.*s:%d\n",
 			(int)pj_strlen(&uri->host), pj_strbuf(&uri->host), uri->port);
-		pj_strdup(rdata->tp_info.pool, &uri->transport_param, (type == (long)transport_type_ws) ? &STR_WS : &STR_WSS);
+		pj_strdup(rdata->tp_info.pool, &uri->transport_param, &STR_WS);
 	}
 
 	rdata->msg_info.via->rport_param = 0;
@@ -431,7 +430,7 @@
 	CHECK_PJSIP_MODULE_LOADED();
 
 	pjsip_transport_register_type(PJSIP_TRANSPORT_RELIABLE, "WS", 5060, &transport_type_ws);
-	pjsip_transport_register_type(PJSIP_TRANSPORT_RELIABLE | PJSIP_TRANSPORT_SECURE, "WSS", 5060, &transport_type_wss);
+	pjsip_transport_register_type(PJSIP_TRANSPORT_RELIABLE | PJSIP_TRANSPORT_SECURE, "WS", 5060, &transport_type_wss);
 
 	if (ast_sip_register_service(&websocket_module) != PJ_SUCCESS) {
 		return AST_MODULE_LOAD_DECLINE;

-- 
To view, visit https://gerrit.asterisk.org/4509
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: Iff77b645f8cc3b7cd35168a6676c26b147f22f42
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: 13
Gerrit-Owner: Matt Jordan <mjordan at digium.com>
Gerrit-Reviewer: Anonymous Coward #1000019
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>