Removing unused SetPassword D-Bus API method
Password update is done through pam_chauthtok() API,
and don't use SetPassword. Removing the unused code.
Tested-by:
N/A.
Change-Id: I42a5b7c73bc2cb2404801df1c1cd057a94a1a924
Signed-off-by: Sumanth Bhat <sumanth.bhat@intel.com>
Signed-off-by: Richard Marian Thomaiyar <richard.marian.thomaiyar@linux.intel.com>
diff --git a/Makefile.am b/Makefile.am
index 9da26ef..b138aea 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -1,9 +1,8 @@
sbin_PROGRAMS = phosphor-user-manager
-noinst_HEADERS = user.hpp user_mgr.hpp users.hpp
+noinst_HEADERS = user_mgr.hpp users.hpp
phosphor_user_manager_SOURCES = \
- user.cpp \
mainapp.cpp \
user_mgr.cpp \
users.cpp
diff --git a/test/Makefile.am b/test/Makefile.am
index 06b7bb2..00f969c 100644
--- a/test/Makefile.am
+++ b/test/Makefile.am
@@ -3,33 +3,28 @@
# Run all 'check' test programs
TESTS = $(check_PROGRAMS)
-# Build/add utest to test suite
-check_PROGRAMS = utest
-utest_CPPFLAGS = -Igtest \
- $(GTEST_CPPFLAGS) \
- $(AM_CPPFLAGS) \
- $(PHOSPHOR_LOGGING_CFLAGS) \
- $(SDBUSPLUS_CFLAGS)
+cppflags = -Igtest \
+ $(GTEST_CPPFLAGS) \
+ $(AM_CPPFLAGS) \
+ $(PHOSPHOR_LOGGING_CFLAGS) \
+ $(SDBUSPLUS_CFLAGS)
-utest_CXXFLAGS = $(PTHREAD_CFLAGS)
+cxxflags = $(PTHREAD_CFLAGS)
-utest_LDFLAGS = -lgtest_main \
- -lgtest \
- $(PTHREAD_LIBS) \
- $(OESDK_TESTCASE_FLAGS) \
- $(PHOSPHOR_DBUS_INTERFACES_LIBS) \
- $(PHOSPHOR_LOGGING_LIBS) \
- $(SDBUSPLUS_LIBS) \
- -lcrypt \
- -lstdc++fs
+ldflags = -lgtest_main \
+ -lgtest \
+ $(PTHREAD_LIBS) \
+ $(OESDK_TESTCASE_FLAGS) \
+ $(PHOSPHOR_DBUS_INTERFACES_LIBS) \
+ $(PHOSPHOR_LOGGING_LIBS) \
+ $(SDBUSPLUS_LIBS) \
+ -lcrypt \
+ -lstdc++fs
-utest_SOURCES = utest.cpp
-utest_LDADD = $(top_builddir)/user.o
-
-check_PROGRAMS += ldap_config_test
-ldap_config_test_CPPFLAGS = $(utest_CPPFLAGS)
-ldap_config_test_CXXFLAGS = $(utest_CXXFLAGS)
-ldap_config_test_LDFLAGS = $(utest_LDFLAGS) \
+check_PROGRAMS = ldap_config_test
+ldap_config_test_CPPFLAGS = $(cppflags)
+ldap_config_test_CXXFLAGS = $(cxxflags)
+ldap_config_test_LDFLAGS = $(ldflags) \
-lldap \
-lgmock
ldap_config_test_SOURCES = ldap_config_test.cpp utils_test.cpp
@@ -38,9 +33,9 @@
$(top_builddir)/phosphor-ldap-config/ldap_serialize.o
check_PROGRAMS += ldap_mapper_test
-ldap_mapper_test_CPPFLAGS = $(utest_CPPFLAGS)
-ldap_mapper_test_CXXFLAGS = $(utest_CXXFLAGS)
-ldap_mapper_test_LDFLAGS = $(utest_LDFLAGS)
+ldap_mapper_test_CPPFLAGS = $(cppflags)
+ldap_mapper_test_CXXFLAGS = $(cxxflags)
+ldap_mapper_test_LDFLAGS = $(ldflags)
ldap_mapper_test_SOURCES = ldap_mapper_test.cpp
ldap_mapper_test_LDADD = $(top_builddir)/phosphor-ldap-mapper/ldap_mapper_entry.o \
$(top_builddir)/phosphor-ldap-mapper/ldap_mapper_mgr.o \
diff --git a/test/utest.cpp b/test/utest.cpp
deleted file mode 100644
index 5e17283..0000000
--- a/test/utest.cpp
+++ /dev/null
@@ -1,184 +0,0 @@
-#include <sys/stat.h>
-#include <string>
-#include <fstream>
-#include <experimental/filesystem>
-#include <gtest/gtest.h>
-#include <sdbusplus/bus.hpp>
-#include "user.hpp"
-namespace phosphor
-{
-namespace user
-{
-
-namespace fs = std::experimental::filesystem;
-
-constexpr auto path = "/dummy/user";
-constexpr auto testShadow = "/tmp/__tshadow__";
-constexpr auto shadowCompare = "/tmp/__tshadowCompare__";
-
-// New password
-constexpr auto password = "passw0rd";
-
-constexpr auto MD5 = "1";
-constexpr auto SHA512 = "6";
-constexpr auto salt = "1G.cK/YP";
-
-// Example entry matching /etc/shadow structure
-constexpr auto spPwdp = "$1$1G.cK/YP$JI5t0oliPxZveXOvLcZ/H.:17344:1:90:7:::";
-
-class UserTest : public ::testing::Test
-{
- public:
- const std::string md5Salt =
- '$' + std::string(MD5) + '$' + std::string(salt) + '$';
- const std::string shaSalt =
- '$' + std::string(SHA512) + '$' + std::string(salt) + '$';
-
- const std::string entry =
- fs::path(path).filename().string() + ':' + std::string(spPwdp);
- sdbusplus::bus::bus bus;
- phosphor::user::User user;
-
- // Gets called as part of each TEST_F construction
- UserTest() : bus(sdbusplus::bus::new_default()), user(bus, path)
- {
- // Create a shadow file entry
- std::ofstream file(testShadow);
- file << entry;
- file.close();
-
- // File to compare against
- std::ofstream compare(shadowCompare);
- compare << entry;
- compare.close();
- }
-
- // Gets called as part of each TEST_F destruction
- ~UserTest()
- {
- if (fs::exists(testShadow))
- {
- fs::remove(testShadow);
- }
-
- if (fs::exists(shadowCompare))
- {
- fs::remove(shadowCompare);
- }
- }
-
- /** @brief wrapper for get crypt field */
- auto getCryptField(char* data)
- {
- return User::getCryptField(std::forward<decltype(data)>(data));
- }
-
- /** @brief wrapper for getSaltString */
- auto getSaltString(const std::string& crypt, const std::string& salt)
- {
- return User::getSaltString(std::forward<decltype(crypt)>(crypt),
- std::forward<decltype(salt)>(salt));
- }
-
- /** @brief wrapper for generateHash */
- auto generateHash(const std::string& password, const std::string& salt)
- {
- return User::generateHash(std::forward<decltype(password)>(password),
- std::forward<decltype(salt)>(salt));
- }
-
- /** @brief Applies the new password */
- auto applyPassword()
- {
- return user.applyPassword(testShadow, password, salt);
- }
-};
-
-/** @brief Makes sure that SHA512 crypt field is extracted
- */
-TEST_F(UserTest, sha512GetCryptField)
-{
- auto salt = const_cast<char*>(shaSalt.c_str());
- EXPECT_EQ(SHA512, this->getCryptField(salt));
-}
-
-/** @brief Makes sure that MD5 crypt field is extracted as default
- */
-TEST_F(UserTest, md55GetCryptFieldDefault)
-{
- auto salt = const_cast<char*>("hello");
- EXPECT_EQ(MD5, this->getCryptField(salt));
-}
-
-/** @brief Makes sure that MD5 crypt field is extracted
- */
-TEST_F(UserTest, md55GetCryptField)
-{
- auto salt = const_cast<char*>(md5Salt.c_str());
- EXPECT_EQ(MD5, this->getCryptField(salt));
-}
-
-/** @brief Makes sure that salt string is put within $$
- */
-TEST_F(UserTest, getSaltString)
-{
- EXPECT_EQ(md5Salt, this->getSaltString(MD5, salt));
-}
-
-/** @brief Makes sure hash is generated correctly
- */
-TEST_F(UserTest, generateHash)
-{
- std::string sample = crypt(password, md5Salt.c_str());
- std::string actual = generateHash(password, md5Salt);
- EXPECT_EQ(sample, actual);
-}
-
-/** @brief Verifies that the correct password is written to file
- */
-TEST_F(UserTest, applyPassword)
-{
- // Update the password
- applyPassword();
-
- // Read files and compare
- std::ifstream shadow(testShadow);
- std::ifstream copy(shadowCompare);
-
- std::string shadowEntry;
- shadow >> shadowEntry;
-
- std::string shadowCompareEntry;
- copy >> shadowCompareEntry;
-
- EXPECT_EQ(shadowEntry, shadowCompareEntry);
-}
-
-/** @brief Verifies the permissions are correct
- */
-TEST_F(UserTest, verifyShadowPermission)
-{
- // Change the permission to 400-> -r--------
- chmod(testShadow, S_IRUSR);
- chmod(shadowCompare, S_IRUSR);
-
- // Update the password so that the temp file is in action
- applyPassword();
-
- // Compare the permission of 2 files.
- // file rename would make sure that the permissions
- // of old are moved to new
- struct stat shadow
- {
- };
- struct stat compare
- {
- };
-
- stat(testShadow, &shadow);
- stat(shadowCompare, &compare);
- EXPECT_EQ(shadow.st_mode, compare.st_mode);
-}
-
-} // namespace user
-} // namespace phosphor
diff --git a/user.cpp b/user.cpp
deleted file mode 100644
index 6999a98..0000000
--- a/user.cpp
+++ /dev/null
@@ -1,264 +0,0 @@
-/**
- * Copyright © 2017 IBM Corporation
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-#include <cstring>
-#include <unistd.h>
-#include <sys/types.h>
-#include <sys/stat.h>
-#include <shadow.h>
-#include <array>
-#include <random>
-#include <errno.h>
-#include <xyz/openbmc_project/Common/error.hpp>
-#include <phosphor-logging/log.hpp>
-#include <phosphor-logging/elog.hpp>
-#include <phosphor-logging/elog-errors.hpp>
-#include "user.hpp"
-#include "file.hpp"
-#include "shadowlock.hpp"
-#include "config.h"
-namespace phosphor
-{
-namespace user
-{
-
-constexpr auto SHADOW_FILE = "/etc/shadow";
-
-// See crypt(3)
-constexpr int SALT_LENGTH = 16;
-
-using namespace phosphor::logging;
-using InsufficientPermission =
- sdbusplus::xyz::openbmc_project::Common::Error::InsufficientPermission;
-using InternalFailure =
- sdbusplus::xyz::openbmc_project::Common::Error::InternalFailure;
-// Sets or updates the password
-void User::setPassword(std::string newPassword)
-{
- // Gate any access to /etc/shadow
- phosphor::user::shadow::Lock lock();
-
- // rewind to the start of shadow entry
- setspent();
-
- // Generate a random string from set [A-Za-z0-9./]
- std::string salt{};
- salt.resize(SALT_LENGTH);
- salt = randomString(SALT_LENGTH);
-
- // 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, newPassword, salt);
- return;
-}
-
-void User::applyPassword(const std::string& shadowFile,
- const std::string& password, const std::string& salt)
-{
- // Needed by getspnam_r
- struct spwd shdp;
- struct spwd* pshdp;
-
- // This should be fine even if SHA512 is used.
- std::array<char, 1024> buffer{};
-
- // Open the shadow file for reading
- phosphor::user::File shadow(shadowFile, "r");
- if ((shadow)() == NULL)
- {
- return raiseException(errno, "Error opening shadow file");
- }
-
- // 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(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
- struct stat st
- {
- };
- auto r = fstat(fileno((shadow)()), &st);
- if (r < 0)
- {
- return raiseException(errno, "Error reading shadow file mode");
- }
-
- r = fchmod(fileno((temp)()), st.st_mode);
- if (r < 0)
- {
- return raiseException(errno, "Error setting temp file mode");
- }
-
- // Read shadow file and process
- while (true)
- {
- auto r = fgetspent_r((shadow)(), &shdp, buffer.data(),
- buffer.max_size(), &pshdp);
- if (r)
- {
- if (errno == EACCES || errno == ERANGE)
- {
- return raiseException(errno, "Error reading shadow file");
- }
- else
- {
- // Seem to have run over all
- break;
- }
- }
-
- // Hash of password if the user matches
- std::string hash{};
-
- // Matched user
- if (user == shdp.sp_namp)
- {
- // Update with new hashed password
- hash = hashPassword(shdp.sp_pwdp, password, salt);
- shdp.sp_pwdp = const_cast<char*>(hash.c_str());
- }
-
- // Apply
- r = putspent(&shdp, (temp)());
- if (r < 0)
- {
- return raiseException(errno, "Error updating temp shadow file");
- }
- } // All entries
-
- // 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(strTempFileName, shadowFile);
- return;
-}
-
-void User::raiseException(int errNo, const std::string& errMsg)
-{
- using namespace std::string_literals;
- if (errNo == EACCES)
- {
- auto message = "Access denied "s + errMsg;
- log<level::ERR>(message.c_str());
- elog<InsufficientPermission>();
- }
- else
- {
- log<level::ERR>(errMsg.c_str(), entry("USER=%s", user.c_str()),
- entry("ERRNO=%d", errNo));
- elog<InternalFailure>();
- }
-}
-
-std::string User::hashPassword(char* spPwdp, const std::string& password,
- const std::string& salt)
-{
- // Parse and get crypt algo
- auto cryptAlgo = getCryptField(spPwdp);
- if (cryptAlgo.empty())
- {
- log<level::ERR>("Error finding crypt algo",
- entry("USER=%s", user.c_str()));
- elog<InternalFailure>();
- }
-
- // Update shadow password pointer with hash
- auto saltString = getSaltString(cryptAlgo, salt);
- return generateHash(password, saltString);
-}
-
-// Returns a random string in set [A-Za-z0-9./]
-// of size numChars
-const std::string User::randomString(int length)
-{
- // Populated random string
- std::string random{};
-
- // Needed per crypt(3)
- std::string set = "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijk"
- "lmnopqrstuvwxyz0123456789./";
-
- // Will be used to obtain a seed for the random number engine
- std::random_device rd;
-
- // Standard mersenne_twister_engine seeded with rd()
- std::mt19937 gen(rd());
-
- std::uniform_int_distribution<> dis(0, set.size() - 1);
- for (int count = 0; count < length; count++)
- {
- // Use dis to transform the random unsigned int generated by
- // gen into a int in [1, SALT_LENGTH]
- random.push_back(set.at(dis(gen)));
- }
- return random;
-}
-
-// Extract crypto algorithm field
-CryptAlgo User::getCryptField(char* spPwdp)
-{
- char* savePtr{};
- if (std::string{spPwdp}.front() != '$')
- {
- return DEFAULT_CRYPT_ALGO;
- }
- return strtok_r(spPwdp, "$", &savePtr);
-}
-
-// Returns specific format of salt string
-std::string User::getSaltString(const std::string& crypt,
- const std::string& salt)
-{
- return '$' + crypt + '$' + salt + '$';
-}
-
-// Given a password and salt, generates hash
-std::string User::generateHash(const std::string& password,
- const std::string& salt)
-{
- return crypt(password.c_str(), salt.c_str());
-}
-
-} // namespace user
-} // namespace phosphor
diff --git a/user.hpp b/user.hpp
deleted file mode 100644
index 2e57702..0000000
--- a/user.hpp
+++ /dev/null
@@ -1,130 +0,0 @@
-#pragma once
-
-#include <cstring>
-#include <experimental/filesystem>
-#include <sdbusplus/bus.hpp>
-#include <sdbusplus/server/object.hpp>
-#include <xyz/openbmc_project/User/Password/server.hpp>
-namespace phosphor
-{
-namespace user
-{
-
-using CryptAlgo = std::string;
-
-namespace fs = std::experimental::filesystem;
-namespace Base = sdbusplus::xyz::openbmc_project::User::server;
-using Interface = sdbusplus::server::object::object<Base::Password>;
-
-/** @class User
- * @brief Responsible for managing a specific user account.
- * It is implementing just the Password interface
- * for now.
- */
-class User : public Interface
-{
- public:
- User() = delete;
- ~User() = default;
- User(const User&) = delete;
- User& operator=(const User&) = delete;
- User(User&&) = delete;
- User& operator=(User&&) = delete;
-
- /** @brief Constructs User object.
- *
- * @param[in] bus - sdbusplus handler
- * @param[in] path - D-Bus path
- */
- User(sdbusplus::bus::bus& bus, const char* path) :
- Interface(bus, path), bus(bus), path(path),
- user(fs::path(path).filename())
- {
- // Do nothing
- }
-
- /** @brief user password set method. If this is called for
- * a user ID that already has the password, the password
- * would be updated, else password would be created.
- * Since this needs an already authenticated session,
- * old password is not needed.
- *
- * @param[in] newPassword - New password
- */
- void setPassword(std::string newPassword) override;
-
- private:
- /** @brief sdbusplus handler */
- sdbusplus::bus::bus& bus;
-
- /** @brief object path */
- const std::string& path;
-
- /** @brief User id extracted from object path */
- const std::string user;
-
- /** @brief Returns a random string from set [A-Za-z0-9./]
- * of length size
- *
- * @param[in] numChars - length of string
- */
- static const std::string randomString(int length);
-
- /** @brief Returns password hash created with crypt algo,
- * salt and password
- *
- * @param[in] spPwdp - sp_pwdp of struct spwd
- * @param[in] password - clear text password
- * @param[in] salt - Random salt
- */
- std::string hashPassword(char* spPwdp, const std::string& password,
- const std::string& salt);
-
- /** @brief Extracts crypto number from the shadow entry for user
- *
- * @param[in] spPwdp - sp_pwdp of struct spwd
- */
- static CryptAlgo getCryptField(char* spPwdp);
-
- /** @brief Generates one-way hash based on salt and password
- *
- * @param[in] password - clear text password
- * @param[in] salt - Combination of crypto method and salt
- * Eg: $1$HELLO$, where in 1 is crypto method
- * and HELLO is salt
- */
- static std::string generateHash(const std::string& password,
- const std::string& salt);
-
- /** @brief Returns salt string with $ delimiter.
- * Eg: If crypt is 1 and salt is HELLO, returns $1$HELLO$
- *
- * @param[in] crypt - Crypt number in string
- * @param[in] salt - salt
- */
- static std::string getSaltString(const std::string& crypt,
- const std::string& salt);
-
- /** @brief Applies the password for a given user.
- * Writes shadow entries into a temp file
- *
- * @param[in] shadowFile - shadow password file
- * @param[in] password - clear text password
- * @param[in] salt - salt
- */
- void applyPassword(const std::string& shadowFile,
- const std::string& password, const std::string& salt);
-
- /** @brief Wrapper for raising exception
- *
- * @param[in] errNo - errno
- * @param[in] errMsg - Error message
- */
- void raiseException(int errNo, const std::string& errMsg);
-
- /** @brief For enabling test cases */
- friend class UserTest;
-};
-
-} // namespace user
-} // namespace phosphor