https://fedorahosted.org/sssd/ticket/1029
The problem here is that we were trying to perform an "optimization" by bulk-deleting the contents of the service and host lists in the sysdb before dumping into it the new data we received from LDAP. This was causing a major performance hit on large deployments, because this recursive delete was repeatedly hitting a weak point of the memberOf plugin. However, upon closer analysis, Sumit pointed out that we don't actually need to rely on the local memberOf plugin in this situation. These patches remove the member/memberOf relationship from host/hostgroup and service/servicegroup entries in the SSSD. As a result, we don't invoke the memberOf plugin during the mass-delete and we see a significant performance increase. The patches [ab]use the fact that we know the DN structure of the hosts, service and groups so that we don't need to go and look them up when constructing the requests. Instead we take the originalMemberOf object and interpret the value directly from it. This is much faster than searching the sysdb for the original object to get its fqdn or cn value.
From df73a0294c75881123c17094f0c067d2d777a427 Mon Sep 17 00:00:00 2001 From: Stephen Gallagher <sgall...@redhat.com> Date: Wed, 5 Oct 2011 14:18:25 -0400 Subject: [PATCH 1/3] HBAC: Do not save member/memberOf links We can just trust the values from the FreeIPA server --- src/providers/ipa/ipa_hbac_common.c | 120 ----------------------------------- 1 files changed, 0 insertions(+), 120 deletions(-) diff --git a/src/providers/ipa/ipa_hbac_common.c b/src/providers/ipa/ipa_hbac_common.c index b37320a484cbf83f00fdbd33573a0a8ca92c64ce..f4ed839fa9e5a166981cbbb76b169c17dee76783 100644 --- a/src/providers/ipa/ipa_hbac_common.c +++ b/src/providers/ipa/ipa_hbac_common.c @@ -98,17 +98,8 @@ ipa_hbac_sysdb_save(struct sysdb_ctx *sysdb, struct sss_domain_info *domain, const char *group_subdir, const char *groupattr_name, size_t group_count, struct sysdb_attrs **groups) { - int lret; errno_t ret, sret; bool in_transaction = false; - const char **orig_member_dns; - size_t i, j, member_count; - struct ldb_message **members; - TALLOC_CTX *tmp_ctx = NULL; - const char *member_dn; - const char *group_id; - struct ldb_message *msg; - char *member_filter; if ((primary_count == 0 || primary == NULL) || (group_count > 0 && groups == NULL)) { @@ -149,117 +140,6 @@ ipa_hbac_sysdb_save(struct sysdb_ctx *sysdb, struct sss_domain_info *domain, group_subdir, ret, strerror(ret))); goto done; } - - /* Third, save the memberships */ - for (i = 0; i < group_count; i++) { - if (!groups[i]) { - ret = EINVAL; - goto done; - } - - talloc_free(tmp_ctx); - tmp_ctx = talloc_new(NULL); - if (tmp_ctx == NULL) { - ret = ENOMEM; - goto done; - } - - ret = sysdb_attrs_get_string(groups[i], - groupattr_name, - &group_id); - if (ret != EOK) { - DEBUG(1, ("Could not determine group attribute name\n")); - goto done; - } - - msg = ldb_msg_new(tmp_ctx); - if (msg == NULL) { - ret = ENOMEM; - goto done; - } - - msg->dn = sysdb_custom_dn(sysdb, msg, domain->name, - group_id, group_subdir); - if (msg->dn == NULL) { - ret = ENOMEM; - goto done; - } - - ret = sysdb_attrs_get_string_array(groups[i], - SYSDB_ORIG_MEMBER, - tmp_ctx, - &orig_member_dns); - - if (ret == EOK) { - /* One or more members were detected, prep the LDB message */ - lret = ldb_msg_add_empty(msg, SYSDB_MEMBER, LDB_FLAG_MOD_ADD, NULL); - if (lret != LDB_SUCCESS) { - ret = sysdb_error_to_errno(lret); - goto done; - } - } else if (ret == ENOENT) { - /* Useless group, has no members */ - orig_member_dns = talloc_array(tmp_ctx, const char *, 1); - if (!orig_member_dns) { - ret = ENOMEM; - goto done; - } - - /* Just set the member list to zero length so we skip - * processing it below - */ - orig_member_dns[0] = NULL; - } else { - DEBUG(1, ("Could not determine original members\n")); - goto done; - } - - for (j = 0; orig_member_dns[j]; j++) { - member_filter = talloc_asprintf(tmp_ctx, "%s=%s", - SYSDB_ORIG_DN, - orig_member_dns[j]); - if (member_filter == NULL) { - ret = ENOMEM; - goto done; - } - - ret = sysdb_search_custom(tmp_ctx, sysdb, - member_filter, primary_subdir, - NULL, &member_count, &members); - talloc_zfree(member_filter); - if (ret != EOK && ret != ENOENT) { - goto done; - } else if (ret == ENOENT || member_count == 0) { - /* No member exists with this orig_dn. Skip it */ - DEBUG(6, ("[%s] does not exist\n", orig_member_dns[j])); - continue; - } else if (member_count > 1) { - /* This probably means corruption in the cache, but - * we'll try to proceed anyway. - */ - DEBUG(1, ("More than one result for DN [%s], skipping\n")); - continue; - } - - member_dn = ldb_dn_get_linearized(members[0]->dn); - if (!member_dn) { - ret = ENOMEM; - goto done; - } - lret = ldb_msg_add_fmt(msg, SYSDB_MEMBER, "%s", member_dn); - if (lret != LDB_SUCCESS) { - ret = sysdb_error_to_errno(lret); - goto done; - } - } - - lret = ldb_modify(sysdb_ctx_get_ldb(sysdb), msg); - if (lret != LDB_SUCCESS) { - ret = sysdb_error_to_errno(lret); - goto done; - } - } - talloc_zfree(tmp_ctx); } ret = sysdb_transaction_commit(sysdb); -- 1.7.6.4
From 6950fc26c6f7ccb4f183eff1d29165a1d12eebe1 Mon Sep 17 00:00:00 2001 From: Stephen Gallagher <sgall...@redhat.com> Date: Wed, 5 Oct 2011 15:58:15 -0400 Subject: [PATCH 2/3] HBAC: Use originalMember for identifying servicegroups --- src/providers/ipa/ipa_hbac_common.c | 92 ++++++++++++++++------------ src/providers/ipa/ipa_hbac_private.h | 5 ++ src/providers/ipa/ipa_hbac_services.c | 109 +++++++++++++++++++++++++++++++++ 3 files changed, 166 insertions(+), 40 deletions(-) diff --git a/src/providers/ipa/ipa_hbac_common.c b/src/providers/ipa/ipa_hbac_common.c index f4ed839fa9e5a166981cbbb76b169c17dee76783..a233c12363e79cfaea67550ed85d610ceee31963 100644 --- a/src/providers/ipa/ipa_hbac_common.c +++ b/src/providers/ipa/ipa_hbac_common.c @@ -404,7 +404,7 @@ static errno_t hbac_eval_service_element(TALLOC_CTX *mem_ctx, struct sysdb_ctx *sysdb, struct sss_domain_info *domain, - const char *hostname, + const char *servicename, struct hbac_request_element **svc_element); static errno_t @@ -586,18 +586,18 @@ static errno_t hbac_eval_service_element(TALLOC_CTX *mem_ctx, struct sysdb_ctx *sysdb, struct sss_domain_info *domain, - const char *hostname, + const char *servicename, struct hbac_request_element **svc_element) { errno_t ret; - size_t i, count; + size_t i, j, count; TALLOC_CTX *tmp_ctx; struct hbac_request_element *svc; struct ldb_message **msgs; - const char *group_name; + struct ldb_message_element *el; struct ldb_dn *svc_dn; - const char *attrs[] = { IPA_CN, NULL }; - const char *service_filter; + const char *memberof_attrs[] = { SYSDB_ORIG_MEMBEROF, NULL }; + char *name; tmp_ctx = talloc_new(mem_ctx); if (tmp_ctx == NULL) return ENOMEM; @@ -608,15 +608,7 @@ hbac_eval_service_element(TALLOC_CTX *mem_ctx, goto done; } - svc->name = hostname; - - service_filter = talloc_asprintf(tmp_ctx, - "(objectClass=%s)", - IPA_HBAC_SERVICE_GROUP); - if (service_filter == NULL) { - ret = ENOMEM; - goto done; - } + svc->name = servicename; svc_dn = sysdb_custom_dn(sysdb, tmp_ctx, domain->name, svc->name, HBAC_SERVICES_SUBDIR); @@ -625,41 +617,61 @@ hbac_eval_service_element(TALLOC_CTX *mem_ctx, goto done; } - /* Find the service groups */ - ret = sysdb_asq_search(tmp_ctx, sysdb, svc_dn, - service_filter, SYSDB_MEMBEROF, - attrs, &count, &msgs); - if (ret != EOK && ret != ENOENT) { - DEBUG(1, ("Could not look up servicegroups\n")); + /* Look up the service to get its originalMemberOf entries */ + ret = sysdb_search_entry(tmp_ctx, sysdb, svc_dn, + LDB_SCOPE_BASE, NULL, + memberof_attrs, + &count, &msgs); + if (ret == ENOENT || count == 0) { + /* We won't be able to identify any groups + * This rule will only match the name or + * a service category of ALL + */ + svc->groups = NULL; + ret = EOK; + goto done; + } else if (ret != EOK) { + goto done; + } else if (count > 1) { + DEBUG(1, ("More than one result for a BASE search!\n")); + ret = EIO; goto done; - } else if (ret == ENOENT) { - count = 0; } - svc->groups = talloc_array(svc, const char *, count + 1); + el = ldb_msg_find_element(msgs[0], SYSDB_ORIG_MEMBEROF); + if (!el) { + /* Service is not a member of any groups + * This rule will only match the name or + * a service category of ALL + */ + svc->groups = NULL; + ret = EOK; + } + + + svc->groups = talloc_array(svc, const char *, el->num_values + 1); if (svc->groups == NULL) { ret = ENOMEM; goto done; } - for (i = 0; i < count; i++) { - group_name = ldb_msg_find_attr_as_string(msgs[i], IPA_CN, NULL); - if (group_name == NULL) { - DEBUG(1, ("Group with no name?\n")); - ret = EINVAL; - goto done; - } - svc->groups[i] = talloc_strdup(svc->groups, - group_name); - if (svc->groups[i] == NULL) { - ret = ENOMEM; - goto done; - } + for (i = j = 0; i < count; i++) { + ret = get_ipa_servicegroupname(tmp_ctx, sysdb, + (const char *)el->values[i].data, + &name); + if (ret != EOK && ret != ENOENT) goto done; - DEBUG(6, ("Added service group [%s] to the eval request\n", - svc->groups[i])); + /* ENOENT means we had a memberOf entry that wasn't a + * service group. We'll just ignore those (could be + * HBAC rules) + */ + + if (ret == EOK) { + svc->groups[j] = talloc_steal(svc->groups, name); + j++; + } } - svc->groups[i] = NULL; + svc->groups[j] = NULL; *svc_element = talloc_steal(mem_ctx, svc); ret = EOK; diff --git a/src/providers/ipa/ipa_hbac_private.h b/src/providers/ipa/ipa_hbac_private.h index 7289a04228cff420cc988c061d5828c0d25b0b52..4c637aa5fcdd7a1b38b48f73e3a0126bdb9cf821 100644 --- a/src/providers/ipa/ipa_hbac_private.h +++ b/src/providers/ipa/ipa_hbac_private.h @@ -157,6 +157,11 @@ hbac_service_attrs_to_rule(TALLOC_CTX *mem_ctx, const char *rule_name, struct sysdb_attrs *rule_attrs, struct hbac_rule_element **services); +errno_t +get_ipa_servicegroupname(TALLOC_CTX *mem_ctx, + struct sysdb_ctx *sysdb, + const char *service_dn, + char **servicename); /* From ipa_hbac_rules.c */ struct tevent_req * diff --git a/src/providers/ipa/ipa_hbac_services.c b/src/providers/ipa/ipa_hbac_services.c index fa580977c4b7774149d3b00298087c931394de53..dace7b235c566082469e6f7b18f972f468e37d4f 100644 --- a/src/providers/ipa/ipa_hbac_services.c +++ b/src/providers/ipa/ipa_hbac_services.c @@ -449,3 +449,112 @@ done: talloc_free(tmp_ctx); return ret; } + +errno_t +get_ipa_servicegroupname(TALLOC_CTX *mem_ctx, + struct sysdb_ctx *sysdb, + const char *service_dn, + char **servicegroupname) +{ + errno_t ret; + struct ldb_dn *dn; + const char *rdn_name; + const char *svc_comp_name; + const char *hbac_comp_name; + const struct ldb_val *rdn_val; + const struct ldb_val *svc_comp_val; + const struct ldb_val *hbac_comp_val; + + /* This is an IPA-specific hack. It may not + * work for non-IPA servers and will need to + * be changed if SSSD ever supports HBAC on + * a non-IPA server. + */ + *servicegroupname = NULL; + + dn = ldb_dn_new(mem_ctx, sysdb_ctx_get_ldb(sysdb), service_dn); + if (dn == NULL) { + ret = ENOMEM; + goto done; + } + + if (!ldb_dn_validate(dn)) { + ret = EINVAL; + goto done; + } + + if (ldb_dn_get_comp_num(dn) < 4) { + /* RDN, services, hbac, and at least one DC= */ + /* If it's fewer, it's not a group DN */ + ret = ENOENT; + goto done; + } + + /* If the RDN name is 'cn' */ + rdn_name = ldb_dn_get_rdn_name(dn); + if (rdn_name == NULL) { + /* Shouldn't happen if ldb_dn_validate() + * passed, but we'll be careful. + */ + ret = EINVAL; + goto done; + } + + if (strcasecmp("cn", rdn_name) != 0) { + /* RDN has the wrong attribute name. + * It's not a service. + */ + ret = ENOENT; + goto done; + } + + /* and the second component is "cn=hbacservicegroups" */ + svc_comp_name = ldb_dn_get_component_name(dn, 1); + if (strcasecmp("cn", svc_comp_name) != 0) { + /* The second component name is not "cn" */ + ret = ENOENT; + goto done; + } + + svc_comp_val = ldb_dn_get_component_val(dn, 1); + if (strncasecmp("hbacservicegroups", + (const char *) svc_comp_val->data, + svc_comp_val->length) != 0) { + /* The second component value is not "hbacservicegroups" */ + ret = ENOENT; + goto done; + } + + /* and the third component is "hbac" */ + hbac_comp_name = ldb_dn_get_component_name(dn, 2); + if (strcasecmp("cn", hbac_comp_name) != 0) { + /* The third component name is not "cn" */ + ret = ENOENT; + goto done; + } + + hbac_comp_val = ldb_dn_get_component_val(dn, 2); + if (strncasecmp("hbac", + (const char *) hbac_comp_val->data, + hbac_comp_val->length) != 0) { + /* The third component value is not "hbac" */ + ret = ENOENT; + goto done; + } + + /* Then the value of the RDN is the group name */ + rdn_val = ldb_dn_get_rdn_val(dn); + *servicegroupname = talloc_strndup(mem_ctx, + (const char *)rdn_val->data, + rdn_val->length); + if (*servicegroupname == NULL) { + ret = ENOMEM; + goto done; + } + + ret = EOK; + +done: + talloc_free(dn); + return ret; +} -- 1.7.6.4
From 16adde4832cf5b1652e599632e8eccf7d3ba934f Mon Sep 17 00:00:00 2001 From: Stephen Gallagher <sgall...@redhat.com> Date: Wed, 5 Oct 2011 16:43:26 -0400 Subject: [PATCH 3/3] HBAC: Use originalMember for identifying hostgroups --- src/providers/ipa/ipa_hbac_common.c | 100 ++++++++++++++++--------------- src/providers/ipa/ipa_hbac_hosts.c | 109 ++++++++++++++++++++++++++++++++++ src/providers/ipa/ipa_hbac_private.h | 5 ++ 3 files changed, 167 insertions(+), 47 deletions(-) diff --git a/src/providers/ipa/ipa_hbac_common.c b/src/providers/ipa/ipa_hbac_common.c index a233c12363e79cfaea67550ed85d610ceee31963..ce8005312ec0c97e4ef4dc2bff74542c851e42b7 100644 --- a/src/providers/ipa/ipa_hbac_common.c +++ b/src/providers/ipa/ipa_hbac_common.c @@ -689,14 +689,14 @@ hbac_eval_host_element(TALLOC_CTX *mem_ctx, struct hbac_request_element **host_element) { errno_t ret; - size_t i, count; + size_t i, j, count; TALLOC_CTX *tmp_ctx; struct hbac_request_element *host; struct ldb_message **msgs; - const char *group_name; + struct ldb_message_element *el; struct ldb_dn *host_dn; - const char *attrs[] = { IPA_CN, NULL }; - const char *host_filter; + const char *memberof_attrs[] = { SYSDB_ORIG_MEMBEROF, NULL }; + char *name; tmp_ctx = talloc_new(mem_ctx); if (tmp_ctx == NULL) return ENOMEM; @@ -713,68 +713,74 @@ hbac_eval_host_element(TALLOC_CTX *mem_ctx, /* We don't know the host (probably an rhost) * So we can't determine it's groups either. */ - host->groups = talloc_array(host, const char *, 1); - if (host->groups == NULL) { - ret = ENOMEM; - goto done; - } - host->groups[0] = NULL; + host->groups = NULL; ret = EOK; goto done; } - host_filter = talloc_asprintf(tmp_ctx, - "(objectClass=%s)", - IPA_HOSTGROUP); - if (host_filter == NULL) { - ret = ENOMEM; - goto done; - } - host_dn = sysdb_custom_dn(sysdb, tmp_ctx, domain->name, - host->name, HBAC_HOSTS_SUBDIR); + host->name, HBAC_HOSTS_SUBDIR); if (host_dn == NULL) { ret = ENOMEM; goto done; } - /* Find the host groups */ - ret = sysdb_asq_search(tmp_ctx, sysdb, host_dn, - host_filter, SYSDB_MEMBEROF, - attrs, &count, &msgs); - if (ret != EOK && ret != ENOENT) { - DEBUG(1, ("Could not look up host groups\n")); - goto done; - } else if (ret == ENOENT) { - count = 0; + /* Look up the host to get its originalMemberOf entries */ + ret = sysdb_search_entry(tmp_ctx, sysdb, host_dn, + LDB_SCOPE_BASE, NULL, + memberof_attrs, + &count, &msgs); + if (ret == ENOENT || count == 0) { + /* We won't be able to identify any groups + * This rule will only match the name or + * a host category of ALL + */ + host->groups = NULL; + ret = EOK; + goto done; + } else if (ret != EOK) { + goto done; + } else if (count > 1) { + DEBUG(1, ("More than one result for a BASE search!\n")); + ret = EIO; + goto done; } - host->groups = talloc_array(host, const char *, count + 1); + el = ldb_msg_find_element(msgs[0], SYSDB_ORIG_MEMBEROF); + if (!el) { + /* Host is not a member of any groups + * This rule will only match the name or + * a host category of ALL + */ + host->groups = NULL; + ret = EOK; + goto done; + } + + + host->groups = talloc_array(host, const char *, el->num_values + 1); if (host->groups == NULL) { ret = ENOMEM; goto done; } - for (i = 0; i < count; i++) { - group_name = ldb_msg_find_attr_as_string(msgs[i], - IPA_CN, - NULL); - if (group_name == NULL) { - DEBUG(1, ("Group with no name?\n")); - ret = EINVAL; - goto done; - } - host->groups[i] = talloc_strdup(host->groups, - group_name); - if (host->groups[i] == NULL) { - ret = ENOMEM; - goto done; - } + for (i = j = 0; i < count; i++) { + ret = get_ipa_hostgroupname(tmp_ctx, sysdb, + (const char *)el->values[i].data, + &name); + if (ret != EOK && ret != ENOENT) goto done; - DEBUG(6, ("Added host group [%s] to the eval request\n", - host->groups[i])); + /* ENOENT means we had a memberOf entry that wasn't a + * host group. We'll just ignore those (could be + * HBAC rules) + */ + + if (ret == EOK) { + host->groups[j] = talloc_steal(host->groups, name); + j++; + } } - host->groups[i] = NULL; + host->groups[j] = NULL; ret = EOK; diff --git a/src/providers/ipa/ipa_hbac_hosts.c b/src/providers/ipa/ipa_hbac_hosts.c index 31e96e546df60d8deca7bcd11ca18f8ea3c14b84..5f5354d892005bfc805323ad1932592bd9b8f75c 100644 --- a/src/providers/ipa/ipa_hbac_hosts.c +++ b/src/providers/ipa/ipa_hbac_hosts.c @@ -522,3 +522,112 @@ done: talloc_free(tmp_ctx); return ret; } + +errno_t +get_ipa_hostgroupname(TALLOC_CTX *mem_ctx, + struct sysdb_ctx *sysdb, + const char *host_dn, + char **hostgroupname) +{ + errno_t ret; + struct ldb_dn *dn; + const char *rdn_name; + const char *hostgroup_comp_name; + const char *account_comp_name; + const struct ldb_val *rdn_val; + const struct ldb_val *hostgroup_comp_val; + const struct ldb_val *account_comp_val; + + /* This is an IPA-specific hack. It may not + * work for non-IPA servers and will need to + * be changed if SSSD ever supports HBAC on + * a non-IPA server. + */ + *hostgroupname = NULL; + + dn = ldb_dn_new(mem_ctx, sysdb_ctx_get_ldb(sysdb), host_dn); + if (dn == NULL) { + ret = ENOMEM; + goto done; + } + + if (!ldb_dn_validate(dn)) { + ret = EINVAL; + goto done; + } + + if (ldb_dn_get_comp_num(dn) < 4) { + /* RDN, hostgroups, accounts, and at least one DC= */ + /* If it's fewer, it's not a group DN */ + ret = ENOENT; + goto done; + } + + /* If the RDN name is 'cn' */ + rdn_name = ldb_dn_get_rdn_name(dn); + if (rdn_name == NULL) { + /* Shouldn't happen if ldb_dn_validate() + * passed, but we'll be careful. + */ + ret = EINVAL; + goto done; + } + + if (strcasecmp("cn", rdn_name) != 0) { + /* RDN has the wrong attribute name. + * It's not a service. + */ + ret = ENOENT; + goto done; + } + + /* and the second component is "cn=hostgroups" */ + hostgroup_comp_name = ldb_dn_get_component_name(dn, 1); + if (strcasecmp("cn", hostgroup_comp_name) != 0) { + /* The second component name is not "cn" */ + ret = ENOENT; + goto done; + } + + hostgroup_comp_val = ldb_dn_get_component_val(dn, 1); + if (strncasecmp("hostgroups", + (const char *) hostgroup_comp_val->data, + hostgroup_comp_val->length) != 0) { + /* The second component value is not "hostgroups" */ + ret = ENOENT; + goto done; + } + + /* and the third component is "accounts" */ + account_comp_name = ldb_dn_get_component_name(dn, 2); + if (strcasecmp("cn", account_comp_name) != 0) { + /* The third component name is not "cn" */ + ret = ENOENT; + goto done; + } + + account_comp_val = ldb_dn_get_component_val(dn, 2); + if (strncasecmp("accounts", + (const char *) account_comp_val->data, + account_comp_val->length) != 0) { + /* The third component value is not "accounts" */ + ret = ENOENT; + goto done; + } + + /* Then the value of the RDN is the group name */ + rdn_val = ldb_dn_get_rdn_val(dn); + *hostgroupname = talloc_strndup(mem_ctx, + (const char *)rdn_val->data, + rdn_val->length); + if (*hostgroupname == NULL) { + ret = ENOMEM; + goto done; + } + + ret = EOK; + +done: + talloc_free(dn); + return ret; +} diff --git a/src/providers/ipa/ipa_hbac_private.h b/src/providers/ipa/ipa_hbac_private.h index 4c637aa5fcdd7a1b38b48f73e3a0126bdb9cf821..32b5d70ce8f648378cfbcb3edb36b958cf17f60f 100644 --- a/src/providers/ipa/ipa_hbac_private.h +++ b/src/providers/ipa/ipa_hbac_private.h @@ -131,6 +131,11 @@ hbac_shost_attrs_to_rule(TALLOC_CTX *mem_ctx, const char *rule_name, struct sysdb_attrs *rule_attrs, struct hbac_rule_element **source_hosts); +errno_t +get_ipa_hostgroupname(TALLOC_CTX *mem_ctx, + struct sysdb_ctx *sysdb, + const char *host_dn, + char **hostgroupname); /* From ipa_hbac_services.c */ struct tevent_req * -- 1.7.6.4
signature.asc
Description: This is a digitally signed message part
_______________________________________________ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://fedorahosted.org/mailman/listinfo/sssd-devel