[SSSD] [PATCHES] SUDO integration - first iteration
Pavel Březina
pbrezina at redhat.com
Tue Nov 1 13:39:36 UTC 2011
Dne 31.10.2011 18:35, Jakub Hrozek napsal(a):
> On 10/10/2011 03:20 PM, Pavel Březina wrote:
>> https://fedorahosted.org/sssd/ticket/623
>>
>> Helpful links:
>> - SUDO plugin API
>> http://www.gratisoft.us/sudo/man/1.8.1/sudo_plugin.man.html
>>
>> What does it do currently?
>> 1. sudo plugin sends input data to responder
>> 2. responder sends back to plugin that user is allowed to run the
>> command
>>
>> What does it not do?
>> - Doesn't require any PAM authentication
>> - Doesn't read anything from LDAP
>> - Doesn't set any environment variables (and reset current environment)
>> This actually can cause some troubles to applications, so far I've
>> encountered an error message in VIM (no $HOME specified)
>>
>> How to test it?
>> 1. Install SUDO version 1.8 or greater
>> I am running 1.8.2 built from source:
>> ftp://ftp.sudo.ws/pub/sudo/sudo-1.8.2.tar.gz
>>
>> 2. Enable SUDO plugin in /etc/sudo.conf
>> Plugin sss_sudo_policy /usr/lib/sudo/libsss_sudoplugin.so
>>
>> Unfortunately, SUDO doesn't allow to have more than one policy
>> plugin activated, so comment out standard sudoers plugin.
>>
>> 3. Enable it in sssd.conf ;-)
>> services += sudo
>>
>> 4. Run sssd
>>
>> 5. Run sudo
>>
>> I left there some debug messages, so don't worry when you see:
>> CMD Return code: 0
>> errnop: 0
>> Command exited with status 0
>>
>> in the output. But tell me if there is anything else than 0 :-)
>>
>
> In overall - good work! Pavel also has follow-up patches, so he wasn't
> idling in the meantime between him sending out this patchset and us
> getting it reviewed. Sorry for the delay, Pavel.
Thank you for your review.
>
> As sudo does not seem to have a bug tracker upstream and the Red Hat
> sudo maintainer is working with us, I'm going to open one generic "SSSD
> Sudo Feature tracker" bug in bugzilla.redhat.com for Fedora rawhide sudo
> and then one bug per feature we'd like to see in sudo blocking this
> tracker bug.
>
>> [PATCH 1/5]
>> Adds a client function that allows client to send data to responder.
>> And set sudo responder protocol version.
>>
>
> Ack but please consider squashing together with the responder part. Does
> this patch make sense standalone?
Most of the patches doesn't make sense to be standalone :)
It is actually used by plugin not the responder and I've put it in a
separate patch so the review could be easier.
>
>> [PATCH 2/5]
>> SUDO plugin.
>>
>
> Please move private declarations (such as struct plugin_state) out of
> the header.
Done.
>
> The patch includes a fallback to sudoers plugin in case this a user is
> not found. Currently this is handled inside the sudoers plugin based on
> the order of "sudo" parameter in nsswitch.conf. The plugin order should
> be made more generic in sudo upstream, but short-term we may want to
> support some kind of fallback ourselves.
>
> There's a TODO in the code we discussed off-list:
> "TODO: Do we need to check major and minor version or major is enough?"
> We need to ask sudo upstream on the specifics of versioning.
>
> We couldn't agree on #defining macros such as APPEND_ELEMENT or
> APPEND_ZERO inside a function and #undefining them later. I know that it
> would be nice to use a nested function for the task if C allowed them,
> but I don't like neither defining macros inside a function nor macros
> silently changing local variables. Other opinions welcome.
>
> Casting pointers such as "result = *((int*)response);" should be changed
> to use our safe memory alignment macros to avoid reintroducing #390.
Done.
>
> get_qualified_command_path() should be enhanced in order to be
> compatible with sudo's find_path(). Long-term sudo should export a
> function to perform the path-lookup so plugins don't have to define
> their own function for the task.
Done.
>
> Nitpicks - sudo_printf_t in policy_open() should be renamed, we use
> underscore for "output" variables.
Done.
> - the harcoded fields[4] should be changed to use a constant. Same
> applies for looping over the fields.
Changed to NULL terminated array.
>
>> [PATCH 3/5]
>> SUDO responder.
>>
>
> Nested macros again. Same comment as for patch #2.
>
> If you want to keep functions that will be implemented in a future patch
> in this one (as opposed to simply adding them when they're available),
> please define a simple skeleton that would just return EOK.
I haven't decided yet if it is necessary (haven't think of it though).
>
>> [PATCH 4/5]
>> Configure and Makefile updates so we can build plugin and responder.
>>
>
> Ack, but this patch might be squashed together into the plugin and
> responder patches respectively, but I don't really have a strong opinion.
>
>> [PATCH 5/5]
>> Just a little update that adds sudo responder to known services of monitor.
>>
>>
>
> Ack, but this could be squashed into the responder itself.
>
Done.
I've attach some more patches currently indexed 5-8. It is a skeleton of
data provider part. Their names should be self-explanatory.
The last one (SUDO responder - refresh SUDOers) is just a DP call for
testing purpose, it will be changed of course :)
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: 0001-SUDO-integration-client-common-interface.patch
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20111101/7c3486dc/attachment.ksh>
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: 0002-SUDO-integration-SUDO-plugin.patch
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20111101/7c3486dc/attachment-0001.ksh>
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: 0003-SUDO-integration-SUDO-responder.patch
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20111101/7c3486dc/attachment-0002.ksh>
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: 0004-SUDO-integration-configure-and-makefile-updated.patch
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20111101/7c3486dc/attachment-0003.ksh>
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: 0005-SUDO-integration-Data-provider-backend-handler.patch
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20111101/7c3486dc/attachment-0004.ksh>
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: 0006-SUDO-integration-Responder-Data-provider-communicati.patch
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20111101/7c3486dc/attachment-0005.ksh>
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: 0007-SUDO-integration-SUDO-LDAP-provider.patch
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20111101/7c3486dc/attachment-0006.ksh>
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: 0008-SUDO-Integration-SUDO-responder-refresh-SUDOers.patch
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20111101/7c3486dc/attachment-0007.ksh>
More information about the sssd-devel
mailing list