| Age | Commit message (Collapse) | Author |
|---|
|
rred is responsible for unpacking and reading the patch files in one go,
but we currently only have hashes for the uncompressed patch files, so
the handler read the entire patch file before dispatching it to the
worker which would read it again – both with an implicit uncompress.
Worse, while the workers operate in parallel the handler is the central
orchestration unit, so having it busy with work means the workers do
(potentially) nothing.
This means rred is working with 'untrusted' data, which is bad. Yet,
having the unpack in the handler meant that the untrusted uncompress was
done as root which isn't better either. Now, we have it at least
contained in a binary which we can harden a bit better. In the long run,
we want hashes for the compressed patch files through to be safe.
|
|
Having every item having its own code to verify the file(s) it handles
is an errorprune process and easy to break, especially if items move
through various stages (download, uncompress, patching, …). With a giant
rework we centralize (most of) the verification to have a better
enforcement rate and (hopefully) less chance for bugs, but it breaks the
ABI bigtime in exchange – and as we break it anyway, it is broken even
harder.
It shouldn't effect most frontends as they don't deal with the acquire
system at all or implement their own items, but some do and will need to
be patched (might be an opportunity to use apt on-board material).
The theory is simple: Items implement methods to decide if hashes need to
be checked (in this stage) and to return the expected hashes for this
item (in this stage). The verification itself is done in worker message
passing which has the benefit that a hashsum error is now a proper error
for the acquire system rather than a Done() which is later revised to a
Failed().
|
|
Valid-Until protects us from long-living downgrade attacks, but not all
repositories have it and an attacker could still use older but still
valid files to downgrade us. While this makes it sounds like a security
improvement now, its a bit theoretical at best as an attacker with
capabilities to pull this off could just as well always keep us days
(but in the valid period) behind and always knows which state we have,
as we tell him with the If-Modified-Since header. This is also why this
is 'silently' ignored and treated as an IMSHit rather than screamed at
the user as this can at best be an annoyance for attackers.
An error here would 'regularily' be encountered by users by out-of-sync
mirrors serving a single run (e.g. load balancer) or in two consecutive
runs on the other hand, so it would just help teaching people ignore it.
That said, most of the code churn is caused by enforcing this additional
requirement. Crisscross from InRelease to Release.gpg is e.g. very
unlikely in practice, but if we would ignore it an attacker could
sidestep it this way.
|
|
Not all servers we are talking to support If-Modified-Since and some are
not even sending Last-Modified for us, so in an effort to detect such
hits we run a hashsum check on the 'old' compared to the 'new' file, we
got the hashes for the 'new' already for "free" from the methods anyway
and hence just need to calculated the old ones.
This allows us to detect hits even with unsupported servers, which in
turn means we benefit from all the new hit behavior also here.
|
|
Especially pdiff-enhanced downloads have the tendency to fail for
various reasons from which we can recover and even a successful download
used to leave the old unpatched index in partial/.
By adding a new method responsible for making the transaction of an
individual file happen we can at specialisations especially for abort
cases to deal with the cleanup.
This also helps in keeping the compressed indexes around if another
index failed instead of keeping the decompressed files, which we
wouldn't pick up in the next call.
|
|
If we get a IMSHit for the Transaction-Manager (= the InRelease file or
as its still supported fallback Release + Release.gpg combo) we can
assume that every file we would queue based on this manager, but already
have locally is current and hence would get an IMSHit, too. We therefore
save us and the server the trouble and skip the queuing in this case.
Beside speeding up repetative executions of 'apt-get update' this way we
also avoid hitting hashsum errors if the indexes are in fact already
updated, but the Release file isn't yet as it is the case on well
behaving mirrors as Release files is updated last.
The implementation is a bit harder than the theory makes it sound as we
still have to keep reverifying the Release files (e.g. to detect now expired
once to avoid an attacker being able to silently stale us) and have to
handle cases in which the Release file hits, but some indexes aren't
present (e.g. user added a new foreign architecture).
|
|
Calculating the final name of an item which it will have after
everything is done and verified successfully is suprisingly complicated
as while they all follow a simple pattern, the URI and where it is
stored varies between the items.
With some (abibreaking) redesign we can abstract this similar to how it
is already down for the partial file location.
Git-Dch: Ignore
|
|
Git-Dch: Ignore
|
|
We have a bunch of classes which are of no use for the outside world,
but were still exported and so needed to preserve ABI/API. Marking them
as hidden to not export them any longer is a big API break in theory,
but in practice nobody is using them – as if they would its a bug.
|
|
feature/no-more-acquire-guessing
Conflicts:
apt-pkg/acquire-item.cc
|
|
|
|
The pkgAcquire::Run() code works uses a while(ToFetch > 0) loop
over the items queued for fetching. This means that we need to
Deqeueue the item if we call AbortTransaction() to avoid a hang.
|
|
Handle Translation-* files exactly like Packages files (with the
expection that it is ok if a download of them fails). Remove all
"guessing" on apts side. This will elimimnate a bunch of errors
releated to captive portals and similar. Its also more correct
and removes another potential attack vector.
|
|
partial files are chowned by the Item baseclass to let the methods work
with them. Now, this baseclass is also responsible for chowning the
files back to root instead of having various deeper levels do this.
The consequence is that all overloaded Failed() methods now call the
Item::Failed base as their first step. The same is done for Done().
The effect is that even in partial files usually don't belong to
_apt anymore, helping sneakernets and reducing possibilities of a bad
method modifying files not belonging to them.
The change is supported by the framework not only supporting being run
as root, but with proper permission management, too, so that privilege
dropping can be tested with them.
|
|
This option controls the maximum size of Release/Release.gpg/InRelease
files. The rational is that we do not know the size of these files in
advance and we want to protect against a denial of service attack
where someone sends us endless amounts of data until the disk is full
(we do know the size all other files (Packages/Sources/debs)).
|
|
feature/acq-trans
Conflicts:
apt-pkg/acquire-item.cc
|
|
Using a different user for calling methods is intended to protect us
from methods running amok (via remotely exploited bugs) by limiting what
can be done by them. By using root:root for the final directories and
just have the files in partial writeable by the methods we enhance this
in sofar as a method can't modify already verified data in its parent
directory anymore.
As a side effect, this also clears most of the problems you could have
if the final directories are shared without user-sharing or if these
directories disappear as they are now again root owned and only the
partial directories contain _apt owned files (usually none if apt isn't
running) and the directory itself is autocreated with the right
permissions.
|
|
|
|
|
|
|
|
Move common code out but do not use subclassing for ::Done
to make it easier to understand what each class is doing when
its done
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
The fileformat of a pdiff index stores currently only SHA1 hashes. With
this change, we look for all other hashes we support as well and take
what we get, so that we can work after the release of jessie to get
right of SHA1 if we want to.
Note that the completely patched file is and was checked against the
hashes collected from the Release file, so this transition isn't mission
critical.
|
|
We are the only possible users of private methods, so we are also the
only users who can potentially export them via using them in inline
methods. The point is: We don't need these symbols exported if we don't
do this, so marking them as hidden removes some methods from the API
without breaking anything as nobody could have used them.
Git-Dch: Ignore
|
|
|
|
|
|
feature/acq-trans
Conflicts:
apt-pkg/acquire-item.cc
apt-pkg/acquire-item.h
methods/gpgv.cc
|
|
A long-lasting FIXME in the acquire code points out the problem that we
e.g. for decompressors assign c-string representations of c++-strings to
the Mode variable, which e.g. cppcheck points out as very bad.
In practice, nothing major happens as the c++-strings do not run out of
scope until Mode would do, but that is bad style and fragile, so the
obvious proper fix is to use a c++ string for storage to begin with.
The slight complications stems from the fact that progress reporting
code in frontends potentially uses Mode and compares it with NULL, which
can't be done with std::string, so instead of just changing the type we
introduce a new variable and deprecate the old one.
Git-Dch: Ignore
|
|
Also rework the way we load the Release file, so it only after
Release.gpg verified the Release file. The rational is that we
never want to load untrusted data into our parsers. Only stuff
verified with gpg or by its hashes get loaded. To load untrusted
data you now need to use apt-get update --allow-unauthenticated.
|
|
Revert because its a API change and the gain does not justify the
extra work to make the required changes in the consumers of this
interface at this point.
|
|
|
|
|
|
|
|
feature/acq-trans
Conflicts:
apt-pkg/acquire-item.cc
apt-pkg/acquire-item.h
methods/copy.cc
test/integration/test-hashsum-verification
|
|
Conflicts:
apt-pkg/acquire-item.cc
apt-pkg/acquire-item.h
apt-pkg/cachefilter.h
configure.ac
debian/changelog
|
|
|
|
|
|
incorrect invalidating of unauthenticated data (CVE-2014-0488)
incorect verification of 304 reply (CVE-2014-0487)
incorrect verification of Acquire::Gzip indexes (CVE-2014-0489)
|
|
|
|
|
|
|
|
|
|
|
|
|
