WIP: ACLs

cts/cli/regression.acls.exp

@@ -537,7 +537,7 @@ crm_attribute: Error performing operation: Permission denied
 * Passed: crm_attribute         - unknownguy: Set fencing-enabled
 =#=#=#= Begin test: unknownguy: Create a resource =#=#=#=
 pcmk__check_acl 	trace: Lack of ACL denies user 'unknownguy' read/write access to /cib/configuration/resources/primitive[@id='dummy']
-pcmk__apply_creation_acl 	trace: ACLs disallow creation of <primitive> with id="dummy"
+check_creation_disallowed 	trace: ACLs disallow creation of <primitive> with id="dummy"
 cibadmin: CIB API call failed: Permission denied
 =#=#=#= End test: unknownguy: Create a resource - Insufficient privileges (4) =#=#=#=
 * Passed: cibadmin              - unknownguy: Create a resource
@@ -555,7 +555,7 @@ crm_attribute: Error performing operation: Permission denied
 * Passed: crm_attribute         - l33t-haxor: Set fencing-enabled
 =#=#=#= Begin test: l33t-haxor: Create a resource =#=#=#=
 pcmk__check_acl 	trace: Parent ACL denies user 'l33t-haxor' read/write access to /cib/configuration/resources/primitive[@id='dummy']
-pcmk__apply_creation_acl 	trace: ACLs disallow creation of <primitive> with id="dummy"
+check_creation_disallowed 	trace: ACLs disallow creation of <primitive> with id="dummy"
 cibadmin: CIB API call failed: Permission denied
 =#=#=#= End test: l33t-haxor: Create a resource - Insufficient privileges (4) =#=#=#=
 * Passed: cibadmin              - l33t-haxor: Create a resource
@@ -639,7 +639,7 @@ crm_attribute: Error performing operation: Permission denied
 =#=#=#= End test: niceguy: Set enable-acl - Insufficient privileges (4) =#=#=#=
 * Passed: crm_attribute         - niceguy: Set enable-acl
 =#=#=#= Begin test: niceguy: Set fencing-enabled =#=#=#=
-pcmk__apply_creation_acl 	trace: ACLs allow creation of <nvpair> with id="cib-bootstrap-options-fencing-enabled"
+check_creation_disallowed 	trace: ACLs allow creation of <nvpair> with id="cib-bootstrap-options-fencing-enabled"
 =#=#=#= Current cib after: niceguy: Set fencing-enabled =#=#=#=
 <cib admin_epoch="0" epoch="10" num_updates="0">
   <configuration>
@@ -716,7 +716,7 @@ pcmk__apply_creation_acl 	trace: ACLs allow creation of <nvpair> with id="cib-bo
 * Passed: crm_attribute         - niceguy: Set fencing-enabled
 =#=#=#= Begin test: niceguy: Create a resource =#=#=#=
 pcmk__check_acl 	trace: Default ACL denies user 'niceguy' read/write access to /cib/configuration/resources/primitive[@id='dummy']
-pcmk__apply_creation_acl 	trace: ACLs disallow creation of <primitive> with id="dummy"
+check_creation_disallowed 	trace: ACLs disallow creation of <primitive> with id="dummy"
 cibadmin: CIB API call failed: Permission denied
 =#=#=#= End test: niceguy: Create a resource - Insufficient privileges (4) =#=#=#=
 * Passed: cibadmin              - niceguy: Create a resource
@@ -1041,8 +1041,8 @@ crm_resource: Error performing operation: Insufficient privileges
 * Passed: crm_resource          - l33t-haxor: Remove a resource meta attribute
 =#=#=#= Begin test: niceguy: Create a resource meta attribute =#=#=#=
 unpack_resources 	error: Resource start-up disabled since no fencing resources have been defined. Either configure some or disable fencing with the fencing-enabled option. NOTE: Clusters with shared data need fencing to ensure data integrity.
