diff options
| -rw-r--r-- | Makefile.am | 3 | ||||
| -rw-r--r-- | test/Makefile.am | 49 | ||||
| -rw-r--r-- | test/utest.cpp | 184 | ||||
| -rw-r--r-- | user.cpp | 264 | ||||
| -rw-r--r-- | user.hpp | 130 |
5 files changed, 23 insertions, 607 deletions
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 @@ AM_CPPFLAGS = -I$(top_srcdir) # 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 @@ ldap_config_test_LDADD = $(top_builddir)/phosphor-ldap-config/ldap_configuratio $(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 |
