Re: [Spice-devel] RFC [spice-gtk] session: Allow to delay sending clipboard to the guest

[Christophe de Dinechin]

> On 17 Jan 2018, at 11:16, Javier Celaya  wrote:
> 
> El vie, 12-01-2018 a las 08:05 -0500, Marc-André Lureau escribió:
>> Hi
>> 
>> - Original Message -
>>> On Thu, Jan 11, 2018 at 12:35:36PM -0500, Marc-André Lureau wrote:
> I agree with you that some help from the windowing/toolkit
> would be good
> to have, but in this case, I doubt we are going to be able to
> do better
> than managing this in spice-gtk.
 
 Yet it is already being solved at a lower level, where you can
 actually
 enforce that behaviour.
>>> 
>>> Yes, it is solved with wayland. The question I'm asking/the problem
>>> I'm
>>> trying to solve is what do we do for existing systems using Xorg
>>> and
>>> gtk+3. With Xorg being phased out (which will still take a few
>>> years),
>>> and gtk+3 being phased out (again, will take at least a few years),
>>> I
>>> don't see this kind of clipboard behaviour changes going into
>>> either of
>>> these. Maybe I'm wrong, but assuming I'm not, then either we fix it
>>> ("it" being xorg + gtk3) in spice-gtk even though that's not the
>>> best
>>> place, or we don't fix it at all.
>>> 
>>> If we decide to do something in spice-gtk, one option is to only
>>> send
>>> the clipboard when the window is focused, which will reduce the
>>> attack
>>> surface for everyone, and hopefully will have minimal impact.
>>> Another option (which is not exclusive) is to add command-
>>> line/runtime
>>> ways of enabling/disabling clipboard sharing, which you will either
>>> have
>>> to know about it if it's enabled by default, or will be quite
>>> disruptive
>>> if we disable clipboard sharing by default.
>> 
>> Is it really a security reason the clipboard behaviour is different
>> on Wayland? For me, this "share on focus" is not a more secure
>> behaviour.
> 
> If I may, IMHO spicy is doing the "secure" thing here: You can select
> to either share the clipboard automatically or manually with the
> corresponding UI actions (copy/paste to/from guest). Doing it manually
> will never expose your clipboard to the guest unadvertedly.

Just curious, how does that work if you select a “Paste” menu option with the 
mouse instead of keyboard?


Thanks
Christophe

> 
> Adding these actions to other SPICE clients requires more work than the
> "share on focus" feature, but as Marc-André says, some UI changes will
> be required anyway.
> 
>> 
>>> 
>>> I'd lean towards doing "clipboard sharing for focused client" +
>>> "command-line/runtime option, with clipboard sharing enabled by
>>> default".
>> 
>> I'd rather stick with a simple command-line & runtime option.
>> ___
>> Spice-devel mailing list
>> Spice-devel@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/spice-devel
>> 
> -- 
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 
>Javier Celaya
> 
> 
> 
> 
>Chief Technology Officer
> 
> 
> 
> 
> 
> 
> 
>javier.cel...@flexvdi.com
> 
> 
> 
> 
> 
>+34 696 969 959 
> 
> 
> 
> 
> 
>@j_celaya
> 
> 
> 
> 
>Legal Information and Privacy Policy
> 
> 
> 
> 
> ___
> Spice-devel mailing list
> Spice-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel

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

Re: [Spice-devel] RFC [spice-gtk] session: Allow to delay sending clipboard to the guest

[Javier Celaya]

El vie, 12-01-2018 a las 08:05 -0500, Marc-André Lureau escribió:
> Hi
> 
> - Original Message -
> > On Thu, Jan 11, 2018 at 12:35:36PM -0500, Marc-André Lureau wrote:
> > > > I agree with you that some help from the windowing/toolkit
> > > > would be good
> > > > to have, but in this case, I doubt we are going to be able to
> > > > do better
> > > > than managing this in spice-gtk.
> > > 
> > > Yet it is already being solved at a lower level, where you can
> > > actually
> > > enforce that behaviour.
> > 
> > Yes, it is solved with wayland. The question I'm asking/the problem
> > I'm
> > trying to solve is what do we do for existing systems using Xorg
> > and
> > gtk+3. With Xorg being phased out (which will still take a few
> > years),
> > and gtk+3 being phased out (again, will take at least a few years),
> > I
> > don't see this kind of clipboard behaviour changes going into
> > either of
> > these. Maybe I'm wrong, but assuming I'm not, then either we fix it
> > ("it" being xorg + gtk3) in spice-gtk even though that's not the
> > best
> > place, or we don't fix it at all.
> > 
> > If we decide to do something in spice-gtk, one option is to only
> > send
> > the clipboard when the window is focused, which will reduce the
> > attack
> > surface for everyone, and hopefully will have minimal impact.
> > Another option (which is not exclusive) is to add command-
> > line/runtime
> > ways of enabling/disabling clipboard sharing, which you will either
> > have
> > to know about it if it's enabled by default, or will be quite
> > disruptive
> > if we disable clipboard sharing by default.
> 
> Is it really a security reason the clipboard behaviour is different
> on Wayland? For me, this "share on focus" is not a more secure
> behaviour.

If I may, IMHO spicy is doing the "secure" thing here: You can select
to either share the clipboard automatically or manually with the
corresponding UI actions (copy/paste to/from guest). Doing it manually
will never expose your clipboard to the guest unadvertedly.

Adding these actions to other SPICE clients requires more work than the
"share on focus" feature, but as Marc-André says, some UI changes will
be required anyway.

> 
> > 
> > I'd lean towards doing "clipboard sharing for focused client" +
> > "command-line/runtime option, with clipboard sharing enabled by
> > default".
> 
> I'd rather stick with a simple command-line & runtime option.
> ___
> Spice-devel mailing list
> Spice-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel
> 
-- 

 

 






  

  
  

  

  

  
  
Javier Celaya
  

  
  
Chief Technology Officer




  
  

javier.cel...@flexvdi.com
  

  
   

+34 696 969 959 
  

  
  
 
@j_celaya
  

  

Legal Information and Privacy Policy
  


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

Re: [Spice-devel] RFC [spice-gtk] session: Allow to delay sending clipboard to the guest

[Christophe Fergeau]

On Fri, Jan 12, 2018 at 08:05:24AM -0500, Marc-André Lureau wrote:
> 
> Is it really a security reason the clipboard behaviour is different on
> Wayland?

I don't know the reason for this behaviour, for me this is similar to
preventing applications from capturing the whole screen.
https://wiki.x.org/wiki/Events/XDC2014/XDC2014DodierPeresSecurity/xorg-talk.pdf
has a slide with
"Some common GUI requirements are un-secure by design
• Clipboard monitoring"
so at least some people wanted to fix some security problems in xorg
clipboard implementation.

> For me, this "share on focus" is not a more secure behaviour.

"VM can monitor everything which goes in your clipboard while you are
not using it" VS "VM can get what is in your clipboard when you switch
to it" sounds more secure to me, even if not perfect.

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] RFC [spice-gtk] session: Allow to delay sending clipboard to the guest

[Marc-André Lureau]

Hi

- Original Message -
> > 
> > Hi
> > 
> > - Original Message -
> > > On Thu, Jan 11, 2018 at 12:35:36PM -0500, Marc-André Lureau wrote:
> > > > > I agree with you that some help from the windowing/toolkit would be
> > > > > good
> > > > > to have, but in this case, I doubt we are going to be able to do
> > > > > better
> > > > > than managing this in spice-gtk.
> > > > 
> > > > Yet it is already being solved at a lower level, where you can actually
> > > > enforce that behaviour.
> > > 
> > > Yes, it is solved with wayland. The question I'm asking/the problem I'm
> > > trying to solve is what do we do for existing systems using Xorg and
> > > gtk+3. With Xorg being phased out (which will still take a few years),
> > > and gtk+3 being phased out (again, will take at least a few years), I
> > > don't see this kind of clipboard behaviour changes going into either of
> > > these. Maybe I'm wrong, but assuming I'm not, then either we fix it
> > > ("it" being xorg + gtk3) in spice-gtk even though that's not the best
> > > place, or we don't fix it at all.
> > > 
> > > If we decide to do something in spice-gtk, one option is to only send
> > > the clipboard when the window is focused, which will reduce the attack
> > > surface for everyone, and hopefully will have minimal impact.
> > > Another option (which is not exclusive) is to add command-line/runtime
> > > ways of enabling/disabling clipboard sharing, which you will either have
> > > to know about it if it's enabled by default, or will be quite disruptive
> > > if we disable clipboard sharing by default.
> > 
> > Is it really a security reason the clipboard behaviour is different on
> > Wayland? For me, this "share on focus" is not a more secure behaviour.
> > 
> 
> The bug report explains in more detail the use case.
> Is describing an administrator user with different client opened sometimes
> having to copy some password with tools like keepass.
> These tools usually clear the clipboard after some time.
> Yes, in this case the user would be better not to be connected to VMs
> and surely having clipboard support disabled is safer but for security
> clipboard should be disable by default, not with an option.

