diff --git a/cts/cli/regression.acls.exp b/cts/cli/regression.acls.exp index 353edeb53c3..521e2d52204 100644 --- a/cts/cli/regression.acls.exp +++ b/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 with id="dummy" +check_creation_disallowed trace: ACLs disallow creation of 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 with id="dummy" +check_creation_disallowed trace: ACLs disallow creation of 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 with id="cib-bootstrap-options-fencing-enabled" +check_creation_disallowed trace: ACLs allow creation of with id="cib-bootstrap-options-fencing-enabled" =#=#=#= Current cib after: niceguy: Set fencing-enabled =#=#=#= @@ -716,7 +716,7 @@ pcmk__apply_creation_acl trace: ACLs allow creation of 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 with id="dummy" +check_creation_disallowed trace: ACLs disallow creation of 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 scaffolding with id="dummy-meta_attributes" is implicitly allowed -pcmk__apply_creation_acl trace: ACLs allow creation of with id="dummy-meta_attributes-target-role" +check_creation_disallowed trace: Creation of scaffolding with id="dummy-meta_attributes" is implicitly allowed +check_creation_disallowed trace: ACLs allow creation of 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 =#=#=#= @@ -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 with id="dummy-meta_attributes-target-role" +check_creation_disallowed trace: ACLs allow creation of 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 =#=#=#= @@ -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 with id="dummy2" +check_creation_disallowed trace: ACLs disallow creation of 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 =#=#=#= Begin test: mike: Create another resource =#=#=#= -pcmk__apply_creation_acl trace: ACLs allow creation of with id="dummy2" +check_creation_disallowed trace: ACLs allow creation of with id="dummy2" =#=#=#= Current cib after: mike: Create another resource =#=#=#= diff --git a/cts/cts-cli.in b/cts/cts-cli.in index 8f557ce4b45..84e3b8ae90f 100644 --- a/cts/cts-cli.in +++ b/cts/cts-cli.in @@ -249,6 +249,7 @@ def sanitize_output(s): (r'(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 diff --git a/include/crm/common/acl.h b/include/crm/common/acl.h index bff52f63417..3b273e2dddb 100644 --- a/include/crm/common/acl.h +++ b/include/crm/common/acl.h @@ -25,8 +25,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); diff --git a/include/crm/common/acl_compat.h b/include/crm/common/acl_compat.h index a87db5ca26f..37e73a93d48 100644 --- a/include/crm/common/acl_compat.h +++ b/include/crm/common/acl_compat.h @@ -30,6 +30,10 @@ 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); + #ifdef __cplusplus } #endif diff --git a/include/crm/common/acl_internal.h b/include/crm/common/acl_internal.h index cb4930bfaa1..5c7403ab915 100644 --- a/include/crm/common/acl_internal.h +++ b/include/crm/common/acl_internal.h @@ -38,6 +38,9 @@ pcmk__is_privileged(const char *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); diff --git a/include/crm/common/xml_element_internal.h b/include/crm/common/xml_element_internal.h index 3bc7197b907..3492e324506 100644 --- a/include/crm/common/xml_element_internal.h +++ b/include/crm/common/xml_element_internal.h @@ -48,7 +48,7 @@ xmlNode *pcmk__xe_first_child(const xmlNode *parent, const char *node_name, 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)(xmlAttr *, 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); diff --git a/lib/cib/cib_utils.c b/lib/cib/cib_utils.c index 49bf047f448..d042c9ff8d4 100644 --- a/lib/cib/cib_utils.c +++ b/lib/cib/cib_utils.c @@ -235,9 +235,9 @@ cib__perform_op_ro(cib__op_fn_t fn, xmlNode *req, xmlNode **current_cib, cib = *current_cib; - if (cib_acl_enabled(*current_cib, user) - && xml_acl_filtered_copy(user, *current_cib, *current_cib, - &cib_filtered)) { + if (cib_acl_enabled(*current_cib, user)) { + cib_filtered = pcmk__acl_filtered_copy(user, (*current_cib)->doc, + *current_cib); if (cib_filtered == NULL) { pcmk__debug("Pre-filtered the entire cib"); @@ -671,12 +671,12 @@ cib__perform_op_rw(enum cib_variant variant, cib__op_fn_t fn, xmlNode *req, if ((rc != pcmk_rc_ok) && cib_acl_enabled(old_versions, user)) { xmlNode *saved_cib = *cib; - if (xml_acl_filtered_copy(user, old_versions, *cib, cib)) { - if (*cib == NULL) { - pcmk__debug("Pre-filtered the entire cib result"); - } - pcmk__xml_free(saved_cib); + *cib = pcmk__acl_filtered_copy(user, old_versions->doc, *cib); + if (*cib == NULL) { + pcmk__debug("Pre-filtered the entire cib result"); } + + pcmk__xml_free(saved_cib); } pcmk__xml_free(top); diff --git a/lib/common/acl.c b/lib/common/acl.c index 693eb4a6efa..5ebf4daf716 100644 --- a/lib/common/acl.c +++ b/lib/common/acl.c @@ -819,142 +819,172 @@ attr_is_id(const xmlAttr *attr, void *user_data) * \note This is compatible with \c pcmk__xe_remove_matching_attrs(). */ static bool -attr_is_not_id(xmlAttr *attr, void *user_data) +attr_is_not_id(const xmlAttr *attr, void *user_data) { return !attr_is_id(attr, user_data); } /*! * \internal - * \brief Rid XML tree of all unreadable nodes and node properties + * \brief Filter out nodes that are unreadable by the current ACL user * - * \param[in,out] xml Root XML node to be purged of attributes + * Access or denial via ACLs is inherited, with more specific ACLs (those that + * match the node directly or match a more recent ancestor) taking precedence + * over less specific ones (those that match a less recent ancestor). Access is + * denied by default, if no ACL matches a node or any of its ancestors. * - * \return true if this node or any of its children are readable - * if false is returned, xml will be freed + * For each node in the tree, check whether any ACL matched the node. If so, + * then use that ACL for the current node. If not, then the current node + * inherits read access or denial from its parent. * - * \note This function is recursive + * Filter each child. Each child inherits read access or denial from the current + * node, unless an ACL matched the child directly. + * + * If the current node is readable, don't modify it. If it's unreadable but has + * at least one readable descendant, then remove all of its attributes other + * than \c PCMK_XA_ID. If it's unreadable and has no readable descendants, + * remove it. + * + * \param[in,out] xml XML tree (will be set to \c NULL if + * freed) + * \param[in] in_readable_context If \c true, the parent of \p xml is + * readable + * + * \note This function assumes that \c pcmk__apply_acls() has already been + * called for \p *xml->doc. + * \note This function is recursive. */ -static bool -purge_xml_attributes(xmlNode *xml) +static void +filter_unreadable_nodes(xmlNode **xml, bool in_readable_context) { + const xml_node_private_t *nodepriv = (*xml)->_private; + bool direct = false; + const char *how = NULL; + const char *name = (const char *) (*xml)->name; + const char *id = pcmk__s(pcmk__xe_id(*xml), "(unset)"); xmlNode *child = NULL; - bool readable_children = false; - xml_node_private_t *nodepriv = xml->_private; + /* If an ACL matched xml directly, update the context for xml and its + * descendants. Otherwise, keep the access that we inherited. + */ if (is_mode_allowed(nodepriv->flags, pcmk__xf_acl_read)) { - pcmk__trace("%s[@" PCMK_XA_ID "=%s] is readable", xml->name, - pcmk__xe_id(xml)); - return true; + in_readable_context = true; + direct = true; + + } else if (pcmk__is_set(nodepriv->flags, pcmk__xf_acl_deny)) { + in_readable_context = false; + direct = true; } - pcmk__xe_remove_matching_attrs(xml, true, attr_is_not_id, NULL); + if (direct) { + how = "directly"; + + } else if (*xml == xmlDocGetRootElement((*xml)->doc)) { + how = "by default"; + + } else { + how = "by inheritance"; + } - child = pcmk__xe_first_child(xml, NULL, NULL, NULL); + pcmk__trace("ACLs %s read access to %s[@" PCMK_XA_ID "='%s'] %s", + (in_readable_context? "allow" : "deny"), name, id, how); + + // Filter children recursively + child = pcmk__xml_first_child(*xml); while (child != NULL) { - xmlNode *tmp = child; + xmlNode *next = pcmk__xml_next(child); - child = pcmk__xe_next(child, NULL); + filter_unreadable_nodes(&child, in_readable_context); + child = next; + } - if (purge_xml_attributes(tmp)) { - readable_children = true; - } + if (in_readable_context) { + // This node is readable, either directly or by inheritance + return; } - if (!readable_children) { - // Nothing readable under here, so purge completely - pcmk__xml_free(xml); + if ((*xml)->children != NULL) { + /* At least one descendant is readable, but this node is not. + * + * Remove all attributes except PCMK_XA_ID. Keep this node as a bare + * "scaffolding" element for structure, so that the path from the root + * to any readable descendant remains intact. Removing this node would + * also remove its readable descendants. + */ + pcmk__xe_remove_matching_attrs(*xml, true, attr_is_not_id, NULL); + return; } - return readable_children; + + // This node and all its descendants are unreadable, so remove it + g_clear_pointer(xml, pcmk__xml_free); } /*! - * \brief Copy ACL-allowed portions of specified XML + * \internal + * \brief Copy XML, filtering out portions that are unreadable based on ACLs + * + * Access or denial via ACLs is inherited, with more specific ACLs (those that + * match the node directly or match a more recent ancestor) taking precedence + * over less specific ones (those that match a less recent ancestor). Access is + * denied by default, if no ACL matches a node or any of its ancestors. + * + * If the user's ACLs grant read access to a node (either directly or through + * the most recent ancestor node matched by an ACL), then that node is kept + * intact. + * + * If the user's ACLs deny read access to a node (either directly, through the + * most recent ancestor node matched by an ACL, or by default), then: + * - If the current node has at least one readable descendant, then all of the + * current node's attributes are removed other than \c PCMK_XA_ID. + * - Otherwise, the current node is removed. * - * \param[in] user Username whose ACLs should be used - * \param[in] acl_source XML containing ACLs - * \param[in] xml XML to be copied - * \param[out] result Copy of XML portions readable via ACLs + * \param[in] user User whose ACLs to use + * \param[in] acl_source XML document whose ACL definitions to use + * \param[in] xml XML to copy * - * \return \c true if \p acl_source and \p xml are non-NULL and ACLs - * are required for \p user, or \c false otherwise + * \return Newly allocated ACL-filtered copy of \p xml, or \c NULL if the entire + * document is filtered out * - * \note If this returns true, caller should use \p result rather than \p xml + * \note The caller is responsible for freeing the return value using + * \c pcmk__xml_free(). */ -bool -xml_acl_filtered_copy(const char *user, xmlNode *acl_source, xmlNode *xml, - xmlNode **result) +xmlNode * +pcmk__acl_filtered_copy(const char *user, xmlDoc *acl_source, xmlNode *xml) { - xmlNode *target = NULL; + xmlNode *result = NULL; xml_doc_private_t *docpriv = NULL; - *result = NULL; - if ((acl_source == NULL) || (acl_source->doc == NULL) || (xml == NULL) - || !pcmk_acl_required(user)) { + pcmk__assert((acl_source != NULL) && (xml != NULL)); - return false; - } - - target = pcmk__xml_copy(NULL, xml); - docpriv = target->doc->_private; - - pcmk__enable_acls(acl_source->doc, target->doc, user); - - pcmk__trace("Filtering XML copy using user '%s' ACLs", user); + result = pcmk__xml_copy(NULL, xml); - for (const GList *iter = docpriv->acls; iter != NULL; iter = iter->next) { - const xml_acl_t *acl = iter->data; - xmlXPathObject *xpath_obj = NULL; - int num_results = 0; - - if ((acl->mode != pcmk__xf_acl_deny) || (acl->xpath == NULL)) { - continue; - } - - xpath_obj = pcmk__xpath_search(target->doc, acl->xpath); - num_results = pcmk__xpath_num_results(xpath_obj); - - for (int i = 0; i < num_results; i++) { - xmlNode *match = pcmk__xpath_result(xpath_obj, i); - - if (match == NULL) { - continue; - } - - // @COMPAT See COMPAT comment in pcmk__apply_acls() - match = pcmk__xpath_match_element(match); - if (match == NULL) { - continue; - } - - if (!purge_xml_attributes(match) && (match == target)) { - pcmk__trace("ACLs deny user '%s' access to entire XML document", - user); - xmlXPathFreeObject(xpath_obj); - return true; - } - } - pcmk__trace("ACLs deny user '%s' access to %s (%d match%s)", user, - acl->xpath, num_results, - pcmk__plural_alt(num_results, "", "es")); - xmlXPathFreeObject(xpath_obj); + if (!pcmk_acl_required(user)) { + // Return an unfiltered copy + return result; } - if (!purge_xml_attributes(target)) { - pcmk__trace("ACLs deny user '%s' access to entire XML document", user); - return true; - } + pcmk__enable_acls(acl_source->doc, result->doc, user); + docpriv = result->doc->_private; if (docpriv->acls == NULL) { pcmk__trace("User '%s' without ACLs denied access to entire XML " "document", user); - pcmk__xml_free(target); - return true; + pcmk__xml_free(result); + return NULL; + } + + pcmk__trace("Filtering XML copy using user '%s' ACLs", user); + filter_unreadable_nodes(&result, false); + + // coverity[check_after_deref : FALSE] Doesn't understand g_clear_pointer + if (result == NULL) { + // Entire document was freed, so don't free docpriv->acls here + pcmk__trace("ACLs deny user '%s' access to entire XML document", user); + return NULL; } g_clear_pointer(&docpriv->acls, pcmk__free_acls); - *result = target; - return true; + return result; } /*! @@ -989,76 +1019,62 @@ implicitly_allowed(const xmlNode *xml) return true; } -#define display_id(xml) pcmk__s(pcmk__xe_id(xml), "") - /*! * \internal - * \brief Drop XML nodes created in violation of ACLs + * \brief Check whether ACLs allow creation of an XML node * - * Given an XML element, free all of its descendant nodes created in violation - * of ACLs, with the exception of allowing "scaffolding" elements (i.e. those - * that aren't in the ACL section and don't have any attributes other than - * \c PCMK_XA_ID). + * "Scaffolding" elements (those that aren't in the ACLs section and don't have + * any attributes other than \c PCMK_XA_ID) are always allowed. * - * \param[in,out] xml XML to check - * \param[in] check_top Whether to apply checks to argument itself - * (if true, xml might get freed) + * \param[in,out] xml XML node * - * \note This function is recursive + * \return \c true if \p xml is newly created and ACLs disallow its creation, or + * \c false otherwise + * + * \note This is compatible with \c pcmk__xml_tree_foreach_remove(). */ -void -pcmk__apply_creation_acl(xmlNode *xml, bool check_top) +static bool +check_creation_disallowed(xmlNode *xml) { + const char *type = (const char *) xml->name; + const char *id = pcmk__s(pcmk__xe_id(xml), ""); xml_node_private_t *nodepriv = xml->_private; - if (pcmk__is_set(nodepriv->flags, pcmk__xf_created)) { - if (implicitly_allowed(xml)) { - pcmk__trace("Creation of <%s> scaffolding with " - PCMK_XA_ID "=\"%s\" is implicitly allowed", - xml->name, display_id(xml)); - - } else if (pcmk__check_acl(xml, NULL, pcmk__xf_acl_write)) { - pcmk__trace("ACLs allow creation of <%s> with " - PCMK_XA_ID "=\"%s\"", - xml->name, display_id(xml)); + if (!pcmk__is_set(nodepriv->flags, pcmk__xf_created)) { + return false; + } - } else if (check_top) { - /* is_root=true should be impossible with check_top=true, but check - * for sanity - */ - bool is_root = (xmlDocGetRootElement(xml->doc) == xml); - xml_doc_private_t *docpriv = xml->doc->_private; - - pcmk__trace("ACLs disallow creation of %s<%s> with " - PCMK_XA_ID "=\"%s\"", - (is_root? "root element " : ""), xml->name, - display_id(xml)); - - // pcmk__xml_free() checks ACLs if enabled, which would fail - pcmk__clear_xml_flags(docpriv, pcmk__xf_acl_enabled); - pcmk__xml_free(xml); - - if (!is_root) { - // If root, the document was freed. Otherwise re-enable ACLs. - pcmk__set_xml_flags(docpriv, pcmk__xf_acl_enabled); - } - return; - - } else { - const bool is_root = (xml == xmlDocGetRootElement(xml->doc)); - - pcmk__notice("ACLs would disallow creation of %s<%s> with " - PCMK_XA_ID "=\"%s\"", - (is_root? "root element " : ""), xml->name, - display_id(xml)); - } + if (implicitly_allowed(xml)) { + pcmk__trace("Creation of <%s> scaffolding with " PCMK_XA_ID "=\"%s\" " + "is implicitly allowed", type, id); + return false; } - for (xmlNode *cIter = pcmk__xml_first_child(xml); cIter != NULL; ) { - xmlNode *child = cIter; - cIter = pcmk__xml_next(cIter); /* In case it is free'd */ - pcmk__apply_creation_acl(child, true); + if (pcmk__check_acl(xml, NULL, pcmk__xf_acl_write)) { + pcmk__trace("ACLs allow creation of <%s> with " PCMK_XA_ID "=\"%s\"", + type, id); + return false; } + + pcmk__trace("ACLs disallow creation of <%s> with " PCMK_XA_ID "=\"%s\"", + type, id); + return true; +} + +/*! + * \internal + * \brief Remove XML nodes created in violation of ACLs + * + * Given an XML tree, free all nodes created in violation of ACLs, with the + * exception of allowing "scaffolding" elements (those that aren't in the ACLs + * section and don't have any attributes other than \c PCMK_XA_ID). + * + * \param[in,out] xml XML tree + */ +void +pcmk__check_creation_acls(xmlNode *xml) +{ + pcmk__xml_tree_foreach_remove(xml, check_creation_disallowed); } /*! @@ -1089,7 +1105,20 @@ xml_acl_disable(xmlNode *xml) /* Catch anything that was created but shouldn't have been */ pcmk__apply_acls(xml->doc); - pcmk__apply_creation_acl(xml, false); + + /* Be sure not to free xml itself. + * + * @TODO Maybe we should free xml if it's newly created and the creation + * is disallowed, but we would need a way to inform the caller. This is + * public API. + */ + xml = pcmk__xml_first_child(xml); + while (xml != NULL) { + xmlNode *next = pcmk__xml_next(xml); + + pcmk__check_creation_acls(xml); + xml = next; + } pcmk__clear_xml_flags(docpriv, pcmk__xf_acl_enabled); } } @@ -1331,5 +1360,19 @@ xml_acl_enabled(const xmlNode *xml) return false; } +bool +xml_acl_filtered_copy(const char *user, xmlNode *acl_source, xmlNode *xml, + xmlNode **result) +{ + if ((acl_source == NULL) || (acl_source->doc == NULL) || (xml == NULL) + || !pcmk_acl_required(user)) { + + return false; + } + + *result = pcmk__acl_filtered_copy(user, acl_source->doc, xml); + return true; +} + // LCOV_EXCL_STOP // End deprecated API diff --git a/lib/common/crmcommon_private.h b/lib/common/crmcommon_private.h index a53ba60746e..01070585529 100644 --- a/lib/common/crmcommon_private.h +++ b/lib/common/crmcommon_private.h @@ -113,6 +113,9 @@ typedef struct { G_GNUC_INTERNAL const char *pcmk__xml_element_type_text(xmlElementType type); +G_GNUC_INTERNAL +void pcmk__xml_tree_foreach_remove(xmlNode *xml, bool (*fn)(xmlNode *)); + G_GNUC_INTERNAL bool pcmk__xml_reset_node_flags(xmlNode *xml, void *user_data); @@ -154,7 +157,7 @@ G_GNUC_INTERNAL void pcmk__apply_acls(xmlDoc *doc); G_GNUC_INTERNAL -void pcmk__apply_creation_acl(xmlNode *xml, bool check_top); +void pcmk__check_creation_acls(xmlNode *xml); G_GNUC_INTERNAL int pcmk__xa_remove(xmlAttr *attr, bool force); @@ -172,9 +175,6 @@ G_GNUC_PRINTF(2, 3); G_GNUC_INTERNAL void pcmk__mark_xml_node_dirty(xmlNode *xml); -G_GNUC_INTERNAL -bool pcmk__marked_as_deleted(xmlAttrPtr a, void *user_data); - G_GNUC_INTERNAL bool pcmk__dump_xml_attr(const xmlAttr *attr, void *user_data); diff --git a/lib/common/digest.c b/lib/common/digest.c index e17aa1ff393..2a2175a2a88 100644 --- a/lib/common/digest.c +++ b/lib/common/digest.c @@ -298,7 +298,7 @@ pcmk__xa_filterable(const char *name) // Return true if a is an attribute that should be filtered static bool -should_filter_for_digest(xmlAttrPtr a, void *user_data) +should_filter_for_digest(const xmlAttr *a, void *user_data) { if (g_str_has_prefix((const char *) a->name, CRM_META "_")) { return true; diff --git a/lib/common/patchset.c b/lib/common/patchset.c index 778be7ff9bd..c43aca60b41 100644 --- a/lib/common/patchset.c +++ b/lib/common/patchset.c @@ -312,34 +312,61 @@ add_changes_to_patchset(xmlNode *xml, xmlNode *patchset) } } +/*! + * \internal + * \brief Check whether a deleted object path contains the configuration element + * + * \param[in] data Deleted object (const pcmk__deleted_xml_t *) + * \param[in] ignored Ignored + * + * \retval 0 if \p data->path contains + * "/" PCMK_XE_CIB "/" PCMK_XE_CONFIGURATION + * \retval 1 otherwise + * + * \note This is a \c GCompareFunc. + */ +static int +config_in_deleted_obj_path(const void *data, const void *ignored) +{ + const pcmk__deleted_xml_t *deleted_obj = data; + + if (strstr(deleted_obj->path, + "/" PCMK_XE_CIB "/" PCMK_XE_CONFIGURATION) != NULL) { + + return 0; + } + + return 1; +} + +/*! + * \internal + * \brief Check whether a CIB XML tree contains a configuration change + * + * \param[in] xml XML tree + * + * \return \c true if \p xml contains a dirty or deleted configuration element, + * or \c false otherwise + */ static bool -is_config_change(xmlNode *xml) +is_config_change(const xmlNode *xml) { - GList *gIter = NULL; xml_node_private_t *nodepriv = NULL; - xml_doc_private_t *docpriv; + xml_doc_private_t *docpriv = xml->doc->_private; xmlNode *config = pcmk__xe_first_child(xml, PCMK_XE_CONFIGURATION, NULL, NULL); - if (config) { + if (config != NULL) { nodepriv = config->_private; } + + // Arbitrary xml may come from the public API, so NULL-check nodepriv if ((nodepriv != NULL) && pcmk__is_set(nodepriv->flags, pcmk__xf_dirty)) { - return TRUE; + return true; } - if ((xml->doc != NULL) && (xml->doc->_private != NULL)) { - docpriv = xml->doc->_private; - for (gIter = docpriv->deleted_objs; gIter; gIter = gIter->next) { - pcmk__deleted_xml_t *deleted_obj = gIter->data; - - if (strstr(deleted_obj->path, - "/" PCMK_XE_CIB "/" PCMK_XE_CONFIGURATION) != NULL) { - return TRUE; - } - } - } - return FALSE; + return (g_list_find_custom(docpriv->deleted_objs, NULL, + config_in_deleted_obj_path) != NULL); } // Guaranteed to return non-NULL diff --git a/lib/common/xml.c b/lib/common/xml.c index 1ac10c2bceb..2cbd9d95a84 100644 --- a/lib/common/xml.c +++ b/lib/common/xml.c @@ -72,16 +72,19 @@ pcmk__xml_element_type_text(xmlElementType type) /*! * \internal - * \brief Apply a function to each XML node in a tree (pre-order, depth-first) + * \brief Call a function for each XML node in a tree (pre-order, depth-first) * * \param[in,out] xml XML tree to traverse - * \param[in,out] fn Function to call for each node (returns \c true to + * \param[in] fn Function to call for each node (returns \c true to * continue traversing the tree or \c false to stop) * \param[in,out] user_data Argument to \p fn * * \return \c false if any \p fn call returned \c false, or \c true otherwise * * \note This function is recursive. + * \note \c fn may not free or unlink its XML argument or any of that node's + * ancestors. \c fn may unlink the descendants of that node, and it may + * free them as long as it also unlinks them. */ bool pcmk__xml_tree_foreach(xmlNode *xml, bool (*fn)(xmlNode *, void *), @@ -107,6 +110,50 @@ pcmk__xml_tree_foreach(xmlNode *xml, bool (*fn)(xmlNode *, void *), return true; } +/*! + * \internal + * \brief Remove XML nodes for which a given function returns \c true + * + * Call a function for each XML node in a tree (pre-order, depth-first). If the + * function returns true, remove the node. This means to free the entire + * document if the node is the document root, or to unlink and free the node and + * its subtree otherwise. ACLs and change tracking are ignored. + * + * \param[in,out] xml XML tree to traverse + * \param[in] fn Function to call for each node (returns \c true to remove + * its argument or \c false to recurse down its argument's + * subtree) + * + * \note This function is recursive. + */ +void +pcmk__xml_tree_foreach_remove(xmlNode *xml, bool (*fn)(xmlNode *)) +{ + if (xml == NULL) { + return; + } + + if (fn(xml)) { + if (xml == xmlDocGetRootElement(xml->doc)) { + pcmk__xml_free_doc(xml->doc); + + } else { + pcmk__xml_free_node(xml); + } + + return; + } + + xml = pcmk__xml_first_child(xml); + + while (xml != NULL) { + xmlNode *next = pcmk__xml_next(xml); + + pcmk__xml_tree_foreach_remove(xml, fn); + xml = next; + } +} + void pcmk__xml_set_parent_flags(xmlNode *xml, uint64_t flags) { @@ -532,6 +579,26 @@ pcmk__xml_position(const xmlNode *xml, enum pcmk__xml_flags ignore_if_set) return position; } +/*! + * \internal + * \brief Check whether an attribute is marked as deleted + * + * \param[in] attr XML attribute + * \param[in] user_data Ignored + * + * \return \c true if \c pcmk__xf_deleted is set for \p attr, or \c false + * otherwise + * + * \note This is compatible with \c pcmk__xe_remove_matching_attrs(). + */ +static bool +marked_as_deleted(const xmlAttr *attr, void *user_data) +{ + const xml_node_private_t *nodepriv = attr->_private; + + return pcmk__is_set(nodepriv->flags, pcmk__xf_deleted); +} + /*! * \internal * \brief Remove all attributes marked as deleted from an XML node @@ -547,7 +614,7 @@ static bool commit_attr_deletions(xmlNode *xml, void *user_data) { pcmk__xml_reset_node_flags(xml, NULL); - pcmk__xe_remove_matching_attrs(xml, true, pcmk__marked_as_deleted, NULL); + pcmk__xe_remove_matching_attrs(xml, true, marked_as_deleted, NULL); return true; } @@ -1819,7 +1886,7 @@ pcmk__xml_mark_changes(xmlNode *old_xml, xmlNode *new_xml) mark_xml_tree_dirty_created(new_child); // Check whether creation was allowed (may free new_child) - pcmk__apply_creation_acl(new_child, true); + pcmk__check_creation_acls(new_child); } } diff --git a/lib/common/xml_attr.c b/lib/common/xml_attr.c index 92b3ecd0c0a..e7916592215 100644 --- a/lib/common/xml_attr.c +++ b/lib/common/xml_attr.c @@ -113,19 +113,6 @@ pcmk__mark_xml_attr_dirty(xmlAttr *a) pcmk__mark_xml_node_dirty(parent); } -// This also clears attribute's flags if not marked as deleted -bool -pcmk__marked_as_deleted(xmlAttrPtr a, void *user_data) -{ - xml_node_private_t *nodepriv = a->_private; - - if (pcmk__is_set(nodepriv->flags, pcmk__xf_deleted)) { - return true; - } - nodepriv->flags = pcmk__xf_none; - return false; -} - /*! * \internal * \brief Append an XML attribute to a buffer diff --git a/lib/common/xml_element.c b/lib/common/xml_element.c index 8782d79dbfc..5e86806bb2c 100644 --- a/lib/common/xml_element.c +++ b/lib/common/xml_element.c @@ -510,7 +510,7 @@ struct remove_xa_if_matching_data { bool force; //! Match function to call for each attribute - bool (*match)(xmlAttr *, void *); + bool (*match)(const xmlAttr *, void *); //! User data argument for match function void *match_data; @@ -558,7 +558,7 @@ remove_xa_if_matching(xmlAttr *attr, void *user_data) */ void pcmk__xe_remove_matching_attrs(xmlNode *element, bool force, - bool (*match)(xmlAttr *, void *), + bool (*match)(const xmlAttr *, void *), void *user_data) { struct remove_xa_if_matching_data data = { diff --git a/lib/pengine/pe_digest.c b/lib/pengine/pe_digest.c index 563965666b0..e72a0cbf2d9 100644 --- a/lib/pengine/pe_digest.c +++ b/lib/pengine/pe_digest.c @@ -60,7 +60,7 @@ pe__free_digests(void *ptr) // Return true if XML attribute name is an element of a given gchar ** array static bool -attr_in_strv(xmlAttrPtr a, void *user_data) +attr_in_strv(const xmlAttr *a, void *user_data) { const char *name = (const char *) a->name; gchar **strv = user_data; @@ -70,7 +70,7 @@ attr_in_strv(xmlAttrPtr a, void *user_data) // Return true if XML attribute name is not an element of a given gchar ** array static bool -attr_not_in_strv(xmlAttrPtr a, void *user_data) +attr_not_in_strv(const xmlAttr *a, void *user_data) { return !attr_in_strv(a, user_data); } @@ -162,7 +162,7 @@ calculate_main_digest(pcmk__op_digest_t *data, pcmk_resource_t *rsc, // Return true if XML attribute name is a Pacemaker-defined fencing parameter static bool -is_fence_param(xmlAttrPtr attr, void *user_data) +is_fence_param(const xmlAttr *attr, void *user_data) { return pcmk_stonith_param((const char *) attr->name); }