Refactor certificate management: integrate CLI11 library and enhance …

…root certificate handling - Introduced CLI11 library as a submodule for improved command-line parsing. - Integrated the CLI11 library into the certificate management utility, replacing the legacy getopt-based approach with a modern API. - Added functionality to download and install root certificates. - Removed outdated references to certificate rotation and revocation in the codebase. - Renamed variables to follow consistent naming conventions (`p12PemString` to `p12_pem_string`). - Enhanced logging and error handling for better clarity and troubleshooting. - Adjusted Makefile to include the new CLI11 library's headers. - Streamlined code to improve readability and maintainability.
epics-base · Dec 6, 2024 · 27a54d8 · 27a54d8
1 parent 6ee52eb
commit 27a54d8
Show file tree

Hide file tree

Showing 11 changed files with 199 additions and 116 deletions.
diff --git a/.gitmodules b/.gitmodules
@@ -7,3 +7,6 @@
 [submodule "bundle/jwt-cpp"]
 	path = bundle/jwt-cpp
 	url = https://github.com/Thalhammer/jwt-cpp.git
+[submodule "bundle/CLI11"]
+	path = bundle/CLI11
+	url = https://github.com/CLIUtils/CLI11.git
diff --git a/bundle/CLI11 b/bundle/CLI11
diff --git a/certs/authn/auth.cpp b/certs/authn/auth.cpp
@@ -74,8 +74,8 @@ std::shared_ptr<CertCreationRequest> Auth::createCertCreationRequest(const std::
  */
 std::string Auth::processCertificateCreationRequest(const std::shared_ptr<CertCreationRequest> &cert_creation_request) const {
     // Forward the ccr to the certificate management service
-    std::string p12PemString(ccr_manager_.createCertificate(cert_creation_request));
-    return p12PemString;
+    std::string p12_pem_string(ccr_manager_.createCertificate(cert_creation_request));
+    return p12_pem_string;
 }
 
 std::shared_ptr<KeyPair> Auth::createKeyPair(const ConfigCommon &config) {

diff --git a/certs/authn/ccrmanager.cpp b/certs/authn/ccrmanager.cpp
@@ -15,7 +15,6 @@ namespace certs {
 using namespace members;
 
 std::string CCRManager::createCertificate(const std::shared_ptr<CertCreationRequest> &cert_creation_request) const {
-    std::string p12PemString;
     auto uri = nt::NTURI({}).build();
     uri += {Struct("query", CCR_PROTOTYPE(cert_creation_request->verifier_fields))};
     auto arg = uri.create();

diff --git a/certs/authn/std/authnstd.cpp b/certs/authn/std/authnstd.cpp
@@ -230,16 +230,16 @@ int main(int argc, char *argv[]) {
 
             // Attempt to create a certificate with the certificate creation
             // request
-            auto p12PemString = authenticator.processCertificateCreationRequest(cert_creation_request);
+            auto p12_pem_string = authenticator.processCertificateCreationRequest(cert_creation_request);
 
             // If the certificate was created successfully,
-            if (!p12PemString.empty()) {
-                log_debug_printf(auths, "Cert generated by PVACMS and successfully received: %s\n", p12PemString.c_str());
+            if (!p12_pem_string.empty()) {
+                log_debug_printf(auths, "Cert generated by PVACMS and successfully received: %s\n", p12_pem_string.c_str());
 
                 // Attempt to write the certificate and private key
                 // to a cert file protected by the configured password
                 auto file_factory =
-                    CertFileFactory::create(config.tls_cert_filename, config.tls_cert_password, key_pair, nullptr, nullptr, "certificate", p12PemString);
+                    CertFileFactory::create(config.tls_cert_filename, config.tls_cert_password, key_pair, nullptr, nullptr, "certificate", p12_pem_string);
                 file_factory->writeCertFile();
 
                 log_info_printf(auths, "New Cert File created using %s: %s\n", METHOD_STRING(authenticator.type_).c_str(), config.tls_cert_filename.c_str());
@@ -248,7 +248,7 @@ int main(int argc, char *argv[]) {
 
                 // Create the root certificate if it is not already there so
                 // that the user can trust it
-                if (file_factory->writeRootPemFile(p12PemString)) {
+                if (file_factory->writeRootPemFile(p12_pem_string)) {
                     return CertAvailability::OK;
                 } else {
                     return CertAvailability::ROOT_CERT_INSTALLED;

diff --git a/certs/certstatusfactory.h b/certs/certstatusfactory.h
@@ -104,7 +104,9 @@ class CertStatusFactory {
         return uint64_number;
     }
 
-   private:
+    static uint64_t getSerialNumber(const ossl_ptr<X509>& cert);
+
+  private:
     const ossl_ptr<X509>& ca_cert_;                          // CA Certificate to encode in the OCSP responses
     const pvxs::ossl_ptr<EVP_PKEY>& ca_pkey_;                // CA Certificate's private key to sign the OCSP responses
     const pvxs::ossl_shared_ptr<STACK_OF(X509)>& ca_chain_;  // CA Certificate chain to encode in the OCSP responses
@@ -123,7 +125,7 @@ class CertStatusFactory {
      * @return a byte array
      */
     static std::vector<uint8_t> ocspResponseToBytes(const pvxs::ossl_ptr<OCSP_BASICRESP>& basic_resp);
-    static uint64_t getSerialNumber(const ossl_ptr<X509>& cert);
+
     static uint64_t getSerialNumber(X509* cert);
 
     /**

diff --git a/certs/pemfilefactory.cpp b/certs/pemfilefactory.cpp
@@ -22,10 +22,10 @@ DEFINE_LOGGER(pemcerts, "pvxs.certs.pem");
  * @return true if the file already exists, false otherwise
  * @throw std::runtime_error if the file cannot be written
  */
-bool PEMFileFactory::createRootPemFile(const std::string& p12PemString, bool overwrite) {
+bool PEMFileFactory::createRootPemFile(const std::string& p12_pem_string, bool overwrite) {
     static constexpr auto kMaxAuthnNameLen = 256;
 
-    ossl_ptr<BIO> bio(BIO_new_mem_buf(p12PemString.data(), p12PemString.size()));
+    ossl_ptr<BIO> bio(BIO_new_mem_buf(p12_pem_string.data(), p12_pem_string.size()));
 
     // Create a stack for the certs
     STACK_OF(X509_INFO) * inf(PEM_X509_INFO_read_bio(bio.get(), NULL, NULL, NULL));

diff --git a/certs/pvacms.cpp b/certs/pvacms.cpp
@@ -84,7 +84,7 @@ namespace certs {
  *
  * This array does not consider leap years
  */
-static const std::string kCertRevokePrefix("CERT:REVOKE");
+static const std::string kCertRoot("CERT:ROOT");
 
 // The current partition number
 uint16_t partition_number = 0;
@@ -112,8 +112,6 @@ Value getCreatePrototype() {
                              Member(TypeCode::String, "issuer"),
                              Member(TypeCode::String, "certid"),
                              Member(TypeCode::String, "statuspv"),
-                             Member(TypeCode::String, "revokepv"),
-                             Member(TypeCode::String, "rotatepv"),
                              Member(TypeCode::String, "cert"),
                              Struct("alarm", "alarm_t",
                                     {
@@ -128,6 +126,60 @@ Value getCreatePrototype() {
     return value;
 }
 
+/**
+ * @brief  The value for a GET root certificate operation
+ * @return  The value for a GET root certificate operation
+ */
+Value getRootValue(const std::string &issuer_id, const ossl_ptr<X509> &ca_cert, const ossl_shared_ptr<STACK_OF(X509)> &ca_chain) {
+    using namespace members;
+    auto value = TypeDef(TypeCode::Struct,
+                         {
+                             Member(TypeCode::UInt64, "serial"),
+                             Member(TypeCode::String, "issuer"),
+                             Member(TypeCode::String, "name"),
+                             Member(TypeCode::String, "org"),
+                             Member(TypeCode::String, "org_unit"),
+                             Member(TypeCode::String, "cert"),
+                             Struct("alarm", "alarm_t",
+                                    {
+                                        Int32("severity"),
+                                        Int32("status"),
+                                        String("message"),
+                                    }),
+                         })
+                     .create();
+    auto subject_name(X509_get_subject_name(ca_cert.get()));
+    auto subject_string(X509_NAME_oneline(subject_name, nullptr, 0));
+    ossl_ptr<char> owned_subject(subject_string, false);
+    if (!owned_subject) {
+        throw std::runtime_error("Unable to get the subject of the CA certificate");
+    }
+    std::string subject(owned_subject.get());
+
+    // Subject part extractor
+    auto extractSubjectPart = [&subject](const std::string& key) -> std::string {
+        std::size_t start = subject.find("/" + key + "=");
+        if (start == std::string::npos) {
+            throw std::runtime_error("Key not found: " + key);
+        }
+        start += key.size() + 2;  // Skip over "/key="
+        std::size_t end = subject.find("/", start);  // Find the end of the current value
+        if (end == std::string::npos) {
+            end = subject.size();
+        }
+        return subject.substr(start, end - start);
+    };
+
+    value["serial"] = pvxs::certs::CertStatusFactory::getSerialNumber(ca_cert);
+    value["issuer"] = issuer_id;
+    value["name"] = extractSubjectPart("CN");
+    value["org"] = extractSubjectPart("O");
+    value["org_unit"] = extractSubjectPart("OU");
+    value["cert"] = CertFactory::certAndCasToPemString(ca_cert, ca_chain.get());
+
+    return value;
+}
+
 /**
  * @brief Reads command line options and sets corresponding variables.
  *
@@ -735,8 +787,6 @@ void onCreateCertificate(ConfigCms &config, sql_ptr &ca_db, const server::Shared
         // Construct and return the reply
         auto cert_id = getCertId(issuer_id, serial);
         auto status_pv = getCertUri(GET_MONITOR_CERT_STATUS_ROOT, cert_id);
-        auto revoke_pv = getCertUri(kCertRevokePrefix, cert_id);
-        auto rotate_pv = RPC_CERT_ROTATE_PV;
         auto reply(getCreatePrototype());
         auto now(time(nullptr));
         reply["status.value.index"] = state;
@@ -746,8 +796,6 @@ void onCreateCertificate(ConfigCms &config, sql_ptr &ca_db, const server::Shared
         reply["issuer"] = issuer_id;
         reply["certid"] = cert_id;
         reply["statuspv"] = status_pv;
-        reply["revokepv"] = revoke_pv;
-        reply["rotatepv"] = rotate_pv;
         reply["cert"] = pem_string;
         op->reply(reply);
     } catch (std::exception &e) {
@@ -1661,8 +1709,13 @@ int main(int argc, char *argv[]) {
 
         // Create the PVs
         SharedPV create_pv(SharedPV::buildReadonly());
+        SharedPV root_pv(SharedPV::buildReadonly());
         SharedWildcardPV status_pv(SharedWildcardPV::buildMailbox());
 
+        // Create Root PV value which won't change
+        // TODO what happens when it changes
+        pvxs::Value root_pv_value = getRootValue(our_issuer_id, ca_cert, ca_chain);
+
         // RPC handlers
         pvxs::ossl_ptr<EVP_PKEY> ca_pub_key(X509_get_pubkey(ca_cert.get()));
         create_pv.onRPC(
@@ -1711,7 +1764,8 @@ int main(int argc, char *argv[]) {
         // Return true to indicate that we want the file monitor time to run after this
         Server pva_server = Server(config, [&status_monitor_params](short evt) { return statusMonitor(status_monitor_params); });
 
-        pva_server.addPV(RPC_CERT_CREATE, create_pv).addPV(GET_MONITOR_CERT_STATUS_PV, status_pv);
+        pva_server.addPV(RPC_CERT_CREATE, create_pv).addPV(GET_MONITOR_CERT_STATUS_PV, status_pv).addPV(kCertRoot, root_pv);
+        root_pv.open(root_pv_value);
 
         if (verbose) {
             std::cout << "Effective config\n" << config;

diff --git a/certs/pvacms.h b/certs/pvacms.h
@@ -31,8 +31,6 @@
 #include "ownedptr.h"
 
 #define RPC_CERT_CREATE "CERT:CREATE"
-#define RPC_CERT_ROTATE_PV "CERT:ROTATE"
-#define RPC_CERT_REVOKE_PV "CERT:REVOKE:????????:*"
 
 #define DEFAULT_KEYCHAIN_FILE "server.p12"
 #define DEFAULT_CA_KEYCHAIN_FILE "ca.p12"

diff --git a/tools/Makefile b/tools/Makefile
@@ -15,7 +15,7 @@ ifeq ($(EVENT2_HAS_OPENSSL),YES)
 SRC_DIRS += $(TOP)/certs
 SRC_DIRS += $(TOP)/src
 USR_CPPFLAGS += -DPVXS_ENABLE_OPENSSL
-USR_CPPFLAGS += -I$(TOP)/certs
+USR_CPPFLAGS += -I$(TOP)/certs -I$(TOP)/bundle/CLI11/include
 
 PROD += pvxcert
 pvxcert_SRCS += cert.cpp