From: Amaury Denoyelle <adenoyelle@haproxy.com>
Date: Fri, 11 Dec 2020 16:53:02 +0000 (+0100)
Subject: MINOR: h1: reject websocket handshake if missing key
X-Git-Tag: v2.4-dev7~87
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=18ee5c3eb0c8afc18627a0d9a096aabb4c61e34f;p=thirdparty%2Fhaproxy.git

MINOR: h1: reject websocket handshake if missing key

If a request is identified as a WebSocket handshake, it must contains a
websocket key header or else it can be reject, following the rfc6455.

A new flag H1_MF_UPG_WEBSOCKET is set on such messages.  For the request
te be identified as a WebSocket handshake, it must contains the headers:
  Connection: upgrade
  Upgrade: websocket

This commit is a compagnon of
"MEDIUM: h1: generate WebSocket key on response if needed" and
"MEDIUM: h1: add a WebSocket key on handshake if needed".

Indeed, it ensures that a WebSocket key is added only from a http/2 side
and not for a http/1 bogus peer.
---

diff --git a/include/haproxy/h1.h b/include/haproxy/h1.h
index 6ac1f61fde..32d6caf8bf 100644
--- a/include/haproxy/h1.h
+++ b/include/haproxy/h1.h
@@ -95,6 +95,7 @@ enum h1m_state {
 #define H1_MF_CLEAN_CONN_HDR    0x00001000 // skip close/keep-alive values of connection headers during parsing
 #define H1_MF_METH_CONNECT      0x00002000 // Set for a response to a CONNECT request
 #define H1_MF_METH_HEAD         0x00004000 // Set for a response to a HEAD request
+#define H1_MF_UPG_WEBSOCKET     0x00008000 // Set for a Websocket upgrade handshake
 
 /* Note: for a connection to be persistent, we need this for the request :
  *   - one of CLEN or CHNK
@@ -147,6 +148,7 @@ int h1_measure_trailers(const struct buffer *buf, unsigned int ofs, unsigned int
 int h1_parse_cont_len_header(struct h1m *h1m, struct ist *value);
 void h1_parse_xfer_enc_header(struct h1m *h1m, struct ist value);
 void h1_parse_connection_header(struct h1m *h1m, struct ist *value);
+void h1_parse_upgrade_header(struct h1m *h1m, struct ist value);
 
 /* for debugging, reports the HTTP/1 message state name */
 static inline const char *h1m_state_str(enum h1m_state msg_state)
diff --git a/src/h1.c b/src/h1.c
index bb8acfb345..ca39d97a4a 100644
--- a/src/h1.c
+++ b/src/h1.c
@@ -186,6 +186,38 @@ void h1_parse_connection_header(struct h1m *h1m, struct ist *value)
 	}
 }
 
+/* Parse the Upgrade: header of an HTTP/1 request.
+ * If "websocket" is found, set H1_MF_UPG_WEBSOCKET flag
+ */
+void h1_parse_upgrade_header(struct h1m *h1m, struct ist value)
+{
+	char *e, *n;
+	struct ist word;
+
+	h1m->flags &= ~H1_MF_UPG_WEBSOCKET;
+
+	word.ptr = value.ptr - 1; // -1 for next loop's pre-increment
+	e = value.ptr + value.len;
+
+	while (++word.ptr < e) {
+		/* skip leading delimiter and blanks */
+		if (HTTP_IS_LWS(*word.ptr))
+			continue;
+
+		n = http_find_hdr_value_end(word.ptr, e); // next comma or end of line
+		word.len = n - word.ptr;
+
+		/* trim trailing blanks */
+		while (word.len && HTTP_IS_LWS(word.ptr[word.len-1]))
+			word.len--;
+
+		if (isteqi(word, ist("websocket")))
+			h1m->flags |= H1_MF_UPG_WEBSOCKET;
+
+		word.ptr = n;
+	}
+}
+
 /* Macros used in the HTTP/1 parser, to check for the expected presence of
  * certain bytes (ef: LF) or to skip to next byte and yield in case of failure.
  */
