From 73c1b04306d5291abde9857b51e6aaee936e57c4 Mon Sep 17 00:00:00 2001
From: "Todd C. Miller" <Todd.Miller@sudo.ws>
Date: Wed, 3 Jun 2020 20:12:04 -0600
Subject: [PATCH] When converting LDAP to sudoers, ignore entries with no
 sudoHost attribute. Otherwise, sudo_ldap_role_to_priv() will treat a NULL
 host list as as the "ALL" wildcard.  This regression was introduced in sudo
 1.8.23, which was the first version to convert LDAP sudoRole objects to
 sudoers internal data structures. Thanks to Andreas Mueller for reporting and
 debugging this problem.

---
 plugins/sudoers/ldap.c |  203 +++++++++++++++++++++++++-----------------
 plugins/sudoers/sssd.c |  236 ++++++++++++++++++++++++++-----------------------
 2 files changed, 247 insertions(+), 192 deletions(-)

--- a/plugins/sudoers/ldap.c
+++ b/plugins/sudoers/ldap.c
@@ -372,16 +372,21 @@ sudo_ldap_check_non_unix_group(LDAP *ld,
  * Extract the dn from an entry and return the first rdn from it.
  */
 static char *
-sudo_ldap_get_first_rdn(LDAP *ld, LDAPMessage *entry)
+sudo_ldap_get_first_rdn(LDAP *ld, LDAPMessage *entry, int *rc)
 {
 #ifdef HAVE_LDAP_STR2DN
     char *dn, *rdn = NULL;
     LDAPDN tmpDN;
-    debug_decl(sudo_ldap_get_first_rdn, SUDOERS_DEBUG_LDAP)
+    debug_decl(sudo_ldap_get_first_rdn, SUDOERS_DEBUG_LDAP);
 
-    if ((dn = ldap_get_dn(ld, entry)) == NULL)
+    if ((dn = ldap_get_dn(ld, entry)) == NULL) {
+	int optrc = ldap_get_option(ld, LDAP_OPT_RESULT_CODE, rc);
+	if (optrc != LDAP_OPT_SUCCESS)
+	    *rc = optrc;
 	debug_return_str(NULL);
-    if (ldap_str2dn(dn, &tmpDN, LDAP_DN_FORMAT_LDAP) == LDAP_SUCCESS) {
+    }
+    *rc = ldap_str2dn(dn, &tmpDN, LDAP_DN_FORMAT_LDAP);
+    if (*rc == LDAP_SUCCESS) {
 	ldap_rdn2str(tmpDN[0], &rdn, LDAP_DN_FORMAT_UFN);
 	ldap_dnfree(tmpDN);
     }
@@ -389,13 +394,22 @@ sudo_ldap_get_first_rdn(LDAP *ld, LDAPMe
     debug_return_str(rdn);
 #else
     char *dn, **edn;
-    debug_decl(sudo_ldap_get_first_rdn, SUDOERS_DEBUG_LDAP)
+    debug_decl(sudo_ldap_get_first_rdn, SUDOERS_DEBUG_LDAP);
 
-    if ((dn = ldap_get_dn(ld, entry)) == NULL)
+    if ((dn = ldap_get_dn(ld, entry)) == NULL) {
+	int optrc = ldap_get_option(ld, LDAP_OPT_RESULT_CODE, rc);
+	if (optrc != LDAP_OPT_SUCCESS)
+	    *rc = optrc;
 	debug_return_str(NULL);
+    }
     edn = ldap_explode_dn(dn, 1);
     ldap_memfree(dn);
-    debug_return_str(edn ? edn[0] : NULL);
+    if (edn == NULL) {
+	*rc = LDAP_NO_MEMORY;
+	debug_return_str(NULL);
+    }
+    *rc = LDAP_SUCCESS;
+    debug_return_str(edn[0]);
 #endif
 }
 
@@ -414,13 +428,21 @@ sudo_ldap_parse_options(LDAP *ld, LDAPMe
 
     bv = sudo_ldap_get_values_len(ld, entry, "sudoOption", &rc);
     if (bv == NULL) {
-	if (rc == LDAP_NO_MEMORY)
+	if (rc == LDAP_NO_MEMORY) {
+	    sudo_warnx(U_("%s: %s"), __func__, U_("unable to allocate memory"));
 	    debug_return_bool(false);
+	}
 	debug_return_bool(true);
     }
 
     /* Use sudoRole in place of file name in defaults. */
-    cn = sudo_ldap_get_first_rdn(ld, entry);
+    cn = sudo_ldap_get_first_rdn(ld, entry, &rc);
+    if (cn == NULL) {
+	if (rc == LDAP_NO_MEMORY) {
+	    sudo_warnx(U_("%s: %s"), __func__, U_("unable to allocate memory"));
+	    goto done;
+	}
+    }
     if (asprintf(&cp, "sudoRole %s", cn ? cn : "UNKNOWN") == -1) {
 	sudo_warnx(U_("%s: %s"), __func__, U_("unable to allocate memory"));
 	goto done;
@@ -1176,6 +1198,90 @@ berval_iter(void **vp)
     return *bv ? (*bv)->bv_val : NULL;
 }
 
+/*
+ * Wrapper for sudo_ldap_role_to_priv() that takes an LDAPMessage.
+ * Returns a struct privilege on success or NULL on failure.
+ */
+static struct privilege *
+ldap_entry_to_priv(LDAP *ld, LDAPMessage *entry, int *rc_out)
+{
+    struct berval **cmnds = NULL, **hosts = NULL;
+    struct berval **runasusers = NULL, **runasgroups = NULL;
+    struct berval **opts = NULL, **notbefore = NULL, **notafter = NULL;
+    struct privilege *priv = NULL;
+    char *cn = NULL;
+    int rc;
+    debug_decl(ldap_entry_to_priv, SUDOERS_DEBUG_LDAP);
+
+    /* Ignore sudoRole without sudoCommand or sudoHost. */
+    cmnds = sudo_ldap_get_values_len(ld, entry, "sudoCommand", &rc);
+    if (cmnds == NULL)
+	goto cleanup;
+    hosts = sudo_ldap_get_values_len(ld, entry, "sudoHost", &rc);
+    if (hosts == NULL)
+	goto cleanup;
+
+    /* Get the entry's dn for long format printing. */
+    if ((cn = sudo_ldap_get_first_rdn(ld, entry, &rc)) == NULL)
+	goto cleanup;
+
+    /* Get sudoRunAsUser / sudoRunAsGroup */
+    runasusers = sudo_ldap_get_values_len(ld, entry, "sudoRunAsUser", &rc);
+    if (runasusers == NULL) {
+	if (rc != LDAP_NO_MEMORY)
+	    runasusers = sudo_ldap_get_values_len(ld, entry, "sudoRunAs", &rc);
+	if (rc == LDAP_NO_MEMORY)
+	    goto cleanup;
+    }
+    runasgroups = sudo_ldap_get_values_len(ld, entry, "sudoRunAsGroup", &rc);
+    if (rc == LDAP_NO_MEMORY)
+	goto cleanup;
+
+    /* Get sudoNotBefore / sudoNotAfter */
+    if (ldap_conf.timed) {
+	notbefore = sudo_ldap_get_values_len(ld, entry, "sudoNotBefore", &rc);
+	if (rc == LDAP_NO_MEMORY)
+	    goto cleanup;
+	notafter = sudo_ldap_get_values_len(ld, entry, "sudoNotAfter", &rc);
+	if (rc == LDAP_NO_MEMORY)
+	    goto cleanup;
+    }
+
+    /* Parse sudoOptions. */
+    opts = sudo_ldap_get_values_len(ld, entry, "sudoOption", &rc);
+    if (rc == LDAP_NO_MEMORY)
+	goto cleanup;
+
+    priv = sudo_ldap_role_to_priv(cn, hosts, runasusers, runasgroups,
+	cmnds, opts, notbefore ? notbefore[0]->bv_val : NULL,
+	notafter ? notafter[0]->bv_val : NULL, false, true, berval_iter);
+    if (priv == NULL) {
+	rc = LDAP_NO_MEMORY;
+	goto cleanup;
+    }
+
+cleanup:
+    if (cn != NULL)
+	ldap_memfree(cn);
+    if (cmnds != NULL)
+	ldap_value_free_len(cmnds);
+    if (hosts != NULL)
+	ldap_value_free_len(hosts);
+    if (runasusers != NULL)
+	ldap_value_free_len(runasusers);
+    if (runasgroups != NULL)
+	ldap_value_free_len(runasgroups);
+    if (opts != NULL)
+	ldap_value_free_len(opts);
+    if (notbefore != NULL)
+	ldap_value_free_len(notbefore);
+    if (notafter != NULL)
+	ldap_value_free_len(notafter);
+
+    *rc_out = rc;
+    debug_return_ptr(priv);
+}
+
 static bool
 ldap_to_sudoers(LDAP *ld, struct ldap_result *lres,
     struct userspec_list *ldap_userspecs)
@@ -1200,83 +1306,16 @@ ldap_to_sudoers(LDAP *ld, struct ldap_re
     m->type = ALL;
     TAILQ_INSERT_TAIL(&us->users, m, entries);
 
-    /* Treat each sudoRole as a separate privilege. */
+    /* Treat each entry as a separate privilege. */
     for (i = 0; i < lres->nentries; i++) {
-	LDAPMessage *entry = lres->entries[i].entry;
-	struct berval **cmnds = NULL, **hosts = NULL;
-	struct berval **runasusers = NULL, **runasgroups = NULL;
-	struct berval **opts = NULL, **notbefore = NULL, **notafter = NULL;
-	struct privilege *priv = NULL;
-	char *cn = NULL;
-
-	/* Ignore sudoRole without sudoCommand. */
-	cmnds = sudo_ldap_get_values_len(ld, entry, "sudoCommand", &rc);
-	if (cmnds == NULL) {
-	    if (rc == LDAP_NO_MEMORY)
-		goto cleanup;
-	    continue;
-	}
-
-	/* Get the entry's dn for long format printing. */
-	if ((cn = sudo_ldap_get_first_rdn(ld, entry)) == NULL)
-	    goto cleanup;
-
-	/* Get sudoHost */
-	hosts = sudo_ldap_get_values_len(ld, entry, "sudoHost", &rc);
-	if (rc == LDAP_NO_MEMORY)
-	    goto cleanup;
+	struct privilege *priv;
 
-	/* Get sudoRunAsUser / sudoRunAsGroup */
-	runasusers = sudo_ldap_get_values_len(ld, entry, "sudoRunAsUser", &rc);
-	if (runasusers == NULL) {
-	    if (rc != LDAP_NO_MEMORY)
-		runasusers = sudo_ldap_get_values_len(ld, entry, "sudoRunAs", &rc);
+	priv = ldap_entry_to_priv(ld, lres->entries[i].entry, &rc);
+	if (priv == NULL) {
 	    if (rc == LDAP_NO_MEMORY)
-		goto cleanup;
-	}
-	runasgroups = sudo_ldap_get_values_len(ld, entry, "sudoRunAsGroup", &rc);
-	if (rc == LDAP_NO_MEMORY)
-	    goto cleanup;
-
-	/* Get sudoNotBefore / sudoNotAfter */
-	if (ldap_conf.timed) {
-	  notbefore = sudo_ldap_get_values_len(ld, entry, "sudoNotBefore", &rc);
-	  if (rc == LDAP_NO_MEMORY)
-	    goto cleanup;
-	  notafter = sudo_ldap_get_values_len(ld, entry, "sudoNotAfter", &rc);
-	  if (rc == LDAP_NO_MEMORY)
-	    goto cleanup;
+		goto oom;
+	    continue;
 	}
-
-	/* Parse sudoOptions. */
-	opts = sudo_ldap_get_values_len(ld, entry, "sudoOption", &rc);
-	if (rc == LDAP_NO_MEMORY)
-	    goto cleanup;
-
-	priv = sudo_ldap_role_to_priv(cn, hosts, runasusers, runasgroups,
-	    cmnds, opts, notbefore ? notbefore[0]->bv_val : NULL,
-	    notafter ? notafter[0]->bv_val : NULL, false, true, berval_iter);
-
-    cleanup:
-	if (cn != NULL)
-	    ldap_memfree(cn);
-	if (cmnds != NULL)
-	    ldap_value_free_len(cmnds);
-	if (hosts != NULL)
-	    ldap_value_free_len(hosts);
-	if (runasusers != NULL)
-	    ldap_value_free_len(runasusers);
-	if (runasgroups != NULL)
-	    ldap_value_free_len(runasgroups);
-	if (opts != NULL)
-	    ldap_value_free_len(opts);
-	if (notbefore != NULL)
-	    ldap_value_free_len(notbefore);
-	if (notafter != NULL)
-	    ldap_value_free_len(notafter);
-
-	if (priv == NULL)
-	    goto oom;
 	TAILQ_INSERT_TAIL(&us->privileges, priv, entries);
     }
 
--- a/plugins/sudoers/sssd.c
+++ b/plugins/sudoers/sssd.c
@@ -238,6 +238,126 @@ val_array_iter(void **vp)
     return *val_array;
 }
 
+/*
+ * Wrapper for sudo_ldap_role_to_priv() that takes an sss rule..
+ * Returns a struct privilege on success or NULL on failure.
+ */
+static struct privilege *
+sss_rule_to_priv(struct sudo_sss_handle *handle, struct sss_sudo_rule *rule,
+    int *rc_out)
+{
+    char **cmnds, **runasusers = NULL, **runasgroups = NULL;
+    char **opts = NULL, **notbefore = NULL, **notafter = NULL;
+    char **hosts = NULL, **cn_array = NULL, *cn = NULL;
+    struct privilege *priv = NULL;
+    int rc;
+    debug_decl(sss_rule_to_priv, SUDOERS_DEBUG_SSSD);
+
+    /* Ignore sudoRole without sudoCommand or sudoHost. */
+    rc = handle->fn_get_values(rule, "sudoCommand", &cmnds);
+    if (rc != 0)
+	goto cleanup;
+    rc = handle->fn_get_values(rule, "sudoHost", &hosts);
+    if (rc != 0)
+	goto cleanup;
+
+    /* Get the entry's dn for long format printing. */
+    rc = handle->fn_get_values(rule, "cn", &cn_array);
+    if (rc != 0)
+	goto cleanup;
+    cn = cn_array[0];
+
+    /* Get sudoRunAsUser / sudoRunAs */
+    rc = handle->fn_get_values(rule, "sudoRunAsUser", &runasusers);
+    switch (rc) {
+    case 0:
+	break;
+    case ENOENT:
+	rc = handle->fn_get_values(rule, "sudoRunAs", &runasusers);
+	switch (rc) {
+	    case 0:
+	    case ENOENT:
+		break;
+	    default:
+		goto cleanup;
+	}
+	break;
+    default:
+	goto cleanup;
+    }
+
+    /* Get sudoRunAsGroup */
+    rc = handle->fn_get_values(rule, "sudoRunAsGroup", &runasgroups);
+    switch (rc) {
+    case 0:
+    case ENOENT:
+	break;
+    default:
+	goto cleanup;
+    }
+
+    /* Get sudoNotBefore */
+    rc = handle->fn_get_values(rule, "sudoNotBefore", &notbefore);
+    switch (rc) {
+    case 0:
+    case ENOENT:
+	break;
+    default:
+	goto cleanup;
+    }
+
+    /* Get sudoNotAfter */
+    rc = handle->fn_get_values(rule, "sudoNotAfter", &notafter);
+    switch (rc) {
+    case 0:
+    case ENOENT:
+	break;
+    default:
+	goto cleanup;
+    }
+
+    /* Parse sudoOptions. */
+    rc = handle->fn_get_values(rule, "sudoOption", &opts);
+    switch (rc) {
+    case 0:
+    case ENOENT:
+	break;
+    default:
+	goto cleanup;
+    }
+
+    priv = sudo_ldap_role_to_priv(cn, hosts, runasusers, runasgroups,
+	cmnds, opts, notbefore ? notbefore[0] : NULL,
+	notafter ? notafter[0] : NULL, false, true, val_array_iter);
+    if (priv == NULL) {
+	rc = ENOMEM;
+	goto cleanup;
+    }
+    rc = 0;
+
+cleanup:
+    if (cn_array != NULL)
+	handle->fn_free_values(cn_array);
+    if (cmnds != NULL)
+	handle->fn_free_values(cmnds);
+    if (hosts != NULL)
+	handle->fn_free_values(hosts);
+    if (runasusers != NULL)
+	handle->fn_free_values(runasusers);
+    if (runasgroups != NULL)
+	handle->fn_free_values(runasgroups);
+    if (opts != NULL)
+	handle->fn_free_values(opts);
+    if (notbefore != NULL)
+	handle->fn_free_values(notbefore);
+    if (notafter != NULL)
+	handle->fn_free_values(notafter);
+
+    *rc_out = rc;
+
+    debug_return_ptr(priv);
+}
+
 static bool
 sss_to_sudoers(struct sudo_sss_handle *handle,
     struct sss_sudo_result *sss_result)
@@ -262,7 +382,7 @@ sss_to_sudoers(struct sudo_sss_handle *h
     TAILQ_INSERT_TAIL(&us->users, m, entries);
 
     /*
-     * Treat each sudoRole as a separate privilege.
+     * Treat each rule as a separate privilege.
      *
      * Sssd has already sorted the rules in descending order.
      * The conversion to a sudoers parse tree requires that entries be
@@ -270,10 +390,8 @@ sss_to_sudoers(struct sudo_sss_handle *h
      */
     for (i = sss_result->num_rules; i-- > 0; ) {
 	struct sss_sudo_rule *rule = sss_result->rules + i;
-	char **cmnds, **runasusers = NULL, **runasgroups = NULL;
-	char **opts = NULL, **notbefore = NULL, **notafter = NULL;
-	char **hosts = NULL, **cn_array = NULL, *cn = NULL;
-	struct privilege *priv = NULL;
+	struct privilege *priv;
+	int rc;
 
 	/*
 	 * We don't know whether a rule was included due to a user/group
@@ -282,113 +400,11 @@ sss_to_sudoers(struct sudo_sss_handle *h
 	if (!sudo_sss_check_user(handle, rule))
 	    continue;
 
-	switch (handle->fn_get_values(rule, "sudoCommand", &cmnds)) {
-	case 0:
-	    break;
-	case ENOENT:
-	    /* Ignore sudoRole without sudoCommand. */
+	if ((priv = sss_rule_to_priv(handle, rule, &rc)) == NULL) {
+	    if (rc == ENOMEM)
+		goto oom;
 	    continue;
-	default:
-	    goto cleanup;
 	}
-
-	/* Get the entry's dn for long format printing. */
-	switch (handle->fn_get_values(rule, "cn", &cn_array)) {
-	case 0:
-	    cn = cn_array[0];
-	    break;
-	case ENOENT:
-	    break;
-	default:
-	    goto cleanup;
-	}
-
-	/* Get sudoHost */
-	switch (handle->fn_get_values(rule, "sudoHost", &hosts)) {
-	case 0:
-	case ENOENT:
-	    break;
-	default:
-	    goto cleanup;
-	}
-
-	/* Get sudoRunAsUser / sudoRunAs */
-	switch (handle->fn_get_values(rule, "sudoRunAsUser", &runasusers)) {
-	case 0:
-	    break;
-	case ENOENT:
-	    switch (handle->fn_get_values(rule, "sudoRunAs", &runasusers)) {
-		case 0:
-		case ENOENT:
-		    break;
-		default:
-		    goto cleanup;
-	    }
-	    break;
-	default:
-	    goto cleanup;
-	}
-
-	/* Get sudoRunAsGroup */
-	switch (handle->fn_get_values(rule, "sudoRunAsGroup", &runasgroups)) {
-	case 0:
-	case ENOENT:
-	    break;
-	default:
-	    goto cleanup;
-	}
-
-	/* Get sudoNotBefore */
-	switch (handle->fn_get_values(rule, "sudoNotBefore", &notbefore)) {
-	case 0:
-	case ENOENT:
-	    break;
-	default:
-	    goto cleanup;
-	}
-
-	/* Get sudoNotAfter */
-	switch (handle->fn_get_values(rule, "sudoNotAfter", &notafter)) {
-	case 0:
-	case ENOENT:
-	    break;
-	default:
-	    goto cleanup;
-	}
-
-	/* Parse sudoOptions. */
-	switch (handle->fn_get_values(rule, "sudoOption", &opts)) {
-	case 0:
-	case ENOENT:
-	    break;
-	default:
-	    goto cleanup;
-	}
-
-	priv = sudo_ldap_role_to_priv(cn, hosts, runasusers, runasgroups,
-	    cmnds, opts, notbefore ? notbefore[0] : NULL,
-	    notafter ? notafter[0] : NULL, false, true, val_array_iter);
-
-    cleanup:
-	if (cn_array != NULL)
-	    handle->fn_free_values(cn_array);
-	if (cmnds != NULL)
-	    handle->fn_free_values(cmnds);
-	if (hosts != NULL)
-	    handle->fn_free_values(hosts);
-	if (runasusers != NULL)
-	    handle->fn_free_values(runasusers);
-	if (runasgroups != NULL)
-	    handle->fn_free_values(runasgroups);
-	if (opts != NULL)
-	    handle->fn_free_values(opts);
-	if (notbefore != NULL)
-	    handle->fn_free_values(notbefore);
-	if (notafter != NULL)
-	    handle->fn_free_values(notafter);
-
-	if (priv == NULL)
-	    goto oom;
 	TAILQ_INSERT_TAIL(&us->privileges, priv, entries);
     }
 
