Certificate delete API – backend.
Till now the Certificate Manager has one-to-one relation with a
Certificate class. And the DELETE API provided by the
Certificate Manager was enough to delete managed by it certificate.
With introducing Mutual-TLS the relation is changing to one-to-many
and current delete API is not sufficient. This commit adds DELETE
interface to Certificate class, so each of them can be removed
individually. This implementation was done on base of current user
account management implementation. The Certificate class exposes the
delete interface on DBus. When the API is called the Certificate
instance calls proper operation on Certificate Manager which
removes it from its internal collection. The rest of the removing
certificate process, including service reset remains as it was.
Tested with uploaded multiple TLS certificates.
Each Certificate exposes Delete interface on dbus and user is able
to delete each of them. The delete API on Certificate Manager object
was replaced with DeleteAll interface and results in deleting all
loaded certificates.
Signed-off-by: Zbigniew Kurzynski <zbigniew.kurzynski@intel.com>
Change-Id: I9dd6fa998e8bd8081fbd13549831bc94a4a7aa54
diff --git a/certificate.cpp b/certificate.cpp
index a070e07..0682216 100644
--- a/certificate.cpp
+++ b/certificate.cpp
@@ -2,6 +2,8 @@
#include "certificate.hpp"
+#include "certs_manager.hpp"
+
#include <openssl/bio.h>
#include <openssl/crypto.h>
#include <openssl/err.h>
@@ -87,10 +89,10 @@
const CertInstallPath& installPath,
const CertUploadPath& uploadPath,
bool isSkipUnitReload,
- const CertWatchPtr& certWatchPtr) :
+ const CertWatchPtr& certWatchPtr, Manager& parent) :
CertIfaces(bus, objPath.c_str(), true),
bus(bus), objectPath(objPath), certType(type), unitToRestart(unit),
- certInstallPath(installPath), certWatchPtr(certWatchPtr)
+ certInstallPath(installPath), certWatchPtr(certWatchPtr), manager(parent)
{
auto installHelper = [this](const auto& filePath) {
if (!compareKeys(filePath))
@@ -643,5 +645,10 @@
elog<InternalFailure>();
}
}
+
+void Certificate::delete_()
+{
+ manager.deleteCertificate(getHash());
+}
} // namespace certs
} // namespace phosphor
diff --git a/certificate.hpp b/certificate.hpp
index 56256ac..9fa2cdd 100644
--- a/certificate.hpp
+++ b/certificate.hpp
@@ -8,16 +8,18 @@
#include <phosphor-logging/elog.hpp>
#include <xyz/openbmc_project/Certs/Certificate/server.hpp>
#include <xyz/openbmc_project/Certs/Replace/server.hpp>
+#include <xyz/openbmc_project/Object/Delete/server.hpp>
namespace phosphor
{
namespace certs
{
+using DeleteIface = sdbusplus::xyz::openbmc_project::Object::server::Delete;
using CertificateIface = sdbusplus::server::object::object<
sdbusplus::xyz::openbmc_project::Certs::server::Certificate>;
using ReplaceIface = sdbusplus::xyz::openbmc_project::Certs::server::Replace;
-using CertIfaces =
- sdbusplus::server::object::object<CertificateIface, ReplaceIface>;
+using CertIfaces = sdbusplus::server::object::object<CertificateIface,
+ ReplaceIface, DeleteIface>;
using CertificateType = std::string;
using UnitsToRestart = std::string;
@@ -33,6 +35,8 @@
using namespace std::placeholders;
namespace fs = std::filesystem;
+class Manager; // Forward declaration for Certificate Manager.
+
// Supported Types.
static constexpr auto SERVER = "server";
static constexpr auto CLIENT = "client";
@@ -73,7 +77,7 @@
const CertificateType& type, const UnitsToRestart& unit,
const CertInstallPath& installPath,
const CertUploadPath& uploadPath, bool isSkipUnitReload,
- const CertWatchPtr& watchPtr);
+ const CertWatchPtr& watchPtr, Manager& parent);
/** @brief Validate certificate and replace the existing certificate
* @param[in] filePath - Certificate file path.
@@ -92,6 +96,11 @@
*/
const std::string& getHash() const;
+ /**
+ * @brief Delete the certificate
+ */
+ void delete_() override;
+
private:
/**
* @brief Populate certificate properties by parsing given certificate file
@@ -175,6 +184,9 @@
/** @brief Stores certificate subject hash */
std::string certHash;
+
+ /** @brief Reference to Certificate Manager */
+ Manager& manager;
};
} // namespace certs
diff --git a/certs_manager.cpp b/certs_manager.cpp
index e1774b6..a134044 100644
--- a/certs_manager.cpp
+++ b/certs_manager.cpp
@@ -3,6 +3,7 @@
#include <openssl/pem.h>
#include <unistd.h>
+#include <algorithm>
#include <phosphor-logging/elog-errors.hpp>
#include <xyz/openbmc_project/Certs/error.hpp>
#include <xyz/openbmc_project/Common/error.hpp>
@@ -128,11 +129,11 @@
installedCerts.emplace_back(std::make_unique<Certificate>(
bus, certObjectPath, certType, unitToRestart, certInstallPath, filePath,
- false, certWatchPtr));
+ false, certWatchPtr, *this));
return certObjectPath;
}
-void Manager::delete_()
+void Manager::deleteAll()
{
// TODO: #Issue 4 when a certificate is deleted system auto generates
// certificate file. At present we are not supporting creation of
@@ -142,6 +143,25 @@
installedCerts.clear();
}
+void Manager::deleteCertificate(const std::string& certHash)
+{
+ 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;
+ });
+ if (certIt != installedCerts.end())
+ {
+ installedCerts.erase(certIt);
+ }
+ else
+ {
+ log<level::ERR>("Certificate does not exist",
+ entry("HASH=%s", certHash.c_str()));
+ elog<InternalFailure>();
+ }
+}
+
std::string Manager::generateCSR(
std::vector<std::string> alternativeNames, std::string challengePassword,
std::string city, std::string commonName, std::string contactPerson,
@@ -562,7 +582,7 @@
installedCerts.emplace_back(std::make_unique<Certificate>(
bus, certObjectPath + std::to_string(certIdCounter++),
certType, unitToRestart, certInstallPath, path.path(), true,
- certWatchPtr));
+ certWatchPtr, *this));
}
catch (const InternalFailure& e)
{
@@ -581,7 +601,7 @@
{
installedCerts.emplace_back(std::make_unique<Certificate>(
bus, certObjectPath + '1', certType, unitToRestart,
- certInstallPath, certInstallPath, true, certWatchPtr));
+ certInstallPath, certInstallPath, true, certWatchPtr, *this));
}
catch (const InternalFailure& e)
{
diff --git a/certs_manager.hpp b/certs_manager.hpp
index 9dd128d..ed28348 100644
--- a/certs_manager.hpp
+++ b/certs_manager.hpp
@@ -9,16 +9,17 @@
#include <sdeventplus/source/event.hpp>
#include <xyz/openbmc_project/Certs/CSR/Create/server.hpp>
#include <xyz/openbmc_project/Certs/Install/server.hpp>
-#include <xyz/openbmc_project/Object/Delete/server.hpp>
+#include <xyz/openbmc_project/Collection/DeleteAll/server.hpp>
namespace phosphor
{
namespace certs
{
using Install = sdbusplus::xyz::openbmc_project::Certs::server::Install;
-using Delete = sdbusplus::xyz::openbmc_project::Object::server::Delete;
+using DeleteAll =
+ sdbusplus::xyz::openbmc_project::Collection::server::DeleteAll;
using CSRCreate = sdbusplus::xyz::openbmc_project::Certs::CSR::server::Create;
-using Ifaces = sdbusplus::server::object::object<Install, CSRCreate, Delete>;
+using Ifaces = sdbusplus::server::object::object<Install, CSRCreate, DeleteAll>;
using X509_REQ_Ptr = std::unique_ptr<X509_REQ, decltype(&::X509_REQ_free)>;
using EVP_PKEY_Ptr = std::unique_ptr<EVP_PKEY, decltype(&::EVP_PKEY_free)>;
@@ -64,10 +65,14 @@
*/
std::string install(const std::string filePath) override;
- /** @brief Delete the certificate (and possibly revert
- * to a self-signed certificate).
+ /** @brief Implementation for DeleteAll
+ * Delete all objects in the collection.
*/
- void delete_() override;
+ void deleteAll() override;
+
+ /** @brief Delete the certificate with given hash.
+ */
+ void deleteCertificate(const std::string& certHash);
/** @brief Generate Private key and CSR file
* Generates the Private key file and CSR file based on the input
diff --git a/test/certs_manager_test.cpp b/test/certs_manager_test.cpp
index 0db4885..95f7191 100644
--- a/test/certs_manager_test.cpp
+++ b/test/certs_manager_test.cpp
@@ -118,7 +118,7 @@
}
void delete_()
{
- manager->delete_();
+ manager->deleteAll();
}
std::string generateCSR(std::vector<std::string> alternativeNames,
@@ -407,6 +407,57 @@
}
}
+/** @brief Test verifiing if delete function works.
+ */
+TEST_F(TestCertificates, TestStorageDeleteCertificate)
+{
+ 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);
+
+ // Check if certificate placeholder dir is empty
+ EXPECT_TRUE(fs::is_empty(verifyDir));
+ mainApp.install(certificateFile);
+
+ // Create new certificate
+ createNewCertificate(true);
+ mainApp.install(certificateFile);
+
+ createNewCertificate(true);
+ mainApp.install(certificateFile);
+
+ std::vector<std::unique_ptr<Certificate>>& certs =
+ manager.getCertificates();
+
+ // All 3 certificates successfully installed and added to manager
+ EXPECT_EQ(certs.size(), 3);
+
+ // Check if certificate placeholder is not empty, there should be 3
+ // certificates
+ EXPECT_FALSE(fs::is_empty(verifyDir));
+
+ certs[0]->delete_();
+ EXPECT_EQ(certs.size(), 2);
+
+ certs[0]->delete_();
+ EXPECT_EQ(certs.size(), 1);
+
+ certs[0]->delete_();
+ EXPECT_EQ(certs.size(), 0);
+
+ // Check if certificate placeholder is empty.
+ EXPECT_TRUE(fs::is_empty(verifyDir));
+}
+
/** @brief Check if install fails if certificate file is empty
*/
TEST_F(TestCertificates, TestEmptyCertificateFile)