Yes, disabled by default

> Considering that this is considered low impact a workaround like the
> focus limitation is enough.
> One more good thing also about the focus limitation is that if you
> keep multiple VM connected but you don't interact with them you
> don't send clipboard messages potentially reducing th network usage.

very marginaly (it's not the content)

> > > 
> > > I'd lean towards doing "clipboard sharing for focused client" +
> > > "command-line/runtime option, with clipboard sharing enabled by
> > > default".
> > 
> > I'd rather stick with a simple command-line & runtime option.
> 
> Not so simple considering that you have to patch different tools
> (boxes, virt-manager, virt-viewer) while just changing spice-gtk
> would be a single patch.

Disabling by default, and command line option is in spice-gtk.

Yes, a UI change is needed in the clients, but I believe that will be needed 
anyway to secure it.
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] RFC [spice-gtk] session: Allow to delay sending clipboard to the guest

[Frediano Ziglio]

> 
> Hi
> 
> - Original Message -
> > On Thu, Jan 11, 2018 at 12:35:36PM -0500, Marc-André Lureau wrote:
> > > > I agree with you that some help from the windowing/toolkit would be
> > > > good
> > > > to have, but in this case, I doubt we are going to be able to do better
> > > > than managing this in spice-gtk.
> > > 
> > > Yet it is already being solved at a lower level, where you can actually
> > > enforce that behaviour.
> > 
> > Yes, it is solved with wayland. The question I'm asking/the problem I'm
> > trying to solve is what do we do for existing systems using Xorg and
> > gtk+3. With Xorg being phased out (which will still take a few years),
> > and gtk+3 being phased out (again, will take at least a few years), I
> > don't see this kind of clipboard behaviour changes going into either of
> > these. Maybe I'm wrong, but assuming I'm not, then either we fix it
> > ("it" being xorg + gtk3) in spice-gtk even though that's not the best
> > place, or we don't fix it at all.
> > 
> > If we decide to do something in spice-gtk, one option is to only send
> > the clipboard when the window is focused, which will reduce the attack
> > surface for everyone, and hopefully will have minimal impact.
> > Another option (which is not exclusive) is to add command-line/runtime
> > ways of enabling/disabling clipboard sharing, which you will either have
> > to know about it if it's enabled by default, or will be quite disruptive
> > if we disable clipboard sharing by default.
> 
> Is it really a security reason the clipboard behaviour is different on
> Wayland? For me, this "share on focus" is not a more secure behaviour.
> 

The bug report explains in more detail the use case.
Is describing an administrator user with different client opened sometimes
having to copy some password with tools like keepass.
These tools usually clear the clipboard after some time.
Yes, in this case the user would be better not to be connected to VMs
and surely having clipboard support disabled is safer but for security
clipboard should be disable by default, not with an option.
Considering that this is considered low impact a workaround like the
focus limitation is enough.
One more good thing also about the focus limitation is that if you
keep multiple VM connected but you don't interact with them you
don't send clipboard messages potentially reducing th network usage.

> > 
> > I'd lean towards doing "clipboard sharing for focused client" +
> > "command-line/runtime option, with clipboard sharing enabled by
> > default".
> 
> I'd rather stick with a simple command-line & runtime option.

Not so simple considering that you have to patch different tools
(boxes, virt-manager, virt-viewer) while just changing spice-gtk
would be a single patch.

Frediano
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] RFC [spice-gtk] session: Allow to delay sending clipboard to the guest

[Marc-André Lureau]

Hi

- Original Message -
> On Thu, Jan 11, 2018 at 12:35:36PM -0500, Marc-André Lureau wrote:
> > > I agree with you that some help from the windowing/toolkit would be good
> > > to have, but in this case, I doubt we are going to be able to do better
> > > than managing this in spice-gtk.
> > 
> > Yet it is already being solved at a lower level, where you can actually
> > enforce that behaviour.
> 
> Yes, it is solved with wayland. The question I'm asking/the problem I'm
> trying to solve is what do we do for existing systems using Xorg and
> gtk+3. With Xorg being phased out (which will still take a few years),
> and gtk+3 being phased out (again, will take at least a few years), I
> don't see this kind of clipboard behaviour changes going into either of
> these. Maybe I'm wrong, but assuming I'm not, then either we fix it
> ("it" being xorg + gtk3) in spice-gtk even though that's not the best
> place, or we don't fix it at all.
> 
> If we decide to do something in spice-gtk, one option is to only send
> the clipboard when the window is focused, which will reduce the attack
> surface for everyone, and hopefully will have minimal impact.
> Another option (which is not exclusive) is to add command-line/runtime
> ways of enabling/disabling clipboard sharing, which you will either have
> to know about it if it's enabled by default, or will be quite disruptive
> if we disable clipboard sharing by default.

Is it really a security reason the clipboard behaviour is different on Wayland? 
For me, this "share on focus" is not a more secure behaviour.

> 
> I'd lean towards doing "clipboard sharing for focused client" +
> "command-line/runtime option, with clipboard sharing enabled by
> default".

I'd rather stick with a simple command-line & runtime option.
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] RFC [spice-gtk] session: Allow to delay sending clipboard to the guest

[Christophe Fergeau]

On Thu, Jan 11, 2018 at 12:35:36PM -0500, Marc-André Lureau wrote:
> > I agree with you that some help from the windowing/toolkit would be good
> > to have, but in this case, I doubt we are going to be able to do better
> > than managing this in spice-gtk.
> 
> Yet it is already being solved at a lower level, where you can actually 
> enforce that behaviour.

Yes, it is solved with wayland. The question I'm asking/the problem I'm
trying to solve is what do we do for existing systems using Xorg and
gtk+3. With Xorg being phased out (which will still take a few years),
and gtk+3 being phased out (again, will take at least a few years), I
don't see this kind of clipboard behaviour changes going into either of
these. Maybe I'm wrong, but assuming I'm not, then either we fix it
("it" being xorg + gtk3) in spice-gtk even though that's not the best
place, or we don't fix it at all.

If we decide to do something in spice-gtk, one option is to only send
the clipboard when the window is focused, which will reduce the attack
surface for everyone, and hopefully will have minimal impact.
Another option (which is not exclusive) is to add command-line/runtime
ways of enabling/disabling clipboard sharing, which you will either have
to know about it if it's enabled by default, or will be quite disruptive
if we disable clipboard sharing by default.

I'd lean towards doing "clipboard sharing for focused client" +
"command-line/runtime option, with clipboard sharing enabled by
default".

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] RFC [spice-gtk] session: Allow to delay sending clipboard to the guest

[Marc-André Lureau]

Hi

- Original Message -
> On Thu, Jan 11, 2018 at 11:42:00AM -0500, Marc-André Lureau wrote:
> > 
> > > (it seems that clipboard system for GTK 4 has been reworked quite a
> > > bit - see https://git.gnome.org/browse/gtk+/log/?h=wip/otte/clipboard
> > > - this is already merged into master)
> > 
> > I have not much time to look at the details, but I don't think we
> > should rush into changing the behavior of the clipboard in spice only.
> > This is more windowing/toolkit level issue.
> 
> The impact of doing this in gtk3 would be greater than doing it in SPICE
> only, I don't really expect gtk+ people to be open to such a change in
> gtk3 at this point. But yes I did not ask, so I could have a nice
> surprise ;)

The impact is even bigger if you think that they did it already at the wayland 
level...

> Once again, spice-gtk is a very special gtk+ application in that one of
> its features is to let arbitrary code interact with the clipboard. This
> is not the case of the case of most gtk+ applications.

spice is proxying the events. You should be aware of what you are running in 
the guest if you enable clipboard sharing.

fwiw, there are other cases where "arbitrary code" could interact with a 
clipboard: 
https://stackoverflow.com/questions/6413036/get-current-clipboard-content/6413100#6413100

> I agree with you that some help from the windowing/toolkit would be good
> to have, but in this case, I doubt we are going to be able to do better
> than managing this in spice-gtk.

Yet it is already being solved at a lower level, where you can actually enforce 
that behaviour.
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] RFC [spice-gtk] session: Allow to delay sending clipboard to the guest

[Christophe Fergeau]

