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