Add sanitization script for XFCE settings in prod #475

eloquence · 2020-02-29T02:19:32Z

Ensures that the "Suspend" and "Hibernate" options are not available in the user menu (provided by the "Action Buttons" plugin in XFCE) or the logout dialog, in staging or production. Adjusts the icon size so desktop icon caption is readable.

This is done via a new dom0 Salt script update-xfce-settings, so we have a single place we can update if XFCE/Qubes behavior changes. I've added it to the RPM but not bumped the RPM version.

Resolves Fully power off workstation after lid close #473
Resolves Tweak updater icon, label #413

Status

Ready for review

Test plan

Testing production config

From a make clean state or a previously provisioned dev environment:

Verify that your desktop icon size is set to the default value (right-click on desktop, "Desktop Settings", "Icons" - should be set to 32px). Cancel out of the dialog.
Click the user menu in the top right and verify that it uses Qubes OS defaults. You should have the following options: Lock Screen, Switch User, Suspend, Shut Down, Log Out. The logout dialog should give you the option to suspend or hibernate.
make clone this branch into dom0
Ensure you have a valid config.json. Run scripts/configure-environment --environment prod to switch it to production. (Note: That script currently messes up JSON formatting, make sure you have a copy if you care about that.)
Run make prep-dom0.
- Observe that the desktop icon is now larger with readable text, and that the icon size has been increased to 64 px.
- Observe that the user menu now has the options Lock Screen, Restart, Shut down, and that you are no longer able to suspend or hibernate form the logout menu.

Testing cleanup

Run make clean
- Observe that the desktop icon size (via settings), menu, and logout dialog have been resest to their original values.

Testing that only the icon size is modified in development environment

Set your config.json to dev using the same method as before.
Rebuild your environment with make all
- Observe that the icon size is increased, and that the user menu and logout dialog are not modified from the default.

Checklist

Contains required RPM updates (but does not bump RPM version yet)
make test run in dom0

emkll

Thanks @eloquence took a first pass

The icon size changes work well, and the full text is visible:

However, had some issues with the power management changes:

Errors while runningdisable-unsafe-power-management and reset-power-management (see inline for error message)
update-xfce-settings is not cleaned by the clean-salt makefile target. We should either prefix the file by sd or securedrop, or add it to the target, see:

securedrop-workstation/Makefile

Line 79 in c54d5dd

clean-salt: assert-dom0 ## Purges SD Salt configuration from dom0

. If we fail to clean that file, it will persist across installs and issues could arise in the future (use of stale files, non-detection of errors in packaging, etc)

emkll · 2020-03-02T17:47:51Z

dom0/update-xfce-settings

+  # Hide hibernate button in logout menu
+  xfconf-query -c xfce4-session -np '/shutdown/ShowHibernate' -t 'bool' -s 'false'
+  # Trim action button menu to the essentials
+  xfconf-query -c xfce4-panel -p '/plugins/plugin-2/items' \


Getting some errors here:

Property "/plugins/plugin-2/items" does not exist on channel "xfce4-panel". If a new property should be created, use the --create option

Hmm, now I'm worried that the plugin number is not consistent from install to install. Let me do some digging on this one, thanks for flagging.

(Should be resolved now.)

emkll · 2020-03-02T17:48:08Z

dom0/update-xfce-settings

+  xfconf-query -c xfce4-session -p '/shutdown/ShowSuspend' -r
+  xfconf-query -c xfce4-session -p '/shutdown/ShowHibernate' -r
+  # Does not retain its default config, so resetting to Qubes default values
+  xfconf-query -c xfce4-panel -p '/plugins/plugin-2/items' \


Same error as above, maybe resolved by resolving the disable action:

Property "/plugins/plugin-2/items" does not exist on channel "xfce4-panel". If a new property should be created, use the --create option

(Should be resolved now.)

emkll · 2020-03-02T17:52:31Z

dom0/update-xfce-settings

+TASK=${1:-none}
+ICONSIZE=64
+
+if ! [ -x "$(command -v xfconf-query)" ]; then


👍 xfconf-query is present by default

