| Age | Commit message (Collapse) | Author |
|---|
|
Earlier gcc versions used to complain that you should add them althrough
there isn't a lot of point to it if you think about it, but now gcc (>= 8)
complains about the attribute being present.
warning: ‘pure’ attribute on function returning ‘void’ [-Wattributes]
Reported-By: gcc -Wattributes
Gbp-Dch: Ignore
|
|
For deb files we always supported falling back from one server to the
other if one failed to download the deb, but that was hardwired in the
handling of this specific item. Moving this alongside the retry
infrastructure we can implement it for all items and allow methods to
use this as well by providing additional URIs in a redirect.
|
|
We have quite a bit of metadata available for the files we acquire, but
the methods weren't told about it and got just the URI. That is indeed
fine for most, but to avoid methods trying to parse the metadata out of
the provided URIs (and fail horribly in edgecases) we can just as well
be nice and tell them stuff directly.
|
|
Moving the Retry-implementation from individual items to the worker
implementation not only gives every file retry capability instead of
just a selected few but also avoids needing to implement it in each item
(incorrectly).
|
|
gcc has problems understanding this construct and additionally thinks it
would produce multiple lines and stuff, so to keep using it isn't really
worth it for the few instances we have: We can just write the long form
there which works better.
Reported-By: gcc
Gbp-Dch: Ignore
|
|
If a InRelease file fails to download with a non-404 error
we assumed there is some general problem with repository like
a webportal or your are blocked from access (wrong auth, Tor, …).
Turns out some server like S3 return 403 if a file doesn't exist.
Allowing this in general seems like a step backwards as 403 is a
reasonable response if auth failed, so failing here seems better
than letting those users run into problems.
What we can do is show our insecure warnings through and allow the
failures for insecure repos: If the repo is signed it is easy to add
an InRelease file and if not you are setup for trouble anyhow.
References: cbbf185c3c55effe47f218a07e7b1f324973a8a6
|
|
Regression-Of: cc1f94c95373670fdfdb8e2d6cf9125181f7df0c
|
|
As a follow up to the last commit, let's replace APT_CONST
with APT_PURE everywhere to clean stuff up.
|
|
The comment says this is intended, but looking at the history reveals
that the comment comes from a different era. Nowadays we don't really
need it anymore (and even back then it was disputeable) as we haven't
used that file for our update in the end and nothing really needs this
file after the update.
Triggered is this by 188f297a2af4c15cb1d502360d1e478644b5b810 which
moves various error conditions forward including this code expecting the
file to exist – but it doesn't need to as download could have failed.
We could fix that by simple checking if the file exists and only stage
it if it does, but instead we don't stage it and instead even rename it
out of the way with our conventional FAILED name (if it exists).
That restores support for partial mirrors (= in this case mirrors which
don't ship pdiff files). Note that apt heals itself even if only such a
mirror is used as the update is successful even if that error is shown.
Closes: 869425
|
|
RenameOnError does the rename already, so the check for existence will
always fail making this some completely harmles but also completely
pointless two lines of code we are better of removing.
Gbp-Dch: Ignore
|
|
Minor grammar fix
[jak@d.o: Fixed up po/]
|
|
This makes it easier to see which headers includes what.
The changes were done by running
git grep -l '#\s*include' \
| grep -E '.(cc|h)$' \
| xargs sed -i -E 's/(^\s*)#(\s*)include/\1#\2 include/'
To modify all include lines by adding a space, and then running
./git-clang-format.sh.
|
|
Including cacheiterators.h before pkgcache.h fails because
pkgcache.h depends on cacheiterators.h.
|
|
Progress only shows if we have an idea of how much files we will
acquire, but if a transaction fails before we have got an idea we ended
up never showing progress even through we know that a failed transaction
will not download additional files.
|
|
Having messages being printed on the error stack and confirm them by
commandline flags is an okayish first step, but some frontends will
probably want to have a more interactive feeling here with a proper
question the user can just press yes/no for as for some frontends a
commandline flag makes no sense…
|
|
This gives the repository owner a chance to explain why this change was
needed – e.g. explaining the organisational changes or simply detailing
the changes in the new release made. Note that this URI is also shown
if the change is accepted, so it also draws attention to release notes
of minor updates (if users watch apt output closely).
|
|
The value of Origin, Label, Codename and co can be used in user
configuration from apts own pinning to unattended upgrades.
A repository changing this values can therefore have serious effects on
the behaviour of apt and other tools using these values.
In a first step we will generate error messages for these changes now
explaining the need for explicit confirmation and provide config options
and commandline flags to accept them.
|
|
There are very many HTTP errorcodes which indicate that the repository
isn't available at the moment or the connection has some kind of
problem. Given that we do not require Release files the result was that
these errors were ignored and the user presented with a message like
"Repository is no longer signed" which sends the user in the wrong
direction.
Instead of trying to figure out which http errorcodes indicate a global
problem we accept only 404 for ignoring and consider all the rest as
hard errors now causing us to stop instantly after the InRelease file
and print the errorcode (with short description from server) received.
|
|
Moving the code responsible for parsing the Index file from ::Done into
the slightly earlier ::VerifyDone allows us to still "fail" the download
if we can't make use of the Index for whatever reason, so that the
progress log correctly displays "Ign" instead of "Get" for the file.
This also makes quiet a few debug messages proper error messages (but
those are still hidden by default for Ign lines).
|
|
If we couldn't find an entry for a Sources file we would generate an
error while for a Packages file we would silently skip it due to
assuming it is missing because it is empty. We can do better by checking
if the repository declares that it supports a component we want to get
the file from and if not say so and hint at the user making a typo.
An example were this helps is mozilla.debian.net which dropped the
firefox-aurora component (as upstream did) meaning no upgrades until the
user notices manually that the repository doesn't provide packages
anymore. With this commit warnings are raised hopefully causing the user
to investigate what is wrong (sooner).
|
|
Modified the wording of an error message when a repository no longer has a release file.
|
|
It says SRCNAME_SRCVER, but the example just gives
the SRCVER part.
Reported-By: Nishanth Aravamudan (nacc) in #ubuntu-devel
|
|
Most of them in (old) code comments. The two instances of user visible
string changes the po files of the manpages are fixed up as well.
Gbp-Dch: Ignore
Reported-By: spellintian
|
|
In ad9416611ab83f7799f2dcb4bf7f3ef30e9fe6f8 we fall back to asking the
original mirror (e.g. a redirector) if we do not get the expected
result. This works for the indexes, but patches are a different beast
and much simpler. Adding this fallback code here seems like overkill as
they are usually right along their Index file, so actually forward the
relevant settings to the patch items which fixes pdiff support combined
with a redirector and partial mirrors as in such a situation the pdiff
patches would be 404 and the complete index would be downloaded.
|
|
We have the last Release file around for other checks, so its trivial to
look if the new Release file contains a new codename (e.g. the user has
"testing" in the sources and it flipped from stretch to buster).
Such a change can be okay and expected, but also be a hint of problems,
so a warning if we see it happen seems okay. We can only print it once
anyhow and frontends and co are likely to ignore/hide it.
|
|
A suite or codename entry in the Release file is checked against the
distribution field in the sources.list entry that lead to the download of that
Release file. This distribution entry can contain slashes in the distribution
field:
deb http://security.debian.org/debian wheezy/updates main
However, the Release file may only contain "wheezy" in the Codename field and
not "wheezy/updates". So a transformation needs to take place that removes the
last / and everything that comes after (e.g. "/updates"). This fails, however,
for valid cases like a reprepro snapshot where the given Codename contains
slashes but is perfectly fine and doesn't need to be transformed. Since that
transformation is essentially just a workaround for special cases like the
security repository, it should be checked if the literal Codename without any
transformations happened is valid and only if isn't the dist should be checked
against the transformated one.
This way special cases like security.debian.org are handled and reprepro
snapshots work too.
The initial patch was taken as insperationto move whole transformation
to CheckDist() which makes this method more accepting & easier to use
(but according to codesearch.d.n we are the only users anyhow).
Thanks: Lukas Anzinger for initial patch
Closes: 644610
|
|
Some people do not recognize the field value with such an arcane name
and/or expect it to refer to something different (e.g. #839257).
We can't just rename it internally as its an avoidance strategy as such
fieldname existed previously with less clear semantics, but we can spare
the general public from this implementation detail.
|
|
A user relying on the deprecated behaviour of apt-get to accept a source
with an unknown pubkey to install a package containing the key expects
that the following 'apt-get update' causes the source to be considered
as trusted, but in case the source hadn't changed in the meantime this
wasn't happening: The source kept being untrusted until the Release file
was changed.
This only effects sources not using InRelease and only apt-get, the apt
binary downright refuses this course of actions, but it is a common way
of adding external sources.
Closes: 838779
|
|
Employ a priority queue instead of a normal queue to hold
the items; and only add items to the running pipeline if
their priority is the same or higher than the priority
of items in the queue.
The priorities are designed for a 3 stage pipeline system:
In stage 1, all Release files and .diff/Index files are fetched. This
allows us to determine what files remain to be fetched, and thus
ensures a usable progress reporting.
In stage 2, all Pdiff patches are fetched, so we can apply them
in parallel with fetching other files in stage 3.
In stage 3, all other files are fetched (complete index files
such as Contents, Packages).
Performance improvements, mainly from fetching the pdiff patches
before complete files, so they can be applied in parallel:
For the 01 Sep 2016 03:35:23 UTC -> 02 Sep 2016 09:25:37 update
of Debian unstable and testing with Contents and appstream for
amd64 and i386, update time reduced from 37 seconds to 24-28
seconds.
Previously, apt would first download new DEP11 icon tarballs
and metadata files, causing the CPU to be idle. By fetching
the diffs in stage 2, we can now patch our contents and Packages
files while we are downloading the DEP11 stuff.
|
|
Without randomizing the order in which we download the index files we
leak needlessly information to the mirrors of which architecture is
native or foreign on this system. More importantly, we leak the order in
which description translations will be used which in most cases will e.g.
have the native tongue first.
Note that the leak effect in practice is limited as apt detects if a file
it wants to download is already available in the latest version from a
previous download and does not query the server in such cases. Combined
with the fact that Translation files are usually updated infrequently
and not all at the same time, so a mirror can never be sure if it got asked
about all files the user wants.
|
|
|
|
This is needed on BSD where root's default group is wheel, not
root.
|
|
This fixes issues with chroots, but the goal here was to get
the test suite working on systems without dpkg.
|
|
This allows other vendors to use different paths, or to build
your own APT in /opt for testing. Note that this uses + 1 in
some places, as the paths we receive are absolute, but we need
to strip of the initial /.
|
|
The C.UTF-8 locale is not portable, so we need to use C, otherwise
we crash on other systems. We can use std::locale::classic() for
that, which might also be a bit cheaper than using locale("C").
|
|
The bugreport shows a segfault caused by the code not doing the correct
magical dance to remove an item from inside a queue in all cases. We
could try hard to fix this, but it is actually better and also easier to
perform these checks (which cause instant failure) earlier so that they
haven't entered queue(s) yet, which in return makes cleanup trivial.
The result is that we actually end up failing "too early" as if we
wouldn't be careful download errors would be logged before that process
was even started. Not a problem for the acquire system, but likely to
confuse users and programs alike if they see the download process
producing errors before apt was technically allowed to do an acquire
(it didn't, so no violation, but it looks like it to the untrained eye).
Closes: 835195
|
|
In af81ab9030229b4ce6cbe28f0f0831d4896fda01 by-hash got implemented as a
special compression type for our usual index files like Packages.
Missing in this scheme was the special .diff/Index index file containing
the info about individual patches for this index file. Deriving from the
index file class directly we inherent the compression handling
infrastructure and in this way also by-hash nearly for free.
Closes: #824926
|
|
The URI we later want to modify to get the file via by-hash was unset in
case a file was only available uncompressed (which is usually not the
case) causing an acquire error.
|
|
In af81ab9030229b4ce6cbe28f0f0831d4896fda01 we implement by-hash as a
special compression type, which breaks this filesize setting as the code
is looking for a foobar.by-hash file then. Dealing this slightly gets
us the intended value. Note that this has no direct effect as this value
will be set in other ways, too, and could only effect progress reporting.
Gbp-Dch: Ignore
|
|
If 9b8034a9fd40b4d05075fda719e61f6eb4c45678 serves the Release files
from a partial mirror we will end up getting 404 for some of the
indexes. Instead of giving up, we will instead ignore our same
redirection mirror constrain and ask the redirection service as a
potential hashsum mismatch is better than keeping the certain 404 error.
|
|
Now that we have the redirections loopchecker centrally in our items we
can use it also to prevent internal redirections to loop caused by
bugs as in a few instances we get into the business of rewriting the URI
we will query by ourself as we predict we would see such a redirect
anyway. Our code has no bugs of course, hence no practical difference. ;)
Gbp-Dch: Ignore
|
|
Having the detection handled in specific (http) workers means that a
redirection loop over different hostnames isn't detected. Its also not a
good idea have this implement in each method independently even if it
would work
|
|
Followup of b58e2c7c56b1416a343e81f9f80cb1f02c128e25.
Still a regression of sorts of 8b79c94af7f7cf2e5e5342294bc6e5a908cacabf.
Closes: 832044
|
|
If another file in the transaction fails and hence dooms the transaction
we can end in a situation in which a -patched file (= rred writes the
result of the patching to it) remains in the partial/ directory.
The next apt call will perform the rred patching again and write its
result again to the -patched file, but instead of starting with an empty
file as intended it will override the content previously in the file
which has the same result if the new content happens to be longer than
the old content, but if it isn't parts of the old content remain in the
file which will pass verification as the new content written to it
matches the hashes and if the entire transaction passes the file will be
moved the lists/ directory where it might or might not trigger errors
depending on if the old content which remained forms a valid file
together with the new content.
This has no real security implications as no untrusted data is involved:
The old content consists of a base file which passed verification and a
bunch of patches which all passed multiple verifications as well, so the
old content isn't controllable by an attacker and the new one isn't
either (as the new content alone passes verification). So the best an
attacker can do is letting the user run into the same issue as in the
report.
Closes: #831762
|
|
We read the entire input file we want to patch anyhow, so we can also
calculate the hash for that file and compare it with what he had
expected it to be.
Note that this isn't really a security improvement as a) the file we
patch is trusted & b) if the input is incorrect, the result will hardly be
matching, so this is just for failing slightly earlier with a more
relevant error message (althrough, in terms of rred its ignored and
complete download attempt instead).
|
|
If a user explicitly requests the download of arch:all apt shouldn't get
in the way and perform its detection dance if arch:all packages are
(also) in arch:any files or not.
This e.g. allows setting arch=all on a source with such a field (or one
which doesn't support all at all, but has the arch:all files like Debian
itself ATM) to get only the arch:all packages from there instead of
behaving like a no-op.
Reported-By: Helmut Grohne on IRC
|
|
All apt versions support numeric as well as 3-character timezones just
fine and its actually hard to write code which doesn't "accidently"
accepts it. So why change? Documenting the Date/Valid-Until fields in
the Release file is easy to do in terms of referencing the
datetime format used e.g. in the Debian changelogs (policy §4.4). This
format specifies only the numeric timezones through, not the nowadays
obsolete 3-character ones, so in the interest of least surprise we should
use the same format even through it carries a small risk of regression
in other clients (which encounter repositories created with
apt-ftparchive).
In case it is really regressing in practice, the hidden option
-o APT::FTPArchive::Release::NumericTimezone=0
can be used to go back to good old UTC as timezone.
The EDSP and EIPP protocols use this 'new' format, the text interface
used to communicate with the acquire methods does not for compatibility
reasons even if none of our methods would be effected and I doubt any
other would (in these instances the timezone is 'GMT' as that is what
HTTP/1.1 requires). Note that this is only true for apt talking to
methods, (libapt-based) methods talking to apt will respond with the
'new' format. It is therefore strongly adviced to support both also in
method input.
|
|
In 3bdff17c894d0c3d0f813d358fc45d7a263f3552 we did it for the datetime
parsing, but we use the same style in the parsing for pdiff (where the
size of the file is in the middle of the three fields) so imbueing here
as well is a good idea.
|
|
Weak had no dedicated option before and Insecure and Downgrade were both
global options, which given the effect they all have on security is
rather bad. Setting them for individual repositories only isn't great
but at least slightly better and also more consistent with other
settings for repositories.
|
|
For "Hash Sum mismatch" that info doesn't make a whole lot of
difference, but for the new insufficient info message an indicator that
while this hashes are there and even match, they aren't enough from a
security standpoint.
|
