Project

General

Profile

Bug #17136

Persistence preset: Greeter settings

Added by segfault 6 months ago. Updated 15 days ago.

Status:
In Progress
Priority:
Normal
Assignee:
Category:
-
Target version:
Start date:
Due date:
% Done:

0%

Feature Branch:
feature/17136-persist-greeter-settings;tps:feature/17136-persist-greeter-settings
Type of work:
Code
Blueprint:
Starter:
Affected tool:
Greeter

Description

(sajolida is watching this)

Settings were loaded from persistent storage.png View (48.4 KB) segfault, 03/07/2020 06:16 PM

Start Tails button insensitive after typing a password but before unlocking.png View (43.9 KB) segfault, 03/07/2020 06:16 PM

Tails Greeter Settings preset in tails-persistence-setup.png View (92.5 KB) segfault, 03/07/2020 06:16 PM

Disable Admin Password.png View (53.6 KB) segfault, 03/12/2020 07:50 AM


Subtasks

Feature #16998: Persistence feature: administration passwordIn Progress


Related issues

Related to Tails - Feature #5501: Persistence preset: locale and accessibility options (language, keyboard, and formats) in Greeter Confirmed 12/31/2015
Related to Tails - Bug #15641: Persistence preset: screen locking password Rejected 06/08/2018
Blocked by Tails - Bug #17526: Move tails-persistence-setup to tails.git Resolved
Blocked by Tails - Feature #15122: Rename Tails Greeter to be more plain Resolved 12/27/2017

Associated revisions

Revision bc2ff803 (diff)
Added by segfault 3 months ago

