[Asterisk-code-review] pjsip / hep: Provide correct local address for Websockets. (asterisk[13])

Jenkins2 asteriskteam at digium.com
Thu Nov 16 10:57:29 CST 2017


Jenkins2 has submitted this change and it was merged. ( https://gerrit.asterisk.org/7223 )

Change subject: pjsip / hep: Provide correct local address for Websockets.
......................................................................

pjsip / hep: Provide correct local address for Websockets.

Previously for PJSIP the local address of WebSocket connections
was set to the remote address. For logging purposes this is
not particularly useful.

The WebSocket API has been extended to allow the local
address to be queried and this is used in PJSIP to set the
local address to the correct value.

The PJSIP HEP support has also been tweaked so that reliable
transports always use the local address on the transport
and do not try to (wrongly) guess. As they are connection
based it is impossible for the source to be anything else.

ASTERISK-26758
ASTERISK-27363

Change-Id: Icd305fd038ad755e2682ab2786e381f6bf29e8ca
---
M include/asterisk/http_websocket.h
M res/res_hep_pjsip.c
M res/res_http_websocket.c
M res/res_pjsip_transport_websocket.c
4 files changed, 98 insertions(+), 63 deletions(-)

Approvals:
  George Joseph: Looks good to me, but someone else must approve
  Kevin Harwell: Looks good to me, approved
  Jenkins2: Approved for Submit



diff --git a/include/asterisk/http_websocket.h b/include/asterisk/http_websocket.h
index d8e7d06..83d5d4f 100644
--- a/include/asterisk/http_websocket.h
+++ b/include/asterisk/http_websocket.h
@@ -344,6 +344,15 @@
 AST_OPTIONAL_API(struct ast_sockaddr *, ast_websocket_remote_address, (struct ast_websocket *session), {return NULL;});
 
 /*!
+ * \brief Get the local address for a WebSocket connection session.
+ *
+ * \retval ast_sockaddr Local address
+ *
+ * \since 13.19.0
+ */
+AST_OPTIONAL_API(struct ast_sockaddr *, ast_websocket_local_address, (struct ast_websocket *session), {return NULL;});
+
+/*!
  * \brief Get whether the WebSocket session is using a secure transport or not.
  *
  * \retval 0 if unsecure
diff --git a/res/res_hep_pjsip.c b/res/res_hep_pjsip.c
index 1614b43..1d99fcc 100644
--- a/res/res_hep_pjsip.c
+++ b/res/res_hep_pjsip.c
@@ -91,35 +91,44 @@
 	pjsip_cid_hdr *cid_hdr;
 	pjsip_from_hdr *from_hdr;
 	pjsip_to_hdr *to_hdr;
-	pjsip_tpmgr_fla2_param prm;
 
 	capture_info = hepv3_create_capture_info(tdata->buf.start, (size_t)(tdata->buf.cur - tdata->buf.start));
 	if (!capture_info) {
 		return PJ_SUCCESS;
 	}
 
-	/* Attempt to determine what IP address will we send this packet out of */
-	pjsip_tpmgr_fla2_param_default(&prm);
-	prm.tp_type = tdata->tp_info.transport->key.type;
-	pj_strset2(&prm.dst_host, tdata->tp_info.dst_name);
-	prm.local_if = PJ_TRUE;
+	if (!(tdata->tp_info.transport->flag & PJSIP_TRANSPORT_RELIABLE)) {
+		pjsip_tpmgr_fla2_param prm;
 
-	/* If we can't get the local address use what we have already */
-	if (pjsip_tpmgr_find_local_addr2(pjsip_endpt_get_tpmgr(ast_sip_get_pjsip_endpoint()), tdata->pool, &prm) != PJ_SUCCESS) {
-		pj_sockaddr_print(&tdata->tp_info.transport->local_addr, local_buf, sizeof(local_buf), 3);
-	} else {
-		if (prm.tp_type & PJSIP_TRANSPORT_IPV6) {
-			snprintf(local_buf, sizeof(local_buf), "[%.*s]:%hu",
-				(int)pj_strlen(&prm.ret_addr),
-				pj_strbuf(&prm.ret_addr),
-				prm.ret_port);
+		/* Attempt to determine what IP address will we send this packet out of */
+		pjsip_tpmgr_fla2_param_default(&prm);
+		prm.tp_type = tdata->tp_info.transport->key.type;
+		pj_strset2(&prm.dst_host, tdata->tp_info.dst_name);
+		prm.local_if = PJ_TRUE;
+
+		/* If we can't get the local address use what we have already */
+		if (pjsip_tpmgr_find_local_addr2(pjsip_endpt_get_tpmgr(ast_sip_get_pjsip_endpoint()), tdata->pool, &prm) != PJ_SUCCESS) {
+			pj_sockaddr_print(&tdata->tp_info.transport->local_addr, local_buf, sizeof(local_buf), 3);
 		} else {
-			snprintf(local_buf, sizeof(local_buf), "%.*s:%hu",
-				(int)pj_strlen(&prm.ret_addr),
-				pj_strbuf(&prm.ret_addr),
-				prm.ret_port);
+			if (prm.tp_type & PJSIP_TRANSPORT_IPV6) {
+				snprintf(local_buf, sizeof(local_buf), "[%.*s]:%hu",
+					(int)pj_strlen(&prm.ret_addr),
+					pj_strbuf(&prm.ret_addr),
+					prm.ret_port);
+			} else {
+				snprintf(local_buf, sizeof(local_buf), "%.*s:%hu",
+					(int)pj_strlen(&prm.ret_addr),
+					pj_strbuf(&prm.ret_addr),
+					prm.ret_port);
+			}
 		}
+	} else {
+		/* For reliable transports they can only ever come from the transport
+		 * local address.
+		 */
+		pj_sockaddr_print(&tdata->tp_info.transport->local_addr, local_buf, sizeof(local_buf), 3);
 	}
+
 	pj_sockaddr_print(&tdata->tp_info.dst_addr, remote_buf, sizeof(remote_buf), 3);
 
 	cid_hdr = PJSIP_MSG_CID_HDR(tdata->msg);
@@ -152,7 +161,6 @@
 	char remote_buf[256];
 	char *uuid;
 	struct hepv3_capture_info *capture_info;
-	pjsip_tpmgr_fla2_param prm;
 
 	capture_info = hepv3_create_capture_info(&rdata->pkt_info.packet, rdata->pkt_info.len);
 	if (!capture_info) {
@@ -164,27 +172,33 @@
 	}
 	pj_sockaddr_print(&rdata->pkt_info.src_addr, remote_buf, sizeof(remote_buf), 3);
 
-	/* Attempt to determine what IP address we probably received this packet on */
-	pjsip_tpmgr_fla2_param_default(&prm);
-	prm.tp_type = rdata->tp_info.transport->key.type;
-	pj_strset2(&prm.dst_host, rdata->pkt_info.src_name);
-	prm.local_if = PJ_TRUE;
+	if (!(rdata->tp_info.transport->flag & PJSIP_TRANSPORT_RELIABLE)) {
+		pjsip_tpmgr_fla2_param prm;
 
-	/* If we can't get the local address use what we have already */
-	if (pjsip_tpmgr_find_local_addr2(pjsip_endpt_get_tpmgr(ast_sip_get_pjsip_endpoint()), rdata->tp_info.pool, &prm) != PJ_SUCCESS) {
-		pj_sockaddr_print(&rdata->tp_info.transport->local_addr, local_buf, sizeof(local_buf), 3);
-	} else {
-		if (prm.tp_type & PJSIP_TRANSPORT_IPV6) {
-			snprintf(local_buf, sizeof(local_buf), "[%.*s]:%hu",
-				(int)pj_strlen(&prm.ret_addr),
-				pj_strbuf(&prm.ret_addr),
-				prm.ret_port);
+		/* Attempt to determine what IP address we probably received this packet on */
+		pjsip_tpmgr_fla2_param_default(&prm);
+		prm.tp_type = rdata->tp_info.transport->key.type;
+		pj_strset2(&prm.dst_host, rdata->pkt_info.src_name);
+		prm.local_if = PJ_TRUE;
+
+		/* If we can't get the local address use what we have already */
+		if (pjsip_tpmgr_find_local_addr2(pjsip_endpt_get_tpmgr(ast_sip_get_pjsip_endpoint()), rdata->tp_info.pool, &prm) != PJ_SUCCESS) {
+			pj_sockaddr_print(&rdata->tp_info.transport->local_addr, local_buf, sizeof(local_buf), 3);
 		} else {
-			snprintf(local_buf, sizeof(local_buf), "%.*s:%hu",
-				(int)pj_strlen(&prm.ret_addr),
-				pj_strbuf(&prm.ret_addr),
-				prm.ret_port);
+			if (prm.tp_type & PJSIP_TRANSPORT_IPV6) {
+				snprintf(local_buf, sizeof(local_buf), "[%.*s]:%hu",
+					(int)pj_strlen(&prm.ret_addr),
+					pj_strbuf(&prm.ret_addr),
+					prm.ret_port);
+			} else {
+				snprintf(local_buf, sizeof(local_buf), "%.*s:%hu",
+					(int)pj_strlen(&prm.ret_addr),
+					pj_strbuf(&prm.ret_addr),
+					prm.ret_port);
+			}
 		}
+	} else {
+		pj_sockaddr_print(&rdata->tp_info.transport->local_addr, local_buf, sizeof(local_buf), 3);
 	}
 
 	uuid = assign_uuid(&rdata->msg_info.cid->id, &rdata->msg_info.to->tag, &rdata->msg_info.from->tag);
diff --git a/res/res_http_websocket.c b/res/res_http_websocket.c
index 2baccc0..75a6eba 100644
--- a/res/res_http_websocket.c
+++ b/res/res_http_websocket.c
@@ -87,18 +87,19 @@
 
 /*! \brief Structure definition for session */
 struct ast_websocket {
-	FILE *f;                          /*!< Pointer to the file instance used for writing and reading */
-	int fd;                           /*!< File descriptor for the session, only used for polling */
-	struct ast_sockaddr address;      /*!< Address of the remote client */
-	enum ast_websocket_opcode opcode; /*!< Cached opcode for multi-frame messages */
-	size_t payload_len;               /*!< Length of the payload */
-	char *payload;                    /*!< Pointer to the payload */
-	size_t reconstruct;               /*!< Number of bytes before a reconstructed payload will be returned and a new one started */
-	int timeout;                      /*!< The timeout for operations on the socket */
-	unsigned int secure:1;            /*!< Bit to indicate that the transport is secure */
-	unsigned int closing:1;           /*!< Bit to indicate that the session is in the process of being closed */
-	unsigned int close_sent:1;        /*!< Bit to indicate that the session close opcode has been sent and no further data will be sent */
-	struct websocket_client *client;  /*!< Client object when connected as a client websocket */
+	FILE *f;                            /*!< Pointer to the file instance used for writing and reading */
+	int fd;                             /*!< File descriptor for the session, only used for polling */
+	struct ast_sockaddr remote_address; /*!< Address of the remote client */
+	struct ast_sockaddr local_address;  /*!< Our local address */
+	enum ast_websocket_opcode opcode;   /*!< Cached opcode for multi-frame messages */
+	size_t payload_len;                 /*!< Length of the payload */
+	char *payload;                      /*!< Pointer to the payload */
+	size_t reconstruct;                 /*!< Number of bytes before a reconstructed payload will be returned and a new one started */
+	int timeout;                        /*!< The timeout for operations on the socket */
+	unsigned int secure:1;              /*!< Bit to indicate that the transport is secure */
+	unsigned int closing:1;             /*!< Bit to indicate that the session is in the process of being closed */
+	unsigned int close_sent:1;          /*!< Bit to indicate that the session close opcode has been sent and no further data will be sent */
+	struct websocket_client *client;    /*!< Client object when connected as a client websocket */
 };
 
 /*! \brief Hashing function for protocols */
@@ -183,7 +184,7 @@
 		if (session->f) {
 			fclose(session->f);
 			ast_verb(2, "WebSocket connection %s '%s' closed\n", session->client ? "to" : "from",
-				ast_sockaddr_stringify(&session->address));
+				ast_sockaddr_stringify(&session->remote_address));
 		}
 	}
 
@@ -316,7 +317,7 @@
 		fclose(session->f);
 		session->f = NULL;
 		ast_verb(2, "WebSocket connection %s '%s' forcefully closed due to fatal write error\n",
-			session->client ? "to" : "from", ast_sockaddr_stringify(&session->address));
+			session->client ? "to" : "from", ast_sockaddr_stringify(&session->remote_address));
 	}
 
 	ao2_unlock(session);
@@ -429,7 +430,12 @@
 
 struct ast_sockaddr * AST_OPTIONAL_API_NAME(ast_websocket_remote_address)(struct ast_websocket *session)
 {
-	return &session->address;
+	return &session->remote_address;
+}
+
+struct ast_sockaddr * AST_OPTIONAL_API_NAME(ast_websocket_local_address)(struct ast_websocket *session)
+{
+	return &session->local_address;
 }
 
 int AST_OPTIONAL_API_NAME(ast_websocket_is_secure)(struct ast_websocket *session)
@@ -890,12 +896,22 @@
 		return 0;
 	}
 
