From 007d8b488787f4c33ced5937f22f99f1b759088a Mon Sep 17 00:00:00 2001 From: David Kalnischkies Date: Wed, 8 Jun 2016 13:44:29 +0200 Subject: [PATCH] edsp: drop privileges before executing solvers Most (if not all) solvers should be able to run perfectly fine without root privileges as they get the entire state they are supposed to work on via stdin and do not perform any action directly, but just pass suggestions on via stdout. The new default is to run them all as _apt hence, but each solver can configure another user if it chooses/must. The security benefits are minimal at best, but it helps preventing silly mistakes (see 35f3ed061f10a25a3fb28bc988fddbb976344c4d) and that is always good. Note that our 'apt' and 'dump' solver already dropped privileges if they had them. --- apt-pkg/edsp.cc | 7 ++++++- apt-private/private-cmndline.cc | 12 +++++------- cmdline/apt-dump-solver.cc | 2 ++ doc/external-dependency-solver-protocol.txt | 5 +++++ test/integration/framework | 4 +++- 5 files changed, 21 insertions(+), 9 deletions(-) diff --git a/apt-pkg/edsp.cc b/apt-pkg/edsp.cc index fcff208c1..890252ba4 100644 --- a/apt-pkg/edsp.cc +++ b/apt-pkg/edsp.cc @@ -968,14 +968,19 @@ static pid_t ExecuteExternal(char const* const type, char const * const binary, dup2(external[3], STDOUT_FILENO); auto const dumpfile = _config->FindFile((std::string("Dir::Log::") + type).c_str()); auto const dumpdir = flNotFile(dumpfile); + auto const runasuser = _config->Find(std::string("APT::") + type + "::" + binary + "::RunAsUser", + _config->Find(std::string("APT::") + type + "::RunAsUser", + _config->Find("APT::Sandbox::User"))); if (dumper.empty() || dumpfile.empty() || dumper == file || CreateAPTDirectoryIfNeeded(dumpdir, dumpdir) == false) { + _config->Set("APT::Sandbox::User", runasuser); + DropPrivileges(); char const * const calling[] = { file.c_str(), nullptr }; execv(calling[0], const_cast(calling)); } else { - char const * const calling[] = { dumper.c_str(), dumpfile.c_str(), file.c_str(), nullptr }; + char const * const calling[] = { dumper.c_str(), "--user", runasuser.c_str(), dumpfile.c_str(), file.c_str(), nullptr }; execv(calling[0], const_cast(calling)); } std::cerr << "Failed to execute " << type << " '" << binary << "'!" << std::endl; diff --git a/apt-private/private-cmndline.cc b/apt-private/private-cmndline.cc index 135ee3c4e..ba64c5b46 100644 --- a/apt-private/private-cmndline.cc +++ b/apt-private/private-cmndline.cc @@ -24,6 +24,8 @@ APT_SENTINEL static bool strcmp_match_in_list(char const * const Cmd, ...) /*{{{*/ { + if (Cmd == nullptr) + return false; va_list args; bool found = false; va_start(args, Cmd); @@ -131,8 +133,9 @@ static bool addArgumentsAPTConfig(std::vector &Args, char con return true; } /*}}}*/ -static bool addArgumentsAPTDumpSolver(std::vector &, char const * const)/*{{{*/ +static bool addArgumentsAPTDumpSolver(std::vector &Args, char const * const)/*{{{*/ { + addArg(0,"user","APT::Solver::RunAsUser",CommandLine::HasArg); return true; } /*}}}*/ @@ -337,12 +340,7 @@ std::vector getCommandArgs(APT_CMD const Program, char const { std::vector Args; Args.reserve(50); - if (Cmd == nullptr) - { - if (Program == APT_CMD::APT_EXTRACTTEMPLATES) - addArgumentsAPTExtractTemplates(Args, Cmd); - } - else if (strcmp(Cmd, "help") == 0) + if (Cmd != nullptr && strcmp(Cmd, "help") == 0) ; // no options for help so no need to implement it in each else switch (Program) diff --git a/cmdline/apt-dump-solver.cc b/cmdline/apt-dump-solver.cc index c6d98cd97..e94021fcf 100644 --- a/cmdline/apt-dump-solver.cc +++ b/cmdline/apt-dump-solver.cc @@ -107,6 +107,8 @@ int main(int argc,const char *argv[]) /*{{{*/ Solver = ExecFork(); if (Solver == 0) { + _config->Set("APT::Sandbox::User", _config->Find("APT::Solver::RunAsUser", _config->Find("APT::Sandbox::User"))); + DropPrivileges(); dup2(external[0], STDIN_FILENO); execv(CmdL.FileList[1], const_cast(CmdL.FileList + 1)); std::cerr << "Failed to execute '" << CmdL.FileList[1] << "'!" << std::endl; diff --git a/doc/external-dependency-solver-protocol.txt b/doc/external-dependency-solver-protocol.txt index c932b8b77..452212602 100644 --- a/doc/external-dependency-solver-protocol.txt +++ b/doc/external-dependency-solver-protocol.txt @@ -70,6 +70,11 @@ configuration documentation for more, and more up to date, information. of the solver you are using if and what is supported as a value here. Defaults to the empty string. +- **APT::Solver::RunAsUser**: if APT itself is run as root it will + change to this user before executing the solver. Defaults to the value + of APT::Sandbox::User, which itself defaults to `_apt`. Can be + disabled by set this option to `root`. + The options **Strict-Pinning** and **Preferences** can also be set for a specific solver only via **APT::Solver::NAME::Strict-Pinning** and **APT::Solver::NAME::Preferences** respectively where `NAME` is the name diff --git a/test/integration/framework b/test/integration/framework index ea577c04e..b7bee8a57 100644 --- a/test/integration/framework +++ b/test/integration/framework @@ -339,7 +339,9 @@ setupenvironment() { # destroys coverage reporting though, so we disable changing user for the calling gpgv echo "Dir::Bin::apt-key \"${BUILDDIRECTORY}/apt-key\";" >> aptconfig.conf if [ "$(id -u)" = '0' ]; then - echo 'Binary::gpgv::Debug::NoDropPrivs "true";' >>aptconfig.conf + echo 'Binary::gpgv::APT::Sandbox::User "root";' >> aptconfig.conf + # same for the solver executables + echo 'APT::Solver::RunAsUser "root";' >> aptconfig.conf fi cat > "${TMPWORKINGDIRECTORY}/rootdir/usr/bin/dpkg" <