From d4b65f644e17923290376e782aa31d51d5fc13f5 Mon Sep 17 00:00:00 2001 From: Liav A Date: Fri, 2 Dec 2022 11:33:57 +0200 Subject: [PATCH] Kernel: Allow opening some device nodes sparingly for jailed processes From now on, we don't allow jailed processes to open all device nodes in /dev, but only allow jailed processes to open /dev/full, /dev/zero, /dev/null, and various TTY and PTY devices (and not including virtual consoles) so we basically restrict applications to what they can do when they are in jail. The motivation for this type of restriction is to ensure that even if a remote code execution occurred, the damage that can be done is very small. We also don't restrict reading and writing on device nodes that were already opened, because that limit seems not useful, especially in the case where we do want to provide an OpenFileDescription to such device but nothing further than that. --- Kernel/Bus/VirtIO/ConsolePort.cpp | 2 +- Kernel/Devices/ConsoleDevice.h | 2 ++ Kernel/Devices/Device.cpp | 10 ++++++++++ Kernel/Devices/Device.h | 2 ++ Kernel/Devices/FullDevice.h | 3 +++ Kernel/Devices/KCOVDevice.cpp | 2 +- Kernel/Devices/NullDevice.h | 4 ++++ Kernel/Devices/RandomDevice.h | 3 +++ Kernel/Devices/SelfTTYDevice.h | 3 +++ Kernel/Devices/ZeroDevice.h | 3 +++ Kernel/TTY/MasterPTY.h | 4 ++++ Kernel/TTY/SlavePTY.h | 3 +++ 12 files changed, 39 insertions(+), 2 deletions(-) diff --git a/Kernel/Bus/VirtIO/ConsolePort.cpp b/Kernel/Bus/VirtIO/ConsolePort.cpp index 0676a7a8b82..2ff8a186742 100644 --- a/Kernel/Bus/VirtIO/ConsolePort.cpp +++ b/Kernel/Bus/VirtIO/ConsolePort.cpp @@ -166,7 +166,7 @@ ErrorOr> ConsolePort::open(int options) if (!m_open) m_console.send_open_control_message(m_port, true); - return File::open(options); + return Device::open(options); } } diff --git a/Kernel/Devices/ConsoleDevice.h b/Kernel/Devices/ConsoleDevice.h index a3913cf4315..3800a58baa0 100644 --- a/Kernel/Devices/ConsoleDevice.h +++ b/Kernel/Devices/ConsoleDevice.h @@ -28,6 +28,8 @@ public: virtual ErrorOr read(OpenFileDescription&, u64, Kernel::UserOrKernelBuffer&, size_t) override; virtual ErrorOr write(OpenFileDescription&, u64, Kernel::UserOrKernelBuffer const&, size_t) override; virtual StringView class_name() const override { return "Console"sv; } + // ^Device + virtual bool is_openable_by_jailed_processes() const override { return true; } void put_char(char); diff --git a/Kernel/Devices/Device.cpp b/Kernel/Devices/Device.cpp index 833a6d9d588..df306c7d925 100644 --- a/Kernel/Devices/Device.cpp +++ b/Kernel/Devices/Device.cpp @@ -58,6 +58,16 @@ ErrorOr> Device::pseudo_path(OpenFileDescription const&) return KString::formatted("device:{},{}", major(), minor()); } +ErrorOr> Device::open(int options) +{ + TRY(Process::current().jail().with([&](auto& my_jail) -> ErrorOr { + if (my_jail && !is_openable_by_jailed_processes()) + return Error::from_errno(EPERM); + return {}; + })); + return File::open(options); +} + void Device::process_next_queued_request(Badge, AsyncDeviceRequest const& completed_request) { SpinlockLocker lock(m_requests_lock); diff --git a/Kernel/Devices/Device.h b/Kernel/Devices/Device.h index 8c5bfd2ab69..dcfb263fd7d 100644 --- a/Kernel/Devices/Device.h +++ b/Kernel/Devices/Device.h @@ -43,6 +43,7 @@ public: MinorNumber minor() const { return m_minor; } virtual ErrorOr> pseudo_path(OpenFileDescription const&) const override; + virtual ErrorOr> open(int options) override; UserID uid() const { return m_uid; } GroupID gid() const { return m_gid; } @@ -50,6 +51,7 @@ public: virtual bool is_device() const override { return true; } virtual void will_be_destroyed() override; virtual void after_inserting(); + virtual bool is_openable_by_jailed_processes() const { return false; } void process_next_queued_request(Badge, AsyncDeviceRequest const&); template diff --git a/Kernel/Devices/FullDevice.h b/Kernel/Devices/FullDevice.h index 4f8df218f01..443d07721fe 100644 --- a/Kernel/Devices/FullDevice.h +++ b/Kernel/Devices/FullDevice.h @@ -20,6 +20,9 @@ public: private: FullDevice(); + // ^Device + virtual bool is_openable_by_jailed_processes() const override { return true; } + // ^CharacterDevice virtual ErrorOr read(OpenFileDescription&, u64, UserOrKernelBuffer&, size_t) override; virtual ErrorOr write(OpenFileDescription&, u64, UserOrKernelBuffer const&, size_t) override; diff --git a/Kernel/Devices/KCOVDevice.cpp b/Kernel/Devices/KCOVDevice.cpp index 647a3ab1610..634562aea46 100644 --- a/Kernel/Devices/KCOVDevice.cpp +++ b/Kernel/Devices/KCOVDevice.cpp @@ -74,7 +74,7 @@ ErrorOr> KCOVDevice::open(int options) kcov_instance->set_state(KCOVInstance::OPENED); proc_instance->set(pid, kcov_instance); - return File::open(options); + return Device::open(options); } ErrorOr KCOVDevice::ioctl(OpenFileDescription&, unsigned request, Userspace arg) diff --git a/Kernel/Devices/NullDevice.h b/Kernel/Devices/NullDevice.h index 255c67f01cb..c7767a1d589 100644 --- a/Kernel/Devices/NullDevice.h +++ b/Kernel/Devices/NullDevice.h @@ -20,6 +20,10 @@ public: private: NullDevice(); + + // ^Device + virtual bool is_openable_by_jailed_processes() const override { return true; } + // ^CharacterDevice virtual ErrorOr read(OpenFileDescription&, u64, UserOrKernelBuffer&, size_t) override; virtual ErrorOr write(OpenFileDescription&, u64, UserOrKernelBuffer const&, size_t) override; diff --git a/Kernel/Devices/RandomDevice.h b/Kernel/Devices/RandomDevice.h index b234792fb91..9983635df76 100644 --- a/Kernel/Devices/RandomDevice.h +++ b/Kernel/Devices/RandomDevice.h @@ -20,6 +20,9 @@ public: private: RandomDevice(); + // ^Device + virtual bool is_openable_by_jailed_processes() const override { return true; } + // ^CharacterDevice virtual ErrorOr read(OpenFileDescription&, u64, UserOrKernelBuffer&, size_t) override; virtual ErrorOr write(OpenFileDescription&, u64, UserOrKernelBuffer const&, size_t) override; diff --git a/Kernel/Devices/SelfTTYDevice.h b/Kernel/Devices/SelfTTYDevice.h index 4bf5b7c733c..54956aaed8f 100644 --- a/Kernel/Devices/SelfTTYDevice.h +++ b/Kernel/Devices/SelfTTYDevice.h @@ -20,6 +20,9 @@ public: private: SelfTTYDevice(); + // ^Device + virtual bool is_openable_by_jailed_processes() const override { return true; } + // ^CharacterDevice virtual ErrorOr> open(int options) override; virtual ErrorOr read(OpenFileDescription&, u64, UserOrKernelBuffer&, size_t) override; diff --git a/Kernel/Devices/ZeroDevice.h b/Kernel/Devices/ZeroDevice.h index 77c9f08e57c..c539d4f0481 100644 --- a/Kernel/Devices/ZeroDevice.h +++ b/Kernel/Devices/ZeroDevice.h @@ -20,6 +20,9 @@ public: private: ZeroDevice(); + // ^Device + virtual bool is_openable_by_jailed_processes() const override { return true; } + // ^CharacterDevice virtual ErrorOr read(OpenFileDescription&, u64, UserOrKernelBuffer&, size_t) override; virtual ErrorOr write(OpenFileDescription&, u64, UserOrKernelBuffer const&, size_t) override; diff --git a/Kernel/TTY/MasterPTY.h b/Kernel/TTY/MasterPTY.h index edb7fb3b34d..31653c9a5bf 100644 --- a/Kernel/TTY/MasterPTY.h +++ b/Kernel/TTY/MasterPTY.h @@ -29,6 +29,10 @@ public: private: explicit MasterPTY(unsigned index, NonnullOwnPtr buffer); + + // ^Device + virtual bool is_openable_by_jailed_processes() const override { return true; } + // ^CharacterDevice virtual ErrorOr read(OpenFileDescription&, u64, UserOrKernelBuffer&, size_t) override; virtual ErrorOr write(OpenFileDescription&, u64, UserOrKernelBuffer const&, size_t) override; diff --git a/Kernel/TTY/SlavePTY.h b/Kernel/TTY/SlavePTY.h index 15c08626743..c576e0a72a2 100644 --- a/Kernel/TTY/SlavePTY.h +++ b/Kernel/TTY/SlavePTY.h @@ -26,6 +26,9 @@ public: virtual FileBlockerSet& blocker_set() override; private: + // ^Device + virtual bool is_openable_by_jailed_processes() const override { return true; } + // ^TTY virtual ErrorOr> pseudo_name() const override; virtual ErrorOr on_tty_write(UserOrKernelBuffer const&, size_t) override;