[Spice-devel] RFC [spice-gtk] session: Allow to delay sending clipboard to the guest
Jakub Janků
janku.jakub.jj at gmail.com
Tue Jan 9 22:26:24 UTC 2018
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 <cfergeau at redhat.com>
> ---
> 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[CLIP
> BOARD_LAST];
> + gboolean clipboard_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 */
> 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,
> + gboolea
> n 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;
> + }
> +}
> +
This discards the previous data, so if the guest requested the
clipboard N times, it only receives one response. Note that the
individual requests could have different types. How does the vdagent
deal with that?
> 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);
>
> /*
Cheers,
Jakub
More information about the Spice-devel
mailing list