Re: [Spice-devel] [spice-gtk] Add --spice-disable-clipboard option

[Christophe Fergeau]

On Tue, Feb 20, 2018 at 12:48:48PM +0100, Victor Toso wrote:
> On Thu, Jan 18, 2018 at 10:31:26AM +0100, Christophe Fergeau wrote:
> > At least on X.org, malicious code could run the equivalent of "watch
> > xsel -o --clipboard" in a VM, and would then be able to track all the
> > clipboard content, even when the spice-gtk widget is not focused.
> > 
> > At the moment, applications call spice_set_session_option(), and then
> > set SpiceGtkSession::auto-clipboard to TRUE (or to its saved state).
> > This commit adds a --spice-disable-clipboard option, and if it's set,
> > SpiceGtkSession::auto-clipboard will not be changeable and will always
> > be FALSE.
> > The only side effect I noticed is that enabling "clipboard sharing" in
> > GNOME Boxes VM preferences will appear to work, but will not enable
> > clipboard, and will be reset to off next time the preferences dialog is
> > open.
> 
> You mean running gnome-boxes --spice-disable-clipboard still
> shows clipboard enabled? Any bug filed already? If yes, I think
> we could add to the commit log.

There's a "clipboard sharing enabled" GObject property on
some spice-gtk object. Boxes uses it to enable/disable clipboard
sharing.
Now if you use --spice-disable-clipboard, this property is always going
to be disabled even if Boxes tries to set it to TRUE. But Boxes is not
aware of that, so when clicks on the checkbox in the UI, it appears to
be toggled, but if you close the dialog and come back, then it will be
shown as disabled again.
I haven't filed yet a Boxes bug for that.

Christophe


signature.asc
Description: PGP signature
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] [spice-gtk] Add --spice-disable-clipboard option

[Victor Toso]

On Thu, Jan 18, 2018 at 10:31:26AM +0100, Christophe Fergeau wrote:
> At least on X.org, malicious code could run the equivalent of "watch
> xsel -o --clipboard" in a VM, and would then be able to track all the
> clipboard content, even when the spice-gtk widget is not focused.
> 
> At the moment, applications call spice_set_session_option(), and then
> set SpiceGtkSession::auto-clipboard to TRUE (or to its saved state).
> This commit adds a --spice-disable-clipboard option, and if it's set,
> SpiceGtkSession::auto-clipboard will not be changeable and will always
> be FALSE.
> The only side effect I noticed is that enabling "clipboard sharing" in
> GNOME Boxes VM preferences will appear to work, but will not enable
> clipboard, and will be reset to off next time the preferences dialog is
> open.

You mean running gnome-boxes --spice-disable-clipboard still
shows clipboard enabled? Any bug filed already? If yes, I think
we could add to the commit log.

