Tails

#3 Updated by intrigeri 2018-06-30 14:20:10

Recent util-linux has:

commit a9cf659e0508c1f56813a7d74c64f67bbc962538
Author: Carlo Caione <carlo@endlessm.com>
Date:   Mon Mar 19 10:31:07 2018 +0000

    lib/randutils: Do not block on getrandom()

    In Endless we have hit a problem when using 'sfdisk' on the really first
    boot to automatically expand the rootfs partition. On this platform
    'sfdisk' is blocking on getrandom() because not enough random bytes are
    available. This is an ARM platform without a hwrng.

    We fix this passing GRND_NONBLOCK to getrandom(). 'sfdisk' will use the
    best entropy it has available and fallback only as necessary.

    Signed-off-by: Carlo Caione <carlo@endlessm.com>

… which might be relevant.

Context: https://lists.debian.org/msgid-search/20180514003034.GI14763@thunk.org

#4 Updated by segfault 2018-06-30 23:45:26

Thanks. Might be relevant if we hit the same problem. But IIRC we already tested the feature branch and found that it does successfully grow the system partition during first boot without significantly increasing boot time.

#5 Updated by Anonymous 2018-08-16 12:07:19

segfault: if this is ready for QA, do you want to assign this to a potential reviewer?

#13 Updated by intrigeri 2018-10-09 12:20:15

One more thing: please test that the resulting USB stick (after the 1st-boot changes) still boots on non-EFI systems (i.e. the changes made to the GPT were sync’ed to the hybrid/protective/whatever MBR).

#17 Updated by intrigeri 2018-10-10 10:37:53

> This doesn’t have to be done in time for 3.10, right? If so, I would like to work on it after 3.10

Right.

#18 Updated by segfault 2018-10-16 12:01:06

intrigeri wrote:
> I’ll review the branch here although some of these comments probably apply to one of the many sibling tickets. Actually, the ticket setup here seems too fine grained (=> overhead) to me so if you prefer, maybe we can merge all these “what we need to do at early boot” tickets into a single one and use its description to track what’s the plan and progress (see e.g. how I’m doing it on Feature #14588).
>
> * I’m warry of /bin/sh -e: if whatever environment this runs under starts to source this script instead of executing it, then -e will be ignored. set -e would make me feel more comfortable. And while you’re at it, go for set -eu :)

Fixed

> * [ -n "$debug" ] && set -x looks like it will fail under set -e in non-debug mode (but that might be a corner case where it won’t actually break, I don’t remember). Using an explicit if/then construct would avoid wondering and diving deep into the darkest corners of the dash doc.

It don’t think it breaks, because I tested the ISO built from this branch with and without debug flag. I changed it to an if/then construct.

> * Have you tested how if [ -b "/dev/disk/by-uuid/${FSUUID}" ] works when booting on DVD?

I will do that on Feature #16057

> * Indentation is inconsistent (mixes tabs & 2-spaces) in the partitioning script.

Fixed

> * Please add a comment that makes the practical consequences of exit 1 explicit. “Skipping partitioning” suggests the boot process will simply continue but a user-facing message can’t convey the amount of technical details I would like to see.

As explained elsewhere (don’t know where that was), the status code is irrelevant in the current implementation of live-config. Is that what you want me to write in a comment?

> * | grep ID_PATH= | sed s/ID_PATH=// is not strict enough for such a strict (not anchored regexps) and could be done with sed only; I suggest replacing these with | sed -n '/^ID_PATH=/ s/^ID_PATH=// p'. Same comments for other |grep|sed constructs.

Done

> * Please quote variables more consistently: given the nature of this script, robustness even in all unexpected corner cases is primordial and we don’t ever want to land into undefined behavior territory.

Done

> * Please give SIZE a more specific name. Also, maybe s/TOTAL_SIZE/DEVICE_SIZE/ (to avoid wondering total size of what?).
>
> Nitpicking:
>
> * Any reason to use $((…)) in one place and $(expr …) in another?

Fixed

> One more thing: please test that the resulting USB stick (after the 1st-boot changes) still boots on non-EFI systems (i.e. the changes made to the GPT were sync’ed to the hybrid/protective/whatever MBR).

Will do that on Feature #16057

> Also, it seems that we need to resize the FAT filesystem as well because while the partition was successfully resized in my tests, the FS is still 2GB:

I will look into that later

#20 Updated by segfault 2018-10-16 14:16:28

