[SSSD] [patches] SDAP: "ldap_group_nesting_level = 0" does not appear to work
Lukas Slebodnik
lslebodn at redhat.com
Mon May 19 06:08:04 UTC 2014
On (17/05/14 22:52), Pavel Reichl wrote:
>Hello Lukas, thanks for comments please see replies inline, new patches
>are attached.
>
>On Fri, 2014-05-16 at 15:34 +0200, Lukas Slebodnik wrote:
>> On (13/05/14 00:07), Pavel Reichl wrote:
>> >Hello,
>> >
>> >please see attached patches solving:
>> >
>> >https://fedorahosted.org/sssd/ticket/2294
>> >
>> >Thanks,
>> >
>> >Pavel Reichl
>>
>> >From 17d8d7c4e53da2291daebfdbbf90d3448df9e765 Mon Sep 17 00:00:00 2001
>> >From: Pavel Reichl <preichl at redhat.com>
>> >Date: Mon, 12 May 2014 15:00:26 +0000
>> >Subject: [PATCH 2/3] SDAP: Make nesting_level = 0 to ignore nested groups
>> >
>> >Make ldap_group_nesting_level = 0 to ignore group nesting entirely.
>> >
>> >Resolves:
>> >https://fedorahosted.org/sssd/ticket/2294
>> >---
>> > src/providers/ldap/sdap_async_groups.c | 94 ++++++++++++++++++++++++++++++++++
>> > 1 file changed, 94 insertions(+)
>> >
>> >diff --git a/src/providers/ldap/sdap_async_groups.c b/src/providers/ldap/sdap_async_groups.c
>> >index 4240bb585d57a80199d040ac44891ee1b6429e05..b956e55bbf4f60e411a6bcf9f9ad79cf977f7144 100644
>> >--- a/src/providers/ldap/sdap_async_groups.c
>> >+++ b/src/providers/ldap/sdap_async_groups.c
>> >@@ -28,6 +28,86 @@
>> > #include "providers/ldap/sdap_idmap.h"
>> > #include "providers/ad/ad_common.h"
>> >
>> >+static errno_t sdap_get_entry_attrs_dn(TALLOC_CTX *mem_ctx,
>> >+ struct sysdb_ctx *ctx,
>> >+ const char *dn_str,
>> >+ const char **attrs,
>> >+ struct ldb_message ***_msgs,
>> >+ size_t *_num_msgs)
>> >+{
>> >+ TALLOC_CTX *tmp_ctx;
>> >+ struct ldb_dn *base_dn;
>> >+ struct ldb_message **msgs;
>> >+ size_t num_msgs;
>> >+ int ret;
>> >+
>> >+ tmp_ctx = talloc_new(NULL);
>> >+ if (tmp_ctx == NULL) {
>> >+ return ENOMEM;
>> >+ }
>> >+
>> >+ base_dn = sysdb_dn(ctx, tmp_ctx, dn_str);
>> >+ if (base_dn == NULL) {
>> >+ ret = ENOMEM;
>> >+ goto done;
>> >+ }
>> >+
>> >+ DEBUG(SSSDBG_TRACE_ALL, "Searching cache for [%s].\n", dn_str);
>> >+ ret = sysdb_search_entry(tmp_ctx, ctx, base_dn, LDB_SCOPE_BASE, "", attrs,
>> >+ &num_msgs, &msgs);
>> >+ if (ret != EOK) {
>> >+ DEBUG(SSSDBG_OP_FAILURE, "sysdb_search_entry failed: [%d]: %s\n",
>> >+ ret, strerror(ret));
>> >+ goto done;
>> >+ }
>> >+
>> >+ *_msgs = talloc_steal(mem_ctx, msgs);
>> >+ *_num_msgs = num_msgs;
>> >+
>> >+done:
>> >+ talloc_zfree(tmp_ctx);
>> >+ return ret;
>> >+}
>> >+
>> >+static errno_t sdap_is_member_type_group(struct sysdb_ctx *sctx,
>> >+ const char* sysdb_member_dn,
>> >+ bool *isGroup)
>> ^^^^^^^^^^^^^^
>> http://www.freeipa.org/page/Coding_Style#Naming
>> HIGHLY RECOMMENDED: Use low case multi word underscore separated notation for
>> naming variables.
>>
>> We have also another "SSSD rule" that output parameter start with underscore
>
>Fixed.
>> >+{
>> >+ struct ldb_message **msgs;
>> >+ size_t num_msgs;
>> >+ const char *attrs[] = {SYSDB_OBJECTCLASS, NULL};
>> >+ TALLOC_CTX *tmp_ctx;
>> >+ errno_t ret;
>> >+ const char *object_class;
>> >+
>> >+ tmp_ctx = talloc_new(NULL);
>> >+ if (tmp_ctx == NULL) {
>> >+ return ENOMEM;
>> >+ }
>> >+ ret = sdap_get_entry_attrs_dn(tmp_ctx, sctx, sysdb_member_dn, attrs, &msgs,
>> >+ &num_msgs);
>> >+ if (ret == EOK) {
>> >+ if (num_msgs != 1) {
>> >+ DEBUG(SSSDBG_OP_FAILURE,
>> >+ "Expected 1 value for %s, got %zu\n", attrs[0], num_msgs);
>> >+ ret = EINVAL;
>> >+ goto done;
>> >+ }
>> >+
>> >+ object_class = ldb_msg_find_attr_as_string(msgs[0],
>> >+ SYSDB_OBJECTCLASS, NULL);
>> >+ if (strcmp(object_class, SYSDB_GROUP_CLASS) == 0) {
>> >+ *isGroup = true;
>> >+ } else {
>> >+ *isGroup = false;
>> >+ }
>> >+ }
>> >+
>> >+done:
>> >+ talloc_free(tmp_ctx);
>> >+ return ret;
>> >+}
>> >+
>> > /* ==Group-Parsing Routines=============================================== */
>> >
>> > static int sdap_find_entry_by_origDN(TALLOC_CTX *memctx,
>> >@@ -1211,6 +1291,10 @@ sdap_process_group_members_2307bis(struct tevent_req *req,
>> > char *strdn;
>> > int ret;
>> > int i;
>> >+ int nesting_level;
>> >+ bool isMemberGroup = false;
>> >+
>> >+ nesting_level = dp_opt_get_int(state->opts->basic, SDAP_NESTING_LEVEL);
>> >
>> > for (i=0; i < memberel->num_values; i++) {
>> > member_dn = (char *)memberel->values[i].data;
>> >@@ -1220,7 +1304,17 @@ sdap_process_group_members_2307bis(struct tevent_req *req,
>> > state->dom,
>> > member_dn,
>> > &strdn);
>> >+
>> > if (ret == EOK) {
>> >+ if (nesting_level == 0) {
>> >+ /* Ignore group members which are groups themselves. */
>> >+ ret = sdap_is_member_type_group(state->sysdb, strdn,
>> >+ &isMemberGroup);
>> I think you reimplemented already existing sysdb functions.
>> I think that aim is to find out if "strdn" is group.
>>
>> strdn is already "sysdb DN" which can be used for function sysdb_search_*
>well actually strdn is pointer to char, which can't be directly used by
>sysdb_search_*, because they expect ldb_dn.
>
>> You can obtain groupname from dn with function sysdb_group_dn_name
>> hint: test_sysdb_group_dn_name
>> and then you can find group with the function sysdb_search_group_by_name
>You are right, but AFIK sysdb_group_dn_nane just returns RDN from DN,
>which will cause problems:
>
Thank you for pasting next two ldb DNs. They are vere helpful
>dn: name=ub,cn=users,cn=default,cn=sysdb
^^^^^^^^
>dn: name=ub,cn=groups,cn=default,cn=sysdb
^^^^^^^^^
>From highlighted parts of DN, it is obvious which one is group.
This fact will simplify your patches.
LS
Side note:
You wrote two functions with prefix "sdap_", but they are not related to LDAP
(sdap is abbreviation of sss LDAP). They are "sysdb" functions with
confusing name.
LS
More information about the sssd-devel
mailing list