From 18593cf75189c0d6d3cbbf729013c875398218ca Mon Sep 17 00:00:00 2001 From: Michael Vogt Date: Wed, 29 Oct 2014 16:32:42 +0100 Subject: Only support Translation-* that are listed in the {In,}Release file Handle Translation-* files exactly like Packages files (with the expection that it is ok if a download of them fails). Remove all "guessing" on apts side. This will elimimnate a bunch of errors releated to captive portals and similar. Its also more correct and removes another potential attack vector. --- apt-pkg/acquire-item.cc | 156 ++++++--------------- apt-pkg/acquire-item.h | 37 ----- apt-pkg/deb/debindexfile.cc | 6 +- .../test-bug-595691-empty-and-broken-archive-files | 10 -- .../test-bug-624218-Translation-file-handling | 11 +- 5 files changed, 46 insertions(+), 174 deletions(-) diff --git a/apt-pkg/acquire-item.cc b/apt-pkg/acquire-item.cc index 704e285b5..0e406f368 100644 --- a/apt-pkg/acquire-item.cc +++ b/apt-pkg/acquire-item.cc @@ -1275,6 +1275,9 @@ string pkgAcqIndex::Custom600Headers() const if (stat(Final.c_str(),&Buf) == 0) msg += "\nLast-Modified: " + TimeRFC1123(Buf.st_mtime); + if(Target->IsOptional()) + msg += "\nFail-Ignore: true"; + return msg; } /*}}}*/ @@ -1297,8 +1300,10 @@ void pkgAcqIndex::Failed(string Message,pkgAcquire::MethodConfig *Cnf) Item::Failed(Message,Cnf); - /// cancel the entire transaction - TransactionManager->AbortTransaction(); + if(Target->IsOptional() && ExpectedHashes.empty() && Stage == STAGE_DOWNLOAD) + Status = StatDone; + else + TransactionManager->AbortTransaction(); } /*}}}*/ // pkgAcqIndex::GetFinalFilename - Return the full final file path /*{{{*/ @@ -1490,57 +1495,6 @@ void pkgAcqIndex::StageDecompressDone(string Message, return; } /*}}}*/ -// AcqIndexTrans::pkgAcqIndexTrans - Constructor /*{{{*/ -// --------------------------------------------------------------------- -/* The Translation file is added to the queue */ -pkgAcqIndexTrans::pkgAcqIndexTrans(pkgAcquire *Owner, - string URI,string URIDesc,string ShortDesc) - : pkgAcqIndex(Owner, URI, URIDesc, ShortDesc, HashStringList()) -{ -} -pkgAcqIndexTrans::pkgAcqIndexTrans(pkgAcquire *Owner, - pkgAcqMetaBase *TransactionManager, - IndexTarget const * const Target, - HashStringList const &ExpectedHashes, - indexRecords *MetaIndexParser) - : pkgAcqIndex(Owner, TransactionManager, Target, ExpectedHashes, MetaIndexParser) -{ -} - /*}}}*/ -// AcqIndexTrans::Custom600Headers - Insert custom request headers /*{{{*/ -string pkgAcqIndexTrans::Custom600Headers() const -{ - string Final = GetFinalFilename(); - - struct stat Buf; - if (stat(Final.c_str(),&Buf) != 0) - return "\nFail-Ignore: true\nIndex-File: true"; - return "\nFail-Ignore: true\nIndex-File: true\nLast-Modified: " + TimeRFC1123(Buf.st_mtime); -} - /*}}}*/ -// AcqIndexTrans::Failed - Silence failure messages for missing files /*{{{*/ -void pkgAcqIndexTrans::Failed(string Message,pkgAcquire::MethodConfig *Cnf) -{ - size_t const nextExt = CompressionExtensions.find(' '); - if (nextExt != std::string::npos) - { - CompressionExtensions = CompressionExtensions.substr(nextExt+1); - Init(RealURI, Desc.Description, Desc.ShortDesc); - Status = StatIdle; - return; - } - - Item::Failed(Message,Cnf); - - // FIXME: this is used often (e.g. in pkgAcqIndexTrans) so refactor - if (Cnf->LocalOnly == true || - StringToBool(LookupTag(Message,"Transient-Failure"),false) == false) - { - // Ignore this - Status = StatDone; - } -} - /*}}}*/ // AcqMetaBase::Add - Add a item to the current Transaction /*{{{*/ void pkgAcqMetaBase::Add(Item *I) { @@ -1999,87 +1953,59 @@ bool pkgAcqMetaBase::CheckDownloadDone(const std::string &Message, /*}}}*/ void pkgAcqMetaBase::QueueIndexes(bool verify) /*{{{*/ { - bool transInRelease = false; - { - std::vector const keys = MetaIndexParser->MetaKeys(); - for (std::vector::const_iterator k = keys.begin(); k != keys.end(); ++k) - // FIXME: Feels wrong to check for hardcoded string here, but what should we do else… - if (k->find("Translation-") != std::string::npos) - { - transInRelease = true; - break; - } - } - // at this point the real Items are loaded in the fetcher ExpectedAdditionalItems = 0; - for (vector ::const_iterator Target = IndexTargets->begin(); + + vector ::const_iterator Target; + for (Target = IndexTargets->begin(); Target != IndexTargets->end(); ++Target) { HashStringList ExpectedIndexHashes; const indexRecords::checkSum *Record = MetaIndexParser->Lookup((*Target)->MetaKey); - bool compressedAvailable = false; - if (Record == NULL) + + // optional target that we do not have in the Release file are + // skipped + if (verify == true && Record == NULL && (*Target)->IsOptional()) + continue; + + // targets without a hash record are a error when verify is required + if (verify == true && Record == NULL) { - if ((*Target)->IsOptional() == true) - { - std::vector types = APT::Configuration::getCompressionTypes(); - for (std::vector::const_iterator t = types.begin(); t != types.end(); ++t) - if (MetaIndexParser->Exists((*Target)->MetaKey + "." + *t) == true) - { - compressedAvailable = true; - break; - } - } - else if (verify == true) - { - Status = StatAuthError; - strprintf(ErrorText, _("Unable to find expected entry '%s' in Release file (Wrong sources.list entry or malformed file)"), (*Target)->MetaKey.c_str()); - return; - } + Status = StatAuthError; + strprintf(ErrorText, _("Unable to find expected entry '%s' in Release file (Wrong sources.list entry or malformed file)"), (*Target)->MetaKey.c_str()); + return; } - else + + if (Record) + ExpectedIndexHashes = Record->Hashes; + + if (_config->FindB("Debug::pkgAcquire::Auth", false)) { - ExpectedIndexHashes = Record->Hashes; - if (_config->FindB("Debug::pkgAcquire::Auth", false)) - { - std::cerr << "Queueing: " << (*Target)->URI << std::endl - << "Expected Hash:" << std::endl; - for (HashStringList::const_iterator hs = ExpectedIndexHashes.begin(); hs != ExpectedIndexHashes.end(); ++hs) - std::cerr << "\t- " << hs->toStr() << std::endl; - std::cerr << "For: " << Record->MetaKeyFilename << std::endl; - } - if (verify == true && ExpectedIndexHashes.empty() == true && (*Target)->IsOptional() == false) - { - Status = StatAuthError; - strprintf(ErrorText, _("Unable to find hash sum for '%s' in Release file"), (*Target)->MetaKey.c_str()); - return; - } - } + std::cerr << "Queueing: " << (*Target)->URI << std::endl + << "Expected Hash:" << std::endl; + for (HashStringList::const_iterator hs = ExpectedIndexHashes.begin(); hs != ExpectedIndexHashes.end(); ++hs) + std::cerr << "\t- " << hs->toStr() << std::endl; + std::cerr << "For: " << Record->MetaKeyFilename << std::endl; - if ((*Target)->IsOptional() == true) + } + if (verify == true && ExpectedIndexHashes.empty() == true) { - if (transInRelease == false || Record != NULL || compressedAvailable == true) - { - if (_config->FindB("Acquire::PDiffs",true) == true && transInRelease == true && - MetaIndexParser->Exists((*Target)->MetaKey + ".diff/Index") == true) - new pkgAcqDiffIndex(Owner, TransactionManager, *Target, ExpectedIndexHashes, MetaIndexParser); - else - new pkgAcqIndexTrans(Owner, TransactionManager, *Target, ExpectedIndexHashes, MetaIndexParser); - } - continue; + Status = StatAuthError; + strprintf(ErrorText, _("Unable to find hash sum for '%s' in Release file"), (*Target)->MetaKey.c_str()); + return; } - /* Queue Packages file (either diff or full packages files, depending + /* Queue the Index file (Packages, Sources, Translation-$foo + (either diff or full packages files, depending on the users option) - we also check if the PDiff Index file is listed in the Meta-Index file. Ideal would be if pkgAcqDiffIndex would test this instead, but passing the required info to it is to much hassle */ if(_config->FindB("Acquire::PDiffs",true) == true && (verify == false || - MetaIndexParser->Exists((*Target)->MetaKey + ".diff/Index") == true)) - new pkgAcqDiffIndex(Owner, TransactionManager, *Target, ExpectedIndexHashes, MetaIndexParser); + MetaIndexParser->Exists((*Target)->MetaKey + ".diff/Index") == true)) + new pkgAcqDiffIndex(Owner, TransactionManager, *Target, ExpectedIndexHashes, MetaIndexParser); else - new pkgAcqIndex(Owner, TransactionManager, *Target, ExpectedIndexHashes, MetaIndexParser); + new pkgAcqIndex(Owner, TransactionManager, *Target, ExpectedIndexHashes, MetaIndexParser); } } /*}}}*/ diff --git a/apt-pkg/acquire-item.h b/apt-pkg/acquire-item.h index 68d5a01ce..7adc1976b 100644 --- a/apt-pkg/acquire-item.h +++ b/apt-pkg/acquire-item.h @@ -1024,43 +1024,6 @@ class pkgAcqIndex : public pkgAcqBaseIndex std::string const &ShortDesc); }; /*}}}*/ -/** \brief An acquire item that is responsible for fetching a {{{ - * translated index file. - * - * The only difference from pkgAcqIndex is that transient failures - * are suppressed: no error occurs if the translated index file is - * missing. - */ -class pkgAcqIndexTrans : public pkgAcqIndex -{ - void *d; - - public: - - virtual void Failed(std::string Message,pkgAcquire::MethodConfig *Cnf); - virtual std::string Custom600Headers() const; - - /** \brief Create a pkgAcqIndexTrans. - * - * \param Owner The pkgAcquire object with which this item is - * associated. - * - * \param URI The URI of the index file that is to be downloaded. - * - * \param URIDesc A "URI-style" description of this index file. - * - * \param ShortDesc A brief description of this index file. - */ - pkgAcqIndexTrans(pkgAcquire *Owner, - std::string URI,std::string URIDesc, - std::string ShortDesc); - pkgAcqIndexTrans(pkgAcquire *Owner, - pkgAcqMetaBase *TransactionManager, - IndexTarget const * const Target, - HashStringList const &ExpectedHashes, - indexRecords *MetaIndexParser); -}; - /*}}}*/ /** \brief Information about an index file. */ /*{{{*/ class IndexTarget { diff --git a/apt-pkg/deb/debindexfile.cc b/apt-pkg/deb/debindexfile.cc index cc1d94d81..779c74abf 100644 --- a/apt-pkg/deb/debindexfile.cc +++ b/apt-pkg/deb/debindexfile.cc @@ -458,9 +458,9 @@ string debTranslationsIndex::IndexURI(const char *Type) const bool debTranslationsIndex::GetIndexes(pkgAcquire *Owner) const { string const TranslationFile = string("Translation-").append(Language); - new pkgAcqIndexTrans(Owner, IndexURI(Language), - Info(TranslationFile.c_str()), - TranslationFile); + new pkgAcqIndex(Owner, IndexURI(Language), + Info(TranslationFile.c_str()), + TranslationFile, HashStringList()); return true; } diff --git a/test/integration/test-bug-595691-empty-and-broken-archive-files b/test/integration/test-bug-595691-empty-and-broken-archive-files index fedf82c92..bca07268c 100755 --- a/test/integration/test-bug-595691-empty-and-broken-archive-files +++ b/test/integration/test-bug-595691-empty-and-broken-archive-files @@ -73,16 +73,6 @@ E: Some index files failed to download. They have been ignored, or old ones used testoverhttp() { forcecompressor "$1" - createemptyfile 'en' - testaptgetupdate "Get: http://localhost:8080 Packages [] -Get: http://localhost:8080 Translation-en -Reading package lists..." "empty file en.$COMPRESS over http" - - createemptyarchive 'en' - testaptgetupdate "Get: http://localhost:8080 Packages [] -Get: http://localhost:8080 Translation-en [] -Reading package lists..." "empty archive en.$COMPRESS over http" - createemptyarchive 'Packages' testaptgetupdate "Get: http://localhost:8080 Packages [] Reading package lists..." "empty archive Packages.$COMPRESS over http" diff --git a/test/integration/test-bug-624218-Translation-file-handling b/test/integration/test-bug-624218-Translation-file-handling index d3c5b08ac..4ec30ee09 100755 --- a/test/integration/test-bug-624218-Translation-file-handling +++ b/test/integration/test-bug-624218-Translation-file-handling @@ -47,16 +47,9 @@ translationslisted() { translationslisted 'with full Index' -# only compressed files available (as it happens on CD-ROM) -sed -i '/i18n\/Translation-[^.]*$/ d' $(find aptarchive -name 'Release') -signreleasefiles - -translationslisted 'with partial Index' - - # no records at all about Translation files (fallback to guessing) -sed -i '/i18n\/Translation-.*$/ d' $(find aptarchive -name 'Release') -signreleasefiles +find aptarchive -name 'Release' -or -name 'InRelease' | xargs rm -f +configallowinsecurerepositories "true"; msgtest 'Download of en as forced language' 'without Index' aptget update -o Acquire::Languages=en | grep -q -e 'Translation-en ' && msgpass || msgfail -- cgit v1.2.3 From 7933b3a1cf56096d6ab30cedf1b736054b5fc84b Mon Sep 17 00:00:00 2001 From: Michael Vogt Date: Wed, 5 Nov 2014 17:28:13 +0100 Subject: test/integration/test-bug-624218-Translation-file-handling: clarify when Translation-* is guessed --- test/integration/test-bug-624218-Translation-file-handling | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/integration/test-bug-624218-Translation-file-handling b/test/integration/test-bug-624218-Translation-file-handling index 4ec30ee09..d32bd513b 100755 --- a/test/integration/test-bug-624218-Translation-file-handling +++ b/test/integration/test-bug-624218-Translation-file-handling @@ -47,7 +47,8 @@ translationslisted() { translationslisted 'with full Index' -# no records at all about Translation files (fallback to guessing) +# No Release file at all, so no records about Translation files +# (fallback to guessing) find aptarchive -name 'Release' -or -name 'InRelease' | xargs rm -f configallowinsecurerepositories "true"; -- cgit v1.2.3 From 87ecd6a2d719aeaeedd99dd500b467e966376f1c Mon Sep 17 00:00:00 2001 From: Michael Vogt Date: Wed, 5 Nov 2014 17:45:37 +0100 Subject: apt-pkg/deb/debindexfile.{cc,h}: kill GetIndexes() --- apt-pkg/deb/debindexfile.cc | 13 ------------- apt-pkg/deb/debindexfile.h | 1 - 2 files changed, 14 deletions(-) diff --git a/apt-pkg/deb/debindexfile.cc b/apt-pkg/deb/debindexfile.cc index 779c74abf..335f9d36e 100644 --- a/apt-pkg/deb/debindexfile.cc +++ b/apt-pkg/deb/debindexfile.cc @@ -452,19 +452,6 @@ string debTranslationsIndex::IndexURI(const char *Type) const return Res; } /*}}}*/ -// TranslationsIndex::GetIndexes - Fetch the index files /*{{{*/ -// --------------------------------------------------------------------- -/* */ -bool debTranslationsIndex::GetIndexes(pkgAcquire *Owner) const -{ - string const TranslationFile = string("Translation-").append(Language); - new pkgAcqIndex(Owner, IndexURI(Language), - Info(TranslationFile.c_str()), - TranslationFile, HashStringList()); - - return true; -} - /*}}}*/ // TranslationsIndex::Describe - Give a descriptive path to the index /*{{{*/ // --------------------------------------------------------------------- /* This should help the user find the index in the sources.list and diff --git a/apt-pkg/deb/debindexfile.h b/apt-pkg/deb/debindexfile.h index e5a1a7873..d727d9547 100644 --- a/apt-pkg/deb/debindexfile.h +++ b/apt-pkg/deb/debindexfile.h @@ -114,7 +114,6 @@ class debTranslationsIndex : public pkgIndexFile // Interface for acquire virtual std::string Describe(bool Short) const; - virtual bool GetIndexes(pkgAcquire *Owner) const; // Interface for the Cache Generator virtual bool Exists() const; -- cgit v1.2.3