Apologies for the delay.

On Mon, Jul 15, 2013 at 08:30:03PM +0300, Alexander Bokovoy wrote:
> Here is the logic:
> 
> 0. Configuration is performed by setting
> 
>    schema-compat-lookup-sssd: <user|group>
>    schema-compat-sssd-min-id: <value>
> 
> in corresponding schema-compat plugin tree (cn=users and cn=groups).
> 
> If schema-compat-sssd-min-id is not set, it will default to 1000. It is
> used to filter out attempts to fetch system users (<1000 on Fedora by
> default).
> 
> 1. On query, we parse query filter to identify what type of request is
> this: user or group lookup and then issue getpwnam_r()/getgrnam_r() and
> getsidbyid() for libsss_nss_idmap to fetch all needed information.
> 
> SSSD caches these requests they should be relatively fast.
> 
> 2. Once we served the request, it is cached in schema-compat cache map.
> The entry in the cache is currently not expired explicitly but I'm
> working on expiring it on wrong authentication -- if PAM stack returns a
> response telling there is no such user.
> 
> 3. Authentication bind for cached entries is done via PAM service
> 'system-auth'. If HBAC rule 'allow_all' is disabled in FreeIPA, one
> needs to create a rule with service 'system-auth' and allow all users to
> access it on IPA masters. Since system-auth is never used explicitly by
> any application (it is always included through PAM stack and only
> top-level PAM service is used to drive the HBAC ruleset), there is no
> problem.
> 
> PAM authentication code is taken from pam_passthru DS plugin. We cannot
> use it unchanged because pam_passthru expects that LDAP entry will exist
> in DS, while it is not true for these synthetic entries representing
> trusted domain users.
> 
> On Fedora one needs pam-devel and libsss_nss_idmap-devel to build the
> plugin with new functionality.

The bits about how to configure this facility need to be in the
documentation somewhere.  Right now there is none being added, and no
new self-tests.

> diff --git a/configure.ac b/configure.ac
> index 8d7cbe1..4a47d36 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -309,6 +309,47 @@ AC_SUBST(ASYNCNS_CFLAGS)
>  AC_SUBST(ASYNCNS_LIBS)
>  fi
>  
> +AC_ARG_WITH(sss_nss_idmap,
> +         AS_HELP_STRING([--with-sss-nss-idmap], [use libsss_nss_idmap]),
> +         use_sss_nss_idmap=$withval,use_sss_nss_idmap=AUTO)
> +if pkg-config sss_nss_idmap 2> /dev/null ; then
> +         if test x$use_sss_nss_idmap != xno ; then
> +             AC_DEFINE(HAVE_SSS_NSS_IDMAP,1,[Define if you have 
> libsss_nss_idmap.])
> +             PKG_CHECK_MODULES(SSS_NSS_IDMAP,sss_nss_idmap)
> +         else
> +             SSS_NSS_IDMAP_CFLAGS=
> +             SSS_NSS_IDMAP_LIBS=
> +         fi
> +else
> +     if test $use_sss_idmap = yes ; then

Should this reference to $use_sss_idmap be referring to
$use_sss_nss_idmap instead?

> +             PKG_CHECK_MODULES(SSS_NSS_IDMAP,sss_nss_idmap)
> +     else
> +             SSS_NSS_IDMAP_CFLAGS=
> +             SSS_NSS_IDMAP_LIBS=
> +     fi
> +fi
> +AM_CONDITIONAL([SSS_NSS_IDMAP], [test x$SSS_NSS_IDMAP_LIBS != x])

I suspect this'll need to quote SSS_NSS_IDMAP_LIBS here, in case its
value ever starts to include whitespace.

> +if x$SSS_NSS_IDMAP_LIBS != x ; then

Likewise here.

> +     AC_CHECK_HEADERS(pam.h)
> +     if test x$ac_cv_header_pam_h = xno ; then
> +             use_pam=yes
> +     else
> +             use_pam=no
> +     fi
> +
> +     if test $use_pam = yes ; then
> +             PAM_CFLAGS=
> +             PAM_LIBS=-lpam
> +     else
> +             AC_ERROR([<pam.h> not found and it is required for SSSD mode])
> +     fi
> +     AC_SUBST(PAM_CFLAGS)
> +     AC_SUBST(PAM_LIBS)
> +fi

Jakub already noted that this should be checking for
<security/pam_appl.h>.