On Thu, Jan 11, 2018 at 11:42:00AM -0500, Marc-André Lureau wrote:
> 
> > (it seems that clipboard system for GTK 4 has been reworked quite a
> > bit - see https://git.gnome.org/browse/gtk+/log/?h=wip/otte/clipboard
> > - this is already merged into master)
> 
> I have not much time to look at the details, but I don't think we
> should rush into changing the behavior of the clipboard in spice only.
> This is more windowing/toolkit level issue.

The impact of doing this in gtk3 would be greater than doing it in SPICE
only, I don't really expect gtk+ people to be open to such a change in
gtk3 at this point. But yes I did not ask, so I could have a nice
surprise ;)
Once again, spice-gtk is a very special gtk+ application in that one of
its features is to let arbitrary code interact with the clipboard. This
is not the case of the case of most gtk+ applications.
I agree with you that some help from the windowing/toolkit would be good
to have, but in this case, I doubt we are going to be able to do better
than managing this in spice-gtk.

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] RFC [spice-gtk] session: Allow to delay sending clipboard to the guest

[Jakub Janku]

On Thu, Jan 11, 2018 at 5:42 PM, Marc-André Lureau
 wrote:
> Hi
>
> - Original Message -
>> On Thu, Jan 11, 2018 at 3:29 PM, Marc-André Lureau
>>  wrote:
>> >
>> > Hi
>> >
>> > - Original Message -
>> > > On Wed, Jan 10, 2018 at 06:48:14PM -0500, Marc-André Lureau wrote:
>> > > > Hi
>> > > >
>> > > > - Original Message -
>> > > > > On Tue, Jan 09, 2018 at 12:16:33PM -0500, Marc-André Lureau wrote:
>> > > > > > I think it's problematic for traditional applications as well.
>> > > > > > clipboard access is probably going to be limited by default and
>> > > > > > only
>> > > > > > accessed through so-called "portals", just like file access etc.
>> > > > > > This
>> > > > > > topic should be brought on desktop / flatpak mailing list.
>> > > > >
>> > > > > Maybe in some distant future, all applications everyone is running
>> > > > > will
>> > > > > be flatpak, and will be using portals to improve security. The same
>> > > > > thing can be said regarding wayland, which does not have this issue.
>> > > > > Some time in the future, this will become a non-issue. However,
>> > > > > solving
>> > > > > this now on x11 is definitely not something which should be related
>> > > > > to
>> > > > > portals/flatpak in my opinion.
>> > > >
>> > > > I propose a --spice-disable-clipboard, and client UI to switch on/off
>> > > > clipboard sharing functionality.
>> > > >
>> > > > Something different will likely break some clipboard users or lower
>> > > > experience.
>> > >
>> > > One additional note on that, I was initially worried about which use
>> > > cases were going to be broken by these changes. Then I realized that the
>> > > very same use cases would be broken when using wayland (after taking into
>> > > account Jakub's comments). Since this potential breakage will happen
>> > > anyway whether we want it or not, it's not going to make a big
>> > > difference if we do the same when running on  X11.
>> >
>> > So this change isn't needed for Wayland, and your patch changes the
>> > clipboard behaviour to be similar as Wayland.
>> >
>> > Why couldn't this be done at Gtk level? This would give a similar clipboard
>> > behaviour for all Gtk app wether they run on Wayland or X, or windows etc.
>> > They would also benefit the same "added security".
>>
>> Do you think this change would make it to GTK3? I think it could
>> potentially break some apps. Note that e.g. spice-vdagent takes
>> advantage of this "security issue" in X11 - it has no window and
>> listens for the clipboard changes all the time.
>
> Those gtk applications would break on wayland today anyway, right?

Probably, unless you force GTK to use XWayland with
gdk_set_allowed_backends("x11").
But pushing this kind of change to GTK 3.22, which is supposed to be
stable, doesn't seem right to me.

>
>> (it seems that clipboard system for GTK 4 has been reworked quite a
>> bit - see https://git.gnome.org/browse/gtk+/log/?h=wip/otte/clipboard
>> - this is already merged into master)
>
> I have not much time to look at the details, but I don't think we should rush 
> into changing the behavior of the clipboard in spice only. This is more 
> windowing/toolkit level issue.

That's right, I just wanted to say it might take considerable amount
of time to fix this issue in spice if we were patching GTK.

Jakub
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] RFC [spice-gtk] session: Allow to delay sending clipboard to the guest

[Marc-André Lureau]

Hi

- Original Message -
> On Thu, Jan 11, 2018 at 3:29 PM, Marc-André Lureau
>  wrote:
> >
> > Hi
> >
> > - Original Message -
> > > On Wed, Jan 10, 2018 at 06:48:14PM -0500, Marc-André Lureau wrote:
> > > > Hi
> > > >
> > > > - Original Message -
> > > > > On Tue, Jan 09, 2018 at 12:16:33PM -0500, Marc-André Lureau wrote:
> > > > > > I think it's problematic for traditional applications as well.
> > > > > > clipboard access is probably going to be limited by default and
> > > > > > only
> > > > > > accessed through so-called "portals", just like file access etc.
> > > > > > This
> > > > > > topic should be brought on desktop / flatpak mailing list.
> > > > >
> > > > > Maybe in some distant future, all applications everyone is running
> > > > > will
> > > > > be flatpak, and will be using portals to improve security. The same
> > > > > thing can be said regarding wayland, which does not have this issue.
> > > > > Some time in the future, this will become a non-issue. However,
> > > > > solving
> > > > > this now on x11 is definitely not something which should be related
> > > > > to
> > > > > portals/flatpak in my opinion.
> > > >
> > > > I propose a --spice-disable-clipboard, and client UI to switch on/off
> > > > clipboard sharing functionality.
> > > >
> > > > Something different will likely break some clipboard users or lower
> > > > experience.
> > >
> > > One additional note on that, I was initially worried about which use
> > > cases were going to be broken by these changes. Then I realized that the
> > > very same use cases would be broken when using wayland (after taking into
> > > account Jakub's comments). Since this potential breakage will happen
> > > anyway whether we want it or not, it's not going to make a big
> > > difference if we do the same when running on  X11.
> >
> > So this change isn't needed for Wayland, and your patch changes the
> > clipboard behaviour to be similar as Wayland.
> >
> > Why couldn't this be done at Gtk level? This would give a similar clipboard
> > behaviour for all Gtk app wether they run on Wayland or X, or windows etc.
> > They would also benefit the same "added security".
> 
> Do you think this change would make it to GTK3? I think it could
> potentially break some apps. Note that e.g. spice-vdagent takes
> advantage of this "security issue" in X11 - it has no window and
> listens for the clipboard changes all the time.

Those gtk applications would break on wayland today anyway, right?

> (it seems that clipboard system for GTK 4 has been reworked quite a
> bit - see https://git.gnome.org/browse/gtk+/log/?h=wip/otte/clipboard
> - this is already merged into master)

I have not much time to look at the details, but I don't think we should rush 
into changing the behavior of the clipboard in spice only. This is more 
windowing/toolkit level issue.
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] RFC [spice-gtk] session: Allow to delay sending clipboard to the guest

[Jakub Janku]

On Thu, Jan 11, 2018 at 3:29 PM, Marc-André Lureau
 wrote:
>
> Hi
>
> - Original Message -
> > On Wed, Jan 10, 2018 at 06:48:14PM -0500, Marc-André Lureau wrote:
> > > Hi
> > >
> > > - Original Message -
> > > > On Tue, Jan 09, 2018 at 12:16:33PM -0500, Marc-André Lureau wrote:
> > > > > I think it's problematic for traditional applications as well.
> > > > > clipboard access is probably going to be limited by default and only
> > > > > accessed through so-called "portals", just like file access etc. This
> > > > > topic should be brought on desktop / flatpak mailing list.
> > > >
> > > > Maybe in some distant future, all applications everyone is running will
> > > > be flatpak, and will be using portals to improve security. The same
> > > > thing can be said regarding wayland, which does not have this issue.
> > > > Some time in the future, this will become a non-issue. However, solving
> > > > this now on x11 is definitely not something which should be related to
> > > > portals/flatpak in my opinion.
> > >
> > > I propose a --spice-disable-clipboard, and client UI to switch on/off
> > > clipboard sharing functionality.
> > >
> > > Something different will likely break some clipboard users or lower
> > > experience.
> >
> > One additional note on that, I was initially worried about which use
> > cases were going to be broken by these changes. Then I realized that the
> > very same use cases would be broken when using wayland (after taking into
> > account Jakub's comments). Since this potential breakage will happen
> > anyway whether we want it or not, it's not going to make a big
> > difference if we do the same when running on  X11.
>
> So this change isn't needed for Wayland, and your patch changes the clipboard 
> behaviour to be similar as Wayland.
>
> Why couldn't this be done at Gtk level? This would give a similar clipboard 
> behaviour for all Gtk app wether they run on Wayland or X, or windows etc. 
> They would also benefit the same "added security".

Do you think this change would make it to GTK3? I think it could
potentially break some apps. Note that e.g. spice-vdagent takes
advantage of this "security issue" in X11 - it has no window and
listens for the clipboard changes all the time.
(it seems that clipboard system for GTK 4 has been reworked quite a
bit - see https://git.gnome.org/browse/gtk+/log/?h=wip/otte/clipboard
- this is already merged into master)

Cheers,
  Jakub
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] RFC [spice-gtk] session: Allow to delay sending clipboard to the guest

