summaryrefslogtreecommitdiff
path: root/Userland
diff options
context:
space:
mode:
authorAndreas Kling <kling@serenityos.org>2021-01-21 09:58:31 +0100
committerAndreas Kling <kling@serenityos.org>2021-01-21 11:08:20 +0100
commit77e0598c6d5ab732cf43297f629867f01da08833 (patch)
tree25c2f625fbbd36e80e8f7aaefc8242410941a8c0 /Userland
parentc9a7f81dc33692da3a079a202862c502cd1de1d0 (diff)
downloadserenity-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.cpp64
-rw-r--r--Userland/Utilities/passwd.cpp34
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;
}