[Spice-devel] [spice-gtk v3 3/3] gtk-session: clipboard: x11: do not request data while on focus

[Victor Toso]

On Thu, Jan 10, 2019 at 05:25:21PM +0400, Marc-André Lureau wrote:
> Hi
> 
> On Thu, Jan 10, 2019 at 4:47 PM Victor Toso <victortoso at redhat.com> wrote:
> >
> > From: Victor Toso <me at victortoso.com>
> >
> > If SpiceGtkSession is holding the keyboard, that's huge indication
> > that the client should not be requesting guest's clipboard data yet.
> >
> > This patch adds a check in clipboard_get() callback, to avoid such
> > requests. In Linux, this only happens with X11 backend.
> >
> > This patch helps to handle a possible state race between who owns the
> > grab between client and agent which could lead to agent clipboard
> > failing or getting stuck, see:
> 
> hmm
> 
> > The way to reproduce the race might depend on guest system and
> > applications but it is connected to amount of VDAGENTD_CLIPBOARD_GRAB
> > sent by the agent which depends on the amount of clipboard changes in
> > the guest. Simple example is on RHEL 6.10, with Gedit, select a text
> > char by char; Client receives VDAGENTD_CLIPBOARD_GRAB every time a new
> > char is selected instead of once when the full message is selected.
> 
> Sorry, you get a lot of clipboard-grab from the remote, but where is
> the problem?

Problem is, why fetch data that no other application is
requesting.

Situations like the one below happens due state changing fast +
idle callbacks for clear/request clipboard data...

    https://gitlab.freedesktop.org/spice/win32/vd_agent/issues/6#note_85246

> > v2 -> v3:
> > * Split the fix in two patches while adding some info in a 3rd patch;
> > * Kept the "clipboard_clear" as it was (Jakub)
> > * Added a "clipboard_grab" log in clipboard_grab() function;
> >
> > Related: https://gitlab.freedesktop.org/spice/win32/vd_agent/issues/6
> > Related: https://gitlab.freedesktop.org/spice/linux/vd_agent/issues/9
> > Related: https://bugzilla.redhat.com/show_bug.cgi?id=1594876
> >
> > Signed-off-by: Victor Toso <victortoso at redhat.com>
> > ---
> >  src/spice-gtk-session.c | 15 +++++++++++++++
> >  1 file changed, 15 insertions(+)
> >
> > diff --git a/src/spice-gtk-session.c b/src/spice-gtk-session.c
> > index 85d5880..f5959f7 100644
> > --- a/src/spice-gtk-session.c
> > +++ b/src/spice-gtk-session.c
> > @@ -763,6 +763,19 @@ static void clipboard_get(GtkClipboard *clipboard,
> >
> >      SPICE_DEBUG("clipboard get");
> >
> > +#ifdef GDK_WINDOWING_X11
> > +    /* Do not request clipboard data while we are still interacting with the
> > +     * Guest; Clipboard data could change shortly and this request would just
> > +     * be wasteful. */
> 
> I don't think there is anything preventing another app the
> clipboard data while spice-gtk has the focus. In fact, that's
> how clipboard manager work afaik.

That means that another application in Client OS might get remote
clipboard data while the user is interacting with remote VM.
That's pretty much a security concern as well. This does not
happen on Wayland and I would rather that we avoid it with X11
too, even if it breaks ClipboardManagers that don't request our
clipboard after we focus-out

Thanks for reviewing,
Victor

> 
> > +    if (GDK_IS_X11_DISPLAY(gdk_display_get_default()) &&
> > +        spice_gtk_session_get_keyboard_has_focus(self)) {
> > +        SPICE_DEBUG("clipboard get: not requesting data while holding focus");
> > +        return;
> > +    }
> > +#endif
> > +    SPICE_DEBUG("clipboard get: has-foucus=%d",
> > +                spice_gtk_session_get_keyboard_has_focus(self));
> > +
> >      selection = get_selection_from_clipboard(s, clipboard);
> >      g_return_if_fail(selection != -1);
> >      g_return_if_fail(info < SPICE_N_ELEMENTS(atom2agent));
> > @@ -818,6 +831,8 @@ static gboolean clipboard_grab(SpiceMainChannel *main, guint selection,
> >                                 guint32* types, guint32 ntypes,
> >                                 gpointer user_data)
> >  {
> > +    SPICE_DEBUG("clipboard_grab");
> > +
> >      g_return_val_if_fail(SPICE_IS_GTK_SESSION(user_data), FALSE);
> >
> >      SpiceGtkSession *self = user_data;
> > --
> > 2.20.1
> >
> > _______________________________________________
> > Spice-devel mailing list
> > Spice-devel at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/spice-devel
> 
> 
> 
> -- 
> Marc-André Lureau
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/spice-devel/attachments/20190110/0558b29f/attachment.sig>