Fix to use mkstemp for temp shadow file creation
Do not rely on randomString() for tempShadowFile, as it uses '/' in random
set, and cause file creation error. Also, it's safe to use mkstemp to create
temp shadow file with random name suffixing shadow file name.
Change-Id: I0b80cc6d7c002e732e22f660e50b0701acac15fe
Signed-off-by: Richard Marian Thomaiyar <richard.marian.thomaiyar@linux.intel.com>
diff --git a/user.cpp b/user.cpp
index f92fe10..447bfcd 100644
--- a/user.cpp
+++ b/user.cpp
@@ -58,22 +58,16 @@
salt.resize(SALT_LENGTH);
salt = randomString(SALT_LENGTH);
- auto tempShadowFile = std::string("/etc/__") +
- randomString(SALT_LENGTH) +
- std::string("__");
-
// Apply the change. Updates could be directly made to shadow
// but then any update must be contained within the boundary
// of that user, else it would run into next entry and thus
// corrupting it. Classic example is when a password is set on
// a user ID that does not have a prior password
- applyPassword(SHADOW_FILE, tempShadowFile,
- newPassword, salt);
+ applyPassword(SHADOW_FILE, newPassword, salt);
return;
}
void User::applyPassword(const std::string& shadowFile,
- const std::string& tempFile,
const std::string& password,
const std::string& salt)
{
@@ -91,14 +85,30 @@
return raiseException(errno, "Error opening shadow file");
}
- // Open the temp shadow file for writing
+ // open temp shadow file, by suffixing random name in shadow file name.
+ std::vector<char> tempFileName(shadowFile.begin(), shadowFile.end());
+ std::vector<char> fileTemplate = {
+ '_', '_', 'X', 'X', 'X', 'X', 'X', 'X', '\0' };
+ tempFileName.insert(
+ tempFileName.end(), fileTemplate.begin(), fileTemplate.end());
+
+ int fd = mkstemp(tempFileName.data());
+ if (fd == -1)
+ {
+ return raiseException(errno, "Error creating temp shadow file");
+ }
+
+ std::string strTempFileName(tempFileName.data());
+ // Open the temp shadow file for writing from provided fd
// By "true", remove it at exit if still there.
// This is needed to cleanup the temp file at exception
- phosphor::user::File temp(tempFile, "w", true);
+ phosphor::user::File temp(fd, strTempFileName, "w", true);
if ((temp)() == NULL)
{
+ close(fd);
return raiseException(errno, "Error opening temp shadow file");
}
+ fd = -1; // don't use fd anymore, as the File object owns it
// Change the permission of this new temp file
// to be same as shadow so that it's secure
@@ -154,9 +164,12 @@
// Done
endspent();
+ // flush contents to file first, before renaming to avoid
+ // corruption during power failure
+ fflush((temp)());
// Everything must be fine at this point
- fs::rename(tempFile, shadowFile);
+ fs::rename(strTempFileName, shadowFile);
return;
}