From 86f8353ce7a343db7b19422db0d37379ac45ff4e Mon Sep 17 00:00:00 2001 From: Julian Andres Klode Date: Thu, 28 Nov 2024 09:55:57 +0100 Subject: Detect working gpgv using gpgv --dump-options This gains us support for not using an absolute path, which is nice, and we can now cache the invocation across fork() in the method. Still keep preferring absolute paths, this avoids execvp() having to iterate the path and reduces bug potential with custom gpgv in /usr/local. Fix the last entry in the list of gpgvs to say gpgv1 --- apt-pkg/contrib/gpgv.cc | 100 +++++++++++++++++++++++++++--------------------- apt-pkg/contrib/gpgv.h | 8 ++++ methods/gpgv.cc | 6 +++ 3 files changed, 71 insertions(+), 43 deletions(-) diff --git a/apt-pkg/contrib/gpgv.cc b/apt-pkg/contrib/gpgv.cc index 611f4bfa7..43c634dde 100644 --- a/apt-pkg/contrib/gpgv.cc +++ b/apt-pkg/contrib/gpgv.cc @@ -18,11 +18,13 @@ #include #include +#include #include #include #include #include #include +#include #include #include @@ -138,6 +140,54 @@ static void APT_PRINTF(5) apt_msg(std::string const &tag, std::ostream &outterm, outterm << errtext << std::flush; } } + +static bool CheckGPGV(std::unordered_map> &checkedCommands, std::string gpgv, bool Debug) +{ + if (checkedCommands.find(gpgv) == checkedCommands.end()) + { + // Create entry + checkedCommands[gpgv]; + FileFd dumpOptions; + pid_t child; + const char *argv[] = {gpgv.c_str(), "--dump-options", nullptr}; + if (unlikely(Debug)) + std::clog << "Executing " << gpgv << " --dump-options" << std::endl; + if (not Popen(argv, dumpOptions, child, FileFd::ReadOnly) && Debug) + return false; + + for (std::string line; dumpOptions.ReadLine(line);) + { + if (unlikely(Debug)) + std::clog << "Read line: " << line << std::endl; + checkedCommands[gpgv].push_front(APT::String::Strip(line)); + } + dumpOptions.Close(); + waitpid(child, NULL, 0); + } + return not checkedCommands[gpgv].empty(); +} + +std::pair> APT::Internal::FindGPGV(bool Debug) +{ + static thread_local std::unordered_map> checkedCommands; + const std::string gpgvVariants[] = { + _config->Find("Apt::Key::gpgvcommand"), + // Prefer absolute path + "/usr/bin/gpgv-sq", + "/usr/bin/gpgv", + "/usr/bin/gpgv2", + "/usr/bin/gpgv1", + "gpgv-sq", + "gpgv", + "gpgv2", + "gpgv1", + }; + for (auto gpgv : gpgvVariants) + if (CheckGPGV(checkedCommands, gpgv, Debug)) + return std::make_pair(gpgv, checkedCommands[gpgv]); + return {}; +} + void ExecGPGV(std::string const &File, std::string const &FileGPG, int const &statusfd, int fd[2], std::string const &key) { @@ -148,13 +198,7 @@ void ExecGPGV(std::string const &File, std::string const &FileGPG, void ExecGPGV(std::string const &File, std::string const &FileGPG, int const &statusfd, int fd[2], std::vector const &KeyFiles) { - #define EINTERNAL 111 - const std::string gpgvVariants[] = { - "/usr/bin/gpgv-sq", - "/usr/bin/gpgv", - "/usr/bin/gpgv2", - "/usr/bin/gpgv", - }; +#define EINTERNAL 111 bool const Debug = _config->FindB("Debug::Acquire::gpgv", false); struct exiter { std::vector files; @@ -165,24 +209,11 @@ void ExecGPGV(std::string const &File, std::string const &FileGPG, } } local_exit; - std::string gpgv = _config->Find("Apt::Key::gpgvcommand"); - if (gpgv.empty() || !FileExists(gpgv)) + auto [gpgv, supportedOptions] = APT::Internal::FindGPGV(Debug); + if (gpgv.empty()) { - gpgv = ""; - for (auto gpgvVariant : gpgvVariants) - { - if (FileExists(gpgvVariant)) - { - gpgv = gpgvVariant; - break; - } - } - - if (gpgv.empty()) - { - apt_error(std::cerr, statusfd, fd, "Couldn't find a gpgv binary"); - local_exit(EINTERNAL); - } + apt_error(std::cerr, statusfd, fd, "Couldn't find a gpgv binary"); + local_exit(EINTERNAL); } std::vector Args; @@ -303,25 +334,8 @@ void ExecGPGV(std::string const &File, std::string const &FileGPG, if (auto assertPubkeyAlgo = _config->Find("Apt::Key::assert-pubkey-algo"); not assertPubkeyAlgo.empty()) { - FileFd dumpOptions; - pid_t child; - if (Debug) - std::clog << "Calling " << gpgv << " --dump-options" << std::endl; - const char* argv[] = {gpgv.c_str(), "--dump-options", nullptr}; - if (not Popen(argv, dumpOptions, child, FileFd::ReadOnly) && Debug) - std::clog << "Failed to call " << gpgv << " --dump-options" << std::endl; - for (std::string line; dumpOptions.ReadLine(line); ) - { - if (Debug) - std::clog << "Read line: " << line << std::endl; - if (APT::String::Strip(line) == "--assert-pubkey-algo") - { - Args.push_back("--assert-pubkey-algo=" + assertPubkeyAlgo); - break; - } - } - dumpOptions.Close(); - waitpid(child, NULL, 0); + if (std::find(supportedOptions.begin(), supportedOptions.end(), "--assert-pubkey-algo") != supportedOptions.end()) + Args.push_back("--assert-pubkey-algo=" + assertPubkeyAlgo); } Configuration::Item const *Opts; diff --git a/apt-pkg/contrib/gpgv.h b/apt-pkg/contrib/gpgv.h index 62c92f271..d4520f3a2 100644 --- a/apt-pkg/contrib/gpgv.h +++ b/apt-pkg/contrib/gpgv.h @@ -11,12 +11,20 @@ #include +#include #include #include class FileFd; +#ifdef APT_COMPILING_APT +namespace APT::Internal +{ +APT_PUBLIC std::pair> FindGPGV(bool Debug); +} +#endif + /** \brief generates and run the command to verify a file with gpgv * * If File and FileSig specify the same file it is assumed that we diff --git a/methods/gpgv.cc b/methods/gpgv.cc index 9f2ccb174..947b585da 100644 --- a/methods/gpgv.cc +++ b/methods/gpgv.cc @@ -190,6 +190,12 @@ string GPGVMethod::VerifyGetSigners(const char *file, const char *outfile, if (Debug == true) std::clog << "inside VerifyGetSigners" << std::endl; + // Abort early if we can't find gpgv, before forking further. This also + // caches the invocation of --dump-options to find the working gpgv across + // our fork() below. + if (APT::Internal::FindGPGV(Debug).first.empty()) + return "Internal error: Cannot find gpgv"; + int fd[2]; if (pipe(fd) < 0) -- cgit v1.2.3-70-g09d2