From 439f447ba8be1a5cc2e0fbb9afd8dcaf519e5109 Mon Sep 17 00:00:00 2001 From: Andreas Kling Date: Thu, 21 Jan 2021 11:17:06 +0100 Subject: [PATCH] LibCore+su+passwd: Don't keep /etc/passwd and /etc/shadow open Now that we've moved to atomic replacement of these files when altering them, we don't need to keep them open for the lifetime of Core::Account so just simplify this and close them when they are not needed. --- Userland/Libraries/LibCore/Account.cpp | 61 ++++++-------------------- Userland/Libraries/LibCore/Account.h | 24 ++-------- Userland/Utilities/passwd.cpp | 4 +- Userland/Utilities/su.cpp | 4 +- 4 files changed, 22 insertions(+), 71 deletions(-) diff --git a/Userland/Libraries/LibCore/Account.cpp b/Userland/Libraries/LibCore/Account.cpp index af20395d4f8..04981b7cc92 100644 --- a/Userland/Libraries/LibCore/Account.cpp +++ b/Userland/Libraries/LibCore/Account.cpp @@ -65,36 +65,14 @@ static Vector get_gids(const StringView& username) return extra_gids; } -Result Account::from_passwd(const passwd& pwd, Core::Account::OpenPasswdFile open_passwd_file, Core::Account::OpenShadowFile open_shadow_file) +Result Account::from_passwd(const passwd& pwd) { - RefPtr passwd_file; - if (open_passwd_file != Core::Account::OpenPasswdFile::No) { - auto open_mode = open_passwd_file == Core::Account::OpenPasswdFile::ReadOnly - ? Core::File::OpenMode::ReadOnly - : Core::File::OpenMode::ReadWrite; - auto file_or_error = Core::File::open("/etc/passwd", open_mode); - if (file_or_error.is_error()) - return file_or_error.error(); - passwd_file = file_or_error.value(); - } - - RefPtr shadow_file; - if (open_shadow_file != Core::Account::OpenShadowFile::No) { - auto open_mode = open_shadow_file == Core::Account::OpenShadowFile::ReadOnly - ? Core::File::OpenMode::ReadOnly - : Core::File::OpenMode::ReadWrite; - auto file_or_error = Core::File::open("/etc/shadow", open_mode); - if (file_or_error.is_error()) - return file_or_error.error(); - shadow_file = file_or_error.value(); - } - - Account account(pwd, get_gids(pwd.pw_name), move(passwd_file), move(shadow_file)); + Account account(pwd, get_gids(pwd.pw_name)); endpwent(); return account; } -Result Account::from_name(const char* username, Core::Account::OpenPasswdFile open_passwd_file, Core::Account::OpenShadowFile open_shadow_file) +Result Account::from_name(const char* username) { struct passwd* pwd = nullptr; errno = 0; @@ -105,10 +83,10 @@ Result Account::from_name(const char* username, Core::Account:: return String(strerror(errno)); } - return from_passwd(*pwd, open_passwd_file, open_shadow_file); + return from_passwd(*pwd); } -Result Account::from_uid(uid_t uid, Core::Account::OpenPasswdFile open_passwd_file, Core::Account::OpenShadowFile open_shadow_file) +Result Account::from_uid(uid_t uid) { struct passwd* pwd = nullptr; errno = 0; @@ -119,7 +97,7 @@ Result Account::from_uid(uid_t uid, Core::Account::OpenPasswdFi return String(strerror(errno)); } - return from_passwd(*pwd, open_passwd_file, open_shadow_file); + return from_passwd(*pwd); } bool Account::authenticate(const char* password) const @@ -169,10 +147,8 @@ void Account::delete_password() m_password_hash = ""; } -Account::Account(const passwd& pwd, Vector extra_gids, RefPtr passwd_file, RefPtr shadow_file) - : m_passwd_file(move(passwd_file)) - , m_shadow_file(move(shadow_file)) - , m_username(pwd.pw_name) +Account::Account(const passwd& pwd, Vector extra_gids) + : m_username(pwd.pw_name) , m_uid(pwd.pw_uid) , m_gid(pwd.pw_gid) , m_gecos(pwd.pw_gecos) @@ -180,9 +156,7 @@ Account::Account(const passwd& pwd, Vector extra_gids, RefPtr , m_shell(pwd.pw_shell) , m_extra_gids(extra_gids) { - if (m_shadow_file) { - load_shadow_file(); - } + load_shadow_file(); } String Account::generate_passwd_file() const @@ -221,17 +195,15 @@ String Account::generate_passwd_file() const void Account::load_shadow_file() { - ASSERT(m_shadow_file); - ASSERT(m_shadow_file->is_open()); - - if (!m_shadow_file->seek(0)) { - ASSERT_NOT_REACHED(); - } + auto file_or_error = Core::File::open("/etc/shadow", Core::File::ReadOnly); + ASSERT(!file_or_error.is_error()); + auto shadow_file = file_or_error.release_value(); + ASSERT(shadow_file->is_open()); Vector entries; for (;;) { - auto line = m_shadow_file->read_line(); + auto line = shadow_file->read_line(); if (line.is_null()) break; auto parts = line.split(':'); @@ -270,11 +242,6 @@ String Account::generate_shadow_file() const bool Account::sync() { - ASSERT(m_passwd_file); - ASSERT(m_passwd_file->mode() == Core::File::OpenMode::ReadWrite); - ASSERT(m_shadow_file); - ASSERT(m_shadow_file->mode() == Core::File::OpenMode::ReadWrite); - auto new_passwd_file_content = generate_passwd_file(); auto new_shadow_file_content = generate_shadow_file(); diff --git a/Userland/Libraries/LibCore/Account.h b/Userland/Libraries/LibCore/Account.h index fdd7d9bea93..433d3c4926d 100644 --- a/Userland/Libraries/LibCore/Account.h +++ b/Userland/Libraries/LibCore/Account.h @@ -30,7 +30,6 @@ #include #include #include -#include #include #include @@ -38,20 +37,8 @@ namespace Core { class Account { public: - enum class OpenPasswdFile { - No, - ReadOnly, - ReadWrite, - }; - - enum class OpenShadowFile { - No, - ReadOnly, - ReadWrite, - }; - - static Result from_name(const char* username, OpenPasswdFile = OpenPasswdFile::No, OpenShadowFile = OpenShadowFile::No); - static Result from_uid(uid_t uid, OpenPasswdFile = OpenPasswdFile::No, OpenShadowFile = OpenShadowFile::No); + static Result from_name(const char* username); + static Result from_uid(uid_t uid); bool authenticate(const char* password) const; bool login() const; @@ -76,17 +63,14 @@ public: bool sync(); private: - static Result from_passwd(const passwd&, OpenPasswdFile, OpenShadowFile); + static Result from_passwd(const passwd&); - Account(const passwd& pwd, Vector extra_gids, RefPtr passwd_file, RefPtr shadow_file); + Account(const passwd& pwd, Vector extra_gids); void load_shadow_file(); String generate_passwd_file() const; String generate_shadow_file() const; - RefPtr m_passwd_file; - RefPtr m_shadow_file; - String m_username; // Contents of passwd field in passwd entry. diff --git a/Userland/Utilities/passwd.cpp b/Userland/Utilities/passwd.cpp index 62c8b321a4e..fc2a382ff5a 100644 --- a/Userland/Utilities/passwd.cpp +++ b/Userland/Utilities/passwd.cpp @@ -74,8 +74,8 @@ int main(int argc, char** argv) uid_t current_uid = getuid(); auto account_or_error = (username) - ? Core::Account::from_name(username, Core::Account::OpenPasswdFile::ReadWrite, Core::Account::OpenShadowFile::ReadWrite) - : Core::Account::from_uid(current_uid, Core::Account::OpenPasswdFile::ReadWrite, Core::Account::OpenShadowFile::ReadWrite); + ? Core::Account::from_name(username) + : Core::Account::from_uid(current_uid); if (account_or_error.is_error()) { warnln("Core::Account::{}: {}", (username) ? "from_name" : "from_uid", account_or_error.error()); diff --git a/Userland/Utilities/su.cpp b/Userland/Utilities/su.cpp index 33776155705..546e28b529c 100644 --- a/Userland/Utilities/su.cpp +++ b/Userland/Utilities/su.cpp @@ -58,8 +58,8 @@ int main(int argc, char** argv) } auto account_or_error = (user) - ? Core::Account::from_name(user, Core::Account::OpenPasswdFile::No, Core::Account::OpenShadowFile::ReadOnly) - : Core::Account::from_uid(0, Core::Account::OpenPasswdFile::No, Core::Account::OpenShadowFile::ReadOnly); + ? Core::Account::from_name(user) + : Core::Account::from_uid(0); if (account_or_error.is_error()) { fprintf(stderr, "Core::Account::from_name: %s\n", account_or_error.error().characters()); return 1;