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 +++++++------ 5 files changed, 69 insertions(+), 28 deletions(-) (limited to 'apt-pkg') 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()); -- cgit v1.2.3-70-g09d2 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(-) (limited to 'apt-pkg') 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-70-g09d2 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 (limited to 'apt-pkg') 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-70-g09d2 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(-) (limited to 'apt-pkg') 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-70-g09d2