Michael Vogt [Tue, 4 Nov 2014 13:47:59 +0000 (14:47 +0100)]
Call "Dequeue()" for items in AbortTransaction() to fix race
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.
The worker is the part closest to the methods, which will call the item
methods according to what it gets back from the methods, it is therefore
a better place to change permissions as it is very central and can do it
now at the point the item is assigned to a method rather than then it is
queued for download (and as before while dequeued via Done/Failure).
Central methods of our infrastructure like this one responsible for
communication with our methods shouldn't be more complicated then they
have to and not claim to have (albeit unlikely) bugs.
While I am not sure about having improved the first part, the bug is now
gone and a few explicit tests check that it stays that way, so nobody
will notice the difference (hopefully) – expect that this should a very
tiny bit faster as well as we don't manually proceed through the string.
It is a very simple hashstring, which is why it isn't contributing to
the usability of a list of them, but it is also trivial to check and
calculate, so it doesn't hurt checking it either as it can combined even
with the simplest other hashes greatly complicate attacks on them as you
suddenly need a same-size hash collision, which is usually a lot harder
to achieve.
tests: support 'installed' release in insertpackage
It is sometimes handy to have an installed package also in the archive,
but this was until now harder than it should as you had to duplicate the
lines, which is especially dangerous while writing the tests as it
easily happens that these two lines divert and so the same-but-different
version detection kicks in.
We can use either and some tests exercise this, but the default should
be what we want to use and that is a split out long description file
which is properly mentioned in the Release file.
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.
Private temporary directories as created by e.g. libpam-tmpdir are nice,
but they are also very effective in preventing our priviledge dropping
to work as TMPDIR will be set to a directory only root has access to, so
working with it as _apt will fail. We circumvent this by extending our
check for a usable TMPDIR setting by checking access rights.
Adds a new testwarning which tests for zero exit and the presents of a
warning in the output, failing if either is not the case or if an error
is found, too. This allows us to change testsuccess to accept only
totally successful executions (= without warnings) which should help
finding regressions.
These functions check the exit code of the command, but for apt commands
we can go further and require an error message for non-zero exits and
none for zero exits.
autorun permission tests for all apt-get update calls
Adds some infrastructure to run tests automatically for certain
commands. The first command being 'apt-get update' (and 'apt update')
which check for correct permission and owner of the files in lists/.
aborted reverify restores file owner and permission
If we get an IMS hit for an InRelease file we use the file we already
have and pass it into reverification, but this changes the permissions
and on abort of the transaction they weren't switched back.
This is now done, additionally, every file in partial which hasn't
failed gets permission and owner changed for root access as well, as it
is very well possible that the next invocation will (re)use these files.
Transactions are run and completed from multiple places, so it happens
for unsigned repos that the Release file was commited even if it was
previously aborted (due to --no-allow-insecure-repositories). The reason
is simply that the "failure" of getting an InRelease/Release.gpg is
currently ignored, so that the acquire process believes that nothing bad
happened and commits the transaction even though the same transaction
was previously aborted.
reenable support for -s (and co) in apt-get source
The conversion to accept only relevant options for commands has
forgotten another one, so adding it again even through the usecase might
very well be equally good served by --print-uris.
We do not support compressed indexes for cdrom sources as we rewrite
some of them, so supporting it correctly could be hard. What we do
instead in the meantime is probably disabling it for cdrom sources.
check for available space, excluding root reserved blocks
We are checking the space requirements for ages, but the check uses the
free blocks count, which includes the blocks reserved for usage by root.
Now that we use an unprivileged user it has no access to these blocks
anymore – and more importantly these blocks are a reserve, they
shouldn't be used by apt without special encouragement by the user as it
would be bad to have dpkg run out of diskspace and maintainerscripts
like man-db skip certain actions if not enough space is available
freely.
don't drop privileges if _apt has not enough rights
Privilege dropping breaks download/source/changelog commands as they
require the _apt user to have write permissions in the current directory,
which is e.g. the case in /tmp, but not in /root, so we disable the
privilege dropping if we deal with such a directory based on idea and
code by Michael Vogt.
The alternative would be to download always to a temp directory and move
it then done, but this breaks partial file support. To resolve this, we
could move to one of our partial/ directories, but this would require a
lock which would block root from using two of these commands in
parallel. As both seems unacceptable we instead let the user choose what
to do: Either a directory is setupped for _apt, downloading as root is
accepted or – which is potentially even better – an unprivileged user is
used for the commands.
Michael Vogt [Tue, 14 Oct 2014 15:00:56 +0000 (17:00 +0200)]
Add new configallowinsecurerepositories to the test framework
Add a new configallowinsecurerepositories that controls the value
of Acquire::AllowInsecureRepositories for the tests. Set it to
"false" for most of the testsuite and only enable it where its
really needed. We want to switch the default for this post-jessie.
do not load filesize in pkgAcqIndexTrans explicitly
The constructor is calling the baseclass pkgAcqIndex which does this
already – and also does it correctly for compressed files which would
overwise lead to the size of uncompressed files to be expected.
Michael Vogt [Mon, 13 Oct 2014 08:57:30 +0000 (10:57 +0200)]
Fix backward compatiblity of the new pkgAcquireMethod::DropPrivsOrDie()
Do not drop privileges in the methods when using a older version of
libapt that does not support the chown magic in partial/ yet. To
do this DropPrivileges() now will ignore a empty Apt::Sandbox::User.
trusted=yes sources are secure, we just don't know why
Do not require a special flag to be present to update trusted=yes
sources as this flag in the sources.list is obviously special enough.
Note that this is just disabling the error message, the user will still
be warned about all the (possible) failures the repository generated, it
is just triggering the acceptance of the warnings on a source-by-source
level.
Similarily, the trusted=no flag doesn't require the user to pass
additional flags to update, if the repository looks fine in the view of
apt it will update just fine. The unauthenticated warnings will "just" be
presented then the data is used.
In case you wonder: Both was the behavior in previous versions, too.
Reimplementing an inline method is opening a can of worms we don't want
to open if we ever want to us a d-pointer in those classes, so we do the
only thing which can save us from hell: move the destructors into the cc
sources and we are good.
Technically not an ABI break as the methods inline or not do the same
(nothing), so a program compiled against the old version still works
with the new version (beside that this version is still in experimental,
so nothing really has been build against this library anyway).
The same message is used for InRelease if fails in gpgv, but the
Release/Release.gpg duo needs to handle the failing download case as
well (InRelease just defers to the duo if download fails) and print a
message accompaning the insecure error to provide a hint on what is
going on.
make --allow-insecure-repositories message an error
Not using this option, but using unsigned (and co) repositories will
cause these repositories to be ignored and data acquiring from them
fails, so this is very well in the realms of an error and helps in
making 'apt-get update' fail with a non-zero error code as well.
Michael Vogt [Wed, 8 Oct 2014 18:13:56 +0000 (20:13 +0200)]
Only rename StatError files in AbortTransaction()
This fixes a race that we see in travis when two copy operations
finish at about the same time but the bad one first. This lead to
a rename of the good one and triggers a error when apt tries to
verify the good version but can no longer find it.
Remove long obsolete (hold, hold-reinstreq, removal-failed) or just
wrong (post-inst-failed vs postinst-failed) values, that have been
autoconverted by dpkg at run-time to their new equivalents, so there
should not be any such instance in any recent system (removal-failed
since dpkg 1.1.4 in Apr 1996, hold and hold-reinstreq since dpkg
1.2.0 in May 1996). dpkg even stopped doing the mapping in 1.15.4
and 1.15.8 respectively.
At the same time sort the list in the same order as they appear in
the dpkg code.
Michael Vogt [Wed, 8 Oct 2014 06:32:42 +0000 (08:32 +0200)]
Fix http pipeline messup detection
The Maximum-Size protection breaks the http pipeline reorder code
because it relies on that the object got fetched entirely so that
it can compare the hash of the downloaded data. So instead of
stopping when the Maximum-Size of the expected item is reached we
only stop when the maximum size of the biggest item in the queue
is reached. This way the pipeline reoder code keeps working.
On travis-ci connect.cc detects a rotation, triggering it store the IP
which is later appended to the error message, which is all nice and
great if we deal with a real server, but in the testcases it just
triggers failures as strings do not match.
Changing user and co works only as root, but can do some things for
methods run as normal user as well to protect them from being able to
call setuid binaries like sudo to elevate their privileges.
Also uses a cheap trick now to build with old unsupporting kernels.
Some distributions (or repositories) do not have as much
"Ign-discipline" as I would like to, so that could be pretty distracting
for our users if enabled by default. It is handy for testcases though.
Michael Vogt [Tue, 7 Oct 2014 18:40:37 +0000 (20:40 +0200)]
Add new Acquire::MaxReleaseFileSize=10*1000*1000 option
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)).
Michael Vogt [Tue, 7 Oct 2014 11:34:28 +0000 (13:34 +0200)]
Ignore EINVAL from prctl(PR_SET_NO_NEW_PRIVS)
Ignore a EINVAL error here as it means that the kernel is too old
to understand this option. We should not fail hard in this case
but just ignore the error.
consistently using Item::Failed in all specializec classes helps setting
up some information bits otherwise unset, so some errors had an empty
reason as an error. Ign is upgraded to display the error message we
ignored to further help in understanding what happens.
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.
ensure partial dirs are 0700 and owned by _apt:root
Reworks the API involved in creating and setting up the fetcher to be a
bit more pleasent to look at and work with as e.g. an empty string for
no lock isn't very nice. With the lock we can also stop creating all our
partial directories "just in case". This way we can also be a bit more
aggressive with the partial directory itself as with a lock, we know we
will gone need it.
Guillem Jover [Thu, 2 Oct 2014 15:48:13 +0000 (17:48 +0200)]
apt-get: Create the temporary downloaded changelog inside tmpdir
The code is creating a secure temporary directory, but then creates
the changelog alongside the tmpdir in the same base directory. This
defeats the secure tmpdir creation, making the filename predictable.
Inject a '/' between the tmpdir and the changelog filename.
projects / apt.git / log
