From 5c8ce7294594b87e830233be1d20999d0fd3dcd5 Mon Sep 17 00:00:00 2001 From: Jan Kolarik Date: Tue, 21 May 2024 15:06:31 +0000 Subject: [PATCH 1/6] commands: Fix using store option Drop from `clean` command as it is not transactional and add it to `builddep`. --- dnf5-plugins/builddep_plugin/builddep.cpp | 1 + dnf5/commands/clean/clean.cpp | 1 - 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/dnf5-plugins/builddep_plugin/builddep.cpp b/dnf5-plugins/builddep_plugin/builddep.cpp index 60d6cf5e8..398cf2f1e 100644 --- a/dnf5-plugins/builddep_plugin/builddep.cpp +++ b/dnf5-plugins/builddep_plugin/builddep.cpp @@ -88,6 +88,7 @@ void BuildDepCommand::set_argument_parser() { allow_erasing = std::make_unique(*this); auto skip_unavailable = std::make_unique(*this); create_allow_downgrade_options(*this); + create_store_option(*this); } void BuildDepCommand::configure() { diff --git a/dnf5/commands/clean/clean.cpp b/dnf5/commands/clean/clean.cpp index b45eccabe..deaa76206 100644 --- a/dnf5/commands/clean/clean.cpp +++ b/dnf5/commands/clean/clean.cpp @@ -127,7 +127,6 @@ void CleanCommand::set_argument_parser() { }); cmd.register_positional_arg(cache_types); - create_store_option(*this); } void CleanCommand::run() { From 8e14c0c0574a75cb92d3b6500f5e9c5e2ba47765 Mon Sep 17 00:00:00 2001 From: Jan Kolarik Date: Tue, 21 May 2024 15:08:40 +0000 Subject: [PATCH 2/6] distro-sync: Add downloadonly option --- dnf5/commands/distro-sync/distro-sync.cpp | 1 + doc/commands/distro-sync.8.rst | 3 +++ 2 files changed, 4 insertions(+) diff --git a/dnf5/commands/distro-sync/distro-sync.cpp b/dnf5/commands/distro-sync/distro-sync.cpp index 2116cdf7e..534c95f98 100644 --- a/dnf5/commands/distro-sync/distro-sync.cpp +++ b/dnf5/commands/distro-sync/distro-sync.cpp @@ -52,6 +52,7 @@ void DistroSyncCommand::set_argument_parser() { allow_erasing = std::make_unique(*this); auto skip_broken = std::make_unique(*this); auto skip_unavailable = std::make_unique(*this); + create_downloadonly_option(*this); create_offline_option(*this); create_store_option(*this); } diff --git a/doc/commands/distro-sync.8.rst b/doc/commands/distro-sync.8.rst index 026f55f58..1e7e8cc02 100644 --- a/doc/commands/distro-sync.8.rst +++ b/doc/commands/distro-sync.8.rst @@ -50,6 +50,9 @@ Options ``--skip-unavailable`` | Allow skipping packages that are not possible to synchronize. All remaining packages will be synchronized. +``--downloadonly`` + | Download the resolved package set without executing an RPM transaction. + ``--offline`` | Store the transaction to be performed offline. See :manpage:`dnf5-offline(8)`, :ref:`Offline command `. From 4ac705eed006bc6e05595da1331ea8de3ee5c584 Mon Sep 17 00:00:00 2001 From: Jan Kolarik Date: Tue, 21 May 2024 15:12:46 +0000 Subject: [PATCH 3/6] locker: Move to public API The functionality is needed by both the library and the dnf5 app. --- {libdnf5 => include/libdnf5}/utils/locker.hpp | 16 +++++++++++++++- libdnf5/base/transaction.cpp | 2 +- libdnf5/utils/locker.cpp | 4 +++- 3 files changed, 19 insertions(+), 3 deletions(-) rename {libdnf5 => include/libdnf5}/utils/locker.hpp (55%) diff --git a/libdnf5/utils/locker.hpp b/include/libdnf5/utils/locker.hpp similarity index 55% rename from libdnf5/utils/locker.hpp rename to include/libdnf5/utils/locker.hpp index 91cae4311..c5de568c9 100644 --- a/libdnf5/utils/locker.hpp +++ b/include/libdnf5/utils/locker.hpp @@ -24,12 +24,26 @@ along with libdnf. If not, see . namespace libdnf5::utils { +/// Object for implementing a simple file mutex mechanism +/// or checking read/write access on a given path. class Locker { public: - explicit Locker(const std::string & path) : path(path){}; + /// Create a Locker object at a given path + explicit Locker(const std::string & path); ~Locker(); + + /// @brief Try to acquire read lock on a given file path + /// @return True if lock acquisition was successful, otherwise false + /// @throws libdnf5::SystemError if an unexpected error occurs when checking the lock state, like insufficient privileges bool read_lock(); + + /// @brief Try to acquire write lock on a given file path + /// @return True if lock acquisition was successful, otherwise false + /// @throws libdnf5::SystemError if an unexpected error occurs when checking the lock state, like insufficient privileges bool write_lock(); + + /// @brief Unlock the existing lock and remove the underlying lock file + /// @throws libdnf5::SystemError if an unexpected error occurs when unlocking void unlock(); private: diff --git a/libdnf5/base/transaction.cpp b/libdnf5/base/transaction.cpp index e03b016bc..aacecaa28 100644 --- a/libdnf5/base/transaction.cpp +++ b/libdnf5/base/transaction.cpp @@ -31,7 +31,6 @@ along with libdnf. If not, see . #include "transaction_impl.hpp" #include "transaction_module_impl.hpp" #include "transaction_package_impl.hpp" -#include "utils/locker.hpp" #include "utils/string.hpp" #include "libdnf5/base/base.hpp" @@ -44,6 +43,7 @@ along with libdnf. If not, see . #include "libdnf5/utils/bgettext/bgettext-lib.h" #include "libdnf5/utils/bgettext/bgettext-mark-domain.h" #include "libdnf5/utils/format.hpp" +#include "libdnf5/utils/locker.hpp" #include #include diff --git a/libdnf5/utils/locker.cpp b/libdnf5/utils/locker.cpp index 7855fab62..69d96e291 100644 --- a/libdnf5/utils/locker.cpp +++ b/libdnf5/utils/locker.cpp @@ -18,7 +18,7 @@ along with libdnf. If not, see . */ -#include "locker.hpp" +#include "libdnf5/utils/locker.hpp" #include "libdnf5/common/exception.hpp" #include "libdnf5/utils/bgettext/bgettext-mark-domain.h" @@ -29,6 +29,8 @@ along with libdnf. If not, see . namespace libdnf5::utils { +Locker::Locker(const std::string & path) : path(path){}; + bool Locker::read_lock() { return lock(F_RDLCK); } From 0b47897d7368ec1bcdf0c69446494d669e66d41c Mon Sep 17 00:00:00 2001 From: Jan Kolarik Date: Tue, 21 May 2024 15:14:00 +0000 Subject: [PATCH 4/6] exception: Add new exception for user insufficient privileges --- include/libdnf5-cli/exception.hpp | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/include/libdnf5-cli/exception.hpp b/include/libdnf5-cli/exception.hpp index f35c60123..445d391ca 100644 --- a/include/libdnf5-cli/exception.hpp +++ b/include/libdnf5-cli/exception.hpp @@ -44,6 +44,14 @@ class AbortedByUserError : public Error { }; +/// Exception is thrown when the user does not have enough privileges to perform requested operation. +class InsufficientPrivilegesError : public Error { +public: + using Error::Error; + const char * get_name() const noexcept override { return "InsufficientPrivilegesError"; } +}; + + /// Exception is thrown when libdnf5 fails to resolve the transaction. class GoalResolveError : public Error { public: From a6b109e68acf897b51c58e5fc38bebcd1d2b072c Mon Sep 17 00:00:00 2001 From: Jan Kolarik Date: Tue, 21 May 2024 15:19:16 +0000 Subject: [PATCH 5/6] main: Implement checking of privileges before executing commands A single checkpoint was implemented after parsing command-line data but before downloading metadata and processing commands, ensuring unified handling of cases where commands are executed under insufficient privileges. --- dnf5/main.cpp | 64 +++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 64 insertions(+) diff --git a/dnf5/main.cpp b/dnf5/main.cpp index 344e2f411..bb6308e00 100644 --- a/dnf5/main.cpp +++ b/dnf5/main.cpp @@ -70,6 +70,7 @@ along with libdnf. If not, see . #include #include #include +#include #include #include #include @@ -1081,6 +1082,62 @@ static void print_no_match_libdnf_plugin_patterns(dnf5::Context & context) { } } +static bool cmd_requires_privileges(dnf5::Context & context) { + // the main, dnf5 command, is allowed + auto cmd = context.get_selected_command(); + auto arg_cmd = cmd->get_argument_parser_command(); + if (arg_cmd->get_parent() == nullptr) { + return false; + } + + // first a hard-coded list of commands that always need to be run with elevated privileges + auto main_arg_cmd = cmd->get_parent_command() != context.get_root_command() ? arg_cmd->get_parent() : arg_cmd; + std::vector privileged_cmds = {"automatic", "offline", "system-upgrade"}; + if (std::find(privileged_cmds.begin(), privileged_cmds.end(), main_arg_cmd->get_id()) != privileged_cmds.end()) { + return true; + } + + // when assumeno is set, system should not be modified + auto & config = context.get_base().get_config(); + if (config.get_assumeno_option().get_value()) { + return false; + } + + auto all_cmd_args = arg_cmd->get_named_args(); + if (main_arg_cmd != arg_cmd) { + all_cmd_args.insert( + all_cmd_args.end(), main_arg_cmd->get_named_args().begin(), main_arg_cmd->get_named_args().end()); + } + + // when downloadonly is defined and set, system should not be modified + auto it_downloadonly = std::find_if( + all_cmd_args.begin(), all_cmd_args.end(), [](auto arg) { return arg->get_long_name() == "downloadonly"; }); + if (it_downloadonly != all_cmd_args.end() && + ((libdnf5::OptionBool *)(*it_downloadonly)->get_linked_value())->get_value()) { + return false; + } + + // otherwise, transactional cmds with store option defined are expected to modify the system + auto it_store = std::find_if( + all_cmd_args.begin(), all_cmd_args.end(), [](auto arg) { return arg->get_long_name() == "store"; }); + return it_store != all_cmd_args.end(); +} + +static bool user_has_privileges(dnf5::Context & context) { + std::filesystem::path lock_file_path = context.get_base().get_config().get_installroot_option().get_value(); + lock_file_path /= "run/dnf/rpm.transaction.lock.tmp"; + + try { + std::filesystem::create_directories(lock_file_path.parent_path()); + libdnf5::utils::Locker locker(lock_file_path); + return locker.write_lock(); + } catch (libdnf5::SystemError & ex) { + return false; + } catch (std::filesystem::filesystem_error & ex) { + return false; + } +} + int main(int argc, char * argv[]) try { dnf5::set_locale(); @@ -1257,6 +1314,13 @@ int main(int argc, char * argv[]) try { dump_repository_configuration(context, repo_id_list); } + if (cmd_requires_privileges(context) && !user_has_privileges(context)) { + throw libdnf5::cli::InsufficientPrivilegesError( + M_("The requested operation requires superuser privileges. Please log in as a user with elevated " + "rights, or use the \"--assumeno\" or \"--downloadonly\" options to run the command without " + "modifying the system state.")); + } + { if (context.get_load_available_repos() != dnf5::Context::LoadAvailableRepos::NONE) { context.load_repos(context.get_load_system_repo()); From 06a0aa0217c0e0b06504f641294b2f99b92f5457 Mon Sep 17 00:00:00 2001 From: Jan Kolarik Date: Tue, 21 May 2024 15:20:21 +0000 Subject: [PATCH 6/6] const: Shared constant defining RPM transaction lock file --- dnf5/main.cpp | 4 +++- include/libdnf5/conf/const.hpp | 2 ++ libdnf5/base/transaction.cpp | 3 ++- 3 files changed, 7 insertions(+), 2 deletions(-) diff --git a/dnf5/main.cpp b/dnf5/main.cpp index bb6308e00..2f60569cc 100644 --- a/dnf5/main.cpp +++ b/dnf5/main.cpp @@ -63,6 +63,7 @@ along with libdnf. If not, see . #include #include #include +#include #include #include #include @@ -1125,7 +1126,8 @@ static bool cmd_requires_privileges(dnf5::Context & context) { static bool user_has_privileges(dnf5::Context & context) { std::filesystem::path lock_file_path = context.get_base().get_config().get_installroot_option().get_value(); - lock_file_path /= "run/dnf/rpm.transaction.lock.tmp"; + lock_file_path /= std::filesystem::path(libdnf5::TRANSACTION_LOCK_FILEPATH).relative_path(); + lock_file_path += ".tmp"; try { std::filesystem::create_directories(lock_file_path.parent_path()); diff --git a/include/libdnf5/conf/const.hpp b/include/libdnf5/conf/const.hpp index 0e6f6cff7..d75610586 100644 --- a/include/libdnf5/conf/const.hpp +++ b/include/libdnf5/conf/const.hpp @@ -42,6 +42,8 @@ const std::vector REPOSITORY_CONF_DIRS{ "/etc/yum.repos.d", "/etc/distro.repos.d", "/usr/share/dnf5/repos.d"}; constexpr const char * REPOS_OVERRIDE_DIR = "/etc/dnf/repos.override.d"; +constexpr const char * TRANSACTION_LOCK_FILEPATH = "/run/dnf/rpmtransaction.lock"; + // More important varsdirs must be on the end of vector const std::vector VARS_DIRS{"/usr/share/dnf5/vars.d", "/etc/dnf/vars"}; diff --git a/libdnf5/base/transaction.cpp b/libdnf5/base/transaction.cpp index aacecaa28..d335d5c1d 100644 --- a/libdnf5/base/transaction.cpp +++ b/libdnf5/base/transaction.cpp @@ -38,6 +38,7 @@ along with libdnf. If not, see . #include "libdnf5/common/sack/exclude_flags.hpp" #include "libdnf5/common/sack/query_cmp.hpp" #include "libdnf5/comps/group/query.hpp" +#include "libdnf5/conf/const.hpp" #include "libdnf5/repo/package_downloader.hpp" #include "libdnf5/rpm/package_query.hpp" #include "libdnf5/utils/bgettext/bgettext-lib.h" @@ -853,7 +854,7 @@ Transaction::TransactionRunResult Transaction::Impl::_run( // acquire the lock std::filesystem::path lock_file_path = config.get_installroot_option().get_value(); - lock_file_path /= "run/dnf/rpmtransaction.lock"; + lock_file_path /= std::filesystem::path(libdnf5::TRANSACTION_LOCK_FILEPATH).relative_path(); std::filesystem::create_directories(lock_file_path.parent_path()); libdnf5::utils::Locker locker(lock_file_path);