+	/* Get our local address for the connected socket */
+	if (ast_getsockname(ser->fd, &session->local_address)) {
+		ast_log(LOG_WARNING, "WebSocket connection from '%s' could not be accepted - failed to get local address\n",
+			ast_sockaddr_stringify(&ser->remote_address));
+		websocket_bad_request(ser);
+		ao2_ref(session, -1);
+		ao2_ref(protocol_handler, -1);
+		return 0;
+	}
+
 	ast_verb(2, "WebSocket connection from '%s' for protocol '%s' accepted using version '%d'\n", ast_sockaddr_stringify(&ser->remote_address), protocol ? : "", version);
 
 	/* Populate the session with all the needed details */
 	session->f = ser->f;
 	session->fd = ser->fd;
-	ast_sockaddr_copy(&session->address, &ser->remote_address);
+	ast_sockaddr_copy(&session->remote_address, &ser->remote_address);
 	session->opcode = -1;
 	session->reconstruct = DEFAULT_RECONSTRUCTION_CEILING;
 	session->secure = ser->ssl ? 1 : 0;
@@ -1350,7 +1366,7 @@
 	ws->f = ws->client->ser->f;
 	ws->fd = ws->client->ser->fd;
 	ws->secure = ws->client->ser->ssl ? 1 : 0;