[Marc-André Lureau]

Hi

- Original Message -
> On Wed, Jan 10, 2018 at 06:48:14PM -0500, Marc-André Lureau wrote:
> > Hi
> > 
> > - Original Message -
> > > On Tue, Jan 09, 2018 at 12:16:33PM -0500, Marc-André Lureau wrote:
> > > > I think it's problematic for traditional applications as well.
> > > > clipboard access is probably going to be limited by default and only
> > > > accessed through so-called "portals", just like file access etc. This
> > > > topic should be brought on desktop / flatpak mailing list.
> > > 
> > > Maybe in some distant future, all applications everyone is running will
> > > be flatpak, and will be using portals to improve security. The same
> > > thing can be said regarding wayland, which does not have this issue.
> > > Some time in the future, this will become a non-issue. However, solving
> > > this now on x11 is definitely not something which should be related to
> > > portals/flatpak in my opinion.
> > 
> > I propose a --spice-disable-clipboard, and client UI to switch on/off
> > clipboard sharing functionality.
> > 
> > Something different will likely break some clipboard users or lower
> > experience.
> 
> One additional note on that, I was initially worried about which use
> cases were going to be broken by these changes. Then I realized that the
> very same use cases would be broken when using wayland (after taking into
> account Jakub's comments). Since this potential breakage will happen
> anyway whether we want it or not, it's not going to make a big
> difference if we do the same when running on  X11.

So this change isn't needed for Wayland, and your patch changes the clipboard 
behaviour to be similar as Wayland.

Why couldn't this be done at Gtk level? This would give a similar clipboard 
behaviour for all Gtk app wether they run on Wayland or X, or windows etc. They 
would also benefit the same "added security".
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] RFC [spice-gtk] session: Allow to delay sending clipboard to the guest

[Christophe Fergeau]

On Wed, Jan 10, 2018 at 06:48:14PM -0500, Marc-André Lureau wrote:
> Hi
> 
> - Original Message -
> > On Tue, Jan 09, 2018 at 12:16:33PM -0500, Marc-André Lureau wrote:
> > > I think it's problematic for traditional applications as well.
> > > clipboard access is probably going to be limited by default and only
> > > accessed through so-called "portals", just like file access etc. This
> > > topic should be brought on desktop / flatpak mailing list.
> > 
> > Maybe in some distant future, all applications everyone is running will
> > be flatpak, and will be using portals to improve security. The same
> > thing can be said regarding wayland, which does not have this issue.
> > Some time in the future, this will become a non-issue. However, solving
> > this now on x11 is definitely not something which should be related to
> > portals/flatpak in my opinion.
> 
> I propose a --spice-disable-clipboard, and client UI to switch on/off 
> clipboard sharing functionality.
> 
> Something different will likely break some clipboard users or lower 
> experience.

One additional note on that, I was initially worried about which use
cases were going to be broken by these changes. Then I realized that the
very same use cases would be broken when using wayland (after taking into
account Jakub's comments). Since this potential breakage will happen
anyway whether we want it or not, it's not going to make a big
difference if we do the same when running on  X11.

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] RFC [spice-gtk] session: Allow to delay sending clipboard to the guest

[Marc-André Lureau]

Hi

- Original Message -
> On Thu, Jan 11, 2018 at 08:10:39AM -0500, Frediano Ziglio wrote:
> > > 
> > > Hi
> > > 
> > > - Original Message -
> > > > On Tue, Jan 09, 2018 at 12:16:33PM -0500, Marc-André Lureau wrote:
> > > > > I think it's problematic for traditional applications as well.
> > > > > clipboard access is probably going to be limited by default and only
> > > > > accessed through so-called "portals", just like file access etc. This
> > > > > topic should be brought on desktop / flatpak mailing list.
> > > > 
> > > > Maybe in some distant future, all applications everyone is running will
> > > > be flatpak, and will be using portals to improve security. The same
> > > > thing can be said regarding wayland, which does not have this issue.
> > > > Some time in the future, this will become a non-issue. However, solving
> > > > this now on x11 is definitely not something which should be related to
> > > > portals/flatpak in my opinion.
> > > 
> > > I propose a --spice-disable-clipboard, and client UI to switch on/off
> > > clipboard sharing functionality.
> > > 
> > > Something different will likely break some clipboard users or lower
> > > experience.
> > 
> > If we consider this a security threat than default should be disabled
> > and there should be a --spice-enable-clipboard. Note that the default
> > option apply to different tools (like virt-manager and boxes).
> > 
> > If we decide to go to the on/off options I would see some options
> > 
> > - default on (like now). The user should be prompted that there's
> >   a security issue and confirm to have understood. Without that
> >   prompt and knowing the issue spice could be potentially considered
> >   not that secure to use. That means the confirmation should be saved
> >   in order to avoid prompting it every time;
> 
> Prompting the user to confirm that they understand a security issue
> is a total disaster. Users will just blindly click through any
> warning message about security if it gets in the way of what they
> are trying to achieve.  At best we'll annoy users.

agree

> 
> > - default off. We could say nothing but I think the user would be
> >   quite frustrated as without any message and with just an update
> >   copy won't work. We could give the user a prompt also in
> >   this case. This seems more secure, if user does not read the
> >   message and click "ok" the data can be leaked.
> > 
> > From user experience and customer feeling somebody could complain
> > that the vmware default (c only with focus like Christophe patch
> > is supposed to do) is quite good and does not require manually
> > enable/disable that making really easy to use.
> 
> This is really much more viable IMHO. It avoids need to prompt user with
> security warnings and avoids extra config options and shouldn't break
> normal usage patterns.

Do you know the content of the clipboards when you switch your focus window?

Doesn't seem safe either to me. I would rather have clipboard sharing disabled 
by default.
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] RFC [spice-gtk] session: Allow to delay sending clipboard to the guest

[Daniel P. Berrange]

On Thu, Jan 11, 2018 at 08:10:39AM -0500, Frediano Ziglio wrote:
> > 
> > Hi
> > 
> > - Original Message -
> > > On Tue, Jan 09, 2018 at 12:16:33PM -0500, Marc-André Lureau wrote:
> > > > I think it's problematic for traditional applications as well.
> > > > clipboard access is probably going to be limited by default and only
> > > > accessed through so-called "portals", just like file access etc. This
> > > > topic should be brought on desktop / flatpak mailing list.
> > > 
> > > Maybe in some distant future, all applications everyone is running will
> > > be flatpak, and will be using portals to improve security. The same
> > > thing can be said regarding wayland, which does not have this issue.
> > > Some time in the future, this will become a non-issue. However, solving
> > > this now on x11 is definitely not something which should be related to
> > > portals/flatpak in my opinion.
> > 
> > I propose a --spice-disable-clipboard, and client UI to switch on/off
> > clipboard sharing functionality.
> > 
> > Something different will likely break some clipboard users or lower
> > experience.
> 
> If we consider this a security threat than default should be disabled
> and there should be a --spice-enable-clipboard. Note that the default
> option apply to different tools (like virt-manager and boxes).
> 
> If we decide to go to the on/off options I would see some options
> 
> - default on (like now). The user should be prompted that there's
>   a security issue and confirm to have understood. Without that
>   prompt and knowing the issue spice could be potentially considered
>   not that secure to use. That means the confirmation should be saved
>   in order to avoid prompting it every time;

Prompting the user to confirm that they understand a security issue
is a total disaster. Users will just blindly click through any
warning message about security if it gets in the way of what they
are trying to achieve.  At best we'll annoy users.

> - default off. We could say nothing but I think the user would be
>   quite frustrated as without any message and with just an update
>   copy won't work. We could give the user a prompt also in
>   this case. This seems more secure, if user does not read the
>   message and click "ok" the data can be leaked.
> 
> From user experience and customer feeling somebody could complain
> that the vmware default (c only with focus like Christophe patch
> is supposed to do) is quite good and does not require manually
> enable/disable that making really easy to use.