> @@ -401,6 +442,13 @@ 
> AC_DEFINE_UNQUOTED(SCH_CONTAINER_CONFIGURATION_RDN_ATTR,"$rdnattr",
>  attrattr=schema-compat-entry-attribute
>  AC_DEFINE_UNQUOTED(SCH_CONTAINER_CONFIGURATION_ATTR_ATTR,"$attrattr",
>                  [Define to name of the attribute which is used to specify 
> attributes to be used when constructing entries.])
> +sssdattr=schema-compat-lookup-sssd
> +AC_DEFINE_UNQUOTED(SCH_CONTAINER_CONFIGURATION_SSSD_ATTR,"$sssdattr",
> +                [Define to name of the attribute which dictates whether or 
> not SSSD on FreeIPA master is consulted about trusted domains' users.])

Is this a boolean attribute?

> diff --git a/src/back-sch-pam.c b/src/back-sch-pam.c
> new file mode 100644
> index 0000000..3266261
> --- /dev/null
> +++ b/src/back-sch-pam.c
> @@ -0,0 +1,361 @@
> +/** BEGIN COPYRIGHT BLOCK
> + * This Program is free software; you can redistribute it and/or modify it 
> under
> + * the terms of the GNU General Public License as published by the Free 
> Software
> + * Foundation; version 2 of the License.
> + *
> + * This Program is distributed in the hope that it will be useful, but 
> WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or 
> FITNESS
> + * FOR A PARTICULAR PURPOSE. See the GNU General Public License for more 
> details.
> + *
> + * You should have received a copy of the GNU General Public License along 
> with
> + * this Program; if not, write to the Free Software Foundation, Inc., 59 
> Temple
> + * Place, Suite 330, Boston, MA 02111-1307 USA.
> + *
> + * In addition, as a special exception, Red Hat, Inc. gives You the 
> additional
> + * right to link the code of this Program with code not covered under the GNU
> + * General Public License ("Non-GPL Code") and to distribute linked 
> combinations
> + * including the two, subject to the limitations in this paragraph. Non-GPL 
> Code
> + * permitted under this exception must only link to the code of this Program
> + * through those well defined interfaces identified in the file named 
> EXCEPTION
> + * found in the source code files (the "Approved Interfaces"). The files of
> + * Non-GPL Code may instantiate templates or use macros or inline functions 
> from
> + * the Approved Interfaces without causing the resulting work to be covered 
> by
> + * the GNU General Public License. Only Red Hat, Inc. may make changes or
> + * additions to the list of Approved Interfaces. You must obey the GNU 
> General
> + * Public License in all respects for all of the Program code and other code 
> used
> + * in conjunction with the Program except the Non-GPL Code covered by this
> + * exception. If you modify this file, you may extend this exception to your
> + * version of the file, but you are not obligated to do so. If you do not 
> wish to
> + * provide this exception without modification, you must delete this 
> exception
> + * statement from your version and license this file solely under the GPL 
> without
> + * exception.
> + *
> + *
> + * Copyright (C) 2005 Red Hat, Inc.
> + * All rights reserved.
> + * END COPYRIGHT BLOCK **/

Did you mean to use the server's GPLv2+exceptions license for this code?
It differs from the rest of the plugin.  And that date of 2005 looks
kind of odd.

> +#ifdef HAVE_CONFIG_H
> +#include "config.h"
> +#endif
> +
> +#include <sys/types.h>
> +#include <stdlib.h>
> +#include <string.h>
> +#include <time.h>
> +#include <unistd.h>
> +
> +#ifdef HAVE_DIRSRV_SLAPI_PLUGIN_H
> +#include <nspr.h>
> +#include <nss.h>
> +#include <dirsrv/slapi-plugin.h>
> +#else
> +#include <slapi-plugin.h>
> +#endif
> +
> +
> +#include <security/pam_appl.h>
> +
> +/*
> + * PAM is not thread safe.  We have to execute any PAM API calls in
> + * a critical section.  This is the lock that protects that code.
> + */
> +static Slapi_Mutex *PAMLock = NULL;

Right now this mutex is being initialized after the server's gone and
launched multiple threads, which if timed right could go badly.  Move
the pointer into the plugin_state structure, and initialize it when the
rest of the structure's being initialized when the compat plugin starts
up.

Also, please avoid using caps in variable names and use underscores,
like the rest of the plugin does.

> +/* Utility struct to wrap strings to avoid mallocs if possible - use
> +   stack allocated string space */
> +#define MY_STATIC_BUF_SIZE 256
> +typedef struct my_str_buf {
> +     char fixbuf[MY_STATIC_BUF_SIZE];
> +     char *str;
> +} MyStrBuf;
> +
> +static char *
> +init_my_str_buf(MyStrBuf *buf, const char *s)
> +{
> +     PR_ASSERT(buf);
> +     if (s && (strlen(s) < sizeof(buf->fixbuf))) {
> +             strcpy(buf->fixbuf, s);
> +             buf->str = buf->fixbuf;
> +     } else {
> +             buf->str = slapi_ch_strdup(s);
> +             buf->fixbuf[0] = 0;
> +     }
> +
> +     return buf->str;
> +}
> +
> +static void
> +delete_my_str_buf(MyStrBuf *buf)
> +{
> +     if (buf->str != buf->fixbuf) {
> +             slapi_ch_free_string(&(buf->str));
> +     }
> +}

This seems unnecessary.

> +/* for third arg to pam_start */
> +struct my_pam_conv_str {
> +     Slapi_PBlock *pb;
> +     char *pam_identity;
> +};
> +
> +/* returns a berval value as a null terminated string */
> +static char *strdupbv(struct berval *bv)
> +{
> +     char *str = slapi_ch_malloc(bv->bv_len+1);
> +     memcpy(str, bv->bv_val, bv->bv_len);
> +     str[bv->bv_len] = 0;
> +     return str;
> +}

Please reformat this so that the function name is in the first column,
to make it easier to find with a trivial grep ^$functionname.  Also
applies to do_pam_auth() below.

In non-error cases, this new string is going to be freed by free().
Please avoid allocating memory with slapi_ch_malloc() if it'll be freed
with free(), or allocating with malloc()/calloc()/strdup() if it'll be
freed with slapi_ch_free().

> +static void
> +free_pam_response(int nresp, struct pam_response *resp)
> +{
> +     int ii;
> +     for (ii = 0; ii < nresp; ++ii) {
> +             if (resp[ii].resp) {
> +                     slapi_ch_free((void **)&(resp[ii].resp));
> +             }
> +     }
> +     slapi_ch_free((void **)&resp);
> +}

In non-error cases, these pointers would be freed by a plugin calling
free().  Please avoid mixing calls to slapi_ch_malloc() and
slapi_ch_free() with malloc()/free()/strdup().

> +/*
> + * This is the conversation function passed into pam_start().  This is what 
> sets the password
> + * that PAM uses to authenticate.  This function is sort of stupid - it 
> assumes all echo off
> + * or binary prompts are for the password, and other prompts are for the 
> username.  Time will
> + * tell if this is actually the case.
> + */
> +static int
> +pam_conv_func(int num_msg, const struct pam_message **msg, struct 
> pam_response **resp, void *mydata)
> +{
> +     int ii;
> +     struct berval *creds;
> +     struct my_pam_conv_str *my_data = (struct my_pam_conv_str *)mydata;
> +    struct pam_response *reply;
> +     int ret = PAM_SUCCESS;
> +
> +    if (num_msg <= 0) {
> +             return PAM_CONV_ERR;
> +     }
> +
> +     /* empty reply structure */
> +    reply = (struct pam_response *)slapi_ch_calloc(num_msg,
> +                                                                             
>   sizeof(struct pam_response));
> +     slapi_pblock_get( my_data->pb, SLAPI_BIND_CREDENTIALS, &creds ); /* the 
> password */
> +     for (ii = 0; ii < num_msg; ++ii) {
> +             /* hard to tell what prompt is for . . . */
> +             /* assume prompts for password are either BINARY or ECHO_OFF */
> +             if (msg[ii]->msg_style == PAM_PROMPT_ECHO_OFF) {
> +                     reply[ii].resp = strdupbv(creds);
> +#ifdef LINUX
> +             } else if (msg[ii]->msg_style == PAM_BINARY_PROMPT) {
> +                     reply[ii].resp = strdupbv(creds);
> +#endif
> +             } else if (msg[ii]->msg_style == PAM_PROMPT_ECHO_ON) { /* 
> assume username */
> +                     reply[ii].resp = slapi_ch_strdup(my_data->pam_identity);
> +             } else if (msg[ii]->msg_style == PAM_ERROR_MSG) {
> +             } else if (msg[ii]->msg_style == PAM_TEXT_INFO) {
> +             } else {
> +                     ret = PAM_CONV_ERR;
> +             }

Replace this with a switch{} construction.

> +     }
> +
> +     if (ret == PAM_CONV_ERR) {
> +             free_pam_response(num_msg, reply);
> +             reply = NULL;
> +     }
> +
> +     *resp = reply;
> +
> +     return ret;
> +}

You're mixing hard tabs and spaces for indentation here.

> +/*
> + * Do the actual work of authenticating with PAM. First, get the PAM identity
> + * based on the method used to convert the BIND identity to the PAM identity.
> + * Set up the structures that pam_start needs and call pam_start().  After
> + * that, call pam_authenticate and pam_acct_mgmt.  Check the various return
> + * values from these functions and map them to their corresponding LDAP BIND
> + * return values.  Return the appropriate LDAP error code.
> + * This function will also set the appropriate LDAP response controls in
> + * the given pblock.
> + * Since this function can be called multiple times, we only want to return
> + * the errors and controls to the user if this is the final call, so the
> + * final_method parameter tells us if this is the last one.  Coupled with
> + * the fallback argument, we can tell if we are able to send the response
> + * back to the client.
> + */

There aren't parameters named final_method or fallback in the function
signature.  Please update the comment to better describe the current
implementation.

> +static int
> +do_pam_auth(
> +     Slapi_PBlock *pb,
> +     char *pam_service, /* name of service for pam_start() */
> +     int pw_response_requested, /* do we need to send pwd policy resp 
> control */
> +     Slapi_Entry *entry
> +)

Fix the formatting for this to match the rest of the plugin, please.

> +{
> +     MyStrBuf pam_id;
> +     const char *binddn = NULL;
> +     Slapi_DN *bindsdn = NULL;
> +     int rc = PAM_SUCCESS;
> +     int retcode = LDAP_SUCCESS;
> +     pam_handle_t *pam_handle;
> +     struct my_pam_conv_str my_data;
> +     struct pam_conv my_pam_conv = {pam_conv_func, NULL};
> +     char *errmsg = NULL; /* free with PR_smprintf_free */
> +
> +     slapi_pblock_get( pb, SLAPI_BIND_TARGET_SDN, &bindsdn );
> +     if (NULL == bindsdn) {
> +             errmsg = PR_smprintf("Null bind dn");
> +             retcode = LDAP_OPERATIONS_ERROR;
> +             pam_id.str = NULL; /* initialize pam_id.str */
> +             goto done; /* skip the pam stuff */
> +     }
> +     binddn = slapi_sdn_get_dn(bindsdn);
> +
> +     char *val = slapi_entry_attr_get_charptr(entry, "uid");
> +     init_my_str_buf(&pam_id, val);
> +     slapi_ch_free_string(&val);

Move the declaration of val to the beginning of the function.  Also, if
we already have a non-const copy of the uid value, there's no need to
bother with the second one -- just use val directly.

> +     if (!pam_id.str) {
> +             errmsg = PR_smprintf("Bind DN [%s] is invalid or not found", 
> binddn);
> +             retcode = LDAP_NO_SUCH_OBJECT; /* user unknown */
> +             goto done; /* skip the pam stuff */
> +     }
> +
> +     /* do the pam stuff */
> +     my_data.pb = pb;
> +     my_data.pam_identity = pam_id.str;
> +     my_pam_conv.appdata_ptr = &my_data;
> +     slapi_lock_mutex(PAMLock);
> +     /* from this point on we are in the critical section */
> +     rc = pam_start(pam_service, pam_id.str, &my_pam_conv, &pam_handle);
> +
> +     if (rc == PAM_SUCCESS) {
> +             /* use PAM_SILENT - there is no user interaction at this point 
> */
> +             rc = pam_authenticate(pam_handle, 0);

Should this call to pam_authenticate be specifying PAM_SILENT or not?

> +             /* check different types of errors here */
> +             if (rc == PAM_USER_UNKNOWN) {
> +                     errmsg = PR_smprintf("User id [%s] for bind DN [%s] 
> does not exist in PAM",
> +                                                              pam_id.str, 
> binddn);
> +                     retcode = LDAP_NO_SUCH_OBJECT; /* user unknown */
> +             } else if (rc == PAM_AUTH_ERR) {
> +                     errmsg = PR_smprintf("Invalid PAM password for user id 
> [%s], bind DN [%s]",
> +                                                              pam_id.str, 
> binddn);
> +                     retcode = LDAP_INVALID_CREDENTIALS; /* invalid creds */
> +             } else if (rc == PAM_MAXTRIES) {
> +                     errmsg = PR_smprintf("Authentication retry limit 
> exceeded in PAM for "
> +                                                              "user id [%s], 
> bind DN [%s]",
> +                                                              pam_id.str, 
> binddn);
> +                     if (pw_response_requested) {
> +                             slapi_pwpolicy_make_response_control(pb, -1, 
> -1, LDAP_PWPOLICY_ACCTLOCKED);
> +                     }
> +                     retcode = LDAP_CONSTRAINT_VIOLATION; /* max retries */
> +             } else if (rc != PAM_SUCCESS) {
> +                     errmsg = PR_smprintf("Unknown PAM error [%s] for user 
> id [%s], bind DN [%s]",
> +                                                              
> pam_strerror(pam_handle, rc), pam_id.str, binddn);
> +                     retcode = LDAP_OPERATIONS_ERROR; /* pam config or 
> network problem */
> +             }

Replace this with a switch{} construction.

> +     }
> +
> +     /* if user authenticated successfully, see if there is anything we need
> +        to report back w.r.t. password or account lockout */
> +     if (rc == PAM_SUCCESS) {
> +             rc = pam_acct_mgmt(pam_handle, 0);
> +             /* check different types of errors here */
> +             if (rc == PAM_USER_UNKNOWN) {
> +                     errmsg = PR_smprintf("User id [%s] for bind DN [%s] 
> does not exist in PAM",
> +                                                              pam_id.str, 
> binddn);
> +                     retcode = LDAP_NO_SUCH_OBJECT; /* user unknown */
> +             } else if (rc == PAM_AUTH_ERR) {
> +                     errmsg = PR_smprintf("Invalid PAM password for user id 
> [%s], bind DN [%s]",
> +                                                              pam_id.str, 
> binddn);
> +                     retcode = LDAP_INVALID_CREDENTIALS; /* invalid creds */
> +             } else if (rc == PAM_PERM_DENIED) {
> +                     errmsg = PR_smprintf("Access denied for PAM user id 
> [%s], bind DN [%s]"
> +                                                              " - see 
> administrator",
> +                                                              pam_id.str, 
> binddn);
> +                     if (pw_response_requested) {
> +                             slapi_pwpolicy_make_response_control(pb, -1, 
> -1, LDAP_PWPOLICY_ACCTLOCKED);
> +                     }
> +                     retcode = LDAP_UNWILLING_TO_PERFORM;
> +             } else if (rc == PAM_ACCT_EXPIRED) {
> +                     errmsg = PR_smprintf("Expired PAM password for user id 
> [%s], bind DN [%s]: "
> +                                                              "reset 
> required",
> +                                                              pam_id.str, 
> binddn);
> +                     slapi_add_pwd_control(pb, LDAP_CONTROL_PWEXPIRED, 0);
> +                     if (pw_response_requested) {
> +                             slapi_pwpolicy_make_response_control(pb, -1, 
> -1, LDAP_PWPOLICY_PWDEXPIRED);
> +                     }
> +                     retcode = LDAP_INVALID_CREDENTIALS;
> +             } else if (rc == PAM_NEW_AUTHTOK_REQD) { /* handled same way as 
> ACCT_EXPIRED */
> +                     errmsg = PR_smprintf("Expired PAM password for user id 
> [%s], bind DN [%s]: "
> +                                                              "reset 
> required",
> +                                                              pam_id.str, 
> binddn);
> +                     slapi_add_pwd_control(pb, LDAP_CONTROL_PWEXPIRED, 0);
> +                     if (pw_response_requested) {
> +                             slapi_pwpolicy_make_response_control(pb, -1, 
> -1, LDAP_PWPOLICY_PWDEXPIRED);
> +                     }
> +                     retcode = LDAP_INVALID_CREDENTIALS;
> +             } else if (rc != PAM_SUCCESS) {
> +                     errmsg = PR_smprintf("Unknown PAM error [%s] for user 
> id [%s], bind DN [%s]",
> +                                                              
> pam_strerror(pam_handle, rc), pam_id.str, binddn);
> +                     retcode = LDAP_OPERATIONS_ERROR; /* unknown */
> +             }

Replace this with a switch{} construction.

> +     }
> +
> +     rc = pam_end(pam_handle, rc);
> +     slapi_unlock_mutex(PAMLock);
> +     /* not in critical section any more */
> +
> +done:
> +     delete_my_str_buf(&pam_id);
> +
> +     if ((retcode == LDAP_SUCCESS) && (rc != PAM_SUCCESS)) {
> +             errmsg = PR_smprintf("Unknown PAM error [%d] for user id [%s], 
> bind DN [%s]",
> +                                                      rc, pam_id.str, 
> binddn);
> +             retcode = LDAP_OPERATIONS_ERROR;
> +     }
> +
> +     if (retcode != LDAP_SUCCESS) {
> +             slapi_send_ldap_result(pb, retcode, NULL, errmsg, 0, NULL);
> +     }
> +
> +     if (errmsg) {
> +             PR_smprintf_free(errmsg);
> +     }
> +
> +     return retcode;
> +}
> +
> +/*
> + * Entry point into the PAM auth code.  Shields the rest of the app
> + * from PAM API code.  Get our config params, then call the actual
> + * code that does the PAM auth.  Can call that code up to 3 times,
> + * depending on what methods are set in the config.
> + */
> +int
> +backend_sch_do_pam_auth(Slapi_PBlock *pb, Slapi_Entry *entry)
> +{
> +     int rc = LDAP_SUCCESS;
> +     MyStrBuf pam_service; /* avoid malloc if possible */
> +     int pw_response_requested;
> +     LDAPControl **reqctrls = NULL;
> +
> +     if (!PAMLock && !(PAMLock = slapi_new_mutex())) {
> +             return LDAP_LOCAL_ERROR;
> +     }
> +
> +     init_my_str_buf(&pam_service, "system-auth");
> +
> +     slapi_pblock_get (pb, SLAPI_REQCONTROLS, &reqctrls);
> +     slapi_pblock_get (pb, SLAPI_PWPOLICY, &pw_response_requested);
> +
> +     /* figure out which method is the last one - we only return error 
> codes, controls
> +        to the client and send a response on the last method */

This comment doesn't seem to be current.

> +     rc = do_pam_auth(pb, pam_service.str, pw_response_requested, entry);
> +
> +     delete_my_str_buf(&pam_service);
> +
> +     return rc;
> +}

reqctrls isn't being used after it's assigned.  Unless I'm missing where
it is, go ahead and remove it.

There's also no need to make a copy of the "system-auth" string when it
can be used directly.

> diff --git a/src/back-sch-sssd.c b/src/back-sch-sssd.c
> new file mode 100644
> index 0000000..8168675
> --- /dev/null
> +++ b/src/back-sch-sssd.c
> @@ -0,0 +1,335 @@
> +/*
> + * Copyright 2008,2009,2010,2011,2012 Red Hat, Inc.
> + *
> + * This Program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; version 2 of the License.
> + *
> + * This Program is distributed in the hope that it will be useful, but
> + * WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + * General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this Program; if not, write to the
> + *
> + *   Free Software Foundation, Inc.
> + *   59 Temple Place, Suite 330
> + *   Boston, MA 02111-1307 USA
> + *
> + */
> +
> +#ifdef HAVE_CONFIG_H
> +#include "config.h"
> +#endif
> +
> +#include <sys/types.h>
> +#include <stdlib.h>
> +#include <string.h>
> +#include <time.h>
> +#include <unistd.h>
> +#include <pwd.h>
> +#include <grp.h>
> +
> +#ifdef HAVE_DIRSRV_SLAPI_PLUGIN_H
> +#include <nspr.h>
> +#include <nss.h>
> +#include <dirsrv/slapi-plugin.h>
> +#else
> +#include <slapi-plugin.h>
> +#endif
> +
> +#include <rpc/xdr.h>
> +#include <sss_nss_idmap.h>
> +
> +#include "backend.h"
> +#include "back-shr.h"
> +#include "plugin.h"
> +#include "map.h"
> +#include "back-sch.h"
> +
> +struct backend_search_filter_config {
> +     bool_t search_user;
> +     bool_t search_group;
> +     bool_t search_uid;
> +     bool_t search_gid;
> +     bool_t name_set;
> +     char   *name;
> +};
> +
> +/* Check simple filter to see if it has
> + * (cn|uid|uidNumber|gidNumber=<value>) or (objectClass=posixGroup)
> + * Called by slapi_filter_apply(). */
> +static int
> +backend_search_filter_has_cn_uid(Slapi_Filter *filter, void *arg)
> +{
> +     struct backend_search_filter_config *config = arg;
> +     struct berval *bval;
> +     char *filter_type;
> +     int f_choice, rc;
> +
> +     f_choice = slapi_filter_get_choice(filter);
> +     rc = slapi_filter_get_ava(filter, &filter_type, &bval);
> +     if ((LDAP_FILTER_EQUALITY == f_choice) && (0 == rc)) {
> +             if (0 == strcasecmp(filter_type, "uidNumber")) {
> +                     config->search_uid = TRUE;
> +                     config->name_set = TRUE;
> +             } else if (0 == strcasecmp(filter_type, "gidNumber")) {
> +                     config->search_gid = TRUE;
> +                     config->name_set = TRUE;
> +             } else if (0 == strcasecmp(filter_type, "uid")) {
> +                     config->search_user = TRUE;
> +                     config->name_set = TRUE;
> +             } else if (0 == strcasecmp(filter_type, "cn")) {
> +                     config->name_set = TRUE;
> +             } else if ((0 == strcasecmp(filter_type, "objectClass")) &&
> +                        (0 == strcasecmp(bval->bv_val, "posixGroup"))) {
> +                     config->search_group = TRUE;
> +             }
> +
> +             if ((NULL == config->name) && config->name_set) {
> +                     config->name = bval->bv_val;

Is bval->bv_val guaranteed to be nul-terminated here?

> +             }
> +     }
> +
> +     if ((config->search_uid ||
> +          config->search_gid ||
> +          config->search_user ||
> +          config->search_group) && (config->name != NULL)) {
> +             return SLAPI_FILTER_SCAN_STOP;
> +     }
> +     return SLAPI_FILTER_SCAN_CONTINUE;
> +}
> +
> +static Slapi_Entry *
> +backend_retrieve_user_entry_from_sssd(char *user_name, bool_t is_uid,
> +                              struct backend_set_data *set_data,
> +                              struct backend_search_cbdata *cbdata)
> +{
> +     struct passwd pwd, *result;
> +     Slapi_Entry *entry;
> +     int rc;
> +     enum sss_id_type id_type;
> +     char *sid_str;
> +
> +     if (set_data->sssd_buffer == NULL) {
> +             return NULL;
> +     }
> +
> +     if (is_uid) {
> +             rc = getpwuid_r(atoi(user_name), &pwd,
> +                             set_data->sssd_buffer,
> +                             set_data->sssd_buffer_len, &result);

Please check this conversion for errors, and that it produces a value
in the range of unsigned int (uid_t is unsigned).

Why do the function names all suggest that we're retrieving data from
SSSD, when we don't verify that?

> +     } else {
> +             rc = getpwnam_r(user_name, &pwd,
> +                             set_data->sssd_buffer,
> +                             set_data->sssd_buffer_len, &result);
> +     }

These need to handle ERANGE errors.

> +     if ((result == NULL) || (rc != 0)) {
> +             return NULL;
> +     }
> +
> +     if (pwd.pw_uid < cbdata->sssd_min_id) {
> +             return NULL;
> +     }
> +
> +     entry = slapi_entry_alloc();
> +     if (entry == NULL) {
> +             return NULL;
> +     }
> +
> +     slapi_entry_add_string(entry,
> +                            "objectClass", "top");
> +     slapi_entry_add_string(entry,
> +                            "objectClass", "posixAccount");
> +     slapi_entry_add_string(entry,
> +                            "objectClass", "extensibleObject");
> +     slapi_entry_add_string(entry,
> +                            "uid", user_name);

If is_uid is true, this is a numeric string.  Intentional?

> +     slapi_entry_attr_set_int(entry,
> +                              "uidNumber", pwd.pw_uid);
> +     slapi_entry_attr_set_int(entry,
> +                              "gidNumber", pwd.pw_gid);
> +     slapi_entry_add_string(entry,
> +                            "gecos", pwd.pw_gecos);
> +     if (strlen(pwd.pw_gecos) > 0) {
> +             slapi_entry_add_string(entry,
> +                                    "cn", pwd.pw_gecos);
> +     } else {
> +             slapi_entry_add_string(entry,
> +                                    "cn", user_name);
> +     }
> +
> +     slapi_entry_add_string(entry,
> +                            "homeDirectory", pwd.pw_dir);
> +     slapi_entry_add_string(entry,
> +                            "loginShell", pwd.pw_shell);
> +     slapi_entry_set_sdn(entry, set_data->container_sdn);
> +     slapi_entry_set_dn(entry, slapi_ch_smprintf("uid=%s,%s",
> +                     user_name, slapi_sdn_get_dn(set_data->container_sdn)));

I don't think that call to slapi_entry_set_sdn() is necessary if we're
calling slapi_entry_set_dn() immediately after it.

This DN needs to be escaped properly.

> +     rc = sss_nss_getsidbyid(pwd.pw_uid, &sid_str, &id_type);
> +     if ((rc == 0) && (sid_str != NULL)) {
> +             slapi_entry_add_string(entry, "objectClass", "ipaNTUserAttrs");

Either this is unnecessary here, or it needs to start being copied into
the entries that we're adding to the cache.

> +             slapi_entry_add_string(entry, "ipaNTSecurityIdentifier", 
> sid_str);
> +             free(sid_str);
> +     }
> +
> +
> +     return entry;
> +}
> +
> +
> +static Slapi_Entry *
> +backend_retrieve_group_entry_from_sssd(char *group_name, bool_t is_gid,
> +                              struct backend_set_data *set_data,
> +                              struct backend_search_cbdata *cbdata)
> +{
> +     struct group grp, *result;
> +     const char *sdn;
> +     Slapi_Entry *entry;
> +     int rc, i;
> +     enum sss_id_type id_type;
> +     char *sid_str;
> +
> +     if (set_data->sssd_buffer == NULL) {
> +             return NULL;
> +     }
> +
> +     if (is_gid) {
> +             rc = getgrgid_r(atoi(group_name), &grp,
> +                             set_data->sssd_buffer,
> +                             set_data->sssd_buffer_len, &result);

Please check this conversion for errors, and that it produces a value
in the range of unsigned int (gid_t is unsigned).

> +     } else {
> +             rc = getgrnam_r(group_name, &grp,
> +                             set_data->sssd_buffer,
> +                             set_data->sssd_buffer_len, &result);
> +     }

These need to handle ERANGE errors, too.

> +     if ((result == NULL) || (rc != 0)) {
> +             return NULL;
> +     }
> +
> +     if (grp.gr_gid < cbdata->sssd_min_id) {
> +             return NULL;
> +     }
> +
> +     entry = slapi_entry_alloc();
> +     if (entry == NULL) {
> +             return NULL;
> +     }
> +
> +     slapi_entry_add_string(entry,
> +                            "objectClass", "top");
> +     slapi_entry_add_string(entry,
> +                            "objectClass", "posixGroup");
> +     slapi_entry_add_string(entry,
> +                            "objectClass", "extensibleObject");
> +     slapi_entry_add_string(entry,
> +                            "cn", group_name);

If is_gid is true, this is a numeric string.  Intentional?

> +     slapi_entry_attr_set_int(entry,
> +                              "gidNumber", grp.gr_gid);
> +
> +     slapi_entry_set_sdn(entry, set_data->container_sdn);
> +     slapi_entry_set_dn(entry,
> +                        slapi_ch_smprintf("cn=%s,%s", group_name,
> +                                          
> slapi_sdn_get_dn(set_data->container_sdn)));

I don't think that call to slapi_entry_set_sdn() is necessary if we're
calling slapi_entry_set_dn() immediately after it.

This DN needs to be escaped properly.

> +     if (grp.gr_mem) {
> +             if (set_data->sssd_relevant_set != NULL) {
> +                     sdn = 
> slapi_sdn_get_dn(set_data->sssd_relevant_set->container_sdn);
> +             } else {
> +                     sdn = slapi_sdn_get_dn(set_data->container_sdn);
> +             }
> +             for (i=0; grp.gr_mem[i]; i++) {
> +                     slapi_entry_add_string(entry, "memberUid",
> +                             slapi_ch_smprintf("uid=%s,%s", grp.gr_mem[i], 
> sdn));
> +             }

The "memberUid" attribute doesn't typically contain DNs.  Did you mean
to use "member" here?  Or to just use the user login name for the value?

> +     }
> +
> +     rc = sss_nss_getsidbyid(grp.gr_gid, &sid_str, &id_type);
> +     if ((rc == 0) && (sid_str != NULL)) {
> +             slapi_entry_add_string(entry, "objectClass", "ipaNTGroupAttrs");

Either this is unnecessary here, or it needs to start being copied into
the entries that we're adding to the cache.

> +             slapi_entry_add_string(entry, "ipaNTSecurityIdentifier", 
> sid_str);
> +             free(sid_str);
> +     }
> +
> +     return entry;
> +}
> +
> +static void
> +backend_search_sssd_send_entry(struct backend_set_data *set_data,
> +                            struct backend_search_cbdata *cbdata,
> +                            Slapi_Entry *entry)
> +{
> +     char *ndn;
> +     if (entry) {
> +             slapi_entry_add_string(entry,
> +                                    "schema-compat-origin", "sssd");
> +             cbdata->matched = TRUE;
> +             ndn = slapi_entry_get_ndn(entry);
> +             backend_set_entry(cbdata->pb, entry, set_data);

Problem: we only have a reading lock at this point.

> +             slapi_send_ldap_search_entry(cbdata->pb, entry,
> +                                          NULL, cbdata->attrs,
> +                                          cbdata->attrsonly);
> +             cbdata->n_entries++;
> +             if (cbdata->closest_match) {
> +                     free(cbdata->closest_match);
> +             }
> +             cbdata->closest_match = strdup(ndn);
> +             /* Entry is created, cache it via map. 
> +              * Next request will be served from the cache */
> +             //backend_set_entry(cbdata->pb, entry, set_data);

Is this commented-out call supposed to be here?

> +             slapi_entry_free(entry);
> +     }
> +}
> +
> +/* Check filter for a component like (uid=<value>) and if found,
> + * perform look up against SSSD and create entry based on that */
> +void
> +backend_search_sssd(struct backend_set_data *set_data,
> +                 struct backend_search_cbdata *cbdata)
> +{
> +     int result, rc;
> +     Slapi_Entry *entry;
> +     struct backend_search_filter_config config = {FALSE, FALSE, FALSE, 
> FALSE, FALSE, NULL};
> +
> +     /* There was no match but we asked to check SSSD */
> +     /* First, we search the filter to see if it includes cn|uid=<value> 
> check */
> +     result = slapi_filter_apply(cbdata->filter,
> +                    backend_search_filter_has_cn_uid, &config, &rc);
> +     if ((result != SLAPI_FILTER_SCAN_STOP) ||
> +         (NULL == config.name)) {
> +             return;
> +     }
> +
> +     /* Drop irrelevant requests. Each set only works with a single type */
> +     if ((cbdata->check_sssd == SCH_SSSD_GROUP) &&
> +         (config.search_uid || config.search_user)) {
> +             return;
> +     }
> +
> +     if ((cbdata->check_sssd == SCH_SSSD_USER) &&
> +         (config.search_gid || config.search_group)) {
> +             return;
> +     }
> +
> +     if ((config.search_gid || config.search_uid) &&
> +         (atol(config.name) < set_data->sssd_min_id)) {
> +             return;
> +     }
> +
> +     if ((config.search_group || config.search_gid) &&
> +         (NULL != config.name)) {
> +             entry = backend_retrieve_group_entry_from_sssd(config.name,
> +                                             config.search_gid, set_data, 
> cbdata);
> +             backend_search_sssd_send_entry(set_data, cbdata, entry);
> +             return;
> +     }
> +
> +     if ((config.search_user || config.search_uid) &&
> +         (NULL != config.name)) {
> +             entry = backend_retrieve_user_entry_from_sssd(config.name,
> +                                             config.search_uid, set_data, 
> cbdata);
> +             backend_search_sssd_send_entry(set_data, cbdata, entry);
> +     }
> +}
> diff --git a/src/back-sch.c b/src/back-sch.c
> index 142bdb9..a235998 100644
> --- a/src/back-sch.c
> +++ b/src/back-sch.c
> @@ -50,23 +50,10 @@
>  #include "format.h"
>  #include "plugin.h"
>  #include "map.h"
> +#include "back-sch.h"
>  
>  #define SCH_CONTAINER_CONFIGURATION_FILTER "(&(" 
> SCH_CONTAINER_CONFIGURATION_GROUP_ATTR "=*)(" 
> SCH_CONTAINER_CONFIGURATION_BASE_ATTR "=*)(" 
> SCH_CONTAINER_CONFIGURATION_FILTER_ATTR "=*)(" 
> SCH_CONTAINER_CONFIGURATION_RDN_ATTR "=*))"
>  
> -/* The data we ask the map cache to keep, for us, for each set. */
> -struct backend_set_data {
> -     struct backend_shr_set_data common;
> -     /* Schema compatibility-specific data. */
> -     Slapi_DN *container_sdn;
> -     char *rdn_format;
> -     char **attribute_format;
> -     bool_t check_access;
> -};
> -struct backend_entry_data {
> -     Slapi_DN *original_entry_dn;
> -     Slapi_Entry *e;
> -};
> -
>  /* Read the name of the NIS master. A dummy function for the schema
>   * compatibility plugin. */
>  void
> @@ -98,9 +85,18 @@ backend_set_config_free_config_contents(void *data)
>               format_free_ref_attr_list(set_data->common.ref_attr_list);
>               format_free_ref_attr_list(set_data->common.inref_attr_list);
>               free(set_data->common.entry_filter);
> +             if (set_data->check_sssd != SCH_SSSD_NONE) {
> +                     /* Remove association to another set (groups/users) */
> +                     if ((set_data->sssd_relevant_set != NULL) &&
> +                         (set_data->sssd_relevant_set->sssd_relevant_set == 
> set_data)) {
> +                             set_data->sssd_relevant_set->sssd_relevant_set 
> = NULL;
> +                             set_data->sssd_relevant_set = NULL;
> +                     }
> +             }
>               slapi_sdn_free(&set_data->container_sdn);
>               free(set_data->rdn_format);
>               backend_shr_free_strlist(set_data->attribute_format);
> +             free(set_data->sssd_buffer);
>       }
>  }
>  void
> @@ -146,6 +142,12 @@ backend_copy_set_config(const struct backend_set_data 
> *data)
>       ret->rdn_format = strdup(data->rdn_format);
>       ret->attribute_format = backend_shr_dup_strlist(data->attribute_format);
>       ret->check_access = data->check_access;
> +     ret->check_sssd = data->check_sssd;
> +     ret->sssd_min_id = data->sssd_min_id;
> +     ret->sssd_buffer = data->sssd_buffer;
> +     ret->sssd_buffer_len = data->sssd_buffer_len;
> +     ret->sssd_relevant_set = data->sssd_relevant_set;
> +
>       if ((ret->common.group == NULL) ||
>           (ret->common.set == NULL) ||
>           (ret->common.bases == NULL) ||
> @@ -164,7 +166,7 @@ backend_set_config_read_config(struct plugin_state 
> *state, Slapi_Entry *e,
>                              const char *group, const char *container,
>                              bool_t *flag, struct backend_shr_set_data **pret)
>  {
> -     char **bases, *entry_filter, **attributes, *rdn_format, *dn;
> +     char **bases, *entry_filter, **attributes, *rdn_format, *dn, 
> *sssd_min_id, *check_sssd;
>       bool_t check_access;
>       struct backend_set_data ret;
>       Slapi_DN *tmp_sdn;
> @@ -179,6 +181,10 @@ backend_set_config_read_config(struct plugin_state 
> *state, Slapi_Entry *e,
>       check_access = backend_shr_get_vattr_boolean(state, e,
>                                                    
> SCH_CONTAINER_CONFIGURATION_ACCESS_ATTR,
>                                                    TRUE);
> +     check_sssd = backend_shr_get_vattr_str(state, e,
> +                                                
> SCH_CONTAINER_CONFIGURATION_SSSD_ATTR);
> +     sssd_min_id = backend_shr_get_vattr_str(state, e,
> +                                            
> SCH_CONTAINER_CONFIGURATION_SSSD_MIN_ID_ATTR);
>       attributes = backend_shr_get_vattr_strlist(state, e,
>                                                  
> SCH_CONTAINER_CONFIGURATION_ATTR_ATTR);
>       /* Populate the returned structure. */
> @@ -213,6 +219,51 @@ backend_set_config_read_config(struct plugin_state 
> *state, Slapi_Entry *e,
>       ret.rdn_format = rdn_format;
>       ret.attribute_format = attributes;
>       ret.check_access = check_access;
> +
> +     if (check_sssd != NULL) {
> +             if (strcasecmp(check_sssd, "group") == 0) {
> +                     ret.check_sssd = SCH_SSSD_GROUP;
> +             } else if (strcasecmp(check_sssd, "user") == 0) {
> +                     ret.check_sssd = SCH_SSSD_USER;
> +             } else {
> +                     ret.check_sssd = SCH_SSSD_NONE;
> +             }
> +     } else {
> +             ret.check_sssd = SCH_SSSD_NONE;
> +     }
> +     ret.sssd_buffer = NULL;
> +
> +     /* Make sure we don't return system users/groups
> +      * by limiting lower bound on searches */
> +     ret.sssd_min_id = 1000; /* default in Fedora */
> +     if (sssd_min_id != NULL) {
> +             ret.sssd_min_id = atol(sssd_min_id);
> +     }
> +
> +     if (ret.sssd_min_id == 0) {
> +             /* enforce id in case of an error or too low limit */
> +             ret.sssd_min_id = 1000;
> +     }
> +
> +     if (ret.check_sssd != SCH_SSSD_NONE) {
> +             /* Auto-populate attributes based on selected SSSD tree
> +              * and add special attribute to track whether the entry 
> requires PAM-based bind */
> +             backend_shr_add_strlist(&ret.attribute_format, 
> "objectClass=extensibleObject");
> +             backend_shr_add_strlist(&ret.attribute_format, 
> "schema-compat-origin=%{schema-compat-origin}");
> +             backend_shr_add_strlist(&ret.attribute_format, 
> "ipaNTSecurityIdentifier=%{ipaNTSecurityIdentifier}");
> +
> +             /* Allocate buffer to be used for getpwnam_r/getgrnam_r 
> requests */
> +             if (ret.check_sssd == SCH_SSSD_USER) {
> +                     ret.sssd_buffer_len = sysconf(_SC_GETPW_R_SIZE_MAX);
> +             } else {
> +                     ret.sssd_buffer_len = sysconf(_SC_GETGR_R_SIZE_MAX);
> +             }
> +             if ((long) ret.sssd_buffer_len == -1 )
> +                     ret.sssd_buffer_len = 16384;
> +             ret.sssd_buffer = malloc(ret.sssd_buffer_len);
> +     }
> +
> +     ret.sssd_relevant_set = NULL;
>       *pret = backend_copy_set_config(&ret);
>       free(ret.common.group);
>       free(ret.common.set);
> @@ -315,7 +366,7 @@ backend_entry_get_usn(Slapi_PBlock *pb, Slapi_Entry *e,
>  }
>  
>  /* Add operational attributes to a synthetic entry. */
> -static void
> +void
>  backend_set_operational_attributes(Slapi_Entry *e,
>                                  struct plugin_state *state,
>                                  time_t timestamp,

This function's being made non-static here, but it's not being called
from any new locations.

> @@ -879,25 +930,6 @@ backend_update_params(Slapi_PBlock *pb, struct 
> plugin_state *state)
>       slapi_entry_free(our_entry);
>  }
>  
> -/* Intercept a search request, and if it belongs to one of our compatibility
> - * trees, answer from our cache before letting the default database have a
> - * crack at it. */
> -struct backend_search_cbdata {
> -     Slapi_PBlock *pb;
> -     struct plugin_state *state;
> -     char *target, *strfilter, **attrs;
> -     int scope, sizelimit, timelimit, attrsonly;
> -     bool_t check_access;
> -     Slapi_DN *target_dn;
> -     Slapi_Filter *filter;
> -
> -     bool_t answer;
> -     int result;
> -     bool_t matched;
> -     char *closest_match, *text;
> -     int n_entries;
> -};
> -
>  static bool_t
>  backend_should_descend(Slapi_DN *this_dn, Slapi_DN *target_dn, int scope)
>  {
> @@ -993,11 +1025,17 @@ backend_search_set_cb(const char *group, const char 
> *set, bool_t flag,
>       struct backend_set_data *set_data;
>       Slapi_Entry *set_entry;
>       int result, n_entries;
> +     int n_entries_sssd;
>       const char *ndn;
>  
>       cbdata = cb_data;
>       set_data = backend_data;
>       cbdata->check_access = set_data->check_access;
> +     cbdata->check_sssd = set_data->check_sssd;
> +     cbdata->sssd_min_id = set_data->sssd_min_id;
> +     /* If any entries were actually returned by the descending callback,
> +      * avoid to look up in sssd even if this set is marked to look up */
> +     n_entries_sssd = cbdata->n_entries;
>  
>       /* Check the set itself, unless it's also the group, in which case we
>        * already evaluated it for this search. */
> @@ -1054,6 +1092,15 @@ backend_search_set_cb(const char *group, const char 
> *set, bool_t flag,
>                                         backend_search_entry_cb, cbdata);
>       }
>  
> +#ifdef HAVE_SSS_NSS_IDMAP
> +     /* If we didn't find an exact match for the entry but asked to look up 
> SSSD,
> +      * then try to search SSSD and if successful, return that entry */
> +     if ((n_entries_sssd == cbdata->n_entries) &&
> +         (cbdata->check_sssd != SCH_SSSD_NONE)) {
> +             backend_search_sssd(set_data, cbdata);
> +     }
> +#endif
> +
>       /* If we didn't find an exact match for the entry, then store this
>        * container's DN as the closest match. */
>       if ((!cbdata->matched) &&
> @@ -1409,6 +1456,7 @@ backend_bind_cb(Slapi_PBlock *pb)
>       struct berval ref;
>       struct berval *urls[] = {&ref, NULL};
>       const char *ndn;
> +     char *is_sssd_origin = NULL;
>  
>       if (wrap_get_call_level() > 0) {
>               return 0;
> @@ -1418,18 +1466,59 @@ backend_bind_cb(Slapi_PBlock *pb)
>       map_rdlock();
>       backend_locate(pb, &data);
>       if (data != NULL) {
> -             ndn = slapi_sdn_get_ndn(data->original_entry_dn);
> -             ref.bv_len = strlen("ldap:///";) + strlen(ndn);
> -             ref.bv_val = malloc(ref.bv_len + 1);
> -             if (ref.bv_val != NULL) {
> -                     sprintf(ref.bv_val, "ldap:///%s";, ndn);
> -                     slapi_send_ldap_result(pb, LDAP_REFERRAL,
> -                                            NULL, NULL, 0, urls);
> -                     free(ref.bv_val);
> +#ifdef HAVE_SSS_NSS_IDMAP
> +             is_sssd_origin = slapi_entry_attr_get_charptr(data->e, 
> "schema-compat-origin");
> +             if ((is_sssd_origin != NULL) &&
> +                 (strcasecmp(is_sssd_origin, "sssd") == 0)) {
> +                     ret = backend_sch_do_pam_auth(pb, data->e);
> +                     if (ret != 0) {
> +                             slapi_send_ldap_result(pb, 
> LDAP_INVALID_CREDENTIALS,
> +                                                    NULL, NULL, 0, NULL);
> +                             ret = -1;
> +                     } else {
> +                             /*
> +                              * If bind succeeded, change authentication 
> information associated
> +                              * with this connection.
> +                              */
> +                             if (ret == LDAP_SUCCESS) {
> +                                     ndn = 
> slapi_ch_strdup(slapi_sdn_get_ndn(data->original_entry_dn));
> +                                     if ((slapi_pblock_set(pb, 
> SLAPI_CONN_DN, (void*)ndn) != 0) ||
> +                                         (slapi_pblock_set(pb, 
> SLAPI_CONN_AUTHMETHOD, SLAPD_AUTH_SIMPLE) != 0)) {
> +                                             ret = LDAP_OPERATIONS_ERROR;
> +                                     } else {
> +                                             LDAPControl **reqctrls = NULL;
> +                                             slapi_pblock_get(pb, 
> SLAPI_REQCONTROLS, &reqctrls);
> +                                             if 
> (slapi_control_present(reqctrls, LDAP_CONTROL_AUTH_REQUEST, NULL, NULL)) {
> +                                                     
> slapi_add_auth_response_control(pb, ndn);
> +                                             }
> +                                     }
> +                             }
> +
> +                             if (ret == LDAP_SUCCESS) {
> +                                     /* we are handling the result */
> +                                     slapi_send_ldap_result(pb, ret, NULL, 
> NULL, 0, NULL);
> +                                     /* tell bind code we handled the result 
> */
> +                                     ret = 0;
> +                             }
> +                     }

This leg's hanging on to the map lock while it calls into PAM.  If we
retrieved the two attributes we care about (uid and
schema-compat-origin), and kept a copy of the DN we're going to assign
if the bind succeeds, we could let go of the lock that keeps the "data"
structure from being modified before calling into PAM.

>               } else {
> -                     slapi_send_ldap_result(pb, LDAP_BUSY,
> -                                            NULL, NULL, 0, NULL);
> +#endif
> +                     ndn = slapi_sdn_get_ndn(data->original_entry_dn);
> +                     ref.bv_len = strlen("ldap:///";) + strlen(ndn);
> +                     ref.bv_val = malloc(ref.bv_len + 1);
> +                     if (ref.bv_val != NULL) {
> +                             sprintf(ref.bv_val, "ldap:///%s";, ndn);
> +                             slapi_send_ldap_result(pb, LDAP_REFERRAL,
> +                                                    NULL, NULL, 0, urls);
> +                             free(ref.bv_val);
> +                     } else {
> +                             slapi_send_ldap_result(pb, LDAP_BUSY,
> +                                                    NULL, NULL, 0, NULL);
> +                     }
> +#ifdef HAVE_SSS_NSS_IDMAP
>               }
> +             free(is_sssd_origin);
> +#endif
>               ret = -1;
>       } else {
>               if (backend_check_scope_pb(pb)) {

Please rework this function so that the conditional logic isn't
interspersed with preprocessor logic.  It's kind of hard to follow.

HTH,

Nalin

_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Reply via email to