[SSSD] [PATCH] SDAP: Relax POSIX check

Lukas Slebodnik lslebodn at redhat.com
Fri Sep 25 05:49:23 UTC 2015


On (24/09/15 17:11), Pavel Reichl wrote:
>
>
>On 09/24/2015 02:12 PM, Pavel Březina wrote:
>>On 09/23/2015 10:55 PM, Pavel Reichl wrote:
>>>Yes, you are missing the rest of the patch :-)
>>>
>>>      errno = 0;
>>>      strtouint32(vals[0]->bv_val, &endptr, 10);
>>>      if (errno || *endptr || (vals[0]->bv_val == endptr)) {
>>>-        DEBUG(SSSDBG_OP_FAILURE,
>>>+        DEBUG(SSSDBG_MINOR_FAILURE,
>>>                "POSIX attribute is not a number: %s\n", vals[0]->bv_val);
>>>-        goto done;
>>>      }
>>>
>>>
>>>We do not require attribute to be a positive number, we just warn about that. But not to make it too relaxed we added check on objectclass.
>>
>>Thanks. Code wise ack, but could you rephrase the commit message?
>>
>OK, I hope it's better now.

>From 4ff85ab612da94fab6591ac7a178d42c314dc63e Mon Sep 17 00:00:00 2001
>From: Pavel Reichl <preichl at redhat.com>
>Date: Tue, 22 Sep 2015 04:41:18 -0400
>Subject: [PATCH] SDAP: Relax POSIX check
>
>Relax the check on UID or GID just to check if at least one of them is
>present but do not require them to be positive numbers.
>
>Add requirement on objectclass attributes to be user or group to make
>check more reliable.
>
>Resolves:
>https://fedorahosted.org/sssd/ticket/2800
>---
> src/providers/ldap/sdap_async.c | 10 ++++++----
> 1 file changed, 6 insertions(+), 4 deletions(-)
>
>diff --git a/src/providers/ldap/sdap_async.c b/src/providers/ldap/sdap_async.c
>index 97c9ea5df61a6516ca74bb73edc9a116b1266c71..b81431f79f21755469bb9ff123d695a2a166e353 100644
>--- a/src/providers/ldap/sdap_async.c
>+++ b/src/providers/ldap/sdap_async.c
>@@ -2586,9 +2586,12 @@ sdap_posix_check_send(TALLOC_CTX *memctx, struct tevent_context *ev,
>     state->attrs[2] = opts->group_map[SDAP_AT_GROUP_GID].name;
>     state->attrs[3] = NULL;
> 
>-    state->filter = talloc_asprintf(state, "(|(%s=*)(%s=*))",
>+    state->filter = talloc_asprintf(state,
>+        "(|(&(%s=*)(objectclass=%s))(&(%s=*)(objectclass=%s)))",
>                                     opts->user_map[SDAP_AT_USER_UID].name,
>-                                    opts->group_map[SDAP_AT_GROUP_GID].name);
>+                                    opts->user_map[SDAP_OC_USER].name,
>+                                    opts->group_map[SDAP_AT_GROUP_GID].name,
>+                                    opts->group_map[SDAP_OC_GROUP].name);
>     if (state->filter == NULL) {
>         ret = ENOMEM;
>         goto fail;
>@@ -2671,9 +2674,8 @@ static errno_t sdap_posix_check_parse(struct sdap_handle *sh,
>     errno = 0;
>     strtouint32(vals[0]->bv_val, &endptr, 10);
>     if (errno || *endptr || (vals[0]->bv_val == endptr)) {
>-        DEBUG(SSSDBG_OP_FAILURE,
>+        DEBUG(SSSDBG_MINOR_FAILURE,
>               "POSIX attribute is not a number: %s\n", vals[0]->bv_val);
>-        goto done;
>     }
> 
>     state->has_posix = true;

There is a common agreement (at least from management side) that we want
to improve code coverage This patch is great opportunity to add regression
test. You can choose either integration test or unit test; you should know
what suits better to this case.

LS


More information about the sssd-devel mailing list