[asterisk-commits] moy: branch moy/webrtc-11 r409359 - /team/moy/webrtc-11/res/res_http_websocket.c

[SVN commits to the Asterisk project]

Author: moy
Date: Sun Mar  2 18:47:24 2014
New Revision: 409359

URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=409359
Log:
Addressed comments on review 3248 for secure websocket fixes

* Rework the WS_SAFE_READ() macro into an inline function
* Use ast_debug() instead of ast_log(LOG_DEBUG) for some extra debug logging

Modified:
    team/moy/webrtc-11/res/res_http_websocket.c

Modified: team/moy/webrtc-11/res/res_http_websocket.c
URL: http://svnview.digium.com/svn/asterisk/team/moy/webrtc-11/res/res_http_websocket.c?view=diff&rev=409359&r1=409358&r2=409359
==============================================================================
--- team/moy/webrtc-11/res/res_http_websocket.c (original)
+++ team/moy/webrtc-11/res/res_http_websocket.c Sun Mar  2 18:47:24 2014
@@ -315,40 +315,42 @@
  *
  * Note during the header parsing stage we try to read in small chunks just what we need, this is buffered data anyways, no expensive syscall required most of the time ...
  */
-#define WS_SAFE_READ(session, buf, len) \
-	do { \
-		int sanity; \
-		size_t rlen; \
-		int xlen = len; \
-		char *rbuf = buf; \
-		for (sanity = 10; sanity; sanity--) { \
-			clearerr(session->f); \
-			rlen = fread(rbuf, 1, xlen, session->f); \
-			if (0 == rlen && ferror(session->f) && errno != EAGAIN) { \
-				ast_log(LOG_ERROR, "Error reading from web socket: %s\n", strerror(errno)); \
-				(*opcode) = AST_WEBSOCKET_OPCODE_CLOSE; \
-				session->closing = 1; \
-				return 0; \
-			} \
-			xlen = (xlen - rlen); \
-			rbuf = rbuf + rlen;\
-			if (0 == xlen) { \
-				break; \
-			} \
-			if (ast_wait_for_input(session->fd, 1000) < 0) { \
-				ast_log(LOG_ERROR, "ast_wait_for_input returned err: %s\n", strerror(errno)); \
-				(*opcode) = AST_WEBSOCKET_OPCODE_CLOSE; \
-				session->closing = 1; \
-				return 0; \
-			} \
-		} \
-		if (!sanity) { \
-			ast_log(LOG_DEBUG, "Websocket seems unresponsive, disconnecting ...\n"); \
-			(*opcode) = AST_WEBSOCKET_OPCODE_CLOSE; \
-			session->closing = 1; \
-			return 0; \
-		} \
-	} while (0);
+static inline int ws_safe_read(struct ast_websocket *session, char *buf, int len, enum ast_websocket_opcode *opcode)
+{
+	int sanity = 0;
+	size_t rlen = 0;
+	int xlen = len;
+	char *rbuf = buf;
+	for (sanity = 10; sanity; sanity--) {
+		clearerr(session->f);
+		rlen = fread(rbuf, 1, xlen, session->f);
+		if (0 == rlen && ferror(session->f) && errno != EAGAIN) {
+			ast_log(LOG_ERROR, "Error reading from web socket: %s\n", strerror(errno));
+			(*opcode) = AST_WEBSOCKET_OPCODE_CLOSE;
+			session->closing = 1;
+			return -1;
+		}
+		xlen = (xlen - rlen);
+		rbuf = rbuf + rlen;
+		if (0 == xlen) {
+			break;
+		}
+		if (ast_wait_for_input(session->fd, 1000) < 0) {
+			ast_log(LOG_ERROR, "ast_wait_for_input returned err: %s\n", strerror(errno));
+			(*opcode) = AST_WEBSOCKET_OPCODE_CLOSE;
+			session->closing = 1;
+			return -1;
+		}
+	}
+	if (!sanity) {
+		ast_log(LOG_WARNING, "Websocket seems unresponsive, disconnecting ...\n");
+		(*opcode) = AST_WEBSOCKET_OPCODE_CLOSE;
+		session->closing = 1;
+		return -1;
+	}
+	return 0;
+}
+
 int AST_OPTIONAL_API_NAME(ast_websocket_read)(struct ast_websocket *session, char **payload, uint64_t *payload_len, enum ast_websocket_opcode *opcode, int *fragmented)
 {
 	char buf[MAXIMUM_FRAME_SIZE] = "";
@@ -361,7 +363,9 @@
 	*payload_len = 0;
 	*fragmented = 0;
 
-	WS_SAFE_READ(session, &buf[0], MIN_WS_HDR_SZ);
+	if (ws_safe_read(session, &buf[0], MIN_WS_HDR_SZ, opcode)) {
+		return 0;
+	}
 	frame_size += MIN_WS_HDR_SZ;
 
 	/* ok, now we have the first 2 bytes, so we know some flags, opcode and payload length (or whether payload length extension will be required) */
@@ -377,7 +381,9 @@
 		options_len += (*payload_len == 126) ? 2 : (*payload_len == 127) ? 8 : 0;
 		if (options_len) {
 			/* read the rest of the header options */
-			WS_SAFE_READ(session, &buf[frame_size], options_len);
+			if (ws_safe_read(session, &buf[frame_size], options_len, opcode)) {
+				return 0;
+			}
 			frame_size += options_len;
 		}
 
@@ -404,7 +410,9 @@
 			return -1;
 		}
 
-		WS_SAFE_READ(session, (*payload), (*payload_len));
+		if (ws_safe_read(session, (*payload), (*payload_len), opcode)) {
+			return 0;
+		}
 
 		/* If a mask is present unmask the payload */
 		if (mask_present) {
@@ -456,7 +464,9 @@
 	} else if (*opcode == AST_WEBSOCKET_OPCODE_CLOSE) {
 		/* Make the payload available so the user can look at the reason code if they so desire */
 		if ((*payload_len) && (new_payload = ast_realloc(session->payload, *payload_len))) {
-			WS_SAFE_READ(session, &buf[frame_size], (*payload_len));
+			if (ws_safe_read(session, &buf[frame_size], (*payload_len), opcode)) {
+				return 0;
+			}
 			session->payload = new_payload;
 			memcpy(session->payload, &buf[frame_size], *payload_len);
 			*payload = session->payload;
@@ -649,7 +659,7 @@
 {
 	int flags, res;
 
-	ast_log(LOG_DEBUG, "Entering WebSocket echo loop\n");
+	ast_debug(1, "Entering WebSocket echo loop\n");
 
 	if ((flags = fcntl(ast_websocket_fd(session), F_GETFL)) == -1) {
 		goto end;
@@ -678,12 +688,12 @@
 		} else if (opcode == AST_WEBSOCKET_OPCODE_CLOSE) {
 			break;
 		} else {
-			ast_log(LOG_DEBUG, "Ignored WebSocket opcode %d\n", opcode);
+			ast_debug(1, "Ignored WebSocket opcode %d\n", opcode);
 		}
 	}
 
 end:
-	ast_log(LOG_DEBUG, "Exitting WebSocket echo loop\n");
+	ast_debug(1, "Exitting WebSocket echo loop\n");
 	ast_websocket_unref(session);
 }