diff options
author | David Kalnischkies <david@kalnischkies.de> | 2019-11-27 12:10:31 +0100 |
---|---|---|
committer | David Kalnischkies <david@kalnischkies.de> | 2019-11-27 22:00:43 +0100 |
commit | 1690c3f87ae45a41e8d3e09bf0b1021c008460b9 (patch) | |
tree | 6b95d5a10f07be7e28188f9b0dcba8b08265701d | |
parent | 62bfe5b6ca3ccfba6313d3f9ab4cb75a24a5557a (diff) | |
download | apt-1690c3f87ae45a41e8d3e09bf0b1021c008460b9.tar.gz |
Remove failed trusted signature instead of index on IMS hit
While passing the combi Release and Release.gpg to the gpgv method for
verification the filename of Release is placed where usually Release.gpg
is assumed in the rest of the code. The "usual" cases like passing
verification and failing verification ending in an error are taking care
of this, but the code path dealing with a failed verification, but
ignoring said failure (e.g. due to trusted=yes) was not which results in
the wrong file being removed later on (in case the index happens to be
unmodified since the last update call) leading us into the abyss of
strange failures (fixed in the previous commit) were nothing should have
changed.
This is not a security issue in this form as the repository needs to fail
verification & the user forcing apt to ignore the failure and carry on
anyhow. It does show however how complicated the code and its various
interconnected paths can become.
Reported-By: Val "pinkieval" Lorentz on IRC
-rw-r--r-- | apt-pkg/acquire-item.cc | 5 | ||||
-rwxr-xr-x | test/integration/test-apt-update-repeated-ims-hit | 2 |
2 files changed, 7 insertions, 0 deletions
diff --git a/apt-pkg/acquire-item.cc b/apt-pkg/acquire-item.cc index 062b39cad..92931d1d7 100644 --- a/apt-pkg/acquire-item.cc +++ b/apt-pkg/acquire-item.cc @@ -2224,6 +2224,11 @@ void pkgAcqMetaSig::Failed(string const &Message,pkgAcquire::MethodConfig const return; // ensures that a Release.gpg file in the lists/ is removed by the transaction + if (not MetaIndexFileSignature.empty()) + { + DestFile = MetaIndexFileSignature; + MetaIndexFileSignature.clear(); + } TransactionManager->TransactionStageRemoval(this, DestFile); // only allow going further if the user explicitly wants it diff --git a/test/integration/test-apt-update-repeated-ims-hit b/test/integration/test-apt-update-repeated-ims-hit index 8630ff5ee..74d46b31b 100755 --- a/test/integration/test-apt-update-repeated-ims-hit +++ b/test/integration/test-apt-update-repeated-ims-hit @@ -53,11 +53,13 @@ testfailure grep 'aptarchive_Release.gpg$' lists.before for i in $(seq 1 3); do msgmsg 'Running update again does not change result' "$i" testwarningmsg "$GPGERROR" apt update + testfileequal lists.before "$(listcurrentlistsdirectory)" done find rootdir/var/lib/apt/lists -name '*Release*' -delete msgmsg 'Running update with a repository gaining hashsums' testwarningmsg "$GPGERROR" apt update +testfileequal lists.before "$(listcurrentlistsdirectory)" changetowebserver find aptarchive -name '*Release*' -delete |