diff options
author | Andreas Kling <kling@serenityos.org> | 2021-05-30 22:06:28 +0200 |
---|---|---|
committer | Andreas Kling <kling@serenityos.org> | 2021-05-30 23:09:37 +0200 |
commit | 33f2eeea4a39dd892bad3930d1f2cfdfa7e3a7f8 (patch) | |
tree | 02f659bb5fdf04603bf1d7b3902856a8ad574bfa /Userland/Utilities | |
parent | dfd988707c013d68f64fb56892480248fdce03a1 (diff) | |
download | serenity-33f2eeea4a39dd892bad3930d1f2cfdfa7e3a7f8.zip |
pls: Drastically simplify this program
Since this program is setuid-root, it should be as simple as possible.
To that end, remove `/etc/plsusers` and use filesystem permissions to
achieve the same thing. `/bin/pls` is now only executable by `root` or
members of the `wheel` group.
Also remove all the logic that went to great lengths to `unveil()` a
minimal set of filesystem paths that may be used for the command.
The complexity-to-benefit ratio did not seem justified, and I think
we're better off keeping this simple.
Finally, remove pledge promises the moment they are no longer needed.
Diffstat (limited to 'Userland/Utilities')
-rw-r--r-- | Userland/Utilities/pls.cpp | 210 |
1 files changed, 40 insertions, 170 deletions
diff --git a/Userland/Utilities/pls.cpp b/Userland/Utilities/pls.cpp index 182e95782c..6200fcc569 100644 --- a/Userland/Utilities/pls.cpp +++ b/Userland/Utilities/pls.cpp @@ -1,179 +1,62 @@ /* * Copyright (c) 2021, Jesse Buhagiar <jooster669@gmail.com> + * Copyright (c) 2021, Andreas Kling <kling@serenityos.org> * * SPDX-License-Identifier: BSD-2-Clause */ -#include <AK/LexicalPath.h> -#include <AK/String.h> -#include <AK/StringBuilder.h> -#include <AK/Types.h> -#include <AK/Vector.h> #include <LibCore/Account.h> #include <LibCore/ArgsParser.h> -#include <LibCore/DirIterator.h> -#include <LibCore/File.h> #include <LibCore/GetPassword.h> #include <stdio.h> -#include <stdlib.h> -#include <string.h> #include <unistd.h> -static constexpr mode_t EXPECTED_PERMS = (S_IFREG | S_IRUSR); - -// Function Definitions -extern "C" int main(int arch, char** argv); -bool unveil_paths(const char*); - -// Unveil paths, given the current user's path and the command they want to execute -bool unveil_paths(const char* command) -{ - // Unveil all trusted paths with browse permissions - auto trusted_directories = Array { "/bin", "/usr/bin", "/usr/local/bin" }; - for (auto directory : trusted_directories) { - if (unveil(directory, "b") < 0) { - perror("unveil"); - return false; - } - } - - // Attempt to unveil command via `realpath` - auto command_path = Core::File::real_path_for(command); - - // Command found via `realpath` (meaning it was probably a locally executed program) - if (!command_path.is_empty()) { - if (unveil(command_path.characters(), "x") == 0) - return true; - return false; - } - - auto command_path_system = Core::find_executable_in_path(command); - if (command_path_system.is_empty()) - return false; - if (unveil(command_path_system.characters(), "x") < 0) - return false; - return true; -} - int main(int argc, char** argv) { - Vector<const char*> command; + Vector<char const*> command; Core::ArgsParser args_parser; args_parser.add_positional_argument(command, "Command to run at elevated privilege level", "command"); args_parser.parse(argc, argv); - // Unveil command path. - // Fail silently to prevent disclosing whether the specified path is valid - auto command_path = LexicalPath(Core::File::real_path_for(command.at(0))).dirname(); - unveil(command_path.characters(), "b"); - - if (pledge("stdio tty rpath exec id", nullptr) < 0) { + if (pledge("stdio rpath exec id tty", nullptr) < 0) { perror("pledge"); return 1; } - if (unveil("/etc/plsusers", "r") < 0) { - perror("unveil"); - return 1; - } - - if (unveil("/etc/passwd", "r") < 0) { - perror("unveil"); - return 1; - } - - if (unveil("/etc/shadow", "r") < 0) { - perror("unveil"); - return 1; - } - - if (unveil("/etc/group", "r") < 0) { - perror("unveil"); - return 1; - } - - // Find and unveil the user's command executable. - // Fail silently to prevent disclosing whether the specified path is valid - unveil_paths(command.at(0)); - - // Lock veil - unveil(nullptr, nullptr); - - // Call `seteuid` so we can access `/etc/plsusers` if (seteuid(0) < 0) { perror("seteuid"); return 1; } - // Check the permissions and owner of /etc/plsusers. This ensures the integrity of the file. - struct stat pls_users_stat; - if (stat("/etc/plsusers", &pls_users_stat) < 0) { - perror("stat"); - return 1; - } - - if (pls_users_stat.st_mode != EXPECTED_PERMS) { - warnln("Error: /etc/plsusers has incorrect permissions."); - return 4; - } - - if (pls_users_stat.st_uid != 0 && pls_users_stat.st_gid != 0) { - warnln("Error: /etc/plsusers is not owned by root."); - return 4; - } - - auto pls_users_file_or_error = Core::File::open("/etc/plsusers", Core::OpenMode::ReadOnly); - if (pls_users_file_or_error.is_error()) { - warnln("Error: Could not open /etc/plsusers: {}", pls_users_file_or_error.error()); - return 1; - } - - const char* username = getlogin(); - bool user_found = false; - for (auto line = pls_users_file_or_error.value()->line_begin(); !line.at_end(); ++line) { - auto line_str = *line; - - // Skip any comments - if (line_str.starts_with("#")) - continue; - - // Our user is in the plsusers file! - if (line_str.to_string() == username) { - user_found = true; - break; + // If the current user is not a superuser, make them authenticate themselves. + if (auto uid = getuid()) { + auto account_or_error = Core::Account::from_uid(uid); + if (account_or_error.is_error()) { + warnln("{}", account_or_error.error()); + return 1; } - } - // User isn't in the plsusers file - if (!user_found) { - warnln("{} is not in the plsusers file!", username); - return 2; + auto const& account = account_or_error.value(); + if (account.has_password()) { + auto password_or_error = Core::get_password(); + if (password_or_error.is_error()) { + warnln("{}", password_or_error.error()); + return 1; + } + + auto const& password = password_or_error.value(); + if (!account.authenticate(password.characters())) { + warnln("Incorrect or disabled password."); + return 1; + } + } } - // The user was in the plsusers file, now let's ask for their password to ensure that it's actually them... - auto account_or_error = Core::Account::from_name(username); - - if (account_or_error.is_error()) { - warnln("Core::Account::from_name: {}", account_or_error.error()); + if (pledge("stdio rpath exec id", nullptr) < 0) { + perror("pledge"); return 1; } - const auto& account = account_or_error.value(); - uid_t current_uid = getuid(); - if (current_uid != 0 && account.has_password()) { - auto password = Core::get_password(); - if (password.is_error()) { - warnln("{}", password.error()); - return 1; - } - - if (!account.authenticate(password.value().characters())) { - warnln("Incorrect or disabled password."); - return 1; - } - } - - // TODO: Support swapping users instead of just defaulting to root if (setgid(0) < 0) { perror("setgid"); return 1; @@ -184,41 +67,28 @@ int main(int argc, char** argv) return 1; } - // Build the arguments list passed to `execvpe` - Vector<const char*> exec_args; - for (const auto& arg : command) { - exec_args.append(arg); + if (pledge("stdio rpath exec", nullptr) < 0) { + perror("pledge"); + return 1; } - // Always terminate with a NULL (to signal end of args list) - exec_args.append(nullptr); - - // Build the environment arguments - StringBuilder builder; - Vector<String> env_args_str; + Vector<char const*> arguments; + for (auto const& arg : command) + arguments.append(arg); + arguments.append(nullptr); - // TERM envvar - char* env_term = getenv("TERM"); + Vector<String> environment_strings; + if (auto* term = getenv("TERM")) + environment_strings.append(String::formatted("TERM={}", term)); - if (env_term != nullptr) { - builder.append("TERM="); - builder.append(env_term); - env_args_str.append(builder.build()); - } + Vector<char const*> environment; + for (auto& item : environment_strings) + environment_strings.append(item.characters()); + environment.append(nullptr); - Vector<const char*> env_args; - for (auto& arg : env_args_str) { - env_args.append(arg.characters()); - } - - // Arguments list must be terminated with NULL argument - env_args.append(nullptr); - - // Execute the desired command - if (execvpe(command.at(0), const_cast<char**>(exec_args.data()), const_cast<char**>(env_args.data())) < 0) { + if (execvpe(command.at(0), const_cast<char**>(arguments.data()), const_cast<char**>(environment.data())) < 0) { perror("execvpe"); exit(1); } - return 0; } |