[Qemu-devel] [PATCH v2 5/6] io: Ignore websocket PING and PONG frames

Brandon Carpenter posted 6 patches 8 years, 5 months ago
There is a newer version of this series
[Qemu-devel] [PATCH v2 5/6] io: Ignore websocket PING and PONG frames
Posted by Brandon Carpenter 8 years, 5 months ago
Keep pings and gratuitous pongs generated by web browsers from killing
websocket connections.

Signed-off-by: Brandon Carpenter <brandon.carpenter@cypherpath.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 3183aeff77..50387050d5 100644
--- a/io/channel-websock.c
+++ b/io/channel-websock.c
@@ -86,6 +86,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;
 
@@ -565,8 +566,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;
         }
     }
@@ -579,6 +583,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);
@@ -634,9 +641,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;
-- 
2.14.1


-- 


CONFIDENTIALITY NOTICE: This e-mail message, including any attachments, is 
for the sole use of the intended recipient(s) and may contain proprietary, 
confidential or privileged information or otherwise be protected by law. 
Any unauthorized review, use, disclosure or distribution is prohibited. If 
you are not the intended recipient, please notify the sender and destroy 
all copies and the original message.

Re: [Qemu-devel] [PATCH v2 5/6] io: Ignore websocket PING and PONG frames
Posted by Daniel P. Berrange 8 years, 5 months ago
On Fri, Sep 08, 2017 at 10:38:00AM -0700, Brandon Carpenter wrote:
> Keep pings and gratuitous pongs generated by web browsers from killing
> websocket connections.
> 
> Signed-off-by: Brandon Carpenter <brandon.carpenter@cypherpath.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 3183aeff77..50387050d5 100644
> --- a/io/channel-websock.c
> +++ b/io/channel-websock.c
> @@ -86,6 +86,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;
>  
> @@ -565,8 +566,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) {

Why would we need to ignore PONG ?  A client should only send a PONG in
response to a PING that we send, and we never send PINGs.  So if we
received a PONG that would be a serious error by the client, which should
cause us to close the connection IMHO>


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [Qemu-devel] [PATCH v2 5/6] io: Ignore websocket PING and PONG frames
Posted by Daniel P. Berrange 8 years, 5 months ago
On Mon, Sep 11, 2017 at 09:38:46AM +0100, Daniel P. Berrange wrote:
> On Fri, Sep 08, 2017 at 10:38:00AM -0700, Brandon Carpenter wrote:
> > Keep pings and gratuitous pongs generated by web browsers from killing
> > websocket connections.
> > 
> > Signed-off-by: Brandon Carpenter <brandon.carpenter@cypherpath.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 3183aeff77..50387050d5 100644
> > --- a/io/channel-websock.c
> > +++ b/io/channel-websock.c
> > @@ -86,6 +86,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;
> >  
> > @@ -565,8 +566,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) {
> 
> Why would we need to ignore PONG ?  A client should only send a PONG in
> response to a PING that we send, and we never send PINGs.  So if we
> received a PONG that would be a serious error by the client, which should
> cause us to close the connection IMHO>

Never mind, I've just seen that the RFC allows clients to send an
unsolicited PONG


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|