Add missing GUI daemon options #221
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
DemiMarie
force-pushed
the
missing-gui-daemon-flags
branch
4 times, most recently
from
210bcc1
to
077275f
Compare
|
PipelineRetryFailed |
Codecov Report
@@ Coverage Diff @@
## master #221 +/- ##
=======================================
Coverage 75.17% 75.17%
=======================================
Files 51 51
Lines 7788 7788
=======================================
Hits 5855 5855
Misses 1933 1933
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
DemiMarie
added a commit
to DemiMarie/qubes-gui-daemon
that referenced
this pull request
Sep 29, 2022
This was supposed to be done in 157ca14, but the implementation was wrong: it set the override-redirect flag to what the GUI daemon thought the current value was, not 0. Therefore, the GUI daemon could become confused as to the override-redirect state of a window. This turns out not to be exploitable in practice unless subwindows are enabled. With subwindows disabled, it is not possible to prevent the window from being marked as docked, which prevents moving, resizing, or mapping the window. Since the window cannot be mapped by the VM, it cannot appear on screen unless the embedder tells the GUI daemon to map it. The embedder will resize the window before sending this message, so there is no window of vulnerability. Furthermore, subwindows are disabled in the default configuration. Because QubesOS/qubes-core-admin-client#221 has not been merged, it is not possible to turn them on with qvm-features. Turning them on manually (by editing qvm-start-daemon or similar) is not supported. Fixes: 157ca14 ("Unset override-redirect flag when docking, instead of preventing dock")
DemiMarie
added a commit
to DemiMarie/qubes-gui-daemon
that referenced
this pull request
Sep 29, 2022
The code had security problems (QubesOS#119), could not be enabled (QubesOS/qubes-core-admin-client#221), and was involved in one exploit for QSB#072. Furthermore, there are no known users: neither the Linux nor the Windows GUI agent use subwindows, and the Wayland GUI agent doesn't use them either. In short, subwindows support seems to cause nothing but problems, so just rip it out.
subwindows= was added in GUI daemon commit QubesOS/qubes-gui-daemon@5978391 and override_redirect= was added in commit QubesOS/qubes-gui-daemon@cd6f308. Allow override_redirect= to be set with qvm-features. subwindows= is deliberately *not* allowed to be set, because it turns out to be insecure (QubesOS/qubes-gui-daemon#119).
DemiMarie
force-pushed
the
missing-gui-daemon-flags
branch
from
077275f
to
4f20c1d
Compare
DemiMarie
added a commit
to DemiMarie/qubes-gui-daemon
that referenced
this pull request
Oct 16, 2022
The code had security problems (QubesOS#119), could not be enabled (QubesOS/qubes-core-admin-client#221), and was involved in one exploit for QSB#072. Furthermore, there are no known users: neither the Linux nor the Windows GUI agent use subwindows, and the Wayland GUI agent doesn't use them either. In short, subwindows support seems to cause nothing but problems, so just rip it out.
marmarek
approved these changes
Oct 17, 2022
marmarek
pushed a commit
to QubesOS/qubes-gui-daemon
that referenced
this pull request
Oct 17, 2022
This was supposed to be done in 157ca14, but the implementation was wrong: it set the override-redirect flag to what the GUI daemon thought the current value was, not 0. Therefore, the GUI daemon could become confused as to the override-redirect state of a window. This turns out not to be exploitable in practice unless subwindows are enabled. With subwindows disabled, it is not possible to prevent the window from being marked as docked, which prevents moving, resizing, or mapping the window. Since the window cannot be mapped by the VM, it cannot appear on screen unless the embedder tells the GUI daemon to map it. The embedder will resize the window before sending this message, so there is no window of vulnerability. Furthermore, subwindows are disabled in the default configuration. Because QubesOS/qubes-core-admin-client#221 has not been merged, it is not possible to turn them on with qvm-features. Turning them on manually (by editing qvm-start-daemon or similar) is not supported. Fixes: 157ca14 ("Unset override-redirect flag when docking, instead of preventing dock") (cherry picked from commit a91639e)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
subwindows= was added in GUI daemon commit QubesOS/qubes-gui-daemon@5978391 and override_redirect= was added in commit QubesOS/qubes-gui-daemon@cd6f308. Allow them to be set via qvm-features.