diff options
author | Andreas Kling <kling@serenityos.org> | 2021-01-21 09:58:31 +0100 |
---|---|---|
committer | Andreas Kling <kling@serenityos.org> | 2021-01-21 11:08:20 +0100 |
commit | 77e0598c6d5ab732cf43297f629867f01da08833 (patch) | |
tree | 25c2f625fbbd36e80e8f7aaefc8242410941a8c0 /Userland | |
parent | c9a7f81dc33692da3a079a202862c502cd1de1d0 (diff) | |
download | serenity-77e0598c6d5ab732cf43297f629867f01da08833.zip |
passwd+LibCore: Make passwd replace /etc files atomically
Before this patch, we had a nasty race condition when changing a user's
password: there was a time window between truncating /etc/shadow and
writing out its new contents, where you could simply "su" to root
without using a password.
Instead of writing directly to /etc/passwd and /etc/shadow, we now
create temporary files in /etc and fill them with the new contents.
Those files are then atomically renamed to /etc/passwd and /etc/shadow.
Sadly, fixing this race requires giving the passwd program a lot more
privileges. This is something we can and should improve upon. :^)
Diffstat (limited to 'Userland')
-rw-r--r-- | Userland/Libraries/LibCore/Account.cpp | 64 | ||||
-rw-r--r-- | Userland/Utilities/passwd.cpp | 34 |
2 files changed, 50 insertions, 48 deletions
diff --git a/Userland/Libraries/LibCore/Account.cpp b/Userland/Libraries/LibCore/Account.cpp index 7e4d40669a..af20395d4f 100644 --- a/Userland/Libraries/LibCore/Account.cpp +++ b/Userland/Libraries/LibCore/Account.cpp @@ -26,12 +26,14 @@ #include <AK/Base64.h> #include <AK/Random.h> +#include <AK/ScopeGuard.h> #include <LibCore/Account.h> #include <LibCore/File.h> #include <grp.h> #include <pwd.h> #include <stdio.h> #include <string.h> +#include <sys/stat.h> #include <unistd.h> namespace Core { @@ -273,39 +275,57 @@ bool Account::sync() ASSERT(m_shadow_file); ASSERT(m_shadow_file->mode() == Core::File::OpenMode::ReadWrite); - // FIXME: Maybe reorganize this to create temporary files and finish it completely before renaming them to /etc/{passwd,shadow} - // If truncation succeeds but write fails, we'll have an empty file :( + auto new_passwd_file_content = generate_passwd_file(); + auto new_shadow_file_content = generate_shadow_file(); - auto new_passwd_file = generate_passwd_file(); - auto new_shadow_file = generate_shadow_file(); - - if (new_passwd_file.is_null() || new_shadow_file.is_null()) { + if (new_passwd_file_content.is_null() || new_shadow_file_content.is_null()) { ASSERT_NOT_REACHED(); } - if (!m_passwd_file->seek(0) || !m_shadow_file->seek(0)) { - ASSERT_NOT_REACHED(); - } + char new_passwd_name[] = "/etc/passwd.XXXXXX"; + char new_shadow_name[] = "/etc/shadow.XXXXXX"; - if (!m_passwd_file->truncate(0)) { - dbgln("Truncating passwd file failed."); - return false; - } + { + auto new_passwd_fd = mkstemp(new_passwd_name); + if (new_passwd_fd < 0) { + perror("mkstemp"); + ASSERT_NOT_REACHED(); + } + ScopeGuard new_passwd_fd_guard = [new_passwd_fd] { close(new_passwd_fd); }; + auto new_shadow_fd = mkstemp(new_shadow_name); + if (new_shadow_fd < 0) { + perror("mkstemp"); + ASSERT_NOT_REACHED(); + } + ScopeGuard new_shadow_fd_guard = [new_shadow_fd] { close(new_shadow_fd); }; - if (!m_passwd_file->write(new_passwd_file)) { - // FIXME: Improve Core::File::write() error reporting. - dbgln("Writing to passwd file failed."); - return false; + if (fchmod(new_passwd_fd, 0644) < 0) { + perror("fchmod"); + ASSERT_NOT_REACHED(); + } + + auto nwritten = write(new_passwd_fd, new_passwd_file_content.characters(), new_passwd_file_content.length()); + if (nwritten < 0) { + perror("write"); + ASSERT_NOT_REACHED(); + } + ASSERT(static_cast<size_t>(nwritten) == new_passwd_file_content.length()); + + nwritten = write(new_shadow_fd, new_shadow_file_content.characters(), new_shadow_file_content.length()); + if (nwritten < 0) { + perror("write"); + ASSERT_NOT_REACHED(); + } + ASSERT(static_cast<size_t>(nwritten) == new_shadow_file_content.length()); } - if (!m_shadow_file->truncate(0)) { - dbgln("Truncating shadow file failed."); + if (rename(new_passwd_name, "/etc/passwd") < 0) { + perror("Failed to install new /etc/passwd"); return false; } - if (!m_shadow_file->write(new_shadow_file)) { - // FIXME: Improve Core::File::write() error reporting. - dbgln("Writing to shadow file failed."); + if (rename(new_shadow_name, "/etc/shadow") < 0) { + perror("Failed to install new /etc/shadow"); return false; } diff --git a/Userland/Utilities/passwd.cpp b/Userland/Utilities/passwd.cpp index b0a36e437a..62c8b321a4 100644 --- a/Userland/Utilities/passwd.cpp +++ b/Userland/Utilities/passwd.cpp @@ -1,5 +1,6 @@ /* * Copyright (c) 2020, Peter Elliott <pelliott@ualberta.ca> + * Copyright (c) 2021, Andreas Kling <kling@serenityos.org> * All rights reserved. * * Redistribution and use in source and binary forms, with or without @@ -39,22 +40,17 @@ int main(int argc, char** argv) return 1; } - if (pledge("stdio wpath rpath cpath tty id", nullptr) < 0) { - perror("pledge"); + if (setegid(0) < 0) { + perror("setegid"); return 1; } - if (unveil("/etc/passwd", "rwc") < 0) { - perror("unveil"); - return 1; - } - - if (unveil("/etc/group", "rwc") < 0) { - perror("unveil"); + if (pledge("stdio wpath rpath wpath cpath fattr tty", nullptr) < 0) { + perror("pledge"); return 1; } - if (unveil("/etc/shadow", "rwc") < 0) { + if (unveil("/etc", "rwc") < 0) { perror("unveil"); return 1; } @@ -86,23 +82,9 @@ int main(int argc, char** argv) return 1; } - // Drop privileges after opening all the files through the Core::Account object. - auto gid = getgid(); - if (setresgid(gid, gid, gid) < 0) { - perror("setresgid"); - return 1; - } - - auto uid = getuid(); - if (setresuid(uid, uid, uid) < 0) { - perror("setresuid"); - return 1; - } - - // Make sure /etc/passwd is open and ready for reading, then we can drop a bunch of pledge promises. setpwent(); - if (pledge("stdio tty", nullptr) < 0) { + if (pledge("stdio rpath wpath cpath fattr tty", nullptr) < 0) { perror("pledge"); return 1; } @@ -131,7 +113,7 @@ int main(int argc, char** argv) target_account.set_password(new_password.value().characters()); } - if (pledge("stdio", nullptr) < 0) { + if (pledge("stdio rpath wpath cpath fattr", nullptr) < 0) { perror("pledge"); return 1; } |