> Have you tested how if [ -b “/dev/disk/by-uuid/${FSUUID}” ] works when booting on DVD?

So, $FSUUID is unset when booting on DVD, which means that the script will wait for the timeout (1 minute currently) before skipping the partitioning.

I remember that I knew about this issue back when I last worked on the script (in February), but I can’t find any notes / tickets about it and don’t remember if I already tried to fix it.

I implemented a fix and will test it after building the ISO.

#22 Updated by segfault 2018-10-17 16:51:54

> One more thing: please test that the resulting USB stick (after the 1st-boot changes) still boots on non-EFI systems (i.e. the changes made to the GPT were sync’ed to the hybrid/protective/whatever MBR).

Done

#23 Updated by segfault 2018-10-17 17:05:06

intrigeri wrote:
> > As explained elsewhere (don’t know where that was), the status code is irrelevant in the current implementation of live-config. Is that what you want me to write in a comment?
>
> Yes, exactly: the fact I did not remember is actually a good example of why a comment would be useful :)

Done

#26 Updated by segfault 2018-10-18 23:41:46

I was able to successfully grow the filesystem with fatresize - but then the device doesn’t boot anymore, because fatresize overwrites the boot code in the VBR.

The device boots again after restoring the original bootcode (and JMP instruction at the beginning of the VBR). Next step: Implement this in the partitioning script.

Oh, and I already spent a total of 8.00 h on this (including the subticket Feature #16057).

#29 Updated by intrigeri 2018-11-02 18:16:30

• Nitpick: if [ -z "${FSUUID}" ] will fail under set -u if $FSUUID is not set (which is different than “set to the empty string”). I think you want if [ -z "${FSUUID:-}" ]. I guess it does not matter much here because live-boot will set FSUUID= but still, it took me some time to understand why it does not break, so when running under set -u, better make it clear. I’m so sorry you have to deal with this shell crap, but that’s what early boot is made of :/ Maybe some day we’ll write Go or Rust for this sort of things that must run without a big Python/Ruby/Perl interpreter, hey :)
• IIRC syslinux’ ldlinux.sys (generated when we run syslinux) includes hard-coded block numbers, or the MBR hard-codes the block number where it can find ldlinux.sys, or similar. This is “slightly” outside of my comfort zone. Can we assume that fatresize won’t mess this up? I know your legacy BIOS tests seem successful, I’m just wondering if we’ve been lucky so far or if we can really count on it. I guess the safer way to avoid wondering would be to run syslinux again after resizing the FS, but arguably that opens another can of robustness worms.

Once you’ve fixed the minor shell issue and satisfied my curiosity wrt. ldlinux.sys, feel free to close this ticket as resolved :)

#31 Updated by intrigeri 2018-11-02 18:40:48

Actually, I’m not sure we should do nothing at all if the device is smaller than 7200 MiB. Nothing prevents one to install there (only Tails Installer would complain but we’re going to deprecate it for initial installation) if they skip the note about this in our doc. And then nothing prevents them from creating a persistent volume. Once that’s done, the only way to upgrade is a manual upgrade. But if the size our ISO has grown a little bit in the meantime, that’s not gonna work, which is a UX regression compared to the current state of things (I think that Tails Installer refuses to install on a too small device but accepts to upgrade as long as the upgrade fits on the destination device, so that people can keep using and upgrading their existing sticks with a 2.5GB system partition). And given the device is filled by the persistent volume, it’s pretty hard to recover from this failure mode.

So I think that we should abort with an error message pointing to the installation doc. Rationale: those sticks are unsupported and we can’t provide a good UX on them. Better abort while the user has not invested too much into them, it’s still time to reinstall on a bigger device, than let them go on, create a persistent volume and be stuck. If plymouth is already running at this point, fine, we can use it to display the error. Otherwise, we have to display the error on the good ol’ text console.

Only newly installed sticks are affected: older ones have a different GUID so we just do nothing (modulo pending discussion on Bug #15987).

If we can easily agree on something like this, let’s just implement it. If it’s not obvious to you, then let’s ask the UX gods™ (aka. sajolida) to tell us what we should do.

Yeah, every plan is doomed to change because it has always missed something :)

#32 Updated by intrigeri 2018-11-02 18:54:24