This is really much more viable IMHO. It avoids need to prompt user with
security warnings and avoids extra config options and shouldn't break
normal usage patterns.


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 :|
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] RFC [spice-gtk] session: Allow to delay sending clipboard to the guest

[Frediano Ziglio]

> 
> Hi
> 
> - Original Message -
> > On Tue, Jan 09, 2018 at 12:16:33PM -0500, Marc-André Lureau wrote:
> > > I think it's problematic for traditional applications as well.
> > > clipboard access is probably going to be limited by default and only
> > > accessed through so-called "portals", just like file access etc. This
> > > topic should be brought on desktop / flatpak mailing list.
> > 
> > Maybe in some distant future, all applications everyone is running will
> > be flatpak, and will be using portals to improve security. The same
> > thing can be said regarding wayland, which does not have this issue.
> > Some time in the future, this will become a non-issue. However, solving
> > this now on x11 is definitely not something which should be related to
> > portals/flatpak in my opinion.
> 
> I propose a --spice-disable-clipboard, and client UI to switch on/off
> clipboard sharing functionality.
> 
> Something different will likely break some clipboard users or lower
> experience.

If we consider this a security threat than default should be disabled
and there should be a --spice-enable-clipboard. Note that the default
option apply to different tools (like virt-manager and boxes).

If we decide to go to the on/off options I would see some options

- default on (like now). The user should be prompted that there's
  a security issue and confirm to have understood. Without that
  prompt and knowing the issue spice could be potentially considered
  not that secure to use. That means the confirmation should be saved
  in order to avoid prompting it every time;
- default off. We could say nothing but I think the user would be
  quite frustrated as without any message and with just an update
  copy won't work. We could give the user a prompt also in
  this case. This seems more secure, if user does not read the
  message and click "ok" the data can be leaked.

From user experience and customer feeling somebody could complain
that the vmware default (c only with focus like Christophe patch
is supposed to do) is quite good and does not require manually
enable/disable that making really easy to use.

From implementation details the off and focus options would require
some code to make c not work (always or on conditions) so I
think we could agree on a patch implementing on/off on the code.
As already understood the agent couldn't force a read of remote
clipboard if disable (or the guest could do a polling of the changes).

I personally agree with Christophe about not using portals for now
and on the focus option.

Frediano
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] RFC [spice-gtk] session: Allow to delay sending clipboard to the guest

[Marc-André Lureau]

Hi

- Original Message -
> On Tue, Jan 09, 2018 at 12:16:33PM -0500, Marc-André Lureau wrote:
> > I think it's problematic for traditional applications as well.
> > clipboard access is probably going to be limited by default and only
> > accessed through so-called "portals", just like file access etc. This
> > topic should be brought on desktop / flatpak mailing list.
> 
> Maybe in some distant future, all applications everyone is running will
> be flatpak, and will be using portals to improve security. The same
> thing can be said regarding wayland, which does not have this issue.
> Some time in the future, this will become a non-issue. However, solving
> this now on x11 is definitely not something which should be related to
> portals/flatpak in my opinion.

I propose a --spice-disable-clipboard, and client UI to switch on/off clipboard 
sharing functionality.

Something different will likely break some clipboard users or lower experience.
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] RFC [spice-gtk] session: Allow to delay sending clipboard to the guest

[Christophe Fergeau]

On Tue, Jan 09, 2018 at 12:16:33PM -0500, Marc-André Lureau wrote:
> I think it's problematic for traditional applications as well.
> clipboard access is probably going to be limited by default and only
> accessed through so-called "portals", just like file access etc. This
> topic should be brought on desktop / flatpak mailing list.

Maybe in some distant future, all applications everyone is running will
be flatpak, and will be using portals to improve security. The same
thing can be said regarding wayland, which does not have this issue.
Some time in the future, this will become a non-issue. However, solving
this now on x11 is definitely not something which should be related to
portals/flatpak in my opinion.

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] RFC [spice-gtk] session: Allow to delay sending clipboard to the guest

[Marc-André Lureau]

Hi

- Original Message -
> 
> > 
> > Hi
> > 
> > - Original Message -
> > > This patch is solving guest <-> client issue, I doubt portals would
> > > address
> > > this
> > > so I think only spice-gtk can address it.
> > 
> > Portals could check the permissions for an application using the clipboard.
> > Whether it's spice-gtk talking to a guest VM or an editor, it doesn't
> > matter.
> > 
> 
> So use these portals (are they cross platform?) to temporarily disable
> clipboard
> access
> 
> > A portal may be overkill (since there would probably be no UI), and a
> > simple
> > static permission could be enough.
> > 
> > This basically would be the same thing as having a
> > --spice-disable-clipboard
> > option.
> > 
> 
> A command line option does not look that temporary. I think an option on the
> GUI
> like c:
> - never;
> - always;

Yes, additional UI may be nice to switch enabling/disabling clipboard sharing 
(although we still need command line and/or config file).
 
> - only if have the focus;
> - special keyboard combination.
> Or crazy stuff like enabling for only some seconds.

The rest feels unnecessarily complex to me.
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] RFC [spice-gtk] session: Allow to delay sending clipboard to the guest

[Frediano Ziglio]

> 
> Hi
> 
> - Original Message -
> > This patch is solving guest <-> client issue, I doubt portals would address
> > this
> > so I think only spice-gtk can address it.
> 
> Portals could check the permissions for an application using the clipboard.
> Whether it's spice-gtk talking to a guest VM or an editor, it doesn't
> matter.
> 

So use these portals (are they cross platform?) to temporarily disable clipboard
access

> A portal may be overkill (since there would probably be no UI), and a simple
> static permission could be enough.
> 
> This basically would be the same thing as having a --spice-disable-clipboard
> option.
> 

A command line option does not look that temporary. I think an option on the GUI
like c:
- never;
- always;
- only if have the focus;
- special keyboard combination.
Or crazy stuff like enabling for only some seconds.

Frediano
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] RFC [spice-gtk] session: Allow to delay sending clipboard to the guest

[Marc-André Lureau]

Hi

- Original Message -
> This patch is solving guest <-> client issue, I doubt portals would address
> this
> so I think only spice-gtk can address it. 

Portals could check the permissions for an application using the clipboard. 
Whether it's spice-gtk talking to a guest VM or an editor, it doesn't matter.

A portal may be overkill (since there would probably be no UI), and a simple 
static permission could be enough.

This basically would be the same thing as having a --spice-disable-clipboard 
option.

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

Re: [Spice-devel] RFC [spice-gtk] session: Allow to delay sending clipboard to the guest

[Jakub Janků]

Hi,

On Tue, 2018-01-09 at 15:35 +0100, Christophe Fergeau wrote:
> This is used to prevent unfocused guests from sniffing the clipboard
> content without the host or other guests noticing. This can be a
> security issue if any VM can track the clipboard activity in the
> session.
> This commit sets a boolean in SpiceGtkSession on focus in/out events.
> The client -> guest sending of clipboard data is then delayed until
> the
> window is focused again. This behaviour matches the behaviour we get
> on
> Wayland.
> 
> This mostly solves https://bugzilla.redhat.com/show_bug.cgi?id=132026
> 3

I wouldn't say this behaviour matches the one on Wayland.
On Wayland, spice-gtk receives no owner-change events when the window
is out of focus and therefore doesn't send any clipboard grabs.
With this patch on X11, clipboard grab messages are sent to guest even
when the window is not focused.

Besides that I don't think delaying the data is a good idea. Some apps
might have a timeout for the requests. afaik there's a timeout of 30
seconds in gtk when requesting the data with
gtk_clipboard_request_contents().

What about saving the last relevant (valid) owner change event in
clipboard_owner_change() callback and dealing with it (requesting
clipboard targets and sending clipboard grab to guest) once the window
gains focus? (This would match the Wayland behaviour more closely.)


There's still the issue, that your potential password stored in the
clipboard is exposed to the remote machine once you focus the spice-gtk 
window (if you don't clear it before entering the window).

So if we really wanted to make this secure, we could add a dialog to
spice-gtk that would enable the user to approve every clipboard data
request. This would be easy to implement with gtk_message_dialog_new().
It's imho an overkill though. Anyone paranoid enough to use it? :)

> Signed-off-by: Christophe Fergeau 
> ---
> Not overly happy with the added complexity, maybe someone will have
> suggestions for a better approach ;) 
 
