Blame SOURCES/kvm-io-send-proper-HTTP-response-for-websocket-errors.patch

9bac43
From 062b6fd62b387f4ea67f800ca956c87b28451a9c Mon Sep 17 00:00:00 2001
9bac43
From: "Daniel P. Berrange" <berrange@redhat.com>
9bac43
Date: Wed, 20 Dec 2017 17:56:43 +0100
9bac43
Subject: [PATCH 03/42] io: send proper HTTP response for websocket errors
9bac43
MIME-Version: 1.0
9bac43
Content-Type: text/plain; charset=UTF-8
9bac43
Content-Transfer-Encoding: 8bit
9bac43
9bac43
RH-Author: Daniel P. Berrange <berrange@redhat.com>
9bac43
Message-id: <20171220175702.29663-2-berrange@redhat.com>
9bac43
Patchwork-id: 78454
9bac43
O-Subject: [RHV-7.5 qemu-kvm-rhev PATCH v2 01/20] io: send proper HTTP response for websocket errors
9bac43
Bugzilla: 1518649
9bac43
RH-Acked-by: John Snow <jsnow@redhat.com>
9bac43
RH-Acked-by: Jeffrey Cody <jcody@redhat.com>
9bac43
RH-Acked-by: Miroslav Rezanina <mrezanin@redhat.com>
9bac43
9bac43
When any error occurs while processing the websockets handshake,
9bac43
QEMU just terminates the connection abruptly. This is in violation
9bac43
of the HTTP specs and does not help the client understand what they
9bac43
did wrong. This is particularly bad when the client gives the wrong
9bac43
path, as a "404 Not Found" would be very helpful.
9bac43
9bac43
Refactor the handshake code so that it always sends a response to
9bac43
the client unless there was an I/O error.
9bac43
9bac43
Fixes bug: #1715186
9bac43
9bac43
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
9bac43
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
9bac43
(cherry picked from commit f69a8bde29354493ff8aea64cc9cb3b531d16337)
9bac43
Signed-off-by: Miroslav Rezanina <mrezanin@redhat.com>
9bac43
---
9bac43
 io/channel-websock.c | 185 ++++++++++++++++++++++++++++++++++++++-------------
9bac43
 1 file changed, 139 insertions(+), 46 deletions(-)
9bac43
9bac43
diff --git a/io/channel-websock.c b/io/channel-websock.c
9bac43
index 5a3badb..f5fac5b 100644
9bac43
--- a/io/channel-websock.c
9bac43
+++ b/io/channel-websock.c
9bac43
@@ -25,6 +25,8 @@
9bac43
 #include "crypto/hash.h"
9bac43
 #include "trace.h"
9bac43
 
9bac43
+#include <time.h>
9bac43
+
9bac43
 
9bac43
 /* Max amount to allow in rawinput/rawoutput buffers */
9bac43
 #define QIO_CHANNEL_WEBSOCK_MAX_BUFFER 8192
9bac43
@@ -44,13 +46,40 @@
9bac43
 #define QIO_CHANNEL_WEBSOCK_CONNECTION_UPGRADE "Upgrade"
9bac43
 #define QIO_CHANNEL_WEBSOCK_UPGRADE_WEBSOCKET "websocket"
9bac43
 
9bac43
-#define QIO_CHANNEL_WEBSOCK_HANDSHAKE_RESPONSE  \
9bac43
+#define QIO_CHANNEL_WEBSOCK_HANDSHAKE_RES_COMMON \
9bac43
+    "Server: QEMU VNC\r\n"                       \
9bac43
+    "Date: %s\r\n"
9bac43
+
9bac43
+#define QIO_CHANNEL_WEBSOCK_HANDSHAKE_RES_OK    \
9bac43
     "HTTP/1.1 101 Switching Protocols\r\n"      \
9bac43
+    QIO_CHANNEL_WEBSOCK_HANDSHAKE_RES_COMMON    \
9bac43
     "Upgrade: websocket\r\n"                    \
9bac43
     "Connection: Upgrade\r\n"                   \
9bac43
     "Sec-WebSocket-Accept: %s\r\n"              \