intrigeri wrote:
> Only newly installed sticks are affected: older ones have a different GUID so we just do nothing (modulo pending discussion on Bug #15987).

Even if we decide, on Bug #15987, to do what was planned, we can still keep these old and too small sticks running: we error out only if the device is too small AND has the known GUID for initial installations, i.e. it’s its first boot. If the device is too small but has another GUID, we exit 0.

#34 Updated by Anonymous 2018-11-02 20:05:01

segfault wrote:
> Oh, and I already spent a total of 8.00 h on this (including the subticket Feature #16057).

Thanks! Is this because of a technical difficulty or did we completely underestimate the time for this task?
Maybe we could keep track somewhere on how much this one was overclocked, for future grants? Feel free to send this information to me by mail.

#35 Updated by segfault 2018-11-07 10:19:02

We should make sure that both the partition label in the GPT and the label in the FAT are still set to “Tails” after running the partitioning script.

#36 Updated by segfault 2018-11-11 20:47:20

> * IIRC syslinux’ ldlinux.sys (generated when we run syslinux) includes hard-coded block numbers, or the MBR hard-codes the block number where it can find ldlinux.sys, or similar. This is “slightly” outside of my comfort zone. Can we assume that fatresize won’t mess this up?

Yes. The first block number of ldlinux.sys is stored in the code part of the VBR, which we restore after running fatresize. And we can be sure to still find ldlinux.sys at this block number, because fatresize only changes the file allocation table and doesn’t move the contents of the filesystem.

#37 Updated by segfault 2018-11-11 20:54:46

> Nitpick: if [ -z "${FSUUID}" ] will fail under set -u if $FSUUID is not set (which is different than “set to the empty string”). I think you want if [ -z "${FSUUID:-}" ]. I guess it does not matter much here because live-boot will set FSUUID= but still, it took me some time to understand why it does not break, so when running under set -u, better make it clear.

Done in e9a28349a608a6a4ed398f791d4f0765c35eb6dc

#39 Updated by intrigeri 2018-11-12 12:22:51

>> * IIRC syslinux’ ldlinux.sys (generated when we run syslinux) includes hard-coded block numbers, or the MBR hard-codes the block number where it can find ldlinux.sys, or similar. This is “slightly” outside of my comfort zone. Can we assume that fatresize won’t mess this up?

> Yes. The first block number of ldlinux.sys is stored in the code part of the VBR, which we restore after running fatresize. And we can be sure to still find ldlinux.sys at this block number, because fatresize only changes the file allocation table and doesn’t move the contents of the filesystem.

Great news and thanks for checking!

#40 Updated by intrigeri 2018-11-12 12:23:52

> Done in e9a28349a608a6a4ed398f791d4f0765c35eb6dc

ACK

#42 Updated by intrigeri 2018-11-17 15:48:25

> Solution 2 also seems more costly to implement as it might require patching Tails Upgrader (what is happening now on small USB sticks right now?)

Tails Upgrader does not care at all about the absolute size of the system partition: it only cares about whether there’s enough space left there to download and apply the upgrade. So there’s no need to patch it for solution 2, I think: the error message we display already covers that case.

> So I think that the UX won’t be super great either way and I’m not super convinced by the UX advantages of solution 1 over solution 2. Seeing that we have a very tight budget to work on this project, I would go for solution 1.

+1 (using a pretty big unit)

> Another solution (solution 3) could be to improve solution 1 to propose people to clone to a new USB stick when we detect that the original one is smaller than 7200 MiB. Maybe starting Tails Installer as the only possible thing after Tails Greeter or something, plus a nice message. Note that if we’re out of budget and solution 1 is super easy to implement, we could keep this solution 3 for later.

For now, to be on the safe side, let’s assume we’ll be out of budget even if we implement the cheapest possible version of solution 1.

> Also, should we add extra warning about the size of the USB stick during the installation steps? Right now we only mention the minimum size in the overview (eg. https://tails.boum.org/install/win/usb-overview/) but it might be worth it to mention it as well in the steps (eg. https://tails.boum.org/install/win/usb/).

Yep. The time when we instruct the candidate user to “Plug the [first/second] USB stick in the computer” seems to be a fine place to remind those who missed this requirement on the overview about it :)
Which made me notice how small, low contrast (light grey on white) this info is there, which is reinforced by the fact it’s below the biggest contiguous black-on-white surface on the page (the 2 USB sticks images), that catches the eye and somewhat tends to hide what’s around it.

#44 Updated by segfault 2018-11-22 14:42:40

And FWIW, the implementation of 1 was not particularly easy, because I have to rebuild and boot Tails everytime I want to test a change in the initramfs script (4 times until now) - I think displaying a warning message in the notification area would have been easier.

#45 Updated by segfault 2018-11-22 15:18:29

> 2. Wait 10 seconds, then reboot

The rebooting doesn’t work currently, the boot continues after the 10 second wait. I want to know which behavior exactly we want before I spend more time trying to fix this.

#46 Updated by intrigeri 2018-11-22 15:49:55

> I’m not happy about the decision to implement solution 1. IMO, this is a needless restriction, and there are legitimate use cases which don’t require being able to upgrade,

FTR I’m not super happy either and it’s always sad to forcibly drop support for use cases (even those that we have explicitly stopped supporting a while ago) that we would like to support. But for me it’s the best trade-off and your “needless restriction” phrasing makes it sound like you’re missing an important part of the Tails picture, which is expected since you’re not exposed to it much personally (yet :)

So let me explain where I’m coming from: we’re already struggling with handling all the possible Tails USB sticks that we’ve let users create; it makes us do more work, and harder work; this impacts help desk and FT people who work with help desk (that is mostly me ATM). So in order to justify making that matrix bigger and this work even harder, the initial development cost is irrelevant: what matters is rather “are the additionally supported use cases worth the running cost of the extra & harder support & maintenance work?”. If the answer is “no”, then this restriction is not needless, it helps us keep Tails sustainable. So far I’ve not seen example use cases that come anywhere near convincing me the answer could be “yes” here but I may be missing something.

> for example if you just want to try out or demonstrate Tails (think crypto party)

I’m not sure I get what user scenario is affected exactly. Do you mean “I go to a cryptoparty with an empty 4GB USB stick, the people who help me install Tails have no clue about the hardware requirements, so when I start my Tails for the first time I get a nasty error message in plymouth” and “I got to a cryptoparty to demonstrate Tails but I don’t remember the hardware requirements so I bring only too small USB sticks”? Or something else?

> or test a USB image in a VM before installing it on a USB stick.

I would like to understand this better:

• Impacted people: who does that and why? Are we talking developers, power users, someone else?
• Impact on these people: indeed, to do that one will need to dd the .img to a 8GB virtual disk as opposed to passing the .img directly to the virtualization software. Assuming one is not doing it for the first time, in libvirt it’s 1 command (dd) vs. 1 command (install the .img with the right permissions to some place where libvirt can read it). So to me it does not make a big difference. But I’d be happy to hear why you feel it’s harder.

> And FWIW, the implementation of 1 was not particularly easy, because I have to rebuild and boot Tails everytime I want to test a change in the initramfs script (4 times until now)

Ouch. At this point I would probably remaster an existing USB image until I get something that works fine, and only then I would rebuild the whole thing.

> I think displaying a warning message in the notification area would have been easier.

But then we would have had to add more code elsewhere specifically to handle this case. More code, more bugs, more maintenance cost, you know the drill :)

#47 Updated by intrigeri 2018-11-22 16:24:30

segfault wrote:
> I want to know which behavior exactly we want before I spend more time trying to fix this.

Keeping in mind that this is a corner case and we have more important topics to work on, IMO any behavior is OK as long as 1. we display the error message long enough for the user to read it (even if they went to brew coffee or something while Tails was starting); 2. it’s hard to circumvent the error and let the boot process continue.

#48 Updated by intrigeri 2018-11-26 13:52:21

intrigeri wrote:
> segfault wrote:
> > I want to know which behavior exactly we want before I spend more time trying to fix this.
>
> Keeping in mind that this is a corner case and we have more important topics to work on, IMO any behavior is OK as long as 1. we display the error message long enough for the user to read it (even if they went to brew coffee or something while Tails was starting); 2. it’s hard to circumvent the error and let the boot process continue.

I’ve quickly implemented (on my own time) something that satisfies my own set of requirements and feels good enough to me. Regardless of what we decide, it seems to be an improvement vs. the initial half-working PoC, and unblocks the test suite work that will start tomorrow, but I won’t be sad if we drop it. What I’ve implemented is that if the device is too small, this message is displayed: “Sorry, this device is too small to run Tails. Please use a device with at least 8 GiB. Press ENTER to shut down.” and then pressing any key powers off the system. sajolida, good enough? Any improvement suggestion?