[SSSD] [PRELIMINARY][PATCH] ifp users and groups

Pavel Březina pbrezina at redhat.com
Mon May 18 09:56:29 UTC 2015


On 05/14/2015 04:34 PM, Jakub Hrozek wrote:
> On Thu, May 14, 2015 at 02:22:53PM +0200, Pavel Březina wrote:
>> On 05/14/2015 01:26 PM, Jakub Hrozek wrote:
>>> On Wed, May 13, 2015 at 04:51:53PM +0200, Pavel Březina wrote:
>>>> On 05/11/2015 05:58 PM, Jakub Hrozek wrote:
>>>>> Thanks for the patches. They work quite well!
>>>>>
>>>>> One bug I found is that if you query a nonexistant object path with
>>>>> GetAll, then all subsequent queries block. Maybe we don't finish some
>>>>> request on error? This is what I tried:
>>>>> [jhrozek at client] sssd $ [(review)] dbus-send --print-reply --system --dest=org.freedesktop.sssd.infopipe /org/freedesktop/sssd/infopipe/Users/redhat_2ecom/10327 org.freedesktop.DBus.Properties.GetAll string:org.freedesktop.sssd.infopipe.Users.User
>>>>> Error org.freedesktop.DBus.Error.NoReply: Message did not receive a reply (timeout by message bus)
>>>>> [jhrozek at client] sssd $ [(review)] dbus-send --print-reply --system --dest=org.freedesktop.sssd.infopipe /org/freedesktop/sssd/infopipe/Users/linux_2etest/465400000 org.freedesktop.DBus.Properties.GetAll string:org.freedesktop.sssd.infopipe.Users.User
>>>>> Error org.freedesktop.DBus.Error.TimedOut: Activation of org.freedesktop.sssd.infopipe timed out
>>>>>
>>>>> The first objectpath is a copy-n-paste error from a different machine,
>>>>> the second is legitimate. All works well unless I use the nonexistant path.
>>>>
>>>> Container wasn't closed properly and it broke the bus.
>>>>
>>>> diff --git a/src/sbus/sssd_dbus_invokers.c b/src/sbus/sssd_dbus_invokers.c
>>>> index b2f0de0..f4d4ba3 100644
>>>> --- a/src/sbus/sssd_dbus_invokers.c
>>>> +++ b/src/sbus/sssd_dbus_invokers.c
>>>> @@ -336,6 +336,12 @@ int sbus_invoke_get_aDOsasDE(DBusMessageIter *iter,
>>>>       /* iterate over keys */
>>>>
>>>>       if (table == NULL) {
>>>> +        dbret = dbus_message_iter_close_container(iter, &it_array);
>>>> +        if (!dbret) {
>>>> +            ret = EIO;
>>>> +            goto done;
>>>> +        }
>>>> +
>>>>           ret = EOK;
>>>>           goto done;
>>>>       }
>>>>
>>>>
>>>> New patches are attached.
>>>
>>> I'm still testing functionality, but Coverity reported dead code:
>>> Error: DEADCODE (CWE-561):
>>> sssd-1.12.90/src/responder/ifp/ifp_groups.c:325: cond_notnull: Condition "req == NULL", taking false branch. Now the value of "req" is not "NULL".
>>> sssd-1.12.90/src/responder/ifp/ifp_groups.c:357: notnull: At condition "req == NULL", the value of "req" cannot be "NULL".
>>> sssd-1.12.90/src/responder/ifp/ifp_groups.c:357: dead_error_condition: The condition "req == NULL" cannot be true.
>>> sssd-1.12.90/src/responder/ifp/ifp_groups.c:358: dead_error_begin: Execution cannot reach this statement: "ret = 12;".
>>> #  356|                                             domain->name, name);
>>> #  357|       if (req == NULL) {
>>> #  358|->         ret = ENOMEM;
>>> #  359|           goto immediately;
>>> #  360|       }
>>>
>>> and ifp unit test is consistently failing for me:
>>> FAIL: ifp_tests
>>> ===============
>>>
>>> [==========] Running 6 test(s).
>>> [ RUN      ] ifp_test_req_create
>>> [       OK ] ifp_test_req_create
>>> [ RUN      ] ifp_test_req_wrong_uid
>>> [       OK ] ifp_test_req_wrong_uid
>>> [ RUN      ] test_el_to_dict
>>> [       OK ] test_el_to_dict
>>> [ RUN      ] test_attr_acl
>>> s2[i]
>>> /home/remote/jhrozek/devel/sssd/src/tests/cmocka/test_ifp.c:214: error: Failure!
>>>
>>> [  FAILED  ] test_attr_acl
>>> [ RUN      ] test_attr_acl_ex
>>> [       OK ] test_attr_acl_ex
>>> [ RUN      ] test_attr_allowed
>>> [       OK ] test_attr_allowed
>>> [==========] 6 test(s) run.
>>> [  PASSED  ] 5 test(s).
>>> [  FAILED  ] 1 test(s), listed below:
>>> [  FAILED  ] test_attr_acl
>>>
>>>   1 FAILED TEST(S)
>>> _______________________________________________
>>> sssd-devel mailing list
>>> sssd-devel at lists.fedorahosted.org
>>> https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
>>>
>>
>> Thanks for the review. New patches are attached.
>
> The patches are now fine, but I found a way to crash the ifp provider by
> looking up one of my users:
> [jhrozek at client] ~ $ [] dbus-send --print-reply --system --dest=org.freedesktop.sssd.infopipe /org/freedesktop/sssd/infopipe/Users org.freedesktop.sssd.infopipe.Users.FindByName string:seuser
> method return sender=:1.64 -> dest=:1.65 reply_serial=2
>     object path "/org/freedesktop/sssd/infopipe/Users/linux_2etest/465400006"
> [jhrozek at client] ~ $ [] dbus-send --print-reply --system --dest=org.freedesktop.sssd.infopipe /org/freedesktop/sssd/infopipe/Users/linux_2etest/465400006 org.freedesktop.DBus.Properties.GetAll string:org.freedesktop.sssd.infopipe.Users.User
> Error org.freedesktop.DBus.Error.NoReply: Did not receive a reply. Possible causes include: the remote application did not send a reply, the message bus security policy blocked the reply, the reply timeout expired, or the network connection was broken.

Thanks for the review, new patches are attached. This is the change:

> diff --git a/src/responder/ifp/ifp_users.c b/src/responder/ifp/ifp_users.c
> index d9e137d..26c1aa6 100644
> --- a/src/responder/ifp/ifp_users.c
> +++ b/src/responder/ifp/ifp_users.c
> @@ -477,6 +477,7 @@ void ifp_users_user_get_groups(struct sbus_request *sbus_req,
>      struct ldb_message *user;
>      struct ldb_result *res;
>      const char **out;
> +    int num_groups;
>      gid_t gid;
>      errno_t ret;
>      int i;
> @@ -515,12 +516,17 @@ void ifp_users_user_get_groups(struct sbus_request *sbus_req,
>          return;
>      }
>
> +    if (res->count == 0) {
> +        return;
> +    }
> +
>      out = talloc_zero_array(sbus_req, const char *, res->count);
>      if (out == NULL) {
>          DEBUG(SSSDBG_CRIT_FAILURE, "talloc_zero_array() failed\n");
>          return;
>      }
>
> +    num_groups = 0;
>      for (i = 0; i < res->count; i++) {
>          gid = sss_view_ldb_msg_find_attr_as_uint64(domain, res->msgs[i],
>                                                     SYSDB_GIDNUM, 0);
> @@ -533,10 +539,12 @@ void ifp_users_user_get_groups(struct sbus_request *sbus_req,
>              DEBUG(SSSDBG_CRIT_FAILURE, "ifp_groups_build_path() failed\n");
>              return;
>          }
> +
> +        num_groups++;
>      }
>
>      *_out = out;
> -    *_size = res->count;
> +    *_size = num_groups;
>  }
>

-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-sbus-provide-custom-error-names.patch
Type: text/x-patch
Size: 2220 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20150518/cf0fa8a9/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-sbus-add-sbus_opath_decompose-_exact.patch
Type: text/x-patch
Size: 10474 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20150518/cf0fa8a9/attachment-0001.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0003-sbus-add-a-sas-get-invoker.patch
Type: text/x-patch
Size: 23836 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20150518/cf0fa8a9/attachment-0002.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0004-IFP-add-org.freedesktop.sssd.infopipe.Users.patch
Type: text/x-patch
Size: 24012 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20150518/cf0fa8a9/attachment-0003.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0005-IFP-add-org.freedesktop.sssd.infopipe.Users.User.patch
Type: text/x-patch
Size: 43176 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20150518/cf0fa8a9/attachment-0004.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0006-IFP-add-org.freedesktop.sssd.infopipe.Groups.patch
Type: text/x-patch
Size: 19082 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20150518/cf0fa8a9/attachment-0005.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0007-IFP-add-org.freedesktop.sssd.infopipe.Groups.Group.patch
Type: text/x-patch
Size: 25892 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20150518/cf0fa8a9/attachment-0006.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0008-IFP-deprecate-GetUserAttr.patch
Type: text/x-patch
Size: 1008 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20150518/cf0fa8a9/attachment-0007.bin>


More information about the sssd-devel mailing list