9bac43
     "Sec-WebSocket-Protocol: binary\r\n"        \
9bac43
     "\r\n"
9bac43
+#define QIO_CHANNEL_WEBSOCK_HANDSHAKE_RES_NOT_FOUND \
9bac43
+    "HTTP/1.1 404 Not Found\r\n"                    \
9bac43
+    QIO_CHANNEL_WEBSOCK_HANDSHAKE_RES_COMMON        \
9bac43
+    "Connection: close\r\n"                         \
9bac43
+    "\r\n"
9bac43
+#define QIO_CHANNEL_WEBSOCK_HANDSHAKE_RES_BAD_REQUEST \
9bac43
+    "HTTP/1.1 400 Bad Request\r\n"                    \
9bac43
+    QIO_CHANNEL_WEBSOCK_HANDSHAKE_RES_COMMON          \
9bac43
+    "Connection: close\r\n"                           \
9bac43
+    "Sec-WebSocket-Version: "                         \
9bac43
+    QIO_CHANNEL_WEBSOCK_SUPPORTED_VERSION             \
9bac43
+    "\r\n"
9bac43
+#define QIO_CHANNEL_WEBSOCK_HANDSHAKE_RES_SERVER_ERR \
9bac43
+    "HTTP/1.1 500 Internal Server Error\r\n"         \
9bac43
+    QIO_CHANNEL_WEBSOCK_HANDSHAKE_RES_COMMON         \
9bac43
+    "Connection: close\r\n"                          \
9bac43
+    "\r\n"
9bac43
+#define QIO_CHANNEL_WEBSOCK_HANDSHAKE_RES_TOO_LARGE  \
9bac43
+    "HTTP/1.1 403 Request Entity Too Large\r\n"      \
9bac43
+    QIO_CHANNEL_WEBSOCK_HANDSHAKE_RES_COMMON         \
9bac43
+    "Connection: close\r\n"                          \
9bac43
+    "\r\n"
9bac43
 #define QIO_CHANNEL_WEBSOCK_HANDSHAKE_DELIM "\r\n"
9bac43
 #define QIO_CHANNEL_WEBSOCK_HANDSHAKE_END "\r\n\r\n"
9bac43
 #define QIO_CHANNEL_WEBSOCK_SUPPORTED_VERSION "13"
9bac43
@@ -123,8 +152,46 @@ enum {
9bac43
     QIO_CHANNEL_WEBSOCK_OPCODE_PONG = 0xA
9bac43
 };
9bac43
 
