#1 Updated by segfault 2019-10-08 19:57:26

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

We also discussed persisting the admin password and the screenlocker password (Bug #15641, Feature #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 (Feature #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 2019-10-08 20:14:53

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.

#10 Updated by segfault 2020-02-14 00:05:56

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.

#19 Updated by intrigeri 2020-03-21 09:03:23

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 Bug #17526 about it.

#20 Updated by sajolida 2020-03-21 19:04:20

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 tails-amd64-feature_6560-secure-boot+force-all-tests-4.5-20200318T2121Z-3d11b413d6+devel@5484d70626.img twice just to be sure.

Security discussion

I read Bug #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 Feature #16998. On Feature #16998#note-3, intrigeri raised a question about cost which is mostly irrelevant now that we have a branch.

@intrigeri: Which bits of Bug #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 Bug #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 Bug #17136 later on. I still think that solving Bug #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?

#24 Updated by intrigeri 2020-03-21 19:38:36

Hi,

sajolida wrote:
> h1. Security discussion

> intrigeri: Which bits of Bug #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 Bug #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 Bug #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 Bug #17136 later on. I still think that solving Bug #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 2020-03-22 11:40:38

@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.

#27 Updated by sajolida 2020-03-24 18:21:06

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!