From 718ae6862181b09dd13c8bb34917fe26048ff90a Mon Sep 17 00:00:00 2001 From: Liav A Date: Fri, 4 Nov 2022 19:20:11 +0200 Subject: [PATCH] Kernel+LibCore+LibC: Implement support for forcing unveil on exec To accomplish this, we add another VeilState which is called LockedInherited. The idea is to apply exec unveil data, similar to execpromises of the pledge syscall, on the current exec'ed program during the execve sequence. When applying the forced unveil data, the veil state is set to be locked but the special state of LockedInherited ensures that if the new program tries to unveil paths, the request will silently be ignored, so the program will continue running without receiving an error, but is still can only use the paths that were unveiled before the exec syscall. This in turn, allows us to use the unveil syscall with a special utility to sandbox other userland programs in terms of what is visible to them on the filesystem, and is usable on both programs that use or don't use the unveil syscall in their code. --- Kernel/API/Syscall.h | 1 + Kernel/API/Unveil.h | 22 ++++ .../SysFS/Subsystems/Kernel/Processes.cpp | 5 + Kernel/Process.cpp | 6 +- Kernel/Process.h | 7 +- Kernel/Syscalls/execve.cpp | 21 ++- Kernel/Syscalls/fork.cpp | 8 ++ Kernel/Syscalls/unveil.cpp | 120 ++++++++++++------ Userland/Libraries/LibC/unistd.cpp | 2 + Userland/Libraries/LibCore/System.cpp | 16 +++ Userland/Libraries/LibCore/System.h | 1 + 11 files changed, 161 insertions(+), 48 deletions(-) create mode 100644 Kernel/API/Unveil.h diff --git a/Kernel/API/Syscall.h b/Kernel/API/Syscall.h index 4e40fdce16..f306e549b1 100644 --- a/Kernel/API/Syscall.h +++ b/Kernel/API/Syscall.h @@ -433,6 +433,7 @@ struct SC_pledge_params { }; struct SC_unveil_params { + int flags; StringArgument path; StringArgument permissions; }; diff --git a/Kernel/API/Unveil.h b/Kernel/API/Unveil.h new file mode 100644 index 0000000000..7d116f8e41 --- /dev/null +++ b/Kernel/API/Unveil.h @@ -0,0 +1,22 @@ +/* + * Copyright (c) 2022, Liav A. + * + * SPDX-License-Identifier: BSD-2-Clause + */ + +#pragma once + +#include +#include + +namespace Kernel { + +enum class UnveilFlags : u32 { + None = 0, + CurrentProgram = 1 << 0, + AfterExec = 1 << 1, +}; + +AK_ENUM_BITWISE_OPERATORS(UnveilFlags); + +} diff --git a/Kernel/FileSystem/SysFS/Subsystems/Kernel/Processes.cpp b/Kernel/FileSystem/SysFS/Subsystems/Kernel/Processes.cpp index 50ddbf3b18..10500a3608 100644 --- a/Kernel/FileSystem/SysFS/Subsystems/Kernel/Processes.cpp +++ b/Kernel/FileSystem/SysFS/Subsystems/Kernel/Processes.cpp @@ -53,6 +53,11 @@ ErrorOr SysFSOverallProcesses::try_generate(KBufferBuilder& builder) case VeilState::Locked: TRY(process_object.add("veil"sv, "Locked")); break; + case VeilState::LockedInherited: + // Note: We don't reveal if the locked state is either by our choice + // or someone else applied it. + TRY(process_object.add("veil"sv, "Locked")); + break; } } else { TRY(process_object.add("pledge"sv, ""sv)); diff --git a/Kernel/Process.cpp b/Kernel/Process.cpp index 5346a4880e..8fe1b42961 100644 --- a/Kernel/Process.cpp +++ b/Kernel/Process.cpp @@ -320,13 +320,14 @@ ErrorOr> Process::try_create(LockRefPtr& firs new_address_space = TRY(Memory::AddressSpace::try_create(nullptr)); } auto unveil_tree = UnveilNode { TRY(KString::try_create("/"sv)), UnveilMetadata(TRY(KString::try_create("/"sv))) }; + auto exec_unveil_tree = UnveilNode { TRY(KString::try_create("/"sv)), UnveilMetadata(TRY(KString::try_create("/"sv))) }; auto credentials = TRY(Credentials::create(uid, gid, uid, gid, uid, gid, {})); - auto process = TRY(adopt_nonnull_lock_ref_or_enomem(new (nothrow) Process(move(name), move(credentials), ppid, is_kernel_process, move(current_directory), move(executable), tty, move(unveil_tree)))); + auto process = TRY(adopt_nonnull_lock_ref_or_enomem(new (nothrow) Process(move(name), move(credentials), ppid, is_kernel_process, move(current_directory), move(executable), tty, move(unveil_tree), move(exec_unveil_tree)))); TRY(process->attach_resources(new_address_space.release_nonnull(), first_thread, fork_parent)); return process; } -Process::Process(NonnullOwnPtr name, NonnullRefPtr credentials, ProcessID ppid, bool is_kernel_process, RefPtr current_directory, RefPtr executable, TTY* tty, UnveilNode unveil_tree) +Process::Process(NonnullOwnPtr name, NonnullRefPtr credentials, ProcessID ppid, bool is_kernel_process, RefPtr current_directory, RefPtr executable, TTY* tty, UnveilNode unveil_tree, UnveilNode exec_unveil_tree) : m_name(move(name)) , m_space(LockRank::None) , m_protected_data_lock(LockRank::None) @@ -335,6 +336,7 @@ Process::Process(NonnullOwnPtr name, NonnullRefPtr credent , m_current_directory(LockRank::None, move(current_directory)) , m_tty(tty) , m_unveil_data(LockRank::None, move(unveil_tree)) + , m_exec_unveil_data(LockRank::None, move(exec_unveil_tree)) , m_wait_blocker_set(*this) { // Ensure that we protect the process data when exiting the constructor. diff --git a/Kernel/Process.h b/Kernel/Process.h index a5bfa6ae90..bf0c1616c2 100644 --- a/Kernel/Process.h +++ b/Kernel/Process.h @@ -84,6 +84,7 @@ enum class VeilState { None, Dropped, Locked, + LockedInherited, }; static constexpr FlatPtr futex_key_private_flag = 0b1; @@ -523,6 +524,9 @@ public: auto& unveil_data() { return m_unveil_data; } auto const& unveil_data() const { return m_unveil_data; } + auto& exec_unveil_data() { return m_exec_unveil_data; } + auto const& exec_unveil_data() const { return m_exec_unveil_data; } + bool wait_for_tracer_at_next_execve() const { return m_wait_for_tracer_at_next_execve; @@ -584,7 +588,7 @@ private: bool add_thread(Thread&); bool remove_thread(Thread&); - Process(NonnullOwnPtr name, NonnullRefPtr, ProcessID ppid, bool is_kernel_process, RefPtr current_directory, RefPtr executable, TTY* tty, UnveilNode unveil_tree); + Process(NonnullOwnPtr name, NonnullRefPtr, ProcessID ppid, bool is_kernel_process, RefPtr current_directory, RefPtr executable, TTY* tty, UnveilNode unveil_tree, UnveilNode exec_unveil_tree); static ErrorOr> try_create(LockRefPtr& first_thread, NonnullOwnPtr name, UserID, GroupID, ProcessID ppid, bool is_kernel_process, RefPtr current_directory = nullptr, RefPtr executable = nullptr, TTY* = nullptr, Process* fork_parent = nullptr); ErrorOr attach_resources(NonnullOwnPtr&&, LockRefPtr& first_thread, Process* fork_parent); static ProcessID allocate_pid(); @@ -878,6 +882,7 @@ private: LockRefPtr m_alarm_timer; SpinlockProtected m_unveil_data; + SpinlockProtected m_exec_unveil_data; OwnPtr m_perf_event_buffer; diff --git a/Kernel/Syscalls/execve.cpp b/Kernel/Syscalls/execve.cpp index 713e1b8832..7026f6090b 100644 --- a/Kernel/Syscalls/execve.cpp +++ b/Kernel/Syscalls/execve.cpp @@ -560,9 +560,24 @@ ErrorOr Process::do_exec(NonnullLockRefPtr main_progr m_environment = move(environment); TRY(m_unveil_data.with([&](auto& unveil_data) -> ErrorOr { - unveil_data.state = VeilState::None; - unveil_data.paths.clear(); - unveil_data.paths.set_metadata({ TRY(KString::try_create("/"sv)), UnveilAccess::None, false }); + TRY(m_exec_unveil_data.with([&](auto& exec_unveil_data) -> ErrorOr { + // Note: If we have exec unveil data being waiting to be dispatched + // to the current execve'd program, then we apply the unveil data and + // ensure it is locked in the new program. + if (exec_unveil_data.state == VeilState::Dropped) { + unveil_data.state = VeilState::LockedInherited; + exec_unveil_data.state = VeilState::None; + unveil_data.paths = TRY(exec_unveil_data.paths.deep_copy()); + } else { + unveil_data.state = VeilState::None; + exec_unveil_data.state = VeilState::None; + unveil_data.paths.clear(); + unveil_data.paths.set_metadata({ TRY(KString::try_create("/"sv)), UnveilAccess::None, false }); + } + exec_unveil_data.paths.clear(); + exec_unveil_data.paths.set_metadata({ TRY(KString::try_create("/"sv)), UnveilAccess::None, false }); + return {}; + })); return {}; })); diff --git a/Kernel/Syscalls/fork.cpp b/Kernel/Syscalls/fork.cpp index 0fdcd53bd4..111636a00e 100644 --- a/Kernel/Syscalls/fork.cpp +++ b/Kernel/Syscalls/fork.cpp @@ -42,6 +42,14 @@ ErrorOr Process::sys$fork(RegisterState& regs) }); })); + TRY(m_exec_unveil_data.with([&](auto& parent_exec_unveil_data) -> ErrorOr { + return child->m_exec_unveil_data.with([&](auto& child_exec_unveil_data) -> ErrorOr { + child_exec_unveil_data.state = parent_exec_unveil_data.state; + child_exec_unveil_data.paths = TRY(parent_exec_unveil_data.paths.deep_copy()); + return {}; + }); + })); + // Note: We take the spinlock of Process::all_instances list because we need // to ensure that when we take the jail spinlock of two processes that we don't // run into a deadlock situation because both processes compete over each other Jail's diff --git a/Kernel/Syscalls/unveil.cpp b/Kernel/Syscalls/unveil.cpp index 496791b03f..83e53cc5a0 100644 --- a/Kernel/Syscalls/unveil.cpp +++ b/Kernel/Syscalls/unveil.cpp @@ -7,6 +7,7 @@ #include #include +#include #include #include #include @@ -25,6 +26,55 @@ static void update_intermediate_node_permissions(UnveilNode& root_node, UnveilAc } } +static ErrorOr update_unveil_data(Process::UnveilData& locked_unveil_data, StringView unveiled_path, UnveilAccess new_permissions) +{ + auto path_parts = KLexicalPath::parts(unveiled_path); + auto it = path_parts.begin(); + // Note: For the sake of completence, we check if the locked state was inherited + // by an execve'd sequence. If that is the case, just silently ignore this. + if (locked_unveil_data.state == VeilState::LockedInherited) + return {}; + // NOTE: We have to check again, since the veil may have been locked by another thread + // while we were parsing the arguments. + if (locked_unveil_data.state == VeilState::Locked) + return EPERM; + + auto& matching_node = locked_unveil_data.paths.traverse_until_last_accessible_node(it, path_parts.end()); + if (it.is_end()) { + // If the path has already been explicitly unveiled, do not allow elevating its permissions. + if (matching_node.was_explicitly_unveiled()) { + if (new_permissions & ~matching_node.permissions()) + return EPERM; + } + + // It is possible that nodes that are "grandchildren" of the matching node have already been unveiled. + // This means that there may be intermediate nodes between this one and the unveiled "grandchildren" + // that inherited the current node's previous permissions. Those nodes now need their permissions + // updated to match the current node. + if (matching_node.permissions() != new_permissions) + update_intermediate_node_permissions(matching_node, new_permissions); + + matching_node.metadata_value().explicitly_unveiled = true; + matching_node.metadata_value().permissions = new_permissions; + locked_unveil_data.state = VeilState::Dropped; + return {}; + } + + auto new_unveiled_path = TRY(KString::try_create(unveiled_path)); + TRY(matching_node.insert( + it, + path_parts.end(), + { move(new_unveiled_path), new_permissions, true }, + [](auto& parent, auto& it) -> ErrorOr> { + auto path = TRY(KString::formatted("{}/{}", parent.path(), *it)); + return UnveilMetadata(move(path), parent.permissions(), false); + })); + + VERIFY(locked_unveil_data.state != VeilState::Locked); + locked_unveil_data.state = VeilState::Dropped; + return {}; +} + ErrorOr Process::sys$unveil(Userspace user_params) { VERIFY_NO_PROCESS_BIG_LOCK(this); @@ -35,7 +85,17 @@ ErrorOr Process::sys$unveil(Userspace return 0; } - if (veil_state() == VeilState::Locked) + if (!((params.flags & to_underlying(UnveilFlags::CurrentProgram)) || (params.flags & to_underlying(UnveilFlags::AfterExec)))) + return EINVAL; + + // Note: If we inherited a locked state, then silently ignore the unveil request, + // and let the user program potentially deal with an ENOENT error later on. + if ((params.flags & static_cast(UnveilFlags::CurrentProgram)) && veil_state() == VeilState::LockedInherited) + return 0; + + // Note: We only lock the unveil state for current program, while allowing adding + // indefinitely unveil data before doing the actual exec(). + if ((params.flags & static_cast(UnveilFlags::CurrentProgram)) && veil_state() == VeilState::Locked) return EPERM; if (!params.path.characters || !params.permissions.characters) @@ -93,48 +153,24 @@ ErrorOr Process::sys$unveil(Userspace return custody_or_error.release_error(); } - auto path_parts = KLexicalPath::parts(new_unveiled_path->view()); - auto it = path_parts.begin(); - return m_unveil_data.with([&](auto& unveil_data) -> ErrorOr { - // NOTE: We have to check again, since the veil may have been locked by another thread - // while we were parsing the arguments. - if (unveil_data.state == VeilState::Locked) - return EPERM; + if (params.flags & static_cast(UnveilFlags::CurrentProgram)) { + TRY(unveil_data().with([&](auto& data) -> ErrorOr { + TRY(update_unveil_data(data, new_unveiled_path->view(), static_cast(new_permissions))); + return {}; + })); + } - auto& matching_node = unveil_data.paths.traverse_until_last_accessible_node(it, path_parts.end()); - if (it.is_end()) { - // If the path has already been explicitly unveiled, do not allow elevating its permissions. - if (matching_node.was_explicitly_unveiled()) { - if (new_permissions & ~matching_node.permissions()) - return EPERM; - } - - // It is possible that nodes that are "grandchildren" of the matching node have already been unveiled. - // This means that there may be intermediate nodes between this one and the unveiled "grandchildren" - // that inherited the current node's previous permissions. Those nodes now need their permissions - // updated to match the current node. - if (matching_node.permissions() != new_permissions) - update_intermediate_node_permissions(matching_node, (UnveilAccess)new_permissions); - - matching_node.metadata_value().explicitly_unveiled = true; - matching_node.metadata_value().permissions = (UnveilAccess)new_permissions; - unveil_data.state = VeilState::Dropped; - return 0; - } - - TRY(matching_node.insert( - it, - path_parts.end(), - { new_unveiled_path.release_nonnull(), (UnveilAccess)new_permissions, true }, - [](auto& parent, auto& it) -> ErrorOr> { - auto path = TRY(KString::formatted("{}/{}", parent.path(), *it)); - return UnveilMetadata(move(path), parent.permissions(), false); - })); - - VERIFY(unveil_data.state != VeilState::Locked); - unveil_data.state = VeilState::Dropped; - return 0; - }); + if (params.flags & static_cast(UnveilFlags::AfterExec)) { + TRY(exec_unveil_data().with([&](auto& data) -> ErrorOr { + // Note: The only valid way to get into this state is by using unveil before doing + // an actual exec with the UnveilFlags::AfterExec flag. Then this state is applied on + // the actual new program unveil data, and never on the m_exec_unveil_data. + VERIFY(data.state != VeilState::LockedInherited); + TRY(update_unveil_data(data, new_unveiled_path->view(), static_cast(new_permissions))); + return {}; + })); + } + return 0; } } diff --git a/Userland/Libraries/LibC/unistd.cpp b/Userland/Libraries/LibC/unistd.cpp index d866e15165..b48dadec11 100644 --- a/Userland/Libraries/LibC/unistd.cpp +++ b/Userland/Libraries/LibC/unistd.cpp @@ -8,6 +8,7 @@ #include #include #include +#include #include #include #include @@ -965,6 +966,7 @@ int pledge(char const* promises, char const* execpromises) int unveil(char const* path, char const* permissions) { Syscall::SC_unveil_params params { + static_cast(UnveilFlags::CurrentProgram), { path, path ? strlen(path) : 0 }, { permissions, permissions ? strlen(permissions) : 0 } }; diff --git a/Userland/Libraries/LibCore/System.cpp b/Userland/Libraries/LibCore/System.cpp index 39602c5846..3c1ab3264b 100644 --- a/Userland/Libraries/LibCore/System.cpp +++ b/Userland/Libraries/LibCore/System.cpp @@ -25,6 +25,7 @@ #include #ifdef AK_OS_SERENITY +# include # include # include # include @@ -91,6 +92,7 @@ static ErrorOr unveil_dynamic_loader() constexpr auto dynamic_loader_permissions = "x"sv; Syscall::SC_unveil_params params { + static_cast(UnveilFlags::CurrentProgram), { dynamic_loader_path.characters_without_null_termination(), dynamic_loader_path.length() }, { dynamic_loader_permissions.characters_without_null_termination(), dynamic_loader_permissions.length() }, }; @@ -110,6 +112,20 @@ ErrorOr unveil(StringView path, StringView permissions) TRY(unveil_dynamic_loader()); Syscall::SC_unveil_params params { + static_cast(UnveilFlags::CurrentProgram), + { parsed_path.characters(), parsed_path.length() }, + { permissions.characters_without_null_termination(), permissions.length() }, + }; + int rc = syscall(SC_unveil, ¶ms); + HANDLE_SYSCALL_RETURN_VALUE("unveil", rc, {}); +} + +ErrorOr unveil_after_exec(StringView path, StringView permissions) +{ + auto const parsed_path = TRY(Core::SessionManagement::parse_path_with_sid(path)); + + Syscall::SC_unveil_params params { + static_cast(UnveilFlags::AfterExec), { parsed_path.characters(), parsed_path.length() }, { permissions.characters_without_null_termination(), permissions.length() }, }; diff --git a/Userland/Libraries/LibCore/System.h b/Userland/Libraries/LibCore/System.h index 0a47c4ead7..e0f49b0982 100644 --- a/Userland/Libraries/LibCore/System.h +++ b/Userland/Libraries/LibCore/System.h @@ -36,6 +36,7 @@ namespace Core::System { ErrorOr beep(); ErrorOr pledge(StringView promises, StringView execpromises = {}); ErrorOr unveil(StringView path, StringView permissions); +ErrorOr unveil_after_exec(StringView path, StringView permissions); ErrorOr sendfd(int sockfd, int fd); ErrorOr recvfd(int sockfd, int options); ErrorOr ptrace_peekbuf(pid_t tid, void const* tracee_addr, Bytes destination_buf);