-pcmk__apply_creation_acl 	trace: Creation of <meta_attributes> scaffolding with id="dummy-meta_attributes" is implicitly allowed
-pcmk__apply_creation_acl 	trace: ACLs allow creation of <nvpair> with id="dummy-meta_attributes-target-role"
+check_creation_disallowed 	trace: Creation of <meta_attributes> scaffolding with id="dummy-meta_attributes" is implicitly allowed
+check_creation_disallowed 	trace: ACLs allow creation of <nvpair> with id="dummy-meta_attributes-target-role"
 Set 'dummy' option: id=dummy-meta_attributes-target-role set=dummy-meta_attributes name=target-role value=Stopped
 =#=#=#= Current cib after: niceguy: Create a resource meta attribute =#=#=#=
 <cib admin_epoch="0" epoch="14" num_updates="0">
@@ -1293,7 +1293,7 @@ Deleted 'dummy' option: id=dummy-meta_attributes-target-role name=target-role
 * Passed: crm_resource          - niceguy: Remove a resource meta attribute
 =#=#=#= Begin test: niceguy: Create a resource meta attribute =#=#=#=
 unpack_resources 	error: Resource start-up disabled since no fencing resources have been defined. Either configure some or disable fencing with the fencing-enabled option. NOTE: Clusters with shared data need fencing to ensure data integrity.
-pcmk__apply_creation_acl 	trace: ACLs allow creation of <nvpair> with id="dummy-meta_attributes-target-role"
+check_creation_disallowed 	trace: ACLs allow creation of <nvpair> with id="dummy-meta_attributes-target-role"
 Set 'dummy' option: id=dummy-meta_attributes-target-role set=dummy-meta_attributes name=target-role value=Started
 =#=#=#= Current cib after: niceguy: Create a resource meta attribute =#=#=#=
 <cib admin_epoch="0" epoch="16" num_updates="0">
@@ -1514,7 +1514,7 @@ cibadmin: CIB API call failed: Permission denied
 =#=#=#= Begin test: niceguy: Replace - create resource =#=#=#=
 pcmk__check_acl 	trace: Default ACL denies user 'niceguy' read/write access to /cib[@epoch]
 pcmk__check_acl 	trace: Default ACL denies user 'niceguy' read/write access to /cib/configuration/resources/primitive[@id='dummy2']
-pcmk__apply_creation_acl 	trace: ACLs disallow creation of <primitive> with id="dummy2"
+check_creation_disallowed 	trace: ACLs disallow creation of <primitive> with id="dummy2"
 cibadmin: CIB API call failed: Permission denied
 =#=#=#= End test: niceguy: Replace - create resource - Insufficient privileges (4) =#=#=#=
 * Passed: cibadmin              - niceguy: Replace - create resource
@@ -2546,7 +2546,7 @@ cibadmin: CIB API call failed: Permission denied
   <status/>
 </cib>
 =#=#=#= Begin test: mike: Create another resource =#=#=#=
-pcmk__apply_creation_acl 	trace: ACLs allow creation of <primitive> with id="dummy2"
+check_creation_disallowed 	trace: ACLs allow creation of <primitive> with id="dummy2"
 =#=#=#= Current cib after: mike: Create another resource =#=#=#=
 <cib admin_epoch="0" epoch="26" num_updates="0">
   <configuration>

cts/cts-cli.in

@@ -249,6 +249,7 @@ def sanitize_output(s):
         (r'(<change-attr name="crm_feature_set" .* value=")[0-9.]*"', r'\1"'),
         (r'(<change-attr name="validate-with" .* value="pacemaker-)[0-9.]+"', r'\1X"'),
         (r'(<cib.*) cib-last-written="[^"]*"', r'\1'),
+        (r'\((check_creation_disallowed.*)@.*\.c:[0-9]+\)', r'\1'),
         (r'crm_feature_set="[^"]*" ', r''),
         (r'@crm_feature_set=[0-9.]+, ', r''),
         (r'\(crm_time_parse_duration@.*\.c:[0-9]+\)', r'crm_time_parse_duration'),
