Refactoring of certificates managing and storing
This commit is about third stage code refactoring proposed by Zbigniew
Kurzynski (zbigniew.kurzynski@intel.com) on the mailing list
("phosphor-certificate-manager refactoring"): "Changing the way of
managing and storing TrustStore certificates".
Following changes are being implemented:
- each certificate has its own and unique ID,
- authority certificates are kept in files with random names under
/etc/ssl/certs/authority and symlinks (based on subject name hash) are
created to satisfy OpenSSL library,
- restarting bmcweb was moved from certificate class to certs_manager
class
- certificate uniqueness is based on certificate ID and checked while
installing and replacing operation in certs_manager class.
Tested by doing installing/replacing/removing operations on certificate
storage using RedFish API.
Signed-off-by: Zbigniew Lukwinski <zbigniew.lukwinski@linux.intel.com>
Change-Id: I0b02a10b940279c46ad9ee07925794262133b1b0
diff --git a/certificate.cpp b/certificate.cpp
index 0682216..1ac5426 100644
--- a/certificate.cpp
+++ b/certificate.cpp
@@ -71,28 +71,101 @@
{NID_ad_timeStamping, "Timestamping"},
{NID_code_sign, "CodeSigning"}};
-std::string Certificate::getSubjectHash(const X509_STORE_CTX_Ptr& storeCtx)
+std::string Certificate::generateCertId(const std::string& certPath)
{
- X509_Ptr cert(X509_STORE_CTX_get_current_cert(storeCtx.get()), ::X509_free);
+ const X509_Ptr cert = loadCert(certPath);
+ unsigned long subjectNameHash = X509_subject_name_hash(cert.get());
+ static constexpr auto CERT_ID_LENGTH = 9;
+ char idBuff[CERT_ID_LENGTH];
+
+ snprintf(idBuff, CERT_ID_LENGTH, "%08lx", subjectNameHash);
+
+ return std::string(idBuff);
+}
+
+std::string
+ Certificate::generateUniqueFilePath(const std::string& directoryPath)
+{
+ char* filePath = tempnam(directoryPath.c_str(), NULL);
+ if (filePath == NULL)
+ {
+ log<level::ERR>(
+ "Error occured while creating random certificate file path",
+ entry("DIR=%s", directoryPath.c_str()));
+ elog<InternalFailure>();
+ }
+ std::string filePathStr(filePath);
+ free(filePath);
+ return filePathStr;
+}
+
+std::string Certificate::generateAuthCertFileX509Path(
+ const std::string& certSrcFilePath, const std::string& certDstDirPath)
+{
+ const X509_Ptr cert = loadCert(certSrcFilePath);
unsigned long hash = X509_subject_name_hash(cert.get());
+ static constexpr auto CERT_HASH_LENGTH = 9;
+ char hashBuf[CERT_HASH_LENGTH];
- char hashBuf[9];
+ snprintf(hashBuf, CERT_HASH_LENGTH, "%08lx", hash);
- sprintf(hashBuf, "%08lx", hash);
+ const std::string certHash(hashBuf);
+ const std::string certDstFileX509Path =
+ certDstDirPath + "/" + certHash + ".0";
+ if (fs::exists(certDstFileX509Path))
+ {
+ log<level::ERR>("Authority certificate x509 file path already used",
+ entry("CERT=%s", certDstFileX509Path.c_str()));
+ elog<InternalFailure>();
+ }
- return std::string(hashBuf);
+ return certDstFileX509Path;
+}
+
+std::string
+ Certificate::generateAuthCertFilePath(const std::string& certSrcFilePath)
+{
+ // If there is a certificate file path (which means certificate replacement
+ // is doing) use it (do not create new one)
+ if (!certFilePath.empty())
+ {
+ return certFilePath;
+ }
+ // If source certificate file is located in the certificates directory use
+ // it (do not create new one)
+ else if (fs::path(certSrcFilePath).parent_path().string() ==
+ certInstallPath)
+ {
+ return certSrcFilePath;
+ }
+ // Otherwise generate new file name/path
+ else
+ {
+ return generateUniqueFilePath(certInstallPath);
+ }
+}
+
+std::string
+ Certificate::generateCertFilePath(const std::string& certSrcFilePath)
+{
+ if (certType == phosphor::certs::AUTHORITY)
+ {
+ return generateAuthCertFilePath(certSrcFilePath);
+ }
+ else
+ {
+ return certInstallPath;
+ }
}
Certificate::Certificate(sdbusplus::bus::bus& bus, const std::string& objPath,
const CertificateType& type,
- const UnitsToRestart& unit,
const CertInstallPath& installPath,
const CertUploadPath& uploadPath,
- bool isSkipUnitReload,
const CertWatchPtr& certWatchPtr, Manager& parent) :
CertIfaces(bus, objPath.c_str(), true),
- bus(bus), objectPath(objPath), certType(type), unitToRestart(unit),
- certInstallPath(installPath), certWatchPtr(certWatchPtr), manager(parent)
+ bus(bus), objectPath(objPath), certType(type), certInstallPath(installPath),
+ certWatchPtr(certWatchPtr), manager(parent)
{
auto installHelper = [this](const auto& filePath) {
if (!compareKeys(filePath))
@@ -113,41 +186,33 @@
appendKeyMap[CLIENT] = appendPrivateKey;
appendKeyMap[AUTHORITY] = [](const std::string& filePath) {};
+ // Generate certificate file path
+ certFilePath = generateCertFilePath(uploadPath);
+
// install the certificate
- install(uploadPath, isSkipUnitReload);
+ install(uploadPath);
this->emit_object_added();
}
Certificate::~Certificate()
{
- std::string installPath = certInstallPath;
-
- if (certType == phosphor::certs::AUTHORITY)
- {
- installPath += "/" + certHash + ".0";
- }
-
- if (!fs::remove(installPath))
+ if (!fs::remove(certFilePath))
{
log<level::INFO>("Certificate file not found!",
- entry("PATH=%s", installPath.c_str()));
- }
- else if (!unitToRestart.empty())
- {
- reloadOrReset(unitToRestart);
+ entry("PATH=%s", certFilePath.c_str()));
}
}
void Certificate::replace(const std::string filePath)
{
- install(filePath, false);
+ manager.replaceCertificate(this, filePath);
}
-void Certificate::install(const std::string& filePath, bool isSkipUnitReload)
+void Certificate::install(const std::string& certSrcFilePath)
{
log<level::INFO>("Certificate install ",
- entry("FILEPATH=%s", filePath.c_str()));
+ entry("FILEPATH=%s", certSrcFilePath.c_str()));
auto errCode = X509_V_OK;
// stop watch for user initiated certificate install
@@ -157,27 +222,28 @@
}
// Verify the certificate file
- fs::path file(filePath);
+ fs::path file(certSrcFilePath);
if (!fs::exists(file))
{
- log<level::ERR>("File is Missing", entry("FILE=%s", filePath.c_str()));
+ log<level::ERR>("File is Missing",
+ entry("FILE=%s", certSrcFilePath.c_str()));
elog<InternalFailure>();
}
try
{
- if (fs::file_size(filePath) == 0)
+ if (fs::file_size(certSrcFilePath) == 0)
{
// file is empty
log<level::ERR>("File is empty",
- entry("FILE=%s", filePath.c_str()));
+ entry("FILE=%s", certSrcFilePath.c_str()));
elog<InvalidCertificate>(Reason("File is empty"));
}
}
catch (const fs::filesystem_error& e)
{
// Log Error message
- log<level::ERR>(e.what(), entry("FILE=%s", filePath.c_str()));
+ log<level::ERR>(e.what(), entry("FILE=%s", certSrcFilePath.c_str()));
elog<InternalFailure>();
}
@@ -205,22 +271,22 @@
elog<InternalFailure>();
}
// Load Certificate file.
- errCode = X509_LOOKUP_load_file(lookup.get(), filePath.c_str(),
+ errCode = X509_LOOKUP_load_file(lookup.get(), certSrcFilePath.c_str(),
X509_FILETYPE_PEM);
if (errCode != 1)
{
log<level::ERR>("Error occured during X509_LOOKUP_load_file call",
- entry("FILE=%s", filePath.c_str()));
+ entry("FILE=%s", certSrcFilePath.c_str()));
elog<InvalidCertificate>(Reason("Invalid certificate file format"));
}
// Load Certificate file into the X509 structre.
- X509_Ptr cert = std::move(loadCert(filePath));
+ X509_Ptr cert = loadCert(certSrcFilePath);
X509_STORE_CTX_Ptr storeCtx(X509_STORE_CTX_new(), ::X509_STORE_CTX_free);
if (!storeCtx)
{
log<level::ERR>("Error occured during X509_STORE_CTX_new call",
- entry("FILE=%s", filePath.c_str()));
+ entry("FILE=%s", certSrcFilePath.c_str()));
elog<InternalFailure>();
}
@@ -228,7 +294,7 @@
if (errCode != 1)
{
log<level::ERR>("Error occured during X509_STORE_CTX_init call",
- entry("FILE=%s", filePath.c_str()));
+ entry("FILE=%s", certSrcFilePath.c_str()));
elog<InternalFailure>();
}
@@ -249,13 +315,14 @@
log<level::INFO>(
"Error occured during X509_verify_cert call, checking for known "
"error",
- entry("FILE=%s", filePath.c_str()), entry("ERRCODE=%d", errCode),
+ entry("FILE=%s", certSrcFilePath.c_str()),
+ entry("ERRCODE=%d", errCode),
entry("ERROR_STR=%s", X509_verify_cert_error_string(errCode)));
}
else
{
log<level::ERR>("Error occured during X509_verify_cert call",
- entry("FILE=%s", filePath.c_str()));
+ entry("FILE=%s", certSrcFilePath.c_str()));
elog<InternalFailure>();
}
@@ -280,7 +347,7 @@
log<level::ERR>("Unsupported Type", entry("TYPE=%s", certType.c_str()));
elog<InternalFailure>();
}
- appendIter->second(filePath);
+ appendIter->second(certSrcFilePath);
// Invoke type specific compare keys function.
auto compIter = typeFuncMap.find(certType);
@@ -289,32 +356,12 @@
log<level::ERR>("Unsupported Type", entry("TYPE=%s", certType.c_str()));
elog<InternalFailure>();
}
- compIter->second(filePath);
-
- std::string newHash = getSubjectHash(storeCtx);
- std::string installPath = certInstallPath;
-
- if (certType == phosphor::certs::AUTHORITY)
- {
- // Save under OpenSSL lib acceptable name
- installPath += "/" + newHash + ".0";
-
- // Check if we are not trying to overwrite already existing certificate
- if (certHash != newHash && fs::exists(installPath) &&
- filePath != installPath)
- {
- using NotAllowed =
- sdbusplus::xyz::openbmc_project::Common::Error::NotAllowed;
- using Reason = xyz::openbmc_project::Common::NotAllowed::REASON;
-
- elog<NotAllowed>(Reason("Certificate already exist"));
- }
- }
+ compIter->second(certSrcFilePath);
// Copy the certificate to the installation path
// During bootup will be parsing existing file so no need to
// copy it.
- if (filePath != installPath)
+ if (certSrcFilePath != certFilePath)
{
std::ifstream inputCertFileStream;
std::ofstream outputCertFileStream;
@@ -327,8 +374,8 @@
try
{
- inputCertFileStream.open(filePath);
- outputCertFileStream.open(installPath, std::ios::out);
+ inputCertFileStream.open(certSrcFilePath);
+ outputCertFileStream.open(certFilePath, std::ios::out);
outputCertFileStream << inputCertFileStream.rdbuf() << std::flush;
inputCertFileStream.close();
outputCertFileStream.close();
@@ -337,45 +384,19 @@
{
log<level::ERR>("Failed to copy certificate",
entry("ERR=%s", e.what()),
- entry("SRC=%s", filePath.c_str()),
- entry("DST=%s", installPath.c_str()));
+ entry("SRC=%s", certSrcFilePath.c_str()),
+ entry("DST=%s", certFilePath.c_str()));
elog<InternalFailure>();
}
-
- if (certHash != newHash && !certHash.empty() &&
- certType == phosphor::certs::AUTHORITY)
- {
- std::string oldPath = certInstallPath + "/" + certHash + ".0";
-
- // Remove previous file
- try
- {
- fs::remove(oldPath);
- }
- catch (const std::exception& e)
- {
- log<level::ERR>("Failed to remove old certificate",
- entry("ERR=%s", e.what()),
- entry("OLD=%s", oldPath.c_str()),
- entry("NEW=%s", installPath.c_str()));
- }
- }
}
- if (!isSkipUnitReload)
- {
- // restart the units
- if (!unitToRestart.empty())
- {
- reloadOrReset(unitToRestart);
- }
- }
+ storageUpdate();
- // Store current hash
- certHash = newHash;
+ // Keep certificate ID
+ certId = generateCertId(certFilePath);
// Parse the certificate file and populate properties
- populateProperties(installPath);
+ populateProperties(certFilePath);
// restart watch
if (certWatchPtr)
@@ -389,14 +410,47 @@
populateProperties(certInstallPath);
}
-const std::string& Certificate::getHash() const
+std::string Certificate::getCertId() const
{
- return certHash;
+ return certId;
+}
+
+bool Certificate::isSame(const std::string& certPath)
+{
+ return getCertId() == generateCertId(certPath);
+}
+
+void Certificate::storageUpdate()
+{
+ if (certType == phosphor::certs::AUTHORITY)
+ {
+ // Create symbolic link in the certificate directory
+ std::string certFileX509Path;
+ try
+ {
+ if (!certFilePath.empty() &&
+ fs::is_regular_file(fs::path(certFilePath)))
+ {
+ certFileX509Path =
+ generateAuthCertFileX509Path(certFilePath, certInstallPath);
+ fs::create_symlink(fs::path(certFilePath),
+ fs::path(certFileX509Path));
+ }
+ }
+ catch (const std::exception& e)
+ {
+ log<level::ERR>("Failed to create symlink for certificate",
+ entry("ERR=%s", e.what()),
+ entry("FILE=%s", certFilePath.c_str()),
+ entry("SYMLINK=%s", certFileX509Path.c_str()));
+ elog<InternalFailure>();
+ }
+ }
}
void Certificate::populateProperties(const std::string& certPath)
{
- X509_Ptr cert = std::move(loadCert(certPath));
+ X509_Ptr cert = loadCert(certPath);
// Update properties if no error thrown
BIO_MEM_Ptr certBio(BIO_new(BIO_s_mem()), BIO_free);
PEM_write_bio_X509(certBio.get(), cert.get());
@@ -624,31 +678,9 @@
return true;
}
-void Certificate::reloadOrReset(const UnitsToRestart& unit)
-{
- constexpr auto SYSTEMD_SERVICE = "org.freedesktop.systemd1";
- constexpr auto SYSTEMD_OBJ_PATH = "/org/freedesktop/systemd1";
- constexpr auto SYSTEMD_INTERFACE = "org.freedesktop.systemd1.Manager";
- try
- {
- auto method =
- bus.new_method_call(SYSTEMD_SERVICE, SYSTEMD_OBJ_PATH,
- SYSTEMD_INTERFACE, "ReloadOrRestartUnit");
- method.append(unit, "replace");
- bus.call_noreply(method);
- }
- catch (const sdbusplus::exception::SdBusError& e)
- {
- log<level::ERR>("Failed to reload or restart service",
- entry("ERR=%s", e.what()),
- entry("UNIT=%s", unit.c_str()));
- elog<InternalFailure>();
- }
-}
-
void Certificate::delete_()
{
- manager.deleteCertificate(getHash());
+ manager.deleteCertificate(this);
}
} // namespace certs
} // namespace phosphor
diff --git a/certificate.hpp b/certificate.hpp
index 9fa2cdd..feffecc 100644
--- a/certificate.hpp
+++ b/certificate.hpp
@@ -22,7 +22,6 @@
ReplaceIface, DeleteIface>;
using CertificateType = std::string;
-using UnitsToRestart = std::string;
using CertInstallPath = std::string;
using CertUploadPath = std::string;
using InputType = std::string;
@@ -67,17 +66,21 @@
* @param[in] bus - Bus to attach to.
* @param[in] objPath - Object path to attach to
* @param[in] type - Type of the certificate
- * @param[in] unit - Units to restart after a certificate is installed
* @param[in] installPath - Path of the certificate to install
* @param[in] uploadPath - Path of the certificate file to upload
- * @param[in] isSkipUnitReload - If true do not restart units
* @param[in] watchPtr - watch on self signed certificate pointer
*/
Certificate(sdbusplus::bus::bus& bus, const std::string& objPath,
- const CertificateType& type, const UnitsToRestart& unit,
- const CertInstallPath& installPath,
- const CertUploadPath& uploadPath, bool isSkipUnitReload,
- const CertWatchPtr& watchPtr, Manager& parent);
+ const CertificateType& type, const CertInstallPath& installPath,
+ const CertUploadPath& uploadPath, const CertWatchPtr& watchPtr,
+ Manager& parent);
+
+ /** @brief Validate and Replace/Install the certificate file
+ * Install/Replace the existing certificate file with another
+ * (possibly CA signed) Certificate file.
+ * @param[in] filePath - Certificate file path.
+ */
+ void install(const std::string& filePath);
/** @brief Validate certificate and replace the existing certificate
* @param[in] filePath - Certificate file path.
@@ -85,16 +88,30 @@
void replace(const std::string filePath) override;
/** @brief Populate certificate properties by parsing certificate file
- * @return void
*/
void populateProperties();
/**
- * @brief Obtain certificate's subject hash
+ * @brief Obtain certificate ID.
*
- * @return certificate's subject hash
+ * @return Certificate ID.
*/
- const std::string& getHash() const;
+ std::string getCertId() const;
+
+ /**
+ * @brief Check if provied certificate is the same as the current one.
+ *
+ * @param[in] certPath - File path for certificate to check.
+ *
+ * @return Checking result. Return true if certificates are the same,
+ * false if not.
+ */
+ bool isSame(const std::string& certPath);
+
+ /**
+ * @brief Update certificate storage.
+ */
+ void storageUpdate();
/**
* @brief Delete the certificate
@@ -111,15 +128,7 @@
*/
void populateProperties(const std::string& certPath);
- /** @brief Validate and Replace/Install the certificate file
- * Install/Replace the existing certificate file with another
- * (possibly CA signed) Certificate file.
- * @param[in] filePath - Certificate file path.
- * @param[in] isSkipUnitReload - If true do not restart units
- */
- void install(const std::string& filePath, bool isSkipUnitReload);
-
- /** @brief Load Certificate file into the X509 structre.
+ /** @brief Load Certificate file into the X509 structure.
* @param[in] filePath - Certificate and key full file path.
* @return pointer to the X509 structure.
*/
@@ -142,23 +151,67 @@
*/
bool compareKeys(const std::string& filePath);
- /** @brief systemd unit reload or reset helper function
- * Reload if the unit supports it and use a restart otherwise.
- * @param[in] unit - service need to reload.
+ /**
+ * @brief Generate certificate ID based on provided certificate file.
+ *
+ * @param[in] certPath - Certificate file path.
+ *
+ * @return Certificate ID as formatted string.
*/
- void reloadOrReset(const UnitsToRestart& unit);
+ std::string generateCertId(const std::string& certPath);
/**
- * @brief Extracts subject hash
+ * @brief Generate file name which is unique in the provided directory.
*
- * @param[in] storeCtx Pointer to X509_STORE_CTX containing certificate
+ * @param[in] directoryPath - Directory path.
*
- * @return Subject hash as formatted string
+ * @return File path.
*/
- static inline std::string
- getSubjectHash(const X509_STORE_CTX_Ptr& storeCtx);
+ std::string generateUniqueFilePath(const std::string& directoryPath);
- /** @brief Type specific function pointer map **/
+ /**
+ * @brief Generate authority certificate file path corresponding with
+ * OpenSSL requirements.
+ *
+ * Prepare authority certificate file path for provied certificate.
+ * OpenSSL puts some restrictions on the certificate file name pattern.
+ * Certificate full file name needs to consists of basic file name which
+ * is certificate subject name hash and file name extension which is an
+ * integer. More over, certificates files names extensions must be
+ * consecutive integer numbers in case many certificates with the same
+ * subject name.
+ * https://www.boost.org/doc/libs/1_69_0/doc/html/boost_asio/reference/ssl__context/add_verify_path.html
+ * https://www.openssl.org/docs/man1.0.2/man3/SSL_CTX_load_verify_locations.html
+ *
+ * @param[in] certSrcFilePath - Certificate source file path.
+ * @param[in] certDstDirPath - Certificate destination directory path.
+ *
+ * @return Authority certificate file path.
+ */
+ std::string generateAuthCertFileX509Path(const std::string& certSrcFilePath,
+ const std::string& certDstDirPath);
+
+ /**
+ * @brief Generate authority certificate file path based on provided
+ * certificate source file path.
+ *
+ * @param[in] certSrcFilePath - Certificate source file path.
+ *
+ * @return Authority certificate file path.
+ */
+ std::string generateAuthCertFilePath(const std::string& certSrcFilePath);
+
+ /**
+ * @brief Generate certificate file path based on provided certificate
+ * source file path.
+ *
+ * @param[in] certSrcFilePath - Certificate source file path.
+ *
+ * @return Certificate file path.
+ */
+ std::string generateCertFilePath(const std::string& certSrcFilePath);
+
+ /** @brief Type specific function pointer map */
std::unordered_map<InputType, InstallFunc> typeFuncMap;
/** @brief sdbusplus handler */
@@ -167,13 +220,16 @@
/** @brief object path */
std::string objectPath;
- /** @brief Type of the certificate **/
+ /** @brief Type of the certificate */
CertificateType certType;
- /** @brief Unit name associated to the service **/
- UnitsToRestart unitToRestart;
+ /** @brief Stores certificate ID */
+ std::string certId;
- /** @brief Certificate file installation path **/
+ /** @brief Stores certificate file path */
+ std::string certFilePath;
+
+ /** @brief Certificate file installation path */
CertInstallPath certInstallPath;
/** @brief Type specific function pointer map for appending private key */
@@ -182,9 +238,6 @@
/** @brief Certificate file create/update watch */
const CertWatchPtr& certWatchPtr;
- /** @brief Stores certificate subject hash */
- std::string certHash;
-
/** @brief Reference to Certificate Manager */
Manager& manager;
};
diff --git a/certs_manager.cpp b/certs_manager.cpp
index ca6f356..2391267 100644
--- a/certs_manager.cpp
+++ b/certs_manager.cpp
@@ -7,6 +7,7 @@
#include <phosphor-logging/elog-errors.hpp>
#include <xyz/openbmc_project/Certs/error.hpp>
#include <xyz/openbmc_project/Common/error.hpp>
+
namespace phosphor
{
namespace certs
@@ -55,6 +56,7 @@
auto permission = fs::perms::owner_read | fs::perms::owner_write |
fs::perms::owner_exec;
fs::permissions(certDirectory, permission, fs::perm_options::replace);
+ storageUpdate();
}
catch (fs::filesystem_error& e)
{
@@ -140,11 +142,21 @@
elog<NotAllowed>(Reason("Certificates limit reached"));
}
- auto certObjectPath = objectPath + '/' + std::to_string(certIdCounter++);
+ std::string certObjectPath;
+ if (isCertificateUnique(filePath))
+ {
+ certObjectPath = objectPath + '/' + std::to_string(certIdCounter);
+ installedCerts.emplace_back(std::make_unique<Certificate>(
+ bus, certObjectPath, certType, certInstallPath, filePath,
+ certWatchPtr, *this));
+ reloadOrReset(unitToRestart);
+ certIdCounter++;
+ }
+ else
+ {
+ elog<NotAllowed>(Reason("Certificate already exist"));
+ }
- installedCerts.emplace_back(std::make_unique<Certificate>(
- bus, certObjectPath, certType, unitToRestart, certInstallPath, filePath,
- false, certWatchPtr, *this));
return certObjectPath;
}
@@ -156,23 +168,43 @@
// deletion if only applicable for REST server and Bmcweb does not allow
// deletion of certificates
installedCerts.clear();
+ storageUpdate();
+ reloadOrReset(unitToRestart);
}
-void Manager::deleteCertificate(const std::string& certHash)
+void Manager::deleteCertificate(const Certificate* const certificate)
{
std::vector<std::unique_ptr<Certificate>>::iterator const& certIt =
std::find_if(installedCerts.begin(), installedCerts.end(),
- [certHash](std::unique_ptr<Certificate> const& cert) {
- return cert->getHash().compare(certHash) == 0;
+ [certificate](std::unique_ptr<Certificate> const& cert) {
+ return (cert.get() == certificate);
});
if (certIt != installedCerts.end())
{
installedCerts.erase(certIt);
+ storageUpdate();
+ reloadOrReset(unitToRestart);
}
else
{
log<level::ERR>("Certificate does not exist",
- entry("HASH=%s", certHash.c_str()));
+ entry("ID=%s", certificate->getCertId().c_str()));
+ elog<InternalFailure>();
+ }
+}
+
+void Manager::replaceCertificate(Certificate* const certificate,
+ const std::string& filePath)
+{
+ if (isCertificateUnique(filePath, certificate))
+ {
+ certificate->install(filePath);
+ storageUpdate();
+ reloadOrReset(unitToRestart);
+ }
+ else
+ {
+ log<level::ERR>("Certificate already exist");
elog<InternalFailure>();
}
}
@@ -591,10 +623,16 @@
{
try
{
- installedCerts.emplace_back(std::make_unique<Certificate>(
- bus, certObjectPath + std::to_string(certIdCounter++),
- certType, unitToRestart, certInstallPath, path.path(), true,
- certWatchPtr, *this));
+ // Assume here any regular file located in certificate directory
+ // contains certificates body. Do not want to use soft links
+ // would add value.
+ if (fs::is_regular_file(path))
+ {
+ installedCerts.emplace_back(std::make_unique<Certificate>(
+ bus, certObjectPath + std::to_string(certIdCounter++),
+ certType, certInstallPath, path.path(), certWatchPtr,
+ *this));
+ }
}
catch (const InternalFailure& e)
{
@@ -612,8 +650,8 @@
try
{
installedCerts.emplace_back(std::make_unique<Certificate>(
- bus, certObjectPath + '1', certType, unitToRestart,
- certInstallPath, certInstallPath, true, certWatchPtr, *this));
+ bus, certObjectPath + '1', certType, certInstallPath,
+ certInstallPath, certWatchPtr, *this));
}
catch (const InternalFailure& e)
{
@@ -682,5 +720,81 @@
}
return privateKey;
}
+
+void Manager::storageUpdate()
+{
+ if (certType == phosphor::certs::AUTHORITY)
+ {
+ // Remove symbolic links in the certificate directory
+ for (auto& certPath : fs::directory_iterator(certInstallPath))
+ {
+ try
+ {
+ if (fs::is_symlink(certPath))
+ {
+ fs::remove(certPath);
+ }
+ }
+ catch (const std::exception& e)
+ {
+ log<level::ERR>(
+ "Failed to remove symlink for certificate",
+ entry("ERR=%s", e.what()),
+ entry("SYMLINK=%s", certPath.path().string().c_str()));
+ elog<InternalFailure>();
+ }
+ }
+ }
+
+ for (const auto& cert : installedCerts)
+ {
+ cert->storageUpdate();
+ }
+}
+
+void Manager::reloadOrReset(const UnitsToRestart& unit)
+{
+ if (!unit.empty())
+ {
+ try
+ {
+ constexpr auto SYSTEMD_SERVICE = "org.freedesktop.systemd1";
+ constexpr auto SYSTEMD_OBJ_PATH = "/org/freedesktop/systemd1";
+ constexpr auto SYSTEMD_INTERFACE =
+ "org.freedesktop.systemd1.Manager";
+
+ auto method =
+ bus.new_method_call(SYSTEMD_SERVICE, SYSTEMD_OBJ_PATH,
+ SYSTEMD_INTERFACE, "ReloadOrRestartUnit");
+ method.append(unit, "replace");
+ bus.call_noreply(method);
+ }
+ catch (const sdbusplus::exception::SdBusError& e)
+ {
+ log<level::ERR>("Failed to reload or restart service",
+ entry("ERR=%s", e.what()),
+ entry("UNIT=%s", unit.c_str()));
+ elog<InternalFailure>();
+ }
+ }
+}
+
+bool Manager::isCertificateUnique(const std::string& filePath,
+ const Certificate* const certToDrop)
+{
+ if (std::any_of(
+ installedCerts.begin(), installedCerts.end(),
+ [&filePath, certToDrop](std::unique_ptr<Certificate> const& cert) {
+ return cert.get() != certToDrop && cert->isSame(filePath);
+ }))
+ {
+ return false;
+ }
+ else
+ {
+ return true;
+ }
+}
+
} // namespace certs
} // namespace phosphor
diff --git a/certs_manager.hpp b/certs_manager.hpp
index ed28348..f6d18dd 100644
--- a/certs_manager.hpp
+++ b/certs_manager.hpp
@@ -25,6 +25,8 @@
using EVP_PKEY_Ptr = std::unique_ptr<EVP_PKEY, decltype(&::EVP_PKEY_free)>;
using CertificatePtr = std::unique_ptr<Certificate>;
+using UnitsToRestart = std::string;
+
class Manager : public Ifaces
{
public:
@@ -62,6 +64,8 @@
* (possibly CA signed) Certificate key file.
*
* @param[in] filePath - Certificate key file path.
+ *
+ * @return Certificate object path.
*/
std::string install(const std::string filePath) override;
@@ -70,9 +74,14 @@
*/
void deleteAll() override;
- /** @brief Delete the certificate with given hash.
+ /** @brief Delete the certificate.
*/
- void deleteCertificate(const std::string& certHash);
+ void deleteCertificate(const Certificate* const certificate);
+
+ /** @brief Replace the certificate.
+ */
+ void replaceCertificate(Certificate* const certificate,
+ const std::string& filePath);
/** @brief Generate Private key and CSR file
* Generates the Private key file and CSR file based on the input
@@ -232,12 +241,35 @@
void createRSAPrivateKeyFile();
/** @brief Getting RSA private key
- * Gettting RSA private key from generated file
+ * Getting RSA private key from generated file
* @param[in] keyBitLength - Key bit length
* @return Pointer to RSA key
*/
EVP_PKEY_Ptr getRSAKeyPair(const int64_t keyBitLength);
+ /** @brief Update certificate storage (remove outdated files, recreate
+ * symbolic links, etc.).
+ */
+ void storageUpdate();
+
+ /** @brief Systemd unit reload or reset helper function
+ * Reload if the unit supports it and use a restart otherwise.
+ * @param[in] unit - service need to reload.
+ */
+ void reloadOrReset(const UnitsToRestart& unit);
+
+ /** @brief Check if provided certificate is unique across all certificates
+ * on the internal list.
+ * @param[in] certFilePath - Path to the file with certificate for
+ * uniqueness check.
+ * @param[in] certToDrop - Pointer to the certificate from the internal
+ * list which should be not taken into account while uniqueness check.
+ * @return Checking result. True if certificate is unique, false if
+ * not.
+ */
+ bool isCertificateUnique(const std::string& certFilePath,
+ const Certificate* const certToDrop = nullptr);
+
/** @brief sdbusplus handler */
sdbusplus::bus::bus& bus;
@@ -268,7 +300,7 @@
/** @brief Watch on self signed certificates */
std::unique_ptr<Watch> certWatchPtr = nullptr;
- /** @brif Parent path i.e certificate directory path */
+ /** @brief Parent path i.e certificate directory path */
fs::path certParentInstallPath;
/** @brief Certificate ID pool */
diff --git a/test/certs_manager_test.cpp b/test/certs_manager_test.cpp
index 4a3c6a4..30f0dd1 100644
--- a/test/certs_manager_test.cpp
+++ b/test/certs_manager_test.cpp
@@ -3,6 +3,13 @@
#include "certificate.hpp"
#include "certs_manager.hpp"
+#include <openssl/bio.h>
+#include <openssl/crypto.h>
+#include <openssl/err.h>
+#include <openssl/evp.h>
+#include <openssl/pem.h>
+#include <openssl/x509v3.h>
+
#include <algorithm>
#include <filesystem>
#include <fstream>
@@ -96,6 +103,35 @@
std::istreambuf_iterator<char>(f2.rdbuf()));
}
+ std::string getCertSubjectNameHash(const std::string& certFilePath)
+ {
+ std::unique_ptr<X509, decltype(&::X509_free)> cert(X509_new(),
+ ::X509_free);
+ if (!cert)
+ {
+ std::string();
+ }
+
+ std::unique_ptr<BIO, decltype(&::BIO_free)> bioCert(
+ BIO_new_file(certFilePath.c_str(), "rb"), ::BIO_free);
+ if (!bioCert)
+ {
+ std::string();
+ }
+
+ X509* x509 = cert.get();
+ if (!PEM_read_bio_X509(bioCert.get(), &x509, nullptr, nullptr))
+ {
+ std::string();
+ }
+
+ unsigned long hash = X509_subject_name_hash(cert.get());
+ static constexpr auto AUTH_CERT_HASH_LENGTH = 9;
+ char hashBuf[AUTH_CERT_HASH_LENGTH];
+ sprintf(hashBuf, "%08lx", hash);
+ return std::string(hashBuf);
+ }
+
protected:
sdbusplus::bus::bus bus;
std::string certificateFile, CSRFile, privateKeyFile, rsaPrivateKeyFilePath;
@@ -213,7 +249,8 @@
EXPECT_FALSE(certs.empty());
- std::string verifyPath = verifyDir + "/" + certs[0]->getHash() + ".0";
+ std::string verifyPath =
+ verifyDir + "/" + getCertSubjectNameHash(certificateFile) + ".0";
// Check that certificate has been created at installation directory
EXPECT_FALSE(fs::is_empty(verifyDir));
@@ -223,14 +260,11 @@
EXPECT_TRUE(compareFiles(certificateFile, verifyPath));
}
-/** @brief Check if in athority mode user can install a certificate with certain
- * subject hash once, but cannot install another one with the same hash
+/** @brief Check if in authority mode user can't install the same
+ * certificate twice.
*/
-TEST_F(TestCertificates, InvokeInstallSameSubjectTwice)
+TEST_F(TestCertificates, InvokeInstallSameCertTwice)
{
- using NotAllowed =
- sdbusplus::xyz::openbmc_project::Common::Error::NotAllowed;
-
std::string endpoint("ldap");
std::string unit("");
std::string type("authority");
@@ -250,23 +284,22 @@
EXPECT_FALSE(certs.empty());
- std::string verifyPath = verifyDir + "/" + certs[0]->getHash() + ".0";
-
// Check that certificate has been created at installation directory
+ std::string verifyPath =
+ verifyDir + "/" + getCertSubjectNameHash(certificateFile) + ".0";
EXPECT_FALSE(fs::is_empty(verifyDir));
EXPECT_TRUE(fs::exists(verifyPath));
// Check that installed cert is identical to input one
EXPECT_TRUE(compareFiles(certificateFile, verifyPath));
- // Try to install another one with the same subject
- createNewCertificate(false);
-
+ using NotAllowed =
+ sdbusplus::xyz::openbmc_project::Common::Error::NotAllowed;
EXPECT_THROW(
{
try
{
- // install second certificate
+ // Try to install the same certificate second time
mainApp.install(certificateFile);
}
catch (const NotAllowed& e)
@@ -281,6 +314,141 @@
EXPECT_TRUE(fs::exists(verifyPath));
}
+/** @brief Check if in authority mode user cannot install a certificate with
+ * certain subject hash twice.
+ */
+TEST_F(TestCertificates, InvokeInstallSameSubjectTwice)
+{
+ std::string endpoint("ldap");
+ std::string unit("");
+ std::string type("authority");
+ std::string verifyDir(certDir);
+ UnitsToRestart verifyUnit(unit);
+ auto objPath = std::string(OBJPATH) + '/' + type + '/' + endpoint;
+ auto event = sdeventplus::Event::get_default();
+ // Attach the bus to sd_event to service user requests
+ bus.attach_event(event.get(), SD_EVENT_PRIORITY_NORMAL);
+ Manager manager(bus, event, objPath.c_str(), type, std::move(unit),
+ std::move(certDir));
+ MainApp mainApp(&manager);
+ mainApp.install(certificateFile);
+
+ std::vector<std::unique_ptr<Certificate>>& certs =
+ manager.getCertificates();
+
+ EXPECT_FALSE(certs.empty());
+
+ // Check that certificate has been created at installation directory
+ std::string verifyPath0 =
+ verifyDir + "/" + getCertSubjectNameHash(certificateFile) + ".0";
+ EXPECT_FALSE(fs::is_empty(verifyDir));
+ EXPECT_TRUE(fs::exists(verifyPath0));
+
+ // Check that installed cert is identical to input one
+ EXPECT_TRUE(compareFiles(certificateFile, verifyPath0));
+
+ // Prepare second certificate with the same subject
+ createNewCertificate();
+
+ using NotAllowed =
+ sdbusplus::xyz::openbmc_project::Common::Error::NotAllowed;
+ EXPECT_THROW(
+ {
+ try
+ {
+ // Install second certificate
+ mainApp.install(certificateFile);
+ }
+ catch (const NotAllowed& e)
+ {
+ throw;
+ }
+ },
+ NotAllowed);
+
+ // Expect there are exactly two certificates in the collection
+ EXPECT_EQ(certs.size(), 1);
+
+ // Check that the original/first certificate has been not removed
+ EXPECT_FALSE(fs::is_empty(verifyDir));
+ EXPECT_TRUE(fs::exists(verifyPath0));
+}
+
+/** @brief Check if in authority mode user can't install more than
+ * AUTHORITY_CERTIFICATES_LIMIT certificates.
+ */
+TEST_F(TestCertificates, InvokeInstallAuthCertLimit)
+{
+ std::string endpoint("ldap");
+ std::string unit("");
+ std::string type("authority");
+ std::string verifyDir(certDir);
+ UnitsToRestart verifyUnit(unit);
+ auto objPath = std::string(OBJPATH) + '/' + type + '/' + endpoint;
+ auto event = sdeventplus::Event::get_default();
+ // Attach the bus to sd_event to service user requests
+ bus.attach_event(event.get(), SD_EVENT_PRIORITY_NORMAL);
+ Manager manager(bus, event, objPath.c_str(), type, std::move(unit),
+ std::move(certDir));
+ MainApp mainApp(&manager);
+
+ std::vector<std::unique_ptr<Certificate>>& certs =
+ manager.getCertificates();
+
+ std::vector<std::string> verifyPaths;
+
+ // Prepare maximum number of ceritificates
+ for (std::size_t i = 0; i < AUTHORITY_CERTIFICATES_LIMIT; ++i)
+ {
+ // Prepare new certificatate
+ createNewCertificate(true);
+
+ // Install ceritificate
+ mainApp.install(certificateFile);
+
+ // Check number of certificates in the collection
+ EXPECT_EQ(certs.size(), i + 1);
+
+ // Check that certificate has been created at installation directory
+ std::string verifyPath =
+ verifyDir + "/" + getCertSubjectNameHash(certificateFile) + ".0";
+ EXPECT_FALSE(fs::is_empty(verifyDir));
+ EXPECT_TRUE(fs::exists(verifyPath));
+
+ // Check that installed cert is identical to input one
+ EXPECT_TRUE(compareFiles(certificateFile, verifyPath));
+
+ // Save current certificate file for later check
+ verifyPaths.push_back(verifyPath);
+ }
+
+ // Prepare new certificatate
+ createNewCertificate(true);
+
+ using NotAllowed =
+ sdbusplus::xyz::openbmc_project::Common::Error::NotAllowed;
+ EXPECT_THROW(
+ {
+ try
+ {
+ // Try to install one more certificate
+ mainApp.install(certificateFile);
+ }
+ catch (const NotAllowed& e)
+ {
+ throw;
+ }
+ },
+ NotAllowed);
+
+ // Check that the original certificate has been not removed
+ EXPECT_FALSE(fs::is_empty(verifyDir));
+ for (int i = 0; i < AUTHORITY_CERTIFICATES_LIMIT; ++i)
+ {
+ EXPECT_TRUE(fs::exists(verifyPaths[i]));
+ }
+}
+
/** @brief Compare the installed certificate with the copied certificate
*/
TEST_F(TestCertificates, CompareInstalledCertificate)
@@ -389,7 +557,8 @@
// Certificate successfully installed
EXPECT_FALSE(certs.empty());
- std::string verifyPath = verifyDir + "/" + certs[0]->getHash() + ".0";
+ std::string verifyPath =
+ verifyDir + "/" + getCertSubjectNameHash(certificateFile) + ".0";
// Check that certificate has been created at installation directory
EXPECT_FALSE(fs::is_empty(verifyDir));