From fc3b1e0076eb230582da854578fb5c9a54891902 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micka=C3=ABl=20Thomas?= Date: Sun, 26 Feb 2017 20:30:01 +0100 Subject: [PATCH] Compare OTP IDs against `yubi_attr` only Currently we trust the LDAP server to only return the `yubi_attr` attribute, yet we loop over all possible attributes when there should only be one. Since the bundled test LDAP server ignores the requested attributes list, we must make sure to only match against the `yubi_attr` attibute as opposed to "all of them". This also fixes an issue where AUTH_NOT_FOUND was returned instead of AUTH_NO_TOKENS when there were no values returned for `yubi_attr` but another attribute's value was considered as a candidate token. --- pam_yubico.c | 28 +++++++++++++++++++--------- 1 file changed, 19 insertions(+), 9 deletions(-) diff --git a/pam_yubico.c b/pam_yubico.c index 49f7726..5a50250 100644 --- a/pam_yubico.c +++ b/pam_yubico.c @@ -234,7 +234,7 @@ authorize_user_token_ldap (struct cfg *cfg, LDAP *ld = NULL; LDAPMessage *result = NULL, *e; BerElement *ber; - char *a; + char *attr_name; char *attrs[2] = {NULL, NULL}; struct berval **vals; @@ -338,20 +338,30 @@ authorize_user_token_ldap (struct cfg *cfg, else { /* Iterate through each returned attribute. */ - for (a = ldap_first_attribute (ld, e, &ber); - a != NULL; a = ldap_next_attribute (ld, e, ber)) + for (attr_name = ldap_first_attribute (ld, e, &ber); + attr_name != NULL; attr_name = ldap_next_attribute (ld, e, ber)) { - if ((vals = ldap_get_values_len (ld, e, a)) != NULL) + if (strcmp(attr_name, cfg->yubi_attr) != 0) { + DBG("Ignored non-requested attribute: %s", attr_name); + continue; + } + if ((vals = ldap_get_values_len (ld, e, attr_name)) != NULL) { yubi_attr_prefix_len = cfg->yubi_attr_prefix ? strlen(cfg->yubi_attr_prefix) : 0; + DBG("LDAP : Found %i values for %s - checking if any of them match '%s:%s'", + ldap_count_values_len(vals), + attr_name, + cfg->yubi_attr_prefix ? cfg->yubi_attr_prefix : "", + token_id ? token_id : "(null)"); + /* Compare each value for the attribute against the token id. */ for (i = 0; vals[i] != NULL; i++) { - DBG("LDAP : Found %i values - checking if any of them match '%s:%s:%s'", - ldap_count_values_len(vals), - vals[i]->bv_val, - cfg->yubi_attr_prefix ? cfg->yubi_attr_prefix : "", token_id); + DBG("LDAP : Checking value %i: %s:%s", + i + 1, + cfg->yubi_attr_prefix ? cfg->yubi_attr_prefix : "", + vals[i]->bv_val); /* Only values containing this prefix are considered. */ if ((!cfg->yubi_attr_prefix || !strncmp (cfg->yubi_attr_prefix, vals[i]->bv_val, yubi_attr_prefix_len))) @@ -370,7 +380,7 @@ authorize_user_token_ldap (struct cfg *cfg, } ldap_value_free_len (vals); } - ldap_memfree (a); + ldap_memfree (attr_name); } if (ber != NULL) ber_free (ber, 0);