@@ -2942,7 +2943,7 @@ class AclsRegressionTest(RegressionTest):
         return [
             ShadowTestGroup(basic_tests + [
                             TestGroup(loop_tests,
-                                      env={"PCMK_trace_functions": "pcmk__check_acl,pcmk__apply_creation_acl"})]),
+                                      env={"PCMK_trace_functions": "pcmk__check_acl,check_creation_disallowed"})]),
         ]
 
 

daemons/attrd/attrd_messages.c

@@ -24,9 +24,10 @@ int minimum_protocol_version = -1;
 static GHashTable *attrd_handlers = NULL;
 
 static bool
-is_sync_point_attr(xmlAttrPtr attr, void *data)
+is_sync_point_attr(const xmlAttr *attr, void *data)
 {
-    return pcmk__str_eq((const char *) attr->name, PCMK__XA_ATTR_SYNC_POINT, pcmk__str_none);
+    return pcmk__str_eq((const char *) attr->name, PCMK__XA_ATTR_SYNC_POINT,
+                        pcmk__str_none);
 }
 
 static int

daemons/controld/controld_join_dc.c

@@ -962,11 +962,9 @@ finalize_join_for(gpointer key, gpointer value, gpointer user_data)
                 }
 
                 remote = pcmk__xe_create(remotes, PCMK_XE_NODE);
-                pcmk__xe_set_props(remote,
-                                   PCMK_XA_ID, node->name,
-                                   PCMK__XA_NODE_STATE, node->state,
-                                   PCMK__XA_CONNECTION_HOST, node->conn_host,
-                                   NULL);
+                pcmk__xe_set(remote, PCMK_XA_ID, node->name);
+                pcmk__xe_set(remote, PCMK__XA_NODE_STATE, node->state);
+                pcmk__xe_set(remote, PCMK__XA_CONNECTION_HOST, node->conn_host);
             }
         }
     }

daemons/fenced/fenced_commands.c

