From: David Kalnischkies Date: Wed, 24 Aug 2016 07:47:48 +0000 (+0200) Subject: do fail on weakhash/loop earlier in acquire X-Git-Tag: 1.3_rc3~17 X-Git-Url: https://git.saurik.com/apt.git/commitdiff_plain/2e2865ae53a65c00dd55a892d5b48458f3110366 do fail on weakhash/loop earlier in acquire 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 --- diff --git a/apt-pkg/acquire-item.cc b/apt-pkg/acquire-item.cc index 7abb7b206..7fbbf08f8 100644 --- a/apt-pkg/acquire-item.cc +++ b/apt-pkg/acquire-item.cc @@ -3279,9 +3279,8 @@ bool pkgAcqArchive::QueueNext() // Create the item Local = false; - QueueURI(Desc); - ++Vf; + QueueURI(Desc); return true; } return false; diff --git a/apt-pkg/acquire-worker.cc b/apt-pkg/acquire-worker.cc index a4fbc7651..5b6634db8 100644 --- a/apt-pkg/acquire-worker.cc +++ b/apt-pkg/acquire-worker.cc @@ -661,55 +661,15 @@ bool pkgAcquire::Worker::QueueItem(pkgAcquire::Queue::QItem *Item) if (OutFd == -1) return false; - HashStringList const hsl = Item->GetExpectedHashes(); - if (isDoomedItem(Item->Owner)) return true; - if (hsl.usable() == false && Item->Owner->HashesRequired() && - _config->Exists("Acquire::ForceHash") == false) - { - std::string const Message = "400 URI Failure" - "\nURI: " + Item->URI + - "\nFilename: " + Item->Owner->DestFile + - "\nFailReason: WeakHashSums"; - - auto const ItmOwners = Item->Owners; - for (auto &O: ItmOwners) - { - O->Status = pkgAcquire::Item::StatAuthError; - O->Failed(Message, Config); - if (Log != nullptr) - Log->Fail(O->GetItemDesc()); - } - // "queued" successfully, the item just instantly failed - return true; - } - - if (Item->Owner->IsRedirectionLoop(Item->URI)) - { - std::string const Message = "400 URI Failure" - "\nURI: " + Item->URI + - "\nFilename: " + Item->Owner->DestFile + - "\nFailReason: RedirectionLoop"; - - auto const ItmOwners = Item->Owners; - for (auto &O: ItmOwners) - { - O->Status = pkgAcquire::Item::StatError; - O->Failed(Message, Config); - if (Log != nullptr) - Log->Fail(O->GetItemDesc()); - } - // "queued" successfully, the item just instantly failed - return true; - } - string Message = "600 URI Acquire\n"; Message.reserve(300); Message += "URI: " + Item->URI; Message += "\nFilename: " + Item->Owner->DestFile; + HashStringList const hsl = Item->GetExpectedHashes(); for (HashStringList::const_iterator hs = hsl.begin(); hs != hsl.end(); ++hs) Message += "\nExpected-" + hs->HashType() + ": " + hs->HashValue(); diff --git a/apt-pkg/acquire.cc b/apt-pkg/acquire.cc index 1efb772b4..affc0c791 100644 --- a/apt-pkg/acquire.cc +++ b/apt-pkg/acquire.cc @@ -269,6 +269,42 @@ void pkgAcquire::Remove(Worker *Work) it is constructed which creates a queue (based on the current queue mode) and puts the item in that queue. If the system is running then the queue might be started. */ +static bool DoesAcquireResultInInstantFailure(pkgAcquire::Item * const Item, + pkgAcquire::MethodConfig const * const Config, pkgAcquireStatus * const Log) +{ + auto SavedDesc = Item->GetItemDesc(); + if (Item->IsRedirectionLoop(SavedDesc.URI)) + { + std::string const Message = "400 URI Failure" + "\nURI: " + SavedDesc.URI + + "\nFilename: " + Item->DestFile + + "\nFailReason: RedirectionLoop"; + + Item->Status = pkgAcquire::Item::StatError; + Item->Failed(Message, Config); + if (Log != nullptr) + Log->Fail(SavedDesc); + return true; + } + + HashStringList const hsl = Item->GetExpectedHashes(); + if (hsl.usable() == false && Item->HashesRequired() && + _config->Exists("Acquire::ForceHash") == false) + { + std::string const Message = "400 URI Failure" + "\nURI: " + SavedDesc.URI + + "\nFilename: " + Item->DestFile + + "\nFailReason: WeakHashSums"; + + auto SavedDesc = Item->GetItemDesc(); + Item->Status = pkgAcquire::Item::StatAuthError; + Item->Failed(Message, Config); + if (Log != nullptr) + Log->Fail(SavedDesc); + return true; + } + return false; +} void pkgAcquire::Enqueue(ItemDesc &Item) { // Determine which queue to put the item in @@ -277,6 +313,13 @@ void pkgAcquire::Enqueue(ItemDesc &Item) if (Name.empty() == true) return; + /* the check for running avoids that we produce errors + in logging before we actually have started, which would + be easier to implement but would confuse users/implementations + so we check the items skipped here in #Startup */ + if (Running && DoesAcquireResultInInstantFailure(Item.Owner, Config, Log)) + return; + // Find the queue structure Queue *I = Queues; for (; I != 0 && I->Name != Name; I = I->Next); @@ -912,10 +955,20 @@ bool pkgAcquire::Queue::Startup() if (Workers == 0) { URI U(Name); - pkgAcquire::MethodConfig *Cnf = Owner->GetConfig(U.Access); - if (Cnf == 0) + pkgAcquire::MethodConfig * const Cnf = Owner->GetConfig(U.Access); + if (unlikely(Cnf == nullptr)) return false; - + + // now-running twin of the pkgAcquire::Enqueue call + for (QItem *I = Items; I != 0; ) + { + bool pointless = false; + for (auto &&O: I->Owners) + if (DoesAcquireResultInInstantFailure(O, Cnf, Owner->Log)) + pointless = true; + I = pointless ? Items : I->Next; + } + Workers = new Worker(this,Cnf,Owner->Log); Owner->Add(Workers); if (Workers->Start() == false) diff --git a/test/integration/framework b/test/integration/framework index 1e356ffaf..d40c9f356 100644 --- a/test/integration/framework +++ b/test/integration/framework @@ -875,6 +875,7 @@ Filename: pool/main/${NAME}/${NAME}_${VERSION}_${arch}.deb" test -z "$DEPENDENCIES" || echo "$DEPENDENCIES" echo "Description: $(printf '%s' "$DESCRIPTION" | head -n 1)" echo "Description-md5: $(printf '%s' "$DESCRIPTION" | md5sum | cut -d' ' -f 1)" + echo "SHA256: 0000000000000000000000000000000000000000000000000000000000000000" echo } >> "${PPATH}/Packages" done diff --git a/test/integration/test-apt-redirect-loop b/test/integration/test-apt-redirect-loop index b54f74276..e1a8aeea0 100755 --- a/test/integration/test-apt-redirect-loop +++ b/test/integration/test-apt-redirect-loop @@ -7,9 +7,6 @@ TESTDIR="$(readlink -f "$(dirname "$0")")" setupenvironment configarchitecture "i386" -insertpackage 'stable' 'apt' 'all' '1' -setupaptarchive --no-update - echo 'alright' > aptarchive/working changetohttpswebserver webserverconfig 'aptwebserver::redirect::replace::/redirectme3/' '/redirectme/' diff --git a/test/integration/test-bug-605394-versioned-or-groups b/test/integration/test-bug-605394-versioned-or-groups index a362463d5..43f35f79b 100755 --- a/test/integration/test-bug-605394-versioned-or-groups +++ b/test/integration/test-bug-605394-versioned-or-groups @@ -24,3 +24,7 @@ if aptget dist-upgrade --trivial-only -o Debug::pkgProblemResolver=1 -o Debug::p else msgpass fi + +# the Packages file includes only MD5 +testfailure aptget dist-upgrade -y +testsuccess grep 'Insufficient information available to perform this download securely' rootdir/tmp/testfailure.output diff --git a/test/integration/test-bug-722207-print-uris-even-if-very-quiet b/test/integration/test-bug-722207-print-uris-even-if-very-quiet index d8d3c7218..82c1d715f 100755 --- a/test/integration/test-bug-722207-print-uris-even-if-very-quiet +++ b/test/integration/test-bug-722207-print-uris-even-if-very-quiet @@ -20,7 +20,7 @@ APTARCHIVE=$(readlink -f ./aptarchive) testsuccessequal "'file://${APTARCHIVE}/pool/main/apt/apt_2_all.deb' apt_2_all.deb 0 " aptget upgrade -qq --print-uris testsuccessequal "'file://${APTARCHIVE}/pool/main/apt/apt_2_all.deb' apt_2_all.deb 0 " aptget dist-upgrade -qq --print-uris testsuccessequal "'file://${APTARCHIVE}/pool/main/apt/apt_2_all.deb' apt_2_all.deb 0 " aptget install apt -qq --print-uris -testsuccessequal "'file://${APTARCHIVE}/pool/main/apt/apt_2_all.deb' apt_2_all.deb 0 " aptget download apt -qq --print-uris +testsuccessequal "'file://${APTARCHIVE}/pool/main/apt/apt_2_all.deb' apt_2_all.deb 0 SHA256:0000000000000000000000000000000000000000000000000000000000000000" aptget download apt -qq --print-uris testsuccessequal "'file://${APTARCHIVE}/apt_2.dsc' apt_2.dsc 9 SHA256:7776436a6d741497f1cd958014e1a05b352224231428152aae39da3c17fd2fd4 'file://${APTARCHIVE}/apt_2.tar.gz' apt_2.tar.gz 12 SHA256:f57f565eabe3fde0ec6e6e0bcc8db1d86fe2b4d6344a380a23520ddbb7728e99" aptget source apt -qq --print-uris testsuccessequal "'http://metadata.ftp-master.debian.org/changelogs/main/a/apt/apt_2_changelog' apt.changelog" aptget changelog apt -qq --print-uris diff --git a/test/integration/test-handle-redirect-as-used-mirror-change b/test/integration/test-handle-redirect-as-used-mirror-change index 2e8fbeff6..2655f713c 100755 --- a/test/integration/test-handle-redirect-as-used-mirror-change +++ b/test/integration/test-handle-redirect-as-used-mirror-change @@ -78,3 +78,9 @@ testsuccessequal "Ign:1 http://0.0.0.0:${APTHTTPPORT}/storage unstable InRelease 404 Not Found Hit:2 http://0.0.0.0:${APTHTTPPORT} unstable Release Reading package lists..." aptget update + +rm -rf rootdir/var/lib/apt/lists +find aptarchive -name 'Release.gpg' -delete +find aptarchive -name 'Release' -delete +testwarning aptget update +testsuccess grep 'does not have a Release file' rootdir/tmp/testwarning.output