Hi,
Alan wrote (23 Nov 2013 12:55:21 GMT) :
> I've started review.
Thank you.
> One test fails (see below) and that's a blocker. I believe there is also
> room for other UX improvements (see comments) but would merge it without
> them.
In what follows, I'm going to be more willing to spend time to improve
UX in 1. supported usecases (as in: the user has not done the contrary
to what the doc says) and 2. non-temporary parts (that is: stuff that
still may be experienced by actual users in 3-6 months). The rest will
be dealt with on a "good enough" basis.
> I tested:
> 1. enabling persistence from 0.20.1 with a custom line. After an
> upgrade to experimental:
> - I get a notification that settings were disabled
> - there are no mounts
> - but there is still a live-presistence.conf owned by
> amnesia:amnesia which is not remamed to .disabled.
> I guess it's expected (as the mountpoint access rights are wrong)
Right, it's expected that persistence is not activated.
It's also expected that live-persistence.conf is not renamed, because
the new permissions checking code only looks for the new-style
persistence.conf, and simply ignores live-persistence.conf (because
it's ignored by persistence setup code anyway, so who cares).
And actually, this is done on purpose: if we did rename it, then when
the user follows the doc (that is, downgrades to 0.21), then they
would *not* benefit from the automatic migration process, which would
be a shame.
> but
> the documentation I'm pointed to from the notification is not really
> clear I think. It might be better to write bolder that one should
> first try to upgrade to 0.21.
In doc/first_steps/persistence/upgrade, in the first section
("Automatic upgrade"), the first bullet point is "If you have skipped
the Tails 0.21 upgrade and have upgraded to a newer version", which
points to how to the 0.21 upgrade process (maybe it's not *that* clear
there, I don't know). We could be make the "have skipped 0.21" case
higher priority on this page somehow, but it would make the doc less
clear for people going the 0.20.1 -> 0.21 way, that is basically the
same people, one hour later, so I'm not sure we gain anything.
Frankly, I don't think I'm able to do much better. sajolida, if you
feel this should be improved and see how, you're much welcome
(deadline: December 7).
Alternatively, you could try and follow the doc, and tell me exactly
where you get lost, or where it's unclear. I know it too well to do
that myself.
> That also means we sould let 0.21
> download-able on mirrors for a while
Sure. We always keep one or three old versions in the "obsolete"
directory anyway, so nothing new.
> 2. enabling persistence from 0.20.1, upgrade to 0.21, add a
> live-additional-software.conf as root, upgrade to experimental.
> Persistence works but live-additional software is disabled, and an
> empty file is created: OK
I don't get why it's OK that live-additional-software is disabled.
Does "add a live-additional-software.conf as root" imply "with correct
access rights" or "with wrong access rights"?
> *But* if I doesn't delete the .insecure_disabled file, on next boot I
> still get the warning notification, even though additional software are
> actually configured. That's a bit confusing. (I don't think this is a
> blocker, but it should be fixed at leaset for next release).
Unless I missed something, the doc page you're pointed to in this case
(doc/first_steps/persistence/recover_insecure) tells you to do *one*
thing:
To enable again your persistent volume, follow the instructions to
[[manually copy your persistent data to a new
device|copy_to_a_new_device]].
Incidentally, it doesn't tell you "alternatively, feel free to do
whatever you want instead, we'll take care of the consequences of
every possible creative action you might take, and we promise we'll
make your life as painless as possible" :)
In the described situation, Tails is in a broken state. We're pointing
the user at documentation that explains how to repair it. If one does
not follow these instructions, no wonder Tails is still broken (and
protests in a different way, I admit that may be confusing). I don't
think we should even try to support this case, and I don't find it
a good use of my time to improve it. So, I beg to strongly disagree
with your "it should be fixed at least for next release". Fair enough?
OTOH, we could as well teach live-persist that files owned by root are
OK, *but* this will add diversity in what's deployed in the wild, and
make things needlessly more complex when we want to add some GUI for
the additional software feature, so I'd rather avoid that.
> 3. setting manually wrong permissions
> * wrong permission on persistence.conf: FAILS
> Persistent folders are mounted even though they are reported as
> unsafe.
Ah, crap. That one is indeed a blocker. We've got a bug in how we're
using live-boot's get_custom_mounts and friends, and I bet some
caching hits us.
The thing is, I don't see why it would occur for wrong permissions
more than for any other access rights that's not correct, and
I suspect it could be not-100%-reproducible (read: racy).
Going to first try and reproduce it, then have a quick look at
live-boot's code, and investigate. I hope I'm able to propose a fix by
the end of the week-end..
Thanks for all this good feedback! (Even when I happened to disagree,
it's been much appreciated :)
Cheers,
--
intrigeri
| GnuPG key @ https://gaffer.ptitcanardnoir.org/intrigeri/intrigeri.asc
| OTR fingerprint @ https://gaffer.ptitcanardnoir.org/intrigeri/otr.asc
