From f30976478e684fc19e48d71881805454ceb6ecae Mon Sep 17 00:00:00 2001 From: Michael Vogt Date: Mon, 6 Oct 2014 11:45:42 +0200 Subject: Rework pkgAcqMeta{Index,Sig,ClearSig}::Done() for readability Move common code out but do not use subclassing for ::Done to make it easier to understand what each class is doing when its done --- apt-pkg/acquire-item.cc | 272 ++++++++++++----------------------- apt-pkg/acquire-item.h | 64 +++++---- test/integration/test-apt-update-ims | 26 +++- 3 files changed, 149 insertions(+), 213 deletions(-) diff --git a/apt-pkg/acquire-item.cc b/apt-pkg/acquire-item.cc index d1cf53fd5..6c04288a8 100644 --- a/apt-pkg/acquire-item.cc +++ b/apt-pkg/acquire-item.cc @@ -1608,62 +1608,24 @@ void pkgAcqMetaSig::Done(string Message,unsigned long long Size, if(AuthPass == false) { - // queue for verify, note that we change DestFile here to point to - // the file we want to verify (needed to make gpgv work) - - string FileName = LookupTag(Message,"Filename"); - if (FileName.empty() == true) - { - Status = StatError; - ErrorText = "Method gave a blank filename"; - return; - } - - if (FileName != DestFile) - { - // We have to copy it into place - Local = true; - Desc.URI = "copy:" + FileName; - QueueURI(Desc); - return; - } - - if(StringToBool(LookupTag(Message,"IMS-Hit"),false) == true) + if(CheckDownloadDone(Message, RealURI) == true) { - IMSHit = true; - // adjust DestFile on i-m-s hit to the one we already have on disk - DestFile = _config->FindDir("Dir::State::lists"); - DestFile += URItoFileName(RealURI); + // destfile will be modified to point to MetaIndexFile for the + // gpgv method, so we need to save it here + MetaIndexFileSignature = DestFile; + QueueForSignatureVerify(MetaIndexFile, MetaIndexFileSignature); } - - // this is the file we verify from - MetaIndexFileSignature = DestFile; - - AuthPass = true; - Desc.URI = "gpgv:" + MetaIndexFileSignature; - DestFile = MetaIndexFile; - QueueURI(Desc); - SetActiveSubprocess("gpgv"); return; } else { - // verify was successful - - // we parse the MetaIndexFile here (and not right after getting - // the pkgAcqMetaIndex) because at this point we can trust the data - // - // load indexes and queue further downloads - MetaIndexParser->Load(MetaIndexFile); - QueueIndexes(true); - - // DestFile points to the the MetaIndeFile at this point, make it - // point back to the Release.gpg file - std::string FinalFile = _config->FindDir("Dir::State::lists"); - FinalFile += URItoFileName(RealURI); - TransactionManager->TransactionStageCopy(this, MetaIndexFileSignature, FinalFile); + if(AuthDone(Message, RealURI) == true) + { + std::string FinalFile = _config->FindDir("Dir::State::lists"); + FinalFile += URItoFileName(RealURI); - Complete = true; + TransactionManager->TransactionStageCopy(this, MetaIndexFileSignature, FinalFile); + } } } /*}}}*/ @@ -1796,57 +1758,78 @@ string pkgAcqMetaIndex::Custom600Headers() const return "\nIndex-File: true\nLast-Modified: " + TimeRFC1123(Buf.st_mtime); } /*}}}*/ -void pkgAcqMetaIndex::Done(string Message,unsigned long long Size,HashStringList const &Hashes, /*{{{*/ +void pkgAcqMetaIndex::Done(string Message,unsigned long long Size, /*{{{*/ + HashStringList const &Hashes, pkgAcquire::MethodConfig *Cfg) { Item::Done(Message,Size,Hashes,Cfg); - // MetaIndexes are done in two passes: one to download the - // metaindex with an appropriate method, and a second to verify it - // with the gpgv method - - if (AuthPass == true) + if(CheckDownloadDone(Message, RealURI)) { - AuthDone(Message); + // 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; + new pkgAcqMetaSig(Owner, TransactionManager, + MetaIndexSigURI, MetaIndexSigURIDesc, + MetaIndexSigShortDesc, MetaIndexFile, IndexTargets, + MetaIndexParser); - // all cool, move Release file into place - Complete = true; + string FinalFile = _config->FindDir("Dir::State::lists"); + FinalFile += URItoFileName(RealURI); + TransactionManager->TransactionStageCopy(this, DestFile, FinalFile); } - else - { - RetrievalDone(Message); - if (!Complete) - // Still more retrieving to do - return; +} + /*}}}*/ +bool pkgAcqMetaBase::AuthDone(string Message, const string &RealURI) /*{{{*/ +{ + // At this point, the gpgv method has succeeded, so there is a + // valid signature from a key in the trusted keyring. We + // perform additional verification of its contents, and use them + // to verify the indexes we are about to download - if (SigFile != "") - { - // There was a signature file, so pass it to gpgv for - // verification - if (_config->FindB("Debug::pkgAcquire::Auth", false)) - std::cerr << "Metaindex acquired, queueing gpg verification (" - << SigFile << "," << DestFile << ")\n"; - AuthPass = true; - Desc.URI = "gpgv:" + SigFile; - QueueURI(Desc); - SetActiveSubprocess("gpgv"); - return; - } + if (!MetaIndexParser->Load(DestFile)) + { + Status = StatAuthError; + ErrorText = MetaIndexParser->ErrorText; + return false; } - if (Complete == true) + if (!VerifyVendor(Message, RealURI)) { - string FinalFile = _config->FindDir("Dir::State::lists"); - FinalFile += URItoFileName(RealURI); - if (SigFile == DestFile) - SigFile = FinalFile; - - // queue for copy in place - TransactionManager->TransactionStageCopy(this, DestFile, FinalFile); + return false; } + + if (_config->FindB("Debug::pkgAcquire::Auth", false)) + std::cerr << "Signature verification succeeded: " + << DestFile << std::endl; + + // Download further indexes with verification + // + // it would be really nice if we could simply do + // if (IMSHit == false) QueueIndexes(true) + // and skip the download if the Release file has not changed + // - but right now the list cleaner will needs to be tricked + // to not delete all our packages/source indexes in this case + QueueIndexes(true); + + return true; +} + /*}}}*/ + /*{{{*/ +void pkgAcqMetaBase::QueueForSignatureVerify(const std::string &MetaIndexFile, + const std::string &MetaIndexFileSignature) +{ + AuthPass = true; + Desc.URI = "gpgv:" + MetaIndexFileSignature; + DestFile = MetaIndexFile; + QueueURI(Desc); + SetActiveSubprocess("gpgv"); } /*}}}*/ -void pkgAcqMetaIndex::RetrievalDone(string Message) /*{{{*/ + /*{{{*/ +bool pkgAcqMetaBase::CheckDownloadDone(const std::string &Message, + const std::string &RealURI) { // We have just finished downloading a Release file (it is not // verified yet) @@ -1856,7 +1839,7 @@ void pkgAcqMetaIndex::RetrievalDone(string Message) /*{{{*/ { Status = StatError; ErrorText = "Method gave a blank filename"; - return; + return false; } if (FileName != DestFile) @@ -1864,7 +1847,7 @@ void pkgAcqMetaIndex::RetrievalDone(string Message) /*{{{*/ Local = true; Desc.URI = "copy:" + FileName; QueueURI(Desc); - return; + return false; } // make sure to verify against the right file on I-M-S hit @@ -1873,101 +1856,13 @@ void pkgAcqMetaIndex::RetrievalDone(string Message) /*{{{*/ { string FinalFile = _config->FindDir("Dir::State::lists"); FinalFile += URItoFileName(RealURI); - if (SigFile == DestFile) - { - SigFile = FinalFile; -#if 0 - // constructor of pkgAcqMetaClearSig moved it out of the way, - // now move it back in on IMS hit for the 'old' file - string const OldClearSig = DestFile + ".reverify"; - if (RealFileExists(OldClearSig) == true) - Rename(OldClearSig, FinalFile); -#endif - } DestFile = FinalFile; } - // queue a signature - if(SigFile != DestFile) - new pkgAcqMetaSig(Owner, TransactionManager, - MetaIndexSigURI, MetaIndexSigURIDesc, - MetaIndexSigShortDesc, DestFile, IndexTargets, - MetaIndexParser); - + // set Item to complete as the remaining work is all local (verify etc) Complete = true; -} - /*}}}*/ -void pkgAcqMetaIndex::AuthDone(string Message) /*{{{*/ -{ - // At this point, the gpgv method has succeeded, so there is a - // valid signature from a key in the trusted keyring. We - // perform additional verification of its contents, and use them - // to verify the indexes we are about to download - - if (!MetaIndexParser->Load(DestFile)) - { - Status = StatAuthError; - ErrorText = MetaIndexParser->ErrorText; - return; - } - - if (!VerifyVendor(Message)) - { - return; - } - - if (_config->FindB("Debug::pkgAcquire::Auth", false)) - std::cerr << "Signature verification succeeded: " - << DestFile << std::endl; -// we ensure this by other means -#if 0 - // do not trust any previously unverified content that we may have - string LastGoodSigFile = _config->FindDir("Dir::State::lists").append("partial/").append(URItoFileName(RealURI)); - if (DestFile != SigFile) - LastGoodSigFile.append(".gpg"); - LastGoodSigFile.append(".reverify"); - if(IMSHit == false && RealFileExists(LastGoodSigFile) == false) - { - for (vector ::const_iterator Target = IndexTargets->begin(); - Target != IndexTargets->end(); - ++Target) - { - // remove old indexes - std::string index = _config->FindDir("Dir::State::lists") + - URItoFileName((*Target)->URI); - unlink(index.c_str()); - // and also old gzipindexes - std::vector types = APT::Configuration::getCompressionTypes(); - for (std::vector::const_iterator t = types.begin(); t != types.end(); ++t) - { - index += '.' + (*t); - unlink(index.c_str()); - } - } - } -#endif - - // Download further indexes with verification - // - // it would be really nice if we could simply do - // if (IMSHit == false) QueueIndexes(true) - // and skip the download if the Release file has not changed - // - but right now the list cleaner will needs to be tricked - // to not delete all our packages/source indexes in this case - QueueIndexes(true); - -#if 0 - // is it a clearsigned MetaIndex file? - if (DestFile == SigFile) - return; - - // Done, move signature file into position - string VerifiedSigFile = _config->FindDir("Dir::State::lists") + - URItoFileName(RealURI) + ".gpg"; - Rename(SigFile,VerifiedSigFile); - chmod(VerifiedSigFile.c_str(),0644); -#endif + return true; } /*}}}*/ void pkgAcqMetaBase::QueueIndexes(bool verify) /*{{{*/ @@ -2056,7 +1951,7 @@ void pkgAcqMetaBase::QueueIndexes(bool verify) /*{{{*/ } } /*}}}*/ -bool pkgAcqMetaIndex::VerifyVendor(string Message) /*{{{*/ +bool pkgAcqMetaBase::VerifyVendor(string Message, const string &RealURI)/*{{{*/ { string::size_type pos; @@ -2273,7 +2168,24 @@ void pkgAcqMetaClearSig::Done(std::string Message,unsigned long long Size, TransactionManager->AbortTransaction(); return; } - pkgAcqMetaIndex::Done(Message, Size, Hashes, Cnf); + + if(AuthPass == false) + { + if(CheckDownloadDone(Message, RealURI) == true) + QueueForSignatureVerify(DestFile, DestFile); + return; + } + else + { + if(AuthDone(Message, RealURI) == true) + { + string FinalFile = _config->FindDir("Dir::State::lists"); + FinalFile += URItoFileName(RealURI); + + // queue for copy in place + TransactionManager->TransactionStageCopy(this, DestFile, FinalFile); + } + } } /*}}}*/ void pkgAcqMetaClearSig::Failed(string Message,pkgAcquire::MethodConfig *Cnf) /*{{{*/ diff --git a/apt-pkg/acquire-item.h b/apt-pkg/acquire-item.h index bab2cc0d7..f12f57262 100644 --- a/apt-pkg/acquire-item.h +++ b/apt-pkg/acquire-item.h @@ -385,6 +385,41 @@ class pkgAcqMetaBase : public pkgAcquire::Item */ void QueueIndexes(bool verify); + + /** \brief Called when a file is finished being retrieved. + * + * If the file was not downloaded to DestFile, a copy process is + * set up to copy it to DestFile; otherwise, Complete is set to \b + * true and the file is moved to its final location. + * + * \param Message The message block received from the fetch + * subprocess. + */ + bool CheckDownloadDone(const std::string &Message, + const std::string &RealURI); + + /** \brief Queue the downloaded Signature for verification */ + void QueueForSignatureVerify(const std::string &MetaIndexFile, + const std::string &MetaIndexFileSignature); + + /** \brief Called when authentication succeeded. + * + * Sanity-checks the authenticated file, queues up the individual + * index files for download, and saves the signature in the lists + * directory next to the authenticated list file. + * + * \param Message The message block received from the fetch + * subprocess. + */ + bool AuthDone(std::string Message, const std::string &RealURI); + + /** \brief Check that the release file is a release file for the + * correct distribution. + * + * \return \b true if no fatal errors were encountered. + */ + bool VerifyVendor(std::string Message, const std::string &RealURI); + public: // transaction code void Add(Item *I); @@ -496,35 +531,6 @@ class pkgAcqMetaIndex : public pkgAcqMetaBase */ std::string SigFile; - /** \brief Check that the release file is a release file for the - * correct distribution. - * - * \return \b true if no fatal errors were encountered. - */ - bool VerifyVendor(std::string Message); - - /** \brief Called when a file is finished being retrieved. - * - * If the file was not downloaded to DestFile, a copy process is - * set up to copy it to DestFile; otherwise, Complete is set to \b - * true and the file is moved to its final location. - * - * \param Message The message block received from the fetch - * subprocess. - */ - void RetrievalDone(std::string Message); - - /** \brief Called when authentication succeeded. - * - * Sanity-checks the authenticated file, queues up the individual - * index files for download, and saves the signature in the lists - * directory next to the authenticated list file. - * - * \param Message The message block received from the fetch - * subprocess. - */ - void AuthDone(std::string Message); - std::string URIDesc; std::string ShortDesc; diff --git a/test/integration/test-apt-update-ims b/test/integration/test-apt-update-ims index 38dcd73fd..61b808b0f 100755 --- a/test/integration/test-apt-update-ims +++ b/test/integration/test-apt-update-ims @@ -30,6 +30,7 @@ runtest() { testfailure ls "rootdir/var/lib/apt/lists/partial/*" } +msgmsg "InRelease" EXPECT="Hit http://localhost:8080 unstable InRelease Hit http://localhost:8080 unstable/main Sources Hit http://localhost:8080 unstable/main amd64 Packages @@ -42,14 +43,11 @@ runtest echo "Acquire::GzipIndexes "1";" > rootdir/etc/apt/apt.conf.d/02compressindex runtest -# FIXME: how can we get rid of this extra line -# "Get:1 http://localhost:8080 unstable Release.gpg" -# +msgmsg "Release/Release.gpg" # with Release/Release.gpg EXPECT="Ign http://localhost:8080 unstable InRelease Hit http://localhost:8080 unstable Release Hit http://localhost:8080 unstable Release.gpg -Get:1 http://localhost:8080 unstable Release.gpg Hit http://localhost:8080 unstable/main Sources Hit http://localhost:8080 unstable/main amd64 Packages Hit http://localhost:8080 unstable/main Translation-en @@ -62,3 +60,23 @@ runtest echo "Acquire::GzipIndexes "1";" > rootdir/etc/apt/apt.conf.d/02compressindex runtest + + +# no Release.gpg or InRelease +msgmsg "Release only" +EXPECT="Ign http://localhost:8080 unstable InRelease +Hit http://localhost:8080 unstable Release +Ign http://localhost:8080 unstable Release.gpg +Hit http://localhost:8080 unstable/main Sources +Hit http://localhost:8080 unstable/main amd64 Packages +Hit http://localhost:8080 unstable/main Translation-en +Reading package lists..." + +find aptarchive -name "Release.gpg" | xargs rm -f + +echo 'Acquire::AllowInsecureRepositories "1";' > rootdir/etc/apt/apt.conf.d/insecure.conf +echo "Acquire::GzipIndexes "0";" > rootdir/etc/apt/apt.conf.d/02compressindex +runtest + +echo "Acquire::GzipIndexes "1";" > rootdir/etc/apt/apt.conf.d/02compressindex +runtest -- cgit v1.2.3