From 3317ad864c997f4897756c0a2989c4199e9cda62 Mon Sep 17 00:00:00 2001 From: David Kalnischkies Date: Sat, 15 Jul 2017 14:12:50 +0200 Subject: use FileFd to parse all apt configuration files Using different ways of opening files means we have different behaviour and error messages for them, so by the same for all we can have more uniformity for users and apt developers alike. --- apt-pkg/contrib/configuration.cc | 23 ++++-------- apt-pkg/contrib/fileutl.cc | 32 +++++++++++++++-- apt-pkg/contrib/fileutl.h | 22 ++++++++++++ apt-pkg/policy.cc | 7 ++-- apt-pkg/sourcelist.cc | 13 +++---- test/integration/test-bug-818628-unreadable-source | 4 +-- test/libapt/configuration_test.cc | 41 ++++++++++++++++++++-- test/libapt/fileutl_test.cc | 13 +++++++ 8 files changed, 123 insertions(+), 32 deletions(-) diff --git a/apt-pkg/contrib/configuration.cc b/apt-pkg/contrib/configuration.cc index 442e31dff..65242bec5 100644 --- a/apt-pkg/contrib/configuration.cc +++ b/apt-pkg/contrib/configuration.cc @@ -840,9 +840,9 @@ bool ReadConfigFile(Configuration &Conf,const string &FName,bool const &AsSectio unsigned const &Depth) { // Open the stream for reading - ifstream F(FName.c_str(),ios::in); - if (F.fail() == true) - return _error->Errno("ifstream::ifstream",_("Opening configuration file %s"),FName.c_str()); + FileFd F; + if (OpenConfigurationFileFd(FName, F) == false) + return false; string LineBuffer; std::stack Stack; @@ -852,26 +852,15 @@ bool ReadConfigFile(Configuration &Conf,const string &FName,bool const &AsSectio int CurLine = 0; bool InComment = false; - while (F.eof() == false) + while (F.Eof() == false) { // The raw input line. std::string Input; + if (F.ReadLine(Input) == false) + Input.clear(); // The input line with comments stripped. std::string Fragment; - // Grab the next line of F and place it in Input. - do - { - char *Buffer = new char[1024]; - - F.clear(); - F.getline(Buffer,sizeof(Buffer) / 2); - - Input += Buffer; - delete[] Buffer; - } - while (F.fail() && !F.eof()); - // Expand tabs in the input line and remove leading and trailing // whitespace. { diff --git a/apt-pkg/contrib/fileutl.cc b/apt-pkg/contrib/fileutl.cc index 630a98ce4..7633b07ef 100644 --- a/apt-pkg/contrib/fileutl.cc +++ b/apt-pkg/contrib/fileutl.cc @@ -2493,15 +2493,35 @@ bool FileFd::Read(int const Fd, void *To, unsigned long long Size, unsigned long } /*}}}*/ // FileFd::ReadLine - Read a complete line from the file /*{{{*/ -// --------------------------------------------------------------------- -/* Beware: This method can be quite slow for big buffers on UNcompressed - files because of the naive implementation! */ char* FileFd::ReadLine(char *To, unsigned long long const Size) { *To = '\0'; if (d == nullptr || Failed()) return nullptr; return d->InternalReadLine(To, Size); +} +bool FileFd::ReadLine(std::string &To) +{ + To.clear(); + if (d == nullptr || Failed()) + return false; + constexpr size_t buflen = 4096; + char buffer[buflen]; + size_t len; + do + { + if (d->InternalReadLine(buffer, buflen) == nullptr) + return false; + len = strlen(buffer); + To.append(buffer, len); + } while (len == buflen - 1 && buffer[len - 2] != '\n'); + // remove the newline at the end + auto const i = To.find_last_not_of("\r\n"); + if (i == std::string::npos) + To.clear(); + else + To.erase(i + 1); + return true; } /*}}}*/ // FileFd::Flush - Flush the file /*{{{*/ @@ -3104,3 +3124,9 @@ bool DropPrivileges() /*{{{*/ return true; } /*}}}*/ +bool OpenConfigurationFileFd(std::string const &File, FileFd &Fd) /*{{{*/ +{ + APT::Configuration::Compressor none(".", "", "", nullptr, nullptr, 0); + return Fd.Open(File, FileFd::ReadOnly, none); +} + /*}}}*/ diff --git a/apt-pkg/contrib/fileutl.h b/apt-pkg/contrib/fileutl.h index 5e857b5c8..06c303809 100644 --- a/apt-pkg/contrib/fileutl.h +++ b/apt-pkg/contrib/fileutl.h @@ -87,7 +87,28 @@ class FileFd } bool Read(void *To,unsigned long long Size,unsigned long long *Actual = 0); bool static Read(int const Fd, void *To, unsigned long long Size, unsigned long long * const Actual = 0); + /** read a complete line or until buffer is full + * + * The buffer will always be \\0 terminated, so at most Size-1 characters are read. + * If the buffer holds a complete line the last character (before \\0) will be + * the newline character \\n otherwise the line was longer than the buffer. + * + * @param To buffer which will hold the line + * @param Size of the buffer to fill + * @param \b nullptr is returned in error cases, otherwise + * the parameter \b To now filled with the line. + */ char* ReadLine(char *To, unsigned long long const Size); + /** read a complete line from the file + * + * Similar to std::getline() the string does \b not include + * the newline, but just the content of the line as the newline + * is not needed to distinguish cases as for the other #ReadLine method. + * + * @param To string which will hold the line + * @return \b true if successful, otherwise \b false + */ + bool ReadLine(std::string &To); bool Flush(); bool Write(const void *From,unsigned long long Size); bool static Write(int Fd, const void *From, unsigned long long Size); @@ -256,5 +277,6 @@ std::vector Glob(std::string const &pattern, int flags=0); bool Popen(const char* Args[], FileFd &Fd, pid_t &Child, FileFd::OpenMode Mode, bool CaptureStderr); bool Popen(const char* Args[], FileFd &Fd, pid_t &Child, FileFd::OpenMode Mode); +APT_HIDDEN bool OpenConfigurationFileFd(std::string const &File, FileFd &Fd); #endif diff --git a/apt-pkg/policy.cc b/apt-pkg/policy.cc index 008c98ecb..69c1fbe10 100644 --- a/apt-pkg/policy.cc +++ b/apt-pkg/policy.cc @@ -343,8 +343,11 @@ bool ReadPinFile(pkgPolicy &Plcy,string File) if (RealFileExists(File) == false) return true; - - FileFd Fd(File,FileFd::ReadOnly); + + FileFd Fd; + if (OpenConfigurationFileFd(File, Fd) == false) + return false; + pkgTagFile TF(&Fd, pkgTagFile::SUPPORT_COMMENTS); if (Fd.IsOpen() == false || Fd.Failed()) return false; diff --git a/apt-pkg/sourcelist.cc b/apt-pkg/sourcelist.cc index 17c5c7a11..9f0c7f8ae 100644 --- a/apt-pkg/sourcelist.cc +++ b/apt-pkg/sourcelist.cc @@ -368,13 +368,12 @@ bool pkgSourceList::ReadAppend(string const &File) /* */ bool pkgSourceList::ParseFileOldStyle(std::string const &File) { - // Open the stream for reading - ifstream F(File.c_str(),ios::in /*| ios::nocreate*/); - if (F.fail() == true) - return _error->Errno("ifstream::ifstream",_("Opening %s"),File.c_str()); + FileFd Fd; + if (OpenConfigurationFileFd(File, Fd) == false) + return false; std::string Buffer; - for (unsigned int CurLine = 1; std::getline(F, Buffer); ++CurLine) + for (unsigned int CurLine = 1; Fd.ReadLine(Buffer); ++CurLine) { // remove comments size_t curpos = 0; @@ -423,7 +422,9 @@ bool pkgSourceList::ParseFileOldStyle(std::string const &File) bool pkgSourceList::ParseFileDeb822(string const &File) { // see if we can read the file - FileFd Fd(File, FileFd::ReadOnly); + FileFd Fd; + if (OpenConfigurationFileFd(File, Fd) == false) + return false; pkgTagFile Sources(&Fd, pkgTagFile::SUPPORT_COMMENTS); if (Fd.IsOpen() == false || Fd.Failed()) return _error->Error(_("Malformed stanza %u in source list %s (type)"),0,File.c_str()); diff --git a/test/integration/test-bug-818628-unreadable-source b/test/integration/test-bug-818628-unreadable-source index cddc79398..0c781c3b9 100755 --- a/test/integration/test-bug-818628-unreadable-source +++ b/test/integration/test-bug-818628-unreadable-source @@ -54,13 +54,13 @@ echo 'Apt::Cmd::Disable-Script-Warning "true";' >> aptconfig.conf msgmsg 'Unreadable sources file' chmod -r rootdir/etc/apt/sources.list.d/apt-test-unstable-deb-src.list -runthemall "E: Opening $TMPWORKINGDIRECTORY/rootdir/etc/apt/sources.list.d/apt-test-unstable-deb-src.list - ifstream::ifstream (13: Permission denied) +runthemall "E: Could not open file $TMPWORKINGDIRECTORY/rootdir/etc/apt/sources.list.d/apt-test-unstable-deb-src.list - open (13: Permission denied) E: The list of sources could not be read." chmod +r rootdir/etc/apt/sources.list.d/apt-test-unstable-deb-src.list msgmsg 'Unreadable config file' chmod -r rootdir/etc/apt/apt.conf.d/unreadable.conf -runthemall "E: Opening configuration file ${TMPWORKINGDIRECTORY}/rootdir/etc/apt/apt.conf.d/unreadable.conf - ifstream::ifstream (13: Permission denied)" +runthemall "E: Could not open file ${TMPWORKINGDIRECTORY}/rootdir/etc/apt/apt.conf.d/unreadable.conf - open (13: Permission denied)" chmod +r rootdir/etc/apt/apt.conf.d/unreadable.conf msgmsg 'Unreadable preferences file' diff --git a/test/libapt/configuration_test.cc b/test/libapt/configuration_test.cc index bdc17cbf4..bfb7144ce 100644 --- a/test/libapt/configuration_test.cc +++ b/test/libapt/configuration_test.cc @@ -1,14 +1,14 @@ #include #include +#include #include #include #include -//FIXME: Test for configuration file parsing; -// currently only integration/ tests test them implicitly +#include "file-helpers.h" TEST(ConfigurationTest,Lists) { @@ -195,3 +195,40 @@ TEST(ConfigurationTest,Merge) EXPECT_EQ("bar", Cnf.Find("option::foo")); EXPECT_EQ("", Cnf.Find("option::empty")); } +TEST(ConfigurationTest, Parsing) +{ + Configuration Cnf; + std::string tempfile; + FileFd fd; + createTemporaryFile("doublesignedfile", fd, &tempfile, R"apt( +SimpleOption "true"; +/* SimpleOption "false"; */ +Answer::Simple "42"; +# This is a comment +List::Option { "In"; "One"; "Line"; }; +// this a comment as well +List::Option2 { "Multi"; +"Line"; # inline comment + "Options"; +}; Trailing "true"; +/* Commented::Out "true"; */ +)apt"); + EXPECT_TRUE(ReadConfigFile(Cnf, tempfile)); + if (tempfile.empty() == false) + unlink(tempfile.c_str()); + EXPECT_TRUE(Cnf.FindB("SimpleOption")); + EXPECT_EQ(42, Cnf.FindI("Answer::Simple")); + EXPECT_TRUE(Cnf.Exists("List::Option")); + auto const singleline = Cnf.FindVector("List::Option"); + EXPECT_EQ(3, singleline.size()); + EXPECT_EQ("In", singleline[0]); + EXPECT_EQ("One", singleline[1]); + EXPECT_EQ("Line", singleline[2]); + auto const multiline = Cnf.FindVector("List::Option2"); + EXPECT_EQ(3, multiline.size()); + EXPECT_EQ("Multi", multiline[0]); + EXPECT_EQ("Line", multiline[1]); + EXPECT_EQ("Options", multiline[2]); + EXPECT_TRUE(Cnf.FindB("Trailing")); + EXPECT_FALSE(Cnf.Exists("Commented::Out")); +} diff --git a/test/libapt/fileutl_test.cc b/test/libapt/fileutl_test.cc index 6cc850033..a702c16ec 100644 --- a/test/libapt/fileutl_test.cc +++ b/test/libapt/fileutl_test.cc @@ -152,6 +152,19 @@ static void TestFileFd(mode_t const a_umask, mode_t const ExpectedFilePermission EXPECT_EQ(strlen(expect), f.Tell()); } #undef APT_INIT_READBACK + { + test.erase(test.length() - 1); + EXPECT_TRUE(f.Seek(0)); + EXPECT_FALSE(f.Eof()); + std::string line; + EXPECT_TRUE(f.ReadLine(line)); + EXPECT_FALSE(f.Failed()); + EXPECT_FALSE(f.Eof()); + EXPECT_EQ(line.length(), test.length()); + EXPECT_EQ(line.length() + 1, f.Tell()); + EXPECT_EQ(f.Size(), f.Tell()); + EXPECT_EQ(line, test); + } f.Close(); EXPECT_FALSE(f.IsOpen()); -- cgit v1.2.3 From 51751106976b1c6afa8f7991790db87b239fcc84 Mon Sep 17 00:00:00 2001 From: David Kalnischkies Date: Sat, 15 Jul 2017 15:08:35 +0200 Subject: show warnings instead of errors if files are unreadable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We used to fail on unreadable config/preferences/sources files, but at least for sources we didn't in the past and it seems harsh to refuse to work because of a single file, especially as the error messages are inconsistent and end up being silly (like suggesting to run apt update to fix the problem…). LP: #1701852 --- apt-pkg/cachefile.cc | 6 +-- apt-pkg/contrib/configuration.cc | 11 ++-- apt-pkg/contrib/fileutl.cc | 13 ++++- apt-pkg/contrib/fileutl.h | 1 + apt-pkg/init.cc | 20 ++++--- apt-pkg/policy.cc | 6 +-- apt-pkg/sourcelist.cc | 36 +++++-------- test/integration/test-bug-818628-unreadable-source | 63 ++++++++++++---------- 8 files changed, 79 insertions(+), 77 deletions(-) diff --git a/apt-pkg/cachefile.cc b/apt-pkg/cachefile.cc index 0116308e5..c070f4b9e 100644 --- a/apt-pkg/cachefile.cc +++ b/apt-pkg/cachefile.cc @@ -161,11 +161,11 @@ bool pkgCacheFile::BuildPolicy(OpProgress * /*Progress*/) if (_error->PendingError() == true) return false; - if (ReadPinFile(*Policy) == false || ReadPinDir(*Policy) == false) - return false; + ReadPinFile(*Policy); + ReadPinDir(*Policy); this->Policy = Policy.release(); - return true; + return _error->PendingError() == false; } /*}}}*/ // CacheFile::BuildDepCache - Open and build the dependency cache /*{{{*/ diff --git a/apt-pkg/contrib/configuration.cc b/apt-pkg/contrib/configuration.cc index 65242bec5..eb873bdba 100644 --- a/apt-pkg/contrib/configuration.cc +++ b/apt-pkg/contrib/configuration.cc @@ -1150,13 +1150,10 @@ bool ReadConfigFile(Configuration &Conf,const string &FName,bool const &AsSectio bool ReadConfigDir(Configuration &Conf,const string &Dir, bool const &AsSectional, unsigned const &Depth) { - vector const List = GetListOfFilesInDir(Dir, "conf", true, true); - - // Read the files - for (vector::const_iterator I = List.begin(); I != List.end(); ++I) - if (ReadConfigFile(Conf,*I,AsSectional,Depth) == false) - return false; - return true; + bool good = true; + for (auto const &I : GetListOfFilesInDir(Dir, "conf", true, true)) + good = ReadConfigFile(Conf, I, AsSectional, Depth) && good; + return good; } /*}}}*/ // MatchAgainstConfig Constructor /*{{{*/ diff --git a/apt-pkg/contrib/fileutl.cc b/apt-pkg/contrib/fileutl.cc index 7633b07ef..33f4f7e09 100644 --- a/apt-pkg/contrib/fileutl.cc +++ b/apt-pkg/contrib/fileutl.cc @@ -402,7 +402,10 @@ std::vector GetListOfFilesInDir(string const &Dir, std::vector c DIR *D = opendir(Dir.c_str()); if (D == 0) { - _error->Errno("opendir",_("Unable to read %s"),Dir.c_str()); + if (errno == EACCES) + _error->WarningE("opendir", _("Unable to read %s"), Dir.c_str()); + else + _error->Errno("opendir", _("Unable to read %s"), Dir.c_str()); return List; } @@ -3126,7 +3129,13 @@ bool DropPrivileges() /*{{{*/ /*}}}*/ bool OpenConfigurationFileFd(std::string const &File, FileFd &Fd) /*{{{*/ { + int const fd = open(File.c_str(), O_RDONLY | O_CLOEXEC | O_NOCTTY); + if (fd == -1) + return _error->WarningE("open", _("Unable to read %s"), File.c_str()); APT::Configuration::Compressor none(".", "", "", nullptr, nullptr, 0); - return Fd.Open(File, FileFd::ReadOnly, none); + if (Fd.OpenDescriptor(fd, FileFd::ReadOnly, none) == false) + return false; + Fd.SetFileName(File); + return true; } /*}}}*/ diff --git a/apt-pkg/contrib/fileutl.h b/apt-pkg/contrib/fileutl.h index 06c303809..19b4ed49e 100644 --- a/apt-pkg/contrib/fileutl.h +++ b/apt-pkg/contrib/fileutl.h @@ -161,6 +161,7 @@ class FileFd inline bool Eof() {return (Flags & HitEof) == HitEof;}; inline bool IsCompressed() {return (Flags & Compressed) == Compressed;}; inline std::string &Name() {return FileName;}; + inline void SetFileName(std::string const &name) { FileName = name; }; FileFd(std::string FileName,unsigned int const Mode,unsigned long AccessMode = 0666); FileFd(std::string FileName,unsigned int const Mode, CompressMode Compress, unsigned long AccessMode = 0666); diff --git a/apt-pkg/init.cc b/apt-pkg/init.cc index af4e6faa0..207f0d902 100644 --- a/apt-pkg/init.cc +++ b/apt-pkg/init.cc @@ -212,14 +212,13 @@ bool pkgInitConfig(Configuration &Cnf) Cnf.CndSet("Acquire::Changelogs::URI::Origin::Ultimedia", "http://packages.ultimediaos.com/changelogs/pool/@CHANGEPATH@/changelog.txt"); Cnf.CndSet("Acquire::Changelogs::AlwaysOnline::Origin::Ubuntu", true); - bool Res = true; - // Read an alternate config file + _error->PushToStack(); const char *Cfg = getenv("APT_CONFIG"); if (Cfg != 0 && strlen(Cfg) != 0) { if (RealFileExists(Cfg) == true) - Res &= ReadConfigFile(Cnf,Cfg); + ReadConfigFile(Cnf, Cfg); else _error->WarningE("RealFileExists",_("Unable to read %s"),Cfg); } @@ -227,30 +226,29 @@ bool pkgInitConfig(Configuration &Cnf) // Read the configuration parts dir std::string const Parts = Cnf.FindDir("Dir::Etc::parts", "/dev/null"); if (DirectoryExists(Parts) == true) - Res &= ReadConfigDir(Cnf,Parts); + ReadConfigDir(Cnf, Parts); else if (APT::String::Endswith(Parts, "/dev/null") == false) _error->WarningE("DirectoryExists",_("Unable to read %s"),Parts.c_str()); // Read the main config file std::string const FName = Cnf.FindFile("Dir::Etc::main", "/dev/null"); if (RealFileExists(FName) == true) - Res &= ReadConfigFile(Cnf,FName); - - if (Res == false) - return false; + ReadConfigFile(Cnf, FName); if (Cnf.FindB("Debug::pkgInitConfig",false) == true) Cnf.Dump(); - + #ifdef APT_DOMAIN if (Cnf.Exists("Dir::Locale")) - { + { bindtextdomain(APT_DOMAIN,Cnf.FindDir("Dir::Locale").c_str()); bindtextdomain(textdomain(0),Cnf.FindDir("Dir::Locale").c_str()); } #endif - return true; + auto const good = _error->PendingError() == false; + _error->MergeWithStack(); + return good; } /*}}}*/ // pkgInitSystem - Initialize the _system calss /*{{{*/ diff --git a/apt-pkg/policy.cc b/apt-pkg/policy.cc index 69c1fbe10..030bab26b 100644 --- a/apt-pkg/policy.cc +++ b/apt-pkg/policy.cc @@ -324,10 +324,10 @@ bool ReadPinDir(pkgPolicy &Plcy,string Dir) return false; // Read the files + bool good = true; for (vector::const_iterator I = List.begin(); I != List.end(); ++I) - if (ReadPinFile(Plcy, *I) == false) - return false; - return true; + good = ReadPinFile(Plcy, *I) && good; + return good; } /*}}}*/ // ReadPinFile - Load the pin file into a Policy /*{{{*/ diff --git a/apt-pkg/sourcelist.cc b/apt-pkg/sourcelist.cc index 9f0c7f8ae..adf598e48 100644 --- a/apt-pkg/sourcelist.cc +++ b/apt-pkg/sourcelist.cc @@ -300,37 +300,32 @@ pkgSourceList::~pkgSourceList() /* */ bool pkgSourceList::ReadMainList() { - // CNC:2003-03-03 - Multiple sources list support. - bool Res = true; -#if 0 - Res = ReadVendors(); - if (Res == false) - return false; -#endif - Reset(); // CNC:2003-11-28 - Entries in sources.list have priority over // entries in sources.list.d. string Main = _config->FindFile("Dir::Etc::sourcelist", "/dev/null"); string Parts = _config->FindDir("Dir::Etc::sourceparts", "/dev/null"); - + + _error->PushToStack(); if (RealFileExists(Main) == true) - Res &= ReadAppend(Main); + ReadAppend(Main); else if (DirectoryExists(Parts) == false && APT::String::Endswith(Parts, "/dev/null") == false) // Only warn if there are no sources.list.d. _error->WarningE("DirectoryExists", _("Unable to read %s"), Parts.c_str()); if (DirectoryExists(Parts) == true) - Res &= ReadSourceDir(Parts); + ReadSourceDir(Parts); else if (Main.empty() == false && RealFileExists(Main) == false && APT::String::Endswith(Parts, "/dev/null") == false) // Only warn if there is no sources.list file. _error->WarningE("RealFileExists", _("Unable to read %s"), Main.c_str()); for (auto && file: _config->FindVector("APT::Sources::With")) - Res &= AddVolatileFile(file, nullptr); + AddVolatileFile(file, nullptr); - return Res; + auto good = _error->PendingError() == false; + _error->MergeWithStack(); + return good; } /*}}}*/ // SourceList::Reset - Clear the sourcelist contents /*{{{*/ @@ -498,17 +493,12 @@ bool pkgSourceList::GetIndexes(pkgAcquire *Owner, bool GetAll) const /* */ bool pkgSourceList::ReadSourceDir(string const &Dir) { - std::vector ext; - ext.push_back("list"); - ext.push_back("sources"); - std::vector const List = GetListOfFilesInDir(Dir, ext, true); - + std::vector const ext = {"list", "sources"}; // Read the files - for (vector::const_iterator I = List.begin(); I != List.end(); ++I) - if (ReadAppend(*I) == false) - return false; - return true; - + bool good = true; + for (auto const &I : GetListOfFilesInDir(Dir, ext, true)) + good = ReadAppend(I) && good; + return good; } /*}}}*/ // GetLastModified() /*{{{*/ diff --git a/test/integration/test-bug-818628-unreadable-source b/test/integration/test-bug-818628-unreadable-source index 0c781c3b9..59051fb5f 100755 --- a/test/integration/test-bug-818628-unreadable-source +++ b/test/integration/test-bug-818628-unreadable-source @@ -16,6 +16,8 @@ insertpackage 'unstable' 'foo' 'amd64' '2' setupaptarchive --no-update +touch rootdir/etc/apt/sources.list.d/apt-test-unstable-deb-src.list +touch rootdir/etc/apt/sources.list.d/apt-test-unstable-deb-src.sources touch rootdir/etc/apt/apt.conf.d/unreadable.conf touch rootdir/etc/apt/preferences.d/unreadable.pref @@ -31,56 +33,61 @@ foo/unstable 2 amd64 [upgradable from: 1] N: There is 1 additional version. Please use the '-a' switch to see it" apt list --upgradable runthemall() { - local ERR1="$1" - local ERR2="$1$2" - testfailureequal "$ERR1" aptcache policy - testfailureequal "$ERR1" aptcache policy foo - testfailureequal "$ERR2" aptcache depends foo - testfailureequal "$ERR2" aptcache rdepends foo - testfailureequal "$ERR2" aptcache search foo - testfailureequal "$ERR1" apt policy - testfailureequal "$ERR1" apt policy foo - testfailureequal "$ERR2" apt depends foo - testfailureequal "$ERR2" apt rdepends foo - testfailureequal "$ERR2" apt search foo - testfailureequal "$ERR2" apt list --upgradable - testfailureequal "$ERR2" apt show foo - testfailureequal "$ERR2" aptcache show foo --no-all-versions - testfailureequal "$ERR2" aptmark auto foo - testfailureequal "$ERR2" aptmark manual foo - testfailureequal "$ERR2" aptmark auto foo + local ERR="$1" + local ERRNOTICEVER="$1${2- +N: There is 1 additional version. Please use the '-a' switch to see it}" + local ERRNOTICEREC="$1${2- +N: There is 1 additional record. Please use the '-a' switch to see it}" + testwarningmsg "$ERR" aptcache policy + testwarningmsg "$ERR" aptcache policy foo + testwarningmsg "$ERR" aptcache depends foo + testwarningmsg "$ERR" aptcache rdepends foo + testwarningmsg "$ERR" aptcache search foo + testwarningmsg "$ERR" apt policy + testwarningmsg "$ERR" apt policy foo + testwarningmsg "$ERR" apt depends foo + testwarningmsg "$ERR" apt rdepends foo + testwarningmsg "$ERR" apt search foo + testwarningmsg "$ERRNOTICEVER" apt list --upgradable + testwarningmsg "$ERRNOTICEREC" apt show foo + testwarningmsg "$ERRNOTICEREC" aptcache show foo --no-all-versions + testwarningmsg "$ERR" aptmark auto foo + testwarningmsg "$ERR" aptmark manual foo + testwarningmsg "$ERR" aptmark auto foo } echo 'Apt::Cmd::Disable-Script-Warning "true";' >> aptconfig.conf -msgmsg 'Unreadable sources file' +msgmsg 'Unreadable one-line-style sources file' chmod -r rootdir/etc/apt/sources.list.d/apt-test-unstable-deb-src.list -runthemall "E: Could not open file $TMPWORKINGDIRECTORY/rootdir/etc/apt/sources.list.d/apt-test-unstable-deb-src.list - open (13: Permission denied) -E: The list of sources could not be read." +runthemall "W: Unable to read $TMPWORKINGDIRECTORY/rootdir/etc/apt/sources.list.d/apt-test-unstable-deb-src.list - open (13: Permission denied)" chmod +r rootdir/etc/apt/sources.list.d/apt-test-unstable-deb-src.list +msgmsg 'Unreadable deb822-style sources file' +chmod -r rootdir/etc/apt/sources.list.d/apt-test-unstable-deb-src.sources +runthemall "W: Unable to read $TMPWORKINGDIRECTORY/rootdir/etc/apt/sources.list.d/apt-test-unstable-deb-src.sources - open (13: Permission denied)" +chmod +r rootdir/etc/apt/sources.list.d/apt-test-unstable-deb-src.sources + msgmsg 'Unreadable config file' chmod -r rootdir/etc/apt/apt.conf.d/unreadable.conf -runthemall "E: Could not open file ${TMPWORKINGDIRECTORY}/rootdir/etc/apt/apt.conf.d/unreadable.conf - open (13: Permission denied)" +runthemall "W: Unable to read ${TMPWORKINGDIRECTORY}/rootdir/etc/apt/apt.conf.d/unreadable.conf - open (13: Permission denied)" chmod +r rootdir/etc/apt/apt.conf.d/unreadable.conf msgmsg 'Unreadable preferences file' chmod -r rootdir/etc/apt/preferences.d/unreadable.pref -runthemall "E: Could not open file ${TMPWORKINGDIRECTORY}/rootdir/etc/apt/preferences.d/unreadable.pref - open (13: Permission denied)" +runthemall "W: Unable to read ${TMPWORKINGDIRECTORY}/rootdir/etc/apt/preferences.d/unreadable.pref - open (13: Permission denied)" chmod +r rootdir/etc/apt/preferences.d/unreadable.pref msgmsg 'Unreadable sources directory' chmod -r rootdir/etc/apt/sources.list.d -runthemall "E: Unable to read $TMPWORKINGDIRECTORY/rootdir/etc/apt/sources.list.d/ - opendir (13: Permission denied)" " -W: You may want to run apt-get update to correct these problems -E: The package cache file is corrupted" +runthemall "W: Unable to read $TMPWORKINGDIRECTORY/rootdir/etc/apt/sources.list.d/ - opendir (13: Permission denied)" "" chmod +r rootdir/etc/apt/sources.list.d msgmsg 'Unreadable config directory' chmod -r rootdir/etc/apt/apt.conf.d -runthemall "E: Unable to read ${TMPWORKINGDIRECTORY}/rootdir/etc/apt/apt.conf.d/ - opendir (13: Permission denied)" +runthemall "W: Unable to read ${TMPWORKINGDIRECTORY}/rootdir/etc/apt/apt.conf.d/ - opendir (13: Permission denied)" chmod +r rootdir/etc/apt/apt.conf.d msgmsg 'Unreadable preferences directory' chmod -r rootdir/etc/apt/preferences.d -runthemall "E: Unable to read ${TMPWORKINGDIRECTORY}/rootdir/etc/apt/preferences.d/ - opendir (13: Permission denied)" +runthemall "W: Unable to read ${TMPWORKINGDIRECTORY}/rootdir/etc/apt/preferences.d/ - opendir (13: Permission denied)" chmod +r rootdir/etc/apt/preferences.d -- cgit v1.2.3 From ea408c560ed85bb4ef7cf8f72f8463653501332c Mon Sep 17 00:00:00 2001 From: David Kalnischkies Date: Fri, 7 Jul 2017 16:24:21 +0200 Subject: reimplement and document auth.conf We have support for an netrc-like auth.conf file since 0.7.25 (closing 518473), but it was never documented in apt that it even exists and netrc seems to have fallen out of usage as a manpage for it no longer exists making the feature even more arcane. On top of that the code was a bit of a mess (as it is written in c-style) and as a result the matching of machine tokens to URIs also a bit strange by checking for less specific matches (= without path) first. We now do a single pass over the stanzas. In practice early adopters of the undocumented implementation will not really notice the differences and the 'new' behaviour is simpler to document and more usual for an apt user. Closes: #811181 --- CMake/config.h.in | 1 + apt-pkg/contrib/netrc.cc | 296 +++++++++++------------------ apt-pkg/contrib/netrc.h | 7 +- doc/CMakeLists.txt | 1 + doc/apt_auth.conf.5.xml | 132 +++++++++++++ methods/aptmethod.h | 19 ++ methods/basehttp.cc | 2 +- methods/basehttp.h | 2 +- methods/curl.cc | 2 +- methods/ftp.cc | 3 +- methods/http.cc | 5 +- test/integration/test-authentication-basic | 1 + test/libapt/authconf_test.cc | 223 ++++++++++++++++++++++ 13 files changed, 499 insertions(+), 195 deletions(-) create mode 100644 doc/apt_auth.conf.5.xml create mode 100644 test/libapt/authconf_test.cc diff --git a/CMake/config.h.in b/CMake/config.h.in index ee822e204..f5a03eedd 100644 --- a/CMake/config.h.in +++ b/CMake/config.h.in @@ -68,6 +68,7 @@ #define APT_8_CLEANER_HEADERS #define APT_9_CLEANER_HEADERS #define APT_10_CLEANER_HEADERS +#define APT_15_CLEANER_HEADERS /* unrolling is faster combined with an optimizing compiler */ #define SHA2_UNROLL_TRANSFORM diff --git a/apt-pkg/contrib/netrc.cc b/apt-pkg/contrib/netrc.cc index 88027c989..27511d413 100644 --- a/apt-pkg/contrib/netrc.cc +++ b/apt-pkg/contrib/netrc.cc @@ -14,205 +14,129 @@ #include #include +#include #include #include -#include -#include -#include -#include -#include -#include #include "netrc.h" -using std::string; - /* Get user and password from .netrc when given a machine name */ - -enum { - NOTHING, - HOSTFOUND, /* the 'machine' keyword was found */ - HOSTCOMPLETE, /* the machine name following the keyword was found too */ - HOSTVALID, /* this is "our" machine! */ - HOSTEND /* LAST enum */ -}; - -/* make sure we have room for at least this size: */ -#define LOGINSIZE 256 -#define PASSWORDSIZE 256 -#define NETRC DOT_CHAR "netrc" - -/* returns -1 on failure, 0 if the host is found, 1 is the host isn't found */ -static int parsenetrc_string (char *host, std::string &login, std::string &password, char *netrcfile = NULL) +bool MaybeAddAuth(FileFd &NetRCFile, URI &Uri) { - FILE *file; - int retcode = 1; - int specific_login = (login.empty() == false); - bool netrc_alloc = false; - - if (!netrcfile) { - char const * home = getenv ("HOME"); /* portable environment reader */ - - if (!home) { - struct passwd *pw; - pw = getpwuid (geteuid ()); - if(pw) - home = pw->pw_dir; - } - - if (!home) - return -1; - - if (asprintf (&netrcfile, "%s%s%s", home, DIR_CHAR, NETRC) == -1 || netrcfile == NULL) - return -1; - else - netrc_alloc = true; - } - - file = fopen (netrcfile, "r"); - if(file) { - char *tok; - char *tok_buf; - bool done = false; - char *netrcbuffer = NULL; - size_t netrcbuffer_size = 0; - - int state = NOTHING; - char state_login = 0; /* Found a login keyword */ - char state_password = 0; /* Found a password keyword */ - int state_our_login = false; /* With specific_login, - found *our* login name */ - - while (!done && getline(&netrcbuffer, &netrcbuffer_size, file) != -1) { - tok = strtok_r (netrcbuffer, " \t\n", &tok_buf); - while (!done && tok) { - if(login.empty() == false && password.empty() == false) { - done = true; - break; - } - - switch(state) { - case NOTHING: - if (!strcasecmp ("machine", tok)) { - /* the next tok is the machine name, this is in itself the - delimiter that starts the stuff entered for this machine, - after this we need to search for 'login' and - 'password'. */ - state = HOSTFOUND; - } - break; - case HOSTFOUND: - /* extended definition of a "machine" if we have a "/" - we match the start of the string (host.startswith(token) */ - if ((strchr(host, '/') && strstr(host, tok) == host) || - (!strcasecmp (host, tok))) { - /* and yes, this is our host! */ - state = HOSTVALID; - retcode = 0; /* we did find our host */ - } - else - /* not our host */ - state = NOTHING; - break; - case HOSTVALID: - /* we are now parsing sub-keywords regarding "our" host */ - if (state_login) { - if (specific_login) - state_our_login = !strcasecmp (login.c_str(), tok); - else - login = tok; - state_login = 0; - } else if (state_password) { - if (state_our_login || !specific_login) - password = tok; - state_password = 0; - } else if (!strcasecmp ("login", tok)) - state_login = 1; - else if (!strcasecmp ("password", tok)) - state_password = 1; - else if(!strcasecmp ("machine", tok)) { - /* ok, there's machine here go => */ - state = HOSTFOUND; - state_our_login = false; - } - break; - } /* switch (state) */ - - tok = strtok_r (NULL, " \t\n", &tok_buf); - } /* while(tok) */ - } /* while getline() */ - - free(netrcbuffer); - fclose(file); - } - - if (netrc_alloc) - free(netrcfile); - - return retcode; -} - -void maybe_add_auth (URI &Uri, string NetRCFile) -{ - if (_config->FindB("Debug::Acquire::netrc", false) == true) - std::clog << "maybe_add_auth: " << (string)Uri - << " " << NetRCFile << std::endl; - if (Uri.Password.empty () == true || Uri.User.empty () == true) - { - if (NetRCFile.empty () == false) - { - std::string login, password; - char *netrcfile = strdup(NetRCFile.c_str()); - - // first check for a generic host based netrc entry - char *host = strdup(Uri.Host.c_str()); - if (host && parsenetrc_string(host, login, password, netrcfile) == 0) + if (Uri.User.empty() == false || Uri.Password.empty() == false) + return true; + if (NetRCFile.IsOpen() == false || NetRCFile.Failed()) + return false; + auto const Debug = _config->FindB("Debug::Acquire::netrc", false); + + std::string lookfor; + if (Uri.Port != 0) + strprintf(lookfor, "%s:%i%s", Uri.Host.c_str(), Uri.Port, Uri.Path.c_str()); + else + lookfor.append(Uri.Host).append(Uri.Path); + + enum + { + NO, + MACHINE, + GOOD_MACHINE, + LOGIN, + PASSWORD + } active_token = NO; + std::string line; + while (NetRCFile.Eof() == false || line.empty() == false) + { + if (line.empty()) { - if (_config->FindB("Debug::Acquire::netrc", false) == true) - std::clog << "host: " << host - << " user: " << login - << " pass-size: " << password.size() - << std::endl; - Uri.User = login; - Uri.Password = password; - free(netrcfile); - free(host); - return; + if (NetRCFile.ReadLine(line) == false) + break; + else if (line.empty()) + continue; } - free(host); - - // if host did not work, try Host+Path next, this will trigger - // a lookup uri.startswith(host) in the netrc file parser (because - // of the "/" - char *hostpath = strdup((Uri.Host + Uri.Path).c_str()); - if (hostpath && parsenetrc_string(hostpath, login, password, netrcfile) == 0) + auto tokenend = line.find_first_of("\t "); + std::string token; + if (tokenend != std::string::npos) + { + token = line.substr(0, tokenend); + line.erase(0, tokenend + 1); + } + else + std::swap(line, token); + if (token.empty()) + continue; + switch (active_token) { - if (_config->FindB("Debug::Acquire::netrc", false) == true) - std::clog << "hostpath: " << hostpath - << " user: " << login - << " pass-size: " << password.size() - << std::endl; - Uri.User = login; - Uri.Password = password; + case NO: + if (token == "machine") + active_token = MACHINE; + break; + case MACHINE: + if (token.find('/') == std::string::npos) + { + if (Uri.Port != 0 && Uri.Host == token) + active_token = GOOD_MACHINE; + else if (lookfor.compare(0, lookfor.length() - Uri.Path.length(), token) == 0) + active_token = GOOD_MACHINE; + else + active_token = NO; + } + else + { + if (APT::String::Startswith(lookfor, token)) + active_token = GOOD_MACHINE; + else + active_token = NO; + } + break; + case GOOD_MACHINE: + if (token == "login") + active_token = LOGIN; + else if (token == "password") + active_token = PASSWORD; + else if (token == "machine") + { + if (Debug) + std::clog << "MaybeAddAuth: Found matching host adding '" << Uri.User << "' and '" << Uri.Password << "' for " + << (std::string)Uri << " from " << NetRCFile.Name() << std::endl; + return true; + } + break; + case LOGIN: + std::swap(Uri.User, token); + active_token = GOOD_MACHINE; + break; + case PASSWORD: + std::swap(Uri.Password, token); + active_token = GOOD_MACHINE; + break; } - free(netrcfile); - free(hostpath); - } - } + } + if (active_token == GOOD_MACHINE) + { + if (Debug) + std::clog << "MaybeAddAuth: Found matching host adding '" << Uri.User << "' and '" << Uri.Password << "' for " + << (std::string)Uri << " from " << NetRCFile.Name() << std::endl; + return true; + } + else if (active_token == NO) + { + if (Debug) + std::clog << "MaybeAddAuth: Found no matching host for " + << (std::string)Uri << " from " << NetRCFile.Name() << std::endl; + return true; + } + else if (Debug) + std::clog << "MaybeAddAuth: Found no matching host (syntax error: " << active_token << ") for " + << (std::string)Uri << " from " << NetRCFile.Name() << std::endl; + return false; } -#ifdef DEBUG -int main(int argc, char* argv[]) +void maybe_add_auth(URI &Uri, std::string NetRCFile) { - char login[64] = ""; - char password[64] = ""; - - if(argc < 2) - return -1; - - if(0 == parsenetrc (argv[1], login, password, argv[2])) { - printf("HOST: %s LOGIN: %s PASSWORD: %s\n", argv[1], login, password); - } + if (FileExists(NetRCFile) == false) + return; + FileFd fd; + if (fd.Open(NetRCFile, FileFd::ReadOnly)) + MaybeAddAuth(fd, Uri); } -#endif diff --git a/apt-pkg/contrib/netrc.h b/apt-pkg/contrib/netrc.h index b5b56f5d4..46d8cab3d 100644 --- a/apt-pkg/contrib/netrc.h +++ b/apt-pkg/contrib/netrc.h @@ -22,10 +22,15 @@ #include #endif +#ifndef APT_15_CLEANER_HEADERS #define DOT_CHAR "." #define DIR_CHAR "/" +#endif class URI; +class FileFd; -void maybe_add_auth (URI &Uri, std::string NetRCFile); +APT_DEPRECATED_MSG("Use FileFd-based MaybeAddAuth instead") +void maybe_add_auth(URI &Uri, std::string NetRCFile); +bool MaybeAddAuth(FileFd &NetRCFile, URI &Uri); #endif diff --git a/doc/CMakeLists.txt b/doc/CMakeLists.txt index a1491428f..d7241eb5e 100644 --- a/doc/CMakeLists.txt +++ b/doc/CMakeLists.txt @@ -66,6 +66,7 @@ endif() add_docbook(apt-man MANPAGE ALL DOCUMENTS apt.8.xml + apt_auth.conf.5.xml apt-cache.8.xml apt-cdrom.8.xml apt.conf.5.xml diff --git a/doc/apt_auth.conf.5.xml b/doc/apt_auth.conf.5.xml new file mode 100644 index 000000000..8a1882604 --- /dev/null +++ b/doc/apt_auth.conf.5.xml @@ -0,0 +1,132 @@ + + %aptent; + %aptverbatiment; + %aptvendor; +]> + + + + + &apt-author.team; + &apt-email; + &apt-product; + + 2017-07-07T00:00:00Z + + + + apt_auth.conf + 5 + APT + + + + + apt_auth.conf + Login configuration file for APT sources and proxies + + +Description +APT configuration files like &sources-list; or &apt-conf; need to be accessible +for everyone using apt tools on the system to have access to all package-related +information like the available packages in a repository. Login information +needed to connect to a proxy or to download data from a repository on the other +hand shouldn't always be accessible by everyone and can hence not be placed in a +file with world-readable file permissions. + +The APT auth.conf file /etc/apt/auth.conf can be used to store +login information in a netrc-like format with restrictive file permissions. + + +netrc-like format +The format defined here is similar to the format of the ~/.netrc +file used by ftp1 +and similar programs interacting with servers. +It is a simple token-based format with the following tokens being recognized; +Unknown tokens will be ignored. Tokens may be separated by spaces, tabs or newlines. + + + +machine hostname[:port][/path] +Entries are looked up by searching for the +machine token matching the +hostname of the URI apt needs login information for. Extending the netrc-format +a portnumber can be specified. If no port is given the token matches for all ports. +Similar the path is optional and only needed and useful if multiple repositories with +different login information reside on the same server. A machine token with a path +matches if the path in the URI starts with the path given in the token. +Once a match is made, the subsequent tokens are processed, stopping when the +end of file is reached or another machine +token is encountered. + + + +login name +The username to be used. + + + +password string +The password to be used. + + + + + + +Example +Supplying login information for a user named apt +with the password debian for the &sources-list; entry +deb http://example.org/debian &debian-stable-codename; main +could be done in the entry directly: +deb http://apt:debian@example.org/debian &debian-stable-codename; main +Alternatively an entry like the following in the auth.conf file could be used: +machine example.org +login apt +password debian +Or alternatively within a single line: +machine example.org login apt password debian +If you need to be more specific all of these lines will also apply to the example entry: +machine example.org/deb login apt password debian +machine example.org/debian login apt password debian +machine example.org/debian/ login apt password debian + +On the other hand neither of the following lines apply: +machine example.org:80 login apt password debian +machine example.org/deb/ login apt password debian +machine example.org/ubuntu login apt password debian +machine example.orga login apt password debian +machine example.net login apt password debian + + + +Notes +Basic support for this feature is present since version 0.7.25, but was +undocumented for years. The documentation was added in version 1.5 changing +also the implementation slightly. For maximum backward compatibility you should +avoid multiple machine tokens with the same hostname, but if +you need multiple they should all have a path specified in the +machine token. + + + +Files + + /etc/apt/auth.conf + Login information for APT sources and proxies in a netrc-like format. + Configuration Item: Dir::Etc::netrc. + + + + + +See Also +&apt-conf; &sources-list; + + + + &manbugs; + + diff --git a/methods/aptmethod.h b/methods/aptmethod.h index 04858e29d..a9af63fb7 100644 --- a/methods/aptmethod.h +++ b/methods/aptmethod.h @@ -5,6 +5,7 @@ #include #include #include +#include #include #include @@ -42,6 +43,24 @@ public: return true; } + bool MaybeAddAuthTo(URI &uri) + { + if (uri.User.empty() == false || uri.Password.empty() == false) + return true; + auto const netrc = _config->FindFile("Dir::Etc::netrc"); + if (netrc.empty() == true) + return true; + // ignore errors with opening the auth file as it doesn't need to exist + _error->PushToStack(); + FileFd authconf(netrc, FileFd::ReadOnly); + _error->RevertToStack(); + if (authconf.IsOpen() == false) + return true; + if (authconf.Seek(0) == false) + return false; + return MaybeAddAuth(authconf, uri); + } + bool CalculateHashes(FetchItem const * const Itm, FetchResult &Res) const APT_NONNULL(2) { Hashes Hash(Itm->ExpectedHashes); diff --git a/methods/basehttp.cc b/methods/basehttp.cc index cc5039c75..03409a8d4 100644 --- a/methods/basehttp.cc +++ b/methods/basehttp.cc @@ -845,7 +845,7 @@ bool BaseHttpMethod::Configuration(std::string Message) /*{{{*/ return true; } /*}}}*/ -bool BaseHttpMethod::AddProxyAuth(URI &Proxy, URI const &Server) const /*{{{*/ +bool BaseHttpMethod::AddProxyAuth(URI &Proxy, URI const &Server) /*{{{*/ { if (std::find(methodNames.begin(), methodNames.end(), "tor") != methodNames.end() && Proxy.User == "apt-transport-tor" && Proxy.Password.empty()) diff --git a/methods/basehttp.h b/methods/basehttp.h index 7000e7b89..d2e085968 100644 --- a/methods/basehttp.h +++ b/methods/basehttp.h @@ -164,7 +164,7 @@ class BaseHttpMethod : public aptMethod virtual void RotateDNS() = 0; virtual bool Configuration(std::string Message) APT_OVERRIDE; - bool AddProxyAuth(URI &Proxy, URI const &Server) const; + bool AddProxyAuth(URI &Proxy, URI const &Server); BaseHttpMethod(std::string &&Binary, char const * const Ver,unsigned long const Flags); virtual ~BaseHttpMethod() {}; diff --git a/methods/curl.cc b/methods/curl.cc index 71149217a..8e06d858d 100644 --- a/methods/curl.cc +++ b/methods/curl.cc @@ -270,7 +270,7 @@ bool HttpsMethod::Fetch(FetchItem *Itm) if (SetupProxy() == false) return _error->Error("Unsupported proxy configured: %s", URI::SiteOnly(Proxy).c_str()); - maybe_add_auth (Uri, _config->FindFile("Dir::Etc::netrc")); + MaybeAddAuthTo(Uri); if (Server == nullptr || Server->Comp(Itm->Uri) == false) Server = CreateServerState(Itm->Uri); diff --git a/methods/ftp.cc b/methods/ftp.cc index 4972337e3..c5c4001fa 100644 --- a/methods/ftp.cc +++ b/methods/ftp.cc @@ -21,7 +21,6 @@ #include #include #include -#include #include #include @@ -1015,7 +1014,7 @@ bool FtpMethod::Fetch(FetchItem *Itm) Res.Filename = Itm->DestFile; Res.IMSHit = false; - maybe_add_auth (Get, _config->FindFile("Dir::Etc::netrc")); + MaybeAddAuthTo(Get); // Connect to the server if (Server == 0 || Server->Comp(Get) == false) diff --git a/methods/http.cc b/methods/http.cc index db4542981..f0cd77139 100644 --- a/methods/http.cc +++ b/methods/http.cc @@ -23,7 +23,6 @@ #include #include #include -#include #include #include @@ -350,7 +349,7 @@ bool UnwrapHTTPConnect(std::string Host, int Port, URI Proxy, std::unique_ptrFindFile("Dir::Etc::netrc")); + Owner->MaybeAddAuthTo(Proxy); if (Proxy.User.empty() == false || Proxy.Password.empty() == false) Req << "Proxy-Authorization: Basic " << Base64Encode(Proxy.User + ":" + Proxy.Password) << "\r\n"; @@ -931,7 +930,7 @@ void HttpMethod::SendReq(FetchItem *Itm) Req << "Proxy-Authorization: Basic " << Base64Encode(Server->Proxy.User + ":" + Server->Proxy.Password) << "\r\n"; - maybe_add_auth (Uri, _config->FindFile("Dir::Etc::netrc")); + MaybeAddAuthTo(Uri); if (Uri.User.empty() == false || Uri.Password.empty() == false) Req << "Authorization: Basic " << Base64Encode(Uri.User + ":" + Uri.Password) << "\r\n"; diff --git a/test/integration/test-authentication-basic b/test/integration/test-authentication-basic index 3bfd076ce..e724e243e 100755 --- a/test/integration/test-authentication-basic +++ b/test/integration/test-authentication-basic @@ -13,6 +13,7 @@ setupaptarchive --no-update changetohttpswebserver --authorization="$(printf '%s' 'star@irc:hunter2' | base64 )" echo 'See, when YOU type hunter2, it shows to us as *******' > aptarchive/bash +echo 'Debug::Acquire::netrc "true";' > rootdir/etc/apt/apt.conf.d/netrcdebug.conf testauthfailure() { testfailure apthelper download-file "${1}/bash" ./downloaded/bash diff --git a/test/libapt/authconf_test.cc b/test/libapt/authconf_test.cc new file mode 100644 index 000000000..10d818ec9 --- /dev/null +++ b/test/libapt/authconf_test.cc @@ -0,0 +1,223 @@ +#include + +#include +#include +#include + +#include + +#include + +#include "file-helpers.h" + +TEST(NetRCTest, Parsing) +{ + FileFd fd; + URI U("http://file.not/open"); + EXPECT_FALSE(MaybeAddAuth(fd, U)); + EXPECT_TRUE(U.User.empty()); + EXPECT_TRUE(U.Password.empty()); + EXPECT_EQ("file.not", U.Host); + EXPECT_EQ("/open", U.Path); + + createTemporaryFile("doublesignedfile", fd, nullptr, R"apt( +machine example.netter login bar password foo +machine example.net login foo password bar + +machine example.org:90 login apt password apt +machine example.org:8080 +login +example password foobar + +machine example.org +login anonymous +password pass + +machine example.com/foo login user1 unknown token password pass1 +machine example.com/bar password pass2 login user2 + unknown token +machine example.com/user login user +machine example.netter login unused password firstentry +machine example.last/debian login debian password rules)apt"); + U = URI("http://example.net/foo"); + EXPECT_TRUE(MaybeAddAuth(fd, U)); + EXPECT_EQ("foo", U.User); + EXPECT_EQ("bar", U.Password); + EXPECT_EQ("example.net", U.Host); + EXPECT_EQ("/foo", U.Path); + + EXPECT_TRUE(fd.Seek(0)); + U = URI("http://user:pass@example.net/foo"); + EXPECT_TRUE(MaybeAddAuth(fd, U)); + EXPECT_EQ("user", U.User); + EXPECT_EQ("pass", U.Password); + EXPECT_EQ("example.net", U.Host); + EXPECT_EQ("/foo", U.Path); + + EXPECT_TRUE(fd.Seek(0)); + U = URI("http://example.org:90/foo"); + EXPECT_TRUE(MaybeAddAuth(fd, U)); + EXPECT_EQ("apt", U.User); + EXPECT_EQ("apt", U.Password); + EXPECT_EQ("example.org", U.Host); + EXPECT_EQ(90, U.Port); + EXPECT_EQ("/foo", U.Path); + + EXPECT_TRUE(fd.Seek(0)); + U = URI("http://example.org:8080/foo"); + EXPECT_TRUE(MaybeAddAuth(fd, U)); + EXPECT_EQ("example", U.User); + EXPECT_EQ("foobar", U.Password); + + EXPECT_TRUE(fd.Seek(0)); + U = URI("http://example.net:42/foo"); + EXPECT_TRUE(MaybeAddAuth(fd, U)); + EXPECT_EQ("foo", U.User); + EXPECT_EQ("bar", U.Password); + + EXPECT_TRUE(fd.Seek(0)); + U = URI("http://example.org/foo"); + EXPECT_TRUE(MaybeAddAuth(fd, U)); + EXPECT_EQ("anonymous", U.User); + EXPECT_EQ("pass", U.Password); + + EXPECT_TRUE(fd.Seek(0)); + U = URI("http://example.com/apt"); + EXPECT_TRUE(MaybeAddAuth(fd, U)); + EXPECT_TRUE(U.User.empty()); + EXPECT_TRUE(U.Password.empty()); + + EXPECT_TRUE(fd.Seek(0)); + U = URI("http://example.com/foo"); + EXPECT_TRUE(MaybeAddAuth(fd, U)); + EXPECT_EQ("user1", U.User); + EXPECT_EQ("pass1", U.Password); + + EXPECT_TRUE(fd.Seek(0)); + U = URI("http://example.com/fooo"); + EXPECT_TRUE(MaybeAddAuth(fd, U)); + EXPECT_EQ("user1", U.User); + EXPECT_EQ("pass1", U.Password); + + EXPECT_TRUE(fd.Seek(0)); + U = URI("http://example.com/fo"); + EXPECT_TRUE(MaybeAddAuth(fd, U)); + EXPECT_TRUE(U.User.empty()); + EXPECT_TRUE(U.Password.empty()); + + EXPECT_TRUE(fd.Seek(0)); + U = URI("http://example.com/bar"); + EXPECT_TRUE(MaybeAddAuth(fd, U)); + EXPECT_EQ("user2", U.User); + EXPECT_EQ("pass2", U.Password); + + EXPECT_TRUE(fd.Seek(0)); + U = URI("http://example.com/user"); + EXPECT_TRUE(MaybeAddAuth(fd, U)); + EXPECT_EQ("user", U.User); + EXPECT_TRUE(U.Password.empty()); + + EXPECT_TRUE(fd.Seek(0)); + U = URI("socks5h://example.last/debian"); + EXPECT_TRUE(MaybeAddAuth(fd, U)); + EXPECT_EQ("debian", U.User); + EXPECT_EQ("rules", U.Password); + + EXPECT_TRUE(fd.Seek(0)); + U = URI("socks5h://example.debian/"); + EXPECT_TRUE(MaybeAddAuth(fd, U)); + EXPECT_TRUE(U.User.empty()); + EXPECT_TRUE(U.Password.empty()); + + EXPECT_TRUE(fd.Seek(0)); + U = URI("socks5h://user:pass@example.debian/"); + EXPECT_TRUE(MaybeAddAuth(fd, U)); + EXPECT_EQ("user", U.User); + EXPECT_EQ("pass", U.Password); +} +TEST(NetRCTest, BadFileNoMachine) +{ + FileFd fd; + createTemporaryFile("doublesignedfile", fd, nullptr, R"apt( +foo example.org login foo1 password bar +machin example.org login foo2 password bar +machine2 example.org login foo3 password bar +)apt"); + + URI U("http://example.org/foo"); + EXPECT_TRUE(MaybeAddAuth(fd, U)); + EXPECT_TRUE(U.User.empty()); + EXPECT_TRUE(U.Password.empty()); +} +TEST(NetRCTest, BadFileEndsMachine) +{ + FileFd fd; + createTemporaryFile("doublesignedfile", fd, nullptr, R"apt( +machine example.org login foo1 password bar +machine)apt"); + + URI U("http://example.org/foo"); + EXPECT_TRUE(MaybeAddAuth(fd, U)); + EXPECT_EQ("foo1", U.User); + EXPECT_EQ("bar", U.Password); + + EXPECT_TRUE(fd.Seek(0)); + U = URI("http://example.net/foo"); + EXPECT_FALSE(MaybeAddAuth(fd, U)); + EXPECT_TRUE(U.User.empty()); + EXPECT_TRUE(U.Password.empty()); + + EXPECT_TRUE(fd.Seek(0)); + U = URI("http://foo:bar@example.net/foo"); + EXPECT_TRUE(MaybeAddAuth(fd, U)); + EXPECT_EQ("foo", U.User); + EXPECT_EQ("bar", U.Password); +} +TEST(NetRCTest, BadFileEndsLogin) +{ + FileFd fd; + createTemporaryFile("doublesignedfile", fd, nullptr, R"apt( +machine example.org login foo1 password bar +machine example.net login)apt"); + + URI U("http://example.org/foo"); + EXPECT_TRUE(MaybeAddAuth(fd, U)); + EXPECT_EQ("foo1", U.User); + EXPECT_EQ("bar", U.Password); + + EXPECT_TRUE(fd.Seek(0)); + U = URI("http://example.net/foo"); + EXPECT_FALSE(MaybeAddAuth(fd, U)); + EXPECT_TRUE(U.User.empty()); + EXPECT_TRUE(U.Password.empty()); + + EXPECT_TRUE(fd.Seek(0)); + U = URI("http://foo:bar@example.net/foo"); + EXPECT_TRUE(MaybeAddAuth(fd, U)); + EXPECT_EQ("foo", U.User); + EXPECT_EQ("bar", U.Password); +} +TEST(NetRCTest, BadFileEndsPassword) +{ + FileFd fd; + createTemporaryFile("doublesignedfile", fd, nullptr, R"apt( +machine example.org login foo1 password bar +machine example.net password)apt"); + + URI U("http://example.org/foo"); + EXPECT_TRUE(MaybeAddAuth(fd, U)); + EXPECT_EQ("foo1", U.User); + EXPECT_EQ("bar", U.Password); + + EXPECT_TRUE(fd.Seek(0)); + U = URI("http://example.net/foo"); + EXPECT_FALSE(MaybeAddAuth(fd, U)); + EXPECT_TRUE(U.User.empty()); + EXPECT_TRUE(U.Password.empty()); + + EXPECT_TRUE(fd.Seek(0)); + U = URI("http://foo:bar@example.net/foo"); + EXPECT_TRUE(MaybeAddAuth(fd, U)); + EXPECT_EQ("foo", U.User); + EXPECT_EQ("bar", U.Password); +} -- cgit v1.2.3 From 6291fa81da6ed4c32d0dde33fa559cd155faff11 Mon Sep 17 00:00:00 2001 From: David Kalnischkies Date: Fri, 7 Jul 2017 21:59:01 +0200 Subject: lookup login info for proxies in auth.conf On HTTP Connect we since recently look into the auth.conf file for login information, so we should really look for all proxies into the file as the argument is the same as for sources entries and it is easier to document (especially as the manpage already mentions it as supported). --- methods/basehttp.cc | 2 +- test/integration/test-authentication-basic | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/methods/basehttp.cc b/methods/basehttp.cc index 03409a8d4..1a3566479 100644 --- a/methods/basehttp.cc +++ b/methods/basehttp.cc @@ -847,6 +847,7 @@ bool BaseHttpMethod::Configuration(std::string Message) /*{{{*/ /*}}}*/ bool BaseHttpMethod::AddProxyAuth(URI &Proxy, URI const &Server) /*{{{*/ { + MaybeAddAuthTo(Proxy); if (std::find(methodNames.begin(), methodNames.end(), "tor") != methodNames.end() && Proxy.User == "apt-transport-tor" && Proxy.Password.empty()) { @@ -857,7 +858,6 @@ bool BaseHttpMethod::AddProxyAuth(URI &Proxy, URI const &Server) /*{{{*/ else Proxy.Password = std::move(pass); } - // FIXME: should we support auth.conf for proxies? return true; } /*}}}*/ diff --git a/test/integration/test-authentication-basic b/test/integration/test-authentication-basic index e724e243e..d29b38256 100755 --- a/test/integration/test-authentication-basic +++ b/test/integration/test-authentication-basic @@ -95,7 +95,9 @@ rewritesourceslist "http://localhost:${APTHTTPPORT}" msgmsg 'proxy to server basic auth' webserverconfig 'aptwebserver::request::absolute' 'uri' -export http_proxy="http://localhost:${APTHTTPPORT}" +# using ip instead of localhost avoids picking up the auth for the repo +# for the proxy as well as we serve them both over the same server… +export http_proxy="http://127.0.0.1:${APTHTTPPORT}" runtest "http://localhost:${APTHTTPPORT}" unset http_proxy -- cgit v1.2.3 From 881ec045b6660e2fe0c6953720260e380ceeeb99 Mon Sep 17 00:00:00 2001 From: David Kalnischkies Date: Fri, 7 Jul 2017 22:21:44 +0200 Subject: allow the auth.conf to be root:root owned Opening the file before we drop privileges in the methods allows us to avoid chowning in the acquire main process which can apply to the wrong file (imagine Binary scoped settings) and surprises users as their permission setup is overridden. There are no security benefits as the file is open, so an evil method could as before read the contents of the file, but it isn't worse than before and we avoid permission problems in this setup. --- apt-pkg/acquire.cc | 15 -------------- methods/aptmethod.h | 57 ++++++++++++++++++++++++++++++++++++----------------- methods/basehttp.cc | 4 ++-- methods/basehttp.h | 2 +- methods/ftp.cc | 4 ++-- methods/ftp.h | 2 +- methods/http.cc | 3 +-- methods/http.h | 2 +- 8 files changed, 47 insertions(+), 42 deletions(-) diff --git a/apt-pkg/acquire.cc b/apt-pkg/acquire.cc index 5e5e146a8..9272c2402 100644 --- a/apt-pkg/acquire.cc +++ b/apt-pkg/acquire.cc @@ -74,21 +74,6 @@ void pkgAcquire::Initialize() QueueMode = QueueHost; if (strcasecmp(Mode.c_str(),"access") == 0) QueueMode = QueueAccess; - - // chown the auth.conf file as it will be accessed by our methods - std::string const SandboxUser = _config->Find("APT::Sandbox::User"); - if (getuid() == 0 && SandboxUser.empty() == false && SandboxUser != "root") // if we aren't root, we can't chown, so don't try it - { - struct passwd const * const pw = getpwnam(SandboxUser.c_str()); - struct group const * const gr = getgrnam(ROOT_GROUP); - if (pw != NULL && gr != NULL) - { - std::string const AuthConf = _config->FindFile("Dir::Etc::netrc"); - if(AuthConf.empty() == false && RealFileExists(AuthConf) && - chown(AuthConf.c_str(), pw->pw_uid, gr->gr_gid) != 0) - _error->WarningE("SetupAPTPartialDirectory", "chown to %s:root of file %s failed", SandboxUser.c_str(), AuthConf.c_str()); - } - } } /*}}}*/ // Acquire::GetLock - lock directory and prepare for action /*{{{*/ diff --git a/methods/aptmethod.h b/methods/aptmethod.h index a9af63fb7..23fd036dd 100644 --- a/methods/aptmethod.h +++ b/methods/aptmethod.h @@ -43,24 +43,6 @@ public: return true; } - bool MaybeAddAuthTo(URI &uri) - { - if (uri.User.empty() == false || uri.Password.empty() == false) - return true; - auto const netrc = _config->FindFile("Dir::Etc::netrc"); - if (netrc.empty() == true) - return true; - // ignore errors with opening the auth file as it doesn't need to exist - _error->PushToStack(); - FileFd authconf(netrc, FileFd::ReadOnly); - _error->RevertToStack(); - if (authconf.IsOpen() == false) - return true; - if (authconf.Seek(0) == false) - return false; - return MaybeAddAuth(authconf, uri); - } - bool CalculateHashes(FetchItem const * const Itm, FetchResult &Res) const APT_NONNULL(2) { Hashes Hash(Itm->ExpectedHashes); @@ -167,5 +149,44 @@ public: } } }; +class aptAuthConfMethod : public aptMethod +{ + FileFd authconf; +public: + virtual bool Configuration(std::string Message) APT_OVERRIDE + { + if (pkgAcqMethod::Configuration(Message) == false) + return false; + + std::string const conf = std::string("Binary::") + Binary; + _config->MoveSubTree(conf.c_str(), NULL); + auto const netrc = _config->FindFile("Dir::Etc::netrc"); + if (netrc.empty() == false) + { + // ignore errors with opening the auth file as it doesn't need to exist + _error->PushToStack(); + authconf.Open(netrc, FileFd::ReadOnly); + _error->RevertToStack(); + } + + DropPrivsOrDie(); + + return true; + } + + bool MaybeAddAuthTo(URI &uri) + { + if (uri.User.empty() == false || uri.Password.empty() == false) + return true; + if (authconf.IsOpen() == false) + return true; + if (authconf.Seek(0) == false) + return false; + return MaybeAddAuth(authconf, uri); + } + + aptAuthConfMethod(std::string &&Binary, char const * const Ver, unsigned long const Flags) APT_NONNULL(3) : + aptMethod(std::move(Binary), Ver, Flags) {} +}; #endif diff --git a/methods/basehttp.cc b/methods/basehttp.cc index 1a3566479..0eb617f89 100644 --- a/methods/basehttp.cc +++ b/methods/basehttp.cc @@ -830,14 +830,14 @@ unsigned long long BaseHttpMethod::FindMaximumObjectSizeInQueue() const /*{{{*/ } /*}}}*/ BaseHttpMethod::BaseHttpMethod(std::string &&Binary, char const * const Ver,unsigned long const Flags) :/*{{{*/ - aptMethod(std::move(Binary), Ver, Flags), Server(nullptr), PipelineDepth(10), + aptAuthConfMethod(std::move(Binary), Ver, Flags), Server(nullptr), PipelineDepth(10), AllowRedirect(false), Debug(false) { } /*}}}*/ bool BaseHttpMethod::Configuration(std::string Message) /*{{{*/ { - if (aptMethod::Configuration(Message) == false) + if (aptAuthConfMethod::Configuration(Message) == false) return false; _config->CndSet("Acquire::tor::Proxy", diff --git a/methods/basehttp.h b/methods/basehttp.h index d2e085968..aadd59168 100644 --- a/methods/basehttp.h +++ b/methods/basehttp.h @@ -115,7 +115,7 @@ struct ServerState virtual ~ServerState() {}; }; -class BaseHttpMethod : public aptMethod +class BaseHttpMethod : public aptAuthConfMethod { protected: virtual bool Fetch(FetchItem *) APT_OVERRIDE; diff --git a/methods/ftp.cc b/methods/ftp.cc index c5c4001fa..341230f69 100644 --- a/methods/ftp.cc +++ b/methods/ftp.cc @@ -960,7 +960,7 @@ bool FTPConn::Get(const char *Path,FileFd &To,unsigned long long Resume, // FtpMethod::FtpMethod - Constructor /*{{{*/ // --------------------------------------------------------------------- /* */ -FtpMethod::FtpMethod() : aptMethod("ftp","1.0",SendConfig) +FtpMethod::FtpMethod() : aptAuthConfMethod("ftp", "1.0", SendConfig) { signal(SIGTERM,SigTerm); signal(SIGINT,SigTerm); @@ -995,7 +995,7 @@ void FtpMethod::SigTerm(int) /* We stash the desired pipeline depth */ bool FtpMethod::Configuration(string Message) { - if (aptMethod::Configuration(Message) == false) + if (aptAuthConfMethod::Configuration(Message) == false) return false; TimeOut = _config->FindI("Acquire::Ftp::Timeout",TimeOut); diff --git a/methods/ftp.h b/methods/ftp.h index 67d00d9f1..1859ddce0 100644 --- a/methods/ftp.h +++ b/methods/ftp.h @@ -72,7 +72,7 @@ class FTPConn ~FTPConn(); }; -class FtpMethod : public aptMethod +class FtpMethod : public aptAuthConfMethod { virtual bool Fetch(FetchItem *Itm) APT_OVERRIDE; virtual bool Configuration(std::string Message) APT_OVERRIDE; diff --git a/methods/http.cc b/methods/http.cc index f0cd77139..fc22180d3 100644 --- a/methods/http.cc +++ b/methods/http.cc @@ -329,7 +329,7 @@ struct HttpConnectFd : public MethodFd }; bool UnwrapHTTPConnect(std::string Host, int Port, URI Proxy, std::unique_ptr &Fd, - unsigned long Timeout, aptMethod *Owner) + unsigned long Timeout, aptAuthConfMethod *Owner) { Owner->Status(_("Connecting to %s (%s)"), "HTTP proxy", URI::SiteOnly(Proxy).c_str()); // The HTTP server expects a hostname with a trailing :port @@ -347,7 +347,6 @@ bool UnwrapHTTPConnect(std::string Host, int Port, URI Proxy, std::unique_ptrMaybeAddAuthTo(Proxy); if (Proxy.User.empty() == false || Proxy.Password.empty() == false) diff --git a/methods/http.h b/methods/http.h index 7a763675c..6d44fbdd4 100644 --- a/methods/http.h +++ b/methods/http.h @@ -93,7 +93,7 @@ class CircleBuf ~CircleBuf(); }; -bool UnwrapHTTPConnect(std::string To, int Port, URI Proxy, std::unique_ptr &Fd, unsigned long Timeout, aptMethod *Owner); +bool UnwrapHTTPConnect(std::string To, int Port, URI Proxy, std::unique_ptr &Fd, unsigned long Timeout, aptAuthConfMethod *Owner); struct HttpServerState: public ServerState { -- cgit v1.2.3 From afd7cd688b70bd50d8fa90199a2ac39d98edf19f Mon Sep 17 00:00:00 2001 From: David Kalnischkies Date: Sat, 8 Jul 2017 11:42:02 +0200 Subject: update URI scheme descriptions in sources.list(5) --- doc/apt-verbatim.ent | 6 +++++ doc/sources.list.5.xml | 61 +++++++++++++++++++++++++++++++++++--------------- 2 files changed, 49 insertions(+), 18 deletions(-) diff --git a/doc/apt-verbatim.ent b/doc/apt-verbatim.ent index b555c5de5..be599d393 100644 --- a/doc/apt-verbatim.ent +++ b/doc/apt-verbatim.ent @@ -15,6 +15,12 @@ " > + + apt_auth.conf + 5 + " +> + apt-get 8 diff --git a/doc/sources.list.5.xml b/doc/sources.list.5.xml index dd057eb32..c4df9aa58 100644 --- a/doc/sources.list.5.xml +++ b/doc/sources.list.5.xml @@ -350,6 +350,40 @@ deb-src [ option1=value1 option2=value2 ] uri suite [component1] [component2] [. The currently recognized URI types are: + http + + The http scheme specifies an HTTP server for an archive and is the most + commonly used method, with many options in the + Acquire::http scope detailed in &apt-conf;. The URI can + directly include login information if the archive requires it, but the use + of &apt-authconf; should be preferred. The method also supports SOCKS5 and + HTTP(S) proxies either configured via apt-specific configuration or + specified by the environment variable http_proxy in the + format (assuming an HTTP proxy requiring authentication) + http://user:pass@server:port/. + The authentication details for proxies can also be supplied via + &apt-authconf;. + Note that these forms of authentication are insecure as the whole + communication with the remote server (or proxy) is not encrypted so a + sufficiently capable attacker can observe and record login as well as all + other interactions. The attacker can not modify the + communication through as APTs data security model is independent of the + chosen transport method. See &apt-secure; for details. + + + https + + The https scheme specifies an HTTPS server for an archive and is very + similar in use and available options to the http scheme. The main + difference is that the communication between apt and server (or proxy) is + encrypted. Note that the encryption does not prevent an attacker from + knowing which server (or proxy) apt is communicating with and deeper + analyses can potentially still reveal which data was downloaded. If this is + a concern the Tor-based schemes mentioned further below might be a suitable + alternative. + + + file The file scheme allows an arbitrary directory in the file system to be @@ -359,27 +393,19 @@ deb-src [ option1=value1 option2=value2 ] uri suite [component1] [component2] [. cdrom - The cdrom scheme allows APT to use a local CD-ROM drive with media + The cdrom scheme allows APT to use a local CD-ROM, DVD or USB drive with media swapping. Use the &apt-cdrom; program to create cdrom entries in the source list. - http - - The http scheme specifies an HTTP server for the archive. If an environment - variable http_proxy is set with the format - http://server:port/, the proxy server specified in - http_proxy will be used. Users of authenticated - HTTP/1.1 proxies may use a string of the format - http://user:pass@server:port/. - Note that this is an insecure method of authentication. - - ftp - The ftp scheme specifies an FTP server for the archive. APT's FTP behavior - is highly configurable; for more information see the - &apt-conf; manual page. Please note that an FTP proxy can be specified + The ftp scheme specifies an FTP server for an archive. Use of FTP is on the + decline in favour of http and https + and many archives either never offered or are retiring FTP access. If you + still need this method many configuration options for it are available in + the Acquire::ftp scope and detailed in &apt-conf;. + Please note that an FTP proxy can be specified by using the ftp_proxy environment variable. It is possible to specify an HTTP proxy (HTTP proxy servers often understand FTP URLs) using this environment variable and only this @@ -407,9 +433,8 @@ deb-src [ option1=value1 option2=value2 ] uri suite [component1] [component2] [. APT can be extended with more methods shipped in other optional packages, which should follow the naming scheme apt-transport-method. - For instance, the APT team also maintains the package apt-transport-https, - which provides access methods for HTTPS URIs with features similar to the http method. - Methods for using e.g. debtorrent are also available - see &apt-transport-debtorrent;. + For instance, the APT team also maintains the package apt-transport-tor, + which provides access methods for HTTP and HTTPS URIs routed via the Tor network. -- cgit v1.2.3 From 054243fd0febfef5f1ba89f61eed0e6a34c6a25f Mon Sep 17 00:00:00 2001 From: David Kalnischkies Date: Fri, 14 Jul 2017 13:49:33 +0200 Subject: show a warning for Debian shutting down FTP services MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We detect the effected sources by matching Release info – that has potential by-catch of repositories which have incorrect field values, but those are better fixed now anyhow. The bigger incorrectness is that this message will not only be printed for the Debian services itself but also for all mirrors not under Debian control but serving Debian like more local/private mirrors which will not (directly) shutdown. It is likely through that many of them will follow suite with less visible announcements or break downright if their upstream source disappears, so having false-positives here seems benefitial for the user in the end. --- apt-private/private-update.cc | 26 ++++++++++++++++++ doc/examples/configure-index | 6 ++++- .../test-apt-get-update-sourceslist-warning | 31 ++++++++++++++++++++++ 3 files changed, 62 insertions(+), 1 deletion(-) create mode 100755 test/integration/test-apt-get-update-sourceslist-warning diff --git a/apt-private/private-update.cc b/apt-private/private-update.cc index 8949dab30..f235a6191 100644 --- a/apt-private/private-update.cc +++ b/apt-private/private-update.cc @@ -8,6 +8,7 @@ #include #include #include +#include #include #include @@ -79,6 +80,31 @@ bool DoUpdate(CommandLine &CmdL) if (Cache.BuildCaches(false) == false) return false; + if (_config->FindB("APT::Get::Update::SourceListWarnings", true)) + { + List = Cache.GetSourceList(); + for (pkgSourceList::const_iterator S = List->begin(); S != List->end(); ++S) + { + if (APT::String::Startswith((*S)->GetURI(), "ftp://") == false) + continue; + pkgCache::RlsFileIterator const RlsFile = (*S)->FindInCache(Cache, false); + if (RlsFile.end() || RlsFile->Origin == 0 || RlsFile->Label == 0) + continue; + char const *const affected[][2] = { + {"Debian", "Debian"}, + {"Debian", "Debian-Security"}, + {"Debian Backports", "Debian Backports"}, + }; + auto const matchRelease = [&](decltype(affected[0]) a) { + return strcmp(RlsFile.Origin(), a[0]) == 0 && strcmp(RlsFile.Label(), a[1]) == 0; + }; + if (std::find_if(std::begin(affected), std::end(affected), matchRelease) != std::end(affected)) + _error->Warning("Debian shuts down public FTP services currently still used in your sources.list(5) as '%s'.\n" + "See press release %s for details.", + (*S)->GetURI().c_str(), "https://debian.org/News/2017/20170425"); + } + } + // show basic stats (if the user whishes) if (_config->FindB("APT::Cmd::Show-Update-Stats", false) == true) { diff --git a/doc/examples/configure-index b/doc/examples/configure-index index 244d7c1c3..61a749495 100644 --- a/doc/examples/configure-index +++ b/doc/examples/configure-index @@ -107,7 +107,11 @@ APT IndexTargets::ReleaseInfo ""; IndexTargets::format ""; - Update::InteractiveReleaseInfoChanges ""; + Update + { + InteractiveReleaseInfoChanges ""; + SourceListWarnings ""; + }; }; Cache diff --git a/test/integration/test-apt-get-update-sourceslist-warning b/test/integration/test-apt-get-update-sourceslist-warning new file mode 100755 index 000000000..b466e85eb --- /dev/null +++ b/test/integration/test-apt-get-update-sourceslist-warning @@ -0,0 +1,31 @@ +#!/bin/sh +set -e + +TESTDIR="$(readlink -f "$(dirname "$0")")" +. "$TESTDIR/framework" + +setupenvironment +configarchitecture 'amd64' +setupaptarchive --no-update + +testsuccess apt update +testsuccess apt update --no-download + +echo 'deb ftp://ftp.tlh.debian.org/debian zurg main' > rootdir/etc/apt/sources.list.d/ftpshutdown.list +cat > rootdir/var/lib/apt/lists/ftp.tlh.debian.org_debian_dists_zurg_Release < Date: Fri, 14 Jul 2017 17:07:22 +0200 Subject: suggest using auth.conf for sources with passwords The feature exists for a long while even if we get around to document it properly only now, so we should push for its adoption a bit to avoid the problems its supposed to solve like avoiding usage of non-world readable configuration files as they can cause strange behaviour for the unsuspecting user (like different solutions as root and non-root). --- apt-private/private-update.cc | 13 +++++++++++++ test/integration/test-apt-get-update-sourceslist-warning | 14 ++++++++++++++ test/integration/test-authentication-basic | 6 +++++- 3 files changed, 32 insertions(+), 1 deletion(-) diff --git a/apt-private/private-update.cc b/apt-private/private-update.cc index f235a6191..c9113ddd3 100644 --- a/apt-private/private-update.cc +++ b/apt-private/private-update.cc @@ -103,6 +103,19 @@ bool DoUpdate(CommandLine &CmdL) "See press release %s for details.", (*S)->GetURI().c_str(), "https://debian.org/News/2017/20170425"); } + for (pkgSourceList::const_iterator S = List->begin(); S != List->end(); ++S) + { + URI uri((*S)->GetURI()); + if (uri.User.empty() && uri.Password.empty()) + continue; + // we can't really predict if a +http method supports everything http does, + // so we play it safe and use a whitelist here. + char const *const affected[] = {"http", "https", "tor+http", "tor+https", "ftp"}; + if (std::find(std::begin(affected), std::end(affected), uri.Access) != std::end(affected)) + // TRANSLATOR: the first two are manpage references, the last the URI from a sources.list + _error->Notice(_("Usage of %s should be preferred over embedding login information directly in the %s entry for '%s'"), + "apt_auth.conf(5)", "sources.list(5)", URI::ArchiveOnly(uri).c_str()); + } } // show basic stats (if the user whishes) diff --git a/test/integration/test-apt-get-update-sourceslist-warning b/test/integration/test-apt-get-update-sourceslist-warning index b466e85eb..a99356b8b 100755 --- a/test/integration/test-apt-get-update-sourceslist-warning +++ b/test/integration/test-apt-get-update-sourceslist-warning @@ -29,3 +29,17 @@ Building dependency tree... All packages are up to date. W: Debian shuts down public FTP services currently still used in your sources.list(5) as 'ftp://ftp.tlh.debian.org/debian/'. See press release https://debian.org/News/2017/20170425 for details." apt update --no-download + + +echo 'deb http://apt:debian@ftp.tlh.debian.org/debian zurg main' > rootdir/etc/apt/sources.list.d/ftpshutdown.list +testsuccessequal "Reading package lists... +Building dependency tree... +All packages are up to date. +N: Usage of apt_auth.conf(5) should be preferred over embedding login information directly in the sources.list(5) entry for 'http://ftp.tlh.debian.org/debian'" apt update --no-download + + +echo 'deb tor+https://apt:debian@ftp.tlh.debian.org/debian zurg main' > rootdir/etc/apt/sources.list.d/ftpshutdown.list +testsuccessequal "Reading package lists... +Building dependency tree... +All packages are up to date. +N: Usage of apt_auth.conf(5) should be preferred over embedding login information directly in the sources.list(5) entry for 'tor+https://ftp.tlh.debian.org/debian'" apt update --no-download diff --git a/test/integration/test-authentication-basic b/test/integration/test-authentication-basic index d29b38256..011f205af 100755 --- a/test/integration/test-authentication-basic +++ b/test/integration/test-authentication-basic @@ -38,7 +38,11 @@ testauthsuccess() { fi rm -rf rootdir/var/lib/apt/lists - testsuccess aptget update + if expr index "$1" '@' >/dev/null; then + testsuccesswithnotice aptget update + else + testsuccess aptget update + fi testsuccessequal 'Reading package lists... Building dependency tree... The following NEW packages will be installed: -- cgit v1.2.3