9bac43
+static void qio_channel_websock_handshake_send_res(QIOChannelWebsock *ioc,
9bac43
+                                                   const char *resmsg,
9bac43
+                                                   ...)
9bac43
+{
9bac43
+    va_list vargs;
9bac43
+    char *response;
9bac43
+    size_t responselen;
9bac43
+
9bac43
+    va_start(vargs, resmsg);
9bac43
+    response = g_strdup_vprintf(resmsg, vargs);
9bac43
+    responselen = strlen(response);
9bac43
+    buffer_reserve(&ioc->encoutput, responselen);
9bac43
+    buffer_append(&ioc->encoutput, response, responselen);
9bac43
+    va_end(vargs);
9bac43
+}
9bac43
+
9bac43
+static gchar *qio_channel_websock_date_str(void)
9bac43
+{
9bac43
+    struct tm tm;
9bac43
+    time_t now = time(NULL);
9bac43
+    char datebuf[128];
9bac43
+
9bac43
+    gmtime_r(&now, &tm;;
9bac43
+
9bac43
+    strftime(datebuf, sizeof(datebuf), "%a, %d %b %Y %H:%M:%S GMT", &tm;;
9bac43
+
9bac43
+    return g_strdup(datebuf);
9bac43
+}
9bac43
+
9bac43
+static void qio_channel_websock_handshake_send_res_err(QIOChannelWebsock *ioc,
9bac43
+                                                       const char *resdata)
9bac43
+{
9bac43
+    char *date = qio_channel_websock_date_str();
9bac43
+    qio_channel_websock_handshake_send_res(ioc, resdata, date);
9bac43
+    g_free(date);
9bac43
+}
9bac43
+
9bac43
 static size_t
9bac43
-qio_channel_websock_extract_headers(char *buffer,
9bac43
+qio_channel_websock_extract_headers(QIOChannelWebsock *ioc,
9bac43
+                                    char *buffer,
9bac43
                                     QIOChannelWebsockHTTPHeader *hdrs,
9bac43
                                     size_t nhdrsalloc,
9bac43
                                     Error **errp)
9bac43
@@ -145,7 +212,7 @@ qio_channel_websock_extract_headers(char *buffer,
9bac43
     nl = strstr(buffer, QIO_CHANNEL_WEBSOCK_HANDSHAKE_DELIM);
9bac43
     if (!nl) {
9bac43
         error_setg(errp, "Missing HTTP header delimiter");
9bac43
-        return 0;
9bac43
+        goto bad_request;
9bac43
     }
9bac43
     *nl = '\0';
9bac43
 
9bac43
@@ -158,18 +225,20 @@ qio_channel_websock_extract_headers(char *buffer,
9bac43
 
9bac43
     if (!g_str_equal(buffer, QIO_CHANNEL_WEBSOCK_HTTP_METHOD)) {
9bac43
         error_setg(errp, "Unsupported HTTP method %s", buffer);
9bac43
-        return 0;
9bac43
+        goto bad_request;
9bac43
     }
9bac43
 
9bac43
     buffer = tmp + 1;
9bac43
     tmp = strchr(buffer, ' ');
9bac43
     if (!tmp) {
9bac43
         error_setg(errp, "Missing HTTP version delimiter");
9bac43
-        return 0;
9bac43
+        goto bad_request;
9bac43
     }
9bac43
     *tmp = '\0';
9bac43
 
9bac43
     if (!g_str_equal(buffer, QIO_CHANNEL_WEBSOCK_HTTP_PATH)) {
9bac43
+        qio_channel_websock_handshake_send_res_err(
9bac43
+            ioc, QIO_CHANNEL_WEBSOCK_HANDSHAKE_RES_NOT_FOUND);
9bac43
         error_setg(errp, "Unexpected HTTP path %s", buffer);
9bac43
         return 0;
9bac43
     }
9bac43
@@ -178,7 +247,7 @@ qio_channel_websock_extract_headers(char *buffer,
9bac43
 
9bac43
     if (!g_str_equal(buffer, QIO_CHANNEL_WEBSOCK_HTTP_VERSION)) {
9bac43
         error_setg(errp, "Unsupported HTTP version %s", buffer);
9bac43
-        return 0;
9bac43
+        goto bad_request;
9bac43
     }
9bac43
 
9bac43
     buffer = nl + strlen(QIO_CHANNEL_WEBSOCK_HANDSHAKE_DELIM);
9bac43
@@ -203,7 +272,7 @@ qio_channel_websock_extract_headers(char *buffer,
9bac43
         sep = strchr(buffer, ':');
9bac43
         if (!sep) {
9bac43
             error_setg(errp, "Malformed HTTP header");
9bac43
-            return 0;
9bac43
+            goto bad_request;
9bac43
         }
9bac43
         *sep = '\0';
9bac43
         sep++;
9bac43
@@ -213,7 +282,7 @@ qio_channel_websock_extract_headers(char *buffer,
9bac43
 
9bac43
         if (nhdrs >= nhdrsalloc) {
9bac43
             error_setg(errp, "Too many HTTP headers");
9bac43
-            return 0;
9bac43
+            goto bad_request;
9bac43
         }
9bac43
 
9bac43
         hdr = &hdrs[nhdrs++];
9bac43
@@ -231,6 +300,11 @@ qio_channel_websock_extract_headers(char *buffer,
9bac43
     } while (nl != NULL);
9bac43
 
9bac43
     return nhdrs;
9bac43
+
9bac43
+ bad_request:
9bac43
+    qio_channel_websock_handshake_send_res_err(
9bac43
+        ioc, QIO_CHANNEL_WEBSOCK_HANDSHAKE_RES_BAD_REQUEST);
9bac43
+    return 0;
9bac43
 }
9bac43
 
9bac43
 static const char *
9bac43
@@ -250,14 +324,14 @@ qio_channel_websock_find_header(QIOChannelWebsockHTTPHeader *hdrs,
9bac43
 }
9bac43
 
9bac43
 
9bac43
-static int qio_channel_websock_handshake_send_response(QIOChannelWebsock *ioc,
9bac43
-                                                       const char *key,
9bac43
-                                                       Error **errp)
9bac43
+static void qio_channel_websock_handshake_send_res_ok(QIOChannelWebsock *ioc,
9bac43
+                                                      const char *key,
9bac43
+                                                      Error **errp)
9bac43
 {
9bac43
     char combined_key[QIO_CHANNEL_WEBSOCK_CLIENT_KEY_LEN +
9bac43
                       QIO_CHANNEL_WEBSOCK_GUID_LEN + 1];
9bac43
-    char *accept = NULL, *response = NULL;
9bac43
-    size_t responselen;
9bac43
+    char *accept = NULL;
9bac43
+    char *date = qio_channel_websock_date_str();
9bac43
 
9bac43
     g_strlcpy(combined_key, key, QIO_CHANNEL_WEBSOCK_CLIENT_KEY_LEN + 1);
9bac43
     g_strlcat(combined_key, QIO_CHANNEL_WEBSOCK_GUID,
9bac43
@@ -271,105 +345,108 @@ static int qio_channel_websock_handshake_send_response(QIOChannelWebsock *ioc,
9bac43
                             QIO_CHANNEL_WEBSOCK_GUID_LEN,
9bac43
                             &accept,
9bac43
                             errp) < 0) {
9bac43
-        return -1;
9bac43
+        qio_channel_websock_handshake_send_res_err(
9bac43
+            ioc, QIO_CHANNEL_WEBSOCK_HANDSHAKE_RES_SERVER_ERR);
9bac43
+        return;
9bac43
     }
9bac43
 
9bac43
-    response = g_strdup_printf(QIO_CHANNEL_WEBSOCK_HANDSHAKE_RESPONSE, accept);
9bac43
-    responselen = strlen(response);
9bac43
-    buffer_reserve(&ioc->encoutput, responselen);
9bac43
-    buffer_append(&ioc->encoutput, response, responselen);
9bac43
+    qio_channel_websock_handshake_send_res(
9bac43
+        ioc, QIO_CHANNEL_WEBSOCK_HANDSHAKE_RES_OK, date, accept);
9bac43
 
9bac43
+    g_free(date);
9bac43
     g_free(accept);
9bac43
-    g_free(response);
9bac43
-
9bac43
-    return 0;
9bac43
 }
9bac43
 
9bac43
-static int qio_channel_websock_handshake_process(QIOChannelWebsock *ioc,
9bac43
-                                                 char *buffer,
9bac43
-                                                 Error **errp)
9bac43
+static void qio_channel_websock_handshake_process(QIOChannelWebsock *ioc,
9bac43
+                                                  char *buffer,
9bac43
+                                                  Error **errp)
9bac43
 {
9bac43
     QIOChannelWebsockHTTPHeader hdrs[32];
9bac43
     size_t nhdrs = G_N_ELEMENTS(hdrs);
9bac43
     const char *protocols = NULL, *version = NULL, *key = NULL,
9bac43
         *host = NULL, *connection = NULL, *upgrade = NULL;
9bac43
 
9bac43
-    nhdrs = qio_channel_websock_extract_headers(buffer, hdrs, nhdrs, errp);
9bac43
+    nhdrs = qio_channel_websock_extract_headers(ioc, buffer, hdrs, nhdrs, errp);
9bac43
     if (!nhdrs) {
9bac43
-        return -1;
9bac43
+        return;
9bac43
     }
9bac43
 
9bac43
     protocols = qio_channel_websock_find_header(
9bac43
         hdrs, nhdrs, QIO_CHANNEL_WEBSOCK_HEADER_PROTOCOL);
9bac43
     if (!protocols) {
9bac43
         error_setg(errp, "Missing websocket protocol header data");
9bac43
-        return -1;
9bac43
+        goto bad_request;
9bac43
     }
9bac43
 
9bac43
     version = qio_channel_websock_find_header(
9bac43
         hdrs, nhdrs, QIO_CHANNEL_WEBSOCK_HEADER_VERSION);
9bac43
     if (!version) {
9bac43
         error_setg(errp, "Missing websocket version header data");
9bac43
-        return -1;
9bac43
+        goto bad_request;
9bac43
     }
9bac43
 
9bac43
     key = qio_channel_websock_find_header(
9bac43
         hdrs, nhdrs, QIO_CHANNEL_WEBSOCK_HEADER_KEY);
9bac43
     if (!key) {
9bac43
         error_setg(errp, "Missing websocket key header data");
9bac43
-        return -1;
9bac43
+        goto bad_request;
9bac43
     }
9bac43
 
9bac43
     host = qio_channel_websock_find_header(
9bac43
         hdrs, nhdrs, QIO_CHANNEL_WEBSOCK_HEADER_HOST);
9bac43
     if (!host) {
9bac43
         error_setg(errp, "Missing websocket host header data");
9bac43
-        return -1;
9bac43
+        goto bad_request;
9bac43
     }
9bac43
 
9bac43
     connection = qio_channel_websock_find_header(
9bac43
         hdrs, nhdrs, QIO_CHANNEL_WEBSOCK_HEADER_CONNECTION);
9bac43
     if (!connection) {
9bac43
         error_setg(errp, "Missing websocket connection header data");
9bac43
-        return -1;
9bac43
+        goto bad_request;
9bac43
     }
9bac43
 
9bac43
     upgrade = qio_channel_websock_find_header(
9bac43
         hdrs, nhdrs, QIO_CHANNEL_WEBSOCK_HEADER_UPGRADE);
9bac43
     if (!upgrade) {
9bac43
         error_setg(errp, "Missing websocket upgrade header data");
9bac43
-        return -1;
9bac43
+        goto bad_request;
9bac43
     }
9bac43
 
9bac43
     if (!g_strrstr(protocols, QIO_CHANNEL_WEBSOCK_PROTOCOL_BINARY)) {
9bac43
         error_setg(errp, "No '%s' protocol is supported by client '%s'",
9bac43
                    QIO_CHANNEL_WEBSOCK_PROTOCOL_BINARY, protocols);
9bac43
-        return -1;
9bac43
+        goto bad_request;
9bac43
     }
9bac43
 
9bac43
     if (!g_str_equal(version, QIO_CHANNEL_WEBSOCK_SUPPORTED_VERSION)) {
9bac43
         error_setg(errp, "Version '%s' is not supported by client '%s'",
9bac43
                    QIO_CHANNEL_WEBSOCK_SUPPORTED_VERSION, version);
9bac43
-        return -1;
9bac43
+        goto bad_request;
9bac43
     }
9bac43
 
9bac43
     if (strlen(key) != QIO_CHANNEL_WEBSOCK_CLIENT_KEY_LEN) {
9bac43
         error_setg(errp, "Key length '%zu' was not as expected '%d'",
9bac43
                    strlen(key), QIO_CHANNEL_WEBSOCK_CLIENT_KEY_LEN);
9bac43
-        return -1;
9bac43
+        goto bad_request;
9bac43
     }
9bac43
 
9bac43
     if (!g_strrstr(connection, QIO_CHANNEL_WEBSOCK_CONNECTION_UPGRADE)) {
9bac43
         error_setg(errp, "No connection upgrade requested '%s'", connection);
9bac43
-        return -1;
9bac43
+        goto bad_request;
9bac43
     }
9bac43
 
9bac43
     if (!g_str_equal(upgrade, QIO_CHANNEL_WEBSOCK_UPGRADE_WEBSOCKET)) {
9bac43
         error_setg(errp, "Incorrect upgrade method '%s'", upgrade);
9bac43
-        return -1;
9bac43
+        goto bad_request;
9bac43
     }
9bac43
 
9bac43
-    return qio_channel_websock_handshake_send_response(ioc, key, errp);
9bac43
+    qio_channel_websock_handshake_send_res_ok(ioc, key, errp);
9bac43
+    return;
9bac43
+
9bac43
+ bad_request:
9bac43
+    qio_channel_websock_handshake_send_res_err(
9bac43
+        ioc, QIO_CHANNEL_WEBSOCK_HANDSHAKE_RES_BAD_REQUEST);
9bac43
 }
9bac43
 
9bac43
 static int qio_channel_websock_handshake_read(QIOChannelWebsock *ioc,
9bac43
@@ -393,20 +470,20 @@ static int qio_channel_websock_handshake_read(QIOChannelWebsock *ioc,
9bac43
                                  QIO_CHANNEL_WEBSOCK_HANDSHAKE_END);
9bac43
     if (!handshake_end) {
9bac43
         if (ioc->encinput.offset >= 4096) {
9bac43
+            qio_channel_websock_handshake_send_res_err(
9bac43
+                ioc, QIO_CHANNEL_WEBSOCK_HANDSHAKE_RES_TOO_LARGE);
9bac43
             error_setg(errp,
9bac43
                        "End of headers not found in first 4096 bytes");
9bac43
-            return -1;
9bac43
+            return 1;
9bac43
         } else {
9bac43
             return 0;
9bac43
         }
9bac43
     }
9bac43
     *handshake_end = '\0';
9bac43
 
9bac43
-    if (qio_channel_websock_handshake_process(ioc,
9bac43
-                                              (char *)ioc->encinput.buffer,
9bac43
-                                              errp) < 0) {
9bac43
-        return -1;
9bac43
-    }
9bac43
+    qio_channel_websock_handshake_process(ioc,
9bac43
+                                          (char *)ioc->encinput.buffer,
9bac43
+                                          errp);
9bac43
 
9bac43
     buffer_advance(&ioc->encinput,
9bac43
                    handshake_end - (char *)ioc->encinput.buffer +
9bac43
@@ -438,8 +515,15 @@ static gboolean qio_channel_websock_handshake_send(QIOChannel *ioc,
9bac43
 
9bac43
     buffer_advance(&wioc->encoutput, ret);
9bac43
     if (wioc->encoutput.offset == 0) {
9bac43
-        trace_qio_channel_websock_handshake_complete(ioc);
9bac43
-        qio_task_complete(task);
9bac43
+        if (wioc->io_err) {
9bac43
+            trace_qio_channel_websock_handshake_fail(ioc);
9bac43
+            qio_task_set_error(task, wioc->io_err);
9bac43
+            wioc->io_err = NULL;
9bac43
+            qio_task_complete(task);
9bac43
+        } else {
9bac43
+            trace_qio_channel_websock_handshake_complete(ioc);
9bac43
+            qio_task_complete(task);
9bac43
+        }
9bac43
         return FALSE;
9bac43
     }
9bac43
     trace_qio_channel_websock_handshake_pending(ioc, G_IO_OUT);
9bac43
@@ -458,6 +542,11 @@ static gboolean qio_channel_websock_handshake_io(QIOChannel *ioc,
9bac43
 
9bac43
     ret = qio_channel_websock_handshake_read(wioc, &err;;
9bac43
     if (ret < 0) {
9bac43
+        /*
9bac43
+         * We only take this path on a fatal I/O error reading from
9bac43
+         * client connection, as most of the time we have an
9bac43
+         * HTTP 4xx err response to send instead
9bac43
+         */
9bac43
         trace_qio_channel_websock_handshake_fail(ioc);
9bac43
         qio_task_set_error(task, err);
9bac43
         qio_task_complete(task);
9bac43
@@ -469,6 +558,10 @@ static gboolean qio_channel_websock_handshake_io(QIOChannel *ioc,
9bac43
         return TRUE;
9bac43
     }
9bac43
 
9bac43
+    if (err) {
9bac43
+        error_propagate(&wioc->io_err, err);
9bac43
+    }
9bac43
+
9bac43
     trace_qio_channel_websock_handshake_reply(ioc);
9bac43
     qio_channel_add_watch(
9bac43
         wioc->master,
9bac43
-- 
9bac43
1.8.3.1
9bac43