Triage the follow-ups from "Rewrite the Persistent Storage settings in Python"
During the review !897 (merged), some minor issues were highlighted. These are the ones that we initially thought didn't deserve an issue by themselves.
In this context, bigger = more expensive.
Harder/more expensive issues
-
Implement design for enabling features from other apps → #19426 -
Split Welcome Screen persistence feature into Administration Password, MAC spoofing, and Unsafe Browser → #18750 -
Avoid inconsistent state causing on-activated/deactivated hooks to run when they should not - intrigeri's proposal: forget it for now
- refresh_features() causes on-activated/deactivated hooks to run when the feature was added/removed from the config file, ignoring whether the bindings are actually mounted or not. This causes failures when the user modifies the config file or manually mounts/unmounts the Persistent Storage partition or any of the bindings.
- Fixed by #19376 (closed)
-
Consistent logo and labels in Applications menu (#18761 (closed)) - intrigeri's proposal: track missing backup icon on a new issue → #19427 (closed)
- Right now "Back Up Persistent Storage" has no icon and "Persistent Storage" has the full backpack image (@sajolida)
-
Config file redesign - intrigeri's proposal: forget it for now
- Separate user-owned config file for custom features and internal config for features which should be activated on boot
-
Optimize test suite performance thanks to #11529 (closed) - Now tracked on #19225 (closed)
-
Allow the user to create Persistent Storage from Tor Connection → #19429 (closed) -
Deleting should check if something is using the Persistent Storage - Quite hard, tried something but reverted the commit
Easier/cheaper issues
-
Check if this branch fixes #16982 (closed); if it does, adjust "Closes" on the MR accordingly -
The tps test suite says we're cleaning up with atexit.register
, but we're not.- intrigeri's proposal: acknowledge the problem in the offending comment
-
Profile performance - intrigeri's proposal: forget it until we learn we have a problem
- fixed by !1008 (merged)
-
Think about attack scenarios targeting the on-activated/on-deactivated hooks, which are executed as root using untrusted input (mount destination directories) - now tracked on tails/tails#19248
-
Think about attack scenarios tricking the user into killing arbitrary processes via the conflicting app dialog - now tracked on tails/tails#19248
-
Maybe deactivate all features before deleting partition - Would call on-deactivate hooks
- Would show conflicting apps messages
-
Test BootDeviceIsSupported on optical drive - What is this about?
- Tried but the ce3003b0f8 and 7466fccb91 isos both don't boot (while the .qemu2 images do boot)
- Tried with an iso based on stable (5840bb43) and BootDeviceIsSupported is false there
-
Use systemd sandboxing features - intrigeri's proposal: forget it here for now
- Not that important, because the privileges the backend needs already allow it to execute arbitrary code as root
-
Add link to documentation in "Failed to activate Feature" dialog when trying to enable dotfiles - segfault: We don't fail anymore when activating empty dotfiles. The dotfiles feature is still impossible to understand without reading the documentation, so we might still want to add a button to open the documentation.
- Done
- segfault: We don't fail anymore when activating empty dotfiles. The dotfiles feature is still impossible to understand without reading the documentation, so we might still want to add a button to open the documentation.
-
Maybe rename executables - intrigeri's proposal: "tps → tpsd" would make it easier to triage bug reports => track on new issue; forget the other proposed changes. → #19430 (closed)
- For example:
- tps-frontend-wrapper -> tails-persistent-storage
- tps -> tpsd
- Consider having a more consistent naming scheme for all our apps, like GNOME does
- now tracked on #19379
-
Encode all state in attributes, so that the client can connect/disconnect at any point without having inconsistent state - intrigeri's proposal: forget it for now
- I don't remember which state is not encoded in attributes -> Check that and then decide what to do about it
-
Handle expected errors in Welcome Screen - What happens currently?
- segfault: We currently handle
IncorrectPassphraseError
but notInvalidConfigFileError
. When an unhandled error occurs while activating the Persistent Storage, we display a notification ("Failed to unlock the Persistent Storage. Please start Tails and send an error report.") and disable the unlock button. - The config file check is broken, see #19407
- segfault: We currently handle
- For example
InvalidConfigFileError
andIncorrectPassphraseError
- What happens currently?
-
Changing passphrase has no effect? Unlocking still worked with the old passphrase. - Can't reproduce
-
Implement reloading on SIGHUP (not really needed currently but nice) - intrigeri's proposal: forget it until it's the best way to solve an actual problem
-
Running tps-frontend-wrapper
twice results in "Oh no! Something has gone wrong."- boyska's proposal: only relevant if there is a practical way for a user to actually incur in that. If the only way to reproduce this bug involves the terminal, it's not a bug.
- segfault: Not reproducible, marking as done
-
Fix ibus in tps-frontend and apps started by it -
Check if it's easy to fix by passing the right environment variables - It's not fixed by adding
env_keep+=AT_SPI_BUS_ADDRESS
andenv_keep+="IBUS_ADDRESS"
toconfig/chroot_local-includes/ etc/sudoers.d/zzz_tps-frontend
- It's not fixed by adding
-
Check if it's a regression - It's not
- segfault: This was a hard one: ibus is actually working in tps-frontend but the only entry fields we have are entries with
input-purpose=password
, which causes GTK to disable ibus for those entries. Not sure if want to change that (we could change theinput-purpose
property).
-
-
Make sure that the Persistent Storage settings open on top of other applications. According to @groente, the old settings don't appear on top, but rather below already running applications. -
Remove Welcome Screen persistent settings hack (#19062 - closed) -
Migrate create-tor-browser-directories
tosystemd-tmpfiles
- intrigeri's proposal: just do it (cheaper than debatting)
- Why: Avoid running this every time logind creates a session for the amnesia user. We can now do it because that script now always does the exact same thing.
-
Migrate from pwscore
topython3-pwquality
- Are there other incentives than avoiding to fork an external command? Does it give better results?
- See also #7002 (closed). I understand that they use the same algorithm. Note that python3-zxcvbn is also in Bullseye. After watching https://media.libreplanet.org/u/libreplanet/m/an-information-theoretic-model-of-privacy-and-security-metrics/ and adding a diceware suggestion in #18148 (closed), I wonder whether we should keep the password meter. See you on #18148 (closed).
- boyska's proposal: forget about it. Can be bundled if we want to improve that part anyway, but no more than that.
-
Catch IncorrectPassphraseError in frontend - Don't remember when/why we added this issue, currently both in the "Change passphrase" dialog and the unlocking view the error is already caught
-
This branch is supposed to fix #7503 (closed), which includes making the visual keyboard works. However, as of 9fce62dd, the screen keyboard does not work, at least to change the passphrase.
-
@boyska started a discussion: (+1 comment) This is just to track this XXX with Gitlab threads. The "sandboxing" word is quite vague in this context, at least to me:
tps
is inherently very privileged; if we have ideas on which kind of sandboxing we could do, we'd better write them down. -
@boyska started a discussion: why should it ever be the case? should a subclass be allowed to set
self.signals = None
? I see no code doing that, so I suppose this is dead code. -
@boyska started a discussion: is this method ever called?
Issues that happen when activating a feature
Most such issues will be solved if we always delete persistent data when deactivating a given persistent storage feature (likely we'll want to let the user know though) i.e. Delete corresponding data when a feature of the... (#8447 - closed) ⇒ not a blocker.
-
If I activate the Tor Bridge feature after connecting to Tor, the bridge configuration previously stored on Persistent Storage is not immediately used, and the amnesic configuration that's used and displayed in the UI does not reflect what's stored on Persistent Storage. - If we always delete the persistent data of a feature when turning it off in #8447 (closed), there would be no previous bridge configuration to be inconsistent with, right?
- boyska's opinion: I think this changes the meaning of "turning it off" to "delete it", which would be quite surprising for me as a user. If UX thinks this is acceptable behavior, for sure it would make reasoning about this a lot easier.
- segfault: IIUC, this can only happen in the following scenario:
- The user configures a Tor Bridge
- The user enables the Tor Bridge feature in the Persistent Storage
- The user disables the Tor Bridge feature in the Persistent Storage
- The user reboots
- The user uses TCA to connect to Tor
- The user enables the Tor Bridge feature, restoring the old data
- If the user would switch steps 5 and 6, i.e. enable the Tor Bridge feature again before using TCA to connect to Tor, the bridges would be used, right? If that is correct, IMO it's not a reasonable expectation that the Tor connection of the current session use the bridges which were activated after the Tor connection was set up -> Not an important issue.
- If we always delete the persistent data of a feature when turning it off in #8447 (closed), there would be no previous bridge configuration to be inconsistent with, right?
-
After activating the Network Connections feature, connections that were already stored on Persistent Storage are still not available to the user. -
After activating the Additional Software feature, packages previously configured in Persistent Storage are not immediately installed → #19428 - Same again: if we always delete the persistent data of a feature when turning it off in #8447 (closed), there would be no additional software to install again, right?
- segfault: It doesn't sound hard to implement this. If this is the only remaining issue which we would solve by always deleting persistent data when the feature is deactivated, we should consider implementing this instead.
- segfault: Actually, I don't think it makes sense to allow activating/deactivating the Additional Software feature in the Persistent Storage app at all. It should be managed via the Additional Software app instead.
-
Activating/deactivating Dotfiles, or a custom Persistent Storage feature, may not be reflected in effective configuration, e.g. if the app whose configuration/data is stored there is currently running. - Shall we check if any process is currently using files in that directory? This would be racy, but better than nothing. It is not enough in all cases, an app may very well keep no file open in its directory, while still having a different runtime state in memory, that will be saved when closing the app. We don't even know the percentage of applications for which this implementation proposal makes sense, or if there's a single one.
- Will happen only occasionally and for power users ⇒ not a blocker.
- Not a blocker and #8447 (closed) will make it more consistent on activation at least. I think that we can ignore the corner case of not updating an apps configuration when deactivating the feature while it's still running.