Greeter: Move settings files to /var/lib/gdm3/settings (refs: #17136)

To make it easier to persist all of these settings.

Revision e2f40e91 (diff)
Added by segfault 3 months ago

Greeter: Don't unlock persistence via the "Start Tails" button (refs: #17136)

We require to unlock the persistence via the "Unlock" button, to ensure
that the user gets some UI feedback when the persistent Greeter settings
are loaded.

The "Start Tails" is now insensitive if a password was entered in the
persistence password entry but persistence was not unlocked.

Revision de010165 (diff)
Added by segfault 3 months ago

Greeter: Split locale setting file into multiple files (refs: #17136)

This is a preparation for loading the settings in the Greeter when
persistence is unlocked.

Revision 44870660 (diff)
Added by segfault 3 months ago

Greeter: Support loading settings (refs: #17136)

Revision 91b8d1d9 (diff)
Added by segfault 2 months ago

Greeter: Support loading settings from persistence (refs: #17136)

Revision 198f66f6 (diff)
Added by segfault 2 months ago

Greeter: Only remove an existing password file if the user disabled it (refs: #17136)

This allows to reuse a persistent hashed password file if the user does
not disable or modify the password setting.

Revision 472c66a0 (diff)
Added by segfault 2 months ago

Greeter: Only remove an existing password file if the user disabled it (refs: #17136)

This allows to reuse a persistent hashed password file if the user does
not disable or modify the password setting.

Revision d0e2f5b3 (diff)
Added by segfault 2 months ago

Greeter: Support loading settings from persistence (refs: #17136)

Revision b30e3402 (diff)
Added by segfault 2 months ago

Greeter: Move settings files to /var/lib/gdm3/settings (refs: #17136)

To make it easier to persist all of these settings.

Revision cda54958 (diff)
Added by segfault 2 months ago

Greeter: Don't unlock persistence via the "Start Tails" button (refs: #17136)

We require to unlock the persistence via the "Unlock" button, to ensure
that the user gets some UI feedback when the persistent Greeter settings
are loaded.

The "Start Tails" is now insensitive if a password was entered in the
persistence password entry but persistence was not unlocked.

Revision 7910d9b2 (diff)
Added by segfault 2 months ago

Greeter: Split locale setting file into multiple files (refs: #17136)

This is a preparation for loading the settings in the Greeter when
persistence is unlocked.

Revision bb33aac9 (diff)
Added by segfault 2 months ago

Greeter: Support loading settings from persistence (refs: #17136)

Revision 191467ca (diff)
Added by segfault about 2 months ago

Design doc: Update path to Greeter settings (refs: #17136)

The settings are now split into different files.

Revision 2fe621b7 (diff)
Added by segfault about 2 months ago

Handle type conversion and quoting in write_settings() (refs: #17136)

Also, use shlex.quote() instead of the deprecated pipes.quote(). See
https://docs.python.org/2/library/pipes.html#pipes.quote

Revision 7fb66f27 (diff)
Added by segfault about 2 months ago

Use exceptions instead of False and None to indicate failure (refs: #17136)

Also fixes a bug which caused the "Settings loaded" infobar to not be
shown if only region settings were loaded (the LocalizationSettingUI.load()
implementation did not return True if a setting was loaded).

Revision 0c1657be (diff)
Added by segfault about 2 months ago

Greeter: Move settings files to /var/lib/gdm3/settings (refs: #17136)

To make it easier to persist all of these settings.

Revision d3df09fe (diff)
Added by segfault about 2 months ago

Greeter: Don't unlock persistence via the "Start Tails" button (refs: #17136)

We require to unlock the persistence via the "Unlock" button, to ensure
that the user gets some UI feedback when the persistent Greeter settings
are loaded.

The "Start Tails" is now insensitive if a password was entered in the
persistence password entry but persistence was not unlocked.

Revision 9dee66cd (diff)
Added by segfault about 2 months ago

Greeter: Split locale setting file into multiple files (refs: #17136)

This is a preparation for loading the settings in the Greeter when
persistence is unlocked.

Revision 547b0010 (diff)
Added by segfault about 2 months ago

Greeter: Support loading settings from persistence (refs: #17136)

Revision bf2a2c1c (diff)
Added by segfault about 2 months ago

Design doc: Update path to Greeter settings (refs: #17136)

The settings are now split into different files.

Revision 210d8967 (diff)
Added by segfault about 2 months ago

Handle type conversion and quoting in write_settings() (refs: #17136)

Also, use shlex.quote() instead of the deprecated pipes.quote(). See
https://docs.python.org/2/library/pipes.html#pipes.quote

Revision dcc34c4c (diff)
Added by segfault about 2 months ago

Use exceptions instead of False and None to indicate failure (refs: #17136)

Also fixes a bug which caused the "Settings loaded" infobar to not be
shown if only region settings were loaded (the LocalizationSettingUI.load()
implementation did not return True if a setting was loaded).

Revision d325ba8f (diff)
Added by segfault about 2 months ago

Handle type conversion and quoting in write_settings() (refs: #17136)

Also, use shlex.quote() instead of the deprecated pipes.quote(). See
https://docs.python.org/2/library/pipes.html#pipes.quote

Revision b6164b0a (diff)
Added by segfault about 2 months ago

Use exceptions instead of False and None to indicate failure (refs: #17136)

Also fixes a bug which caused the "Settings loaded" infobar to not be
shown if only region settings were loaded (the LocalizationSettingUI.load()
implementation did not return True if a setting was loaded).

Revision c0c723df (diff)
Added by segfault about 2 months ago

Handle type conversion and quoting in write_settings() (refs: #17136)

Also, use shlex.quote() instead of the deprecated pipes.quote(). See
https://docs.python.org/2/library/pipes.html#pipes.quote

Revision 49e90498 (diff)
Added by segfault about 2 months ago

Use exceptions instead of False and None to indicate failure (refs: #17136)

Also fixes a bug which caused the "Settings loaded" infobar to not be
shown if only region settings were loaded (the LocalizationSettingUI.load()
implementation did not return True if a setting was loaded).

Revision 3bb8f74d (diff)
Added by segfault about 2 months ago

Re-add the boolean return value to load() (refs: #17136)

Turns out there was actually a good reason to have those, because we
only want to add additional settings if the loaded value is not the
default value and if they haven't been added before.

Revision d77adcce (diff)
Added by segfault 10 days ago

Add preset for Greeter settings (refs: #17136)

History

#1 Updated by segfault 6 months ago

We wanted to persist the region, keyboard and formats settings since more than 6 years (#5501).

We also discussed persisting the admin password and the screenlocker password (#15641, #16998).

I propose that we simply add a persistence preset which persists all the Greeter settings.

The workflow for the user would then look like this:
  • Boot and wait for the Greeter
  • If they require a non-US keyboard layout to enter the password, configure that
  • Enter the persistence password and unlock. Now the Greeter loads the persistent settings and updates its window accordingly.
  • Change settings if required. These changes will be persisted.
  • Start Tails
I think this would increase UX a lot, especially for users who configure the same settings for each boot, which I expect is the case quite often:
  • For language, keyboard layout and region, it's clear that they are usually the same
  • Regarding the network connection: For users who require a bridge to connect to Tor, it could actually be dangerous if they ever forget to configure that bridge. So for those users, being able to persist that setting would also increase security, not only UX.
  • MAC address spoofing and admin password are settings which might only be set on occasion.
  • In case that we allow enabling/disabling the Unsafe Browser in the Greeter (#17085), this would also be a setting that would be set to the same value during most boots.

Note that the Greeter already provides a good overview of the configured settings and a good UX to change those.

Implementation would be simple:
We already store the Greeter settings in files, in /var/lib/gdm3/. We could move those to /var/lib/gdm3/settings/ and add a persistence preset for that directory, i.e. /live/persistence/TailsData_unlocked/greeter_settings/ would be bind-mounted to /var/lib/gdm3/settings/ when persistence is unlocked.
Then the only thing we would still have to implement is reading the values from the settings files and updating the Greeter UI accordingly, which shouldn't be too hard (I know the Greeter code quite well now).

#2 Updated by segfault 6 months ago

We should disable the automatic unlocking of the persistence via the "Start Tails" button, to prevent that user-configured settings get overwritten with persistent settings without any UI feedback.

I think the GNOME way to do this is to simply make the "Start Tails" button insensitive if a password was entered in the persistence entry but the persistence was not unlocked.

#3 Updated by sajolida 5 months ago

  • Related to Feature #5501: Persistence preset: locale and accessibility options (language, keyboard, and formats) in Greeter added

#4 Updated by sajolida 5 months ago

  • Related to Bug #15641: Persistence preset: screen locking password added

#5 Updated by sajolida 5 months ago

  • Related to Feature #16998: Persistence feature: administration password added

#6 Updated by segfault 3 months ago

  • Status changed from New to In Progress

#7 Updated by segfault 2 months ago

  • Status changed from In Progress to Needs Validation
  • Target version set to Tails_4.3
  • Feature Branch set to feature/17136-persist-greeter-settings;tps:feature/17136-persist-greeter-settings

I implemented something. The workflow is now as described above. Was a bit more work than I expected, because I had to (or wanted to) refactor the Greeter even more.

I'm not sure if that qualifies for a bugfix release. Feel free to change the target version to 4.5 if you think that it does not.

#8 Updated by hefee 2 months ago

  • Assignee set to hefee

#9 Updated by hefee 2 months ago

  • Status changed from Needs Validation to In Progress
  • Assignee changed from hefee to segfault
  • Target version changed from Tails_4.3 to Tails_4.5
  • Affected tool set to Greeter
TODO:
  • /var/lib/gdm3/settings/tails.persistence is only referenced in wiki/src/blueprint/TailsGreeter/design.mdwn vs /var/lib/live/config/tails.persistence that is referenced in source config/chroot_local-includes/usr/lib/python3/dist-packages/tailsgreeter/config.py
  • Is is possible to read login.defs to be able to set --method=sha512crypt in tailsgreeter/settings/admin.py?
  • If we persist the admin password, we may end up, that Debian may switch the hash function (update the login.defs file). That's why we should also save the used hash method to the file, than we are able to detect, that passwords are not compatible for the current hash method (no need to implement the detection now, as it does not seem that SHA512 will be broken soon, but who knows).
  • what about read and rewrite password file, can this happen? Because than the password hashed two times, as load will return only the hashed password.
  • 'TAILS_MACSPOOF_ENABLED': pipes.quote(str(value)).lower(), in tailsgreeter/settings/macspoof.py is not consistent with other files. str(value).lower() is more consistent. (see also next suggestion)
Suggestion:
  • move more logic into write_settings, to handle boolean and strings, and quote the strings inside that function, than you don't need to remember to quote the strings all over the place and have one implementation how you write the boolean values.
Nitpicking:
  • Don't use False and/or None to indicate that nothing is read, use Exceptions instead, that is more Python style.

move to 4.5 target, as this is can't be applied to a bugfix release.

#10 Updated by segfault about 2 months ago

Thanks a lot for the review!

hefee wrote:

TODO:
  • /var/lib/gdm3/settings/tails.persistence is only referenced in wiki/src/blueprint/TailsGreeter/design.mdwn vs /var/lib/live/config/tails.persistence that is referenced in source config/chroot_local-includes/usr/lib/python3/dist-packages/tailsgreeter/config.py

The reference to /var/lib/gdm3/settings/tails.persistence was indeed outdated. Fixed that now.

  • Is is possible to read login.defs to be able to set --method=sha512crypt in tailsgreeter/settings/admin.py?

According to its man page, login.defs does not configure the hash algorithm of user passwords, because that's handled by PAM:

Note: This only affect the generation of group passwords. The generation 
of user passwords is done by PAM and subject to the PAM configuration. It
is recommended to set this variable consistently with the PAM configuration.

We could attempt to parse the PAM config files in /etc/pam.d/ and choose the algorithm accordingly, but I suspect that that would be error prone. Instead, I added a build hook which checks that PAM is still configured to use sha512 and aborts the build if it's not (so that we notice and can adjust the code). What do you think about that?

  • If we persist the admin password, we may end up, that Debian may switch the hash function (update the login.defs file). That's why we should also save the used hash method to the file, than we are able to detect, that passwords are not compatible for the current hash method (no need to implement the detection now, as it does not seem that SHA512 will be broken soon, but who knows).

Agreed and done.

  • what about read and rewrite password file, can this happen? Because than the password hashed two times, as load will return only the hashed password.

I don't see any case where a loaded password does get written again. But indeed, the current code makes this case unnecessarily likely to happen. We shouldn't have to actually set the UI setting's password value to the loaded value. I changed this now.

  • 'TAILS_MACSPOOF_ENABLED': pipes.quote(str(value)).lower(), in tailsgreeter/settings/macspoof.py is not consistent with other files. str(value).lower() is more consistent. (see also next suggestion)

I think I kept the pipes.quote part from the old code. And it's used consistently in macspoof.py, network.py, and admin.py.

Suggestion:
  • move more logic into write_settings, to handle boolean and strings, and quote the strings inside that function, than you don't need to remember to quote the strings all over the place and have one implementation how you write the boolean values.

Done.

Nitpicking:
  • Don't use False and/or None to indicate that nothing is read, use Exceptions instead, that is more Python style.

Fine, I changed this. Note that in LocalizationSettingUI.load() and the implementations of AdditionalSetting.load() I'm now "handling" the exceptions by simply re-raising them. I'm aware that this is equivalent to not handling them at all, but I prefer doing this explicitly, because it makes it more obvious to the reader what is going on.

I'm keeping the ticket assigned to me because I didn't test the above changes yet.

#11 Updated by segfault about 1 month ago

  • Status changed from In Progress to Needs Validation
  • Assignee deleted (segfault)

I found and fixed a few more bugs. The feature seems to work now, at least according to my manual tests. It would be nice to have automatic tests for this. I would look into that, but before I spend even more time on this feature (which I already spent way more time on that I expected), I would like to get some input from @sajolida, because I'm getting worried that I didn't get the opinion on my UX decisions from our UX team lead yet.

#13 Updated by intrigeri about 1 month ago

  • Assignee set to sajolida

Putting this explicitly on sajolida's plate, as for the same reasons mentioned by segfault, I don't think we should spend time on the code review before we know there's a dev/UX agreement.

#14 Updated by sajolida 28 days ago

  • Related to Feature #15122: Rename Tails Greeter to be more plain added

#15 Updated by sajolida 28 days ago

  • Assignee changed from sajolida to intrigeri

@segfault: I'm glad that you're persisting (sic) on your quest to "Persist All The Things" :)

I haven't tested it yet but, only from the screenshots, it looks great!

I'm wondering how it relates to:

  • #16998: Persistence feature: administration password

Does it mean that, with this feature enabled, the administration password would be persistent, like Mac Spoofing or Network Configuration? I'm not against it but it goes against our advice of only setting up an administration password when needed. Having an administration password set all the time might be a security risk. But it's a UX improvement and I'll benefit myself from it every day, so I won't be the one arguing against it :)

  • #15641: Persistence preset: screen locking password

If we persist the Administration Password, we will de facto allow persisting the screen locking password but, same as before, it might encourage people to set an administration password when they don't really need it. Again, I'm doing this myself every day so I won't blame people for doing it themselves.

  • #15122: Rename Tails Greeter to be more plain

I think it would be the first time that we display the word "Tails Greeter" in an interface. So far, I think that it was only written in the doc. I'll try to do this change in the doc in time for 4.5, so that we don't have to modify these strings after they are released.

Reassigning to intrigeri for code review and opinion on persisting the Administration Password.

#16 Updated by segfault 27 days ago

sajolida wrote:

@segfault: I'm glad that you're persisting (sic) on your quest to "Persist All The Things" :)

:)

I haven't tested it yet but, only from the screenshots, it looks great!

Thanks, that's a relief!

I'm wondering how it relates to:

  • #16998: Persistence feature: administration password

Does it mean that, with this feature enabled, the administration password would be persistent, like Mac Spoofing or Network Configuration? I'm not against it but it goes against our advice of only setting up an administration password when needed. Having an administration password set all the time might be a security risk. But it's a UX improvement and I'll benefit myself from it every day, so I won't be the one arguing against it :)

Yes, if the preset is enabled, the password setting will be stored (hashed and salted) and automatically loaded just like the other Greeter settings. If the user doesn't need the admin password anymore, they can disable it by clicking on the list item after it has been loaded (see screenshot). This change will be persisted too, i.e. the password will be removed from the persistence.

If that's not good enough, we could also add a checkbox to the additional settings dialog to "Only enable once", if persistence is unlocked and the preset is enabled. But if we did that, I'm not sure how we should handle the case that the user first sets an admin password and then unlocks persistence.

  • #15641: Persistence preset: screen locking password

If we persist the Administration Password, we will de facto allow persisting the screen locking password but, same as before, it might encourage people to set an administration password when they don't really need it. Again, I'm doing this myself every day so I won't blame people for doing it themselves.

We could also rename the setting to just "Password", and make it optional to enable administration access.

  • #15122: Rename Tails Greeter to be more plain

I think it would be the first time that we display the word "Tails Greeter" in an interface. So far, I think that it was only written in the doc. I'll try to do this change in the doc in time for 4.5, so that we don't have to modify these strings after they are released.

I see you proposed there to rename the Greeter to "Welcome Screen". I'm not sure about calling the persistence preset "Greeter Settings" anyway, but renaming it to "Welcome Screen Settings" feels even worse, because we're not actually changing settings of the Greeter/Welcome Screen, but settings of the desktop environment and the rest of the system, which happen to be configurable in the Greeter/Welcome Screen. Maybe "Tails Login Settings" would be better.

#17 Updated by intrigeri 24 days ago

  • Blocked by Bug #17526: Move tails-persistence-setup to tails.git added

#18 Updated by intrigeri 24 days ago

  • Status changed from Needs Validation to In Progress
  • Assignee changed from intrigeri to segfault

Hi,

segfault wrote:

sajolida wrote:

segfault: I'm glad that you're persisting (sic) on your quest to "Persist All The Things" :)

:)

+1 :)

Regarding persisting the administration password, we had a long discussion on this topic on #15641.
I'm keeping in mind that any of us may have changed their mind since then :)
Still, it seems to me we're lacking a little bit of facilitation work here: sum up the areas where we reached agreement, and list the remaining points of friction with the corresponding arguments. If nobody does this work, then either we'll restart this thinking + discussion from scratch (possibly missing important bits), or everybody will basically have to do that work on their own. Both seem wasteful to me.
segfault, would you like to do this reading + summarizing?

Regarding code review:

  • hefee did an initial review in February. Maybe ask them if they would like to follow up on this? I'm fine with being the fallback reviewer if needed.
  • The branch has stray commits that I believe come from a buggy rebase, e.g. fa64ff39752d75915443baf86923ead3be6297e5 which duplicates d4722d41f144283b7b67cfd97ba9c01133227e65.
  • I've reviewed the t-p-s branch at 544e35ade0f5f509688fa94e054386584a052848 and it LGTM.

Regarding automated tests:

  • I've updated the t-p-s branch to make its own "upstream" test suite pass.
  • I agree with segfault: it would be nice to have automated tests for this new feature.
    • I can't volunteer to write those tests myself in time for the 4.5 freeze. I have to focus on sponsor deliverables.
    • I suggest asking anonym if he can do that (with lower priority than reviewing #8415 and friends, because here as well, we should focus on sponsor deliverables).
    • I suggest whoever writes the tests first focuses on non-password things, since there might be further changes in that area.

Finally, I realized that moving tails-perl5lib to tails.git broke the release process of t-p-s. Ouch! Not sure how I can have missed that :/
I've filed #17526 about it.

#19 Updated by intrigeri 18 days ago

intrigeri wrote:

Finally, I realized that moving tails-perl5lib to tails.git broke the release process of t-p-s. Ouch! Not sure how I can have missed that :/
I've filed #17526 about it.

#17526 is now ready for a review (but not a merge yet). Regarding the impact on this very ticket, the good news is that we now have a way to ship changes to t-p-s; but the bad news is that we'll have to transplant the work done in persistence-setup.git to tails.git. Last time I had to do something like this, IIRC I exported the relevant commits with git format-patch, then adjusted the file paths in the resulting Git patches, then applied them in tails.git with git am.

Sorry again for the inconvenience!

#20 Updated by sajolida 18 days ago

Target version: 4.6

Disclaimer: I tried to test this as per 283adb96ff but I can't see the new Persistent Storage feature in the config.

I burnt twice just to be sure.

Security discussion

I read #17136 very quickly again. I didn't do it in depth enough to understand it fully again because it was a very complex and painful discussion that's mostly irrelevant for the matter at hand here.

It was focused on discussing the interface and interactions to allow persisting the screening locking password (1/2 passwords?, before of after unlocking the Persistent Storage?, automatic screen locking? etc.)

I didn't find it in any security discussion on whether we are fine with making it easier for people to always have an Administration Password (and not only sometimes, as currently recommended).

That's why I created #16998. On #16998#note-3, intrigeri raised a question about cost which is mostly irrelevant now that we have a branch.

@intrigeri: Which bits of #15641 made you think that we already had a long discussion about this?

I'm myself fine with making it easier for people to always have an Administration Password and segfault seems to be fine with this as well.
@hefee didn't raise any concern about it either during their 1st review.

So yeah, @intrigeri + @hefee, if you have security concerns with making it easier for people to always have an Administration Password vs. only sometimes, please let us know.

My experience is that, it's so painful to realize once you are already in GNOME that you need an Administration Password and restart, that I got into the habit of always setting one in Tails Greeter, just in case, even if I end up not needing it in my session.

Of course, I'm not a representative user but people who use Tails a lot and only sometimes need an Administration Password might do the same because it's very hard to know in advance whether you will need it or not. For me, this convenience greatly compensates the security cost of having one set up in cases when I don't need it.

Setting up one all the time also provides me with automatic screen locking which is both a convenience and security gain.

Last, segfault's design still allows people to disable it when they don't need it. This is also persistent: if you disable the Administration Password once, it won't be added back automatically the next time. That's also the password reset feature :)

UX

  • Regarding the name of the Persistent Storage feature. I think that the problem lie in our proposals being noun strings [1] and not on how we call this screen. We should make it clear that we're not talking about how this screen is configured but about the settings that are made available through this screen. Unfolding the noun string could help. The description of the feature as well.

I propose:

Name: Settings on the Welcome Screen
Description: Language, administration password, and additional settings

[1]: https://www.plainlanguage.gov/guidelines/words/avoid-noun-strings/

  • Regarding persisting the language and region settings. I'm glad that this branch would make it easier in some cases. Still, I don't think that it will solve all or most cases. For example, if I configure my Persistent Storage with a passphrase on a French keyboard, I won't be able to unlock it with the default English keyboard. I need to set the keyboard first and then unlock the Persistent Storage. That's why on #17136 we are proposing to store these settings in cleartext.

But I'm fine with moving keeping this debate aside, more on with this branch, and go back to the drawing board for #17136 later on. I still think that solving #17136 properly (ie. in cleartext) is far more important than persisting the additional settings, but now that we have some code here it's too late to reprioritize and it's probably better to finish this.

To be explicit, it don't think that we can target 4.5 for this work since it seems like it lacks automated tests, maybe another code review, the end of the security discussion, and a proper UX review once I get it to work.

Could this be squeezed into Tails 4.6?

#21 Updated by intrigeri 18 days ago

  • Target version changed from Tails_4.5 to Tails_4.6

#22 Updated by sajolida 18 days ago

  • Related to deleted (Feature #15122: Rename Tails Greeter to be more plain)

#23 Updated by sajolida 18 days ago

  • Blocked by Feature #15122: Rename Tails Greeter to be more plain added

#24 Updated by intrigeri 18 days ago

Hi,

sajolida wrote:

Security discussion

intrigeri: Which bits of #15641 made you think that we already had a long discussion about this?

When I have to do "page down" 22 times to scroll through a Redmine issue, I call it a long discussion. To answer your "which bits?" question, I would have to read all that discussion (in which I did not participate), which is precisely what I wanted to avoid.

I'm myself fine with making it easier for people to always have an Administration Password and segfault seems to be fine with this as well.
hefee didn't raise any concern about it either during their 1st review.

Great :)

So yeah, @intrigeri + @hefee, if you have security concerns with making it easier for people to always have an Administration Password vs. only sometimes, please let us know.

Initially I missed, or did not understand, that you were asking my opinion specifically regarding security. Now that I understood this:

  • I have no security concerns off the top of my head.
  • I'm not passionate enough about this topic to go through #15641 myself, which I feel I should do to give my opinion here.
  • I'm satisfied with the fact sajolida and segfault agree with each other.

So please don't block on me :)

  • Regarding persisting the language and region settings. I'm glad that this branch would make it easier in some cases. Still, I don't think that it will solve all or most cases. For example, if I configure my Persistent Storage with a passphrase on a French keyboard, I won't be able to unlock it with the default English keyboard. I need to set the keyboard first and then unlock the Persistent Storage. That's why on #17136 we are proposing to store these settings in cleartext.

But I'm fine with moving keeping this debate aside, more on with this branch, and go back to the drawing board for #17136 later on. I still think that solving #17136 properly (ie. in cleartext) is far more important than persisting the additional settings, but now that we have some code here it's too late to reprioritize and it's probably better to finish this.

Agreed on all counts.

To be explicit, it don't think that we can target 4.5 for this work since it seems like it lacks automated tests, maybe another code review, the end of the security discussion, and a proper UX review once I get it to work.

+ the freeze for 4.5 is supposed to happen very soon now.

Could this be squeezed into Tails 4.6?

No objection from my part in principle as long as this new feature gets test coverage.
We have good test coverage for the current state of Greeter + Persistence, so the risk of regressions vs. baseline (== when this new feature is disabled) is low.

#25 Updated by hefee 17 days ago

@sajolida wrote:

So yeah, intrigeri + hefee, if you have security concerns with making it easier for people to always have an Administration Password vs. only sometimes, please let us know.

I don't have any security concerns aka nothing new that is not said already.

#26 Updated by sajolida 16 days ago

  • Description updated (diff)

#27 Updated by sajolida 15 days ago

Ok, security concerns solved then. That was easy :)

I'll do a UX review as soon as I get a working image. Let's see if we can make it to 4.6!

Also available in: Atom PDF