Hi Jeremy,
I really like how it works, that you can use the same port value for html5 and
gtk clients.
Not everything was clear to me, so I put some comments inline.
On Fri, 2015-10-30 at 15:52 -0500, Jeremy White wrote:
> We do this by auto detecting the inbound http(s) 'GET' and probing
> for a well formulated WebSocket binary connection, such as used
> by the spice-html5 client. If detected, we implement a set of
> cover functions that abstract the read/write/writev functions,
> in a fashion similar to the SASL implemented.
>
> This includes a limited implementation of the WebSocket protocol,
> sufficient for our purposes.
>
> Signed-off-by: Jeremy White <jwhite@xxxxxxxxxxxxxxx>
> ---
> server/Makefile.am | 2 +
> server/reds.c | 6 +
> server/reds_stream.c | 98 ++++++++++++
> server/reds_stream.h | 2 +
> server/websocket.c | 432
> +++++++++++++++++++++++++++++++++++++++++++++++++++
> server/websocket.h | 62 ++++++++
> 6 files changed, 602 insertions(+)
> create mode 100644 server/websocket.c
> create mode 100644 server/websocket.h
>
> diff --git a/server/Makefile.am b/server/Makefile.am
> index 87288cc..e2f5452 100644
> --- a/server/Makefile.am
> +++ b/server/Makefile.am
> @@ -113,6 +113,8 @@ libspice_server_la_SOURCES = \
> reds-private.h \
> reds_stream.c \
> reds_stream.h \
> + websocket.c \
> + websocket.h \
> reds_sw_canvas.c \
> reds_sw_canvas.h \
> snd_worker.c \
> diff --git a/server/reds.c b/server/reds.c
> index 1f6774e..701ac9d 100644
> --- a/server/reds.c
> +++ b/server/reds.c
> @@ -2172,6 +2172,7 @@ static void reds_handle_link_error(void *opaque, int
> err)
> reds_link_free(link);
> }
>
> +static void reds_handle_new_link(RedLinkInfo *link);
> static void reds_handle_read_header_done(void *opaque)
> {
> RedLinkInfo *link = (RedLinkInfo *)opaque;
> @@ -2182,6 +2183,11 @@ static void reds_handle_read_header_done(void *opaque)
> header->size = GUINT32_FROM_LE(header->size);
>
> if (header->magic != SPICE_MAGIC) {
> + if (reds_stream_is_websocket(link->stream,
> + (guchar *) header, sizeof(*header))) {
> + reds_handle_new_link(link);
> + return;
> + }
This looks hacky, it would be nice to detect that we are dealing with websockets
before reds_handle_read_header_done() is called.
> reds_send_link_error(link, SPICE_LINK_ERR_INVALID_MAGIC);
> reds_link_free(link);
> return;
> diff --git a/server/reds_stream.c b/server/reds_stream.c
> index 3b47391..9f813cf 100644
> --- a/server/reds_stream.c
> +++ b/server/reds_stream.c
> @@ -30,6 +30,7 @@
> #include <sys/socket.h>
>
> #include <glib.h>
> +#include "websocket.h"
>
> #include <openssl/err.h>
>
> @@ -76,6 +77,17 @@ typedef struct RedsSASL {
> } RedsSASL;
> #endif
>
> +typedef struct {
> + int closed;
> +
> + websocket_frame_t read_frame;
> + guint64 write_remainder;
> +
> + ssize_t (*raw_read)(RedsStream *s, void *buf, size_t nbyte);
> + ssize_t (*raw_write)(RedsStream *s, const void *buf, size_t nbyte);
> + ssize_t (*raw_writev)(RedsStream *s, const struct iovec *iov, int
> iovcnt);
> +} RedsWebSocket;
> +
> struct RedsStreamPrivate {
> SSL *ssl;
>
> @@ -85,6 +97,8 @@ struct RedsStreamPrivate {
>
> AsyncRead async_read;
>
> + RedsWebSocket *ws;
> +
> /* life time of info:
> * allocated when creating RedsStream.
> * deallocated when main_dispatcher handles the
> SPICE_CHANNEL_EVENT_DISCONNECTED
> @@ -1068,3 +1082,87 @@ error:
> return FALSE;
> }
> #endif
> +
> +static ssize_t stream_websocket_read(RedsStream *s, void *buf, size_t size)
> +{
> + int rc;
> +
> + if (s->priv->ws->closed)
> + return 0;
> +
> + rc = websocket_read((void *)s, buf, size, &s->priv->ws->read_frame,
> + (websocket_write_cb_t) s->priv->ws->raw_read,
> + (websocket_write_cb_t) s->priv->ws->raw_write);
> +
> + if (rc == 0)
> + s->priv->ws->closed = 1;
> +
> + return rc;
> +}
> +
> +static ssize_t stream_websocket_write(RedsStream *s, const void *buf, size_t
> size)
> +{
> + if (s->priv->ws->closed) {
> + errno = EPIPE;
> + return -1;
> + }
> + return websocket_write((void *)s, buf, size, &s->priv->ws-
> >write_remainder,
> + (websocket_write_cb_t) s->priv->ws->raw_write);
> +}
> +
> +static ssize_t stream_websocket_writev(RedsStream *s, const struct iovec
> *iov, int iovcnt)
> +{
> + if (s->priv->ws->closed) {
> + errno = EPIPE;
> + return -1;
> + }
> + return websocket_writev((void *)s, (struct iovec *) iov, iovcnt, &s-
> >priv->ws->write_remainder,
> + (websocket_writev_cb_t) s->priv->ws->raw_writev);
> +}
> +
> +/*
> + If we detect that a newly opened stream appears to be using
> + the WebSocket protocol, we will put in place cover functions
> + that will speak WebSocket to the client, but allow the server
> + to continue to use normal stream read/write/writev semantics.
> +*/
> +bool reds_stream_is_websocket(RedsStream *stream, unsigned char *buf, int
> len)
> +{
> + char rbuf[4096];
> + int rc;
> +
> + if (stream->priv->ws) {
> + return FALSE;
> + }
> +
> + memcpy(rbuf, buf, len);
> + rc = stream->priv->read(stream, rbuf + len, sizeof(rbuf) - len);
> + if (rc <= 0)
> + return FALSE;
> + len += rc;
> +
> + if (websocket_is_start(rbuf, len)) {
> + char outbuf[1024];
> +
> + websocket_create_reply(rbuf, len, outbuf);
> + rc = stream->priv->write(stream, outbuf, strlen(outbuf));
> + if (rc == strlen(outbuf)) {
> + stream->priv->ws = spice_malloc0(sizeof(*stream->priv->ws));
It is not freed
> +
> + stream->priv->ws->raw_read = stream->priv->read;
> + stream->priv->ws->raw_write = stream->priv->write;
> +
> + stream->priv->read = stream_websocket_read;
> + stream->priv->write = stream_websocket_write;
> +
> + if (stream->priv->writev) {
> + stream->priv->ws->raw_writev = stream->priv->writev;
> + stream->priv->writev = stream_websocket_writev;
> + }
> +
> + return TRUE;
> + }
> + }
> +
> + return FALSE;
> +}
> diff --git a/server/reds_stream.h b/server/reds_stream.h
> index b5889e3..3e64500 100644
> --- a/server/reds_stream.h
> +++ b/server/reds_stream.h
> @@ -74,6 +74,8 @@ int reds_stream_enable_ssl(RedsStream *stream, SSL_CTX
> *ctx);
> void reds_stream_set_info_flag(RedsStream *stream, unsigned int flag);
> int reds_stream_get_family(RedsStream *stream);
>
> +bool reds_stream_is_websocket(RedsStream *stream, unsigned char *buf, int
> len);
> +
> typedef enum {
> REDS_SASL_ERROR_OK,
> REDS_SASL_ERROR_GENERIC,
> diff --git a/server/websocket.c b/server/websocket.c
> new file mode 100644
> index 0000000..4940914
> --- /dev/null
> +++ b/server/websocket.c
> @@ -0,0 +1,432 @@
> +/* -*- Mode: C; c-basic-offset: 4; indent-tabs-mode: nil -*- */
> +/*
> + Copyright (C) 2015 Jeremy White
> +
> + This library is free software; you can redistribute it and/or
> + modify it under the terms of the GNU Lesser General Public
> + License as published by the Free Software Foundation; either
> + version 2.1 of the License, or (at your option) any later version.
> +
> + This library is distributed in the hope that it will be useful,
> + but WITHOUT ANY WARRANTY; without even the implied warranty of
> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + Lesser General Public License for more details.
> +
> + You should have received a copy of the GNU Lesser General Public
> + License along with this library; if not, see <http://www.gnu.org/licenses/
> >.
> +*/
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <string.h>
> +#include <ctype.h>
> +#include <errno.h>
> +
> +#include <sys/types.h>
> +#include <sys/socket.h>
> +#include <unistd.h>
> +
> +#include <glib.h>
> +
> +#include "websocket.h"
> +#include "common/log.h"
> +
> +/* Perform a case insenstive search for needle in haystack.
> + If found, return a pointer to the byte after the end of needle.
> + Otherwise, return NULL */
> +static gchar *find_str(gchar *haystack, const char *needle, int n)
> +{
> + int i;
> +
> + for (i = 0; i < n; i++) {
> + if ((n - i) < strlen(needle))
> + break;
> +
> + if (g_ascii_strncasecmp(haystack + i, needle, strlen(needle)) == 0)
> + return haystack + i + strlen(needle);
> + }
> + return NULL;
> +}
> +
> +/* Extract WebSocket style length. Returns 0 if not enough data present,
> + Always updates the output 'used' variable to the number of bytes
> + required to extract the length; useful for tracking where the
> + mask will be.
> +*/
> +static guint64 extract_length(guint8 *buf, int *used)
> +{
> + int i;
> + guint64 outlen = (*buf++) & LENGTH_MASK;
> +
> + (*used)++;
> +
> + switch(outlen) {
> + case LENGTH_64BIT:
> + *used += 8;
> + outlen = 0;
> + for (i = 56; i >= 0; i -= 8)
> + outlen |= (*buf++) << i;
> + break;
> +
> + case LENGTH_16BIT:
> + *used += 2;
> + outlen = ((*buf) << 8) | *(buf + 1);
> + break;
> +
> + default:
> + break;
> + }
> + return outlen;
> +}
> +
> +static int frame_bytes_needed(websocket_frame_t *frame)
> +{
> + int needed = 2;
> + if (frame->header_pos < needed)
> + return needed - frame->header_pos;
> +
> + switch(frame->header[1] & LENGTH_MASK) {
> + case LENGTH_64BIT:
> + needed += 8;
> + break;
> + case LENGTH_16BIT:
> + needed += 2;
> + break;
> + }
> +
> + if (frame->header[1] & MASK_FLAG) {
> + needed += 4;
> + }
> +
> + return needed - frame->header_pos;
> +}
> +
> +/*
> +* Generate WebSocket style response key, based on the
> +* original key sent to us
> +* If non null, caller must free returned key string.
> +*/
> +static gchar *generate_reply_key(gchar *buf, int len)
> +{
> + GChecksum *checksum = NULL;
> + gchar *b64 = NULL;
> + guint8 *sha1;
> + gsize sha1_size;
> + gchar *key;
> + gchar *p;
> + gchar *k;
> +
> + key = find_str(buf, "Sec-WebSocket-Key:", len);
> + if (key) {
> + p = strchr(key, '\r');
> + if (p && p - buf < len) {
> + k = g_strndup(key, p - key);
> + k = g_strstrip(k);
> + checksum = g_checksum_new(G_CHECKSUM_SHA1);
> + g_checksum_update(checksum, (guchar *) k, strlen(k));
> + g_checksum_update(checksum, (guchar *) WEBSOCKET_GUID,
> strlen(WEBSOCKET_GUID));
> + g_free(k);
> +
> + sha1_size = g_checksum_type_get_length(G_CHECKSUM_SHA1);
> + sha1 = g_malloc(sha1_size);
> +
> + g_checksum_get_digest(checksum, sha1, &sha1_size);
> +
> + b64 = g_base64_encode(sha1, sha1_size);
> +
> + g_checksum_free(checksum);
> + g_free(sha1);
> + }
> + }
> +
> + return b64;
> +}
> +
> +
> +static void websocket_clear_frame(websocket_frame_t *frame)
> +{
> + memset(frame, 0, sizeof(*frame));
> +}
> +
> +/* Extract a frame header of data from a set of data transmitted by
> + a WebSocket client. Returns bytes consumed if a frame
> + is available, otherwise returns 0 */
> +static int websocket_get_frame_header(websocket_frame_t *frame)
> +{
> + int fin;
> + int used = 0;
> +
> + if (frame->header_pos < frame_bytes_needed(frame))
> + return 0;
> +
> + fin = frame->header[0] & FIN_FLAG;
> + frame->type = frame->header[0] & TYPE_MASK;
> + used++;
> +
> + frame->masked = frame->header[1] & MASK_FLAG;
> +
> + /* This is a Spice specific optimization. We don't really
> + care about assembling frames fully, so we treat
> + a frame in process as a finished frame and pass it along. */
> + if (!fin && frame->type == 0)
> + frame->type = BINARY_FRAME;
> +
> + frame->expected_len = extract_length(frame->header + used, &used);
> +
> + if (frame->masked) {
> + memcpy(frame->mask, frame->header + used, 4);
> + used += 4;
> + }
> +
> + frame->relayed = 0;
> + frame->frame_ready = 1;
> +
> + return used;
> +}
> +
> +static int relay_data(guint8* buf, size_t size, websocket_frame_t *frame)
> +{
> + int i;
> + int n = MIN(size, frame->expected_len - frame->relayed);
> +
> + if (frame->masked) {
> + for (i = 0; i < n && frame->relayed < frame->expected_len; i++,
> frame->relayed++)
> + *buf++ ^= frame->mask[frame->relayed % 4];
> + }
> +
> + return n;
> +}
> +
> +int websocket_read(void *opaque, guchar *buf, int size, websocket_frame_t
> *frame,
> + websocket_read_cb_t read_cb,
> + websocket_write_cb_t write_cb)
> +{
> + int n = 0;
> + int rc;
> +
> + while (size > 0) {
> + if (! frame->frame_ready) {
> + rc = read_cb(opaque, frame->header + frame->header_pos,
> frame_bytes_needed(frame));
> + if (rc <= 0) {
> + if (n > 0 && rc == -1 && (errno == EINTR || errno == EAGAIN))
> + return n;
> +
> + return rc;
> + }
> + frame->header_pos += rc;
> +
> + websocket_get_frame_header(frame);
> + }
> + else if (frame->type == CLOSE_FRAME) {
> + websocket_ack_close(opaque, write_cb);
> + websocket_clear_frame(frame);
> + return 0;
> + }
> + else if (frame->type == BINARY_FRAME) {
> + rc = read_cb(opaque, buf, MIN(size, frame->expected_len - frame-
> >relayed));
> + if (rc <= 0) {
> + if (n > 0 && rc == -1 && (errno == EINTR || errno == EAGAIN))
> + return n;
> +
> + return rc;
> + }
> +
> + rc = relay_data(buf, rc, frame);
> + n += rc;
> + buf += rc;
> + size -= rc;
> + if (frame->relayed >= frame->expected_len)
> + websocket_clear_frame(frame);
> +
> + }
> + else {
> + /* TODO - We don't handle PING at this point */
> + spice_warning("Unexpected WebSocket frame.type %d. Failure now
> likely.", frame->type);
> + websocket_clear_frame(frame);
> + continue;
> + }
> + }
> +
> + return n;
> +}
> +
> +static int fill_header(guint8 *header, guint64 len)
> +{
> + guint64 shiftlen;
> + int used = 0;
> + int i;
> +
> + header[0] = FIN_FLAG | BINARY_FRAME;
> + used++;
> +
> + header[1] = 0;
> + used++;
> + shiftlen = len;
> + if (len > 65535) {
> + header[1] |= LENGTH_64BIT;
> + for (i = 9; i >= 2; i--) {
> + header[i] = shiftlen & 0xFF;
> + shiftlen = shiftlen >> 8;
> + }
> + used += 8;
> + }
> +
> + else if (len > 125) {
> + header[1] |= LENGTH_16BIT;
> + header[2] = len >> 8;
> + header[3] = len & 0xFF;
> + used += 2;
> + }
> +
> + else
> + header[1] |= len;
> +
> + return used;
> +}
> +
> +static void constrain_iov(struct iovec *iov, int iovcnt,
> + struct iovec **iov_out, int *iov_out_cnt,
> + guint64 maxlen)
> +{
> + int i, j;
> +
> + *iov_out = iov;
> + *iov_out_cnt = iovcnt;
> +
> + for (i = 0; i < iovcnt && maxlen > 0; i++) {
> + if (iov[i].iov_len > maxlen) {
> + /* TODO - This code has never triggered afaik... */
> + *iov_out_cnt = i + 1;
> + *iov_out = malloc((*iov_out_cnt) * sizeof (**iov_out));
> + for (j = 0; j < i; j++) {
> + (*iov_out)[j].iov_base = iov[j].iov_base;
> + (*iov_out)[j].iov_len = iov[j].iov_len;
> + }
> + (*iov_out)[j].iov_base = iov[j].iov_base;
> + (*iov_out)[j].iov_len = maxlen;
> + break;
> + }
> + maxlen -= iov[i].iov_len;
> + }
> +}
> +
> +
> +/* Write a WebSocket frame with the enclosed data out. */
> +int websocket_writev(void *opaque, struct iovec *iov, int iovcnt, guint64
> *remainder,
> + websocket_writev_cb_t writev_cb)
> +{
> + guint8 header[WEBSOCKET_MAX_HEADER_SIZE];
> + guint64 len;
> + int rc = -1;
> + struct iovec *iov_out;
> + int iov_out_cnt;
> + int i;
> + int header_len;
> +
> + if (*remainder > 0) {
> + constrain_iov(iov, iovcnt, &iov_out, &iov_out_cnt, *remainder);
> + rc = writev_cb(opaque, iov_out, iov_out_cnt);
> + if (iov_out != iov)
> + free(iov_out);
> +
> + if (rc <= 0)
> + return rc;
> +
> + *remainder -= rc;
> + return rc;
> + }
> +
> + iov_out_cnt = iovcnt + 1;
> + iov_out = malloc(iov_out_cnt * sizeof(*iov_out));
> + if (! iov_out)
> + return -1;
> +
> + for (i = 0, len = 0; i < iovcnt; i++) {
> + len += iov[i].iov_len;
> + iov_out[i + 1] = iov[i];
> + }
> +
> + memset(header, 0, sizeof(header));
> + header_len = fill_header(header, len);
> + iov_out[0].iov_len = header_len;
> + iov_out[0].iov_base = header;
> + rc = writev_cb(opaque, iov_out, iov_out_cnt);
> + free(iov_out);
> + if (rc <= 0)
> + return rc;
> +
> + rc -= header_len;
> +
> + spice_assert(rc >= 0);
> +
> + /* Key point: if we did not write out all the data, remember how
> + much more data the client is expecting, and write that data without
> + a header of any kind the next time around */
> + *remainder = len - rc;
> +
> + return rc;
> +}
> +
> +int websocket_write(void *opaque, const guchar *buf, int len, guint64
> *remainder,
> + websocket_write_cb_t write_cb)
> +{
> + guint8 header[WEBSOCKET_MAX_HEADER_SIZE];
> + int rc;
> + int header_len;
> +
> + memset(header, 0, sizeof(header));
> + header_len = fill_header(header, len);
> +
> + if (*remainder == 0) {
> + rc = write_cb(opaque, header, header_len);
> + if (rc <= 0)
> + return rc;
> +
> + if (rc != header_len) {
> + /* TODO - In theory, we can handle this case. In practice,
> + it does not occur, and does not seem to be worth
> + the code complexity */
> + errno = EPIPE;
> + return -1;
> + }
> + }
> +
> + return write_cb(opaque, buf, len);
> +}
> +
> +void websocket_ack_close(void *opaque, websocket_write_cb_t write_cb)
> +{
> + unsigned char header[2];
> +
> + header[0] = FIN_FLAG | CLOSE_FRAME;
> + header[1] = 0;
> +
> + write_cb(opaque, header, sizeof(header));
> +}
> +
> +int websocket_is_start(gchar *buf, int len)
It sounds like boolean
> +{
> + if (len < 4)
> + return 0;
> +
> + if (find_str(buf, "GET", len) == (buf + 3) &&
> + find_str(buf, "Sec-WebSocket-Protocol: binary", len) &&
> + find_str(buf, "Sec-WebSocket-Key:", len) &&
> + buf[len - 2] == '\r' && buf[len - 1] == '\n')
> + return 1;
> +
> + return 0;
> +}
> +
> +int websocket_create_reply(gchar *buf, int len, gchar *outbuf)
It always returns 0
> +{
> + gchar *key;
> +
> + key = generate_reply_key(buf, len);
Can NULL be valid value? Should we have a warning for it, or change the return
value?
> + sprintf(outbuf, "HTTP/1.1 101 Switching Protocols\r\n"
> + "Upgrade: websocket\r\n"
> + "Connection: Upgrade\r\n"
> + "Sec-WebSocket-Accept: %s\r\n"
> + "Sec-WebSocket-Protocol: binary\r\n\r\n", key);
> + g_free(key);
> + return 0;
> +}
> diff --git a/server/websocket.h b/server/websocket.h
> new file mode 100644
> index 0000000..b35cb5e
> --- /dev/null
> +++ b/server/websocket.h
> @@ -0,0 +1,62 @@
> +/*
> + * Copyright (C) 2015 Jeremy White
> + *
> + * This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * This library is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this library; if not, see <http://www.gnu.org/licenses
> />.
> + */
> +
> +#define WEBSOCKET_MAX_HEADER_SIZE (1 + 9 + 4)
> +
> +#define FIN_FLAG 0x80
> +#define TYPE_MASK 0x0F
> +
> +#define BINARY_FRAME 0x2
> +#define CLOSE_FRAME 0x8
> +#define PING_FRAME 0x9
> +#define PONG_FRAME 0xA
> +
> +#define LENGTH_MASK 0x7F
> +#define LENGTH_16BIT 0x7E
> +#define LENGTH_64BIT 0x7F
> +
> +#define MASK_FLAG 0x80
> +
> +#define WEBSOCKET_GUID "258EAFA5-E914-47DA-95CA-C5AB0DC85B11"
> +
> +typedef struct
> +{
> + int type;
> + int masked;
> + guint8 header[WEBSOCKET_MAX_HEADER_SIZE];
> + int header_pos;
> + int frame_ready:1;
> + guint8 mask[4];
> + guint64 relayed;
> + guint64 expected_len;
> +} websocket_frame_t;
I would prefer to have these ^ definitions private.
> +
> +typedef size_t (*websocket_writev_cb_t)(void *opaque, struct iovec *iov, int
> iovcnt);
> +typedef size_t (*websocket_write_cb_t)(void *opaque, const void *buf, size_t
> nbyte);
> +typedef size_t (*websocket_read_cb_t)(void *opaque, const void *buf, size_t
> nbyte);
RedsStreamPrivate has these ^ as ssize_t
> +
> +int websocket_is_start(gchar *buf, int len);
> +int websocket_create_reply(gchar *buf, int len, gchar *outbuf);
> +int websocket_read(void *opaque, guchar *buf, int len, websocket_frame_t
> *frame,
> + websocket_read_cb_t read_cb,
> + websocket_write_cb_t write_cb);
> +int websocket_write(void *opaque, const guchar *buf, int len, guint64
> *remainder,
> + websocket_write_cb_t write_cb);
> +int websocket_writev(void *opaque, struct iovec *iov, int iovcnt, guint64
> *remainder,
> + websocket_writev_cb_t writev_cb);
> +void websocket_ack_close(void *opaque, websocket_write_cb_t write_cb);
> +
extra line
Thanks,
Pavel
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/spice-devel