[SSSD] [PATCHES] backend: initialize sudo only when it is enabled in services

Pavel Březina pbrezina at redhat.com
Mon Sep 10 07:35:25 UTC 2012


On 09/07/2012 11:31 AM, Sumit Bose wrote:
> On Wed, Sep 05, 2012 at 03:05:49PM +0200, Pavel Březina wrote:
>> 0001 fixes https://fedorahosted.org/sssd/ticket/1458
>> 0002 fixes a memory leak when be_process_init() fails. I think it
>> should be fixed, even though the backend is in this case immediately
>> terminated
>
> Hi,
>
> the patches compile without errors and work as expected. Nevertheless I
> have a few comments:
>
> 0001:
>   - I think it make sense to skip the initialization of the sudo target
>     if there is no responder configured. But to avoid surprises on the
>     user side I would recommend to do the initialization if it is
>     configured explicitly in sssd.conf, i.e sudo_provider = ldap, and
>     give a warning in the logs that the corresponding responder will not
>     be running.

I discussed this with Jakub before writing the patch. sudo_provider
currently defaults to id_provider. If I get it right, you are
suggesting to make the default value "none".

We agreed with Jakub that this makes more pressure on the user to
understand the configuration so we chose not to do it. Although the
configurations I've been seeing so far are mostly generated by
authconfig or ipa-client-install (where are all providers set
explicitly) so I don't think it is a problem after all. What do you say
Jakub?

If we decide to do it this way, it will also require changes in man page.

>> +    ret = confdb_get_string(ctx->cdb, ctx, CONFDB_MONITOR_CONF_ENTRY,
>> +                            CONFDB_MONITOR_ACTIVE_SERVICES, NULL, &services);
>> +    if (ret != EOK) {
>> +        DEBUG(SSSDBG_FATAL_FAILURE,
>> +              ("Unable to read from confdb [%d]: %s\n", ret, strerror(ret)));
>> +        return ret;
>> +    }
>> +
>
>   - I would prefer to use confdb_get_string_as_list() and use strcmp()
>     later on, to be on the safe side.
>
> 0002: ACK
>
> bye,
> Sumit
> _______________________________________________
> sssd-devel mailing list
> sssd-devel at lists.fedorahosted.org
> https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
>





More information about the sssd-devel mailing list