don't leak FD in AutoProxyDetect command return parsing
which accidentally made the proxy autodetection code also read
the scripts output on stderr, not only on stdout when it switched
the code from popen() to Popen().
VersionHash: Do not skip too long dependency lines
If the dependency line does not contain spaces in the repository
but does in the dpkg status file (because dpkg normalized the
dependency list), the dpkg line might be longer than the line
in the repository. If it now happens to be longer than 1024
characters, it would be skipped, causing the hashes to be
out of date.
Note that we have to bump the minor cache version again as
this changes the format slightly, and we might get mismatches
with an older src cache otherwise.
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.
try not to call memcpy with length 0 in hash calculations
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".
If the inner Base256ToNum() returned false, it did not set
Num to a new value, causing it to be uninitialized, and thus
might have caused the function to exit despite a good result.
Also document why the Res = Num, if (Res != Num) magic is done.
TagFile: Fix off-by-one errors in comment stripping
Adding 1 to the value of d->End - current makes restLength one byte
too long: If we pass memchr(current, ..., restLength) has thus
undefined behavior.
Also, reading the value of current has undefined behavior if
current >= d->End, not only for current > d->End:
Consider a string of length 1, that is d->End = d->Current + 1.
We can only read at d->Current + 0, but d->Current + 1 is beyond
the end of the string.
This probably caused several inexplicable build failures on hurd-i386
in the past, and just now caused a build failure on Ubuntu's amd64
builder.
Fix segfault and out-of-bounds read in Binary fields
If a Binary field contains one or more spaces before a comma, the
code produced a segmentation fault, as it accidentally set a pointer
to 0 instead of the value of the pointer.
If the comma is at the beginning of the field, the code would
create a binStartNext that points one element before the start
of the string, which is undefined behavior.
We also need to check that we do not exit the string during the
replacement of spaces before commas: A string of the form " ,"
would normally exit the boundary of the Buffer:
don't loop on pinning pkgs from absolute debs by regex
An absolute filename for a *.deb file starts with a /. A package with
the name of the file is inserted in the cache which is provided by the
"real" package for internal reasons. The pinning code detects a regex
based wildcard by having the regex start with /. That is no problem
as a / can not be included in a package name… expect that our virtual
filename package can and does.
We fix this two ways actually: First, a regex is only being considered a
regex if it also ends with / (we don't support flags). That stops our
problem with the virtual filename packages already, but to be sure we
also do not enter the loop if matcher and package name are equal.
It has to be noted that the creation of pins for virtual packages like
the here effected filename packages is pointless as only versions can be
pinned, but checking that a package is really purely virtual is too
costly compared to just creating an unused pin.
Instead of erroring out when receiving a SIGINT, let the
child deal with it - we'll error out anyway if the child
exits with an error or due to the signal. Also ignore
SIGQUIT, as system() ignores it.
This basically fixes Bug #832593, but: we are running
the hooks via sh -c. Some shells exit with a signal
error even if the command they are executing catches
the signal and exits successfully. So far, this has
been noticed on dash, which unfortunately, is our
default shell.
Example:
$ cat trap.sh
trap 'echo int' INT; sleep 10; exit 0
$ if dash -c ./trap.sh; then echo OK: $?; else echo FAIL: $?; fi
^Cint
FAIL: 130
$ if mksh -c ./trap.sh; then echo OK: $?; else echo FAIL: $?; fi
^Cint
OK: 0
$ if bash -c ./trap.sh; then echo OK: $?; else echo FAIL: $?; fi
^Cint
OK: 0
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.
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.
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.
drop incorrect const attribute from DirectoryExists
Since its existence in 2010 DirectoryExists was always marked with this
attribute, but for no real reason. Arguably a check for the existence of
the file is not modifying global state, so theoretically this shouldn't
be a problem. It is wrong from a logical point of view through as
between two calls the directory could be created so the promise we made
to the compiler that it could remove the second call would be wrong, so
API wise it is wrong.
It's a bit mysterious that this is only observeable on ppc64el and can be
fixed by reordering code ever so slightly, but in the end its more our
fault for adding this attribute than the compilers fault for doing
something silly based on the attribute.
pass --force-remove-essential to dpkg only if needed
APT (usually) knows which package is essential or not, so we can avoid
passing this force flag to dpkg unconditionally if the user hasn't
chosen a non-default essential handling obscuring the information.
if the FileFd failed already following calls should fail, too
There is no point in trying to perform Write/Read on a FileFd which
already failed as they aren't going to work as expected, so we should
make sure that they fail early on and hard.
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.
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).
The flush call is a no-op in most FileFd implementations so this isn't
as critical as it might sound as the only non-trivial implementation is
in the buffered writer, which tends not be used to buffer another
buffer…
keep trying with next if connection to a SRV host failed
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.
report all instead of first error up the acquire chain
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.
don't change owner/perms/times through file:// symlinks
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.
use the right key for compressor configuration dump
The generated dump output is incorrect in sofar as it uses the name as
the key for this compressor, but they don't need to be equal as is the
case if you force some of the inbuilt ones to be disabled as our testing
framework does it at times.
This is hidden from changelog as nobody will actually notice while
describing it in a few words make it sound like an important change…
As the volatile sources are parsed last they were sorted behind the
dpkg/status file and hence are treated as a downgrade, which isn't
really what you want to happen as from a user POV its an upgrade.
If we have a (e.g. locally built) deb file installed and do try to
install it again apt complained about this being a downgrade, but it
wasn't as it is the very same version… it was just confused into not
merging the versions together which looks like a downgrade then.
The same size assumption is usually good, but given that volatile files
are parsed last (even after the status file) the base assumption no
longer holds, but is easy to adept without actually changing anything in
practice.
protect only the latest same-source providers from autoremove
Traditionally all providers are protected providing something as apt
can't know which of them is actually really providing the functionality
for the user ensuring that we don't propose the removal of used stuff,
but that is of course also keeping stuff around which could be removed.
That can cause the collection of multiple old providers until the
provided package is itself no longer needed (e.g. out-of-tree kernel
modules). We combat this by marking providers only from the newest
source package version so that old providers built by older versions of
the same source package can be garbage collected.
more explicit MarkRequired algorithm code (part 2)
As the previous commit, this shouldn't change behavior at all, but
beside being more explicit and perhaps faster its also considerably
shorter (granted, mostly by if0-block elimination).
Piling everything in a single if statement always made my head wobble,
but it hasn't even a benefit as the most common case of a package which
isn't installed passes all of the old if and lands in the non-existent
else-part of the inner if. So beside a subjective cleanup of what goes
on this implementation should also be a bit faster.
factor out Pkg/DepIterator prettyprinters into own header
The old prettyprinters have only access to the struct they pretty print,
which isn't enough usually as we want to know for a package also a bit
of state information like which version is the candidate.
We therefore need to pull the DepCache into context and hence use a
temporary struct which is printed instead of the iterator itself.
write auto-bits before calling dpkg & again after if needed
Writing first means that even in the event of a power-failure the
autobit is saved for future processing instead of "forgotten" so that
the package is treated as manually installed.
In some cases we have to re-run the writing after dpkg is done through
as dpkg can let packages disappear and in such cases apt will move
autobits around (or in that case non-autobits) which we need to store.
if reading of autobit state failed, let write fail
If we can't read the old file we can't just move forward as that would
discard potentially discard old data (especially other fields). We let
it fail only after we are done writing the new file so a user has the
chance to look into and merge the new data (which is otherwise
discarded).
We deploy atomic renames for some files, but these renames also happen
if something about the file failed which isn't really the point of the
exercise…
Fix buffer overflow in debListParser::VersionHash()
If a package file is formatted in a way that that no space
follows a deprecated "<", we would reformat it to "<=" and
increase the length of the output by 1, which can break.
Under normal circumstances with "<=" this should not be an
issue.
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.
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.
ensure filesize of deb is included in the hashes list
Filesize is a silly hash all by itself, but in combination with others
it can be a strong opponent, so ensuring that it is in the list of
hashes and hence checked by the normal course of action the acquire
process takes is a good thing.
Dominic Benson [Mon, 20 Jun 2016 12:47:46 +0000 (13:47 +0100)]
Reinstate caching of file hashes in apt-ftparchive
Check for cached hash entries to determine which (if any) hash types
need to be generated for the current file.
In 1.0.9, each hash type was handled by a separate method, each of
which checked the cache. It looks like when these code paths were
unified (in a311fb96b84757ef8628e6a754232614a53b7891) the cache
checks were not incorporated into the new method.
Commands are probably better of always having output through as the
fall through to the generic proxy settings is likely not intended. As
documenting and implementing this more consistently is kind of a
regression through, it is split off into the next commit.
apt-key: change to / before find to satisfy its CWD needs
First seen on hurd, but easily reproducible on all systems by removing
the 'execution' bit from the current working directory and watching some
tests (mostly the no-output expecting tests) fail due to find printing:
"find: Failed to restore initial working directory: …"
Samuel Thibault says in the bugreport:
| To do its work, find first records the $PWD, then goes to
| /etc/apt/trusted.gpg.d/ to find the files, and then goes back to $PWD.
|
| On Linux, getting $PWD from the 700 directory happens to work by luck
| (POSIX says that getcwd can return [EACCES]: Search permission was denied
| for the current directory, or read or search permission was denied for a
| directory above the current directory in the file hierarchy). And going
| back to $PWD fails, and thus find returns 1, but at least it emitted its
| output.
|
| On Hurd, getting $PWD from the 700 directory fails, and find thus aborts
| immediately, without emitting any output, and thus no keyring is found.
|
| So, to summarize, the issue is that since apt-get update runs find as a
| non-root user, running it from a 700 directory breaks find.
Solved as suggested by changing to '/' before running find, with some
paranoia extra care taking to ensure the paths we give to find are really
absolute paths first (they really should, but TMPDIR=. or a similar
Dir::Etc::trustedparts setting could exist somewhere in the wild).
The commit takes also the opportunity to make these lines slightly less
error ignoring and the two find calls using (mostly) the same parameters.
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.
This fixes Debian/apt#13 and the launchpad bug listed below,
but is far more advanced. I went through private-cmndline.cc
and looked at the supported options.
fail instead of segfault on unreadable config files
The report mentions "apt list --upgradable", but there are others which
have inconsistent behavior ranging from segfaulting to doing something
with the partial (and hence incomplete) data. We had a recent report
about sources.list (#818628), this one mentions prefences, the obvious
next step is conf files… so the testcase is adapted to check for all
three in file and directory versions and run a bunch of commands each
time which should all have more or less the same behavior in such a case
(aka error out).
respect user pinning in M-A:same version (un)screwing
Using Pkg.CandVersion() here is wrong as its implementation will return
a candidate based just on the default policy settings ignoring user
preferences and otherwise set candidates (aka: it sidesteps the
pkgDepCache).
This causes M-A:same libraries to be detected as screwed even through
they aren't, so that they end up being kept back.
update: Run Post-Invoke-Success if not all sources failed
Failures can happen and APT regardless will do a partial cache
update anyway. Because APT ensures that the list directory is
in a sane state, it makes sense to also call success hooks if
success was only partial - otherwise it loses sync with APT.
Most importantly, this causes the appstream cache to be empty,
see launchpad bug #1562733.
This is somewhat overly optimistic though: As soon as any repository
has nonexisting optional files, the missing optional files are also
treated as success, which means a single broken repository without an
InRelease file still runs Success hooks, even though it really should
not.
don't sent uninstallable rc-only versions via EDSP
Versions which are only available in dpkg/status aren't installable and
apt doesn't pick them as candidate for this reason – for the same reason
such packages shouldn't be sent to an external solver via EDSP. The
packages are pinned to -1, but if the solver has strict pinning disabled
it could end up picking this version anyhow – which is a request apt can
not satisfy.
allow redirection for items without a space in the desc again
Broken in a4b8112b19763cbd2c12b81d55bc7d43a591d610.
If an item has a description which includes no space and is redirected
to another mirror the code which wants to rewrite the description
expects a space in there, but can't find it and the unguarded substr
command on the string will fail with an exception thrown…
don't show NO_PUBKEY warning if repo is signed by another key
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.
James McCoy [Wed, 20 Apr 2016 02:27:21 +0000 (22:27 -0400)]
deb822: Restore support for <multivalue>-{Add,Remove}
Redesign of multivalue options in 463c8d801595ce5ac94d7c032264820be7434232
caused the parser to look for <multivalue>{Add,Remove} (no hyphen)
instead of the expected <multivalue>-{Add,Remove}.
Patrick Cable [Wed, 27 Apr 2016 20:55:55 +0000 (16:55 -0400)]
refactored no_proxy code to work regardless of where https proxy is set
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.
allow uncompressed files to be empty in store again
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)
recheck Pre-Depends satisfaction in SmartConfigure
Regression introduced in commit 590f1923121815b36ef889033c1c416a23cbe9a2
(2011!) causing apt to not check if Pre-Depends are satisfied before
calling --configure. This managed to hide so perfectly well for years as
Pre-Depends aren't that common, apt prefers upgrading these packages
first and checks for satisfaction is already in SmartUnpack, so there
is only a small window of oppertunity to break a pre-dependency relation
(usually with an unpack).
Verified by logchecking with two provided status files in the buglog.
I would have liked to write a test, but I wasn't able to reach the needed
complexity to get apt to fail – but the change is small and reasonable,
so what could possible go wrong™, right?
It handy to be able to point apt at reading a compressed dpkg/status
file in debugging cases, which worked pre-1.1 but somewhere down the
line in the massive refactoring. Restoring this behavior in a central
place for all realfile index files instead of just for the status file.
(This has no effect on index files acquired from an archive – those are
handled by different classes and support compressed files just fine)
There is a good chance that the attempt will fail, but if a user
mentions certain packages explicitly on the commandline there is a
chance that this will consist of a broken system which is resolved
by upgrading more packages then just the mentioned.
This limitation was not effecting external resolvers.
With the previous commit we track the state of transactions, so we can
now use our knowledge to avoid processing data for a transaction which
was already closed (via an abort in this case).
This is needed as multiple independent processes are interacting in the
process, so there isn't a simple immediate full-engine stop and it would
also be bad to teach each and every item how to check if its manager
has failed subordinate and what to do in that case.
In the pdiff case, which deals (potentially) with many items during its
lifetime e.g. a hashsum mismatch in another file can abort the
transaction the file we try to patch via pdiff belongs to. This causes
some of the items (which are already done) to be aborted with it, but
items still in the process of acquisition continue in the processing and
will later try to use all the items together failing in strange ways as
cleanup already happened.
The chosen solution is to dry up the communication channels instead by
ignoring new requests for data acquisition, canceling requests which are
not assigned to a queue and not calling Done/Failed on items anymore.
This means that e.g. already started or pending (e.g. pipelined)
downloads aren't stopped and continue as normal for now, but they remain
in partial/ and aren't processed further so the next update command will
pick them up and put them to good use while the current process fails
updating (for this transaction group) in an orderly fashion.
Closes: 817240
Thanks: Barr Detwix & Vincent Lefevre for log files
We want to keep track of the state of a transaction overall to base
future decisions on it, but as a pre-requirement we have to make sure
that a transaction isn't commited twice (which happened if the download
of InRelease failed and Release takes over).
It also happened to create empty commits after a transaction was already
aborted in cases in which the Release files were rejected.
This isn't effecting security at the moment, but to ensure this isn't
happening again and can never be bad a bunch of fatal error messages are
added to make regressions on this front visible.
Hardly noticeable, but given that we have the option to easily enable
it, lets enable it as every newline in the message is written
individually by the code.
projects / apt.git / log
