From f69a8bde29354493ff8aea64cc9cb3b531d16337 Mon Sep 17 00:00:00 2001
From: "Daniel P. Berrange" <berrange@redhat.com>
Date: Wed, 6 Sep 2017 11:33:17 +0100
Subject: [PATCH 01/11] io: send proper HTTP response for websocket errors
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
When any error occurs while processing the websockets handshake,
QEMU just terminates the connection abruptly. This is in violation
of the HTTP specs and does not help the client understand what they
did wrong. This is particularly bad when the client gives the wrong
path, as a "404 Not Found" would be very helpful.
Refactor the handshake code so that it always sends a response to
the client unless there was an I/O error.
Fixes bug: #1715186
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
io/channel-websock.c | 185 ++++++++++++++++++++++++++++++++-----------
1 file changed, 139 insertions(+), 46 deletions(-)
diff --git a/io/channel-websock.c b/io/channel-websock.c
index 5a3badbec2..f5fac5b422 100644
--- a/io/channel-websock.c
+++ b/io/channel-websock.c
@@ -25,6 +25,8 @@
#include "crypto/hash.h"
#include "trace.h"
+#include <time.h>
+
/* Max amount to allow in rawinput/rawoutput buffers */
#define QIO_CHANNEL_WEBSOCK_MAX_BUFFER 8192
@@ -44,13 +46,40 @@
#define QIO_CHANNEL_WEBSOCK_CONNECTION_UPGRADE "Upgrade"
#define QIO_CHANNEL_WEBSOCK_UPGRADE_WEBSOCKET "websocket"
-#define QIO_CHANNEL_WEBSOCK_HANDSHAKE_RESPONSE \
+#define QIO_CHANNEL_WEBSOCK_HANDSHAKE_RES_COMMON \
+ "Server: QEMU VNC\r\n" \
+ "Date: %s\r\n"
+
+#define QIO_CHANNEL_WEBSOCK_HANDSHAKE_RES_OK \
"HTTP/1.1 101 Switching Protocols\r\n" \
+ QIO_CHANNEL_WEBSOCK_HANDSHAKE_RES_COMMON \
"Upgrade: websocket\r\n" \
"Connection: Upgrade\r\n" \
"Sec-WebSocket-Accept: %s\r\n" \
"Sec-WebSocket-Protocol: binary\r\n" \
"\r\n"
+#define QIO_CHANNEL_WEBSOCK_HANDSHAKE_RES_NOT_FOUND \
+ "HTTP/1.1 404 Not Found\r\n" \
+ QIO_CHANNEL_WEBSOCK_HANDSHAKE_RES_COMMON \
+ "Connection: close\r\n" \
+ "\r\n"
+#define QIO_CHANNEL_WEBSOCK_HANDSHAKE_RES_BAD_REQUEST \
+ "HTTP/1.1 400 Bad Request\r\n" \
+ QIO_CHANNEL_WEBSOCK_HANDSHAKE_RES_COMMON \
+ "Connection: close\r\n" \
+ "Sec-WebSocket-Version: " \
+ QIO_CHANNEL_WEBSOCK_SUPPORTED_VERSION \
+ "\r\n"
+#define QIO_CHANNEL_WEBSOCK_HANDSHAKE_RES_SERVER_ERR \
+ "HTTP/1.1 500 Internal Server Error\r\n" \
+ QIO_CHANNEL_WEBSOCK_HANDSHAKE_RES_COMMON \
+ "Connection: close\r\n" \
+ "\r\n"
+#define QIO_CHANNEL_WEBSOCK_HANDSHAKE_RES_TOO_LARGE \
+ "HTTP/1.1 403 Request Entity Too Large\r\n" \
+ QIO_CHANNEL_WEBSOCK_HANDSHAKE_RES_COMMON \
+ "Connection: close\r\n" \
+ "\r\n"
#define QIO_CHANNEL_WEBSOCK_HANDSHAKE_DELIM "\r\n"
#define QIO_CHANNEL_WEBSOCK_HANDSHAKE_END "\r\n\r\n"
#define QIO_CHANNEL_WEBSOCK_SUPPORTED_VERSION "13"
@@ -123,8 +152,46 @@ enum {
QIO_CHANNEL_WEBSOCK_OPCODE_PONG = 0xA
};
+static void qio_channel_websock_handshake_send_res(QIOChannelWebsock *ioc,
+ const char *resmsg,
+ ...)
+{
+ va_list vargs;
+ char *response;
+ size_t responselen;
+
+ va_start(vargs, resmsg);
+ response = g_strdup_vprintf(resmsg, vargs);
+ responselen = strlen(response);
+ buffer_reserve(&ioc->encoutput, responselen);
+ buffer_append(&ioc->encoutput, response, responselen);
+ va_end(vargs);
+}
+
+static gchar *qio_channel_websock_date_str(void)
+{
+ struct tm tm;
+ time_t now = time(NULL);
+ char datebuf[128];
+
+ gmtime_r(&now, &tm);
+
+ strftime(datebuf, sizeof(datebuf), "%a, %d %b %Y %H:%M:%S GMT", &tm);
+
+ return g_strdup(datebuf);
+}
+
+static void qio_channel_websock_handshake_send_res_err(QIOChannelWebsock *ioc,
+ const char *resdata)
+{
+ char *date = qio_channel_websock_date_str();
+ qio_channel_websock_handshake_send_res(ioc, resdata, date);
+ g_free(date);
+}
+
static size_t
-qio_channel_websock_extract_headers(char *buffer,
+qio_channel_websock_extract_headers(QIOChannelWebsock *ioc,
+ char *buffer,
QIOChannelWebsockHTTPHeader *hdrs,
size_t nhdrsalloc,
Error **errp)
@@ -145,7 +212,7 @@ qio_channel_websock_extract_headers(char *buffer,
nl = strstr(buffer, QIO_CHANNEL_WEBSOCK_HANDSHAKE_DELIM);
if (!nl) {
error_setg(errp, "Missing HTTP header delimiter");
- return 0;
+ goto bad_request;
}
*nl = '\0';
@@ -158,18 +225,20 @@ qio_channel_websock_extract_headers(char *buffer,
if (!g_str_equal(buffer, QIO_CHANNEL_WEBSOCK_HTTP_METHOD)) {
error_setg(errp, "Unsupported HTTP method %s", buffer);
- return 0;
+ goto bad_request;
}
buffer = tmp + 1;
tmp = strchr(buffer, ' ');
if (!tmp) {
error_setg(errp, "Missing HTTP version delimiter");
- return 0;
+ goto bad_request;
}
*tmp = '\0';
if (!g_str_equal(buffer, QIO_CHANNEL_WEBSOCK_HTTP_PATH)) {
+ qio_channel_websock_handshake_send_res_err(
+ ioc, QIO_CHANNEL_WEBSOCK_HANDSHAKE_RES_NOT_FOUND);
error_setg(errp, "Unexpected HTTP path %s", buffer);
return 0;
}
@@ -178,7 +247,7 @@ qio_channel_websock_extract_headers(char *buffer,
if (!g_str_equal(buffer, QIO_CHANNEL_WEBSOCK_HTTP_VERSION)) {
error_setg(errp, "Unsupported HTTP version %s", buffer);
- return 0;
+ goto bad_request;
}
buffer = nl + strlen(QIO_CHANNEL_WEBSOCK_HANDSHAKE_DELIM);
@@ -203,7 +272,7 @@ qio_channel_websock_extract_headers(char *buffer,
sep = strchr(buffer, ':');
if (!sep) {
error_setg(errp, "Malformed HTTP header");
- return 0;
+ goto bad_request;
}
*sep = '\0';
sep++;
@@ -213,7 +282,7 @@ qio_channel_websock_extract_headers(char *buffer,
if (nhdrs >= nhdrsalloc) {
error_setg(errp, "Too many HTTP headers");
- return 0;
+ goto bad_request;
}
hdr = &hdrs[nhdrs++];
@@ -231,6 +300,11 @@ qio_channel_websock_extract_headers(char *buffer,
} while (nl != NULL);
return nhdrs;
+
+ bad_request:
+ qio_channel_websock_handshake_send_res_err(
+ ioc, QIO_CHANNEL_WEBSOCK_HANDSHAKE_RES_BAD_REQUEST);
+ return 0;
}
static const char *
@@ -250,14 +324,14 @@ qio_channel_websock_find_header(QIOChannelWebsockHTTPHeader *hdrs,
}
-static int qio_channel_websock_handshake_send_response(QIOChannelWebsock *ioc,
- const char *key,
- Error **errp)
+static void qio_channel_websock_handshake_send_res_ok(QIOChannelWebsock *ioc,
+ const char *key,
+ Error **errp)
{
char combined_key[QIO_CHANNEL_WEBSOCK_CLIENT_KEY_LEN +
QIO_CHANNEL_WEBSOCK_GUID_LEN + 1];
- char *accept = NULL, *response = NULL;
- size_t responselen;
+ char *accept = NULL;
+ char *date = qio_channel_websock_date_str();
g_strlcpy(combined_key, key, QIO_CHANNEL_WEBSOCK_CLIENT_KEY_LEN + 1);
g_strlcat(combined_key, QIO_CHANNEL_WEBSOCK_GUID,
@@ -271,105 +345,108 @@ static int qio_channel_websock_handshake_send_response(QIOChannelWebsock *ioc,
QIO_CHANNEL_WEBSOCK_GUID_LEN,
&accept,
errp) < 0) {
- return -1;
+ qio_channel_websock_handshake_send_res_err(
+ ioc, QIO_CHANNEL_WEBSOCK_HANDSHAKE_RES_SERVER_ERR);
+ return;
}
- response = g_strdup_printf(QIO_CHANNEL_WEBSOCK_HANDSHAKE_RESPONSE, accept);
- responselen = strlen(response);
- buffer_reserve(&ioc->encoutput, responselen);
- buffer_append(&ioc->encoutput, response, responselen);
+ qio_channel_websock_handshake_send_res(
+ ioc, QIO_CHANNEL_WEBSOCK_HANDSHAKE_RES_OK, date, accept);
+ g_free(date);
g_free(accept);
- g_free(response);
-
- return 0;
}
-static int qio_channel_websock_handshake_process(QIOChannelWebsock *ioc,
- char *buffer,
- Error **errp)
+static void qio_channel_websock_handshake_process(QIOChannelWebsock *ioc,
+ char *buffer,
+ Error **errp)
{
QIOChannelWebsockHTTPHeader hdrs[32];
size_t nhdrs = G_N_ELEMENTS(hdrs);
const char *protocols = NULL, *version = NULL, *key = NULL,
*host = NULL, *connection = NULL, *upgrade = NULL;
- nhdrs = qio_channel_websock_extract_headers(buffer, hdrs, nhdrs, errp);
+ nhdrs = qio_channel_websock_extract_headers(ioc, buffer, hdrs, nhdrs, errp);
if (!nhdrs) {
- return -1;
+ return;
}
protocols = qio_channel_websock_find_header(
hdrs, nhdrs, QIO_CHANNEL_WEBSOCK_HEADER_PROTOCOL);
if (!protocols) {
error_setg(errp, "Missing websocket protocol header data");
- return -1;
+ goto bad_request;
}
version = qio_channel_websock_find_header(
hdrs, nhdrs, QIO_CHANNEL_WEBSOCK_HEADER_VERSION);
if (!version) {
error_setg(errp, "Missing websocket version header data");
- return -1;
+ goto bad_request;
}
key = qio_channel_websock_find_header(
hdrs, nhdrs, QIO_CHANNEL_WEBSOCK_HEADER_KEY);
if (!key) {
error_setg(errp, "Missing websocket key header data");
- return -1;
+ goto bad_request;
}
host = qio_channel_websock_find_header(
hdrs, nhdrs, QIO_CHANNEL_WEBSOCK_HEADER_HOST);
if (!host) {
error_setg(errp, "Missing websocket host header data");
- return -1;
+ goto bad_request;
}
connection = qio_channel_websock_find_header(
hdrs, nhdrs, QIO_CHANNEL_WEBSOCK_HEADER_CONNECTION);
if (!connection) {
error_setg(errp, "Missing websocket connection header data");
- return -1;
+ goto bad_request;
}
upgrade = qio_channel_websock_find_header(
hdrs, nhdrs, QIO_CHANNEL_WEBSOCK_HEADER_UPGRADE);
if (!upgrade) {
error_setg(errp, "Missing websocket upgrade header data");
- return -1;
+ goto bad_request;
}
if (!g_strrstr(protocols, QIO_CHANNEL_WEBSOCK_PROTOCOL_BINARY)) {
error_setg(errp, "No '%s' protocol is supported by client '%s'",
QIO_CHANNEL_WEBSOCK_PROTOCOL_BINARY, protocols);
- return -1;
+ goto bad_request;
}
if (!g_str_equal(version, QIO_CHANNEL_WEBSOCK_SUPPORTED_VERSION)) {
error_setg(errp, "Version '%s' is not supported by client '%s'",
QIO_CHANNEL_WEBSOCK_SUPPORTED_VERSION, version);
- return -1;
+ goto bad_request;
}
if (strlen(key) != QIO_CHANNEL_WEBSOCK_CLIENT_KEY_LEN) {
error_setg(errp, "Key length '%zu' was not as expected '%d'",
strlen(key), QIO_CHANNEL_WEBSOCK_CLIENT_KEY_LEN);
- return -1;
+ goto bad_request;
}
if (!g_strrstr(connection, QIO_CHANNEL_WEBSOCK_CONNECTION_UPGRADE)) {
error_setg(errp, "No connection upgrade requested '%s'", connection);
- return -1;
+ goto bad_request;
}
if (!g_str_equal(upgrade, QIO_CHANNEL_WEBSOCK_UPGRADE_WEBSOCKET)) {
error_setg(errp, "Incorrect upgrade method '%s'", upgrade);
- return -1;
+ goto bad_request;
}
- return qio_channel_websock_handshake_send_response(ioc, key, errp);
+ qio_channel_websock_handshake_send_res_ok(ioc, key, errp);
+ return;
+
+ bad_request:
+ qio_channel_websock_handshake_send_res_err(
+ ioc, QIO_CHANNEL_WEBSOCK_HANDSHAKE_RES_BAD_REQUEST);
}
static int qio_channel_websock_handshake_read(QIOChannelWebsock *ioc,
@@ -393,20 +470,20 @@ static int qio_channel_websock_handshake_read(QIOChannelWebsock *ioc,
QIO_CHANNEL_WEBSOCK_HANDSHAKE_END);
if (!handshake_end) {
if (ioc->encinput.offset >= 4096) {
+ qio_channel_websock_handshake_send_res_err(
+ ioc, QIO_CHANNEL_WEBSOCK_HANDSHAKE_RES_TOO_LARGE);
error_setg(errp,
"End of headers not found in first 4096 bytes");
- return -1;
+ return 1;
} else {
return 0;
}
}
*handshake_end = '\0';
- if (qio_channel_websock_handshake_process(ioc,
- (char *)ioc->encinput.buffer,
- errp) < 0) {
- return -1;
- }
+ qio_channel_websock_handshake_process(ioc,
+ (char *)ioc->encinput.buffer,
+ errp);
buffer_advance(&ioc->encinput,
handshake_end - (char *)ioc->encinput.buffer +
@@ -438,8 +515,15 @@ static gboolean qio_channel_websock_handshake_send(QIOChannel *ioc,
buffer_advance(&wioc->encoutput, ret);
if (wioc->encoutput.offset == 0) {
- trace_qio_channel_websock_handshake_complete(ioc);
- qio_task_complete(task);
+ if (wioc->io_err) {
+ trace_qio_channel_websock_handshake_fail(ioc);
+ qio_task_set_error(task, wioc->io_err);
+ wioc->io_err = NULL;
+ qio_task_complete(task);
+ } else {
+ trace_qio_channel_websock_handshake_complete(ioc);
+ qio_task_complete(task);
+ }
return FALSE;
}
trace_qio_channel_websock_handshake_pending(ioc, G_IO_OUT);
@@ -458,6 +542,11 @@ static gboolean qio_channel_websock_handshake_io(QIOChannel *ioc,
ret = qio_channel_websock_handshake_read(wioc, &err);
if (ret < 0) {
+ /*
+ * We only take this path on a fatal I/O error reading from
+ * client connection, as most of the time we have an
+ * HTTP 4xx err response to send instead
+ */
trace_qio_channel_websock_handshake_fail(ioc);
qio_task_set_error(task, err);
qio_task_complete(task);
@@ -469,6 +558,10 @@ static gboolean qio_channel_websock_handshake_io(QIOChannel *ioc,
return TRUE;
}
+ if (err) {
+ error_propagate(&wioc->io_err, err);
+ }
+
trace_qio_channel_websock_handshake_reply(ioc);
qio_channel_add_watch(
wioc->master,
From 3a3f8705962c8c8a47a9b981ffd5aab7274ad508 Mon Sep 17 00:00:00 2001
From: "Daniel P. Berrange" <berrange@redhat.com>
Date: Wed, 6 Sep 2017 11:38:36 +0100
Subject: [PATCH 02/11] io: include full error message in websocket handshake
trace
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
When the websocket handshake fails it is useful to log the real
error message via the trace points for debugging purposes.
Fixes bug: #1715186
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
io/channel-websock.c | 7 ++++---
io/trace-events | 2 +-
2 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/io/channel-websock.c b/io/channel-websock.c
index f5fac5b422..6ddcec1549 100644
--- a/io/channel-websock.c
+++ b/io/channel-websock.c
@@ -507,7 +507,7 @@ static gboolean qio_channel_websock_handshake_send(QIOChannel *ioc,
&err);
if (ret < 0) {
- trace_qio_channel_websock_handshake_fail(ioc);
+ trace_qio_channel_websock_handshake_fail(ioc, error_get_pretty(err));
qio_task_set_error(task, err);
qio_task_complete(task);
return FALSE;
@@ -516,7 +516,8 @@ static gboolean qio_channel_websock_handshake_send(QIOChannel *ioc,
buffer_advance(&wioc->encoutput, ret);
if (wioc->encoutput.offset == 0) {
if (wioc->io_err) {
- trace_qio_channel_websock_handshake_fail(ioc);
+ trace_qio_channel_websock_handshake_fail(
+ ioc, error_get_pretty(wioc->io_err));
qio_task_set_error(task, wioc->io_err);
wioc->io_err = NULL;
qio_task_complete(task);
@@ -547,7 +548,7 @@ static gboolean qio_channel_websock_handshake_io(QIOChannel *ioc,
* client connection, as most of the time we have an
* HTTP 4xx err response to send instead
*/
- trace_qio_channel_websock_handshake_fail(ioc);
+ trace_qio_channel_websock_handshake_fail(ioc, error_get_pretty(err));
qio_task_set_error(task, err);
qio_task_complete(task);
return FALSE;
diff --git a/io/trace-events b/io/trace-events
index 3d233698d0..6459f71f5b 100644
--- a/io/trace-events
+++ b/io/trace-events
@@ -46,7 +46,7 @@ qio_channel_websock_new_server(void *ioc, void *master) "Websock new client ioc=
qio_channel_websock_handshake_start(void *ioc) "Websock handshake start ioc=%p"
qio_channel_websock_handshake_pending(void *ioc, int status) "Websock handshake pending ioc=%p status=%d"
qio_channel_websock_handshake_reply(void *ioc) "Websock handshake reply ioc=%p"
-qio_channel_websock_handshake_fail(void *ioc) "Websock handshake fail ioc=%p"
+qio_channel_websock_handshake_fail(void *ioc, const char *msg) "Websock handshake fail ioc=%p err=%s"
qio_channel_websock_handshake_complete(void *ioc) "Websock handshake complete ioc=%p"
# io/channel-command.c
From 33badfd1e3735b877e41939100511c65572be6b9 Mon Sep 17 00:00:00 2001
From: "Daniel P. Berrange" <berrange@redhat.com>
Date: Wed, 6 Sep 2017 14:49:41 +0100
Subject: [PATCH 03/11] io: use case insensitive check for Connection & Upgrade
websock headers
When checking the value of the Connection and Upgrade HTTP headers
the websock RFC (6455) requires the comparison to be case insensitive.
The Connection value should be an exact match not a substring.
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
io/channel-websock.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/io/channel-websock.c b/io/channel-websock.c
index 6ddcec1549..2258557a21 100644
--- a/io/channel-websock.c
+++ b/io/channel-websock.c
@@ -431,12 +431,12 @@ static void qio_channel_websock_handshake_process(QIOChannelWebsock *ioc,
goto bad_request;
}
- if (!g_strrstr(connection, QIO_CHANNEL_WEBSOCK_CONNECTION_UPGRADE)) {
+ if (strcasecmp(connection, QIO_CHANNEL_WEBSOCK_CONNECTION_UPGRADE) != 0) {
error_setg(errp, "No connection upgrade requested '%s'", connection);
goto bad_request;
}
- if (!g_str_equal(upgrade, QIO_CHANNEL_WEBSOCK_UPGRADE_WEBSOCKET)) {
+ if (strcasecmp(upgrade, QIO_CHANNEL_WEBSOCK_UPGRADE_WEBSOCKET) != 0) {
error_setg(errp, "Incorrect upgrade method '%s'", upgrade);
goto bad_request;
}
From a75d6f07613af7ec5b016b31b117436e32ce7a5f Mon Sep 17 00:00:00 2001
From: Brandon Carpenter <brandon.carpenter@cypherpath.com>
Date: Tue, 12 Sep 2017 08:21:47 -0700
Subject: [PATCH 04/11] ui: Always remove an old VNC channel watch before
adding a new one
Also set saved handle to zero when removing without adding a new watch.
Signed-off-by: Brandon Carpenter <brandon.carpenter@cypherpath.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Daniel P. Berrange <berrange@redhat.com>
---
ui/vnc-auth-vencrypt.c | 3 +++
ui/vnc-ws.c | 6 ++++++
ui/vnc.c | 4 ++++
3 files changed, 13 insertions(+)
diff --git a/ui/vnc-auth-vencrypt.c b/ui/vnc-auth-vencrypt.c
index f0bec204b3..7833631275 100644
--- a/ui/vnc-auth-vencrypt.c
+++ b/ui/vnc-auth-vencrypt.c
@@ -75,6 +75,9 @@ static void vnc_tls_handshake_done(QIOTask *task,
vnc_client_error(vs);
error_free(err);
} else {
+ if (vs->ioc_tag) {
+ g_source_remove(vs->ioc_tag);
+ }
vs->ioc_tag = qio_channel_add_watch(
vs->ioc, G_IO_IN | G_IO_OUT, vnc_client_io, vs, NULL);
start_auth_vencrypt_subauth(vs);
diff --git a/ui/vnc-ws.c b/ui/vnc-ws.c
index aeaafe2c21..6ccad22cef 100644
--- a/ui/vnc-ws.c
+++ b/ui/vnc-ws.c
@@ -37,6 +37,9 @@ static void vncws_tls_handshake_done(QIOTask *task,
error_free(err);
} else {
VNC_DEBUG("TLS handshake complete, starting websocket handshake\n");
+ if (vs->ioc_tag) {
+ g_source_remove(vs->ioc_tag);
+ }
vs->ioc_tag = qio_channel_add_watch(
QIO_CHANNEL(vs->ioc), G_IO_IN, vncws_handshake_io, vs, NULL);
}
@@ -97,6 +100,9 @@ static void vncws_handshake_done(QIOTask *task,
} else {
VNC_DEBUG("Websock handshake complete, starting VNC protocol\n");
vnc_start_protocol(vs);
+ if (vs->ioc_tag) {
+ g_source_remove(vs->ioc_tag);
+ }
vs->ioc_tag = qio_channel_add_watch(
vs->ioc, G_IO_IN, vnc_client_io, vs, NULL);
}
diff --git a/ui/vnc.c b/ui/vnc.c
index af810f0547..9f8d5a1b1f 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -1122,6 +1122,7 @@ static void vnc_disconnect_start(VncState *vs)
vnc_set_share_mode(vs, VNC_SHARE_MODE_DISCONNECTED);
if (vs->ioc_tag) {
g_source_remove(vs->ioc_tag);
+ vs->ioc_tag = 0;
}
qio_channel_close(vs->ioc, NULL);
vs->disconnecting = TRUE;
@@ -2934,6 +2935,9 @@ static void vnc_connect(VncDisplay *vd, QIOChannelSocket *sioc,
VNC_DEBUG("New client on socket %p\n", vs->sioc);
update_displaychangelistener(&vd->dcl, VNC_REFRESH_INTERVAL_BASE);
qio_channel_set_blocking(vs->ioc, false, NULL);
+ if (vs->ioc_tag) {
+ g_source_remove(vs->ioc_tag);
+ }
if (websocket) {
vs->websocket = 1;
if (vd->tlscreds) {
From eefa3d8ef649f9055611361e2201cca49f8c3433 Mon Sep 17 00:00:00 2001
From: Brandon Carpenter <brandon.carpenter@cypherpath.com>
Date: Tue, 12 Sep 2017 08:21:48 -0700
Subject: [PATCH 05/11] io: Small updates in preparation for websocket changes
Gets rid of unnecessary bit shifting and performs proper EOF checking to
avoid a large number of repeated calls to recvmsg() when a client
abruptly terminates a connection (bug fix).
Signed-off-by: Brandon Carpenter <brandon.carpenter@cypherpath.com>
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
io/channel-websock.c | 64 +++++++++++++-------------------------------
1 file changed, 19 insertions(+), 45 deletions(-)
diff --git a/io/channel-websock.c b/io/channel-websock.c
index 2258557a21..4e5afb2e95 100644
--- a/io/channel-websock.c
+++ b/io/channel-websock.c
@@ -110,13 +110,11 @@
/* Magic 7-bit length to indicate use of 64-bit payload length */
#define QIO_CHANNEL_WEBSOCK_PAYLOAD_LEN_MAGIC_64_BIT 127
-/* Bitmasks & shifts for accessing header fields */
+/* Bitmasks for accessing header fields */
#define QIO_CHANNEL_WEBSOCK_HEADER_FIELD_FIN 0x80
#define QIO_CHANNEL_WEBSOCK_HEADER_FIELD_OPCODE 0x0f
#define QIO_CHANNEL_WEBSOCK_HEADER_FIELD_HAS_MASK 0x80
#define QIO_CHANNEL_WEBSOCK_HEADER_FIELD_PAYLOAD_LEN 0x7f
-#define QIO_CHANNEL_WEBSOCK_HEADER_SHIFT_FIN 7
-#define QIO_CHANNEL_WEBSOCK_HEADER_SHIFT_HAS_MASK 7
typedef struct QIOChannelWebsockHeader QIOChannelWebsockHeader;
@@ -586,7 +584,7 @@ static void qio_channel_websock_encode(QIOChannelWebsock *ioc)
return;
}
- header.ws.b0 = (1 << QIO_CHANNEL_WEBSOCK_HEADER_SHIFT_FIN) |
+ header.ws.b0 = QIO_CHANNEL_WEBSOCK_HEADER_FIELD_FIN |
(QIO_CHANNEL_WEBSOCK_OPCODE_BINARY_FRAME &
QIO_CHANNEL_WEBSOCK_HEADER_FIELD_OPCODE);
if (ioc->rawoutput.offset <
@@ -613,8 +611,8 @@ static void qio_channel_websock_encode(QIOChannelWebsock *ioc)
}
-static ssize_t qio_channel_websock_decode_header(QIOChannelWebsock *ioc,
- Error **errp)
+static int qio_channel_websock_decode_header(QIOChannelWebsock *ioc,
+ Error **errp)
{
unsigned char opcode, fin, has_mask;
size_t header_size;
@@ -633,11 +631,9 @@ static ssize_t qio_channel_websock_decode_header(QIOChannelWebsock *ioc,
return QIO_CHANNEL_ERR_BLOCK;
}
- fin = (header->b0 & QIO_CHANNEL_WEBSOCK_HEADER_FIELD_FIN) >>
- QIO_CHANNEL_WEBSOCK_HEADER_SHIFT_FIN;
+ fin = header->b0 & QIO_CHANNEL_WEBSOCK_HEADER_FIELD_FIN;
opcode = header->b0 & QIO_CHANNEL_WEBSOCK_HEADER_FIELD_OPCODE;
- has_mask = (header->b1 & QIO_CHANNEL_WEBSOCK_HEADER_FIELD_HAS_MASK) >>
- QIO_CHANNEL_WEBSOCK_HEADER_SHIFT_HAS_MASK;
+ has_mask = header->b1 & QIO_CHANNEL_WEBSOCK_HEADER_FIELD_HAS_MASK;
payload_len = header->b1 & QIO_CHANNEL_WEBSOCK_HEADER_FIELD_PAYLOAD_LEN;
if (opcode == QIO_CHANNEL_WEBSOCK_OPCODE_CLOSE) {
@@ -655,7 +651,7 @@ static ssize_t qio_channel_websock_decode_header(QIOChannelWebsock *ioc,
return -1;
}
if (!has_mask) {
- error_setg(errp, "websocket frames must be masked");
+ error_setg(errp, "client websocket frames must be masked");
return -1;
}
if (opcode != QIO_CHANNEL_WEBSOCK_OPCODE_BINARY_FRAME) {
@@ -687,8 +683,8 @@ static ssize_t qio_channel_websock_decode_header(QIOChannelWebsock *ioc,
}
-static ssize_t qio_channel_websock_decode_payload(QIOChannelWebsock *ioc,
- Error **errp)
+static int qio_channel_websock_decode_payload(QIOChannelWebsock *ioc,
+ Error **errp)
{
size_t i;
size_t payload_len;
@@ -729,7 +725,7 @@ static ssize_t qio_channel_websock_decode_payload(QIOChannelWebsock *ioc,
buffer_reserve(&ioc->rawinput, payload_len);
buffer_append(&ioc->rawinput, ioc->encinput.buffer, payload_len);
buffer_advance(&ioc->encinput, payload_len);
- return payload_len;
+ return 0;
}
@@ -809,8 +805,8 @@ static ssize_t qio_channel_websock_read_wire(QIOChannelWebsock *ioc,
if (ret < 0) {
return ret;
}
- if (ret == 0 &&
- ioc->encinput.offset == 0) {
+ if (ret == 0 && ioc->encinput.offset == 0) {
+ ioc->io_eof = TRUE;
return 0;
}
ioc->encinput.offset += ret;
@@ -822,10 +818,6 @@ static ssize_t qio_channel_websock_read_wire(QIOChannelWebsock *ioc,
if (ret < 0) {
return ret;
}
- if (ret == 0) {
- ioc->io_eof = TRUE;
- break;
- }
}
ret = qio_channel_websock_decode_payload(ioc, errp);
@@ -1090,14 +1082,12 @@ struct QIOChannelWebsockSource {
};
static gboolean
-qio_channel_websock_source_prepare(GSource *source,
- gint *timeout)
+qio_channel_websock_source_check(GSource *source)
{
QIOChannelWebsockSource *wsource = (QIOChannelWebsockSource *)source;
GIOCondition cond = 0;
- *timeout = -1;
- if (wsource->wioc->rawinput.offset) {
+ if (wsource->wioc->rawinput.offset || wsource->wioc->io_eof) {
cond |= G_IO_IN;
}
if (wsource->wioc->rawoutput.offset < QIO_CHANNEL_WEBSOCK_MAX_BUFFER) {
@@ -1108,19 +1098,11 @@ qio_channel_websock_source_prepare(GSource *source,
}
static gboolean
-qio_channel_websock_source_check(GSource *source)
+qio_channel_websock_source_prepare(GSource *source,
+ gint *timeout)
{
- QIOChannelWebsockSource *wsource = (QIOChannelWebsockSource *)source;
- GIOCondition cond = 0;
-
- if (wsource->wioc->rawinput.offset) {
- cond |= G_IO_IN;
- }
- if (wsource->wioc->rawoutput.offset < QIO_CHANNEL_WEBSOCK_MAX_BUFFER) {
- cond |= G_IO_OUT;
- }
-
- return cond & wsource->condition;
+ *timeout = -1;
+ return qio_channel_websock_source_check(source);
}
static gboolean
@@ -1130,17 +1112,9 @@ qio_channel_websock_source_dispatch(GSource *source,
{
QIOChannelFunc func = (QIOChannelFunc)callback;
QIOChannelWebsockSource *wsource = (QIOChannelWebsockSource *)source;
- GIOCondition cond = 0;
-
- if (wsource->wioc->rawinput.offset) {
- cond |= G_IO_IN;
- }
- if (wsource->wioc->rawoutput.offset < QIO_CHANNEL_WEBSOCK_MAX_BUFFER) {
- cond |= G_IO_OUT;
- }
return (*func)(QIO_CHANNEL(wsource->wioc),
- (cond & wsource->condition),
+ qio_channel_websock_source_check(source),
user_data);
}
From ff1300e626949fa9850b0f91dc5e8c2cb45b6a88 Mon Sep 17 00:00:00 2001
From: Brandon Carpenter <brandon.carpenter@cypherpath.com>
Date: Tue, 12 Sep 2017 08:21:49 -0700
Subject: [PATCH 06/11] io: Add support for fragmented websocket binary frames
Allows fragmented binary frames by saving the previous opcode. Handles
the case where an intermediary (i.e., web proxy) fragments frames
originally sent unfragmented by the client.
Signed-off-by: Brandon Carpenter <brandon.carpenter@cypherpath.com>
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
include/io/channel-websock.h | 1 +
io/channel-websock.c | 26 ++++++++++++++++++--------
2 files changed, 19 insertions(+), 8 deletions(-)
diff --git a/include/io/channel-websock.h b/include/io/channel-websock.h
index 3c9ff84727..7c896557c5 100644
--- a/include/io/channel-websock.h
+++ b/include/io/channel-websock.h
@@ -65,6 +65,7 @@ struct QIOChannelWebsock {
guint io_tag;
Error *io_err;
gboolean io_eof;
+ uint8_t opcode;
};
/**
diff --git a/io/channel-websock.c b/io/channel-websock.c
index 4e5afb2e95..909d6367f0 100644
--- a/io/channel-websock.c
+++ b/io/channel-websock.c
@@ -636,28 +636,38 @@ static int qio_channel_websock_decode_header(QIOChannelWebsock *ioc,
has_mask = header->b1 & QIO_CHANNEL_WEBSOCK_HEADER_FIELD_HAS_MASK;
payload_len = header->b1 & QIO_CHANNEL_WEBSOCK_HEADER_FIELD_PAYLOAD_LEN;
+ /* Save or restore opcode. */
+ if (opcode) {
+ ioc->opcode = opcode;
+ } else {
+ opcode = ioc->opcode;
+ }
+
if (opcode == QIO_CHANNEL_WEBSOCK_OPCODE_CLOSE) {
/* disconnect */
return 0;
}
/* Websocket frame sanity check:
- * * Websocket fragmentation is not supported.
- * * All websockets frames sent by a client have to be masked.
+ * * Fragmentation is only supported for binary frames.
+ * * All frames sent by a client MUST be masked.
* * Only binary encoding is supported.
*/
if (!fin) {
- error_setg(errp, "websocket fragmentation is not supported");
- return -1;
+ if (opcode != QIO_CHANNEL_WEBSOCK_OPCODE_BINARY_FRAME) {
+ error_setg(errp, "only binary websocket frames may be fragmented");
+ return -1;
+ }
+ } else {
+ if (opcode != QIO_CHANNEL_WEBSOCK_OPCODE_BINARY_FRAME) {
+ error_setg(errp, "only binary websocket frames are supported");
+ return -1;
+ }
}
if (!has_mask) {
error_setg(errp, "client websocket frames must be masked");
return -1;
}
- if (opcode != QIO_CHANNEL_WEBSOCK_OPCODE_BINARY_FRAME) {
- error_setg(errp, "only binary websocket frames are supported");
- return -1;
- }
if (payload_len < QIO_CHANNEL_WEBSOCK_PAYLOAD_LEN_MAGIC_16_BIT) {
ioc->payload_remain = payload_len;
From 3a29640e2cbae9d47b89ffaf98ed358920eb6797 Mon Sep 17 00:00:00 2001
From: Brandon Carpenter <brandon.carpenter@cypherpath.com>
Date: Tue, 12 Sep 2017 08:21:50 -0700
Subject: [PATCH 07/11] io: Allow empty websocket payload
Some browsers send pings/pongs with no payload, so allow empty payloads
instead of closing the connection.
Signed-off-by: Brandon Carpenter <brandon.carpenter@cypherpath.com>
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
io/channel-websock.c | 64 +++++++++++++++++++++-----------------------
1 file changed, 31 insertions(+), 33 deletions(-)
diff --git a/io/channel-websock.c b/io/channel-websock.c
index 909d6367f0..b19b5d96da 100644
--- a/io/channel-websock.c
+++ b/io/channel-websock.c
@@ -697,44 +697,42 @@ static int qio_channel_websock_decode_payload(QIOChannelWebsock *ioc,
Error **errp)
{
size_t i;
- size_t payload_len;
+ size_t payload_len = 0;
uint32_t *payload32;
- if (!ioc->payload_remain) {
- error_setg(errp,
- "Decoding payload but no bytes of payload remain");
- return -1;
+ if (ioc->payload_remain) {
+ /* If we aren't at the end of the payload, then drop
+ * off the last bytes, so we're always multiple of 4
+ * for purpose of unmasking, except at end of payload
+ */
+ if (ioc->encinput.offset < ioc->payload_remain) {
+ payload_len = ioc->encinput.offset - (ioc->encinput.offset % 4);
+ } else {
+ payload_len = ioc->payload_remain;
+ }
+ if (payload_len == 0) {
+ return QIO_CHANNEL_ERR_BLOCK;
+ }
+
+ ioc->payload_remain -= payload_len;
+
+ /* unmask frame */
+ /* process 1 frame (32 bit op) */
+ payload32 = (uint32_t *)ioc->encinput.buffer;
+ for (i = 0; i < payload_len / 4; i++) {
+ payload32[i] ^= ioc->mask.u;
+ }
+ /* process the remaining bytes (if any) */
+ for (i *= 4; i < payload_len; i++) {
+ ioc->encinput.buffer[i] ^= ioc->mask.c[i % 4];
+ }
}
- /* If we aren't at the end of the payload, then drop
- * off the last bytes, so we're always multiple of 4
- * for purpose of unmasking, except at end of payload
- */
- if (ioc->encinput.offset < ioc->payload_remain) {
- payload_len = ioc->encinput.offset - (ioc->encinput.offset % 4);
- } else {
- payload_len = ioc->payload_remain;
+ if (payload_len) {
+ buffer_reserve(&ioc->rawinput, payload_len);
+ buffer_append(&ioc->rawinput, ioc->encinput.buffer, payload_len);
+ buffer_advance(&ioc->encinput, payload_len);
}
- if (payload_len == 0) {
- return QIO_CHANNEL_ERR_BLOCK;
- }
-
- ioc->payload_remain -= payload_len;
-
- /* unmask frame */
- /* process 1 frame (32 bit op) */
- payload32 = (uint32_t *)ioc->encinput.buffer;
- for (i = 0; i < payload_len / 4; i++) {
- payload32[i] ^= ioc->mask.u;
- }
- /* process the remaining bytes (if any) */
- for (i *= 4; i < payload_len; i++) {
- ioc->encinput.buffer[i] ^= ioc->mask.c[i % 4];
- }
-
- buffer_reserve(&ioc->rawinput, payload_len);
- buffer_append(&ioc->rawinput, ioc->encinput.buffer, payload_len);
- buffer_advance(&ioc->encinput, payload_len);
return 0;
}
From 01af17fc002414ee1ac0800babfb0edc2bef1a7d Mon Sep 17 00:00:00 2001
From: Brandon Carpenter <brandon.carpenter@cypherpath.com>
Date: Tue, 12 Sep 2017 08:21:51 -0700
Subject: [PATCH 08/11] io: Ignore websocket PING and PONG frames
Keep pings and gratuitous pongs generated by web browsers from killing
websocket connections.
Signed-off-by: Brandon Carpenter <brandon.carpenter@cypherpath.com>
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
io/channel-websock.c | 21 +++++++++++++++++----
1 file changed, 17 insertions(+), 4 deletions(-)
diff --git a/io/channel-websock.c b/io/channel-websock.c
index b19b5d96da..bfe4008d83 100644
--- a/io/channel-websock.c
+++ b/io/channel-websock.c
@@ -115,6 +115,7 @@
#define QIO_CHANNEL_WEBSOCK_HEADER_FIELD_OPCODE 0x0f
#define QIO_CHANNEL_WEBSOCK_HEADER_FIELD_HAS_MASK 0x80
#define QIO_CHANNEL_WEBSOCK_HEADER_FIELD_PAYLOAD_LEN 0x7f
+#define QIO_CHANNEL_WEBSOCK_CONTROL_OPCODE_MASK 0x8
typedef struct QIOChannelWebsockHeader QIOChannelWebsockHeader;
@@ -659,8 +660,11 @@ static int qio_channel_websock_decode_header(QIOChannelWebsock *ioc,
return -1;
}
} else {
- if (opcode != QIO_CHANNEL_WEBSOCK_OPCODE_BINARY_FRAME) {
- error_setg(errp, "only binary websocket frames are supported");
+ if (opcode != QIO_CHANNEL_WEBSOCK_OPCODE_BINARY_FRAME &&
+ opcode != QIO_CHANNEL_WEBSOCK_OPCODE_PING &&
+ opcode != QIO_CHANNEL_WEBSOCK_OPCODE_PONG) {
+ error_setg(errp, "unsupported opcode: %#04x; only binary, ping, "
+ "and pong websocket frames are supported", opcode);
return -1;
}
}
@@ -673,6 +677,9 @@ static int qio_channel_websock_decode_header(QIOChannelWebsock *ioc,
ioc->payload_remain = payload_len;
header_size = QIO_CHANNEL_WEBSOCK_HEADER_LEN_7_BIT;
ioc->mask = header->u.m;
+ } else if (opcode & QIO_CHANNEL_WEBSOCK_CONTROL_OPCODE_MASK) {
+ error_setg(errp, "websocket control frame is too large");
+ return -1;
} else if (payload_len == QIO_CHANNEL_WEBSOCK_PAYLOAD_LEN_MAGIC_16_BIT &&
ioc->encinput.offset >= QIO_CHANNEL_WEBSOCK_HEADER_LEN_16_BIT) {
ioc->payload_remain = be16_to_cpu(header->u.s16.l16);
@@ -728,9 +735,15 @@ static int qio_channel_websock_decode_payload(QIOChannelWebsock *ioc,
}
}
+ /* Drop the payload of ping/pong packets */
+ if (ioc->opcode == QIO_CHANNEL_WEBSOCK_OPCODE_BINARY_FRAME) {
+ if (payload_len) {
+ buffer_reserve(&ioc->rawinput, payload_len);
+ buffer_append(&ioc->rawinput, ioc->encinput.buffer, payload_len);
+ }
+ }
+
if (payload_len) {
- buffer_reserve(&ioc->rawinput, payload_len);
- buffer_append(&ioc->rawinput, ioc->encinput.buffer, payload_len);
buffer_advance(&ioc->encinput, payload_len);
}
return 0;
From 268a53f50de795481dd73ffd0e0c1339ad3dc44b Mon Sep 17 00:00:00 2001
From: Brandon Carpenter <brandon.carpenter@cypherpath.com>
Date: Tue, 12 Sep 2017 08:21:52 -0700
Subject: [PATCH 09/11] io: Reply to ping frames
Add an immediate ping reply (pong) to the outgoing stream when a ping
is received. Unsolicited pongs are ignored.
Signed-off-by: Brandon Carpenter <brandon.carpenter@cypherpath.com>
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
include/io/channel-websock.h | 1 +
io/channel-websock.c | 66 ++++++++++++++++++++++++------------
2 files changed, 45 insertions(+), 22 deletions(-)
diff --git a/include/io/channel-websock.h b/include/io/channel-websock.h
index 7c896557c5..ff32d8651b 100644
--- a/include/io/channel-websock.h
+++ b/include/io/channel-websock.h
@@ -60,6 +60,7 @@ struct QIOChannelWebsock {
Buffer encoutput;
Buffer rawinput;
Buffer rawoutput;
+ Buffer ping_reply;
size_t payload_remain;
QIOChannelWebsockMask mask;
guint io_tag;
diff --git a/io/channel-websock.c b/io/channel-websock.c
index bfe4008d83..b6fc0c9b8e 100644
--- a/io/channel-websock.c
+++ b/io/channel-websock.c
@@ -573,7 +573,8 @@ static gboolean qio_channel_websock_handshake_io(QIOChannel *ioc,
}
-static void qio_channel_websock_encode(QIOChannelWebsock *ioc)
+static void qio_channel_websock_encode_buffer(Buffer *output,
+ uint8_t opcode, Buffer *buffer)
{
size_t header_size;
union {
@@ -581,33 +582,37 @@ static void qio_channel_websock_encode(QIOChannelWebsock *ioc)
QIOChannelWebsockHeader ws;
} header;
- if (!ioc->rawoutput.offset) {
- return;
- }
-
header.ws.b0 = QIO_CHANNEL_WEBSOCK_HEADER_FIELD_FIN |
- (QIO_CHANNEL_WEBSOCK_OPCODE_BINARY_FRAME &
- QIO_CHANNEL_WEBSOCK_HEADER_FIELD_OPCODE);
- if (ioc->rawoutput.offset <
- QIO_CHANNEL_WEBSOCK_PAYLOAD_LEN_THRESHOLD_7_BIT) {
- header.ws.b1 = (uint8_t)ioc->rawoutput.offset;
+ (opcode & QIO_CHANNEL_WEBSOCK_HEADER_FIELD_OPCODE);
+ if (buffer->offset < QIO_CHANNEL_WEBSOCK_PAYLOAD_LEN_THRESHOLD_7_BIT) {
+ header.ws.b1 = (uint8_t)buffer->offset;
header_size = QIO_CHANNEL_WEBSOCK_HEADER_LEN_7_BIT;
- } else if (ioc->rawoutput.offset <
+ } else if (buffer->offset <
QIO_CHANNEL_WEBSOCK_PAYLOAD_LEN_THRESHOLD_16_BIT) {
header.ws.b1 = QIO_CHANNEL_WEBSOCK_PAYLOAD_LEN_MAGIC_16_BIT;
- header.ws.u.s16.l16 = cpu_to_be16((uint16_t)ioc->rawoutput.offset);
+ header.ws.u.s16.l16 = cpu_to_be16((uint16_t)buffer->offset);
header_size = QIO_CHANNEL_WEBSOCK_HEADER_LEN_16_BIT;
} else {
header.ws.b1 = QIO_CHANNEL_WEBSOCK_PAYLOAD_LEN_MAGIC_64_BIT;
- header.ws.u.s64.l64 = cpu_to_be64(ioc->rawoutput.offset);
+ header.ws.u.s64.l64 = cpu_to_be64(buffer->offset);
header_size = QIO_CHANNEL_WEBSOCK_HEADER_LEN_64_BIT;
}
header_size -= QIO_CHANNEL_WEBSOCK_HEADER_LEN_MASK;
- buffer_reserve(&ioc->encoutput, header_size + ioc->rawoutput.offset);
- buffer_append(&ioc->encoutput, header.buf, header_size);
- buffer_append(&ioc->encoutput, ioc->rawoutput.buffer,
- ioc->rawoutput.offset);
+ buffer_reserve(output, header_size + buffer->offset);
+ buffer_append(output, header.buf, header_size);
+ buffer_append(output, buffer->buffer, buffer->offset);
+}
+
+
+static void qio_channel_websock_encode(QIOChannelWebsock *ioc)
+{
+ if (!ioc->rawoutput.offset) {
+ return;
+ }
+ qio_channel_websock_encode_buffer(
+ &ioc->encoutput, QIO_CHANNEL_WEBSOCK_OPCODE_BINARY_FRAME,
+ &ioc->rawoutput);
buffer_reset(&ioc->rawoutput);
}
@@ -652,7 +657,7 @@ static int qio_channel_websock_decode_header(QIOChannelWebsock *ioc,
/* Websocket frame sanity check:
* * Fragmentation is only supported for binary frames.
* * All frames sent by a client MUST be masked.
- * * Only binary encoding is supported.
+ * * Only binary and ping/pong encoding is supported.
*/
if (!fin) {
if (opcode != QIO_CHANNEL_WEBSOCK_OPCODE_BINARY_FRAME) {
@@ -713,6 +718,11 @@ static int qio_channel_websock_decode_payload(QIOChannelWebsock *ioc,
* for purpose of unmasking, except at end of payload
*/
if (ioc->encinput.offset < ioc->payload_remain) {
+ /* Wait for the entire payload before processing control frames
+ * because the payload will most likely be echoed back. */
+ if (ioc->opcode & QIO_CHANNEL_WEBSOCK_CONTROL_OPCODE_MASK) {
+ return QIO_CHANNEL_ERR_BLOCK;
+ }
payload_len = ioc->encinput.offset - (ioc->encinput.offset % 4);
} else {
payload_len = ioc->payload_remain;
@@ -735,13 +745,18 @@ static int qio_channel_websock_decode_payload(QIOChannelWebsock *ioc,
}
}
- /* Drop the payload of ping/pong packets */
if (ioc->opcode == QIO_CHANNEL_WEBSOCK_OPCODE_BINARY_FRAME) {
if (payload_len) {
+ /* binary frames are passed on */
buffer_reserve(&ioc->rawinput, payload_len);
buffer_append(&ioc->rawinput, ioc->encinput.buffer, payload_len);
}
- }
+ } else if (ioc->opcode == QIO_CHANNEL_WEBSOCK_OPCODE_PING) {
+ /* ping frames produce an immediate reply */
+ buffer_reset(&ioc->ping_reply);
+ qio_channel_websock_encode_buffer(
+ &ioc->ping_reply, QIO_CHANNEL_WEBSOCK_OPCODE_PONG, &ioc->encinput);
+ } /* pong frames are ignored */
if (payload_len) {
buffer_advance(&ioc->encinput, payload_len);
@@ -799,6 +814,7 @@ static void qio_channel_websock_finalize(Object *obj)
buffer_free(&ioc->encoutput);
buffer_free(&ioc->rawinput);
buffer_free(&ioc->rawoutput);
+ buffer_free(&ioc->ping_reply);
object_unref(OBJECT(ioc->master));
if (ioc->io_tag) {
g_source_remove(ioc->io_tag);
@@ -855,7 +871,13 @@ static ssize_t qio_channel_websock_write_wire(QIOChannelWebsock *ioc,
{
ssize_t ret;
ssize_t done = 0;
- qio_channel_websock_encode(ioc);
+
+ /* ping replies take priority over binary data */
+ if (!ioc->ping_reply.offset) {
+ qio_channel_websock_encode(ioc);
+ } else if (!ioc->encoutput.offset) {
+ buffer_move_empty(&ioc->encoutput, &ioc->ping_reply);
+ }
while (ioc->encoutput.offset > 0) {
ret = qio_channel_write(ioc->master,
@@ -930,7 +952,7 @@ static void qio_channel_websock_set_watch(QIOChannelWebsock *ioc)
return;
}
- if (ioc->encoutput.offset) {
+ if (ioc->encoutput.offset || ioc->ping_reply.offset) {
cond |= G_IO_OUT;
}
if (ioc->encinput.offset < QIO_CHANNEL_WEBSOCK_MAX_BUFFER &&
From 530ca60c16c83435d4becc9916d74fa43e003815 Mon Sep 17 00:00:00 2001
From: Brandon Carpenter <brandon.carpenter@cypherpath.com>
Date: Tue, 12 Sep 2017 08:21:53 -0700
Subject: [PATCH 10/11] io: Attempt to send websocket close messages to client
Make a best effort attempt to close websocket connections according to
the RFC. Sends the close message, as room permits in the socket buffer,
and immediately closes the socket.
Signed-off-by: Brandon Carpenter <brandon.carpenter@cypherpath.com>
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
io/channel-websock.c | 68 ++++++++++++++++++++++++++++++++++++++++++--
1 file changed, 65 insertions(+), 3 deletions(-)
diff --git a/io/channel-websock.c b/io/channel-websock.c
index b6fc0c9b8e..3195eb2eb8 100644
--- a/io/channel-websock.c
+++ b/io/channel-websock.c
@@ -188,6 +188,15 @@ static void qio_channel_websock_handshake_send_res_err(QIOChannelWebsock *ioc,
g_free(date);
}
+enum {
+ QIO_CHANNEL_WEBSOCK_STATUS_NORMAL = 1000,
+ QIO_CHANNEL_WEBSOCK_STATUS_PROTOCOL_ERR = 1002,
+ QIO_CHANNEL_WEBSOCK_STATUS_INVALID_DATA = 1003,
+ QIO_CHANNEL_WEBSOCK_STATUS_POLICY = 1008,
+ QIO_CHANNEL_WEBSOCK_STATUS_TOO_LARGE = 1009,
+ QIO_CHANNEL_WEBSOCK_STATUS_SERVER_ERR = 1011,
+};
+
static size_t
qio_channel_websock_extract_headers(QIOChannelWebsock *ioc,
char *buffer,
@@ -617,6 +626,27 @@ static void qio_channel_websock_encode(QIOChannelWebsock *ioc)
}
+static ssize_t qio_channel_websock_write_wire(QIOChannelWebsock *, Error **);
+
+
+static void qio_channel_websock_write_close(QIOChannelWebsock *ioc,
+ uint16_t code, const char *reason)
+{
+ buffer_reserve(&ioc->rawoutput, 2 + (reason ? strlen(reason) : 0));
+ *(uint16_t *)(ioc->rawoutput.buffer + ioc->rawoutput.offset) =
+ cpu_to_be16(code);
+ ioc->rawoutput.offset += 2;
+ if (reason) {
+ buffer_append(&ioc->rawoutput, reason, strlen(reason));
+ }
+ qio_channel_websock_encode_buffer(
+ &ioc->encoutput, QIO_CHANNEL_WEBSOCK_OPCODE_CLOSE, &ioc->rawoutput);
+ buffer_reset(&ioc->rawoutput);
+ qio_channel_websock_write_wire(ioc, NULL);
+ qio_channel_shutdown(ioc->master, QIO_CHANNEL_SHUTDOWN_BOTH, NULL);
+}
+
+
static int qio_channel_websock_decode_header(QIOChannelWebsock *ioc,
Error **errp)
{
@@ -630,6 +660,9 @@ static int qio_channel_websock_decode_header(QIOChannelWebsock *ioc,
error_setg(errp,
"Decoding header but %zu bytes of payload remain",
ioc->payload_remain);
+ qio_channel_websock_write_close(
+ ioc, QIO_CHANNEL_WEBSOCK_STATUS_SERVER_ERR,
+ "internal server error");
return -1;
}
if (ioc->encinput.offset < QIO_CHANNEL_WEBSOCK_HEADER_LEN_7_BIT) {
@@ -662,19 +695,29 @@ static int qio_channel_websock_decode_header(QIOChannelWebsock *ioc,
if (!fin) {
if (opcode != QIO_CHANNEL_WEBSOCK_OPCODE_BINARY_FRAME) {
error_setg(errp, "only binary websocket frames may be fragmented");
+ qio_channel_websock_write_close(
+ ioc, QIO_CHANNEL_WEBSOCK_STATUS_POLICY ,
+ "only binary frames may be fragmented");
return -1;
}
} else {
if (opcode != QIO_CHANNEL_WEBSOCK_OPCODE_BINARY_FRAME &&
+ opcode != QIO_CHANNEL_WEBSOCK_OPCODE_CLOSE &&
opcode != QIO_CHANNEL_WEBSOCK_OPCODE_PING &&
opcode != QIO_CHANNEL_WEBSOCK_OPCODE_PONG) {
- error_setg(errp, "unsupported opcode: %#04x; only binary, ping, "
- "and pong websocket frames are supported", opcode);
+ error_setg(errp, "unsupported opcode: %#04x; only binary, close, "
+ "ping, and pong websocket frames are supported", opcode);
+ qio_channel_websock_write_close(
+ ioc, QIO_CHANNEL_WEBSOCK_STATUS_INVALID_DATA ,
+ "only binary, close, ping, and pong frames are supported");
return -1;
}
}
if (!has_mask) {
error_setg(errp, "client websocket frames must be masked");
+ qio_channel_websock_write_close(
+ ioc, QIO_CHANNEL_WEBSOCK_STATUS_PROTOCOL_ERR,
+ "client frames must be masked");
return -1;
}
@@ -684,6 +727,9 @@ static int qio_channel_websock_decode_header(QIOChannelWebsock *ioc,
ioc->mask = header->u.m;
} else if (opcode & QIO_CHANNEL_WEBSOCK_CONTROL_OPCODE_MASK) {
error_setg(errp, "websocket control frame is too large");
+ qio_channel_websock_write_close(
+ ioc, QIO_CHANNEL_WEBSOCK_STATUS_PROTOCOL_ERR,
+ "control frame is too large");
return -1;
} else if (payload_len == QIO_CHANNEL_WEBSOCK_PAYLOAD_LEN_MAGIC_16_BIT &&
ioc->encinput.offset >= QIO_CHANNEL_WEBSOCK_HEADER_LEN_16_BIT) {
@@ -701,7 +747,7 @@ static int qio_channel_websock_decode_header(QIOChannelWebsock *ioc,
}
buffer_advance(&ioc->encinput, header_size);
- return 1;
+ return 0;
}
@@ -751,6 +797,22 @@ static int qio_channel_websock_decode_payload(QIOChannelWebsock *ioc,
buffer_reserve(&ioc->rawinput, payload_len);
buffer_append(&ioc->rawinput, ioc->encinput.buffer, payload_len);
}
+ } else if (ioc->opcode == QIO_CHANNEL_WEBSOCK_OPCODE_CLOSE) {
+ /* close frames are echoed back */
+ error_setg(errp, "websocket closed by peer");
+ if (payload_len) {
+ /* echo client status */
+ qio_channel_websock_encode_buffer(
+ &ioc->encoutput, QIO_CHANNEL_WEBSOCK_OPCODE_CLOSE,
+ &ioc->encinput);
+ qio_channel_websock_write_wire(ioc, NULL);
+ qio_channel_shutdown(ioc->master, QIO_CHANNEL_SHUTDOWN_BOTH, NULL);
+ } else {
+ /* send our own status */
+ qio_channel_websock_write_close(
+ ioc, QIO_CHANNEL_WEBSOCK_STATUS_NORMAL, "peer requested close");
+ }
+ return -1;
} else if (ioc->opcode == QIO_CHANNEL_WEBSOCK_OPCODE_PING) {
/* ping frames produce an immediate reply */
buffer_reset(&ioc->ping_reply);
From 59f183bbd54eecffb8915bffe03f9c2720b28bcc Mon Sep 17 00:00:00 2001
From: "Daniel P. Berrange" <berrange@redhat.com>
Date: Thu, 21 Sep 2017 11:00:47 +0100
Subject: [PATCH 11/11] io: add trace events for websockets frame handling
It is useful to trace websockets frame encoding/decoding when debugging
problems.
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
io/channel-websock.c | 23 ++++++++++++++++++-----
io/trace-events | 5 +++++
2 files changed, 23 insertions(+), 5 deletions(-)
diff --git a/io/channel-websock.c b/io/channel-websock.c
index 3195eb2eb8..d1d471f86e 100644
--- a/io/channel-websock.c
+++ b/io/channel-websock.c
@@ -582,7 +582,8 @@ static gboolean qio_channel_websock_handshake_io(QIOChannel *ioc,
}
-static void qio_channel_websock_encode_buffer(Buffer *output,
+static void qio_channel_websock_encode_buffer(QIOChannelWebsock *ioc,
+ Buffer *output,
uint8_t opcode, Buffer *buffer)
{
size_t header_size;
@@ -608,6 +609,7 @@ static void qio_channel_websock_encode_buffer(Buffer *output,
}
header_size -= QIO_CHANNEL_WEBSOCK_HEADER_LEN_MASK;
+ trace_qio_channel_websock_encode(ioc, opcode, header_size, buffer->offset);
buffer_reserve(output, header_size + buffer->offset);
buffer_append(output, header.buf, header_size);
buffer_append(output, buffer->buffer, buffer->offset);
@@ -620,7 +622,7 @@ static void qio_channel_websock_encode(QIOChannelWebsock *ioc)
return;
}
qio_channel_websock_encode_buffer(
- &ioc->encoutput, QIO_CHANNEL_WEBSOCK_OPCODE_BINARY_FRAME,
+ ioc, &ioc->encoutput, QIO_CHANNEL_WEBSOCK_OPCODE_BINARY_FRAME,
&ioc->rawoutput);
buffer_reset(&ioc->rawoutput);
}
@@ -640,7 +642,8 @@ static void qio_channel_websock_write_close(QIOChannelWebsock *ioc,
buffer_append(&ioc->rawoutput, reason, strlen(reason));
}
qio_channel_websock_encode_buffer(
- &ioc->encoutput, QIO_CHANNEL_WEBSOCK_OPCODE_CLOSE, &ioc->rawoutput);
+ ioc, &ioc->encoutput, QIO_CHANNEL_WEBSOCK_OPCODE_CLOSE,
+ &ioc->rawoutput);
buffer_reset(&ioc->rawoutput);
qio_channel_websock_write_wire(ioc, NULL);
qio_channel_shutdown(ioc->master, QIO_CHANNEL_SHUTDOWN_BOTH, NULL);
@@ -682,6 +685,9 @@ static int qio_channel_websock_decode_header(QIOChannelWebsock *ioc,
opcode = ioc->opcode;
}
+ trace_qio_channel_websock_header_partial_decode(ioc, payload_len,
+ fin, opcode, (int)has_mask);
+
if (opcode == QIO_CHANNEL_WEBSOCK_OPCODE_CLOSE) {
/* disconnect */
return 0;
@@ -746,6 +752,8 @@ static int qio_channel_websock_decode_header(QIOChannelWebsock *ioc,
return QIO_CHANNEL_ERR_BLOCK;
}
+ trace_qio_channel_websock_header_full_decode(
+ ioc, header_size, ioc->payload_remain, ioc->mask.u);
buffer_advance(&ioc->encinput, header_size);
return 0;
}
@@ -791,6 +799,9 @@ static int qio_channel_websock_decode_payload(QIOChannelWebsock *ioc,
}
}
+ trace_qio_channel_websock_payload_decode(
+ ioc, ioc->opcode, ioc->payload_remain);
+
if (ioc->opcode == QIO_CHANNEL_WEBSOCK_OPCODE_BINARY_FRAME) {
if (payload_len) {
/* binary frames are passed on */
@@ -803,7 +814,7 @@ static int qio_channel_websock_decode_payload(QIOChannelWebsock *ioc,
if (payload_len) {
/* echo client status */
qio_channel_websock_encode_buffer(
- &ioc->encoutput, QIO_CHANNEL_WEBSOCK_OPCODE_CLOSE,
+ ioc, &ioc->encoutput, QIO_CHANNEL_WEBSOCK_OPCODE_CLOSE,
&ioc->encinput);
qio_channel_websock_write_wire(ioc, NULL);
qio_channel_shutdown(ioc->master, QIO_CHANNEL_SHUTDOWN_BOTH, NULL);
@@ -817,7 +828,8 @@ static int qio_channel_websock_decode_payload(QIOChannelWebsock *ioc,
/* ping frames produce an immediate reply */
buffer_reset(&ioc->ping_reply);
qio_channel_websock_encode_buffer(
- &ioc->ping_reply, QIO_CHANNEL_WEBSOCK_OPCODE_PONG, &ioc->encinput);
+ ioc, &ioc->ping_reply, QIO_CHANNEL_WEBSOCK_OPCODE_PONG,
+ &ioc->encinput);
} /* pong frames are ignored */
if (payload_len) {
@@ -1176,6 +1188,7 @@ static int qio_channel_websock_close(QIOChannel *ioc,
{
QIOChannelWebsock *wioc = QIO_CHANNEL_WEBSOCK(ioc);
+ trace_qio_channel_websock_close(ioc);
return qio_channel_close(wioc->master, errp);
}
diff --git a/io/trace-events b/io/trace-events
index 6459f71f5b..801b5dcb61 100644
--- a/io/trace-events
+++ b/io/trace-events
@@ -48,6 +48,11 @@ qio_channel_websock_handshake_pending(void *ioc, int status) "Websock handshake
qio_channel_websock_handshake_reply(void *ioc) "Websock handshake reply ioc=%p"
qio_channel_websock_handshake_fail(void *ioc, const char *msg) "Websock handshake fail ioc=%p err=%s"
qio_channel_websock_handshake_complete(void *ioc) "Websock handshake complete ioc=%p"
+qio_channel_websock_header_partial_decode(void *ioc, size_t payloadlen, unsigned char fin, unsigned char opcode, unsigned char has_mask) "Websocket header decoded ioc=%p payload-len=%zu fin=0x%x opcode=0x%x has_mask=0x%x"
+qio_channel_websock_header_full_decode(void *ioc, size_t headerlen, size_t payloadlen, uint32_t mask) "Websocket header decoded ioc=%p header-len=%zu payload-len=%zu mask=0x%x"
+qio_channel_websock_payload_decode(void *ioc, uint8_t opcode, size_t payload_remain) "Websocket header decoded ioc=%p opcode=0x%x payload-remain=%zu"
+qio_channel_websock_encode(void *ioc, uint8_t opcode, size_t payloadlen, size_t headerlen) "Websocket encoded ioc=%p opcode=0x%x header-len=%zu payload-len=%zu"
+qio_channel_websock_close(void *ioc) "Websocket close ioc=%p"
# io/channel-command.c
qio_channel_command_new_pid(void *ioc, int writefd, int readfd, int pid) "Command new pid ioc=%p writefd=%d readfd=%d pid=%d"