> 
> 
>  src/spice-gtk-session-priv.h |  1 +
>  src/spice-gtk-session.c  | 97
> +---
>  src/spice-widget.c   |  5 +++
>  3 files changed, 97 insertions(+), 6 deletions(-)
> 
> diff --git a/src/spice-gtk-session-priv.h b/src/spice-gtk-session-
> priv.h
> index 0dbc9e96..efd36806 100644
> --- a/src/spice-gtk-session-priv.h
> +++ b/src/spice-gtk-session-priv.h
> @@ -44,6 +44,7 @@ void
> spice_gtk_session_set_keyboard_has_focus(SpiceGtkSession *self,
> gboolean ke
>  void spice_gtk_session_set_mouse_has_pointer(SpiceGtkSession *self,
> gboolean  mouse_has_pointer);
>  gboolean spice_gtk_session_get_keyboard_has_focus(SpiceGtkSession
> *self);
>  gboolean spice_gtk_session_get_mouse_has_pointer(SpiceGtkSession
> *self);
> +void spice_gtk_session_set_clipboard_delayed(SpiceGtkSession *self,
> gboolean clipboard_delayed);
>  
>  G_END_DECLS
>  
> diff --git a/src/spice-gtk-session.c b/src/spice-gtk-session.c
> index 31f60dc4..15f2b531 100644
> --- a/src/spice-gtk-session.c
> +++ b/src/spice-gtk-session.c
> @@ -59,6 +59,17 @@ struct _SpiceGtkSessionPrivate {
>  gbooleanclip_hasdata[CLIPBOARD_LAST];
>  gbooleanclip_grabbed[CLIPBOARD_LAST];
>  gbooleanclipboard_by_guest[CLIPBOARD_LAST];
> +
> +/* Used to prevent sending host->guest clipboard data when the
> guest
> + * display is not focused
> + */
> +gbooleanclipboard_delayed;
> +gbooleanclipboard_pending_selection_release[CLIP
> BOARD_LAST];
> +gbooleanclipboard_pending_selection[CLIPBOARD_LA
> ST];
> +guint32 clipboard_pending_type[CLIPBOARD_LAST];
> +guchar  *clipboard_pending_data[CLIPBOARD_LAST];
> +gsize   clipboard_pending_len[CLIPBOARD_LAST];
> +
>  /* auto-usbredir related */
>  gbooleanauto_usbredir_enable;
>  int auto_usbredir_reqs;
> @@ -666,6 +677,24 @@ static void clipboard_get_targets(GtkClipboard
> *clipboard,
>  s->nclip_targets[selection] = 0;
>  }
>  
> +static void
> +clipboard_selection_release(SpiceGtkSession *self, int selection)
> +{
> +g_return_if_fail(selection >= 0);
> +g_return_if_fail(selection < CLIPBOARD_LAST);
> +
> +g_warning("selection_release");
> +
> +if (!self->priv->clipboard_delayed) {
> +spice_main_channel_clipboard_selection_release(self->priv-
> >main, selection);
> +} else {
> +self->priv->clipboard_pending_selection_release[selection] =
> TRUE;
> +g_clear_pointer(>priv-
> >clipboard_pending_data[selection], g_free);
> +self->priv->clipboard_pending_len[selection] = 0;
> +self->priv->clipboard_pending_selection[selection] = FALSE;
> +}
> +}
> +
>  static void clipboard_owner_change(GtkClipboard*clipboard,
> 

Re: [Spice-devel] RFC [spice-gtk] session: Allow to delay sending clipboard to the guest

[Frediano Ziglio]

> 
> Hi
> 
> - Original Message -
> > On Tue, Jan 09, 2018 at 11:08:44AM -0500, Marc-André Lureau wrote:
> > > - Original Message -
> > > > 
> > > > What makes spice different here is that it's used to access a VM, and a
> > > > VM is supposed to give you isolation. If some hostile code is running
> > > > in
> > > > the VM, its impact on the host/client OS should be minimal. The fact
> > > > that a VM with an open client connection can monitor everything that
> > > > goes in the clipboard breaks that isolation. For example, I have a ton
> > > > of password going through my clipboard, which I don't necessarily want
> > > > VM to have direct access to.
> > > 
> > > Spice isn't that tied to the VM or isolation concept. It's a remote
> > > display protocol, aiming to blur the lines between remote and locate
> > > applications or desktop. As such, it's not that different from say,
> > > the X protocol or the Wayland protocol...
> > 
> > In theory it's not tied to VMs, in practice, 90% of the time (and I'm
> > being conservative) it's used to access VMs.
> > 
> > > > > I am not that familiar with Wayland clipboard behaviour, could you
> > > > > explained what changed? That could help me to understand this patch
> > > > > better.
> > > > 
> > > > I'll detail this in the commit log, but if you try the 'watch' command
> > > > from above in a VM, then copy something to your clipboard on the
> > > > client,
> > > > you'll notice that the clipboard content shows up in the VM only after
> > > > you give it focus. In a way, this answers your "this shouldn't be
> > > > solved
> > > > at the spice/spice-gtk level" concern, and this was indeed solved at a
> > > > different level. However, we still have the issue on x11 for now.
> > > 
> > > Ok, but then I think we should accept the fact that this is a x11
> > > "limitation", like many others x11 security issues. If not, try to fix
> > > it at a different level, like the toolkit.
> > 
> > For most applications using gtk+, this really is a non-issue, or
> > something not really important, so it does not really make sense to do
> > that at the gtk+ level. spice on the other hand takes local clipboard
> > data, and forwards it remotely, so I think it makes sense that it's
> > careful with what it does with it.
> 
> I think it's problematic for traditional applications as well. clipboard
> access is probably going to be limited by default and only accessed through
> so-called "portals", just like file access etc. This topic should be brought
> on desktop / flatpak mailing list.
> 
> Thus I don't believe the problem should be solved at spice-gtk level, as this
> can potentially break some use cases, like clipboard managers running in the
> guest etc

This patch is solving guest <-> client issue, I doubt portals would address this
so I think only spice-gtk can address it. The agent could do something but the
agent is controlled by the guest so cannot protect client data.

On the implementation I didn't have a deep look but saving data from clipboard
to later usage seems really wrong. I don't know how clipboard works on Unix or
GTK but on Windows data copies are done (or can be done) lazily to avoid huge
copies or optimize some cases (for instance same app c or avoiding useless
conversions). I would just save which clipboards were changed during out of
focus (so kind of some bools variable) and tell the guest the change when we
receive the focus. Can't we do it?

Frediano
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] RFC [spice-gtk] session: Allow to delay sending clipboard to the guest

[Marc-André Lureau]

Hi

- Original Message -
> On Tue, Jan 09, 2018 at 11:08:44AM -0500, Marc-André Lureau wrote:
> > - Original Message -
> > > 
> > > What makes spice different here is that it's used to access a VM, and a
> > > VM is supposed to give you isolation. If some hostile code is running in
> > > the VM, its impact on the host/client OS should be minimal. The fact
> > > that a VM with an open client connection can monitor everything that
> > > goes in the clipboard breaks that isolation. For example, I have a ton
> > > of password going through my clipboard, which I don't necessarily want
> > > VM to have direct access to.
> > 
> > Spice isn't that tied to the VM or isolation concept. It's a remote
> > display protocol, aiming to blur the lines between remote and locate
> > applications or desktop. As such, it's not that different from say,
> > the X protocol or the Wayland protocol...
> 
> In theory it's not tied to VMs, in practice, 90% of the time (and I'm
> being conservative) it's used to access VMs.
> 
> > > > I am not that familiar with Wayland clipboard behaviour, could you
> > > > explained what changed? That could help me to understand this patch
> > > > better.
> > > 
> > > I'll detail this in the commit log, but if you try the 'watch' command
> > > from above in a VM, then copy something to your clipboard on the client,
> > > you'll notice that the clipboard content shows up in the VM only after
> > > you give it focus. In a way, this answers your "this shouldn't be solved
> > > at the spice/spice-gtk level" concern, and this was indeed solved at a
> > > different level. However, we still have the issue on x11 for now.
> > 
> > Ok, but then I think we should accept the fact that this is a x11
> > "limitation", like many others x11 security issues. If not, try to fix
> > it at a different level, like the toolkit.
> 
> For most applications using gtk+, this really is a non-issue, or
> something not really important, so it does not really make sense to do
> that at the gtk+ level. spice on the other hand takes local clipboard
> data, and forwards it remotely, so I think it makes sense that it's
> careful with what it does with it.

I think it's problematic for traditional applications as well. clipboard access 
is probably going to be limited by default and only accessed through so-called 
"portals", just like file access etc. This topic should be brought on desktop / 
flatpak mailing list.

Thus I don't believe the problem should be solved at spice-gtk level, as this 
can potentially break some use cases, like clipboard managers running in the 
guest etc
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] RFC [spice-gtk] session: Allow to delay sending clipboard to the guest

