QubesOS / qubes-issues

The Qubes OS Project issue tracker
https://www.qubes-os.org/doc/issue-tracking/
539 stars 48 forks source link

GUI-related feature flags not validated #7730

Open DemiMarie opened 2 years ago

DemiMarie commented 2 years ago

How to file a helpful issue

Qubes OS release

R4.1

Brief summary

Various GUI-related features are not validated by qubesd. Once https://github.com/QubesOS/qubes-core-admin-client/pull/221 lands, this will cause qvm-start-daemon to produce an invalid GUI daemon configuration file, preventing the GUI daemon from starting.

Steps to reproduce

Have https://github.com/QubesOS/qubes-core-admin-client/pull/221 and set gui-subwindows feature to something invalid.

Expected behavior

Feature set call is rejected by qubesd. Attempts to set unknown gui-* features should likely be rejected unconditionally, as these could acquire a meaning in the future.

Actual behavior

Feature set call is not rejected by qubesd.

GWeck commented 2 years ago

Even documented gui-* features are handled inconsistently:

All very confusing.

marmarek commented 2 years ago

Documentation says "To change a given GUI option globally, set the gui-default-{option} feature on the GuiVM for that qube." - if your GuiVM is dom0 (the default) then gui-default-* needs to be set on dom0.

GWeck commented 2 years ago

This makes it clear: gui-default-allow-utf8-titles should have been set not for <VMname>, but for dom0, or possibly sys-gui or sys-gui-gpu. But then this would apply to all qubes using that GuiVM, not just that qube, as stated in the documentation of qvm-features. I checked, and it works as expected, and I updated the documentation accordingly (see pull request #207 Thank you for this clarification!

The confusion about this behavior could perhaps be reduced by

marmarek commented 2 years ago
  • the --unset command just clears them but does not remove them.

Can you clarify? What exact command you tried and what was the result?

GWeck commented 2 years ago

I repeated the tests now, and qvm-features --unset now removed the meaningless parameters, like it should. Probably the earlier tests brought something into an inconsistent state causing the command to fail. So I suppose that there is nothing to do in this respect. Sorry for the inconvenience!

Still it would be nice if any attempt to set a gui-default-* option for a non-GuiVM would cause some meaningfull warning.

alimirjamali commented 4 days ago

Various GUI-related features are not validated by qubesd.

This appears to be easy to solve. The current GUI_DAEMON_OPTIONS is a list of tuples (feature, type). And type could be changed to lambda validation functions to be used in retrieve_gui_daemon_options function. I will work on it.

alimirjamali commented 3 days ago

User-end validation (within qvm-start-daemon) is done. It skips invalid values and logs an error messages. Also sends a visual notification via notify-send.

Now the same logic could be applied to qubesd to forbid users from setting invalid values for recognized features and setting unknown gui-* and default-gui-* features (but is it really necessary with the above patch?)

There are some concerns. Specific libraries are required for validating (clipboard) key sequences and valid window_background_color values. Using them within qubes-core-admin-client should be OK (hopefully). Depending qubesd on them is something else.