Explorar o código

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.
Andreas Kling %!s(int64=4) %!d(string=hai) anos
pai
achega
33f2eeea4a
Modificáronse 4 ficheiros con 42 adicións e 183 borrados
  1. 0 4
      Base/etc/plsusers
  2. 1 5
      Base/usr/share/man/man8/pls.md
  3. 1 4
      Meta/build-root-filesystem.sh
  4. 40 170
      Userland/Utilities/pls.cpp

+ 0 - 4
Base/etc/plsusers

@@ -1,4 +0,0 @@
-# plsusers file
-# Put any users you want to allow to run programs as root here
-root
-anon

+ 1 - 5
Base/usr/share/man/man8/pls.md

@@ -10,8 +10,7 @@ $ pls [command]
 
 ## Description
 
-Executes a command as the root user (uid and gid 0), given that the user executing `pls` is located in
-the plsusers file.
+Executes a command as superuser (UID and GID 0). This command is only available for users in the `wheel` group.
 
 It is possible to execute commands that contain hyphenated options via the use of `--`, which signifies the
 end of command options. For example:
@@ -20,9 +19,6 @@ end of command options. For example:
 $ pls -- ls -la
 ```
 
-## Files
-/etc/plsusers - List of users that can run `pls`
-
 ## Examples
 
 ```sh

+ 1 - 4
Meta/build-root-filesystem.sh

@@ -50,9 +50,6 @@ chmod 660 mnt/etc/WindowServer.ini
 chown $window_uid:$window_gid mnt/etc/WindowServer.ini
 echo "/bin/sh" > mnt/etc/shells
 
-chmod 0400 mnt/etc/plsusers
-chown 0:0 mnt/etc/plsusers
-
 chown 0:$wheel_gid mnt/bin/su
 chown 0:$wheel_gid mnt/bin/passwd
 chown 0:$wheel_gid mnt/bin/ping
@@ -66,8 +63,8 @@ chown 0:0 mnt/res/kernel.map
 chmod 0400 mnt/res/kernel.map
 chmod 0400 mnt/boot/Kernel
 chmod 4750 mnt/bin/su
+chmod 4750 mnt/bin/pls
 chmod 4755 mnt/bin/passwd
-chmod 4751 mnt/bin/pls
 chmod 4755 mnt/bin/ping
 chmod 4755 mnt/bin/traceroute
 chmod 4750 mnt/bin/reboot

+ 40 - 170
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;
 }