Just added this check for future-proofing and in case folks try to run it outside of Qubes

eloquence · 2020-03-02T19:52:53Z

Errors while running disable-unsafe-power-management and reset-power-management (see inline for error message)

I believe the root cause is that the items are not initialized until the user modifies the default. I never ran into this during testing because I had modified the setting already. This should now be resolved; please let me know if you still see errors.

update-xfce-settings is not cleaned by the clean-salt makefile target.

This should also be resolved.

eloquence · 2020-03-02T20:51:37Z

I'm currently noticing that the settings changes never seem to take effect when run via Salt. The xfconf-query command runs just fine as the GUI user, but it has no effect, even if DISPLAY is set. Will poke further; @emkll, if you have time, would be good to know if you can reproduce.

eloquence · 2020-03-03T00:31:14Z

xfconf-query requires a DBUS session for the user whose settings are being changed. I'm now exporting the expected value of this variable for the GUI user if it is not set. I believe this will only work while the GUI user has an active session, which should be true both for dev and prod installs (it works for me in dev, I've not tested prod yet).

eloquence · 2020-03-03T01:10:23Z

I've added a test plan; I believe this is ready for a formal review now. I've not added a test plan for the securedrop-admin workflow that installs the RPM, would appreciate guidance on the best way to go about that for a PR like this one.

emkll

Thanks @eloquence for the changes, this now works as expected for me.

One request: can we remove the "log off" option as well? the reason i ask is that it's very similar to sleep (without the sleep) where it's implied that the session is ended. In reality, it's similar to just locking the screen: The option Save session for future logins is enabled by default, and VMs are always powered on during that time.

I will be running a couple more times through the test plan on a clean install to ensure this logic is reliable, because this logic has no test coverage, due to it being limited to staging and production environments.

eloquence · 2020-03-03T22:14:29Z

That's a great flag @emkll , it should be easy to remove, will poke while Qubes is doing something slow :)

eloquence · 2020-03-03T22:34:49Z

Well, it would be easy, if XFCE exposed that property like it does for "Suspend" and "Hibernate", but it does not. Internet wisdom suggests altering the logout script to force a shutdown on logout.

It sounds like the behavior you describe may be captured by the "save on exit" preference in the session manager, which we can programmatically alter, but it is not checked for my user account, and I don't recall changing it. Will test with a fresh account to examine default behavior.

eloquence · 2020-03-04T02:12:51Z

To recap, altering what appears on the "Log out" dialog beyond removing removing the suspend and hibernate buttons is difficult, but I think we can just remove the "Log out" dialog altogether (which may have been what you were suggesting). In that case we don't have to update the preferences for the dialog either, simplifying the code somewhat.

I'll do some testing to see if that works and if so, will make those changes. The user will then just get the "Shut down" and "Restart" options in the user menu (the "action buttons" plugin).

eloquence · 2020-03-04T05:50:24Z

I think we can just remove the "Log out" dialog altogether

I've now removed it from the user actions. There's still a "Log out" action in the application menu, which will bring up the dialog (which is why I kept the code to remove "suspend" and "hibernate" from that dialog); I think that's reasonable for this iteration, and perhaps in a future release we can investigate updating that menu, as well.

emkll

Thanks @eloquence, looks good to merge from my perspective. Before merging, would you mind squashing your commits?

Tested as follows:

Testing production config

From a make clean state or a previously provisioned dev environment:

Verify that your desktop icon size is set to the default value (right-click on desktop, "Desktop Settings", "Icons" - should be set to 32px). Cancel out of the dialog.
Click the user menu in the top right and verify that it uses Qubes OS defaults. You should have the following options: Lock Screen, Switch User, Suspend, Shut Down, Log Out. The logout dialog should give you the option to suspend or hibernate.
~~make clone this branch into dom0~~ I built/used an RPM instead
Ensure you have a valid config.json. Run scripts/configure-environment --environment prod to switch it to production. (Note: That script currently messes up JSON formatting, make sure you have a copy if you care about that.)
Run make prep-dom0.
- Observe that the desktop icon is now larger with readable text, and that the icon size has been increased to 64 px.
- Observe that the user menu now has the options Lock Screen, Restart, Shut down, and that you are no longer able to suspend or hibernate form the logout menu.

