12236 getmembers_DN doesn't properly handle errors from __ns_ldap_dn2uid

12240 nss_ldap does not properly look up group members by distinguished name
Reviewed by: Jorge Schrauwen <jorge@blackdot.be>
Reviewed by: Gordon Ross <gordon.w.ross@gmail.com>
Reviewed by: Andy Fiddaman <omnios@citrus-it.co.uk>
Reviewed by: Matt Barden <matt.barden@nexenta.com>
Approved by: Dan McDonald <danmcd@joyent.com>

4 files changed, 189 insertions, 148 deletions

diff --git a/usr/src/lib/libsldap/common/ns_internal.h b/usr/src/lib/libsldap/common/ns_internal.h
index 9a7e076c5a..f6fb0f0bb0 100644
--- a/usr/src/lib/libsldap/common/ns_internal.h
+++ b/usr/src/lib/libsldap/common/ns_internal.h
@@ -22,6 +22,7 @@
 /*
  * Copyright (c) 1999, 2010, Oracle and/or its affiliates. All rights reserved.
  * Copyright 2017 Nexenta Systems, Inc.  All rights reserved.
+ * Copyright 2020 Joyent, Inc.
  */
 
 
@@ -67,8 +68,7 @@ extern "C" {
 #define	UIDNUMFILTER_SSD	"(&(%%s)(uidnumber=%s))"
 #define	UIDFILTER		"(&(objectclass=posixAccount)(uid=%s))"
 #define	UIDFILTER_SSD		"(&(%%s)(uid=%s))"
-#define	UIDDNFILTER	"(&(objectclass=posixAccount)(distinguishedName=%s))"
-#define	UIDDNFILTER_SSD		"(&(%%s)(distinguishedName=%s))"
+#define	UIDDNFILTER		"(objectclass=posixAccount)"
 
 #define	HOSTFILTER		"(&(objectclass=ipHost)(cn=%s))"
 #define	HOSTFILTER_SSD		"(&(%%s)(cn=%s))"
@@ -765,6 +765,7 @@ int __s_api_requestServer(const char *request, const char *server,
 
 /* ************ internal sldap-api functions *********** */
 void	__ns_ldap_freeEntry(ns_ldap_entry_t *ep);
+void	__ns_ldap_freeASearchDesc(ns_ldap_search_desc_t *);
 void	__s_api_split_key_value(char *buffer, char **name, char **value);
 int	__s_api_printResult(ns_ldap_result_t *);
 int	__s_api_getSearchScope(int *, ns_ldap_error_t **);
@@ -856,25 +857,16 @@ ns_ldap_error_t *__ns_ldap_print_config(int);
 void		__ns_ldap_default_config();
 int		__ns_ldap_download(const char *, char *, char *,
 				ns_ldap_error_t **);
-int
-__ns_ldap_check_dns_preq(int foreground,
-		int mode_verbose,
-		int mode_quiet,
-		const char *fname,
-		ns_ldap_self_gssapi_config_t config,
-		ns_ldap_error_t **errpp);
-int
-__ns_ldap_check_gssapi_preq(int foreground,
-		int mode_verbose,
-		int mode_quiet,
-		ns_ldap_self_gssapi_config_t config,
-		ns_ldap_error_t **errpp);
-int
-__ns_ldap_check_all_preq(int foreground,
-		int mode_verbose,
-		int mode_quiet,
-		ns_ldap_self_gssapi_config_t config,
-		ns_ldap_error_t **errpp);
+int __ns_ldap_check_dns_preq(int foreground, int mode_verbose, int mode_quiet,
+    const char *fname, ns_ldap_self_gssapi_config_t config,
+    ns_ldap_error_t **errpp);
+
+int __ns_ldap_check_gssapi_preq(int foreground, int mode_verbose,
+    int mode_quiet, ns_ldap_self_gssapi_config_t config,
+    ns_ldap_error_t **errpp);
+
+int __ns_ldap_check_all_preq(int foreground, int mode_verbose, int mode_quiet,
+    ns_ldap_self_gssapi_config_t config, ns_ldap_error_t **errpp);
 
 /* internal un-exposed APIs */
 ns_cred_t	*__ns_ldap_dupAuth(const ns_cred_t *authp);
diff --git a/usr/src/lib/libsldap/common/ns_mapping.c b/usr/src/lib/libsldap/common/ns_mapping.c
index 7a30a090f0..af41831d41 100644
--- a/usr/src/lib/libsldap/common/ns_mapping.c
+++ b/usr/src/lib/libsldap/common/ns_mapping.c
@@ -20,6 +20,7 @@
  */
 /*
  * Copyright (c) 2001, 2010, Oracle and/or its affiliates. All rights reserved.
+ * Copyright 2020 Joyent, Inc.
  */
 
 #include <stdlib.h>
@@ -201,7 +202,7 @@ __s_api_add_map2hash(ns_config_t *config, ns_hashtype_t type,
  * Assume space is the only legal whitespace.
  * attributeMap syntax:
  * attributeMap      = serviceId ":" origAttribute "="
- * 			attributes
+ *			attributes
  * origAttribute     = attribute
  * attributes        = wattribute *( space wattribute )
  * wattribute        = whsp newAttribute whsp
@@ -210,7 +211,7 @@ __s_api_add_map2hash(ns_config_t *config, ns_hashtype_t type,
  *
  * objectclassMap syntax:
  * objectclassMap    = serviceId ":" origObjectclass "="
- * 			objectclass
+ *			objectclass
  * origObjectclass   = objectclass
  * objectclass       = keystring
  */
@@ -295,7 +296,7 @@ __s_api_parse_map(char *cp, char **sid, char **origA, char ***mapA)
 }
 
 
-static void
+void
 __ns_ldap_freeASearchDesc(ns_ldap_search_desc_t *ptr)
 {
 	if (ptr == NULL)
@@ -662,7 +663,7 @@ int __ns_ldap_getSearchDescriptors(
 	int			cnt, max;
 	int			vers;
 	ns_config_t		*cfg;
-	ns_ldap_search_desc_t 	*ret;
+	ns_ldap_search_desc_t	*ret;
 
 	if ((desc == NULL) || (errorp == NULL))
 		return (NS_LDAP_INVALID_PARAM);
diff --git a/usr/src/lib/libsldap/common/ns_reads.c b/usr/src/lib/libsldap/common/ns_reads.c
index 0d112955d9..da987fd81a 100644
--- a/usr/src/lib/libsldap/common/ns_reads.c
+++ b/usr/src/lib/libsldap/common/ns_reads.c
@@ -21,6 +21,7 @@
 /*
  * Copyright (c) 1999, 2010, Oracle and/or its affiliates. All rights reserved.
  * Copyright 2017 Nexenta Systems, Inc.  All rights reserved.
+ * Copyright 2020 Joyent, Inc.
  */
 
 #include <stdio.h>
@@ -3595,103 +3596,172 @@ __ns_ldap_list_batch_end(ns_ldap_list_batch_t *batch)
 	return (rc);
 }
 
+typedef struct lookup_data {
+	const char *lkd_dn;
+	const char *lkd_service;
+	const char *lkd_filter;
+	const ns_cred_t *lkd_cred;
+	ns_conn_user_t *lkd_user;
+} lookup_data_t;
+
 /*
- * find_domainname performs one or more LDAP searches to
- * find the value of the nisdomain attribute associated with
- * the input DN (with no retry).
+ * This creates a service search descriptor that can be used to
+ * retrieve a specific DN by using the DN as the basedn with a search
+ * scope of 'base'. We don't use any service SSDs in this instance since
+ * they are intended to search specific locations/subtrees and filter the
+ * results, while here we are wanting to retrieve a specific entry.
  */
-
 static int
-find_domainname(const char *dn, char **domainname, const ns_cred_t *cred,
-    ns_ldap_error_t **errorp, ns_conn_user_t *conn_user)
+lookup_create_ssd(lookup_data_t *dn_data, ns_ldap_search_desc_t **descpp)
 {
+	ns_ldap_search_desc_t *dptr;
+
+	*descpp = NULL;
 
+	dptr = calloc(1, sizeof (ns_ldap_search_desc_t));
+	if (dptr == NULL)
+		return (NS_LDAP_MEMORY);
+
+	dptr->basedn = strdup(dn_data->lkd_dn);
+	dptr->scope = NS_LDAP_SCOPE_BASE;
+	dptr->filter = strdup(dn_data->lkd_filter);
+
+	if (dptr->basedn == NULL || dptr->filter == NULL) {
+		__ns_ldap_freeASearchDesc(dptr);
+		return (NS_LDAP_MEMORY);
+	}
+
+	*descpp = dptr;
+	return (NS_LDAP_SUCCESS);
+}
+
+static int
+lookup_dn(lookup_data_t *dn_data, const char **attrs,
+    ns_ldap_result_t **resultp, ns_ldap_error_t **errorp)
+{
 	ns_ldap_cookie_t	*cookie;
-	ns_ldap_search_desc_t	**sdlist;
-	ns_ldap_search_desc_t	*dptr;
-	int			rc;
-	char			**value;
+	int			rc = 0;
 	int			flags = 0;
 
-	*domainname = NULL;
 	*errorp = NULL;
+	*resultp = NULL;
+
+	if (dn_data == NULL || dn_data->lkd_dn == NULL ||
+	    dn_data->lkd_dn[0] == '\0' || dn_data->lkd_filter == NULL)
+		return (NS_LDAP_INVALID_PARAM);
 
-	/* Initialize State machine cookie */
 	cookie = init_search_state_machine();
-	if (cookie == NULL) {
+	if (cookie == NULL)
 		return (NS_LDAP_MEMORY);
-	}
-	cookie->conn_user = conn_user;
 
-	/* see if need to follow referrals */
-	rc = __s_api_toFollowReferrals(flags,
-	    &cookie->followRef, errorp);
-	if (rc != NS_LDAP_SUCCESS) {
-		delete_search_cookie(cookie);
-		return (rc);
-	}
+	rc = __s_api_toFollowReferrals(flags, &cookie->followRef, errorp);
+	if (rc != NS_LDAP_SUCCESS)
+		goto out;
 
-	/* Create default service Desc */
-	sdlist = (ns_ldap_search_desc_t **)calloc(2,
-	    sizeof (ns_ldap_search_desc_t *));
-	if (sdlist == NULL) {
-		delete_search_cookie(cookie);
-		cookie = NULL;
-		return (NS_LDAP_MEMORY);
-	}
-	dptr = (ns_ldap_search_desc_t *)
-	    calloc(1, sizeof (ns_ldap_search_desc_t));
-	if (dptr == NULL) {
-		free(sdlist);
-		delete_search_cookie(cookie);
-		cookie = NULL;
-		return (NS_LDAP_MEMORY);
+	/* 1 for SSD, 1 for terminating NULL */
+	cookie->sdlist = calloc(2, sizeof (ns_ldap_search_desc_t *));
+	if (cookie->sdlist == NULL) {
+		rc = NS_LDAP_MEMORY;
+		goto out;
 	}
-	sdlist[0] = dptr;
 
-	/* search base is dn */
-	dptr->basedn = strdup(dn);
-
-	/* search scope is base */
-	dptr->scope = NS_LDAP_SCOPE_BASE;
+	rc = lookup_create_ssd(dn_data, &cookie->sdlist[0]);
+	if (rc != NS_LDAP_SUCCESS)
+		goto out;
 
-	/* search filter is "nisdomain=*" */
-	dptr->filter = strdup(_NIS_FILTER);
+	if (dn_data->lkd_service != NULL) {
+		/*
+		 * If a service was specified, set that on the cookie so
+		 * that search_state_machine() will properly map
+		 * attributes and objectclasses.
+		 */
+		cookie->service = strdup(dn_data->lkd_service);
+		if (cookie->service == NULL) {
+			rc = NS_LDAP_MEMORY;
+			goto out;
+		}
+	}
 
-	cookie->sdlist = sdlist;
-	cookie->i_filter = strdup(dptr->filter);
-	cookie->i_attr = nis_domain_attrs;
-	cookie->i_auth = cred;
+	cookie->i_attr = attrs;
+	cookie->i_auth = dn_data->lkd_cred;
 	cookie->i_flags = 0;
+	cookie->i_filter = strdup(dn_data->lkd_filter);
+	if (cookie->i_filter == NULL) {
+		rc = NS_LDAP_MEMORY;
+		goto out;
+	}
 
-	/* Process search */
-	rc = search_state_machine(cookie, INIT, 0);
-
-	/* Copy domain name if found */
+	/*
+	 * Actually perform the search. The return value is only used when
+	 * iterating through multiple results. Since we are searching with
+	 * a scope of base, we will always get at most one result entry,
+	 * we ignore the return value and look at err_rc to determine if
+	 * there were any errors.
+	 */
+	(void) search_state_machine(cookie, INIT, 0);
 	rc = cookie->err_rc;
+
 	if (rc != NS_LDAP_SUCCESS) {
-		if (conn_user != NULL && conn_user->ns_error != NULL) {
-			*errorp = conn_user->ns_error;
-			conn_user->ns_error = NULL;
+		ns_conn_user_t *user = dn_data->lkd_user;
+
+		if (user != NULL && user->ns_error != NULL) {
+			*errorp = user->ns_error;
+			user->ns_error = NULL;
 		} else {
 			*errorp = cookie->errorp;
+			cookie->errorp = NULL;
 		}
-	}
-	if (cookie->result == NULL)
+	} else if (cookie->result != NULL) {
+		*resultp = cookie->result;
+		cookie->result = NULL;
+	} else {
 		rc = NS_LDAP_NOTFOUND;
-	if (rc == NS_LDAP_SUCCESS) {
-		value = __ns_ldap_getAttr(cookie->result->entry,
-		    _NIS_DOMAIN);
-		if (value[0])
-			*domainname = strdup(value[0]);
-		else
-			rc = NS_LDAP_NOTFOUND;
 	}
-	if (cookie->result != NULL)
-		(void) __ns_ldap_freeResult(&cookie->result);
-	cookie->errorp = NULL;
+
+out:
 	delete_search_cookie(cookie);
-	cookie = NULL;
+	return (rc);
+}
+
+/*
+ * find_domainname performs one or more LDAP searches to
+ * find the value of the nisdomain attribute associated with
+ * the input DN (with no retry).
+ */
+
+static int
+find_domainname(const char *dn, char **domainname, const ns_cred_t *cred,
+    ns_ldap_error_t **errorp, ns_conn_user_t *conn_user)
+{
+	lookup_data_t		ldata;
+	ns_ldap_result_t	*result;
+	char			**value;
+	int			rc;
+
+	*domainname = NULL;
+	*errorp = NULL;
+
+	ldata.lkd_dn = dn;
+	ldata.lkd_service = NULL;
+	ldata.lkd_filter = _NIS_FILTER;
+	ldata.lkd_cred = cred;
+	ldata.lkd_user = conn_user;
+
+	rc = lookup_dn(&ldata, nis_domain_attrs, &result, errorp);
+	if (rc != NS_LDAP_SUCCESS)
+		return (rc);
+
+	value = __ns_ldap_getAttr(result->entry, _NIS_DOMAIN);
+
+	if (value != NULL && value[0] != NULL) {
+		*domainname = strdup(value[0]);
+		if (*domainname == NULL)
+			rc = NS_LDAP_MEMORY;
+	} else {
+		rc = NS_LDAP_NOTFOUND;
+	}
+
+	(void) __ns_ldap_freeResult(&result);
 	return (rc);
 }
 
@@ -4239,73 +4309,51 @@ static const char *dn2uid_attrs[] = {
 };
 
 int
-__ns_ldap_dn2uid(const char *dn, char **userID, const ns_cred_t *cred,
+__ns_ldap_dn2uid(const char *dn, char **userIDp, const ns_cred_t *cred,
     ns_ldap_error_t **errorp)
 {
-	ns_ldap_result_t	*result = NULL;
-	char		*filter, *userdata;
-	char		errstr[MAXERROR];
-	char		**value;
-	int		rc = 0;
-	size_t		len;
+	lookup_data_t		ldata;
+	ns_ldap_result_t	*result;
+	char			**value;
+	int			rc;
 
 	*errorp = NULL;
-	*userID = NULL;
+	*userIDp = NULL;
 	if ((dn == NULL) || (dn[0] == '\0'))
 		return (NS_LDAP_INVALID_PARAM);
 
-	len = strlen(UIDDNFILTER) + strlen(dn) + 1;
-	filter = malloc(len);
-	if (filter == NULL) {
-		return (NS_LDAP_MEMORY);
-	}
-	(void) snprintf(filter, len, UIDDNFILTER, dn);
-
-	len = strlen(UIDDNFILTER_SSD) + strlen(dn) + 1;
-	userdata = malloc(len);
-	if (userdata == NULL) {
-		free(filter);
-		return (NS_LDAP_MEMORY);
-	}
-	(void) snprintf(userdata, len, UIDDNFILTER_SSD, dn);
+	/*
+	 * Many LDAP servers do not support using the dn in a search
+	 * filter. As a result, we unfortunately cannot  use __ns_ldap_list()
+	 * to lookup the DN. Instead we perform a search with the baseDN
+	 * being the DN we are looking for with a scope of 'base' to
+	 * return the entry, as this should be supported by all LDAP servers.
+	 */
+	ldata.lkd_dn = dn;
 
 	/*
-	 * Unlike uid2dn, we DO want attribute mapping, so that
-	 * "uid" is mapped to/from samAccountName, for example.
+	 * Since we are looking up a user account by its DN, use the attribute
+	 * and objectclass mappings (if present) for the passwd service.
 	 */
-	rc = __ns_ldap_list("passwd", filter,
-	    __s_api_merge_SSD_filter,
-	    dn2uid_attrs, cred, 0,
-	    &result, errorp, NULL,
-	    userdata);
-	free(filter);
-	filter = NULL;
-	free(userdata);
-	userdata = NULL;
-	if (rc != NS_LDAP_SUCCESS)
-		goto out;
+	ldata.lkd_service = "passwd";
+	ldata.lkd_filter = UIDDNFILTER;
+	ldata.lkd_cred = cred;
+	ldata.lkd_user = NULL;
 
-	if (result->entries_count > 1) {
-		(void) sprintf(errstr,
-		    gettext("Too many entries are returned for %s"), dn);
-		MKERROR(LOG_WARNING, *errorp, NS_LDAP_INTERNAL, strdup(errstr),
-		    NS_LDAP_MEMORY);
-		rc = NS_LDAP_INTERNAL;
-		goto out;
-	}
+	rc = lookup_dn(&ldata, dn2uid_attrs, &result, errorp);
+	if (rc != NS_LDAP_SUCCESS)
+		return (rc);
 
 	value = __ns_ldap_getAttr(result->entry, _P_UID);
-	if (value == NULL || value[0] == NULL) {
+	if (value != NULL && value[0] != NULL) {
+		*userIDp = strdup(value[0]);
+		if (*userIDp == NULL)
+			rc = NS_LDAP_MEMORY;
+	} else {
 		rc = NS_LDAP_NOTFOUND;
-		goto out;
 	}
 
-	*userID = strdup(value[0]);
-	rc = NS_LDAP_SUCCESS;
-
-out:
 	(void) __ns_ldap_freeResult(&result);
-	result = NULL;
 	return (rc);
 }
 
diff --git a/usr/src/lib/nsswitch/ldap/common/getgrent.c b/usr/src/lib/nsswitch/ldap/common/getgrent.c
index 184891c9d3..765059e86a 100644
--- a/usr/src/lib/nsswitch/ldap/common/getgrent.c
+++ b/usr/src/lib/nsswitch/ldap/common/getgrent.c
@@ -23,6 +23,7 @@
  * Use is subject to license terms.
  *
  * Copyright 2017 Nexenta Systems, Inc.  All rights reserved.
+ * Copyright 2020 Joyent, Inc.
  */
 
 #include <grp.h>
@@ -239,7 +240,7 @@ getmembers_DN(char **bufpp, int *lenp, ns_ldap_attr_t *members)
 	char	*buffer;
 	int	buflen;
 	int	i, len;
-	int	nss_result = 0;
+	int	nss_result = 0; /* used by TEST_AND_ADJUST macro */
 	int	firsttime;
 
 	buffer = *bufpp;
@@ -263,9 +264,8 @@ getmembers_DN(char **bufpp, int *lenp, ns_ldap_attr_t *members)
 		if (member_dn[0] == '\0')
 			continue;
 
-		nss_result = __ns_ldap_dn2uid(member_dn,
-		    &member_uid, NULL, &error);
-		if (nss_result != NS_LDAP_SUCCESS) {
+		if (__ns_ldap_dn2uid(member_dn,
+		    &member_uid, NULL, &error) != NS_LDAP_SUCCESS) {
 			(void) __ns_ldap_freeError(&error);
 			error = NULL;
 			continue;