Rewrite the Persistent Storage settings in Python
Why
persistence-setup
was written in Perl and, looking at the Git history, it might have prevented other people than intrigeri to do significant changes to it. I could only spot @segfault adding a feature in #17136 (closed).
It's an application that is here to stay, one of the first steps in the user journey, and in need of UX improvements.
I'm hoping that rewriting it in Python might also us to reduce the bus factor and improve it more rapidly.
User research findings
During #18784 (closed), the Python rewrite would have helped with the following issues. Only P3 and P4 we asked to save the Wi-Fi password to the Persistent Storage:
- P3 and P4 connected to the Wi-Fi after creating a Persistent Storage but before restarting Tails. They got disappointed by a lost password after restarting.
- P3 got the Persistent Storage settings open on full screen (while P4 didn't on the same laptop). The Python rewrite does better at this.
- P4 found the intro of the Persistent Storage scary. It also doesn't help understanding that the Persistent Storage is useful for (on top of all these warnings).
- I got confused myself about how to close the current Persistent Storage that has no closing widget in its top bar.
Scope
The issues tackled during the first iteration of the rewrite are listed on !339 (closed), that closes them.
Design
As part of this work, we're implementing parts of this redesign: https://gitlab.tails.boum.org/tails/blueprints/-/wikis/Persistent-Storage-redesign
TODO
Blockers for public call for testing
Issues that happen when deactivating a feature
We decided to try to consistently apply deactivation immediately. Once we implement deletion-on-deactivation (Delete corresponding data when a feature of the... (#8447 - closed)), we'll have new UI and in there we can tell the user what's going to happen, e.g. we're going to disconnect from the network. That's a 2nd new reason to implement this feature (on top of fixing most issues that happen when activating a feature at runtime). Not a blocker for beta, but the beta will tell us if it's a blocker for release.
-
After deactivating the Tor Bridge feature, the persistent bridge is still used, even after disconnecting from the network and reconnecting. -
After deactivating the Network Connections feature, the connection previously configured from Persistent Storage is still used, and all connections stored on Persistent Storage are still available. - After deactivating the Additional Software feature, packages already installed during login remain installed: it's OK.
Other issues
-
After deactivating the Additional Software feature and restarting Tails, Tails tells me that the installation of my additional software has failed. Indeed, they're still configured in live-additional-software.conf
, but not available offline anymore. After connecting to Tor, the Additional Software upgrade service installs these packages, which is not what the user wants. I did not check whether it's a regression. Adding a coupleConditionPathIsMountPoint=
totails-additional-software-*.service
should fix this. Alternatively we could emptylive-additional-software.conf
when deactivating this feature, but this would be inconsistent with how we're currently handling other persistent data when deactivating features. -
Check that USB stick was installed with a supported installation method: -
Partition table is GPT -
There's a (system) partition called Tails
- Rationale: let's not pretend supporting anything else (e.g. USB sticks installed with Rufus or similar), which can either break partition creation with an obscure error message, or more generally put Tails in a situation we never considered, which yields bugs, poor UX, and bug reports that are hard to do anything about.
- Conclusion: not blocker for beta, but add them if it's cheap, else let's wait and see if more new bugs are reported.
- Implementation notes:
- Should probably be implemented in
BootDevice.get_tails_boot_device
- For the error message, see Explain the user why creating a Persistent Stor... (#18734)
- Should probably be implemented in
-
-
Finish implementing design for passphrase input... (#19146): use the text from the design, postpone the rest -
Identify blockers in the delta between the current implementation and the design -
Compare and list the differences -
Check with UX what is a blocker for releasing a first iteration of the Python rewrite -
Ensure the important missing bits are tracked somewhere
-
-
Test enabling/disabling without restarting the non-trivial features -
Printers -
Network Connections -
Tor Bridge -
Additional Software -
GnuPG (in particular background processes such as gpg-agent)
-
-
live-additional-software.conf
is empty after initial Persistent Storage setup for Additional Software, unless/until the user closestps-frontend
[intrigeri]- Explanation:
tails-additional-software
waits fortps-frontend
to complete (vialaunch_tps_frontend
→launch_x_application
) before it writes the correct contents tolive-additional-software.conf
- Simplest solution:
-
start tps-frontend
in the background -
wait for the AdditionalSoftware feature to be active
-
-
Once this is fixed, remove the temporary workaround in the test suite: git log --grep 'Test suite: workaround regression in Additional Software'
- Explanation:
-
Allow non-privileged applications, that are not allowed to talk to tps, to enable their persistent storage feature on an already unlocked Persistent Storage [intrigeri] - We currently have 1 single use case for this:
tails-additional-software-config
(the GUI to configure your list of Additional Software, that has a button to actually enable Additional Software persistent storage feature) runs as the amnesia user. So for this 1st iteration, something that would solve this problem somehow would be good enough. Keeping in mind that long term, the design showcases more examples for such a feature. - Proposal 1: Short-term hack, which is not ideal in terms of security, but not worse than current Tails: add support for something like the old
--force_enable_preset AdditionalSoftware
and allowamnesia
to callsudo -u tails-persistent-storage /usr/local/lib/connect-drop-tps --force_enable_preset AdditionalSoftware
. It's not obviously cheaper than implement than the nicer solution described below, though.- Alternate way: add a D-Bus method to tps's feature interface "ActivateAndRaise", that would activate the persistent feature and open the tps GUI so the user sees the resulting config.
- The caller needs to wait until the persistence feature is enabled using
tpscli is-active $FEATURE
.
- Proposal 2: We could have some sort of privileged proxy (aka. portal) with a UI. Any application running as amnesia could ask this portal to enable a persistence feature, and the portal would 1. let the user authorize the operation with the just-in-time dialog; 2. take care of the progress UI; 3. itself be allowed to ask tps to do the work.
- This proxy can be either an app that amnesia can start with sudo, or a D-Bus activated service.
- The non-privileged app needs to know when the operation finishes and whether it succeeded.
- In some cases this will lead us to ask essentially the same question twice in a row: in the app (or GNOME notification) and then with the portal. It's 1 more step for the user; it also allows us to give just-in-time help.
- How would it work to be connected to 2 D-Bus buses (the regular amnesia one, where it would act as a service, and the tps one, as a client)?
- Conclusion: we'll go with Proposal 1 for the first iteration, that's probably much cheaper and is not a security regression. The fancier bits will have to wait.
- Update: actually it was simpler than this, see be97bf0f.
-
Implement a fix -
Remove launch_tps_frontend
, which is not used anymore
- We currently have 1 single use case for this:
-
Design and implement user story wrt. Dotfiles [intrigeri] - If I try to activate the Dotfiles feature, then I'm told I should first create
/live/persistence/TailsData_unlocked/dotfiles
myself, but I'm not allowed to do so without being root. The feature is not activated.- IMO we should create that directory automatically. Incidentally, we already have the code to do that in
_activate_using_symlinks()
, but we never reach it in this scenario. -- intrigeri → sajolida agreed.
- IMO we should create that directory automatically. Incidentally, we already have the code to do that in
- If I try to activate the Dotfiles feature, and I've already manually created
/live/persistence/TailsData_unlocked/dotfiles
myself but it's still empty, then I'm told I should first copy files into it. The feature is not activated.- IMO we should not error out, and instead we should activate the feature, even if there's nothing to do about it yet. The design includes an info button for the user to learn how to use this. -- intrigeri → sajolida agreed.
- If I upgrade a Tails 5.3.1 with dotfiles feature enabled (which created an empty dotfiles directory) to this branch, I'm told persistence could not be enabled. Actually everything except dotfiles could be enabled.
-
Once this is fixed, remove the temporary workaround in the test suite: git log --grep 'Test suite: add temporary workaround for tps buggy user story wrt. dotfiles' -F
- If I try to activate the Dotfiles feature, then I'm told I should first create
-
Tor Connection writes to non-persistent config file (via FD passed by connect-drop
) after making/var/lib/tca
persistent [intrigeri]- Impact:
- The persistent
tca.conf
is empty after configuring bridges configured in Tor Connection at the same time as enabling persistent Tor config (persistent_tor_bridges.feature
). - If I upgrade a Tails 5.3.1 with Tor bridges feature enabled and a bridge saved in there, at the "Configure a Tor bridge" step, TCA tells me "Failed to configure your Persistent Storage", and tps complains that we're asking it to enable the
TorConfiguration
feature that's already enabled. But my bridge is correctly configured.
- The persistent
- When do we read this data?
- Can we unlock PS from the GNOME session? (i.e. after tca has started) If so, we have a problem. Else, it's simpler.
- I disabled the half-baked support for unlocking PS from the GNOME session, so that won't be a problem yet.
- Can we unlock PS from the GNOME session? (i.e. after tca has started) If so, we have a problem. Else, it's simpler.
- Potential solutions:
- pass the FD after the fact: hard to get it right → no
- Instead of passing a FD to
tca.conf
to Tor Connection, mediate all read/write operations to that file viatca-portal
. Beware: line/arg length limit for JsonRPC thingie in tca-portal might be in place.
- Impact:
-
Review and cleanup config/chroot_local-includes/etc/sudoers.d/zzz_tps-frontend
- That file contains commented out rules, which suggests it's somewhat WIP.
-
Can we remove these commented out rules? -
Do we need all the config lines that are currently enabled?
-
Run the test suite without the "XXX" commits that grant extra debugging permissions [intrigeri] -
Manually test upgrading a Tails 5.x with persistent storage to this branch [intrigeri] -
Adapt test suite to new Persistent Storage (#19001 - closed) -
Drop incomplete attempt to support unlocking Persistent Storage after login - The design does not include this feature, but the actual implementation does. In the current state, it's not going to work well for some features: at least for Network connections and Printers, mounting a new config in
/etc
after the corresponding service has started is not going to be effective. Similarly, Additional Software won't be installed, and TCA won't use the config file from persistent storage. Let's not include half-baked features in this first iteration.
- The design does not include this feature, but the actual implementation does. In the current state, it's not going to work well for some features: at least for Network connections and Printers, mounting a new config in
-
Drop or finish incomplete attempt to support deleting an unlocked Persistent Storage - This feature would be nice, but it is not required in the design. It's implemented but the check for conflicting apps is somewhat buggy: the modal dialog asking me to close the conflicting app, with a single Cancel button, never vanishes even after I have closed it; however the deletion operation resumes and completes successfully. Let's do whatever is cheaper between hiding the Delete button for unlocked Persistent Storage, and fixing this.
-
Adjust to, or revert, the removal of persistence_is_enabled
intails-greeter.sh
- We still use this function in the
unsafe-browser
script.
- We still use this function in the
-
Verify that /usr/local/bin/electrum
's check for persistence works- Why: I don't think the
amnesia
user can usetpscli is-active
-- intrigeri - How: test without the debugging permissions set by the "XXX" commits in
/etc/dbus-1/system.d/org.boum.tails.PersistentStorage.conf
- Why: I don't think the
-
Go through the "XXX" in the diff and triage them -
live-additional-software.conf
is incorrectly owned byroot:root
- Reproducer: "Scenario: I set up Additional Software when installing a package without persistent partition and the package is installed next time I start Tails"
-
config/chroot_local-includes/usr/local/lib/persistent-storage/on-activated-hooks/create-live-additional-software-config
supposedly creates this file with correct ownership, but the first commit in this MR essentially reverted pythonlib@b4669cdc, which apparently is still necessary as long as the config file is not owned by root. The fix for this should amend ccba8c5f.
-
Unlocking a newly created Persistent Storage fails after reboot (both in the Welcome Screen and in the amnesia GNOME session) - How to reproduce: start Tails, create Persistent Storage with all features enabled except dotfiles, reboot, type passphrase, click Unlock
-
Fix Welcome Screen persistent settings -
Fix our custom desktop icons: they lack their custom icon and name -
Encode plan wrt. what to do with the Language and Region feature -
Update obsolete references to the tails-persistence-setup
user (or revert the incomplete renaming)-
config/chroot_local-includes/usr/lib/python3/dist-packages/tailslib/__init__.py
-
-
Remove obsolete tails-synchronize-data-to-new-persistent-volume
code -
Run the behave tests in CI - Broken in GitLab CI: these tests require privileged Docker containers to be set up in the GitLab Runner (namespaces help with mounting as unprivileged user, but they don't help with creating/mounting loop devices) so it's broken.
- Proposed short-term solution: #17803 (comment 186462)
-
Fix the behave tests when run inside Tails - Not only this will allow us to run these tests in our automated test suite, but perhaps more importantly, it will give us confidence that our code works as intended in the context of Tails, which is where is matters.
-
Run the config file unit tests in CI - Broken on GitLab CI:
-
This job tries to connect to a udisks daemon (which is not started). Do they really need udisks? Could we mock this somehow? -
This job depends on the tails-persistent-storage
user, that does not exist in the test container.
-
- Broken when run inside Tails as part of our automated test suite:
chattr +S
does not work on tmpfs, which is what we mount on/tmp
in Tails; neither does it work on overlayfs.
- Broken on GitLab CI:
-
Enable Additional Software by default - We released this change in !800 (merged), the Python port must follow suit.
-
Fix behave tests: they are outdated wrt. the code -
ImportError: cannot import name 'MOUNTFLAG_NOSYMFOLLOW' from 'tps.mountutil'
-
NameError: name 'MountException' is not defined
-
NameError: name 'SymlinkSourceDirectoryError' is not defined
-
NameError: name 'errno' is not defined
-
-
Sort out config/chroot_local-includes/usr/share/applications/org.boum.tails.PersistentStorage.desktop
vs.config/chroot_local-includes/usr/share/applications/org.boum.tails.PersistentStorage.desktop.in
, so make sure we get translations from Transifex -
Ensure tps-frontend and apps started by it are localized like the Desktop session -
Add again the Persistent Storage settings to the Favorites submenu -
Fix UI processes inheriting environment variables from root - Explains this issue found by sajolida:
-
Fix the shortcut to the Persistent directory (or remove it). Right now it opens a window of the Files browser that has /root
as home.
-
- Idea: Already drop privileges in the tps-frontend-wrapper and only pass the dbus socket fd, somehow use that as a
GIOStream
and callGio.DBusConnection.new
with it.
- Explains this issue found by sajolida:
-
When dropping privileges in tps-frontend, make sure we only keep open file descriptors which we want to be opened as root -
Test accessibility in tps-frontend and apps started by it - As of d4a6b400, screen reader works but screen keyboard does not (see below).
-
Test ibus in tps-frontend and apps started by it - As of d4a6b400, it does not work.
-
Show custom features -
Use Gio.DBusObjectManagerServer.export() on server side -
Use interface-proxy-properties-changed signal of Gio.DBusObjectManagerClient on client side -
Custom features can't be activated again when deactivated (because they are removed from the config file) - IMO, to solve this, we should either not offer actions at all on custom features, or only offer to remove them. This would be consistent with the fact we don't support adding them from the UI, and they cannot be enabled without reboot. -- intrigeri
-
-
Make tps-frontend's features view accessible to screen readers - To test with a real screen reader: https://help.gnome.org/users/orca/stable/index.html.en
-
Section headings (Personal Data, etc.) must be label-for
the corresponding GtkListBox -
Give each GtkSwitch an AtkObject::accessible-name
-
Fix unit tests - The "Destination directory created below /home/amnesia is owned by amnesia" scenario fails
-
Re-enable other GitLab CI jobs -
Bring back the Additional Software configuration button -
Keep the backend service constantly running, to avoid having to deal with problems that stem from it not remaining around - It uses little enough RAM that it would be OK to do so.
- Notes that are obsoleted by the above results, but that are useful to explain why it would be beneficial to keep the service running:
- Implement exit-on-idle
- We don't want the service to run in the background all the time, because that would increase RAM usage unnecessarily. I looked into it but didn't get it to work. My workaround was to make the app call
Quit()
when it exits. That doesn't work for the command-line client though (we don't want it to start and stop the service for each single call). We might still be able to work around that for now, because currently, onlyis-active
command of the command-line client used, which we could implement without using the service.
- We don't want the service to run in the background all the time, because that would increase RAM usage unnecessarily. I looked into it but didn't get it to work. My workaround was to make the app call
- Implement exit-on-idle
-
Clean up Git history - The branch has many "WIP" and "fixup" commits.
-
Remove the "XXX" commits
Public call for testing
We'll do this (exact version numbers to be adjusted depending on exact timing):
- In October, shortly after releasing 5.6:
- Release 5.8~beta1 from this branch; go through our regular release process & QA, so the resulting image can be trusted, but skip building incremental upgrades
- Call for testing:
- Please either create a new Tails USB stick where you'll create a new Persistent Storage, or, better: manually upgrade to 5.8~beta1 and use it with your existing Persistent Storage.
- Make it clear that a manual "upgrade" to 5.6 will be needed
- November: release 5.7, ensure users of 5.8~beta1 are told by our Upgrader to manually "upgrade" to 5.7
Blockers for final release
We have moved the tasks that won't be done as part or !897 (merged) to dedicated issues: https://gitlab.tails.boum.org/groups/tails/-/boards?label_name[]=C%3APersistence&milestone_title=Tails_5.8
-
Fix icons for GnuPG and OpenSSH client features - The Perl implementation did this to make this work:
$theme->append_search_path('/usr/share/pixmaps/cryptui/48x48');
- The Perl implementation did this to make this work:
-
Finish the removal of /var/lib/live/config/tails.persistence
git grep -F tails.persistence
-
Check if conflicting apps message shows localized app names -
Check memory consumption of tps service to decide whether idle-on-exit is a blocker - VIRT=310696, RES=25944, SHR=13404; 12.6M according to
systemctl status tps.service
⇒ it's OK to keep the backend service constantly running
- VIRT=310696, RES=25944, SHR=13404; 12.6M according to
-
Identify new tps functionality which need new integration tests -
Avoid inconsistent state due to bugs - For example, this should not result in a non-recoverable error:
- Have an unlocked persistent storage with activated Persistent Directory
- Open file descriptor inside Persistent Directory:
vi /home/amnesia/Persistent
- Try to delete the Persistent Storage
- Deletion fails
- Try to activate the Persistent Directory again
- See error message "chattr +S returned non-zero exit status 1"
- No way to recover from this
- For example, this should not result in a non-recoverable error:
-
Implement error message when starting from ISO image - Right now it's returning "Oh no! Something has gone wrong."
-
Make writes to the config file atomic - to avoid the potential issue that users unplug the Tails device at the wrong time and end up with a broken config file
- can be solved by writing to a temporary file on the same filesystem and moving it
-
Remove the backup file once we have atomic writes (because it's not needed anymore then)
Non-blockers
Now tracked on #19215 (closed)