From 8eafc759544298211cd0bfaa3919afc0fadd47d1 Mon Sep 17 00:00:00 2001 From: David Kalnischkies Date: Wed, 13 May 2015 16:09:12 +0200 Subject: detect Releasefile IMS hits even if the server doesn't Not all servers we are talking to support If-Modified-Since and some are not even sending Last-Modified for us, so in an effort to detect such hits we run a hashsum check on the 'old' compared to the 'new' file, we got the hashes for the 'new' already for "free" from the methods anyway and hence just need to calculated the old ones. This allows us to detect hits even with unsupported servers, which in turn means we benefit from all the new hit behavior also here. --- apt-pkg/acquire-item.cc | 31 ++++++++++---- apt-pkg/acquire-item.h | 2 +- apt-pkg/acquire-worker.cc | 8 ++-- methods/https.cc | 2 +- test/integration/framework | 9 ++++ test/integration/test-apt-update-expected-size | 1 + test/integration/test-apt-update-not-modified | 58 +++++++++++++++++++++++++- test/integration/test-apt-update-rollback | 1 + test/integration/test-apt-update-transactions | 2 + 9 files changed, 99 insertions(+), 15 deletions(-) diff --git a/apt-pkg/acquire-item.cc b/apt-pkg/acquire-item.cc index 1090912f5..78dace12c 100644 --- a/apt-pkg/acquire-item.cc +++ b/apt-pkg/acquire-item.cc @@ -253,7 +253,10 @@ bool pkgAcquire::Item::Rename(string From,string To) strprintf(S, _("rename failed, %s (%s -> %s)."), strerror(errno), From.c_str(),To.c_str()); Status = StatError; - ErrorText += S; + if (ErrorText.empty()) + ErrorText = S; + else + ErrorText = ErrorText + ": " + S; return false; } /*}}}*/ @@ -1794,7 +1797,7 @@ void pkgAcqMetaSig::Done(string Message,unsigned long long Size, if(AuthPass == false) { - if(CheckDownloadDone(Message) == true) + if(CheckDownloadDone(Message, Hashes) == true) { // destfile will be modified to point to MetaIndexFile for the // gpgv method, so we need to save it here @@ -1837,7 +1840,8 @@ void pkgAcqMetaSig::Failed(string Message,pkgAcquire::MethodConfig *Cnf)/*{{{*/ Status = StatDone; } else { _error->Error("%s", downgrade_msg.c_str()); - Rename(MetaIndexFile, MetaIndexFile+".FAILED"); + if (TransactionManager->IMSHit == false) + Rename(MetaIndexFile, MetaIndexFile+".FAILED"); Item::Failed("Message: " + downgrade_msg, Cnf); TransactionManager->AbortTransaction(); return; @@ -1922,12 +1926,12 @@ void pkgAcqMetaIndex::Done(string Message,unsigned long long Size, /*{{{*/ { Item::Done(Message,Size,Hashes,Cfg); - if(CheckDownloadDone(Message)) + if(CheckDownloadDone(Message, Hashes)) { // we have a Release file, now download the Signature, all further // verify/queue for additional downloads will be done in the // pkgAcqMetaSig::Done() code - std::string MetaIndexFile = DestFile; + std::string const MetaIndexFile = DestFile; new pkgAcqMetaSig(Owner, TransactionManager, MetaIndexSigURI, MetaIndexSigURIDesc, MetaIndexSigShortDesc, MetaIndexFile, IndexTargets, @@ -2008,7 +2012,7 @@ void pkgAcqMetaBase::QueueForSignatureVerify(const std::string &MetaIndexFile, } /*}}}*/ // pkgAcqMetaBase::CheckDownloadDone /*{{{*/ -bool pkgAcqMetaBase::CheckDownloadDone(const std::string &Message) +bool pkgAcqMetaBase::CheckDownloadDone(const std::string &Message, HashStringList const &Hashes) { // We have just finished downloading a Release file (it is not // verified yet) @@ -2031,7 +2035,18 @@ bool pkgAcqMetaBase::CheckDownloadDone(const std::string &Message) // make sure to verify against the right file on I-M-S hit IMSHit = StringToBool(LookupTag(Message,"IMS-Hit"),false); - if(IMSHit) + if (IMSHit == false) + { + // detect IMS-Hits servers haven't detected by Hash comparison + std::string FinalFile = GetFinalFilename(); + if (RealFileExists(FinalFile) && Hashes.VerifyFile(FinalFile) == true) + { + IMSHit = true; + unlink(DestFile.c_str()); + } + } + + if(IMSHit == true) { // for simplicity, the transaction manager is always InRelease // even if it doesn't exist. @@ -2273,7 +2288,7 @@ void pkgAcqMetaClearSig::Done(std::string Message,unsigned long long Size, if(AuthPass == false) { - if(CheckDownloadDone(Message) == true) + if(CheckDownloadDone(Message, Hashes) == true) QueueForSignatureVerify(DestFile, DestFile); return; } diff --git a/apt-pkg/acquire-item.h b/apt-pkg/acquire-item.h index 33a28671c..646de8416 100644 --- a/apt-pkg/acquire-item.h +++ b/apt-pkg/acquire-item.h @@ -429,7 +429,7 @@ class pkgAcqMetaBase : public pkgAcquire::Item /*{{{*/ * \param Message The message block received from the fetch * subprocess. */ - bool CheckDownloadDone(const std::string &Message); + bool CheckDownloadDone(const std::string &Message, HashStringList const &Hashes); /** \brief Queue the downloaded Signature for verification */ void QueueForSignatureVerify(const std::string &MetaIndexFile, diff --git a/apt-pkg/acquire-worker.cc b/apt-pkg/acquire-worker.cc index bee01e620..9254e20a3 100644 --- a/apt-pkg/acquire-worker.cc +++ b/apt-pkg/acquire-worker.cc @@ -333,13 +333,12 @@ bool pkgAcquire::Worker::RunMessages() // see if there is a hash to verify HashStringList ReceivedHashes; - HashStringList expectedHashes = Owner->HashSums(); - for (HashStringList::const_iterator hs = expectedHashes.begin(); hs != expectedHashes.end(); ++hs) + for (char const * const * type = HashString::SupportedHashes(); *type != NULL; ++type) { - std::string const tagname = hs->HashType() + "-Hash"; + std::string const tagname = std::string(*type) + "-Hash"; std::string const hashsum = LookupTag(Message, tagname.c_str()); if (hashsum.empty() == false) - ReceivedHashes.push_back(HashString(hs->HashType(), hashsum)); + ReceivedHashes.push_back(HashString(*type, hashsum)); } if(_config->FindB("Debug::pkgAcquire::Auth", false) == true) @@ -349,6 +348,7 @@ bool pkgAcquire::Worker::RunMessages() for (HashStringList::const_iterator hs = ReceivedHashes.begin(); hs != ReceivedHashes.end(); ++hs) std::clog << "\t- " << hs->toStr() << std::endl; std::clog << "ExpectedHash:" << endl; + HashStringList expectedHashes = Owner->HashSums(); for (HashStringList::const_iterator hs = expectedHashes.begin(); hs != expectedHashes.end(); ++hs) std::clog << "\t- " << hs->toStr() << std::endl; std::clog << endl; diff --git a/methods/https.cc b/methods/https.cc index 712e9ee73..fa143439a 100644 --- a/methods/https.cc +++ b/methods/https.cc @@ -444,7 +444,7 @@ bool HttpsMethod::Fetch(FetchItem *Itm) char err[255]; snprintf(err, sizeof(err) - 1, "HttpError%i", Server->Result); SetFailReason(err); - _error->Error("%s", err); + _error->Error("%i %s", Server->Result, Server->Code); // unlink, no need keep 401/404 page content in partial/ unlink(File->Name().c_str()); return false; diff --git a/test/integration/framework b/test/integration/framework index 2a53e8365..8c8936ead 100644 --- a/test/integration/framework +++ b/test/integration/framework @@ -1005,6 +1005,15 @@ signreleasefiles() { msgdone "info" } +redatereleasefiles() { + local DATE="$(date -d "$1" '+%a, %d %b %Y %H:%M:%S %Z')" + for release in $(find aptarchive/ -name 'Release'); do + sed -i "s/^Date: .*$/Date: ${DATE}/" $release + touch -d "$DATE" $release + done + signreleasefiles "${2:-Joe Sixpack}" +} + webserverconfig() { local WEBSERVER="${3:-http://localhost:8080}" local NOCHECK=false diff --git a/test/integration/test-apt-update-expected-size b/test/integration/test-apt-update-expected-size index 7efccaa57..55a5da848 100755 --- a/test/integration/test-apt-update-expected-size +++ b/test/integration/test-apt-update-expected-size @@ -26,6 +26,7 @@ test_inreleasetoobig() { } test_packagestoobig() { + redatereleasefiles '+1hour' # append junk at the end of the Packages.gz/Packages SIZE="$(stat --printf=%s aptarchive/dists/unstable/main/binary-i386/Packages)" find aptarchive/dists -name 'Packages*' | while read pkg; do diff --git a/test/integration/test-apt-update-not-modified b/test/integration/test-apt-update-not-modified index 2dc56e76c..a67ecb760 100755 --- a/test/integration/test-apt-update-not-modified +++ b/test/integration/test-apt-update-not-modified @@ -12,7 +12,7 @@ insertpackage 'unstable' 'apt' 'all' '1.0' setupaptarchive --no-update methodtest() { - msgmsg 'Test with' "$1" + msgmsg 'Test InRelease with' "$1" rm -rf rootdir/var/lib/apt/lists # get our cache populated testsuccess aptget update @@ -36,6 +36,62 @@ Reading package lists..." aptget update Get:1 $1 unstable/main amd64 Packages [$(stat -c '%s' 'aptarchive/dists/unstable/main/binary-amd64/Packages.gz') B] Reading package lists..." aptget update testfileequal 'listsdir.lst' "$(listcurrentlistsdirectory)" + + webserverconfig 'aptwebserver::support::modified-since' 'false' + webserverconfig 'aptwebserver::support::last-modified' 'false' + testsuccessequal "Get:1 $1 unstable InRelease [$(stat -c '%s' 'aptarchive/dists/unstable/InRelease') B] +Reading package lists..." aptget update + webserverconfig 'aptwebserver::support::modified-since' 'true' + webserverconfig 'aptwebserver::support::last-modified' 'true' + + msgmsg 'Test Release.gpg with' "$1" + rm -rf rootdir/var/lib/apt/lists + cp -a aptarchive/dists aptarchive/dists.good + find aptarchive/dists -name 'InRelease' -delete + # get our cache populated + testsuccess aptget update + listcurrentlistsdirectory > listsdir.lst + + # hit again with a good cache + testsuccessequal "Ign $1 unstable InRelease + 404 Not Found +Hit $1 unstable Release +Hit $1 unstable Release.gpg +Reading package lists..." aptget update + testfileequal 'listsdir.lst' "$(listcurrentlistsdirectory)" + + # drop an architecture, which means the file should be gone now + configarchitecture 'i386' + sed '/_binary-amd64_Packages/ d' listsdir.lst > listsdir-without-amd64.lst + testsuccessequal "Ign $1 unstable InRelease + 404 Not Found +Hit $1 unstable Release +Hit $1 unstable Release.gpg +Reading package lists..." aptget update + testfileequal 'listsdir-without-amd64.lst' "$(listcurrentlistsdirectory)" + + # readd arch so its downloaded again + configarchitecture 'amd64' 'i386' + testsuccessequal "Ign $1 unstable InRelease + 404 Not Found +Hit $1 unstable Release +Hit $1 unstable Release.gpg +Get:1 $1 unstable/main amd64 Packages [$(stat -c '%s' 'aptarchive/dists/unstable/main/binary-amd64/Packages.gz') B] +Reading package lists..." aptget update + testfileequal 'listsdir.lst' "$(listcurrentlistsdirectory)" + + webserverconfig 'aptwebserver::support::modified-since' 'false' + webserverconfig 'aptwebserver::support::last-modified' 'false' + testsuccessequal "Ign $1 unstable InRelease + 404 Not Found +Get:1 $1 unstable Release [$(stat -c '%s' 'aptarchive/dists/unstable/Release') B] +Get:2 $1 unstable Release.gpg [$(stat -c '%s' 'aptarchive/dists/unstable/Release.gpg') B] +Reading package lists..." aptget update + webserverconfig 'aptwebserver::support::modified-since' 'true' + webserverconfig 'aptwebserver::support::last-modified' 'true' + + rm -rf aptarchive/dists + cp -a aptarchive/dists.good aptarchive/dists } changetowebserver diff --git a/test/integration/test-apt-update-rollback b/test/integration/test-apt-update-rollback index f4500b69d..29fe1ab56 100755 --- a/test/integration/test-apt-update-rollback +++ b/test/integration/test-apt-update-rollback @@ -158,6 +158,7 @@ test_inrelease_to_broken_gzip() { msgmsg "Test InRelease to broken gzip" start_with_good_inrelease + redatereleasefiles '+2hours' # append junk at the end of the compressed file echo "lala" >> $APTARCHIVE/dists/unstable/main/source/Sources.gz touch -d '+2min' $APTARCHIVE/dists/unstable/main/source/Sources.gz diff --git a/test/integration/test-apt-update-transactions b/test/integration/test-apt-update-transactions index 67dd633f9..63b318056 100755 --- a/test/integration/test-apt-update-transactions +++ b/test/integration/test-apt-update-transactions @@ -47,6 +47,7 @@ testrun() { testsetup() { msgmsg 'Test with no initial data over' "$1" + redatereleasefiles 'now' rm -rf rootdir/var/lib/apt/lists mkdir -p rootdir/var/lib/apt/lists/partial listcurrentlistsdirectory > listsdir.lst @@ -55,6 +56,7 @@ testsetup() { msgmsg 'Test with initial data over' "$1" testsuccess aptget update listcurrentlistsdirectory > listsdir.lst + redatereleasefiles '+1hour' testrun 'listsdir.lst' } -- cgit v1.2.3-70-g09d2