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

Sumit Bose sbose at redhat.com
Mon Sep 10 08:48:16 UTC 2012


On Mon, Sep 10, 2012 at 09:35:25AM +0200, Pavel Březina wrote:
> 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".

No, sorry for not being clear here. I think the id_provider default is
right. I just wanted to say that if for whatever reasons the
sudo_provider parameter is used explicitly in sssd.conf the
sudo_provider should be initialization even if sudo is not listed in
services.

bye,
Sumit

> 
> 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
> >
> 
> 
> _______________________________________________
> 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