-	ast_sockaddr_copy(&ws->address, &ws->client->ser->remote_address);
+	ast_sockaddr_copy(&ws->remote_address, &ws->client->ser->remote_address);
 	return WS_OK;
 }
 
diff --git a/res/res_pjsip_transport_websocket.c b/res/res_pjsip_transport_websocket.c
index c04594f..b1f560c 100644
--- a/res/res_pjsip_transport_websocket.c
+++ b/res/res_pjsip_transport_websocket.c
@@ -206,20 +206,16 @@
 	pj_sockaddr_parse(pj_AF_UNSPEC(), 0, pj_cstr(&buf, ws_addr_str), &newtransport->transport.key.rem_addr);
 	if (newtransport->transport.key.rem_addr.addr.sa_family == pj_AF_INET6()) {
 		newtransport->transport.key.type = transport_type_wss_ipv6;
-		newtransport->transport.local_name.host.ptr = (char *)pj_pool_alloc(pool, PJ_INET6_ADDRSTRLEN);
-		pj_sockaddr_print(&newtransport->transport.key.rem_addr, newtransport->transport.local_name.host.ptr, PJ_INET6_ADDRSTRLEN, 0);
 	} else {
 		newtransport->transport.key.type = transport_type_wss;
-		newtransport->transport.local_name.host.ptr = (char *)pj_pool_alloc(pool, PJ_INET_ADDRSTRLEN);
-		pj_sockaddr_print(&newtransport->transport.key.rem_addr, newtransport->transport.local_name.host.ptr, PJ_INET_ADDRSTRLEN, 0);
 	}
 
 	newtransport->transport.addr_len = pj_sockaddr_get_len(&newtransport->transport.key.rem_addr);
 
-	pj_sockaddr_cp(&newtransport->transport.local_addr, &newtransport->transport.key.rem_addr);
-
-	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);
+	ws_addr_str = ast_sockaddr_stringify(ast_websocket_local_address(newtransport->ws_session));
+	pj_sockaddr_parse(pj_AF_UNSPEC(), 0, pj_cstr(&buf, ws_addr_str), &newtransport->transport.local_addr);
+	pj_strdup2(pool, &newtransport->transport.local_name.host, ast_sockaddr_stringify_host(ast_websocket_local_address(newtransport->ws_session)));
+	newtransport->transport.local_name.port = ast_sockaddr_port(ast_websocket_local_address(newtransport->ws_session));
 
 	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);

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

Gerrit-Project: asterisk
Gerrit-Branch: 13
Gerrit-MessageType: merged
Gerrit-Change-Id: Icd305fd038ad755e2682ab2786e381f6bf29e8ca
Gerrit-Change-Number: 7223
Gerrit-PatchSet: 1
Gerrit-Owner: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Jenkins2
Gerrit-Reviewer: Kevin Harwell <kharwell at digium.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20171116/af266467/attachment-0001.html>


More information about the asterisk-code-review mailing list