[Christophe Fergeau]

On Tue, Jan 09, 2018 at 11:08:44AM -0500, Marc-André Lureau wrote:
> - Original Message -
> > 
> > What makes spice different here is that it's used to access a VM, and a
> > VM is supposed to give you isolation. If some hostile code is running in
> > the VM, its impact on the host/client OS should be minimal. The fact
> > that a VM with an open client connection can monitor everything that
> > goes in the clipboard breaks that isolation. For example, I have a ton
> > of password going through my clipboard, which I don't necessarily want
> > VM to have direct access to.
> 
> Spice isn't that tied to the VM or isolation concept. It's a remote
> display protocol, aiming to blur the lines between remote and locate
> applications or desktop. As such, it's not that different from say,
> the X protocol or the Wayland protocol...

In theory it's not tied to VMs, in practice, 90% of the time (and I'm
being conservative) it's used to access VMs.

> > > I am not that familiar with Wayland clipboard behaviour, could you
> > > explained what changed? That could help me to understand this patch
> > > better.
> > 
> > I'll detail this in the commit log, but if you try the 'watch' command
> > from above in a VM, then copy something to your clipboard on the client,
> > you'll notice that the clipboard content shows up in the VM only after
> > you give it focus. In a way, this answers your "this shouldn't be solved
> > at the spice/spice-gtk level" concern, and this was indeed solved at a
> > different level. However, we still have the issue on x11 for now.
> 
> Ok, but then I think we should accept the fact that this is a x11
> "limitation", like many others x11 security issues. If not, try to fix
> it at a different level, like the toolkit.

For most applications using gtk+, this really is a non-issue, or
something not really important, so it does not really make sense to do
that at the gtk+ level. spice on the other hand takes local clipboard
data, and forwards it remotely, so I think it makes sense that it's
careful with what it does with it.

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] RFC [spice-gtk] session: Allow to delay sending clipboard to the guest

[Marc-André Lureau]

Hi

- Original Message -
> Hey,
> 
> On Tue, Jan 09, 2018 at 09:46:48AM -0500, Marc-André Lureau wrote:
> > - Original Message -
> > > This is used to prevent unfocused guests from sniffing the clipboard
> > > content without the host or other guests noticing. This can be a
> > > security issue if any VM can track the clipboard activity in the
> > > session.
> > > This commit sets a boolean in SpiceGtkSession on focus in/out events.
> > > The client -> guest sending of clipboard data is then delayed until the
> > > window is focused again. This behaviour matches the behaviour we get on
> > > Wayland.
> > > 
> > > This mostly solves https://bugzilla.redhat.com/show_bug.cgi?id=1320263
> > 
> > As Hans corrected in the bug, the data isn't actually transferred until the
> > guest actually requested it.
> > 
> > Now, a malicious guest could try to get the clipboard content in a loop,
> > even without previous notification of clipboard content.
> 
> Yes, that's the issue, for example 'watch xsel -o --clipboard'
> 
> > However, isn't this true for any application running in the client
> > desktop? What makes Spice guest different here? And by that I mean
> > that the problem shouldn't probably be solved at the spice/spice-gtk
> > level.
> 
> What makes spice different here is that it's used to access a VM, and a
> VM is supposed to give you isolation. If some hostile code is running in
> the VM, its impact on the host/client OS should be minimal. The fact
> that a VM with an open client connection can monitor everything that
> goes in the clipboard breaks that isolation. For example, I have a ton
> of password going through my clipboard, which I don't necessarily want
> VM to have direct access to.

Spice isn't that tied to the VM or isolation concept. It's a remote display 
protocol, aiming to blur the lines between remote and locate applications or 
desktop. As such, it's not that different from say, the X protocol or the 
Wayland protocol...

> 
> 
> > I am not that familiar with Wayland clipboard behaviour, could you
> > explained what changed? That could help me to understand this patch
> > better.
> 
> I'll detail this in the commit log, but if you try the 'watch' command
> from above in a VM, then copy something to your clipboard on the client,
> you'll notice that the clipboard content shows up in the VM only after
> you give it focus. In a way, this answers your "this shouldn't be solved
> at the spice/spice-gtk level" concern, and this was indeed solved at a
> different level. However, we still have the issue on x11 for now.

Ok, but then I think we should accept the fact that this is a x11 "limitation", 
like many others x11 security issues. If not, try to fix it at a different 
level, like the toolkit.
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] RFC [spice-gtk] session: Allow to delay sending clipboard to the guest

[Christophe Fergeau]

Hey,

On Tue, Jan 09, 2018 at 09:46:48AM -0500, Marc-André Lureau wrote:
> - Original Message -
> > This is used to prevent unfocused guests from sniffing the clipboard
> > content without the host or other guests noticing. This can be a
> > security issue if any VM can track the clipboard activity in the
> > session.
> > This commit sets a boolean in SpiceGtkSession on focus in/out events.
> > The client -> guest sending of clipboard data is then delayed until the
> > window is focused again. This behaviour matches the behaviour we get on
> > Wayland.
> > 
> > This mostly solves https://bugzilla.redhat.com/show_bug.cgi?id=1320263
> 
> As Hans corrected in the bug, the data isn't actually transferred until the 
> guest actually requested it.
> 
> Now, a malicious guest could try to get the clipboard content in a loop, even 
> without previous notification of clipboard content.

Yes, that's the issue, for example 'watch xsel -o --clipboard'

> However, isn't this true for any application running in the client
> desktop? What makes Spice guest different here? And by that I mean
> that the problem shouldn't probably be solved at the spice/spice-gtk
> level.

What makes spice different here is that it's used to access a VM, and a
VM is supposed to give you isolation. If some hostile code is running in
the VM, its impact on the host/client OS should be minimal. The fact
that a VM with an open client connection can monitor everything that
goes in the clipboard breaks that isolation. For example, I have a ton
of password going through my clipboard, which I don't necessarily want
VM to have direct access to.


> I am not that familiar with Wayland clipboard behaviour, could you
> explained what changed? That could help me to understand this patch
> better.

I'll detail this in the commit log, but if you try the 'watch' command
from above in a VM, then copy something to your clipboard on the client,
you'll notice that the clipboard content shows up in the VM only after
you give it focus. In a way, this answers your "this shouldn't be solved
at the spice/spice-gtk level" concern, and this was indeed solved at a
different level. However, we still have the issue on x11 for now.

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] RFC [spice-gtk] session: Allow to delay sending clipboard to the guest

[Marc-André Lureau]

Hi

- Original Message -
> This is used to prevent unfocused guests from sniffing the clipboard
> content without the host or other guests noticing. This can be a
> security issue if any VM can track the clipboard activity in the
> session.
> This commit sets a boolean in SpiceGtkSession on focus in/out events.
> The client -> guest sending of clipboard data is then delayed until the
> window is focused again. This behaviour matches the behaviour we get on
> Wayland.
> 
> This mostly solves https://bugzilla.redhat.com/show_bug.cgi?id=1320263

As Hans corrected in the bug, the data isn't actually transferred until the 
guest actually requested it.

Now, a malicious guest could try to get the clipboard content in a loop, even 
without previous notification of clipboard content.

However, isn't this true for any application running in the client desktop? 
What makes Spice guest different here? And by that I mean that the problem 
shouldn't probably be solved at the spice/spice-gtk level.

I am not that familiar with Wayland clipboard behaviour, could you explained 
what changed? That could help me to understand this patch better.

thanks

