[SSSD] [PATCH][PRELIMINARY] sudo: send username and uid while requesting default options

Pavel Březina pbrezina at redhat.com
Wed Apr 25 15:05:36 UTC 2012


The subdomains patches messed with one line which was causing troubles.
Here are the rebased patches.

On 04/23/2012 12:51 PM, Pavel Březina wrote:
> On 04/23/2012 10:03 AM, Jakub Hrozek wrote:
>> On Fri, Apr 13, 2012 at 12:46:01PM +0200, Pavel Březina wrote:
>>> On 03/26/2012 09:47 PM, Jakub Hrozek wrote:
>>>> On Mon, Mar 19, 2012 at 02:46:02PM +0100, Pavel Březina wrote:
>>>>> https://fedorahosted.org/sssd/ticket/1239
>>>>>
>>>>> [PATCH 1/2]
>>>>> Finally removes EOK constant from sudo api header. It is not used in
>>>>> the SUDO code so it does not require their changes.
>>>>>
>>>>
>>>> Looks good to me, it'w weird to use EOK in an external header (even
>>>> though it is correctly defined).
>>>>
>>>> (Note: this patch shouldn't be pushed even though I don't have any
>>>> comments. It is an ABI break and we need to coordinate with Daniel K.)
>>>>
>>>>> [PATCH 2/2]
>>>>> This does what is requested in the ticket. It seems to be very huge
>>>>> but
>>>>> in fact it is mainly changing the variable. Basically I tried to get
>>>>> rid of domain ctx where possible, leave it only in initgroups part and
>>>>> use command ctx elsewhere.
>>>>>
>>>>
>>>> Still, it is hard to review the huge patch. Can you split it into
>>>> smaller ones? What about creating one that removes the duplication
>>>> between _get_sudorules and _get_defaults, one that converts to using
>>>> cmdctx and one that adds the uid support?
>>>
>>> The patches are attached.
>>>
>>> I want to get those patches acked because I need to deliver the updated
>>> interface to Dan so he can update the sudo binary.
>>>
>>> It does not implement the in-memory cache (yet? :-)) due to the
>>> discussion with simo.
>>>
>>> The patches expects "sudo api: check sss_status instead of errnop in
>>> sss_sudo_send_recv_generic()" (already acked on the list and waiting
>>> to be pushed).
>>
>> Patch 0001: sudo api: remove EOK
>> Ack
>>
>> Patch 0002: sudo responder: remove code duplication in commands
>> I think it would be a good idea to also check if rawname[rawname_len] ==
>> '\0' when parsing username, this would ensure that all the string checks
>> work OK.
>
> Done.
>
>>
>> Now that several sudosrv_response_append_* functions were removed from
>> header, can you make them static? In particular
>> sudosrv_response_append_string(), sudosrv_response_append_uint32(),
>> sudosrv_response_append_rule(), sudosrv_response_append_attr().
>
> Done.
>
>> Patch 0003: sudo sysdb: make sysdb_get_sudo_user_info more configurable
>> Ack
>>
>> Patch 0004: sudo api: send uid, username and domainname
>> Please get rid of the C++ comments in sudosrv_cmd(). If you need to
>> leave code in but disabled, please use #if 0/#endif
>
> Thanks. I'm using keybord shortcut to comment out whole block with
> these slashes and I fortgot that we have this rule :)
>
> The whole hunk that
>> changes sudosrv_cmd() seems like it belongs to patch #2.
>
> Not really. It requires the new protocol which is implemented in this
> patch. It makes the two command more similar than they was before.
>
>> There's another // comment in sudosrv_get_sudorules_from_cache.
>>
>> The logic looks good to me, though.
>
> Thank you for the review.
>
> I'm attaching a 6th patch that removes all code related to the
> in-memory cache as it will not be used anymore.
>
>>
>> Even when the patches are acked, they shouldn't be pushed right?
>
> The patches work so they can be pushed IMHO. And I will use them as a
> starting point for the new design. [1]
>
> The only problem is that Daniel did not find the time yet to change the
> sudo sources. So we have no consumer that would understand the new
> protocol.
>
>
> [1] https://fedorahosted.org/sssd/wiki/DesignDocs/SUDOCachingRules
>
>
> _______________________________________________
> sssd-devel mailing list
> sssd-devel at lists.fedorahosted.org
> https://fedorahosted.org/mailman/listinfo/sssd-devel

-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-sudo-api-remove-EOK.patch
Type: text/x-patch
Size: 3447 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20120425/573069ff/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-sudo-responder-remove-code-duplication-in-commands.patch
Type: text/x-patch
Size: 26384 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20120425/573069ff/attachment-0001.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0003-sudo-responder-get-rid-of-dctx-where-possible.patch
Type: text/x-patch
Size: 19585 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20120425/573069ff/attachment-0002.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0004-sudo-sysdb-make-sysdb_get_sudo_user_info-more-config.patch
Type: text/x-patch
Size: 3224 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20120425/573069ff/attachment-0003.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0005-sudo-api-send-uid-username-and-domainname.patch
Type: text/x-patch
Size: 30799 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20120425/573069ff/attachment-0004.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0006-sudo-responder-discard-in-memory-cache.patch
Type: text/x-patch
Size: 14123 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20120425/573069ff/attachment-0005.bin>


More information about the sssd-devel mailing list