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 <cfergeau@xxxxxxxxxx> > --- > 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 { > gboolean clip_hasdata[CLIPBOARD_LAST]; > gboolean clip_grabbed[CLIPBOARD_LAST]; > gboolean clipboard_by_guest[CLIPBOARD_LAST]; > + > + /* Used to prevent sending host->guest clipboard data when the guest > + * display is not focused > + */ > + gboolean clipboard_delayed; > + gboolean > clipboard_pending_selection_release[CLIPBOARD_LAST]; > + gboolean clipboard_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 */ > gboolean auto_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(&self->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, > gpointer user_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 (self->priv->clipboard_pending_selection_release[i]) { > + g_warn_if_fail(!self->priv->clipboard_pending_selection[i]); > + spice_main_channel_clipboard_selection_release(self->priv->main, > i); > + self->priv->clipboard_pending_selection_release[i] = FALSE; > + } > + if (!self->priv->clipboard_pending_selection[i]) { > + continue; > + } > + spice_main_channel_clipboard_selection_notify(self->priv->main, > + i, > + > self->priv->clipboard_pending_type[i], > + > self->priv->clipboard_pending_data[i], > + > self->priv->clipboard_pending_len[i]); > + g_clear_pointer(&self->priv->clipboard_pending_data[i], g_free); > + self->priv->clipboard_pending_len[i] = 0; > + self->priv->clipboard_pending_selection[i] = FALSE; > + } > +} > + > +static void > +clipboard_selection_notify(SpiceGtkSession *self, int selection, guint32 > type, > + const guchar *data, gsize len) > +{ > + g_return_if_fail(selection >= 0); > + g_return_if_fail(selection < CLIPBOARD_LAST); > + > + if (!self->priv->clipboard_delayed) { > + spice_main_channel_clipboard_selection_notify(self->priv->main, > + selection, type, > + data, len); > + } else { > + self->priv->clipboard_pending_selection[selection] = TRUE; > + self->priv->clipboard_pending_type[selection] = type; > + g_clear_pointer(&self->priv->clipboard_pending_data[selection], > g_free); > + self->priv->clipboard_pending_data[selection] = g_memdup(data, len); > + self->priv->clipboard_pending_len[selection] = len; > + self->priv->clipboard_pending_selection_release[selection] = FALSE; > + } > +} > + > static void clipboard_received_text_cb(GtkClipboard *clipboard, > const gchar *text, > gpointer user_data) > @@ -965,10 +1052,8 @@ static void clipboard_received_text_cb(GtkClipboard > *clipboard, > > data = (const guchar *) (conv != NULL ? conv : text); > notify_agent: > - spice_main_channel_clipboard_selection_notify(self->priv->main, > selection, > - > VD_AGENT_CLIPBOARD_UTF8_TEXT, > - data, > - (data != NULL) ? len : 0); > + clipboard_selection_notify(self, selection, > VD_AGENT_CLIPBOARD_UTF8_TEXT, > + data, (data != NULL) ? len : 0); > g_free(conv); > } > > @@ -1021,7 +1106,7 @@ static void clipboard_received_cb(GtkClipboard > *clipboard, > */ > g_warn_if_fail(type != VD_AGENT_CLIPBOARD_UTF8_TEXT); > > - spice_main_channel_clipboard_selection_notify(s->main, selection, type, > data, len); > + clipboard_selection_notify(self, selection, type, data, len); > } > > static gboolean clipboard_request(SpiceMainChannel *main, guint selection, > diff --git a/src/spice-widget.c b/src/spice-widget.c > index 316043a9..7fd8349b 100644 > --- a/src/spice-widget.c > +++ b/src/spice-widget.c > @@ -1833,6 +1833,8 @@ static gboolean focus_in_event(GtkWidget *widget, > GdkEventFocus *focus G_GNUC_UN > > DISPLAY_DEBUG(display, "%s", __FUNCTION__); > > + spice_gtk_session_set_clipboard_delayed(d->gtk_session, FALSE); > + > /* > * Ignore focus in when we already have the focus > * (this happens when doing an ungrab from the leave_event callback). > @@ -1868,6 +1870,9 @@ static gboolean focus_out_event(GtkWidget *widget, > GdkEventFocus *focus G_GNUC_U > SpiceDisplay *display = SPICE_DISPLAY(widget); > > DISPLAY_DEBUG(display, "%s", __FUNCTION__); > + > + spice_gtk_session_set_clipboard_delayed(display->priv->gtk_session, > TRUE); > + > update_display(NULL); > > /* > -- > 2.14.3 > > _______________________________________________ > Spice-devel mailing list > Spice-devel@xxxxxxxxxxxxxxxxxxxxx > https://lists.freedesktop.org/mailman/listinfo/spice-devel > _______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/spice-devel