[Asterisk-code-review] res pjsip WebRTC/websockets: Fix usage of WS vs WSS. (asterisk[13])
Joshua Colp
asteriskteam at digium.com
Wed Mar 1 18:25:33 CST 2017
Joshua Colp has submitted this change and it was merged. ( https://gerrit.asterisk.org/4973 )
Change subject: res_pjsip WebRTC/websockets: Fix usage of WS vs WSS.
......................................................................
res_pjsip WebRTC/websockets: Fix usage of WS vs WSS.
According to the RFC[1] WSS should only be used in the Via header
for secure Websockets.
* Use WSS in Via for secure transport.
* Only register one transport with the WS name because it would be
ambiguous. Outgoing requests may try to find the transport by name and
pjproject only finds the first one registered. This may mess up unsecure
websockets but the impact should be minimal. Firefox and Chrome do not
support anything other than secure websockets anymore.
* Added and updated some debug messages concerning websockets.
* security_events.c: Relax case restriction when determining security
transport type.
* The res_pjsip_nat module has been updated to not touch the transport
on Websocket originating messages.
[1] https://tools.ietf.org/html/rfc7118
ASTERISK-26796 #close
Change-Id: Ie3a0fb1a41101a4c1e49d875a8aa87b189e7ab12
---
M CHANGES
M res/res_pjsip/security_events.c
M res/res_pjsip_nat.c
M res/res_pjsip_transport_websocket.c
4 files changed, 54 insertions(+), 15 deletions(-)
Approvals:
Kevin Harwell: Looks good to me, approved
Richard Mudgett: Looks good to me, but someone else must approve
Anonymous Coward #1000019: Verified
diff --git a/CHANGES b/CHANGES
index 00347f3..77a9af5 100644
--- a/CHANGES
+++ b/CHANGES
@@ -22,6 +22,14 @@
* The 'Comedian Mail' prompts can now be overriden using the 'vm-login' and
'vm-newuser' configuration options in voicemail.conf.
+res_pjsip_transport_websocket
+------------------
+ * Removed non-secure websocket support. Firefox and Chrome have not allowed
+ non-secure websockets for quite some time so this shouldn't be an issue
+ for people. Attempting to use a non-secure websocket may or may not work
+ when Asterisk attempts to send SIP requests to do something like initiate
+ call hangup.
+
------------------------------------------------------------------------------
--- Functionality changes from Asterisk 13.13.0 to Asterisk 13.14.0 ----------
------------------------------------------------------------------------------
diff --git a/res/res_pjsip/security_events.c b/res/res_pjsip/security_events.c
index 5c30e1c..ef3a748 100644
--- a/res/res_pjsip/security_events.c
+++ b/res/res_pjsip/security_events.c
@@ -44,9 +44,9 @@
} else if (rdata->tp_info.transport->key.type == PJSIP_TRANSPORT_TLS ||
rdata->tp_info.transport->key.type == PJSIP_TRANSPORT_TLS6) {
return AST_TRANSPORT_TLS;
- } else if (!strcmp(rdata->tp_info.transport->type_name, "WS")) {
+ } else if (!strcasecmp(rdata->tp_info.transport->type_name, "WS")) {
return AST_TRANSPORT_WS;
- } else if (!strcmp(rdata->tp_info.transport->type_name, "WSS")) {
+ } else if (!strcasecmp(rdata->tp_info.transport->type_name, "WSS")) {
return AST_TRANSPORT_WSS;
} else {
return 0;
diff --git a/res/res_pjsip_nat.c b/res/res_pjsip_nat.c
index 59ef71c..7404ef5 100644
--- a/res/res_pjsip_nat.c
+++ b/res/res_pjsip_nat.c
@@ -35,7 +35,9 @@
static void rewrite_uri(pjsip_rx_data *rdata, pjsip_sip_uri *uri)
{
pj_cstr(&uri->host, rdata->pkt_info.src_name);
- if (strcasecmp("udp", rdata->tp_info.transport->type_name)) {
+ if (!strcasecmp("WSS", rdata->tp_info.transport->type_name)) {
+ /* WSS is special, we don't want to overwrite the URI at all as it needs to be ws */
+ } else if (strcasecmp("udp", rdata->tp_info.transport->type_name)) {
uri->transport_param = pj_str(rdata->tp_info.transport->type_name);
} else {
uri->transport_param.slen = 0;
diff --git a/res/res_pjsip_transport_websocket.c b/res/res_pjsip_transport_websocket.c
index 82ade56..ff8e346 100644
--- a/res/res_pjsip_transport_websocket.c
+++ b/res/res_pjsip_transport_websocket.c
@@ -38,7 +38,6 @@
#include "asterisk/res_pjsip_session.h"
#include "asterisk/taskprocessor.h"
-static int transport_type_ws;
static int transport_type_wss;
/*!
@@ -149,6 +148,7 @@
pjsip_endpoint *endpt = ast_sip_get_pjsip_endpoint();
struct pjsip_tpmgr *tpmgr = pjsip_endpt_get_tpmgr(endpt);
+ char *ws_addr_str;
pj_pool_t *pool;
pj_str_t buf;
pj_status_t status;
@@ -183,9 +183,23 @@
goto on_error;
}
- pj_sockaddr_parse(pj_AF_UNSPEC(), 0, pj_cstr(&buf, ast_sockaddr_stringify(ast_websocket_remote_address(newtransport->ws_session))), &newtransport->transport.key.rem_addr);
+ /*
+ * The type_name here is mostly used by log messages eihter in
+ * pjproject or Asterisk. Other places are reconstituting subscriptions
+ * after a restart (which could never work for a websocket connection anyway),
+ * received MESSAGE requests to set PJSIP_TRANSPORT, and most importantly
+ * by pjproject when generating the Via header.
+ */
+ newtransport->transport.type_name = ast_websocket_is_secure(newtransport->ws_session)
+ ? "WSS" : "WS";
+
+ ws_addr_str = ast_sockaddr_stringify(ast_websocket_remote_address(newtransport->ws_session));
+ ast_debug(4, "Creating websocket transport for %s:%s\n",
+ newtransport->transport.type_name, ws_addr_str);
+
+ pj_sockaddr_parse(pj_AF_UNSPEC(), 0, pj_cstr(&buf, ws_addr_str), &newtransport->transport.key.rem_addr);
newtransport->transport.key.rem_addr.addr.sa_family = pj_AF_INET();
- newtransport->transport.key.type = ast_websocket_is_secure(newtransport->ws_session) ? transport_type_wss : transport_type_ws;
+ newtransport->transport.key.type = transport_type_wss;
newtransport->transport.addr_len = pj_sockaddr_get_len(&newtransport->transport.key.rem_addr);
@@ -196,7 +210,6 @@
newtransport->transport.local_name.host.slen = pj_ansi_strlen(newtransport->transport.local_name.host.ptr);
newtransport->transport.local_name.port = pj_sockaddr_get_port(&newtransport->transport.key.rem_addr);
- newtransport->transport.type_name = (char *)pjsip_transport_get_type_name(newtransport->transport.key.type);
newtransport->transport.flag = pjsip_transport_get_flag_from_type((pjsip_transport_type_e)newtransport->transport.key.type);
newtransport->transport.info = (char *)pj_pool_alloc(newtransport->transport.pool, 64);
@@ -382,19 +395,27 @@
long type = rdata->tp_info.transport->key.type;
- if (type != (long)transport_type_ws && type != (long)transport_type_wss) {
+ if (type != (long) transport_type_wss) {
return PJ_FALSE;
}
- if ((contact = pjsip_msg_find_hdr(rdata->msg_info.msg, PJSIP_H_CONTACT, NULL)) && !contact->star &&
- (PJSIP_URI_SCHEME_IS_SIP(contact->uri) || PJSIP_URI_SCHEME_IS_SIPS(contact->uri))) {
+ contact = pjsip_msg_find_hdr(rdata->msg_info.msg, PJSIP_H_CONTACT, NULL);
+ if (contact
+ && !contact->star
+ && (PJSIP_URI_SCHEME_IS_SIP(contact->uri) || PJSIP_URI_SCHEME_IS_SIPS(contact->uri))) {
pjsip_sip_uri *uri = pjsip_uri_get_uri(contact->uri);
+ const pj_str_t *txp_str = &STR_WS;
+
+ ast_debug(4, "%s re-writing Contact URI from %.*s:%d%s%.*s to %s:%d;transport=%s\n",
+ pjsip_rx_data_get_info(rdata),
+ (int)pj_strlen(&uri->host), pj_strbuf(&uri->host), uri->port,
+ pj_strlen(&uri->transport_param) ? ";transport=" : "",
+ (int)pj_strlen(&uri->transport_param), pj_strbuf(&uri->transport_param),
+ rdata->pkt_info.src_name ?: "", rdata->pkt_info.src_port, pj_strbuf(txp_str));
pj_cstr(&uri->host, rdata->pkt_info.src_name);
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, &STR_WS);
+ pj_strdup(rdata->tp_info.pool, &uri->transport_param, txp_str);
}
rdata->msg_info.via->rport_param = 0;
@@ -429,8 +450,16 @@
{
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, "WS", 5060, &transport_type_wss);
+ /*
+ * We only need one transport type defined. Firefox and Chrome
+ * do not support anything other than secure websockets anymore.
+ *
+ * Also we really cannot have two transports with the same name
+ * because it would be ambiguous. Outgoing requests may try to
+ * find the transport by name and pjproject only finds the first
+ * one registered.
+ */
+ 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/4973
To unsubscribe, visit https://gerrit.asterisk.org/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: Ie3a0fb1a41101a4c1e49d875a8aa87b189e7ab12
Gerrit-PatchSet: 5
Gerrit-Project: asterisk
Gerrit-Branch: 13
Gerrit-Owner: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Anonymous Coward #1000019
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Jørgen H <asterisk.org at hovland.cx>
Gerrit-Reviewer: Kevin Harwell <kharwell at digium.com>
Gerrit-Reviewer: Richard Mudgett <rmudgett at digium.com>
More information about the asterisk-code-review
mailing list