> 
> Signed-off-by: Christophe Fergeau 
> ---
> Not overly happy with the added complexity, maybe someone will have
> suggestions for a better approach ;)
> 
> 
>  src/spice-gtk-session-priv.h |  1 +
>  src/spice-gtk-session.c  | 97
>  +---
>  src/spice-widget.c   |  5 +++
>  3 files changed, 97 insertions(+), 6 deletions(-)
> 
> diff --git a/src/spice-gtk-session-priv.h b/src/spice-gtk-session-priv.h
> index 0dbc9e96..efd36806 100644
> --- a/src/spice-gtk-session-priv.h
> +++ b/src/spice-gtk-session-priv.h
> @@ -44,6 +44,7 @@ void
> spice_gtk_session_set_keyboard_has_focus(SpiceGtkSession *self, gboolean ke
>  void spice_gtk_session_set_mouse_has_pointer(SpiceGtkSession *self, gboolean
>  mouse_has_pointer);
>  gboolean spice_gtk_session_get_keyboard_has_focus(SpiceGtkSession *self);
>  gboolean spice_gtk_session_get_mouse_has_pointer(SpiceGtkSession *self);
> +void spice_gtk_session_set_clipboard_delayed(SpiceGtkSession *self, gboolean
> clipboard_delayed);
>  
>  G_END_DECLS
>  
> diff --git a/src/spice-gtk-session.c b/src/spice-gtk-session.c
> index 31f60dc4..15f2b531 100644
> --- a/src/spice-gtk-session.c
> +++ b/src/spice-gtk-session.c
> @@ -59,6 +59,17 @@ struct _SpiceGtkSessionPrivate {
>  gbooleanclip_hasdata[CLIPBOARD_LAST];
>  gbooleanclip_grabbed[CLIPBOARD_LAST];
>  gbooleanclipboard_by_guest[CLIPBOARD_LAST];
> +
> +/* Used to prevent sending host->guest clipboard data when the guest
> + * display is not focused
> + */
> +gbooleanclipboard_delayed;
> +gboolean
> clipboard_pending_selection_release[CLIPBOARD_LAST];
> +gbooleanclipboard_pending_selection[CLIPBOARD_LAST];
> +guint32 clipboard_pending_type[CLIPBOARD_LAST];
> +guchar  *clipboard_pending_data[CLIPBOARD_LAST];
> +gsize   clipboard_pending_len[CLIPBOARD_LAST];
> +
>  /* auto-usbredir related */
>  gbooleanauto_usbredir_enable;
>  int auto_usbredir_reqs;
> @@ -666,6 +677,24 @@ static void clipboard_get_targets(GtkClipboard
> *clipboard,
>  s->nclip_targets[selection] = 0;
>  }
>  
> +static void
> +clipboard_selection_release(SpiceGtkSession *self, int selection)
> +{
> +g_return_if_fail(selection >= 0);
> +g_return_if_fail(selection < CLIPBOARD_LAST);
> +
> +g_warning("selection_release");
> +
> +if (!self->priv->clipboard_delayed) {
> +spice_main_channel_clipboard_selection_release(self->priv->main,
> selection);
> +} else {
> +self->priv->clipboard_pending_selection_release[selection] = TRUE;
> +g_clear_pointer(>priv->clipboard_pending_data[selection],
> g_free);
> +self->priv->clipboard_pending_len[selection] = 0;
> +self->priv->clipboard_pending_selection[selection] = FALSE;
> +}
> +}
> +
>  static void clipboard_owner_change(GtkClipboard*clipboard,
> GdkEventOwnerChange *event,
> gpointeruser_data)
> @@ -685,7 +714,7 @@ static void clipboard_owner_change(GtkClipboard
> *clipboard,
>  if (s->clip_grabbed[selection]) {
>  s->clip_grabbed[selection] = FALSE;
>  if (spice_main_channel_agent_test_capability(s->main,
>  VD_AGENT_CAP_CLIPBOARD_BY_DEMAND))
> -spice_main_channel_clipboard_selection_release(s->main,
> selection);
> +clipboard_selection_release(self, selection);
>  }
>  
>  switch (event->reason) {
> @@ -714,6 +743,18 @@ typedef struct
>  guint selection;
>  } RunInfo;
>  
> +static void
> +clipboard_delayed_selection_notify(SpiceGtkSession *self);
> +
> +G_GNUC_INTERNAL 

[Spice-devel] RFC [spice-gtk] session: Allow to delay sending clipboard to the guest

[Christophe Fergeau]

This is used to prevent unfocused guests from sniffing the clipboard
content without the host or other guests noticing. This can be a
security issue if any VM can track the clipboard activity in the
session.
This commit sets a boolean in SpiceGtkSession on focus in/out events.
The client -> guest sending of clipboard data is then delayed until the
window is focused again. This behaviour matches the behaviour we get on
Wayland.

This mostly solves https://bugzilla.redhat.com/show_bug.cgi?id=1320263

Signed-off-by: Christophe Fergeau 
---
Not overly happy with the added complexity, maybe someone will have
suggestions for a better approach ;)


 src/spice-gtk-session-priv.h |  1 +
 src/spice-gtk-session.c  | 97 +---
 src/spice-widget.c   |  5 +++
 3 files changed, 97 insertions(+), 6 deletions(-)

diff --git a/src/spice-gtk-session-priv.h b/src/spice-gtk-session-priv.h
index 0dbc9e96..efd36806 100644
--- a/src/spice-gtk-session-priv.h
+++ b/src/spice-gtk-session-priv.h
@@ -44,6 +44,7 @@ void spice_gtk_session_set_keyboard_has_focus(SpiceGtkSession 
*self, gboolean ke
 void spice_gtk_session_set_mouse_has_pointer(SpiceGtkSession *self, gboolean  
mouse_has_pointer);
 gboolean spice_gtk_session_get_keyboard_has_focus(SpiceGtkSession *self);
 gboolean spice_gtk_session_get_mouse_has_pointer(SpiceGtkSession *self);
+void spice_gtk_session_set_clipboard_delayed(SpiceGtkSession *self, gboolean 
clipboard_delayed);
 
 G_END_DECLS
 
diff --git a/src/spice-gtk-session.c b/src/spice-gtk-session.c
index 31f60dc4..15f2b531 100644
--- a/src/spice-gtk-session.c
+++ b/src/spice-gtk-session.c
@@ -59,6 +59,17 @@ struct _SpiceGtkSessionPrivate {
 gbooleanclip_hasdata[CLIPBOARD_LAST];
 gbooleanclip_grabbed[CLIPBOARD_LAST];
 gbooleanclipboard_by_guest[CLIPBOARD_LAST];
+
+/* Used to prevent sending host->guest clipboard data when the guest
+ * display is not focused
+ */
+gbooleanclipboard_delayed;
+gboolean
clipboard_pending_selection_release[CLIPBOARD_LAST];
+gbooleanclipboard_pending_selection[CLIPBOARD_LAST];
+guint32 clipboard_pending_type[CLIPBOARD_LAST];
+guchar  *clipboard_pending_data[CLIPBOARD_LAST];
+gsize   clipboard_pending_len[CLIPBOARD_LAST];
+
 /* auto-usbredir related */
 gbooleanauto_usbredir_enable;
 int auto_usbredir_reqs;
@@ -666,6 +677,24 @@ static void clipboard_get_targets(GtkClipboard *clipboard,
 s->nclip_targets[selection] = 0;
 }
 
+static void
+clipboard_selection_release(SpiceGtkSession *self, int selection)
+{
+g_return_if_fail(selection >= 0);
+g_return_if_fail(selection < CLIPBOARD_LAST);
+
+g_warning("selection_release");
+
+if (!self->priv->clipboard_delayed) {
+spice_main_channel_clipboard_selection_release(self->priv->main, 
selection);
+} else {
+self->priv->clipboard_pending_selection_release[selection] = TRUE;
+g_clear_pointer(>priv->clipboard_pending_data[selection], 
g_free);
+self->priv->clipboard_pending_len[selection] = 0;
+self->priv->clipboard_pending_selection[selection] = FALSE;
+}
+}
+
 static void clipboard_owner_change(GtkClipboard*clipboard,
GdkEventOwnerChange *event,
gpointeruser_data)
@@ -685,7 +714,7 @@ static void clipboard_owner_change(GtkClipboard
*clipboard,
 if (s->clip_grabbed[selection]) {
 s->clip_grabbed[selection] = FALSE;
 if (spice_main_channel_agent_test_capability(s->main, 
VD_AGENT_CAP_CLIPBOARD_BY_DEMAND))
-spice_main_channel_clipboard_selection_release(s->main, selection);
+clipboard_selection_release(self, selection);
 }
 
 switch (event->reason) {
@@ -714,6 +743,18 @@ typedef struct
 guint selection;
 } RunInfo;
 
+static void
+clipboard_delayed_selection_notify(SpiceGtkSession *self);
+
+G_GNUC_INTERNAL void spice_gtk_session_set_clipboard_delayed(SpiceGtkSession 
*self,
+ gboolean 
clipboard_delayed)
+{
+self->priv->clipboard_delayed = clipboard_delayed;
+if (!self->priv->clipboard_delayed) {
+clipboard_delayed_selection_notify(self);
+}
+}
+
 static void clipboard_got_from_guest(SpiceMainChannel *main, guint selection,
  guint type, const guchar *data, guint 
size,
  gpointer user_data)
@@ -927,6 +968,52 @@ static char *fixup_clipboard_text(SpiceGtkSession *self, 
const char *text, int *
 return conv;
 }
 
+static void
+clipboard_delayed_selection_notify(SpiceGtkSession *self)
+{
+unsigned int i;
+
+for (i = 0; i < CLIPBOARD_LAST; i++) {
+if