Testing cleanup

Run make clean
- Observe that the desktop icon size (via settings), menu, and logout dialog have been resest to their original values.

Testing that only the icon size is modified in development environment

Set your config.json to dev using the same method as before.
Rebuild your environment with make all
- Observe that the icon size is increased, and that the user menu and logout dialog are not modified from the default.

before :

after make all:

after make clean:

I realize now that I unfortunately hid the icon on the second screenshot, but the icon size is correctly changed.

Removes suspend/hibernate options and hides logout in user menu (due to session save behavior); adjusts icon size for usability and accessibility.

eloquence · 2020-03-05T16:31:20Z

Done. :)

emkll

thanks @eloquence !

Add sanitization script for XFCE settings in prod

emkll suggested changes Mar 2, 2020

View reviewed changes

eloquence force-pushed the sanitize-xfce-settings branch from ec4cfcd to f2a4eda Compare March 2, 2020 19:59

eloquence marked this pull request as ready for review March 3, 2020 01:06

emkll reviewed Mar 3, 2020

View reviewed changes

emkll reviewed Mar 5, 2020

View reviewed changes

Add sanitization script for XFCE settings in prod

16656d3

Removes suspend/hibernate options and hides logout in user menu (due to session save behavior); adjusts icon size for usability and accessibility.

eloquence force-pushed the sanitize-xfce-settings branch from 9deed56 to 16656d3 Compare March 5, 2020 16:30

emkll approved these changes Mar 5, 2020

View reviewed changes

emkll merged commit 7917067 into master Mar 5, 2020

emkll deleted the sanitize-xfce-settings branch March 5, 2020 16:56

emkll mentioned this pull request Mar 5, 2020

Force hard system shutdown instead of suspend/hibernate #178

Closed

conorsch mentioned this pull request Oct 23, 2020

Prod uninstall doesn't reset power menu #625

Closed

cfm pushed a commit that referenced this pull request Apr 1, 2024

Merge pull request #475 from freedomofpress/sanitize-xfce-settings

0d8d209

Add sanitization script for XFCE settings in prod

Add sanitization script for XFCE settings in prod #475

Add sanitization script for XFCE settings in prod #475

Uh oh!

Conversation

eloquence commented Feb 29, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Status

Test plan

Testing production config

Testing cleanup

Testing that only the icon size is modified in development environment

Checklist

Uh oh!

emkll left a comment

Choose a reason for hiding this comment

Uh oh!

emkll Mar 2, 2020

Choose a reason for hiding this comment

Uh oh!

eloquence Mar 2, 2020

Choose a reason for hiding this comment

Uh oh!

eloquence Mar 3, 2020

Choose a reason for hiding this comment

Uh oh!

emkll Mar 2, 2020

Choose a reason for hiding this comment

Uh oh!

eloquence Mar 3, 2020

Choose a reason for hiding this comment

Uh oh!

emkll Mar 2, 2020

Choose a reason for hiding this comment

Uh oh!

eloquence Mar 2, 2020

Choose a reason for hiding this comment

Uh oh!

eloquence commented Mar 2, 2020

Uh oh!

eloquence commented Mar 2, 2020

Uh oh!

eloquence commented Mar 3, 2020

Uh oh!

eloquence commented Mar 3, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

emkll left a comment

Choose a reason for hiding this comment

Uh oh!

eloquence commented Mar 3, 2020

Uh oh!

eloquence commented Mar 3, 2020

Uh oh!

eloquence commented Mar 4, 2020

Uh oh!

eloquence commented Mar 4, 2020

Uh oh!

emkll left a comment

Choose a reason for hiding this comment

Testing production config

Testing cleanup

Testing that only the icon size is modified in development environment

Uh oh!

eloquence commented Mar 5, 2020

Uh oh!

emkll left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

eloquence commented Feb 29, 2020 •

edited

Loading

eloquence commented Mar 3, 2020 •

edited

Loading