From 070a3e49fdd8a6ecd5c71f6ee018ebe89ccd7a1f Mon Sep 17 00:00:00 2001 From: Vishwanatha Subbanna Date: Wed, 6 Sep 2017 11:40:45 +0530 Subject: Update shadow password file with new password Change-Id: Ida7c1aba6f17ac6f006f159d08e2638808f3a54c Signed-off-by: Vishwanatha Subbanna --- Makefile.am | 4 +- configure.ac | 7 +++ file.hpp | 73 ++++++++++++++++++++++++++ shadowlock.hpp | 52 ++++++++++++++++++ user.cpp | 162 +++++++++++++++++++++++++++++++++++++++++++++++++++++---- user.hpp | 33 +++++++++++- 6 files changed, 318 insertions(+), 13 deletions(-) create mode 100644 file.hpp create mode 100644 shadowlock.hpp diff --git a/Makefile.am b/Makefile.am index aebc8a2..1e1db3e 100644 --- a/Makefile.am +++ b/Makefile.am @@ -8,8 +8,10 @@ phosphor_user_manager_SOURCES = \ phosphor_user_manager_LDFLAGS = $(SDBUSPLUS_LIBS) \ $(PHOSPHOR_DBUS_INTERFACES_LIBS) \ + $(PHOSPHOR_LOGGING_LIBS) \ -lcrypt \ -lstdc++fs phosphor_user_manager_CXXFLAGS = $(SYSTEMD_CFLAGS) \ - $(PHOSPHOR_DBUS_INTERFACES_CFLAGS) + $(PHOSPHOR_DBUS_INTERFACES_CFLAGS) \ + $(PHOSPHOR_LOGGING_CFLAGS) diff --git a/configure.ac b/configure.ac index 9144029..c6bb3fe 100644 --- a/configure.ac +++ b/configure.ac @@ -14,11 +14,18 @@ AC_PROG_MAKE_SET # Checks for libraries. PKG_CHECK_MODULES([SDBUSPLUS], [sdbusplus],, [AC_MSG_ERROR([Could not find sdbusplus...openbmc/sdbusplus package required])]) PKG_CHECK_MODULES([PHOSPHOR_DBUS_INTERFACES], [phosphor-dbus-interfaces],, [AC_MSG_ERROR([Could not find phosphor-dbus-interfaces...openbmc/phosphor-dbus-interfaces package required])]) +PKG_CHECK_MODULES([PHOSPHOR_LOGGING], [phosphor-logging],, [AC_MSG_ERROR([Could not find phosphor-logging...openbmc/phosphor-logging package required])]) AC_ARG_VAR(USER_MANAGER_BUSNAME, [The Dbus busname to own]) AS_IF([test "x$USER_MANAGER_BUSNAME" == "x"], [USER_MANAGER_BUSNAME="xyz.openbmc_project.User.Manager"]) AC_DEFINE_UNQUOTED([USER_MANAGER_BUSNAME], ["$USER_MANAGER_BUSNAME"], [The DBus busname to own]) +# Default crypt algorithm to choose if one not found in shadow file +# Per crypt(3), 1 is for MD5 +AC_ARG_VAR(DEFAULT_CRYPT_ALGO, [The default crypt algorithm if one not found in shadow]) +AS_IF([test "x$DEFAULT_CRYPT_ALGO" == "x"], [DEFAULT_CRYPT_ALGO="1"]) +AC_DEFINE_UNQUOTED([DEFAULT_CRYPT_ALGO], ["$DEFAULT_CRYPT_ALGO"], [The default crypt algorithm if one not found in shadow]) + # Checks for typedefs, structures, and compiler characteristics. AX_CXX_COMPILE_STDCXX_14([noext]) AX_APPEND_COMPILE_FLAGS([-Wall -Werror], [CXXFLAGS]) diff --git a/file.hpp b/file.hpp new file mode 100644 index 0000000..ae5cf85 --- /dev/null +++ b/file.hpp @@ -0,0 +1,73 @@ +#pragma once + +#include +#include +namespace phosphor +{ +namespace user +{ + +namespace fs = std::experimental::filesystem; + +/** @class File + * @brief Responsible for handling file pointer + * Needed by putspent(3) + */ +class File +{ + private: + /** @brief handler for operating on file */ + FILE *fp = NULL; + + /** @brief File name. Needed in the case where the temp + * needs to be removed + */ + const std::string& name; + + /** @brief Should the file be removed at exit */ + bool removeOnExit = false; + + public: + File() = delete; + File(const File&) = delete; + File& operator=(const File&) = delete; + File(File&&) = delete; + File& operator=(File&&) = delete; + + /** @brief Opens file and uses it to do file operation + * + * @param[in] name - File name + * @param[in] mode - File open mode + * @param[in] removeOnExit - File to be removed at exit or no + */ + File(const std::string& name, + const std::string& mode, + bool removeOnExit = false) : + name(name), + removeOnExit(removeOnExit) + { + fp = fopen(name.c_str(), mode.c_str()); + } + + ~File() + { + if (fp) + { + fclose(fp); + } + + // Needed for exception safety + if (removeOnExit && fs::exists(name)) + { + fs::remove(name); + } + } + + auto operator()() + { + return fp; + } +}; + +} // namespace user +} // namespace phosphor diff --git a/shadowlock.hpp b/shadowlock.hpp new file mode 100644 index 0000000..dc17b5a --- /dev/null +++ b/shadowlock.hpp @@ -0,0 +1,52 @@ +#pragma once + +#include +#include +#include +#include +#include +#include +namespace phosphor +{ +namespace user +{ +namespace shadow +{ + +using InternalFailure = sdbusplus::xyz::openbmc_project::Common:: + Error::InternalFailure; +using namespace phosphor::logging; + +/** @class Lock + * @brief Responsible for locking and unlocking /etc/shadow + */ +class Lock +{ + public: + Lock(const Lock&) = delete; + Lock& operator=(const Lock&) = delete; + Lock(Lock&&) = delete; + Lock& operator=(Lock&&) = delete; + + /** @brief Default constructor that just locks the shadow file */ + Lock() + { + if (!lckpwdf()) + { + log("Locking Shadow failed"); + elog(); + } + } + ~Lock() + { + if(!ulckpwdf()) + { + log("Un-Locking Shadow failed"); + elog(); + } + } +}; + +} // namespace shadow +} // namespace user +} // namespace phosphor diff --git a/user.cpp b/user.cpp index f4b0e5e..24cd988 100644 --- a/user.cpp +++ b/user.cpp @@ -16,16 +16,57 @@ #include #include #include +#include #include #include +#include +#include #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; + // 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); + + 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); + return; +} + +void User::applyPassword(const std::string& shadowFile, + const std::string& tempFile, + const std::string& password, + const std::string& salt) { // Needed by getspnam_r struct spwd shdp; @@ -34,33 +75,132 @@ void User::setPassword(std::string newPassword) // This should be fine even if SHA512 is used. std::array buffer{}; - // 1: Read /etc/shadow for the user - auto r = getspnam_r(user.c_str(), &shdp, buffer.data(), - buffer.max_size(), &pshdp); + // Open the shadow file for reading + phosphor::user::File shadow(shadowFile, "r"); + if ((shadow)() == NULL) + { + throw std::runtime_error("Error opening shadow file"); + // TODO: Throw error + } + + // Open the temp shadow file for writing + // 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); + if ((temp)() == NULL) + { + throw std::runtime_error("Error opening temp shadow file"); + // TODO: Throw error + } + + // 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; - // TODO: Throw an error + throw std::runtime_error("Error reading permission of shadow"); + // TODO: Throw error } - // Done reading + r = fchmod(fileno((temp)()), st.st_mode); + if (r < 0) + { + throw std::runtime_error("Error setting permission on temp file"); + // TODO: Throw error + } + + // Read shadow file and process + while (true) + { + auto r = fgetspent_r((shadow)(), &shdp, buffer.data(), + buffer.max_size(), &pshdp); + if (r) + { + // Done with all entries + 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(hash.c_str()); + } + + // Apply + r = putspent(&shdp, (temp)()); + if (r < 0) + { + throw std::runtime_error("Error updating temp shadow entry"); + // TODO: Throw exception + } + } // All entries + + // Done endspent(); - // 2: Parse and get crypt algo - auto cryptAlgo = getCryptField(shdp.sp_pwdp); + // Everything must be fine at this point + fs::rename(tempFile, shadowFile); + return; +} + +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()) { - // TODO: Throw error getting crypt field + throw std::runtime_error("Error finding crypt algo"); + // TODO: Throw error } - // TODO: Update the password in next commit - return; + // 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); } diff --git a/user.hpp b/user.hpp index aa40820..4c18c7a 100644 --- a/user.hpp +++ b/user.hpp @@ -65,6 +65,24 @@ class User : public Interface /** @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 @@ -81,7 +99,7 @@ class User : public Interface static std::string generateHash(const std::string& password, const std::string& salt); - /** @brief returns salt string with $ delimiter. + /** @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 @@ -89,6 +107,19 @@ class User : public Interface */ 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] tempFile - Temporary file + * @param[in] password - clear text password + * @param[in] salt - salt + */ + void applyPassword(const std::string& shadowFile, + const std::string& tempFile, + const std::string& password, + const std::string& salt); }; } // namespace user -- cgit v1.2.1