We've been experiencing issues where the qemu websocket server closes connections from noVNC clients for no apparent reason. Debugging shows that certain web browsers are injecting ping and pong frames when the connection becomes idle. Some browsers send those frames without a payload, which also is causing closure. This patch series addresses these issues by making the websocket server more conformant to RFC 6455 - The WebSocket Protocol. Remembering the opcode is sufficient for handling fragmented frames from the client, which may be introduced by an intermediary server/proxy. Respond to pings and ignore pongs rather than close the connection as many browsers use ping/pong to test an idle connection. Close connections according to the RFC, including providing a reason code and message to aid debugging of unexpected disconnects. Empty payloads should not cause a disconnect. While updating the websocket code, several other bugs were discovered for which patches are also included early in the set. Brandon Carpenter (7): io: Always remove an old channel watch before adding a new one io: Small updates in preparation for websocket changes io: Add support for fragmented websocket binary frames io: Allow empty websocket payload io: Ignore websocket PING and PONG frames io: Reply to ping frames io: Attempt to send websocket close messages to client include/io/channel-websock.h | 2 + io/channel-websock.c | 282 +++++++++++++++++++++++++++---------------- ui/vnc-auth-vencrypt.c | 3 + ui/vnc-ws.c | 6 + ui/vnc.c | 4 + 5 files changed, 194 insertions(+), 103 deletions(-) -- 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.
On Tue, Sep 12, 2017 at 08:21:46AM -0700, Brandon Carpenter wrote: > We've been experiencing issues where the qemu websocket server closes > connections from noVNC clients for no apparent reason. Debugging shows > that certain web browsers are injecting ping and pong frames when the > connection becomes idle. Some browsers send those frames without a > payload, which also is causing closure. This patch series addresses > these issues by making the websocket server more conformant to RFC > 6455 - The WebSocket Protocol. > > Remembering the opcode is sufficient for handling fragmented frames from > the client, which may be introduced by an intermediary server/proxy. > Respond to pings and ignore pongs rather than close the connection as > many browsers use ping/pong to test an idle connection. Close > connections according to the RFC, including providing a reason code and > message to aid debugging of unexpected disconnects. Empty payloads > should not cause a disconnect. > > While updating the websocket code, several other bugs were discovered > for which patches are also included early in the set. > > Brandon Carpenter (7): > io: Always remove an old channel watch before adding a new one > io: Small updates in preparation for websocket changes > io: Add support for fragmented websocket binary frames > io: Allow empty websocket payload > io: Ignore websocket PING and PONG frames > io: Reply to ping frames > io: Attempt to send websocket close messages to client > > include/io/channel-websock.h | 2 + > io/channel-websock.c | 282 +++++++++++++++++++++++++++---------------- > ui/vnc-auth-vencrypt.c | 3 + > ui/vnc-ws.c | 6 + > ui/vnc.c | 4 + > 5 files changed, 194 insertions(+), 103 deletions(-) > > -- > 2.14.1 Hi Brandon, Thanks for the patch series! Please CC the relevant maintainer on future patches. I've CCed Daniel Berrange on this email to bring your work to his attention. $ scripts/get_maintainer.pl -f io/channel-websock.c "Daniel P. Berrange" <berrange@redhat.com> (maintainer:I/O Channels) qemu-devel@nongnu.org (open list:All patches CC here) Stefan > > > -- > > > 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. >
How are things looking, Daniel. I believe every comment from the previous version of the series was addressed. I also wanted to mention that I put together a Python script, which acts as a proxy between noVNC and qemu, and can inject various frame types, fragment frames, and shutdown the socket in various ways to help exercise the different code paths. I would be happy to post it here to help test the changes. Thanks, -- Brandon Carpenter | Software Engineer Cypherpath, Inc. 400 Columbia Point Drive Ste 101 | Richland, Washington USA Office: (650) 713-3060 On Tue, Sep 12, 2017 at 8:21 AM, Brandon Carpenter <brandon.carpenter@cypherpath.com> wrote: > We've been experiencing issues where the qemu websocket server closes > connections from noVNC clients for no apparent reason. Debugging shows > that certain web browsers are injecting ping and pong frames when the > connection becomes idle. Some browsers send those frames without a > payload, which also is causing closure. This patch series addresses > these issues by making the websocket server more conformant to RFC > 6455 - The WebSocket Protocol. > > Remembering the opcode is sufficient for handling fragmented frames > from > the client, which may be introduced by an intermediary server/proxy. > Respond to pings and ignore pongs rather than close the connection as > many browsers use ping/pong to test an idle connection. Close > connections according to the RFC, including providing a reason code > and > message to aid debugging of unexpected disconnects. Empty payloads > should not cause a disconnect. > > While updating the websocket code, several other bugs were discovered > for which patches are also included early in the set. > > Brandon Carpenter (7): > io: Always remove an old channel watch before adding a new one > io: Small updates in preparation for websocket changes > io: Add support for fragmented websocket binary frames > io: Allow empty websocket payload > io: Ignore websocket PING and PONG frames > io: Reply to ping frames > io: Attempt to send websocket close messages to client > > include/io/channel-websock.h | 2 + > io/channel-websock.c | 282 > +++++++++++++++++++++++++++---------------- > ui/vnc-auth-vencrypt.c | 3 + > ui/vnc-ws.c | 6 + > ui/vnc.c | 4 + > 5 files changed, 194 insertions(+), 103 deletions(-) > > -- > 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.
On Wed, Sep 20, 2017 at 09:56:07AM -0700, Brandon Carpenter wrote: > How are things looking, Daniel. I believe every comment from the previous > version of the series was addressed. > > I also wanted to mention that I put together a Python script, which acts as > a proxy between noVNC and qemu, and can inject various frame types, fragment > frames, and shutdown the socket in various ways to help exercise the > different code paths. I would be happy to post it here to help test the > changes. Sorry for the delay - I've reviewed this now and it looks good. I've made a few whitespace changes in places, but I've queued it for my next pull request now. 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 :|
Cool. Thanks for helping me through the process and for the great feedback. -- Brandon Carpenter | Software Engineer Cypherpath, Inc. 400 Columbia Point Drive Ste 101 | Richland, Washington USA Office: (650) 713-3060 On Thu, Sep 21, 2017 at 2:55 AM, Daniel P. Berrange <berrange@redhat.com> wrote: > On Wed, Sep 20, 2017 at 09:56:07AM -0700, Brandon Carpenter wrote: >> How are things looking, Daniel. I believe every comment from the >> previous >> version of the series was addressed. >> >> I also wanted to mention that I put together a Python script, which >> acts as >> a proxy between noVNC and qemu, and can inject various frame types, >> fragment >> frames, and shutdown the socket in various ways to help exercise the >> different code paths. I would be happy to post it here to help test >> the >> changes. > > Sorry for the delay - I've reviewed this now and it looks good. I've > made a > few whitespace changes in places, but I've queued it for my next pull > request > now. > > 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 :| -- 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.
© 2016 - 2024 Red Hat, Inc.