@@ -153,6 +153,8 @@ fenced_has_watchdog_device(void)
 void
 fenced_foreach_device(GHFunc fn, gpointer user_data)
 {
+    pcmk__assert(fn != NULL);
+
     if (device_table == NULL) {
         return;
     }
@@ -170,6 +172,8 @@ fenced_foreach_device(GHFunc fn, gpointer user_data)
 void
 fenced_foreach_device_remove(GHRFunc fn)
 {
+    pcmk__assert(fn != NULL);
+
     if (device_table == NULL) {
         return;
     }

daemons/pacemakerd/pacemakerd.c

@@ -67,12 +67,12 @@ PCMK__OUTPUT_ARGS("features")
 static int
 pacemakerd_features_xml(pcmk__output_t *out, va_list args) {
     gchar **feature_list = g_strsplit(CRM_FEATURES, " ", 0);
+    xmlNode *xml = pcmk__output_xml_create_parent(out, PCMK_XE_PACEMAKERD);
+
+    pcmk__xe_set(xml, PCMK_XA_VERSION, PACEMAKER_VERSION);
+    pcmk__xe_set(xml, PCMK_XA_BUILD, BUILD_VERSION);
+    pcmk__xe_set(xml, PCMK_XA_FEATURE_SET, CRM_FEATURE_SET);
 
-    pcmk__output_xml_create_parent(out, PCMK_XE_PACEMAKERD,
-                                   PCMK_XA_VERSION, PACEMAKER_VERSION,
-                                   PCMK_XA_BUILD, BUILD_VERSION,
-                                   PCMK_XA_FEATURE_SET, CRM_FEATURE_SET,
-                                   NULL);
     out->begin_list(out, NULL, NULL, PCMK_XE_FEATURES);
 
     for (char **s = feature_list; *s != NULL; s++) {

devel/Makefile.am

@@ -70,12 +70,15 @@ coverity: $(COVTAR)
 # of them are designed so that things execute in the proper order (which is
 # not the same as GNU make's order-only prerequisites).
 
+# @COMPAT Prior to GLib 2.58, the implementation of g_clear_pointer()
+# triggers the INCONSISTENT_UNION_ACCESS warning
 .PHONY: coverity-analyze
 coverity-analyze: $(COVERITY_DIR)
 	@echo ""
 	@echo "Analyzing (waiting for coverity license if necessary) ..."
 	cd $(top_builddir) && cov-analyze --dir "$<" --wait-for-license	\
-		--security --aggressiveness-level "$(COVLEVEL)"
+		--security --aggressiveness-level "$(COVLEVEL)"		\
+		--disable INCONSISTENT_UNION_ACCESS
 
 .PHONY: $(COVEMACS)
 $(COVEMACS): coverity-analyze

include/crm/common/acl.h

@@ -24,11 +24,6 @@ extern "C" {
  */
 
 void xml_acl_disable(xmlNode *xml);
-bool xml_acl_denied(const xmlNode *xml);
-bool xml_acl_filtered_copy(const char *user, xmlNode* acl_source, xmlNode *xml,
-                           xmlNode **result);
-
-bool pcmk_acl_required(const char *user);
 
 #ifdef __cplusplus
 }

include/crm/common/acl_compat.h

@@ -30,6 +30,16 @@ extern "C" {
 //! \deprecated Do not use
 bool xml_acl_enabled(const xmlNode *xml);
 
+//! \deprecated Do not use
+bool xml_acl_filtered_copy(const char *user, xmlNode *acl_source, xmlNode *xml,
+                           xmlNode **result);
+
+//! \deprecated Do not use
+bool xml_acl_denied(const xmlNode *xml);
+
+//! \deprecated Do not use
+bool pcmk_acl_required(const char *user);
+
 #ifdef __cplusplus
 }
 #endif

include/crm/common/acl_internal.h

@@ -36,8 +36,26 @@ pcmk__is_privileged(const char *user)
     return user && (!strcmp(user, CRM_DAEMON_USER) || !strcmp(user, "root"));
 }
 
+/*!
+ * \internal
+ * \brief Check whether an ACL is required for a given user to access the CIB
+ *
+ * \param[in] user  User name
+ *
+ * \return \c true if \p user requires an ACL to access the CIB, or \c false
+ *         otherwise
+ */
+static inline bool
+pcmk__acl_required(const char *user)
+{
+    return !pcmk__str_empty(user) && !pcmk__is_privileged(user);
+}
+
 void pcmk__enable_acls(xmlDoc *source, xmlDoc *target, const char *user);
 
+xmlNode *pcmk__acl_filtered_copy(const char *user, xmlDoc *acl_source,
+                                 xmlNode *xml);
+
 bool pcmk__check_acl(xmlNode *xml, const char *attr_name,
                      enum pcmk__xml_flags mode);
 

include/crm/common/internal.h

@@ -60,6 +60,7 @@
 #include <crm/common/servers_internal.h>
 #include <crm/common/tls_internal.h>
 #include <crm/common/utils_internal.h>
+// xml_attr_internal.h intentionally left out
 // xml_comment_internal.h intentionally left out
 // xml_element_internal.h intentionally left out
 // xml_idref_internal.h intentionally left out

include/crm/common/output_internal.h

@@ -757,11 +757,9 @@ void pcmk__output_set_log_filter(pcmk__output_t *out, const char *file,
  *
  * \param[in,out] out  The output functions structure.
  * \param[in]     name The name of the node to be created.
- * \param[in]     ...     Name/value pairs to set as XML properties.
  */
-xmlNodePtr
-pcmk__output_xml_create_parent(pcmk__output_t *out, const char *name, ...)
-G_GNUC_NULL_TERMINATED;
+xmlNode *
+pcmk__output_xml_create_parent(pcmk__output_t *out, const char *name);
 
 /*!
  * \internal
@@ -781,11 +779,9 @@ pcmk__output_xml_add_node_copy(pcmk__output_t *out, xmlNodePtr node);
  *
  * \param[in,out] out  The output functions structure.
  * \param[in]     name The name of the node to be created.
- * \param[in]     ...     Name/value pairs to set as XML properties.
  */
-xmlNodePtr
-pcmk__output_create_xml_node(pcmk__output_t *out, const char *name, ...)
-G_GNUC_NULL_TERMINATED;
+xmlNode *
+pcmk__output_create_xml_node(pcmk__output_t *out, const char *name);
 
 /*!
  * \internal

include/crm/common/xml_attr_internal.h

@@ -0,0 +1,32 @@
+/*
+ * Copyright 2025 the Pacemaker project contributors
+ *
+ * The version control history for this file may have further details.
+ *
+ * This source code is licensed under the GNU Lesser General Public License
+ * version 2.1 or later (LGPLv2.1+) WITHOUT ANY WARRANTY.
+ */
+
+#ifndef PCMK__CRM_COMMON_XML_ATTR_INTERNAL__H
+#define PCMK__CRM_COMMON_XML_ATTR_INTERNAL__H
+
+/*
+ * Internal-only wrappers for and extensions to libxml2 for processing XML
+ * attributes
+ */
+
+#include <stdbool.h>        // bool
+
+#include <libxml/tree.h>    // xmlAttr
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+bool pcmk__xa_insert_dup(const xmlAttr *attr, void *user_data);
+
+#ifdef __cplusplus
+}
+#endif
+
+#endif  // PCMK__XML_ATTR_INTERNAL__H

include/crm/common/xml_element_internal.h

@@ -37,13 +37,19 @@ extern "C" {
 
 const char *pcmk__xe_add_last_written(xmlNode *xe);
 
+bool pcmk__xe_foreach_attr(xmlNode *xml, bool (*fn)(xmlAttr *, void *),
+                           void *user_data);
+bool pcmk__xe_foreach_const_attr(const xmlNode *xml,
+                                 bool (*fn)(const xmlAttr *, void *),
+                                 void *user_data);
+
 xmlNode *pcmk__xe_first_child(const xmlNode *parent, const char *node_name,
                               const char *attr_n, const char *attr_v);
 
 void pcmk__xe_remove_attr(xmlNode *element, const char *name);
 bool pcmk__xe_remove_attr_cb(xmlNode *xml, void *user_data);
 void pcmk__xe_remove_matching_attrs(xmlNode *element, bool force,
-                                    bool (*match)(xmlAttrPtr, void *),
+                                    bool (*match)(const xmlAttr *, void *),
                                     void *user_data);
 int pcmk__xe_delete_match(xmlNode *xml, xmlNode *search);
 int pcmk__xe_replace_match(xmlNode *xml, xmlNode *replace);
@@ -80,31 +86,6 @@ void pcmk__xe_sort_attrs(xmlNode *xml);
 void pcmk__xe_set_id(xmlNode *xml, const char *format, ...)
     G_GNUC_PRINTF(2, 3);
 
-/*!
- * \internal
- * \brief Like pcmk__xe_set_props, but takes a va_list instead of
- *        arguments directly.
- *
- * \param[in,out] node   XML to add attributes to
- * \param[in]     pairs  NULL-terminated list of name/value pairs to add
- */
-void
-pcmk__xe_set_propv(xmlNodePtr node, va_list pairs);
-
-/*!
- * \internal
- * \brief Add a NULL-terminated list of name/value pairs to the given
- *        XML node as properties.
- *
- * \param[in,out] node XML node to add properties to
- * \param[in]     ...  NULL-terminated list of name/value pairs
- *
- * \note A NULL name terminates the arguments; a NULL value will be skipped.
- */
-void
-pcmk__xe_set_props(xmlNodePtr node, ...)
-G_GNUC_NULL_TERMINATED;
-
 /*!
  * \internal
  * \brief Get first attribute of an XML element