@@ -828,6 +860,9 @@ int h1_headers_to_hdr_list(char *start, const char *stop,
 						break;
 					}
 				}
+				else if (isteqi(n, ist("upgrade"))) {
+					h1_parse_upgrade_header(h1m, v);
+				}
 				else if (!(h1m->flags & (H1_MF_HDRS_ONLY|H1_MF_RESP)) && isteqi(n, ist("host"))) {
 					if (host_idx == -1) {
 						struct ist authority;
diff --git a/src/mux_h1.c b/src/mux_h1.c
index a49531f55d..413ed09bec 100644
--- a/src/mux_h1.c
+++ b/src/mux_h1.c
@@ -1298,6 +1298,51 @@ static void h1_set_tunnel_mode(struct h1s *h1s)
 	}
 }
 
+/* Search for a websocket key header. The message should have been identified
+ * as a valid websocket handshake.
+ * Returns 0 if no key found
+ */
+static int h1_search_websocket_key(struct h1s *h1s, struct h1m *h1m, struct htx *htx)
+{
+	struct htx_blk *blk;
+	enum htx_blk_type type;
+	struct ist n;
+	int ws_key_found = 0, idx;
+
+	idx = htx_get_head(htx); // returns the SL that we skip
+	while ((idx = htx_get_next(htx, idx)) != -1) {
+		blk = htx_get_blk(htx, idx);
+		type = htx_get_blk_type(blk);
+
+		if (type == HTX_BLK_UNUSED)
+			continue;
+
+		if (type != HTX_BLK_HDR)
+			break;
+
+		n = htx_get_blk_name(htx, blk);
+
+		if (isteqi(n, ist("sec-websocket-key")) &&
+		    !(h1m->flags & H1_MF_RESP)) {
+			ws_key_found = 1;
+			break;
+		}
+		else if (isteqi(n, ist("sec-websocket-accept")) &&
+		         h1m->flags & H1_MF_RESP) {
+			ws_key_found = 1;
+			break;
+		}
+	}
+
+	/* missing websocket key, reject the message */
+	if (!ws_key_found) {
+		htx->flags |= HTX_FL_PARSING_ERROR;
+		return 0;
+	}
+
+	return 1;
+}
+
 /*
  * Parse HTTP/1 headers. It returns the number of bytes parsed if > 0, or 0 if
  * it couldn't proceed. Parsing errors are reported by setting H1S_F_*_ERROR
@@ -1327,6 +1372,21 @@ static size_t h1_process_headers(struct h1s *h1s, struct h1m *h1m, struct htx *h
 		goto end;
 	}
 
+	/* If websocket handshake, search for the websocket key */
+	if ((h1m->flags & (H1_MF_CONN_UPG|H1_MF_UPG_WEBSOCKET)) ==
+	    (H1_MF_CONN_UPG|H1_MF_UPG_WEBSOCKET)) {
+		int ws_ret = h1_search_websocket_key(h1s, h1m, htx);
+		if (!ws_ret) {
+			TRACE_DEVEL("leaving on websocket missing key", H1_EV_RX_DATA|H1_EV_RX_HDRS, h1s->h1c->conn, h1s);
+			h1s->flags |= H1S_F_PARSING_ERROR;
+			TRACE_USER("parsing error, reject H1 message", H1_EV_RX_DATA|H1_EV_RX_HDRS|H1_EV_H1S_ERR, h1s->h1c->conn, h1s);
+			h1_capture_bad_message(h1s->h1c, h1s, h1m, buf);
+
+			ret = 0;
+			goto end;
+		}
+	}
+
 	if (h1m->err_pos >= 0)  {
 		/* Maybe we found an error during the parsing while we were
 		 * configured not to block on that, so we have to capture it