Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Drag and drop can drop data on wrong in-VM window #8429

Open
DemiMarie opened this issue Aug 17, 2023 · 18 comments
Open

Drag and drop can drop data on wrong in-VM window #8429

DemiMarie opened this issue Aug 17, 2023 · 18 comments
Labels
affects-4.1 This issue affects Qubes OS 4.1. affects-4.2 This issue affects Qubes OS 4.2. C: gui-virtualization diagnosed Technical diagnosis has been performed (see issue comments). fixed-by-wayland Bugs that would not appear if Qubes OS used Wayland instead of X11 P: default Priority: default. Default priority for new issues, to be replaced given sufficient information.

Comments

@DemiMarie
Copy link

How to file a helpful issue

Qubes OS release

Any, probably since before 1.0.

Brief summary

It is possible for drag and drop to drop data on a window that is currently covered by a different window.

Steps to reproduce

  1. Open two gnome-text-editor windows.
  2. Cover one (but not the other) with a window from a different qube.
  3. Drag and drop text from the uncovered one to where the covered one is.

Expected behavior

Nothing happens, since cross-qube drag and drop is not supported.

Actual behavior

The text gets dropped into a hidden window.

@DemiMarie DemiMarie added T: bug P: default Priority: default. Default priority for new issues, to be replaced given sufficient information. fixed-by-wayland Bugs that would not appear if Qubes OS used Wayland instead of X11 labels Aug 17, 2023
@DemiMarie DemiMarie self-assigned this Aug 17, 2023
@andrewdavidwong
Copy link
Member

Not sure if this is already covered by #1455 or #3267. I suppose it's a slightly different case.

@andrewdavidwong andrewdavidwong added affects-4.1 This issue affects Qubes OS 4.1. affects-4.2 This issue affects Qubes OS 4.2. C: gui-virtualization labels Aug 18, 2023
@jamke
Copy link

jamke commented Aug 18, 2023

I though it is a Xen limitation, because I do not know what should be done from dom0 for preventing dropping if code in dom0 sees that mouse released over window of different qube.
If it is really possible to fix, it will be great, I have this bug for ages.

@DemiMarie
Copy link
Author

The root cause of this bug is that drag and drop is handled by the X server within the guest instead of involving the host. The Qubes team does not have the resources to patch the X server to fix this bug. Switching to Wayland will allow this to be solved by having the in-guest Wayland compositor forward all drag and drop requests to the host. This allows the “where should this data be dropped?” decision to be offloaded to the host’s Wayland compositor. The host’s compositor knows where all of the windows are and which windows are visible at any given time, so it can make the correct decision. Security will not be harmed because the host-side drag and drop will be of an object with a magic MIME type only the GUI daemon understands and with contents that are not guest-controlled. A GUI daemon will ignore data dropped by a different GUI daemon or by something that is not a GUI daemon.

@marmarek
Copy link
Member

@DemiMarie your answer is widely inaccurate.
First of all, this not "a bug", but a decision to not implement this feature at this time. And the reason is much more on higher level than a technical details. Specifically, data transfer between qubes should be an explicit action, with a proper policy enforcement etc, and drag and drop makes it too easy to make mistakes.
At some point we may decide to implement it (on X11, or Wayland, or both) as an opt-in feature or such, but since this feature ease shooting oneself in the foot, I assign it rather low priority.

@andrewdavidwong
Copy link
Member

andrewdavidwong commented Aug 19, 2023

First of all, this not "a bug", but a decision to not implement this feature at this time.

What feature are you referring to, exactly, @marmarek? I think there might be a misunderstanding, because as I understand this issue, @DemiMarie is not saying (at least not in the original description) that she wants something to happen when dragging-and-dropping from one qube to another. Rather, she is saying (at least in the original description) that (currently) nothing should happen when dragging-and-dropping from one qube to another (since no such cross-qube drag-and-drop feature has been implemented). The bug is that something does happen, namely that the dragged thing lands in a same-VM window that happens to be underneath the cross-VM window onto which the user dragged.

And the reason is much more on higher level than a technical details. Specifically, data transfer between qubes should be an explicit action, with a proper policy enforcement etc, and drag and drop makes it too easy to make mistakes. At some point we may decide to implement it (on X11, or Wayland, or both) as an opt-in feature or such, but since this feature ease shooting oneself in the foot, I assign it rather low priority.

