Refactor SecurityClient::update method to support ASMEMBERPVT and…

… `int asl` parameters. Replace in-place string modifications with more secure methods and relocate `PutOperationCache` destructor to header. Include `credentials.cpp` and `securityclient.cpp` in build system. Introduce `ASMember` struct management in `pvacms.cpp` to handle security access settings. Improve error handling and logging related to security configuration.
epics-base · Dec 9, 2024 · 698ebb5 · 698ebb5
1 parent 945b370
commit 698ebb5
Show file tree

Hide file tree

Showing 4 changed files with 56 additions and 22 deletions.
diff --git a/certs/Makefile b/certs/Makefile
@@ -12,6 +12,7 @@ ifeq ($(EVENT2_HAS_OPENSSL),YES)
 USR_CPPFLAGS += -DPVXS_ENABLE_OPENSSL -I$(TOP)/bundle/CLI11/include
 AUTHN = $(TOP)/certs/authn
 SRC_DIRS += $(TOP)/src
+SRC_DIRS += $(TOP)/ioc
 SRCS += p12filefactory.cpp
 SRCS += pemfilefactory.cpp
 SRCS += certfilefactory.cpp
@@ -22,6 +23,7 @@ PROD_LIBS = pvxs Com
 # access to API and private headers
 USR_CPPFLAGS += -I$(TOP)/src/pvxs
 USR_CPPFLAGS += -I$(TOP)/src
+USR_CPPFLAGS += -I$(TOP)/ioc
 
 #INC += certstatus.h
 INC += security.h
@@ -31,6 +33,8 @@ pvacms_SRCS += pvacms.cpp
 pvacms_SRCS += configcms.cpp
 pvacms_SRCS += certstatus.cpp
 pvacms_SRCS += certstatusfactory.cpp
+pvacms_SRCS += credentials.cpp
+pvacms_SRCS += securityclient.cpp
 
 pvacms_LIBS += $(EPICS_BASE_IOC_LIBS)
 pvacms_SYS_LIBS += sqlite3 ssl crypto

diff --git a/certs/pvacms.cpp b/certs/pvacms.cpp
@@ -67,6 +67,8 @@
 #include "ownedptr.h"
 #include "sqlite3.h"
 #include "utilpvt.h"
+#include "securityclient.h"
+#include "credentials.h"
 
 DEFINE_LOGGER(pvacms, "pvxs.certs.cms");
 DEFINE_LOGGER(pvacmsmonitor, "pvxs.certs.stat");
