[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