From 6a0c8fc82b11eda69defa5497aa5385520a0c605 Mon Sep 17 00:00:00 2001 From: Fredrik Thulin Date: Thu, 3 Mar 2011 10:11:16 +0100 Subject: [PATCH 1/7] authorize_user_token_ldap: Use correct LDAP free function. Patch by judas.iscariote. --- pam_yubico.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pam_yubico.c b/pam_yubico.c index f7a8fd3..30a2471 100644 --- a/pam_yubico.c +++ b/pam_yubico.c @@ -304,7 +304,7 @@ authorize_user_token_ldap (const char *ldap_uri, retval = 1; } } - ldap_value_free (vals); + ldap_value_free_len (vals); } ldap_memfree (a); } From 90a7fd0f0a34957e3d1848159257b8f7c2957352 Mon Sep 17 00:00:00 2001 From: Fredrik Thulin Date: Thu, 3 Mar 2011 10:19:55 +0100 Subject: [PATCH 2/7] Avoid LDAP warnings about deprecated functions. Patch by judas.iscariote. --- pam_yubico.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/pam_yubico.c b/pam_yubico.c index 30a2471..21dc98e 100644 --- a/pam_yubico.c +++ b/pam_yubico.c @@ -66,6 +66,13 @@ #include #ifdef HAVE_LIBLDAP +/* Some functions like ldap_init, ldap_simple_bind_s, ldap_unbind are + deprecated but still available. We will drop support for 'ldapserver' + (in favour of 'ldap_uri' and update to using the new functions instead + soon. +*/ +#define LDAP_DEPRECATED 1 + #include #define PORT_NUMBER LDAP_PORT #endif From 01897ebb9ef00e81c1ce56a2db93bf5be9fa7475 Mon Sep 17 00:00:00 2001 From: Fredrik Thulin Date: Thu, 3 Mar 2011 10:31:30 +0100 Subject: [PATCH 3/7] Use LDAPv3 instead of LDAPv2. LDAPv2 was declared historical in 2003, and is now not supported by for example Mac OS X Server's Open Directory. Patch by maxsanna81@gmail.com. --- pam_yubico.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/pam_yubico.c b/pam_yubico.c index 21dc98e..ebadfc2 100644 --- a/pam_yubico.c +++ b/pam_yubico.c @@ -220,6 +220,7 @@ authorize_user_token_ldap (const char *ldap_uri, D(("called")); int retval = 0; + int protocol; #ifdef HAVE_LIBLDAP LDAP *ld; LDAPMessage *result, *e; @@ -272,6 +273,10 @@ authorize_user_token_ldap (const char *ldap_uri, } } + /* LDAPv2 is historical -- RFC3494. */ + protocol = LDAP_VERSION3; + ldap_set_option (ld, LDAP_OPT_PROTOCOL_VERSION, &protocol); + /* Bind anonymously to the LDAP server. */ rc = ldap_simple_bind_s (ld, NULL, NULL); if (rc != LDAP_SUCCESS) From bfd8efd68291860e60939211531a24116bea49f5 Mon Sep 17 00:00:00 2001 From: Fredrik Thulin Date: Thu, 3 Mar 2011 10:58:34 +0100 Subject: [PATCH 4/7] Don't segfault on unset LDAP parameters. When ldapserver / ldap_uri was specified, but not for example user_attr, authorize_user_token_ldap() used to cause a segmentation fault. --- pam_yubico.c | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/pam_yubico.c b/pam_yubico.c index ebadfc2..bb0ff2c 100644 --- a/pam_yubico.c +++ b/pam_yubico.c @@ -230,6 +230,19 @@ authorize_user_token_ldap (const char *ldap_uri, struct berval **vals; int i, rc; + if (user_attr == NULL) { + D (("Trying to look up user to YubiKey mapping in LDAP, but user_attr not set!")); + return 0; + } + if (yubi_attr == NULL) { + D (("Trying to look up user to YubiKey mapping in LDAP, but yubi_attr not set!")); + return 0; + } + if (ldapdn == NULL) { + D (("Trying to look up user to YubiKey mapping in LDAP, but ldapdn not set!")); + return 0; + } + /* Allocation of memory for search strings depending on input size */ char *find = malloc((strlen(user_attr)+strlen(ldapdn)+strlen(user)+3)*sizeof(char)); char *sr = malloc((strlen(yubi_attr)+4)*sizeof(char)); From 0bb1630abf7776dc067feaaaf8e7be05efb4f833 Mon Sep 17 00:00:00 2001 From: Fredrik Thulin Date: Thu, 3 Mar 2011 12:38:34 +0100 Subject: [PATCH 5/7] authorize_user_token_ldap: sr was under-allocated by one byte. Also change strcat's to sprintf to make code easier to maintain. --- pam_yubico.c | 24 +++++------------------- 1 file changed, 5 insertions(+), 19 deletions(-) diff --git a/pam_yubico.c b/pam_yubico.c index bb0ff2c..e39fd4d 100644 --- a/pam_yubico.c +++ b/pam_yubico.c @@ -245,27 +245,13 @@ authorize_user_token_ldap (const char *ldap_uri, /* Allocation of memory for search strings depending on input size */ char *find = malloc((strlen(user_attr)+strlen(ldapdn)+strlen(user)+3)*sizeof(char)); - char *sr = malloc((strlen(yubi_attr)+4)*sizeof(char)); + char *sr = malloc((strlen(yubi_attr)+5)*sizeof(char)); - char sep[2] = ","; - char eq[2] = "="; - char sren[4] = "=*)"; + sprintf (find, "%s=%s,%s", user_attr, user, ldapdn); + sprintf (sr, "(%s=*)", yubi_attr); - sr[0] = '('; - sr[1] = '\0'; - find[0]='\0'; - - strcat (find, user_attr); - strcat (find, eq); - strcat (find, user); - strcat (find, sep); - strcat (find, ldapdn); - - strcat (sr, yubi_attr); - strcat (sr, sren); - - D(("find: %s",find)); - D(("sr: %s",sr)); + D(("LDAP : find: %s",find)); + D(("LDAP : sr: %s",sr)); /* Get a handle to an LDAP connection. */ if (ldap_uri) From a9ef97ea4ca31e23738e630b99bd474d74ffcc35 Mon Sep 17 00:00:00 2001 From: Fredrik Thulin Date: Thu, 3 Mar 2011 12:48:43 +0100 Subject: [PATCH 6/7] authorize_user_token_ldap: Don't leak memory on failures. --- pam_yubico.c | 35 +++++++++++++++++++++++------------ 1 file changed, 23 insertions(+), 12 deletions(-) diff --git a/pam_yubico.c b/pam_yubico.c index e39fd4d..b4d50a3 100644 --- a/pam_yubico.c +++ b/pam_yubico.c @@ -222,14 +222,16 @@ authorize_user_token_ldap (const char *ldap_uri, int retval = 0; int protocol; #ifdef HAVE_LIBLDAP - LDAP *ld; - LDAPMessage *result, *e; + LDAP *ld = NULL; + LDAPMessage *result = NULL, *e; BerElement *ber; char *a; struct berval **vals; int i, rc; + char *find = NULL, *sr = NULL; + if (user_attr == NULL) { D (("Trying to look up user to YubiKey mapping in LDAP, but user_attr not set!")); return 0; @@ -244,8 +246,8 @@ authorize_user_token_ldap (const char *ldap_uri, } /* Allocation of memory for search strings depending on input size */ - char *find = malloc((strlen(user_attr)+strlen(ldapdn)+strlen(user)+3)*sizeof(char)); - char *sr = malloc((strlen(yubi_attr)+5)*sizeof(char)); + find = malloc((strlen(user_attr)+strlen(ldapdn)+strlen(user)+3)*sizeof(char)); + sr = malloc((strlen(yubi_attr)+5)*sizeof(char)); sprintf (find, "%s=%s,%s", user_attr, user, ldapdn); sprintf (sr, "(%s=*)", yubi_attr); @@ -260,7 +262,8 @@ authorize_user_token_ldap (const char *ldap_uri, if (rc != LDAP_SUCCESS) { D (("ldap_init: %s", ldap_err2string (rc))); - return 0; + retval = 0; + goto done; } } else @@ -268,7 +271,8 @@ authorize_user_token_ldap (const char *ldap_uri, if ((ld = ldap_init (ldapserver, PORT_NUMBER)) == NULL) { D (("ldap_init")); - return 0; + retval = 0; + goto done; } } @@ -281,7 +285,8 @@ authorize_user_token_ldap (const char *ldap_uri, if (rc != LDAP_SUCCESS) { D (("ldap_simple_bind_s: %s", ldap_err2string (rc))); - return (0); + retval = 0; + goto done; } /* Search for the entry. */ @@ -294,7 +299,8 @@ authorize_user_token_ldap (const char *ldap_uri, { D (("ldap_search_ext_s: %s", ldap_err2string (rc))); - return (0); + retval = 0; + goto done; } e = ldap_first_entry (ld, result); @@ -326,12 +332,17 @@ authorize_user_token_ldap (const char *ldap_uri, } - ldap_msgfree (result); - ldap_unbind (ld); + done: + if (result != NULL) + ldap_msgfree (result); + if (ld != NULL) + ldap_unbind (ld); /* free memory allocated for search strings */ - free(find); - free(sr); + if (find != NULL) + free(find); + if (sr != NULL) + free(sr); #else D (("Trying to use LDAP, but this function is not compiled in pam_yubico!!")); From abb0b7e4e4d9ed0e09778815328126c6813b0d78 Mon Sep 17 00:00:00 2001 From: Fredrik Thulin Date: Thu, 3 Mar 2011 14:14:54 +0100 Subject: [PATCH 7/7] authorize_user_token_ldap: Only fetch the attribute we're interested in. Previous version fetched ALL attributes of the identified object, and treated them all equal when looking for the YubiKey token identifier. --- pam_yubico.c | 42 ++++++++++++++++++++++-------------------- 1 file changed, 22 insertions(+), 20 deletions(-) diff --git a/pam_yubico.c b/pam_yubico.c index b4d50a3..fce243c 100644 --- a/pam_yubico.c +++ b/pam_yubico.c @@ -226,6 +226,7 @@ authorize_user_token_ldap (const char *ldap_uri, LDAPMessage *result = NULL, *e; BerElement *ber; char *a; + char *attrs[2] = {NULL, NULL}; struct berval **vals; int i, rc; @@ -245,16 +246,6 @@ authorize_user_token_ldap (const char *ldap_uri, return 0; } - /* Allocation of memory for search strings depending on input size */ - find = malloc((strlen(user_attr)+strlen(ldapdn)+strlen(user)+3)*sizeof(char)); - sr = malloc((strlen(yubi_attr)+5)*sizeof(char)); - - sprintf (find, "%s=%s,%s", user_attr, user, ldapdn); - sprintf (sr, "(%s=*)", yubi_attr); - - D(("LDAP : find: %s",find)); - D(("LDAP : sr: %s",sr)); - /* Get a handle to an LDAP connection. */ if (ldap_uri) { @@ -289,12 +280,18 @@ authorize_user_token_ldap (const char *ldap_uri, goto done; } - /* Search for the entry. */ - D (("ldap-dn: %s", find)); - D (("ldap-filter: %s", sr)); + /* Allocation of memory for search strings depending on input size */ + find = malloc((strlen(user_attr)+strlen(ldapdn)+strlen(user)+3)*sizeof(char)); + sprintf (find, "%s=%s,%s", user_attr, user, ldapdn); + + attrs[0] = (char *) yubi_attr; + + D(("LDAP : look up object '%s', ask for attribute '%s'", find, yubi_attr)); + + /* Search for the entry. */ if ((rc = ldap_search_ext_s (ld, find, LDAP_SCOPE_BASE, - sr, NULL, 0, NULL, NULL, LDAP_NO_LIMIT, + NULL, attrs, 0, NULL, NULL, LDAP_NO_LIMIT, LDAP_NO_LIMIT, &result)) != LDAP_SUCCESS) { D (("ldap_search_ext_s: %s", ldap_err2string (rc))); @@ -304,15 +301,19 @@ authorize_user_token_ldap (const char *ldap_uri, } e = ldap_first_entry (ld, result); - if (e != NULL) + if (e == NULL) { - - /* Iterate through each attribute in the entry. */ + D (("No result from LDAP search")); + } + else + { + /* Iterate through each returned attribute. */ for (a = ldap_first_attribute (ld, e, &ber); a != NULL; a = ldap_next_attribute (ld, e, ber)) { if ((vals = ldap_get_values_len (ld, e, a)) != NULL) { + /* Compare each value for the attribute against the token id. */ for (i = 0; vals[i] != NULL; i++) { if (!strncmp (token_id, vals[i]->bv_val, strlen (token_id))) @@ -320,16 +321,17 @@ authorize_user_token_ldap (const char *ldap_uri, D (("Token Found :: %s", vals[i]->bv_val)); retval = 1; } + else + { + D (("No match : (%s) %s != %s", a, vals[i]->bv_val, token_id)); + } } ldap_value_free_len (vals); } ldap_memfree (a); } if (ber != NULL) - { ber_free (ber, 0); - } - } done: