From 5985c230c8ac85fe2b2eb504b798377843bdc7cd Mon Sep 17 00:00:00 2001 From: David Kalnischkies Date: Fri, 20 Sep 2013 13:34:22 +0200 Subject: do not trust FileFd::Eof() in pkgTagFile::Fill() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Eof check was added (by me of course) in 0aae6d14390193e25ab6d0fd49295bd7b131954f as part of a fix up ~a month ago (at DebConf). The idea was not that bad, but doesn't make that much sense either as this bit is set by the FileFd based on Actual as well, so this is basically doing the same check again – with the difference that the HitEof bit can still linger from a previous Read we did at the end of the file, but have seek'd away from it now. Combined with the length of entries, entry order and other not that easily controllable conditions you can be 'lucky' enough to hit this problem in a way which even visible (truncating of other fields might not be visible easily, like 'Tags' and others). Closes: 723705 Thanks: Cyril Brulebois --- apt-pkg/tagfile.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'apt-pkg') diff --git a/apt-pkg/tagfile.cc b/apt-pkg/tagfile.cc index b91e868e2..e0802e3d5 100644 --- a/apt-pkg/tagfile.cc +++ b/apt-pkg/tagfile.cc @@ -164,7 +164,7 @@ bool pkgTagFile::Fill() unsigned long long const dataSize = d->Size - ((d->End - d->Buffer) + 1); if (d->Fd.Read(d->End, dataSize, &Actual) == false) return false; - if (Actual != dataSize || d->Fd.Eof() == true) + if (Actual != dataSize) d->Done = true; d->End += Actual; } -- cgit v1.2.3 From b4140ecf132d15adf740f23330054b6788d4f9a6 Mon Sep 17 00:00:00 2001 From: David Kalnischkies Date: Sat, 21 Sep 2013 14:23:02 +0200 Subject: don't strip :any from dependencies in single-arch The parser goes a bit to far by stripping :any from dependencies in a single architecture environment. the flag "Multi-Arch: allowed" doesn't care any architecture restrictions in that case (as in single arch everything is native), but it still limits the possible versions statisfying the dependency so stripping :any over-simplifies in upgrade situations from "Multi-Arch: none" to "Multi-Arch: allowed". Closes: 723586 --- apt-pkg/deb/deblistparser.cc | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) (limited to 'apt-pkg') diff --git a/apt-pkg/deb/deblistparser.cc b/apt-pkg/deb/deblistparser.cc index 87aab6ee2..68d544e1f 100644 --- a/apt-pkg/deb/deblistparser.cc +++ b/apt-pkg/deb/deblistparser.cc @@ -635,7 +635,7 @@ bool debListParser::ParseDepends(pkgCache::VerIterator &Ver, string Version; unsigned int Op; - Start = ParseDepends(Start,Stop,Package,Version,Op,false,!MultiArchEnabled); + Start = ParseDepends(Start, Stop, Package, Version, Op, false, false); if (Start == 0) return _error->Error("Problem parsing dependency %s",Tag); size_t const found = Package.rfind(':'); @@ -717,9 +717,7 @@ bool debListParser::ParseProvides(pkgCache::VerIterator &Ver) } } - if (MultiArchEnabled == false) - return true; - else if ((Ver->MultiArch & pkgCache::Version::Allowed) == pkgCache::Version::Allowed) + if ((Ver->MultiArch & pkgCache::Version::Allowed) == pkgCache::Version::Allowed) { string const Package = string(Ver.ParentPkg().Name()).append(":").append("any"); return NewProvidesAllArch(Ver, Package, Ver.VerStr()); -- cgit v1.2.3 From 6c34cccad778bd8db0ce03b7596cbef03afa9688 Mon Sep 17 00:00:00 2001 From: David Kalnischkies Date: Thu, 26 Sep 2013 13:32:49 +0200 Subject: pkg from only trusted sources keeps being trusted --allow-unauthenticated switches the download to a pre-0.6 system in which a package can come from any source, rather than that trusted packages can only come from trusted sources. To allow this the flag used to set all packages as untrusted, which is a bit much, so we check now if the package can be acquired via an untrusted source and only if this is the case set it as untrusted. As APT nowadays supports setting sources as trusted via a flag in the sources.list this mode shouldn't be used that much anymore though. [Note that this is not the patch from the BTS] Closes: 617690 --- apt-pkg/acquire-item.cc | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-) (limited to 'apt-pkg') diff --git a/apt-pkg/acquire-item.cc b/apt-pkg/acquire-item.cc index 97b2d1e29..222b78671 100644 --- a/apt-pkg/acquire-item.cc +++ b/apt-pkg/acquire-item.cc @@ -1719,27 +1719,34 @@ pkgAcqArchive::pkgAcqArchive(pkgAcquire *Owner,pkgSourceList *Sources, } // check if we have one trusted source for the package. if so, switch - // to "TrustedOnly" mode + // to "TrustedOnly" mode - but only if not in AllowUnauthenticated mode + bool const allowUnauth = _config->FindB("APT::Get::AllowUnauthenticated", false); + bool const debugAuth = _config->FindB("Debug::pkgAcquire::Auth", false); + bool seenUntrusted = false; for (pkgCache::VerFileIterator i = Version.FileList(); i.end() == false; ++i) { pkgIndexFile *Index; if (Sources->FindIndex(i.File(),Index) == false) continue; - if (_config->FindB("Debug::pkgAcquire::Auth", false)) - { + + if (debugAuth == true) std::cerr << "Checking index: " << Index->Describe() - << "(Trusted=" << Index->IsTrusted() << ")\n"; - } - if (Index->IsTrusted()) { + << "(Trusted=" << Index->IsTrusted() << ")" << std::endl; + + if (Index->IsTrusted() == true) + { Trusted = true; - break; + if (allowUnauth == false) + break; } + else + seenUntrusted = true; } // "allow-unauthenticated" restores apts old fetching behaviour // that means that e.g. unauthenticated file:// uris are higher // priority than authenticated http:// uris - if (_config->FindB("APT::Get::AllowUnauthenticated",false) == true) + if (allowUnauth == true && seenUntrusted == true) Trusted = false; // Select a source -- cgit v1.2.3 From 8fa042ca39dcb39d544f015f4a924c5dbc10ad2c Mon Sep 17 00:00:00 2001 From: David Kalnischkies Date: Mon, 30 Sep 2013 18:51:40 +0200 Subject: don't consider holds for autoremoval MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We can't remove packages which are held back by the user with a hold, so marking them (or its dependencies) as garbage will lead our autoremover into madness – and given that the package is important enough that the user has held it back it can't be garbage (at least at the moment), so even if a front-end wants to use the info just for information display its a good idea to not consider it garbage for them. Closes: 724995 --- apt-pkg/depcache.cc | 7 ++++--- apt-pkg/depcache.h | 2 +- 2 files changed, 5 insertions(+), 4 deletions(-) (limited to 'apt-pkg') diff --git a/apt-pkg/depcache.cc b/apt-pkg/depcache.cc index 978a893f7..a06789cdf 100644 --- a/apt-pkg/depcache.cc +++ b/apt-pkg/depcache.cc @@ -896,6 +896,7 @@ char const* PrintMode(char const mode) case pkgDepCache::ModeInstall: return "Install"; case pkgDepCache::ModeKeep: return "Keep"; case pkgDepCache::ModeDelete: return "Delete"; + case pkgDepCache::ModeGarbage: return "Garbage"; default: return "UNKNOWN"; } } @@ -1726,8 +1727,6 @@ bool pkgDepCache::MarkRequired(InRootSetFunc &userFunc) follow_recommends = MarkFollowsRecommends(); follow_suggests = MarkFollowsSuggests(); - - // do the mark part, this is the core bit of the algorithm for(PkgIterator p = PkgBegin(); !p.end(); ++p) { @@ -1738,7 +1737,9 @@ bool pkgDepCache::MarkRequired(InRootSetFunc &userFunc) // be nice even then a required package violates the policy (#583517) // and do the full mark process also for required packages (p.CurrentVer().end() != true && - p.CurrentVer()->Priority == pkgCache::State::Required)) + p.CurrentVer()->Priority == pkgCache::State::Required) || + // packages which can't be changed (like holds) can't be garbage + (IsModeChangeOk(ModeGarbage, p, 0, false) == false)) { // the package is installed (and set to keep) if(PkgState[p->ID].Keep() && !p.CurrentVer().end()) diff --git a/apt-pkg/depcache.h b/apt-pkg/depcache.h index d9c95349b..61c9aa559 100644 --- a/apt-pkg/depcache.h +++ b/apt-pkg/depcache.h @@ -128,7 +128,7 @@ class pkgDepCache : protected pkgCache::Namespace enum InternalFlags {AutoKept = (1 << 0), Purge = (1 << 1), ReInstall = (1 << 2), Protected = (1 << 3)}; enum VersionTypes {NowVersion, InstallVersion, CandidateVersion}; - enum ModeList {ModeDelete = 0, ModeKeep = 1, ModeInstall = 2}; + enum ModeList {ModeDelete = 0, ModeKeep = 1, ModeInstall = 2, ModeGarbage = 3}; /** \brief Represents an active action group. * -- cgit v1.2.3 From 3c8030a4977536e9d3a1954adc68082ae1c6d5a2 Mon Sep 17 00:00:00 2001 From: David Kalnischkies Date: Thu, 3 Oct 2013 01:12:18 +0200 Subject: refactor onError relabeling of DestFile as '.FAILED' This helps ensure three things: - each error is reported via ReportMirrorFailure - if DestFile doesn't exist, do not attempt rename - renames happen for every error The last one wasn't the case for Size mismatches, which isn't nice, but not a exploitable problem per-se as the file isn't picked up and remains in partial/ where the following download-try will at most take it for a partial request which fails the hashsum verification later on Git-Dch: Ignore --- apt-pkg/acquire-item.cc | 75 ++++++++++++++++++++++++++++--------------------- apt-pkg/acquire-item.h | 19 +++++++++++-- 2 files changed, 60 insertions(+), 34 deletions(-) (limited to 'apt-pkg') diff --git a/apt-pkg/acquire-item.cc b/apt-pkg/acquire-item.cc index 222b78671..fcc7c7404 100644 --- a/apt-pkg/acquire-item.cc +++ b/apt-pkg/acquire-item.cc @@ -143,6 +143,32 @@ void pkgAcquire::Item::Rename(string From,string To) } } /*}}}*/ +bool pkgAcquire::Item::RenameOnError(pkgAcquire::Item::RenameOnErrorState const error)/*{{{*/ +{ + if(FileExists(DestFile)) + Rename(DestFile, DestFile + ".FAILED"); + + switch (error) + { + case HashSumMismatch: + ErrorText = _("Hash Sum mismatch"); + Status = StatAuthError; + ReportMirrorFailure("HashChecksumFailure"); + break; + case SizeMismatch: + ErrorText = _("Size mismatch"); + Status = StatAuthError; + ReportMirrorFailure("SizeFailure"); + break; + case InvalidFormat: + ErrorText = _("Invalid file format"); + Status = StatError; + // do not report as usually its not the mirrors fault, but Portal/Proxy + break; + } + return false; +} + /*}}}*/ // Acquire::Item::ReportMirrorFailure /*{{{*/ // --------------------------------------------------------------------- void pkgAcquire::Item::ReportMirrorFailure(string FailCode) @@ -595,9 +621,7 @@ void pkgAcqIndexDiffs::Finish(bool allDone) if(!ExpectedHash.empty() && !ExpectedHash.VerifyFile(DestFile)) { - Status = StatAuthError; - ErrorText = _("MD5Sum mismatch"); - Rename(DestFile,DestFile + ".FAILED"); + RenameOnError(HashSumMismatch); Dequeue(); return; } @@ -866,10 +890,7 @@ void pkgAcqIndex::Done(string Message,unsigned long long Size,string Hash, if (!ExpectedHash.empty() && ExpectedHash.toStr() != Hash) { - Status = StatAuthError; - ErrorText = _("Hash Sum mismatch"); - Rename(DestFile,DestFile + ".FAILED"); - ReportMirrorFailure("HashChecksumFailure"); + RenameOnError(HashSumMismatch); return; } @@ -878,22 +899,18 @@ void pkgAcqIndex::Done(string Message,unsigned long long Size,string Hash, if (Verify == true) { FileFd fd(DestFile, FileFd::ReadOnly); - pkgTagSection sec; - pkgTagFile tag(&fd); - - // Only test for correctness if the file is not empty (empty is ok) - if (fd.Size() > 0) { - if (_error->PendingError() || !tag.Step(sec)) { - Status = StatError; - _error->DumpErrors(); - Rename(DestFile,DestFile + ".FAILED"); - return; - } else if (!sec.Exists("Package")) { - Status = StatError; - ErrorText = ("Encountered a section with no Package: header"); - Rename(DestFile,DestFile + ".FAILED"); - return; - } + // Only test for correctness if the file is not empty (empty is ok) + if (fd.FileSize() > 0) + { + pkgTagSection sec; + pkgTagFile tag(&fd); + + // all our current indexes have a field 'Package' in each section + if (_error->PendingError() == true || tag.Step(sec) == false || sec.Exists("Package") == false) + { + RenameOnError(InvalidFormat); + return; + } } } @@ -1907,18 +1924,14 @@ void pkgAcqArchive::Done(string Message,unsigned long long Size,string CalcHash, // Check the size if (Size != Version->Size) { - Status = StatError; - ErrorText = _("Size mismatch"); + RenameOnError(SizeMismatch); return; } // Check the hash if(ExpectedHash.toStr() != CalcHash) { - Status = StatError; - ErrorText = _("Hash Sum mismatch"); - if(FileExists(DestFile)) - Rename(DestFile,DestFile + ".FAILED"); + RenameOnError(HashSumMismatch); return; } @@ -2058,9 +2071,7 @@ void pkgAcqFile::Done(string Message,unsigned long long Size,string CalcHash, // Check the hash if(!ExpectedHash.empty() && ExpectedHash.toStr() != CalcHash) { - Status = StatError; - ErrorText = _("Hash Sum mismatch"); - Rename(DestFile,DestFile + ".FAILED"); + RenameOnError(HashSumMismatch); return; } diff --git a/apt-pkg/acquire-item.h b/apt-pkg/acquire-item.h index 10c855e63..6b4f73708 100644 --- a/apt-pkg/acquire-item.h +++ b/apt-pkg/acquire-item.h @@ -83,7 +83,7 @@ class pkgAcquire::Item : public WeakPointable * overwritten. */ void Rename(std::string From,std::string To); - + public: /** \brief The current status of this item. */ @@ -281,6 +281,21 @@ class pkgAcquire::Item : public WeakPointable * pkgAcquire::Remove. */ virtual ~Item(); + + protected: + + enum RenameOnErrorState { + HashSumMismatch, + SizeMismatch, + InvalidFormat + }; + + /** \brief Rename failed file and set error + * + * \param state respresenting the error we encountered + * \param errorMsg a message describing the error + */ + bool RenameOnError(RenameOnErrorState const state); }; /*}}}*/ /** \brief Information about an index patch (aka diff). */ /*{{{*/ @@ -982,7 +997,7 @@ class pkgAcqArchive : public pkgAcquire::Item * * \param Version The package version to download. * - * \param StoreFilename A location in which the actual filename of + * \param[out] StoreFilename A location in which the actual filename of * the package should be stored. It will be set to a guessed * basename in the constructor, and filled in with a fully * qualified filename once the download finishes. -- cgit v1.2.3 From d57f6084aaa3972073114973d149ea2291b36682 Mon Sep 17 00:00:00 2001 From: David Kalnischkies Date: Thu, 3 Oct 2013 15:11:21 +0200 Subject: use pkgAcqArchive in 'download' for proper errors With a bit of trickery we can reuse the usual infrastructure we have in place to acquire deb files for the 'download' operation as well, which gains us authentification check & display, error messages, correct filenames and "downloads" from the root-owned archives. --- apt-pkg/acquire-item.cc | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) (limited to 'apt-pkg') diff --git a/apt-pkg/acquire-item.cc b/apt-pkg/acquire-item.cc index fcc7c7404..04505b35a 100644 --- a/apt-pkg/acquire-item.cc +++ b/apt-pkg/acquire-item.cc @@ -1768,9 +1768,8 @@ pkgAcqArchive::pkgAcqArchive(pkgAcquire *Owner,pkgSourceList *Sources, // Select a source if (QueueNext() == false && _error->PendingError() == false) - _error->Error(_("I wasn't able to locate a file for the %s package. " - "This might mean you need to manually fix this package."), - Version.ParentPkg().Name()); + _error->Error(_("Can't find a source to download version '%s' of '%s'"), + Version.VerStr(), Version.ParentPkg().FullName(false).c_str()); } /*}}}*/ // AcqArchive::QueueNext - Queue the next file source /*{{{*/ -- cgit v1.2.3 From 342df712331004aa4907c9dbdf4b7728d087efb0 Mon Sep 17 00:00:00 2001 From: David Kalnischkies Date: Thu, 3 Oct 2013 21:34:52 +0200 Subject: fix lzma-support detection via xz binary Clear() only clears a config option, not removing it and an empty setting still exists. Hence we set the option instead to the xz path so that the later existance check can find a binary for the test --- apt-pkg/aptconfiguration.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'apt-pkg') diff --git a/apt-pkg/aptconfiguration.cc b/apt-pkg/aptconfiguration.cc index 4f9b84e00..115d11616 100644 --- a/apt-pkg/aptconfiguration.cc +++ b/apt-pkg/aptconfiguration.cc @@ -453,7 +453,7 @@ void Configuration::setDefaultConfigurationForCompressors() { _config->CndSet("Dir::Bin::bzip2", "/bin/bzip2"); _config->CndSet("Dir::Bin::xz", "/usr/bin/xz"); if (FileExists(_config->FindFile("Dir::Bin::xz")) == true) { - _config->Clear("Dir::Bin::lzma"); + _config->Set("Dir::Bin::lzma", _config->FindFile("Dir::Bin::xz")); _config->Set("APT::Compressor::lzma::Binary", "xz"); if (_config->Exists("APT::Compressor::lzma::CompressArg") == false) { _config->Set("APT::Compressor::lzma::CompressArg::", "--format=lzma"); -- cgit v1.2.3