[Asterisk-code-review] websocket: Consider pending SSL data when waiting for socket input (asterisk[16])

[Sean Bright]

Sean Bright has uploaded this change for review. ( https://gerrit.asterisk.org/c/asterisk/+/13508 )


Change subject: websocket: Consider pending SSL data when waiting for socket input
......................................................................

websocket: Consider pending SSL data when waiting for socket input

When TLS is in use, checking the readiness of the underlying FD is insufficient
for determining if there is data available to be read. So before polling the
FD, check if there is any buffered data in the TLS layer and use that first.

ASTERISK-28562 #close
Reported by: Robert Sutton

Change-Id: I95fcb3e2004700d5cf8e5ee04943f0115b15e10d
---
M include/asterisk/http_websocket.h
M include/asterisk/iostream.h
M main/iostream.c
M res/res_http_websocket.c
M res/res_pjsip_transport_websocket.c
5 files changed, 51 insertions(+), 4 deletions(-)



  git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/08/13508/1

diff --git a/include/asterisk/http_websocket.h b/include/asterisk/http_websocket.h
index 2180ef4..6fd2d0a 100644
--- a/include/asterisk/http_websocket.h
+++ b/include/asterisk/http_websocket.h
@@ -338,6 +338,20 @@
 AST_OPTIONAL_API(int, ast_websocket_fd, (struct ast_websocket *session), { errno = ENOSYS; return -1;});
 
 /*!
+ * \brief Wait for the WebSocket session to be ready to be read.
+ * \since 16.8.0
+ * \since 17.2.0
+ *
+ * \param session Pointer to the WebSocket session
+ * \param timeout the number of milliseconds to wait
+ *
+ * \retval -1 if error occurred
+ * \retval 0 if the timeout expired
+ * \retval 1 if the WebSocket session is ready for reading
+ */
+AST_OPTIONAL_API(int, ast_websocket_wait_for_input, (struct ast_websocket *session, int timeout), { errno = ENOSYS; return -1; });
+
+/*!
  * \brief Get the remote address for a WebSocket connected session.
  *
  * \retval ast_sockaddr Remote address
diff --git a/include/asterisk/iostream.h b/include/asterisk/iostream.h
index 17376ea..602fefb 100644
--- a/include/asterisk/iostream.h
+++ b/include/asterisk/iostream.h
@@ -127,6 +127,20 @@
 int ast_iostream_get_fd(struct ast_iostream *stream);
 
 /*!
+ * \brief Wait for input on the iostream's file descriptor
+ * \since 16.8.0
+ * \since 17.2.0
+ *
+ * \param stream A pointer to an iostream
+ * \param timeout the number of milliseconds to wait
+ *
+ * \retval -1 if error occurred
+ * \retval 0 if the timeout expired
+ * \retval 1 if the stream is ready for reading
+ */
+int ast_iostream_wait_for_input(struct ast_iostream *stream, int timeout);
+
+/*!
  * \brief Make an iostream non-blocking.
  *
  * \param stream A pointer to an iostream
diff --git a/main/iostream.c b/main/iostream.c
index 15131c0..d060b6d 100644
--- a/main/iostream.c
+++ b/main/iostream.c
@@ -86,6 +86,20 @@
 	return stream->fd;
 }
 
+int ast_iostream_wait_for_input(struct ast_iostream *stream, int timeout)
+{
+#if defined(DO_SSL)
+	/* Because SSL is read in blocks, it's possible that the last time we read we
+	   got more than we asked for and it is now buffered inside OpenSSL. If that
+	   is the case, calling ast_wait_for_input() will block until the fd is ready
+	   for reading again, which might never happen. */
+	if (stream->ssl && SSL_pending(stream->ssl)) {
+		return 1;
+	}
+#endif
+	return ast_wait_for_input(stream->fd, timeout);
+}
+
 void ast_iostream_nonblock(struct ast_iostream *stream)
 {
 	ast_fd_set_flags(stream->fd, O_NONBLOCK);
diff --git a/res/res_http_websocket.c b/res/res_http_websocket.c
index e79066b..63fccdd 100644
--- a/res/res_http_websocket.c
+++ b/res/res_http_websocket.c
@@ -427,6 +427,11 @@
 	return session->closing ? -1 : ast_iostream_get_fd(session->stream);
 }
 
+int AST_OPTIONAL_API_NAME(ast_websocket_wait_for_input)(struct ast_websocket *session, int timeout)
+{
+	return session->closing ? -1 : ast_iostream_wait_for_input(session->stream, timeout);
+}
+
 struct ast_sockaddr * AST_OPTIONAL_API_NAME(ast_websocket_remote_address)(struct ast_websocket *session)
 {
 	return &session->remote_address;
@@ -545,8 +550,8 @@
 				break;
 			}
 		}
-		if (ast_wait_for_input(ast_iostream_get_fd(session->stream), 1000) < 0) {
-			ast_log(LOG_ERROR, "ast_wait_for_input returned err: %s\n", strerror(errno));
+		if (ast_iostream_wait_for_input(session->stream, 1000) < 0) {
+			ast_log(LOG_ERROR, "ast_iostream_wait_for_input returned err: %s\n", strerror(errno));
 			*opcode = AST_WEBSOCKET_OPCODE_CLOSE;
 			session->closing = 1;
 			ao2_unlock(session);
@@ -974,7 +979,7 @@
 		goto end;
 	}
 
-	while ((res = ast_wait_for_input(ast_websocket_fd(session), -1)) > 0) {
+	while ((res = ast_websocket_wait_for_input(session, -1)) > 0) {
 		char *payload;
 		uint64_t payload_len;
 		enum ast_websocket_opcode opcode;
diff --git a/res/res_pjsip_transport_websocket.c b/res/res_pjsip_transport_websocket.c
index 6383f68..4f47a8c 100644
--- a/res/res_pjsip_transport_websocket.c
+++ b/res/res_pjsip_transport_websocket.c
@@ -392,7 +392,7 @@
 	transport = create_data.transport;
 	read_data.transport = transport;
 
-	while (ast_wait_for_input(ast_websocket_fd(session), -1) > 0) {
+	while (ast_websocket_wait_for_input(session, -1) > 0) {
 		enum ast_websocket_opcode opcode;
 		int fragmented;
 

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

Gerrit-Project: asterisk
Gerrit-Branch: 16
Gerrit-Change-Id: I95fcb3e2004700d5cf8e5ee04943f0115b15e10d
Gerrit-Change-Number: 13508
Gerrit-PatchSet: 1
Gerrit-Owner: Sean Bright <sean.bright at gmail.com>
Gerrit-MessageType: newchange
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20200102/731a0c0f/attachment.html>