commit 17b8827818db7eb96e0016ec04cf025cd84fffc6
author Jason M. Bills <jason.m.bills@intel.com> Mon May 22 14:24:02 2023 -0700
committer Jason M. Bills <jason.m.bills@intel.com> Tue Aug 08 13:02:30 2023 -0700
tree 915ee1092ceae908a0d4335e9e101d81f9677c1b
parent ac921a5e34027da72b96190b52f8ac9b053fc9c0

Remove temp file on failure

If the parameter is not found, the temp file is left in the file system
since it isn't renamed. This deletes the temp file in that case.

If the source file is not found, the temp file is left in the file
system. This deletes the temp file in that case.

This updates the tests to cover these two conditions and confirm that
the temp file is not present.

Tested:
Corrupted the faillock.conf deny parameter and the pwquality.conf minlen
parameter and confirmed that after attempting to modify them through
Redfish, the _tmp files are not left in the /etc/security folder.

Change-Id: I59e543d12f7e509533182339d224c4e8dc9b580d
Signed-off-by: Jason M. Bills <jason.m.bills@intel.com>

test/user_mgr_test.cpp

diff --git a/test/user_mgr_test.cpp b/test/user_mgr_test.cpp
index d5049a0..0a19918 100644
--- a/test/user_mgr_test.cpp
+++ b/test/user_mgr_test.cpp
@@ -401,6 +401,28 @@
     EXPECT_EQ(deny, "3");
 }
 
+TEST_F(UserMgrInTest, SetPamModuleArgValueTempFileOnSuccess)
+{
+    EXPECT_EQ(setPamModuleArgValue("pam_pwhistory.so", "remember", "1"), 0);
+
+    std::string tmpFile = tempPamConfigFile + "_tmp";
+    EXPECT_FALSE(std::filesystem::exists(tmpFile));
+}
+
+TEST_F(UserMgrInTest, SetPamModuleConfValueTempFileOnSuccess)
+{
+    EXPECT_EQ(setPamModuleConfValue(tempPWQualityConfigFile, "minlen", "16"),
+              0);
+
+    std::string tmpFile = tempPWQualityConfigFile + "_tmp";
+    EXPECT_FALSE(std::filesystem::exists(tmpFile));
+
+    EXPECT_EQ(setPamModuleConfValue(tempFaillockConfigFile, "deny", "3"), 0);
+
+    tmpFile = tempFaillockConfigFile + "_tmp";
+    EXPECT_FALSE(std::filesystem::exists(tmpFile));
+}
+
 TEST_F(UserMgrInTest, GetPamModuleArgValueOnFailure)
 {
     EXPECT_NO_THROW(dumpStringToFile("whatever", tempPamConfigFile));
@@ -458,6 +480,47 @@
     EXPECT_EQ(setPamModuleConfValue(tempFaillockConfigFile, "deny", "3"), -1);
 }
 
+TEST_F(UserMgrInTest, SetPamModuleArgValueTempFileOnFailure)
+{
+    EXPECT_NO_THROW(dumpStringToFile("whatever", tempPamConfigFile));
+    EXPECT_EQ(setPamModuleArgValue("pam_pwhistory.so", "remember", "1"), -1);
+
+    std::string tmpFile = tempPamConfigFile + "_tmp";
+    EXPECT_FALSE(std::filesystem::exists(tmpFile));
+
+    EXPECT_NO_THROW(removeFile(tempPamConfigFile));
+    EXPECT_EQ(setPamModuleArgValue("pam_pwhistory.so", "remember", "1"), -1);
+
+    EXPECT_FALSE(std::filesystem::exists(tmpFile));
+}
+
+TEST_F(UserMgrInTest, SetPamModuleConfValueTempFileOnFailure)
+{
+    EXPECT_NO_THROW(dumpStringToFile("whatever", tempPWQualityConfigFile));
+    EXPECT_EQ(setPamModuleConfValue(tempPWQualityConfigFile, "minlen", "16"),
+              -1);
+
+    std::string tmpFile = tempPWQualityConfigFile + "_tmp";
+    EXPECT_FALSE(std::filesystem::exists(tmpFile));
+
+    EXPECT_NO_THROW(removeFile(tempPWQualityConfigFile));
+    EXPECT_EQ(setPamModuleConfValue(tempPWQualityConfigFile, "minlen", "16"),
+              -1);
+
+    EXPECT_FALSE(std::filesystem::exists(tmpFile));
+
+    EXPECT_NO_THROW(dumpStringToFile("whatever", tempFaillockConfigFile));
+    EXPECT_EQ(setPamModuleConfValue(tempFaillockConfigFile, "deny", "3"), -1);
+
+    tmpFile = tempFaillockConfigFile + "_tmp";
+    EXPECT_FALSE(std::filesystem::exists(tmpFile));
+
+    EXPECT_NO_THROW(removeFile(tempFaillockConfigFile));
+    EXPECT_EQ(setPamModuleConfValue(tempFaillockConfigFile, "deny", "3"), -1);
+
+    EXPECT_FALSE(std::filesystem::exists(tmpFile));
+}
+
 TEST_F(UserMgrInTest, IsUserExistEmptyInputThrowsError)
 {
     EXPECT_THROW(

user_mgr.cpp

diff --git a/user_mgr.cpp b/user_mgr.cpp
index 6d58fa8..9cc68e1 100644
--- a/user_mgr.cpp
+++ b/user_mgr.cpp
@@ -736,6 +736,8 @@
     {
         lg2::error("Failed to open pam configuration file {FILENAME}",
                    "FILENAME", fileName);
+        // Delete the unused tmp file
+        std::remove(tmpFileName.c_str());
         return failure;
     }
     std::string line;
@@ -783,6 +785,8 @@
             return success;
         }
     }
+    // No changes, so delete the unused tmp file
+    std::remove(tmpFileName.c_str());
     return failure;
 }
 
@@ -797,6 +801,8 @@
     {
         lg2::error("Failed to open pam configuration file {FILENAME}",
                    "FILENAME", confFile);
+        // Delete the unused tmp file
+        std::remove(tmpConfFile.c_str());
         return failure;
     }
     std::string line;
@@ -841,6 +847,8 @@
             return success;
         }
     }
+    // No changes, so delete the unused tmp file
+    std::remove(tmpConfFile.c_str());
     return failure;
 }