@@ -75,6 +77,22 @@ DEFINE_LOGGER(pvafms, "pvxs.certs.fms");
 namespace pvxs {
 namespace certs {
 
+struct ASMember {
+    std::string name;
+    ASMEMBERPVT mem;
+    ASMember() : ASMember("DEFAULT") {}
+    ASMember(const std::string &n) : name(n) {
+        if (asAddMember(&mem, name.c_str()))
+            throw std::runtime_error(SB() << "Unable to create ASMember " << n);
+        // mem references name.c_str()
+    }
+    ~ASMember() {
+        // all clients must be disconnected...
+        if (asRemoveMember(&mem))
+            log_err_printf(pvacms, "Unable to cleanup ASMember %s\n", name.c_str());
+    }
+};
+
 /**
  * @brief These are the cumulative total days in a year at the start of each month,
  *
@@ -1073,6 +1091,7 @@ void createDefaultAdminClientCert(ConfigCms &config, sql_ptr &ca_db, ossl_ptr<EV
 
     auto cert_file_factory = IdFileFactory::create(config.admin_cert_filename, config.admin_cert_password, key_pair, nullptr, nullptr, "certificate",
                                                    pem_string, !config.admin_private_key_filename.empty());
+    cert_file_factory->writeIdentityFile();
 
     std::string from = std::ctime(&certificate_factory.not_before_);
     std::string to = std::ctime(&certificate_factory.not_after_);
@@ -1760,7 +1779,10 @@ int main(int argc, char *argv[]) {
         // Set security if configured
         if (!config.ca_acf_filename.empty()) {
             log_warn_printf(pvacms, "PVACMS secured with %s\n", config.ca_acf_filename.c_str());
-            asSetFilename(config.ca_acf_filename.c_str());
+            asInitFile(config.ca_acf_filename.c_str(), "");
+        } else {
+            log_err_printf(pvacms, "****EXITING****: PVACMS Access Security Policy File Required%s", "\n");
+            return 1;
         }
 
         // Create the PVs
@@ -1803,20 +1825,25 @@ int main(int argc, char *argv[]) {
             auto state = value["state"].as<std::string>();
             std::transform(state.begin(), state.end(), state.begin(), ::toupper);
 
-/*
             // Get credentials for this operation
             auto creds = op->credentials();
 
+            pvxs::ioc::Credentials credentials(*creds);
 
             // Get security client from channel
-            SecurityClient securityClient = op->channel()->securityClient();
+            pvxs::ioc::SecurityClient securityClient;
+
+            // TODO move somewhere else
+            //      We're using DEFAULT ASG
+            //      ASmember needs to outlive the server (to allow all clients to disconnect)
+            static ASMember as_member;
+            securityClient.update(as_member.mem, ASL1, credentials);
 
             if (!securityClient.canWrite()) {
                 log_err_printf(pvacms, "PVACMS Client Not Authorised%s", "\n");
                 op->error(pvxs::SB() << state << " operation not authorized on " << issuer_id << ":" << serial );
                 return;
             }
-*/
 
             if (state == "REVOKED") {
                 onRevoke(config, ca_db, our_issuer_id, pv, std::move(op), pv_name, parameters, ca_pkey, ca_cert, ca_chain);

diff --git a/ioc/securityclient.cpp b/ioc/securityclient.cpp
@@ -16,27 +16,31 @@
 namespace pvxs {
 namespace ioc {
 
-void SecurityClient::update(dbChannel* ch, Credentials& cred) {
+void SecurityClient::update(ASMEMBERPVT mem, int asl, Credentials& cred) {
     SecurityClient temp;
     temp.cli.resize(cred.cred.size(), nullptr);
 
     for (size_t i = 0, N = temp.cli.size(); i < N; i++) {
         /* asAddClient() fails secure to no-permission */
         (void)asAddClientX(&temp.cli[i],
-                dbChannelRecord(ch)->asp,
-                dbChannelFldDes(ch)->as_level,
-                cred.cred[i].c_str(),
-                // TODO switch to vector of char to accommodate inplace modifications to string
-                const_cast<char*>(cred.method.c_str()),
-                const_cast<char*>(cred.authority.c_str()),
-                const_cast<char*>(cred.host.data()),
-                true // isTLS TODO fix this!!!
-                );
+                           mem,
+                           asl,
+                           cred.cred[i].c_str(),
+          // TODO switch to vector of char to accommodate inplace modifications to string
+                           const_cast<char*>(cred.method.c_str()),
+                           const_cast<char*>(cred.authority.c_str()),
+                           const_cast<char*>(cred.host.data()),
+                           true // isTLS TODO fix this!!!
+        );
     }
 
     cli.swap(temp.cli);
 }
 
+void SecurityClient::update(dbChannel* ch, Credentials& cred) {
+    update(dbChannelRecord(ch)->asp, dbChannelFldDes(ch)->as_level, cred);
+}
+
 SecurityClient::~SecurityClient() {
     for (auto asc: cli) {
         asRemoveClient(&asc);
@@ -48,12 +52,5 @@ bool SecurityClient::canWrite() const {
         return asCheckPut(asc);
     });
 }
-
-PutOperationCache::~PutOperationCache() {
-    // To avoid bug epics-base: unchecked access to notify.chan
-    if (notify.chan) {
-        dbNotifyCancel(&notify);
-    }
-}
 } // pvxs
 } // ioc
diff --git a/ioc/securityclient.h b/ioc/securityclient.h
@@ -27,6 +27,7 @@ class SecurityClient {
 	std::vector<ASCLIENTPVT> cli;
 	~SecurityClient();
 	void update(dbChannel* ch, Credentials& cred);
+	void update(ASMEMBERPVT mem, int asl, Credentials& cred);
 	bool canWrite() const;
 };
 
@@ -68,7 +69,12 @@ struct PutOperationCache : public SingleSecurityCache {
 	Value valueToSet;
 	std::unique_ptr<server::ExecOp> putOperation;
     INST_COUNTER(PutOperationCache);
-	~PutOperationCache();
+    ~PutOperationCache() {
+        // To avoid bug epics-base: unchecked access to notify.chan
+        if (notify.chan) {
+            dbNotifyCancel(&notify);
+        }
+    }
 };
 
 } // pvxs