Re: [Tails-dev] Please start reviewing bugfix/7345-upgrade-f…

Poista viesti

Vastaa
Lähettäjä: anonym
Päiväys:  
Vastaanottaja: The Tails public development discussion list
Aihe: Re: [Tails-dev] Please start reviewing bugfix/7345-upgrade-from-iso-from-1.0-to-1.1
16/06/14 18:27, intrigeri wrote:
> Hi,
>
> [This email is primarily meant for anonym, with additional bits near
> the end for those who want to comment on how we could handle the
> "Upgrade from ISO" mess for 1.0.1-->1.1, from a "communicating with
> users" perspective.]


Code review, Tails Git:

> commit ce9316410c2d21497e6680735c17d2f60705c6c0
> +# Safeguards
> +[ "${LB_BOOTLOADER}"   = "syslinux" ] || exit 0
> +[ "${LB_ARCHITECTURE}" = "i386"     ] || exit 0


I'm not completely sure this is good advice, but what about instead
exiting with 1 (and printing an appropriate error) to fail the build so
we'd be forced to revisit this issue if we ever add support for (or
switch to) something else than syslinux or i386?

> commit b7f5054339ead7257eb49b973f24679534b3cf9f


Just a comment: This will not work with Tails <1.1 fed to --old-iso
since the test suite has been updated for Wheezy...

The changes in liveusb-creator looks good, but beware that I'm not at
all familiar with the code base. Kudos for the very informative commit
messages, which helped a lot!

> I've spent quite some time implementing what I believe is the proper
> solution to #7345 ("Tails 1.1~beta1 created by upgrade from ISO from
> a 1.0 USB does not boot").
>
> I'm not done yet, but that's a relatively large piece of work, and
> I expect it will take a bit of time to review this all, so it would be
> good if anonym (or anyone else) could start ASAP.
>
> In "short", the root of #7345 is that Tails Installer runs the running
> system's syslinux against the target device. In cases when the running
> system is older than the newly upgraded one (which is usually the
> case, else one would simply "Clone and Upgrade"), the target drive
> then gets a mix of new COM32R (*.c32) modules, a probably old MBR, and
> old ldlinux.sys (put in place by running syslinux). This kind of mix
> is generally unsupported: it *might* work when mixing files coming
> from close-enough versions of syslinux (untested, and upstream is
> explicitly not recommending that), but it definitely doesn't work when
> mixing 4.x and 6.x files.
>
> Unfortunately, we can't simply use a new, working ldlinux.sys that
> could be found in the ISO, as this file depends on the filesystem
> layout (it contains the block number at which another bit of syslinux
> can be found). So, any proper, long-term-proof solution seems to imply
> to run the version of syslinux found in the just-upgraded system.
> My initial idea was to simply loop-mount it, chroot and boom, but this
> requires root privileges, and I'd rather not even try to do it safely.
>
> So, my solution is to copy the syslinux binary and its MBR, at ISO
> build time, into a new "utils" directory at the root of the ISO
> filesystem. Then both liveusb-creator and tails-iuk run that
> (presumably newer) copy of syslinux, and setup that (presumably newer)
> MBR, at the end of the upgrade process.


This seems safer and quite simple, even though the duplication (syslinux
both inside the squashfs and on the image's root fs) is a bit unfortunate.

In the future we could avoid that and use the in-squashfs syslinux
binary without root privileges by employing stuff like fuseiso (in
Debian), squashfuse [0] (not in Debian, but see [1]), and fakechroot (in
Debian) instead of a real chroot for running the syslinux binary.

Beyond eliminating the duplication of syslinux, we'd get the "mechanism
to run post-upgrade scripts" you mention in comment 4 of #7345 [2],
which may come in handy in the future.

[0] https://github.com/vasi/squashfuse
[1] As an alternative to squashfuse it should be easy to make a
    sudo-safe wrapper around `mount -o loop -t squashfs ...`, that
    checks that the destination folder is writeable by the $SUDO_USER.
[2] https://labs.riseup.net/code/issues/7345#note-4


> If I get it right, then the
> only tricky part we'll have to be careful about in the future is that
> the syslinux binary shipped in Tails version N+1 must run fine in
> Tails version N. E.g. we need to ship a *Squeeze* backport of
> syslinux 6 in Tails 1.1, which is actually what we're doing already.
> Ideally, this binary should also run fine on Tails version N+1.
> Given that this binary is currently only linked to glibc, this should
> not be too much of a hassle.


Ok. I don't like yet another strange thing to think about when preparing
releases, so here's another reason for eventually moving to the
(fake)chroot approach.

> I've successfully tested "Upgrade from ISO", from Tails 1.0.1, after
> installing liveusb-creator 3.11.6+tails1-5.bugfix.7345~3.gbp21039c
> from the bugfix-7345-upgrade-from-iso-from-1.0-to-1.1 APT suite,
> feeding it with an ISO built from yesterday's
> bugfix/7345-upgrade-from-iso-from-1.0-to-1.1 branch. The resulting
> device boots just fine, and has the expected files and MBR in place.
> It would be good if someone else could try and reproduce this.


I've now reproduced your positive results. Great stuff!

> I'll also have to test all other installation and upgrade scenarios,
> to make sure I didn't break anything else.


If you would enumerate these I can perhaps help a bit on Sunday,
depending on how much other review work there's for me then.

> I've not tested my changes to tails-iuk in a live environment yet.
> The test cases I've added to its test suite should cover most of it,
> but we'll see.
>
> I'm not sure when exactly I can complete this work, that has already
> taken me a lot of time. I hope I can do this on Friday/Saturday, else
> it may have to wait until next Tuesday (June 24) or Wednesday (June
> 25). Same for the UEFI doc, by the way.


Ok. Just a reminder: I'll only be available to review stuff on June
22th, starting at around 12:00 CEST, so you may want to prioritize stuff
you want me to look at to be ready then.

> Left to do: think how we announce to our users either that they must
> *not* use "Upgrade from ISO" in 1.0.1, to upgrade to 1.1; or, that
> they must first upgrade the liveusb-creator package somehow.
> Obviously, the first solution is easier for us, and given that the
> second one implies steps that many of our users may find too hard to
> follow, perhaps it's not worth the hassle of documenting it.


I'm confused. Isn't the purpose of this bugfix branch to allow the 1.0.1
-> 1.1 upgrade using "Upgrade from ISO" (even if it requires some
additional steps *this* time)? If so I definitely think we should
instruct users to upgrade liveusb-creator. Or do you just want to have a
fix for when this happens next time? I guess that may happen frequently
from now on to improve the UEFI support, or at least when the version we
use now goes stable.

Cheers!