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

Delete this message

Reply to this message
Author: intrigeri
Date:  
To: The Tails public development discussion list, sajolida
Subject: Re: [Tails-dev] Please start reviewing bugfix/7345-upgrade-from-iso-from-1.0-to-1.1
Hi,

@sajolida: the thing you care about is the last bit.

anonym wrote (19 Jun 2014 14:11:48 GMT) :
>> 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?


Agreed. I've merely copied'n'pasted what we already have in other
syslinux-related binary_local-hooks, so I think changing this
consistently belongs to a different ticket and branch => filed #7420.

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


Right. I was aware of it, but I wanted to start with adding a check
(untested yet) that ensures we don't see regressions on that front in
the future. Anyway, that's merely an additional test, that should
(hopefully) not break anything else. Good enough?

> 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!


Thanks for the nice words :)

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


It certainly would be interesting to look deeper into this some day.

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


Cool.

>> 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 want to test clone'n'upgrade, clone'n'install, and a "real" IUK
upgrade. All these from 1.1~betaN and 1.0.1 + the new liveusb-creator.
Help is warmly welcome.

I'd like to also add to the design document a minimal discussion of
the security impact of these changes: basically, anyone who can feed
an arbitrary IUK can run arbitrary code as the tails-install-iuk user
(we don't care, the same adversary can plant a persistent rootkit
anyway, but then it made me think and file #7410, that I'd like to fix
at the same time); and anyone who can feed an arbitrary ISO into
liveusb-creator can run arbitrary code as the user running
liveusb-creator (no big deal either). In both cases, no big deal, but
I'd like to document *why* it's no big deal, so that it's easier for
others to review my reasoning.

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


Will do. Thanks for the reminder.

> 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?


My intent was to fix for real the underlying, deeper problem that
causes #7345. I wanted to investigate the feasability of this
solution. Now it seems to be proven, so we can actually discuss
whether we want to suggest anyone to use this solution *this* time, or
just merge, add to known issues in the 1.1 release notes, and be happy
that the problem won't show up anymore. Cc'ing sajolida to get
his opinion.

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


Yes, I expect we'll regularly update syslinux, starting with jumping
to a much newer 6.03~preN or 6.03 final ASAP.

Cheers,
--
intrigeri