| Age | Commit message (Collapse) | Author |
|---|
|
It is perfectly valid behavior for a server to respond with
Connection: close eventually, even when pipelining. Turning
off pipelining due to that is wrong. For example, some Ubuntu
mirrors close the connection after 101 requests. If I have
more packages to install, only the first 101 would benefit
from pipelining.
This commit introduces a new check to only turn of pipelining
for future connections if the pipeline for this connection did
not have 3 successful fetches before, that should work quite well to
detect broken server/proxy combinations like in bug 832113.
(cherry picked from commit df696650b7a8c58bbd92e0e1619e956f21010a96)
LP: #1794957
(cherry picked from commit 3de7454c796f245371c33076ae01529d6d36d715)
|
|
This reverts commit dd547ebaffd2aceb42e2908f1d5f0ab386af6cb1.
LP: #1751225
|
|
APT considered any response with a Content-Length to have a
body, even if the value of the header was 0. A 0 length body
however, is equal to no body.
(cherry picked from commit d47fb34ae03566feec7fec6dccba80e45fa03e6f)
|
|
When APT was trying multiple addresses, any later error
somewhere else would be reported with ConnectionRefused
or ConnectionTimedOut as the FailReason because that
was set by early connect attempts. This causes APT to
handle the failures differently, leading to some weirdly
breaking test cases (like the changed one).
Add debugging to the previously failing test case so
we can find out when something goes wrong there again.
(cherry picked from commit d3a70c3e5ae68a0e5a3d4667dd1d0fc0887e6263)
|
|
An SRV record includes a portnumber to use with the host given, but apt
was ignoring the portnumber and instead used either the port given by
the user for the initial host or the default port for the service.
In practice the service usually runs on another host on the default
port, so it tends to work as intended and even if not and apt can't get
a connection there it will gracefully fallback to contacting the initial
host with the right port, so its a user invisible bug most of the time.
(cherry picked from commit 9bdc09016f9570389451dd619d7e878bfeaa91df)
|
|
This fixes issues with sourceforge where the redirector includes
such a Content-Range in a 302 redirect. Since we do not really know
what file is meant in a redirect, let's just ignore it for all
responses other than 416 and 206.
Maybe we should also get rid of the other errors, and just ignore
the field in those cases as well?
LP: #1657567
(cherry picked from commit 4759a702081297bde66982efed8b2b7fd39ca27c)
(cherry picked from commit b5d0e1be09fd07e693bae8046848059f578d029f)
|
|
rred can fail for a plentory of reasons, but its failure is usually
recoverable (Ign lines) so it shouldn't leak unrequested debug messages
to an observing user.
Closes: #850759
(cherry picked from commit 2984d7aec37e09b473c7b99f43d20622c25dc99d)
(cherry picked from commit d0a345d4a41802e9129b78d70aabd6239a3c651a)
|
|
(cherry picked from commit 49b91f6903804183dbe1abb12ce1f9803a3dee5f)
(cherry picked from commit 4034726bfa6d9835bca90c6b4cd276c2fba2aea8)
|
|
Curl requires URLs to be urlencoded. We are however giving it
undecoded URLs. This causes it go completely nuts if there is
a space in the URI, producing requests like:
GET /a file HTTP/1.1
which the servers then interpret as a GET request for "/a" with
HTTP version "file" or some other non-sense.
This works around the issue by encoding the path component of
the URL. I'm not sure if we should encode other parts of the URL
as well, this one seems to do the trick for the actual issue at
hand.
A more correct fix is to avoid the dequoting and (re-)quoting
of URLs when a redirect occurs / a new request is sent. That's
been on the radar for probably a year or two now, but nobody
bothered implementing that yet.
LP: #1651923
(cherry picked from commit 994515e689dcc5f963f5fed58284831750a5da03)
(cherry picked from commit 438b1d78b4c33d0a97406f0a7071e3c413dc0aa3)
|
|
In 105503b4b470c124bc0c271bd8a50e25ecbe9133 we got a warning implemented
for unreadable files which greatly improves the behavior of apt update
already as everything will work as long as we don't need the keys
included in these files. The behavior if they are needed is still
strange through as update will fail claiming missing keys and a manual
test (which the user will likely perform as root) will be successful.
Passing the new warning generated by apt-key through to apt is a bit
strange from an interface point of view, but basically duplicating the
warning code in multiple places doesn't feel right either. That means we
have no translation for the message through as apt-key has no i18n yet.
It also means that if the user has a bunch of sources each of them will
generate a warning for each unreadable file which could result in quite
a few duplicated warnings, but "too many" is better than none.
Closes: 834973
(cherry picked from commit 29c590951f812d9e9c4f17706e34f2c3315fb1f6)
|
|
Followup of b58e2c7c56b1416a343e81f9f80cb1f02c128e25.
Still a regression of sorts of 8b79c94af7f7cf2e5e5342294bc6e5a908cacabf.
Closes: 832044
(cherry picked from commit 7303e11ff28f920a6277c159aa46f80c007350bb)
|
|
Commit 3af3ac2f5ec007badeded46a94be2bd06b9917a2 (released in 1.3~pre1)
implements proper fallback for SRV, but that works actually too good
as the RFC defines that such an SRV record should indicate that the
server doesn't provide this service and apt should respect this.
The solution is hence to fail again as requested even if that isn't what
the user (and perhaps even the server admins) wanted. At least we will
print a message now explicitly mentioning SRV to point people in the
right direction.
Reported-In: https://bugs.kali.org/view.php?id=3525
Reported-By: Raphaël Hertzog
(cherry picked from commit 99fdd8034b4a5cdb0100a33d0b3d5e26079c1695)
|
|
memcpy is marked as nonnull for its input, but ignores the input anyhow
if the declared length is zero. Our SHA2 implementations do this as
well, it was "just" MD5 and SHA1 missing, so we add the length check
here as well as along the callstack as it is really pointless to do all
these method calls for "nothing".
Reported-By: gcc -fsanitize=undefined
(cherry picked from commit 644478e8db56f305601c3628a74e53de048b28c8)
|
|
If a server closes a connection after sending us a file that tends to
mean that its a type of server who always closes the connection – it is
therefore relatively pointless to try pipelining with it even if it
isn't a problem by itself: apt is just restarting the pipeline each
time after it got served one file and the connection is closed.
The problem starts if one or more proxies are between the server and apt
and they disagree about how the connection should be as in the
bugreporters case where the responses apt gets contain both Keep-Alive
and Proxy-Connection headers (which apt both ignores) indicating a
proxy is trying to keep a connection open while the response also
contains "Connection: close" indicating the opposite which apt
understands and respects as it is required to do.
We avoid stepping into this abyss by not performing pipelining anymore
if we got a respond with the indication to close connection if the
response was otherwise a success – error messages are sent by some
servers via this method as their pages tend to be created dynamically
and hence their size isn't known a priori to them.
Closes: #832113
(cherry picked from commit 9714d522056e5256f5a2de587d88eba7cb3291c2)
|
|
It seems completely pointless from a server-POV to sent empty header
fields, so most of them don't do it (simply proven by this limitation
existing since day one) – but it is technically allowed by the RFC as
the surounding whitespaces are optional and Github seems to like sending
"X-Geo-Block-List:\r\n" since recently (bug reports in other http
clients indicate July) at least sometimes as the reporter claims to have
seen it on https only even through it can happen with both.
Closes: 834048
(cherry picked from commit 148c049150cc39f2e40894c1684dc2aefea1117e)
|
|
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
(cherry picked from commit 0e071dfe205ad21d8b929b4bb8164b008dc7c474)
|
|
Also fixes message itself to mention the correct option name as noticed
in #832113.
(cherry picked from commit b9c20219dc17db1d29eaf297263a4b008bd1b90b)
|
|
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).
(cherry picked from commit 6e71ec6fcdcaa926c98fa58cd4af38e42556df15)
|
|
Instead of only trying the first host we get via SRV, we try them all as
we are supposed to and if that isn't working we try to connect to the
host itself as if we hadn't seen any SRV records. This was already the
intend of the old code, but it failed to hide earlier problems for the
next call, which would unconditionally fail then resulting in an all
around failure to connect. With proper stacking we can also keep the
error messages of each call around (and in the order tried) so if the
entire connection fails we can report all the things we have tried while
we discard the entire stack if something works out in the end.
(cherry picked from commit 3af3ac2f5ec007badeded46a94be2bd06b9917a2)
|
|
If we don't give a specific error to report up it is likely that all
error currently in the error stack are equally important, so reporting
just one could turn out to be confusing e.g. if name resolution failed
in a SRV record list.
(cherry picked from commit b50dfa6b2dd2d459e0c2746ac9367982b96ffac0)
|
|
If we have files in partial/ from a previous invocation or similar such
those could be symlinks created by file:// sources. The code is
expecting only real files through and happily changes owner,
modification times and permission on the file the symlink points to
which tend to be files we have no business in touching in this way.
Permissions of symlinks shouldn't be changed, changing owner is usually
pointless to, but just to be sure we pick the easy way out and use
lchown, check for symlinks before chmod/utimes.
Reported-By: Mattia Rizzolo on IRC
(cherry picked from commit 3465138575e1fd0d5892d9b6be1ae232eb873460)
|
|
methods/http.cc:640:13: runtime error: reference binding to null pointer
of type 'struct FileFd'
This reference is never used in the cases it has a nullptr, so the
practical difference is non-existent, but its a bug still.
Reported-By: gcc -fsanitize=undefined
(cherry picked from commit 4460551841d909d3ee9c1de00156ed3cdf8b1665)
|
|
Seen in #828011 if we fail to parse a header field like Last-Modified we
end up interpreting the data as response header for coming requests in
case we don't rotate to a new server in DNS rotation.
(cherry picked from commit cc0a4c82b3c132abba9b9ec35fd61bc8b45a1b80)
|
|
wu-ftpd sends the response without parens, whereas we expect
them.
I did not test the patch, but it should work. I added another
return true if Pos is still npos after the second find to make
sure we don't add npos to the string.
Thanks: Lukasz Stelmach for the initial patch
Closes: #420940
(cherry picked from commit 25a694165ae46c159e0d91bf0b27717f00dbc66e)
|
|
Setting the C++ locale via std::locale::global(std::locale("")); which
would otherwise default to the default C locale (aka: unaffected by
setlocale) effects the formatting of numeric types in IO streams, which
for output for humans is perfectly sensible, but breaks our many text
interfaces used and parsed by us and others without expecting the
numbers to be formatted.
Closes: #825396
(cherry picked from commit b58e2c7c56b1416a343e81f9f80cb1f02c128e25)
|
|
Daniel Kahn Gillmor highlights in the bugreport that security isn't
improving by having the user import additional keys – especially as
importing keys securely is hard.
The bugreport was initially about dropping the warning to a notice, but
in given the previously mentioned observation and the fact that we
weren't printing a warning (or a notice) for expired or revoked keys
providing a signature we drop it completely as the code to display a
message if this was the only key is in another path – and is considered
critical.
Closes: 618445
(Backported from commit fb7b11ebb852fa255053ecab605bc9cfe9de0603)
|
|
when using the https transport mechanism, $no_proxy is ignored if apt is
getting it's proxy information from $https_proxy (as opposed to
Acquire::https::Proxy somewhere in apt config). if the source of proxy
information is Acquire::https::Proxy set in apt.conf (or apt.conf.d),
then $no_proxy is honored.
(cherry picked from commit 8707edd9e4684ed68856cd8eeff15ebd1e8c88ea)
|
|
With the previous fix for file applied we can again hit repositories
which contain uncompressed empty files, which since the introduction of
the central store: method wasn't accounted for anymore as we forbid
empty compressed files.
|
|
A silly of-by-one error in the stripping of the extension to check for
the uncompressed filename broken in an attempt to support all
compressions in commit a09f6eb8fc67cd2d836019f448f18580396185e5.
Fixing this highlights also mistakes in the handling of the Alt-Filename
in libapt which would cause apt to remove the file from the repository
(if root has the needed rights – aka the disk isn't readonly or similar)
|
|
Introduces APT::Hashes::<NAME> with entries Untrusted and Weak
which can be set to true to cause the hash to be treated as
untrusted and/or weak.
|
|
Our own gpgv method can declare a digest algorithm as untrusted and
handles these as worthless signatures. If gpgv comes with inbuilt
untrusted (which is called weak in official terminology) which it e.g.
does for MD5 in recent versions we should handle it in the same way.
To check this we use the most uncommon still fully trusted hash as a
configureable one via a hidden config option to toggle through all of
the three states a hash can be in.
|
|
Using erase(pos) is invalid in our case here as pos must be a valid and
derefenceable iterator, which isn't the case for an end-iterator (like
if we had no good signature).
The problem runs deeper still through as VALIDSIG is a keyid while
GOODSIG is just a longid so comparing them will always fail.
Closes: 818910
|
|
There was a complaint that, in the previous message,
the key fingerprint could be mistaken for a SHA1 digest
due to the (SHA1) after it.
Gbp-Dch: ignore
|
|
This should be easy to extend in the future and allow us to simplify
the error handling cases somewhat.
Thanks: Ron Lee for wording suggestions
|
|
We will drop support for those in the future.
Also adjust the std::array to be a std::vector, as that's easier to
maintain.
|
|
This can be used by workers to send warnings to the main
program. The messages will be passed to _error->Warning()
by APT with the URI prepended.
We are not going to make that really public now, as the
interface might change a bit.
|
|
We added weak signatures to BadSigners, meaning that a Release file
signed by both a weak signature and a strong signature would be
rejected; preventing people from migrating from DSA to RSA keys
in a sane way.
Instead of using BadSigners, treat weak signatures like expired
keys: They are no good signatures, and they are worthless.
Gbp-Dch: ignore
|
|
This keeps a list of weak digest algorithms. For now, only MD5
is disabled, as SHA1 breaks to many repos.
|
|
This reverts commit 76a71a1237d22c1990efbc19ce0e02aacf572576.
That commit broke the test suite.
Gbp-Dch: ignore
|
|
ERRSIG is created whenever a key uses an unknown/weak digest
algorithm, for example. This allows us to report a more useful
error than just "unknown apt-key error.":
The following signatures were invalid: ERRSIG 13B00F1FD2C19886 1 2 01 1457609403 5
While still not being the best reportable error message, it's
better than unknown apt-key error and hopefully redirects users
to complain to their repository owners.
|
|
We basically ignored errors from writing and flushing, let's
not do that.
|
|
Reported-By: cppcheck
Git-Dch: Ignore
|
|
Just enabling it for anyone breaks with HTTP/1.0 servers and
proxies sometimes.
Closes: #810796
|
|
There is no reason to enforce that the file we start the bootstrap with
is compressed with a compressor which is available online. This allows
us to change the on-disk format as well as deals with repositories
adding/removing support for a specific compressor.
|
|
Adding a new compressor method meant adding a new method as well – even
if that boilt down to just linking to our generalized decompressor with
a new name. That is unneeded busywork if we can instead just call the
generalized decompressor and let it figure out which compressor to use
based on the filenames rather than by program name.
For compatibility we ship still 'gzip', 'bzip2' and co, but they are
just links to our "new" 'store' method.
|
|
Remove the SingleInstance flag so we can use the new randomized
queue feature to run parallel.
|
|
The PopFromSrvRecs() already removed the entry from the active
list, so the extra SrvRecords.erase() was incorrect.
Git-Dch: ignore
|
|
Buffered writes improve performance a lot, given that we spent
about 78% of the time in _write.
|
|
This accidentally slipped in in a previous commit, but it should
be used only for testing mode.
Reported-By: David Kalnischkies <david@kalnischkies.de>
|
|
This converts all callers that read machine-generated data,
callers that might work with user input are not converted.
|