You're referring to #6842 here, right? I don't think this issue (at least in the original description) really has anything to do with that (besides mentioning the concept). This issue is more like: "Since #6842 has not been implemented, cross-VM drag-and-drop shouldn't do anything, but instead it does this unexpected same-VM thing." It's neutral about whether #6842 should be implemented or not. But I admit that @DemiMarie's subsequent comment does seem to advocate more strongly for a specific solution that involves implementing #6842. In this respect, the original issue description and subsequent comment seem to differ.

@jamke

This comment was marked as off-topic.

@andrewdavidwong
Copy link
Member

About a bit of off-topic: should cross-qube drag and drop be implemented? [...]

This already has its own issue (#6842) and, as you acknowledged, is off-topic here. Please help us keep the issue tracker organized by avoiding off-topic remarks in issues.

@DemiMarie
Copy link
Author

First of all, this not "a bug", but a decision to not implement this feature at this time.

What feature are you referring to, exactly, @marmarek? I think there might be a misunderstanding, because as I understand this issue, @DemiMarie is not saying (at least not in the original description) that she wants something to happen when dragging-and-dropping from one qube to another. Rather, she is saying (at least in the original description) that (currently) nothing should happen when dragging-and-dropping from one qube to another (since no such cross-qube drag-and-drop feature has been implemented). The bug is that something does happen, namely that the dragged thing lands in a same-VM window that happens to be underneath the cross-VM window onto which the user dragged.

Exactly!

And the reason is much more on higher level than a technical details. Specifically, data transfer between qubes should be an explicit action, with a proper policy enforcement etc, and drag and drop makes it too easy to make mistakes. At some point we may decide to implement it (on X11, or Wayland, or both) as an opt-in feature or such, but since this feature ease shooting oneself in the foot, I assign it rather low priority.

You're referring to #6842 here, right? I don't think this issue (at least in the original description) really has anything to do with that (besides mentioning the concept). This issue is more like: "Since #6842 has not been implemented, cross-VM drag-and-drop shouldn't do anything, but instead it does this unexpected same-VM thing."

Yup!

It's neutral about whether #6842 should be implemented or not. But I admit that @DemiMarie's subsequent comment does seem to advocate more strongly for a specific solution that involves implementing #6842. In this respect, the original issue description and subsequent comment seem to differ.

That is not how I indended #8429 (comment). The GUI qube’s Wayland compositor needs to be involved in all drag and drop operations (even same-VM ones!) because it is the only part of the system that knows what window the user actually dropped the data onto. This does not mean that drag and drop between VMs, or from a VM to the host, will succeed. It will fail because the non-standard MIME type offered by the GUI daemon is not recognized by the destination program. Even if it was accepted, zero bytes would be transferred.

@DemiMarie DemiMarie removed their assignment Mar 5, 2024
@DemiMarie
Copy link
Author

DemiMarie commented Mar 5, 2024

I think this is an R: upstream issue Resolution: This issue pertains to software that the Qubes OS Project does not develop or control. in the X server. I believe (but have not tested) that Xwayland (without Qubes OS) behaves the same way.

Once Qubes OS switches to Wayland, native Wayland clients will not be impacted, but Xwayland clients will continue to be impacted.

@adw feel free to close this.

@andrewdavidwong andrewdavidwong added the R: upstream issue Resolution: This issue pertains to software that the Qubes OS Project does not develop or control. label Mar 6, 2024

This comment was marked as outdated.

@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Mar 6, 2024
@jamke
Copy link

jamke commented Mar 7, 2024

I think this is an R: upstream issue in the X server. I believe (but have not tested) that Xwayland (without Qubes OS) behaves the same way.

Well, how can we call it a Xorg issue, if in all existing GNU/Linux distributions with Xorg there is no such bug.

So, it is almost exclusively a Qubes OS problem, maybe inherited from Xen.
Does bare Xen with Xorg have this bug too?

@DemiMarie
Copy link
Author

I think this is an R: upstream issue in the X server. I believe (but have not tested) that Xwayland (without Qubes OS) behaves the same way.

Well, how can we call it a Xorg issue, if in all existing GNU/Linux distributions with Xorg there is no such bug.

So, it is almost exclusively a Qubes OS problem, maybe inherited from Xen. Does bare Xen with Xorg have this bug too?

It is not a Qubes OS problem. It also apples to Xwayland, which is the only X server still under active development. The fundamental problems are:

  1. X11 drag and drop is handled entirely via the X server and the two clients. The Qubes OS GUI agent (in Qubes OS) or Wayland compositor (in Xwayland) is not involved.
  2. Neither the X server nor the clients are aware of if a window is currently visible or not. Neither Xwayland nor the Qubes OS GUI protocol expose this information.

There are several ways to solve this problem:

  1. Provide the X server with enough information to behave correctly, and modify the X server to use this information.
  2. Modify the X server to proxy all drag and drop operations via the window manager, which can in turn delegate the selection of a destination window (not the data transfer!) to the GUI VM.
  3. Switch to a Wayland compositor in the guest, which can in turn delegate the selection of a destination window (not the data transfer!) to the GUI VM.

Options 1 and 2 both require X server changes. However, Xorg is in maintenance mode right now, so the Qubes OS team would need to do this work. Such work would compete with other tasks that need to be done, such as Wayland support (option 3). Additionally, option 1 also creates an information leak, because “is this window visible?” is not information that a VM needs to have.

Option 3, however, needs to be done anyway, for at least two reasons:

  1. I am not aware of a maintainable solution for GPU acceleration with the current GUI protocol. To the best of my knowledge, I know more about GPU acceleration than anyone else on the Qubes team, and I certainly do not feel comfortable adding such support without substantial review by third parties. The requirements, especially around synchronization, are just too subtle. Furthermore, any bugs would be nearly impossible to fix without outside help.
  2. The Linux graphics stack is moving to Wayland. GTK4 is Wayland-first, and GTK5’s X11 backend may not be build by default. Therefore, continuing to only support X11 is increasingly unsustainable. I, at least, would much prefer to implement Wayland support now, before applications start acquiring hard dependencies on it.

Overall, the reason for closing this issue is not that it cannot be fixed, but rather that fixing it would require a substantial investment into a program (Xorg) and a protocol (X11) that is ultimately a technological dead end. The time I spend on fixing this problem could also be spent on providing Wayland support to all users, which will in turn allow for GPU-accelerated rendering in guests. In turn, GPU acceleration is a massive performance improvement. For more than a few people, it is the feature that will make using Qubes OS possible.

@jamke
Copy link

jamke commented Mar 8, 2024

It is not a Qubes OS problem. It also apples to Xwayland, which is the only X server still under active development. The fundamental problems are:

X11 drag and drop is handled entirely via the X server and the two clients. The Qubes OS GUI agent (in Qubes OS) or Wayland compositor (in Xwayland) is not involved.
Neither the X server nor the clients are aware of if a window is currently visible or not. Neither Xwayland nor the Qubes OS GUI protocol expose this information.

I understand you, just that I propose to look at this issue from the user's point of view. The situation is this:

  1. In GNU/Linux based on Xorg, Xwayland and etc. drag and drop works as expected. So, upstream will not fix what it has no issue with.
  2. In Qubes OS drag and drop does not work properly. It would be fine, if cross-qube drag and drop did not work, after all it may raise security concern. But the real issue is THAT IT WORKS INCORRECTLY, user drops stuff to non-visible windows, WRONG window, it's a bug for sure.

You talk about technical details, that it currently cannot be fixed in easy or proper way. But from user perspective it is a QubesOS-specific bug, details does not matter for them.

Overall, the reason for closing this issue is not that it cannot be fixed, but rather that fixing it would require a substantial investment into a program (Xorg) and a protocol (X11) that is ultimately a technological dead end.

That I also understand and agree on. I personally think that it is not so important to fix this bug quickly, because it exists in Qubes OS for a looong time, at least starting from like R2beta when I used Qubes OS for the first time. All users face this issue, but after several wrong doings, they get a habit to avoid it.

I just do not agree that it is possible to close such tickets as upstream issue, because it mean that this exact problem exists for some other systems, which is not a case, AFAIK. I hope, I explained well enough.

@andrewdavidwong @DemiMarie
To my opinion, the issue exists and Qubes OS users face it (and GNU/Linux users do not) so I propose the issue to stay open until the wrong behavior described in the title of the issue will be possible to solve in Qubes OS (does not matter, due to upstream improvements or not). The issue of wrong drap and drop target window can stay open for as long as needed without being addressed by devs.

@DemiMarie
Copy link
Author

  • In GNU/Linux based on Xorg, Xwayland and etc. drag and drop works as expected. So, upstream will not fix what it has no issue with.

The same problem can be reproduced with Xwayland and Weston, so I filed https://gitlab.freedesktop.org/xorg/xserver/-/issues/1650 and then https://gitlab.freedesktop.org/wayland/weston/-/issues/889.

@marmarek
Copy link
Member

marmarek commented Mar 8, 2024

I also don't agree with "upstream issue" classification. The real issue is that window order (and visibility) in VM may be different than what user sees. It's basically a limitation of how qubes-gui agent works, not a fundamental X11 issue like @DemiMarie tries to describe it. After all, it works just fine on non-qubes X11 systems.

Anyway, I wouldn't expect it to be fixed anytime soon...

@DemiMarie
Copy link
Author

I also don't agree with "upstream issue" classification. The real issue is that window order (and visibility) in VM may be different than what user sees. It's basically a limitation of how qubes-gui agent works, not a fundamental X11 issue like @DemiMarie tries to describe it. After all, it works just fine on non-qubes X11 systems.

It’s not just a Qubes bug — Weston + Xwayland is also affected. That said, I think this is fixable (at least in theory), so removing R: upstream issue Resolution: This issue pertains to software that the Qubes OS Project does not develop or control. .

Anyway, I wouldn't expect it to be fixed anytime soon...

I have an idea for how to fix the problem:

  1. Create a single full-screen window. This window is completely transparent, so it (assuming I didn’t make a mistake somewhere) won’t show up in screenshots. Call this window A. A acts as a stand-in for the rest of the system.
  2. Whenever the mouse moves over a window, the GUI agent moves that window in front of A.
  3. Whenever the mouse leaves a window, the GUI agent moves that window behind A.
  4. A doesn’t support drag and drop at all, so any attempt to drop onto it fails.

For this to work, the GUI agent needs to know which (if any) window the mouse is over at any given time. This information is available via MSG_CROSSING events, so no changes to the GUI daemon or GUI protocol are required.

The trade-off of this approach is that windows that the mouse is not over will appear to be obscured. I’m not sure if that is a problem or not.

@DemiMarie DemiMarie reopened this Mar 8, 2024
@DemiMarie DemiMarie removed the R: upstream issue Resolution: This issue pertains to software that the Qubes OS Project does not develop or control. label Mar 8, 2024
@andrewdavidwong andrewdavidwong added the diagnosed Technical diagnosis has been performed (see issue comments). label Mar 9, 2024
@jamke
Copy link

jamke commented Mar 9, 2024

I have an idea for how to fix the problem:

I am not sure that such hack should be implemented. It can cause problems with software that relies on mouse moving over events and etc. The current situation is not that bad, just not obvious to new users.

@DemiMarie
Copy link
Author

I have an idea for how to fix the problem:

I am not sure that such hack should be implemented. It can cause problems with software that relies on mouse moving over events and etc. The current situation is not that bad, just not obvious to new users.

@marmarek: do you think this should be R: declined Resolution: While a legitimate bug or proposal, it has been decided that no action will be taken. ?

The current situation is:

  • Fixing this with X11 appears to be very difficult without ugly hacks or information leaks.
  • The problem also appears when dragging and dropping between Xwayland applications under at least one Wayland compositor (Weston).
  • Once Wayland support is implemented, Wayland-native applications will simply not have the problem because of Drag and drop can drop data on wrong in-VM window #8429 (comment).
  • We need Wayland for GPU acceleration anyway.
  • GPU acceleration will happen eventually, so Wayland will happen eventually.
  • A proper fix (that does not require ugly hacks) will requre X server changes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
affects-4.1 This issue affects Qubes OS 4.1. affects-4.2 This issue affects Qubes OS 4.2. C: gui-virtualization diagnosed Technical diagnosis has been performed (see issue comments). fixed-by-wayland Bugs that would not appear if Qubes OS used Wayland instead of X11 P: default Priority: default. Default priority for new issues, to be replaced given sufficient information.
Projects
None yet
Development

No branches or pull requests

4 participants