Re: [Spice-devel] Graceful failure when VNC client connects to SPICE server ?
On Wed, Jan 25, 2017 at 05:59:44PM +, Daniel P. Berrange wrote:
> On Wed, Jan 25, 2017 at 12:38:09PM -0500, Frediano Ziglio wrote:
> > >
> > > On Wed, Jan 25, 2017 at 12:23:35PM -0500, Frediano Ziglio wrote:
> > > > >
> > > > > When used with QEMU at least, both SPICE and VNC live in the same port
> > > > > range (5900+). It is thus not entirely uncommon for a user to
> > > > > mistakenly
> > > > > connect to a SPICE server with a VNC client, or vica-verca.
> > > > >
> > > > > When connecting to VNC server with a SPICE client, you quickly get an
> > > > > error. This is because the VNC server sends its short greeting and
> > > > > then
> > > > > sees the RedLinkMess from the SPICE client, realizes its garbage and
> > > > > drops the connection.
> > > > >
> > > > > When connecting to a SPICE server with a VNC client though, you get an
> > > > > indefinite hang. The VNC client is waiting for the VNC greeting, which
> > > > > the SPICE server will never send. The SPICE server is waiting for the
> > > > > RedLinkMess which the VNC client will never send.
> > > > >
> > > > > Since VNC is a server sends first protocol and SPICE is a client sends
> > > > > first protocol, it would seem this problem is impossible to fix, but
> > > > > I think it is possible to tweak things so it can be more gracefully
> > > > > handled.
> > > > >
> > > > > In VNC the protocol starts with the follow data sent:
> > > > >
> > > > > Server: "RFB 003.008\n"
> > > > > Client: "RFB 003.008\n"
> > > > >
> > > > > What I'm thinking is that we could easily change the VNC client so
> > > > > that
> > > > > it will send some bytes first. eg
> > > > >
> > > > > Client: "RFB "
> > > > > Server: "RFB 003.008\n"
> > > > > Client: "003.008\n"
> > > > >
> > > > > From the VNC server POV, it'll still be receiving the same 12 bytes
> > > > > from
> > > > > the client - it just happens that 4 of them might arrive a tiny bit
> > > > > earlier than the other 8 of them. IOW nothing should break in the VNC
> > > > > server from this change.
> > > > >
> > > > > I tried this, but we still get a hang in the SPICE server. The problem
> > > > > is that the SPICE server code is waiting for the full RedLinkMess
> > > > > message to be received before checking its content.
> > > > >
> > > > > Can we change the SPICE server so that it reads the 'uint32 magic'
> > > > > first,
> > > > > validates that, and then reads the remainder of RedLinkMess ?
> > > > >
> > > > > This would solve the problem, as the SPICE server would see 'RFB ' as
> > > > > the
> > > > > magic, realize it was garbage and so close the connection. The VNC
> > > > > client
> > > > > which is waiting for the VNC greeting will thus terminate rather than
> > > > > hanging
> > > > > forever.
> > > > >
> > > > > Regards,
> > > > > Daniel
> > > >
> > > > Perhaps another solution would be to add a timeout to spice server so
> > > > after, say, 2/3 seconds it would close the connection.
> > > >
> > > > This would work even without changing the VNC client.
> > >
> > > I'm not a fan of timeouts because this could negatively impact on
> > > genuine spice clients which hit a latency spike on the network
> > > for whatever reason. The incremental read of RedLinkMess by constrast
> > > is guaranteed to not have any chance of negative impact on spice clients.
> > >
> > > Regards,
> > > Daniel
> >
> > So you would like something like that:
> >
> >
> > diff --git a/server/reds.c b/server/reds.c
> > index 8db70ee..50098df 100644
> > --- a/server/reds.c
> > +++ b/server/reds.c
> > @@ -2264,12 +2264,6 @@ static void reds_handle_read_header_done(void
> > *opaque)
> > header->minor_version = GUINT32_FROM_LE(header->minor_version);
> > header->size = GUINT32_FROM_LE(header->size);
> >
> > -if (header->magic != SPICE_MAGIC) {
> > -reds_send_link_error(link, SPICE_LINK_ERR_INVALID_MAGIC);
> > -reds_link_free(link);
> > -return;
> > -}
> > -
> > if (header->major_version != SPICE_VERSION_MAJOR) {
> > if (header->major_version > 0) {
> > reds_send_link_error(link, SPICE_LINK_ERR_VERSION_MISMATCH);
> > @@ -2296,13 +2290,31 @@ static void reds_handle_read_header_done(void
> > *opaque)
> > link);
> > }
> >
> > +static void reds_handle_read_magic_done(void *opaque)
> > +{
> > +RedLinkInfo *link = (RedLinkInfo *)opaque;
> > +const SpiceLinkHeader *header = &link->link_header;
> > +
> > +if (header->magic != SPICE_MAGIC) {
> > +reds_send_link_error(link, SPICE_LINK_ERR_INVALID_MAGIC);
> > +reds_link_free(link);
> > +return;
> > +}
> > +
> > +reds_stream_async_read(link->stream,
> > + ((uint8_t *)&link->link_header) +
> > sizeof(header->magic),
> > + sizeof(SpiceLinkHeader) - sizeof(header->magic),
> > + reds_handle_read_header_done,
> > + link)
Re: [Spice-devel] Graceful failure when VNC client connects to SPICE server ?
On Wed, Jan 25, 2017 at 12:38:09PM -0500, Frediano Ziglio wrote:
> >
> > On Wed, Jan 25, 2017 at 12:23:35PM -0500, Frediano Ziglio wrote:
> > > >
> > > > When used with QEMU at least, both SPICE and VNC live in the same port
> > > > range (5900+). It is thus not entirely uncommon for a user to mistakenly
> > > > connect to a SPICE server with a VNC client, or vica-verca.
> > > >
> > > > When connecting to VNC server with a SPICE client, you quickly get an
> > > > error. This is because the VNC server sends its short greeting and then
> > > > sees the RedLinkMess from the SPICE client, realizes its garbage and
> > > > drops the connection.
> > > >
> > > > When connecting to a SPICE server with a VNC client though, you get an
> > > > indefinite hang. The VNC client is waiting for the VNC greeting, which
> > > > the SPICE server will never send. The SPICE server is waiting for the
> > > > RedLinkMess which the VNC client will never send.
> > > >
> > > > Since VNC is a server sends first protocol and SPICE is a client sends
> > > > first protocol, it would seem this problem is impossible to fix, but
> > > > I think it is possible to tweak things so it can be more gracefully
> > > > handled.
> > > >
> > > > In VNC the protocol starts with the follow data sent:
> > > >
> > > > Server: "RFB 003.008\n"
> > > > Client: "RFB 003.008\n"
> > > >
> > > > What I'm thinking is that we could easily change the VNC client so that
> > > > it will send some bytes first. eg
> > > >
> > > > Client: "RFB "
> > > > Server: "RFB 003.008\n"
> > > > Client: "003.008\n"
> > > >
> > > > From the VNC server POV, it'll still be receiving the same 12 bytes from
> > > > the client - it just happens that 4 of them might arrive a tiny bit
> > > > earlier than the other 8 of them. IOW nothing should break in the VNC
> > > > server from this change.
> > > >
> > > > I tried this, but we still get a hang in the SPICE server. The problem
> > > > is that the SPICE server code is waiting for the full RedLinkMess
> > > > message to be received before checking its content.
> > > >
> > > > Can we change the SPICE server so that it reads the 'uint32 magic'
> > > > first,
> > > > validates that, and then reads the remainder of RedLinkMess ?
> > > >
> > > > This would solve the problem, as the SPICE server would see 'RFB ' as
> > > > the
> > > > magic, realize it was garbage and so close the connection. The VNC
> > > > client
> > > > which is waiting for the VNC greeting will thus terminate rather than
> > > > hanging
> > > > forever.
> > > >
> > > > Regards,
> > > > Daniel
> > >
> > > Perhaps another solution would be to add a timeout to spice server so
> > > after, say, 2/3 seconds it would close the connection.
> > >
> > > This would work even without changing the VNC client.
> >
> > I'm not a fan of timeouts because this could negatively impact on
> > genuine spice clients which hit a latency spike on the network
> > for whatever reason. The incremental read of RedLinkMess by constrast
> > is guaranteed to not have any chance of negative impact on spice clients.
> >
> > Regards,
> > Daniel
>
> So you would like something like that:
>
>
> diff --git a/server/reds.c b/server/reds.c
> index 8db70ee..50098df 100644
> --- a/server/reds.c
> +++ b/server/reds.c
> @@ -2264,12 +2264,6 @@ static void reds_handle_read_header_done(void *opaque)
> header->minor_version = GUINT32_FROM_LE(header->minor_version);
> header->size = GUINT32_FROM_LE(header->size);
>
> -if (header->magic != SPICE_MAGIC) {
> -reds_send_link_error(link, SPICE_LINK_ERR_INVALID_MAGIC);
> -reds_link_free(link);
> -return;
> -}
> -
> if (header->major_version != SPICE_VERSION_MAJOR) {
> if (header->major_version > 0) {
> reds_send_link_error(link, SPICE_LINK_ERR_VERSION_MISMATCH);
> @@ -2296,13 +2290,31 @@ static void reds_handle_read_header_done(void *opaque)
> link);
> }
>
> +static void reds_handle_read_magic_done(void *opaque)
> +{
> +RedLinkInfo *link = (RedLinkInfo *)opaque;
> +const SpiceLinkHeader *header = &link->link_header;
> +
> +if (header->magic != SPICE_MAGIC) {
> +reds_send_link_error(link, SPICE_LINK_ERR_INVALID_MAGIC);
> +reds_link_free(link);
> +return;
> +}
> +
> +reds_stream_async_read(link->stream,
> + ((uint8_t *)&link->link_header) +
> sizeof(header->magic),
> + sizeof(SpiceLinkHeader) - sizeof(header->magic),
> + reds_handle_read_header_done,
> + link);
> +}
> +
> static void reds_handle_new_link(RedLinkInfo *link)
> {
> reds_stream_set_async_error_handler(link->stream,
> reds_handle_link_error);
> reds_stream_async_read(link->stream,
> (uint8_t *)&link->link_header,
> - sizeof(SpiceLinkHeader),
> - re
Re: [Spice-devel] Graceful failure when VNC client connects to SPICE server ?
>
> On Wed, Jan 25, 2017 at 12:23:35PM -0500, Frediano Ziglio wrote:
> > >
> > > When used with QEMU at least, both SPICE and VNC live in the same port
> > > range (5900+). It is thus not entirely uncommon for a user to mistakenly
> > > connect to a SPICE server with a VNC client, or vica-verca.
> > >
> > > When connecting to VNC server with a SPICE client, you quickly get an
> > > error. This is because the VNC server sends its short greeting and then
> > > sees the RedLinkMess from the SPICE client, realizes its garbage and
> > > drops the connection.
> > >
> > > When connecting to a SPICE server with a VNC client though, you get an
> > > indefinite hang. The VNC client is waiting for the VNC greeting, which
> > > the SPICE server will never send. The SPICE server is waiting for the
> > > RedLinkMess which the VNC client will never send.
> > >
> > > Since VNC is a server sends first protocol and SPICE is a client sends
> > > first protocol, it would seem this problem is impossible to fix, but
> > > I think it is possible to tweak things so it can be more gracefully
> > > handled.
> > >
> > > In VNC the protocol starts with the follow data sent:
> > >
> > > Server: "RFB 003.008\n"
> > > Client: "RFB 003.008\n"
> > >
> > > What I'm thinking is that we could easily change the VNC client so that
> > > it will send some bytes first. eg
> > >
> > > Client: "RFB "
> > > Server: "RFB 003.008\n"
> > > Client: "003.008\n"
> > >
> > > From the VNC server POV, it'll still be receiving the same 12 bytes from
> > > the client - it just happens that 4 of them might arrive a tiny bit
> > > earlier than the other 8 of them. IOW nothing should break in the VNC
> > > server from this change.
> > >
> > > I tried this, but we still get a hang in the SPICE server. The problem
> > > is that the SPICE server code is waiting for the full RedLinkMess
> > > message to be received before checking its content.
> > >
> > > Can we change the SPICE server so that it reads the 'uint32 magic' first,
> > > validates that, and then reads the remainder of RedLinkMess ?
> > >
> > > This would solve the problem, as the SPICE server would see 'RFB ' as the
> > > magic, realize it was garbage and so close the connection. The VNC client
> > > which is waiting for the VNC greeting will thus terminate rather than
> > > hanging
> > > forever.
> > >
> > > Regards,
> > > Daniel
> >
> > Perhaps another solution would be to add a timeout to spice server so
> > after, say, 2/3 seconds it would close the connection.
> >
> > This would work even without changing the VNC client.
>
> I'm not a fan of timeouts because this could negatively impact on
> genuine spice clients which hit a latency spike on the network
> for whatever reason. The incremental read of RedLinkMess by constrast
> is guaranteed to not have any chance of negative impact on spice clients.
>
> Regards,
> Daniel
So you would like something like that:
diff --git a/server/reds.c b/server/reds.c
index 8db70ee..50098df 100644
--- a/server/reds.c
+++ b/server/reds.c
@@ -2264,12 +2264,6 @@ static void reds_handle_read_header_done(void *opaque)
header->minor_version = GUINT32_FROM_LE(header->minor_version);
header->size = GUINT32_FROM_LE(header->size);
-if (header->magic != SPICE_MAGIC) {
-reds_send_link_error(link, SPICE_LINK_ERR_INVALID_MAGIC);
-reds_link_free(link);
-return;
-}
-
if (header->major_version != SPICE_VERSION_MAJOR) {
if (header->major_version > 0) {
reds_send_link_error(link, SPICE_LINK_ERR_VERSION_MISMATCH);
@@ -2296,13 +2290,31 @@ static void reds_handle_read_header_done(void *opaque)
link);
}
+static void reds_handle_read_magic_done(void *opaque)
+{
+RedLinkInfo *link = (RedLinkInfo *)opaque;
+const SpiceLinkHeader *header = &link->link_header;
+
+if (header->magic != SPICE_MAGIC) {
+reds_send_link_error(link, SPICE_LINK_ERR_INVALID_MAGIC);
+reds_link_free(link);
+return;
+}
+
+reds_stream_async_read(link->stream,
+ ((uint8_t *)&link->link_header) +
sizeof(header->magic),
+ sizeof(SpiceLinkHeader) - sizeof(header->magic),
+ reds_handle_read_header_done,
+ link);
+}
+
static void reds_handle_new_link(RedLinkInfo *link)
{
reds_stream_set_async_error_handler(link->stream, reds_handle_link_error);
reds_stream_async_read(link->stream,
(uint8_t *)&link->link_header,
- sizeof(SpiceLinkHeader),
- reds_handle_read_header_done,
+ sizeof(link->link_header.magic),
+ reds_handle_read_magic_done,
link);
}
?
I don't think the extra code execute is much of a problem.
Frediano
___
Spice-devel mailing list
Re: [Spice-devel] Graceful failure when VNC client connects to SPICE server ?
On Wed, Jan 25, 2017 at 12:23:35PM -0500, Frediano Ziglio wrote: > > > > When used with QEMU at least, both SPICE and VNC live in the same port > > range (5900+). It is thus not entirely uncommon for a user to mistakenly > > connect to a SPICE server with a VNC client, or vica-verca. > > > > When connecting to VNC server with a SPICE client, you quickly get an > > error. This is because the VNC server sends its short greeting and then > > sees the RedLinkMess from the SPICE client, realizes its garbage and > > drops the connection. > > > > When connecting to a SPICE server with a VNC client though, you get an > > indefinite hang. The VNC client is waiting for the VNC greeting, which > > the SPICE server will never send. The SPICE server is waiting for the > > RedLinkMess which the VNC client will never send. > > > > Since VNC is a server sends first protocol and SPICE is a client sends > > first protocol, it would seem this problem is impossible to fix, but > > I think it is possible to tweak things so it can be more gracefully > > handled. > > > > In VNC the protocol starts with the follow data sent: > > > > Server: "RFB 003.008\n" > > Client: "RFB 003.008\n" > > > > What I'm thinking is that we could easily change the VNC client so that > > it will send some bytes first. eg > > > > Client: "RFB " > > Server: "RFB 003.008\n" > > Client: "003.008\n" > > > > From the VNC server POV, it'll still be receiving the same 12 bytes from > > the client - it just happens that 4 of them might arrive a tiny bit > > earlier than the other 8 of them. IOW nothing should break in the VNC > > server from this change. > > > > I tried this, but we still get a hang in the SPICE server. The problem > > is that the SPICE server code is waiting for the full RedLinkMess > > message to be received before checking its content. > > > > Can we change the SPICE server so that it reads the 'uint32 magic' first, > > validates that, and then reads the remainder of RedLinkMess ? > > > > This would solve the problem, as the SPICE server would see 'RFB ' as the > > magic, realize it was garbage and so close the connection. The VNC client > > which is waiting for the VNC greeting will thus terminate rather than > > hanging > > forever. > > > > Regards, > > Daniel > > Perhaps another solution would be to add a timeout to spice server so > after, say, 2/3 seconds it would close the connection. > > This would work even without changing the VNC client. I'm not a fan of timeouts because this could negatively impact on genuine spice clients which hit a latency spike on the network for whatever reason. The incremental read of RedLinkMess by constrast is guaranteed to not have any chance of negative impact on spice clients. Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://entangle-photo.org -o-http://search.cpan.org/~danberr/ :| ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] Graceful failure when VNC client connects to SPICE server ?
> > When used with QEMU at least, both SPICE and VNC live in the same port > range (5900+). It is thus not entirely uncommon for a user to mistakenly > connect to a SPICE server with a VNC client, or vica-verca. > > When connecting to VNC server with a SPICE client, you quickly get an > error. This is because the VNC server sends its short greeting and then > sees the RedLinkMess from the SPICE client, realizes its garbage and > drops the connection. > > When connecting to a SPICE server with a VNC client though, you get an > indefinite hang. The VNC client is waiting for the VNC greeting, which > the SPICE server will never send. The SPICE server is waiting for the > RedLinkMess which the VNC client will never send. > > Since VNC is a server sends first protocol and SPICE is a client sends > first protocol, it would seem this problem is impossible to fix, but > I think it is possible to tweak things so it can be more gracefully > handled. > > In VNC the protocol starts with the follow data sent: > > Server: "RFB 003.008\n" > Client: "RFB 003.008\n" > > What I'm thinking is that we could easily change the VNC client so that > it will send some bytes first. eg > > Client: "RFB " > Server: "RFB 003.008\n" > Client: "003.008\n" > > From the VNC server POV, it'll still be receiving the same 12 bytes from > the client - it just happens that 4 of them might arrive a tiny bit > earlier than the other 8 of them. IOW nothing should break in the VNC > server from this change. > > I tried this, but we still get a hang in the SPICE server. The problem > is that the SPICE server code is waiting for the full RedLinkMess > message to be received before checking its content. > > Can we change the SPICE server so that it reads the 'uint32 magic' first, > validates that, and then reads the remainder of RedLinkMess ? > > This would solve the problem, as the SPICE server would see 'RFB ' as the > magic, realize it was garbage and so close the connection. The VNC client > which is waiting for the VNC greeting will thus terminate rather than hanging > forever. > > Regards, > Daniel Perhaps another solution would be to add a timeout to spice server so after, say, 2/3 seconds it would close the connection. This would work even without changing the VNC client. Frediano ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] Graceful failure when VNC client connects to SPICE server ?
When used with QEMU at least, both SPICE and VNC live in the same port range (5900+). It is thus not entirely uncommon for a user to mistakenly connect to a SPICE server with a VNC client, or vica-verca. When connecting to VNC server with a SPICE client, you quickly get an error. This is because the VNC server sends its short greeting and then sees the RedLinkMess from the SPICE client, realizes its garbage and drops the connection. When connecting to a SPICE server with a VNC client though, you get an indefinite hang. The VNC client is waiting for the VNC greeting, which the SPICE server will never send. The SPICE server is waiting for the RedLinkMess which the VNC client will never send. Since VNC is a server sends first protocol and SPICE is a client sends first protocol, it would seem this problem is impossible to fix, but I think it is possible to tweak things so it can be more gracefully handled. In VNC the protocol starts with the follow data sent: Server: "RFB 003.008\n" Client: "RFB 003.008\n" What I'm thinking is that we could easily change the VNC client so that it will send some bytes first. eg Client: "RFB " Server: "RFB 003.008\n" Client: "003.008\n" From the VNC server POV, it'll still be receiving the same 12 bytes from the client - it just happens that 4 of them might arrive a tiny bit earlier than the other 8 of them. IOW nothing should break in the VNC server from this change. I tried this, but we still get a hang in the SPICE server. The problem is that the SPICE server code is waiting for the full RedLinkMess message to be received before checking its content. Can we change the SPICE server so that it reads the 'uint32 magic' first, validates that, and then reads the remainder of RedLinkMess ? This would solve the problem, as the SPICE server would see 'RFB ' as the magic, realize it was garbage and so close the connection. The VNC client which is waiting for the VNC greeting will thus terminate rather than hanging forever. Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://entangle-photo.org -o-http://search.cpan.org/~danberr/ :| ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
