Apt and debootstrap authenticate files which they download. However, sometimes lb_build downloads files directly. Run `grep wget /usr/lib/live' to find some of the places where it is done.
When doing so, lb_build doesn't check if these files are original. An attacker can modify these files to affect the build process. For example, she can replace debian-installer kernel or initrd with arbitrary files (/usr/lib/live/build/binary_debian-installer).

yes, this is on the todo for quite a while.

Raising the severity of this, considering I am almost completely done
with building the patch for it, I'd really like to see this get into
Jessie, and considering that it allows complete compromise of a live
image and any installations from it, unless the user actually knows to
deploy a work around (which is not discussed at all in documentation and
perfectly easy therefore for a user to just assume it is just secure to
use with remote archives). Also contacting the security team to inquire
about a CVE being issued, for formalities sake.

Worth noting for the record, since this isn't documented anywhere:
 - The only work around to avoid compromise would be to create and use a
local archive instead of a remote one, separately taking steps to ensure
integrity of the local archive before use.
 - Even if you do this, if you opt to use the daily edition of the
installer image, this is downloaded directly from a debian server,
exposing you to compromise.

This security issue stretches back all the way as far as git history
goes, to 0.99-1. Attempting to update versions affected to update the
record, possibly causing correct listing against debian releases in
security trackers...

Patch attached. I strongly feel this should get into Jessie, so it's
been built on the debian-4.0-old branch.

This is a large patch, and I have broken certain components across
multiple commits. Here is a brief overview of the files in the attached
archive, which should be applied in numerical order.
 - #01-03 are patches for dependency issues reported separately, they
are required to be applied first in order for file #11 to then apply an
update on top of this. Daniel, I am aware that you're not accepting my
dependency changes exactly as I suggested, so you'll need to adapt these
as necessary, or if you really want to, you could skip all of these
(#03-04 & #11) without affecting the overall solution.
 - #04 is the patch for issue #1 and #4 of bug #774196, which needs
applying before my solution then changes these lines further in commit
#10. I am aware that in the debian-next branch you have just added
commits to remove sparc support, which creates a bit of an unfortunate
conflict...
 - #05-10 and #12 represent the actual solution. #05-09 do some "prep"
stuff like adding new config params, with #10 and #12 implementing the
bulk of the solution. Commits #08-09 rewrite some particular sections of
code before #10 can apply the new solution on top of it; I did them as
separate commits simply for clarity over what exactly changed.

In addition to secure wget downloads, there are one or two other
enhancements that are incidentally provided by the solution, so to
summarise the changelog:
 - Security verification of wget downloaded files added
 - Documentation and code updated to warn about use of daily installer
build, which cannot be downloaded securely
 - Caching of Contents-{arch}.gz files
 - New params for control over certain cache areas added
 - New init build stage added (used by new cache flushing mechanism
that's used for control over cache use of certain cached items)
 - Updated caching directory structure a little, as described further in
#774523

I think that essentially covers it (rushing writing this a little to get
it submitted while Daniel is actively working on LB bug reports :P )

Note: Unfortunately, Ubuntu building support will be completely broken
after this patch is supplied because Ubuntu repositories do not
currently match the Debian layout. Daniel and I feel that it is Ubuntu's
responsibility to sort out their layout, so for Ubuntu building to work
again either they must do that, or a patch must be built (which will be
relatively large, contact me if anyone else is intent on doing this so
that I can guide you on exactly what is required). This is partly
discussed in bug #774378.

Sorry, when I wrote that you can ignore "(#03-04 & #11)" if you're not
interested in adopting the included dependency changes (to control and
bug script), I actually meant #01-03 & #11 !!!

patches 05 to 12 are for wget, spanning over 3433 lines.. even if we'd
speedy review, test, and apply that.. i don't think the release team
will approve it in the current stage of the jessie release.

unrelated to that, i'd prefer applying patches on debian-next (5.x) only
anyway, and cherry-picking them to debian-old-4.0 where required.

not a problem, it wasn't working before, and it's not working afterwards
- so no change from the users perspective.

Well considering it addresses a significant security issue (only
relative to LB images, not Debian installs in general of course), which
I would consider to be of critical severity considering the significant
ease and scope of compromise, I would have thought they would make an
exception. The release date for Jessie is flexible after all (I believe,
with only a hope that it might get done before the end of the month, not
a solid deadline). With that said, considering it is security related,
the security team might accept a Jessie patch after release, but it's
just easier all around isn't if it a new version gets pushed into Jessie
now?

If it helps at all, I've done a fair bit of testing and bug fixing of it
with Debian builds before I submitted it. Ubuntu builds are already
broken, so progress-linux is the only dist untested. I am pretty damn
confident with it at this stage though. I do understand that you might
want to do some amount of testing yourself however.

Could we at least try and see if the release team will accept it? Or at
least establish contact to ask?

I understand the desire to build on 5.x at this point. It's a little
unfortunate that certain 5.x patches cause a little conflict with
pushing this in cleanly to both 5.x and 4.x. I was hoping to get this
done before too much changed in 5.x, but it took a lot of work to
complete and it's too late. I can and will assist in a rebase of this
patch for the 5.x branch also if you need me to.

In fact I'm building a rebase on 5.x now, at least so I have a 4.x and
5.x branch to rework #773775 on top of (some of which conflicts with this).

The problem is though, and this is why I built it directly for 4.x,
there are conflicts with at least cdebootstrap removal and sparc removal
in 5.x. So either you've got to rebase the 4.x patch onto 5.x to get it
in 5.x and resolve the conflicts like I'm doing now, or if you cherry
pick this from 5.x, you're going to have to either have to have a
modified 4.x specific veriosn like I've just built, or otherwise you're
forced to also cherry pick the cdebootstrap and sparc removal commits
also, and I'm not sure that you'd want to push those to 4.x, though this
would be the easiest solution, considering that I doubt you'd want to
rebase 5.x onto this big solution of mine, considering the 5.x git
branch is already public.

testing before I submit it...

Okay, here is a copy rebased onto 5.x. The early package dependency and
sparc fix commit dependencies are gone in this one, just leaving the
solution itself. The second to last commit adds a gpg/gpg2 dependency.

As previously expressed, cherry picking this back to 4.x is certainly
easier IMO, avoiding you (Daniel) having to do the work of carefully
reworking the 4.x path onto 5.x as I just did, or merging slightly
different copies of the solution to each branch, having to review and
compare both. However of course to cherry pick back to 4.x, this
requires also bringing at least the following changes with it:
 - removal of cdebootstrap (or careful merging of my solution in the
config script
 - removal of sparc support
 - dependency updates in control file (or carefuly merging of my
solution in the control file, or ignoring the second to last commit in
my 5.x solution in the backport).
 - the 'LIVE_IMAGE_PARENT_ARCHIVE_AREAS' python cleanup commit (actually
the 4.x version was based on this also).

In case it's helpful to reviewers, here's a brief high level overview of
some aspects of the new solution I'm providing:
Apart from Release and Release.gpg which are treated as a pair, files
being downloaded and the files needed for verification are downloaded
and verified in a chain of function calls. It employs a somewhat smart
use of caching. When the build process starts, by default dist-info and
installer image files (vmlinuz and inird) are flushed, though the user
has control over this with new parameters (and can also optionally flush
them via the clean script). Throughout the process, files are reused
from the cache if they exist in the cache, but only as long as they pass
verification, and as long as verification isn't disabled (which can be
done via --wget-secure false, similar to apt). If verification fails (at
any point in the verification chain), any cached copies of files that
were used are flushed (it keeps track of them for each download), and
they are redownloaded for a second try. This means that a mixture of
some old (or broken partial downloads) and some new files will not cause
a failure, if they should pass, they will, if not, the ones used from
the cache will be replaced, and one further verification attempt made.
Furthermore it keeps track of files recently verified, so reduce
unnecessary work, though unfortunately it can't track across the entire
build process due to the fact that each stage is executed through the lb
script, which resets the environment (not a big deal though, it still
adds some efficiency, particularly in the installer stage). Much of the
new code makes use of a new 'Get_cache_path' function, which, providing
a more central path compositor, helps reduce mistakes through avoiding
code repetition.

Additional patch attached (based on 5.x work). It promotes the gpgv
dependency to recommends from suggests, since it's now used for secure
verification of wget downloads, and so should sit at the same level as wget.

Small additional patch attached to correct wrong usage string in new
init flush-cache script (copy & paste error when creating new file).

Small additional patch attached.

I'm relatively new to unix shell scripting, and towards the end of
development I was having some issues with error return codes and set -e
in one or two places. I thought I had addressed the issue properly, but
running into the issue again now in other code, I've gained a better
understanding of it and realised that a small fix is in order here.

Testing this has also highlighted another issue, which I will provide as
a follow up once I've fully resolved it.

Updated archive of full set of commits attached, including two new ones.

Overview:
 - Commits #01-08 : A copy of the original set build against LB 5.x
 - Commit #09: Fix handling of return codes (previously submitted
earlier today)
 - Commit #10: Fix incorrect usage tracking logic (new)
 - Commit #11: Fix broken file purge ability (new)
 - Commit #12: Fix wrong usage string in new file (previously submitted)
 - Commit #13: Promote gpgv dependency (previously submitted)

Aside from the patch tweaking error return code handling submitted
earlier today, when testing that new patch, I tried the code against a
set of cached files, including a few random ones which I deliberately
corrupted. Previously I had tried one corrupt file, which must have only
been the file targeted for download, which it had corrected perfectly,
but I must have not tested with other cached but corrupted files in the
verification chain, because doing so now it didn't work as expected.
They were picked up as invalid, it attempted to purge files used from
the cache, performing a second chance routine, but unfortunately there
was a bug in the purge code preventing file deletion, and a flaw in the
usage tracking logic, so it redownloaded the target file, but failed to
replace other files in the verification chain, reusing the corrupt
copies, failing the second chance verification, and thus halting the
build process.

All of that is fixed now though with the few small additional commits
added to the attached set :)

I can only assume that no-one has tried to actually execute the new code
here yet apart from myself, since no-one has pointed out the file
permissions error I have made...

I've been doing a weird thing of developing on Windows, then copying the
files to a Debian VM for testing, and although I've been a little wary
of the possibility of permissions issues when adding new executable
files, I wasn't certain whether or not there was actually a problem
until just now. A few new executable files were added in my work here,
which have been added with mode 100644, when they actually need to be
100755 to function correctly.

I have modified the patches to correct the permission mode where
necessary, re-imported them on top of debian-next, and re-extracted
them. The previous set of patches should be discarded in favour of these.

Daniel, in terms of review, should you have already started, the only
thing I have changed is correcting the file permission mode for a few
files. There were no significant changes that this was rebased upon, so
the only other thing that should have changed is the hashes, otherwise
they should be identical, as I'm sure you'll find a way to verify,
saving you from having to start from scratch to be sure I'm not doing
anything funny.

Rebased copy attached.

Specifically this was just done to take into account the bug #774200
improvement (using same trace file download logic in binary_disk as in
source_disk). So only that small piece of code in binary_disk should
have changed since the last upload (excluding hashs/timestamps obviously).

Control: tag -1 - patch
Control: severity -1 + wishlist

I quickly reviewed the patch set and it's way too intrusive for me to
commit it. I think the goal is laudable but the approach taken is
overkill.

We don't need new options, we just want something safe by default. If
anything we should be using apt even more...

I don't plan to spend time on this large problem given that live-build.
live-build is in a low maintenance mode and that kind of large change
is not the kind of thing that we are interested in.

Cheers,

Patrick (adrelanos) from the Kicksecure team and myself discovered this
vulnerability before realizing this bug existed. I developed a full
proof-of-concept exploit for it, and informed the Debian Security Team
about it. They got back to me and don't appear to have a problem with me
publishing the details, so I've made my original email to the Debian
Security Team public as a GitHub gist (so as to not flood the BTS with
a massive comment). The PoC instructions are
here: https://gist.github.com/ArrayBolt3/99d1296a6d82b5a6f2453943eaf85520

If you're using live-build, I'd highly recommend setting the various
`--mirror` and `--parent-mirror` settings in your `lb config` commands
explicitly, specifying HTTPS repos for each of those settings. It's not
a perfect solution, but as long as no one has compromised HTTPS, it
should be sufficient to plug this hole.

Hello Aaron,

Additionally, you can use `--debian-installer-distribution git`, which
will rebuild the installer from source code, therefore disarms a
potential attack via the initrd path, leaving only a potential attack
vector for the other files that are downloaded with wget.

Note that by using HTTPS versions, you'll lose the ability to use a
proxy, which means that the amount of network traffic will increase.

Certainly a same tool to perform the attack (mitmproxy) can be used as a
https-proxy, but to me that sounds like it would open a whole new can of
worms.

With kind regards,
Roland Clobus