> https://bugzilla.redhat.com/show_bug.cgi?id=1320263
> ---
>  man/spice-client.pod |  4 
>  src/spice-gtk-session.c  | 16 +++-
>  src/spice-option.c   |  6 ++
>  src/spice-session-priv.h |  1 +
>  src/spice-session.c  | 35 +++
>  5 files changed, 61 insertions(+), 1 deletion(-)
> 
> diff --git a/man/spice-client.pod b/man/spice-client.pod
> index 7288b84e..76a7d207 100644
> --- a/man/spice-client.pod
> +++ b/man/spice-client.pod
> @@ -111,6 +111,10 @@ SPICE_DEBUG environment variable, or using 
> G_MESSAGES_DEBUG=all
>  
>  Disable audio support
>  
> +=item --spice-disable-clipboard
> +
> +Disable clipboard sharing between client OS and guest OS
> +
>  =item --spice-disable-usbredir
>  
>  Disable USB redirection support
> diff --git a/src/spice-gtk-session.c b/src/spice-gtk-session.c
> index 31f60dc4..c3546209 100644
> --- a/src/spice-gtk-session.c
> +++ b/src/spice-gtk-session.c
> @@ -309,6 +309,20 @@ static void spice_gtk_session_finalize(GObject *gobject)
>  G_OBJECT_CLASS(spice_gtk_session_parent_class)->finalize(gobject);
>  }
>  
> +static void spice_gtk_session_set_auto_clipboard(SpiceGtkSession *self,
> + gboolean auto_clipboard)
> +{
> +gboolean disable_clipboard;
> +g_object_get(self->priv->session,
> + "disable-clipboard", _clipboard,
> + NULL);
> +if (disable_clipboard && auto_clipboard) {
> +g_debug("not enabling clipboard sharing as it was disabled on the 
> command-line");
> +auto_clipboard = FALSE;
> +}
> +self->priv->auto_clipboard_enable = auto_clipboard;
> +}
> +
>  static void spice_gtk_session_get_property(GObject*gobject,
> guint   prop_id,
> GValue *value,
> @@ -352,7 +366,7 @@ static void spice_gtk_session_set_property(GObject  
> *gobject,
>  s->session = g_value_get_object(value);
>  break;
>  case PROP_AUTO_CLIPBOARD:
> -s->auto_clipboard_enable = g_value_get_boolean(value);
> +spice_gtk_session_set_auto_clipboard(self, 
> g_value_get_boolean(value));
>  break;
>  case PROP_AUTO_USBREDIR: {
>  SpiceDesktopIntegration *desktop_int;
> diff --git a/src/spice-option.c b/src/spice-option.c
> index 6b400bca..0f2f795d 100644
> --- a/src/spice-option.c
> +++ b/src/spice-option.c
> @@ -21,6 +21,7 @@
>  #include 
>  #include 
>  #include "spice-session.h"
> +#include "spice-session-priv.h"
>  #include "spice-util.h"
>  #include "spice-channel-priv.h"
>  #include "usb-device-manager.h"
> @@ -35,6 +36,7 @@ static char *usbredir_auto_redirect_filter = NULL;
>  static char *usbredir_redirect_on_connect = NULL;
>  static gboolean smartcard = FALSE;
>  static gboolean disable_audio = FALSE;
> +static gboolean disable_clipboard = FALSE;
>  static gboolean disable_usbredir = FALSE;
>  static gint cache_size = 0;
>  static gint glz_window_size = 0;
> @@ -203,6 +205,8 @@ GOptionGroup* spice_get_option_group(void)
>N_("Subject of the host certificate (field=value pairs separated 
> by commas)"), N_("") },
>  { "spice-disable-audio", '\0', 0, G_OPTION_ARG_NONE, _audio,
>N_("Disable audio support"), NULL },
> +{ "spice-disable-clipboard", '\0', 0, G_OPTION_ARG_NONE, 
> _clipboard,
> +  N_("Disable client/guest clipboard sharing"), NULL },
>  { "spice-smartcard", '\0', 0, G_OPTION_ARG_NONE, ,
>N_("Enable smartcard support"), NULL },
>  { "spice-smartcard-certificates", '\0', 0, G_OPTION_ARG_STRING, 
> _certificates,
> @@ -312,6 +316,8 @@ void spice_set_session_option(SpiceSession *session)
>  g_object_set(session, "enable-usbredir", FALSE, NULL);
>  if (disable_audio)
>  g_object_set(session, "enable-audio", FALSE, NULL);
> +if (disable_clipboard)
> +spice_session_set_disable_clipboard(session, TRUE);
>   

Re: [Spice-devel] [spice-gtk] Add --spice-disable-clipboard option

[Christophe Fergeau]

On Thu, Jan 18, 2018 at 12:13:45PM +0100, Christophe Fergeau wrote:
> On Thu, Jan 18, 2018 at 12:06:34PM +0100, Marc-André Lureau wrote:
> > Hi
> > 
> > On Thu, Jan 18, 2018 at 10:31 AM, Christophe Fergeau
> >  wrote:
> > > At least on X.org, malicious code could run the equivalent of "watch
> > > xsel -o --clipboard" in a VM, and would then be able to track all the
> > > clipboard content, even when the spice-gtk widget is not focused.
> > >
> > > At the moment, applications call spice_set_session_option(), and then
> > > set SpiceGtkSession::auto-clipboard to TRUE (or to its saved state).
> > > This commit adds a --spice-disable-clipboard option, and if it's set,
> > > SpiceGtkSession::auto-clipboard will not be changeable and will always
> > > be FALSE.
> > > The only side effect I noticed is that enabling "clipboard sharing" in
> > > GNOME Boxes VM preferences will appear to work, but will not enable
> > > clipboard, and will be reset to off next time the preferences dialog is
> > > open.
> > >
> > > https://bugzilla.redhat.com/show_bug.cgi?id=1320263
> > 
> > Looks reasonable to me. However, I thought we wanted a way to disable
> > clipboard by default.
> > 
> > Wouldn't it make sense to introduce some GSetting key(s) for that instead?
> > 
> > This way, the behaviour can be enforced globally without changing the
> > way applications are started.
> 
> I think you want both, you don't necessarily want c for all or none of
> your VMs. I don't know if we can check if the admin locked down a
> particular GSettings through the API? If the global value is locked down
> to FALSE, then we should enforce it, otherwise we should accept
> --spice-disable-clipboard.
> So a GSettings patch would probably be a followup to that one.

Can we move forward with that command line addition? Or is adding a
GSettings key a prerequisite to getting this in?

Thanks,

Christophe


signature.asc
Description: PGP signature
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] [spice-gtk] Add --spice-disable-clipboard option

[Christophe Fergeau]

On Thu, Jan 18, 2018 at 12:06:34PM +0100, Marc-André Lureau wrote:
> Hi
> 
> On Thu, Jan 18, 2018 at 10:31 AM, Christophe Fergeau
>  wrote:
> > At least on X.org, malicious code could run the equivalent of "watch
> > xsel -o --clipboard" in a VM, and would then be able to track all the
> > clipboard content, even when the spice-gtk widget is not focused.
> >
> > At the moment, applications call spice_set_session_option(), and then
> > set SpiceGtkSession::auto-clipboard to TRUE (or to its saved state).
> > This commit adds a --spice-disable-clipboard option, and if it's set,
> > SpiceGtkSession::auto-clipboard will not be changeable and will always
> > be FALSE.
> > The only side effect I noticed is that enabling "clipboard sharing" in
> > GNOME Boxes VM preferences will appear to work, but will not enable
> > clipboard, and will be reset to off next time the preferences dialog is
> > open.
> >
> > https://bugzilla.redhat.com/show_bug.cgi?id=1320263
> 
> Looks reasonable to me. However, I thought we wanted a way to disable
> clipboard by default.
> 
> Wouldn't it make sense to introduce some GSetting key(s) for that instead?
> 
> This way, the behaviour can be enforced globally without changing the
> way applications are started.

I think you want both, you don't necessarily want c for all or none of
your VMs. I don't know if we can check if the admin locked down a
particular GSettings through the API? If the global value is locked down
to FALSE, then we should enforce it, otherwise we should accept
--spice-disable-clipboard.
So a GSettings patch would probably be a followup to that one.

Christophe


signature.asc
Description: PGP signature
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] [spice-gtk] Add --spice-disable-clipboard option

[Marc-André Lureau]

Hi

On Thu, Jan 18, 2018 at 10:31 AM, Christophe Fergeau
 wrote:
> At least on X.org, malicious code could run the equivalent of "watch
> xsel -o --clipboard" in a VM, and would then be able to track all the
> clipboard content, even when the spice-gtk widget is not focused.
>
> At the moment, applications call spice_set_session_option(), and then
> set SpiceGtkSession::auto-clipboard to TRUE (or to its saved state).
> This commit adds a --spice-disable-clipboard option, and if it's set,
> SpiceGtkSession::auto-clipboard will not be changeable and will always
> be FALSE.
> The only side effect I noticed is that enabling "clipboard sharing" in
> GNOME Boxes VM preferences will appear to work, but will not enable
> clipboard, and will be reset to off next time the preferences dialog is
> open.
>
> https://bugzilla.redhat.com/show_bug.cgi?id=1320263

Looks reasonable to me. However, I thought we wanted a way to disable
clipboard by default.

Wouldn't it make sense to introduce some GSetting key(s) for that instead?

This way, the behaviour can be enforced globally without changing the
way applications are started.

> ---
>  man/spice-client.pod |  4 
>  src/spice-gtk-session.c  | 16 +++-
>  src/spice-option.c   |  6 ++
>  src/spice-session-priv.h |  1 +
>  src/spice-session.c  | 35 +++
>  5 files changed, 61 insertions(+), 1 deletion(-)
>
> diff --git a/man/spice-client.pod b/man/spice-client.pod
> index 7288b84e..76a7d207 100644
> --- a/man/spice-client.pod
> +++ b/man/spice-client.pod
> @@ -111,6 +111,10 @@ SPICE_DEBUG environment variable, or using 
> G_MESSAGES_DEBUG=all
>
>  Disable audio support
>
> +=item --spice-disable-clipboard
> +
> +Disable clipboard sharing between client OS and guest OS
> +
>  =item --spice-disable-usbredir
>
>  Disable USB redirection support
> diff --git a/src/spice-gtk-session.c b/src/spice-gtk-session.c
> index 31f60dc4..c3546209 100644
> --- a/src/spice-gtk-session.c
> +++ b/src/spice-gtk-session.c
> @@ -309,6 +309,20 @@ static void spice_gtk_session_finalize(GObject *gobject)
>  G_OBJECT_CLASS(spice_gtk_session_parent_class)->finalize(gobject);
>  }
>
> +static void spice_gtk_session_set_auto_clipboard(SpiceGtkSession *self,
> + gboolean auto_clipboard)
> +{
> +gboolean disable_clipboard;
> +g_object_get(self->priv->session,
> + "disable-clipboard", _clipboard,
> + NULL);
> +if (disable_clipboard && auto_clipboard) {
> +g_debug("not enabling clipboard sharing as it was disabled on the 
> command-line");
> +auto_clipboard = FALSE;
> +}
> +self->priv->auto_clipboard_enable = auto_clipboard;
> +}
> +
>  static void spice_gtk_session_get_property(GObject*gobject,
> guint   prop_id,
> GValue *value,
> @@ -352,7 +366,7 @@ static void spice_gtk_session_set_property(GObject  
> *gobject,
>  s->session = g_value_get_object(value);
>  break;
>  case PROP_AUTO_CLIPBOARD:
> -s->auto_clipboard_enable = g_value_get_boolean(value);
> +spice_gtk_session_set_auto_clipboard(self, 
> g_value_get_boolean(value));
>  break;
>  case PROP_AUTO_USBREDIR: {
>  SpiceDesktopIntegration *desktop_int;
> diff --git a/src/spice-option.c b/src/spice-option.c
> index 6b400bca..0f2f795d 100644
> --- a/src/spice-option.c
> +++ b/src/spice-option.c
> @@ -21,6 +21,7 @@
>  #include 
>  #include 
>  #include "spice-session.h"
> +#include "spice-session-priv.h"
>  #include "spice-util.h"
>  #include "spice-channel-priv.h"
>  #include "usb-device-manager.h"
> @@ -35,6 +36,7 @@ static char *usbredir_auto_redirect_filter = NULL;
>  static char *usbredir_redirect_on_connect = NULL;
>  static gboolean smartcard = FALSE;
>  static gboolean disable_audio = FALSE;
> +static gboolean disable_clipboard = FALSE;
>  static gboolean disable_usbredir = FALSE;
>  static gint cache_size = 0;
>  static gint glz_window_size = 0;
> @@ -203,6 +205,8 @@ GOptionGroup* spice_get_option_group(void)
>N_("Subject of the host certificate (field=value pairs separated 
> by commas)"), N_("") },
>  { "spice-disable-audio", '\0', 0, G_OPTION_ARG_NONE, _audio,
>N_("Disable audio support"), NULL },
> +{ "spice-disable-clipboard", '\0', 0, G_OPTION_ARG_NONE, 
> _clipboard,
> +  N_("Disable client/guest clipboard sharing"), NULL },
>  { "spice-smartcard", '\0', 0, G_OPTION_ARG_NONE, ,
>N_("Enable smartcard support"), NULL },
>  { "spice-smartcard-certificates", '\0', 0, G_OPTION_ARG_STRING, 
> _certificates,
> @@ -312,6 +316,8 @@ void spice_set_session_option(SpiceSession *session)
>  g_object_set(session, "enable-usbredir", FALSE, NULL);
>  if (disable_audio)
>  g_object_set(session, 

[Spice-devel] [spice-gtk] Add --spice-disable-clipboard option

[Christophe Fergeau]

At least on X.org, malicious code could run the equivalent of "watch
xsel -o --clipboard" in a VM, and would then be able to track all the
clipboard content, even when the spice-gtk widget is not focused.

At the moment, applications call spice_set_session_option(), and then
set SpiceGtkSession::auto-clipboard to TRUE (or to its saved state).
This commit adds a --spice-disable-clipboard option, and if it's set,
SpiceGtkSession::auto-clipboard will not be changeable and will always
be FALSE.
The only side effect I noticed is that enabling "clipboard sharing" in
GNOME Boxes VM preferences will appear to work, but will not enable
clipboard, and will be reset to off next time the preferences dialog is
open.

https://bugzilla.redhat.com/show_bug.cgi?id=1320263
---
 man/spice-client.pod |  4 
 src/spice-gtk-session.c  | 16 +++-
 src/spice-option.c   |  6 ++
 src/spice-session-priv.h |  1 +
 src/spice-session.c  | 35 +++
 5 files changed, 61 insertions(+), 1 deletion(-)

diff --git a/man/spice-client.pod b/man/spice-client.pod
index 7288b84e..76a7d207 100644
--- a/man/spice-client.pod
+++ b/man/spice-client.pod
@@ -111,6 +111,10 @@ SPICE_DEBUG environment variable, or using 
G_MESSAGES_DEBUG=all
 
 Disable audio support
 
+=item --spice-disable-clipboard
+
+Disable clipboard sharing between client OS and guest OS
+
 =item --spice-disable-usbredir
 
 Disable USB redirection support
diff --git a/src/spice-gtk-session.c b/src/spice-gtk-session.c
index 31f60dc4..c3546209 100644
--- a/src/spice-gtk-session.c
+++ b/src/spice-gtk-session.c
@@ -309,6 +309,20 @@ static void spice_gtk_session_finalize(GObject *gobject)
 G_OBJECT_CLASS(spice_gtk_session_parent_class)->finalize(gobject);
 }
 
+static void spice_gtk_session_set_auto_clipboard(SpiceGtkSession *self,
+ gboolean auto_clipboard)
+{
+gboolean disable_clipboard;
+g_object_get(self->priv->session,
+ "disable-clipboard", _clipboard,
+ NULL);
+if (disable_clipboard && auto_clipboard) {
+g_debug("not enabling clipboard sharing as it was disabled on the 
command-line");
+auto_clipboard = FALSE;
+}
+self->priv->auto_clipboard_enable = auto_clipboard;
+}
+
 static void spice_gtk_session_get_property(GObject*gobject,
guint   prop_id,
GValue *value,
@@ -352,7 +366,7 @@ static void spice_gtk_session_set_property(GObject  
*gobject,
 s->session = g_value_get_object(value);
 break;
 case PROP_AUTO_CLIPBOARD:
-s->auto_clipboard_enable = g_value_get_boolean(value);
+spice_gtk_session_set_auto_clipboard(self, g_value_get_boolean(value));
 break;
 case PROP_AUTO_USBREDIR: {
 SpiceDesktopIntegration *desktop_int;
diff --git a/src/spice-option.c b/src/spice-option.c
index 6b400bca..0f2f795d 100644
--- a/src/spice-option.c
+++ b/src/spice-option.c
@@ -21,6 +21,7 @@
 #include 
 #include 
 #include "spice-session.h"
+#include "spice-session-priv.h"
 #include "spice-util.h"
 #include "spice-channel-priv.h"
 #include "usb-device-manager.h"
@@ -35,6 +36,7 @@ static char *usbredir_auto_redirect_filter = NULL;
 static char *usbredir_redirect_on_connect = NULL;
 static gboolean smartcard = FALSE;
 static gboolean disable_audio = FALSE;
+static gboolean disable_clipboard = FALSE;
 static gboolean disable_usbredir = FALSE;
 static gint cache_size = 0;
 static gint glz_window_size = 0;
@@ -203,6 +205,8 @@ GOptionGroup* spice_get_option_group(void)
   N_("Subject of the host certificate (field=value pairs separated by 
commas)"), N_("") },
 { "spice-disable-audio", '\0', 0, G_OPTION_ARG_NONE, _audio,
   N_("Disable audio support"), NULL },
+{ "spice-disable-clipboard", '\0', 0, G_OPTION_ARG_NONE, 
_clipboard,
+  N_("Disable client/guest clipboard sharing"), NULL },
 { "spice-smartcard", '\0', 0, G_OPTION_ARG_NONE, ,
   N_("Enable smartcard support"), NULL },
 { "spice-smartcard-certificates", '\0', 0, G_OPTION_ARG_STRING, 
_certificates,
@@ -312,6 +316,8 @@ void spice_set_session_option(SpiceSession *session)
 g_object_set(session, "enable-usbredir", FALSE, NULL);
 if (disable_audio)
 g_object_set(session, "enable-audio", FALSE, NULL);
+if (disable_clipboard)
+spice_session_set_disable_clipboard(session, TRUE);
 if (cache_size)
 g_object_set(session, "cache-size", cache_size, NULL);
 if (glz_window_size)
diff --git a/src/spice-session-priv.h b/src/spice-session-priv.h
index 03005aac..459a04b8 100644
--- a/src/spice-session-priv.h
+++ b/src/spice-session-priv.h
@@ -91,6 +91,7 @@ void spice_session_set_shared_dir(SpiceSession *session, 
const gchar *dir);
 gboolean spice_session_get_audio_enabled(SpiceSession *session);
 gboolean