Depending on the way a cleartext password is stored, SASL authentication may or may not work. There are two issues causing this problem. The first issue is that we aren't consistent with the way we store cleartext passwords. If you set the global password storage scheme to "clear" and use ldapmodify to add a userpassword with a value of "password", then you get this stored in the entry: userPassword: password Regardless of the global storage scheme setting, using ldapmodify to add a userpassword with a value of "{clear}password" results in: userPassword: {clear}password When doing a simple bind, the server-side bind code will strip off the storage scheme from the userPassword attribute value before doing a comparison with the password provided by the client attempting to bind. This brings us to the second problem. When doing a SASL bind using a mechanism that actually requires a userPassword, we don't strip off the storage scheme before setting the SASL password property. This causes SASL authentication to fail unless the client application used "{clear}password" as their secret when generating their credentials. The attached diffs address both of these issues. Here's what the changes do: - When the storage scheme is set to clear on the server, append the "{clear}" storage scheme prefix to the password unless a different scheme was specified in the operation. - Strip off the "{clear}" storage scheme prefix if it is present when setting the SASL password property. The code will still deal with cleartext passwords that do not have a storage scheme prefix. - Print a warning message in the errors log at the trace level when a SASL bind is attempted for an entry that has a password present that uses a storage scheme other than "{clear}". - If a password is using a storage scheme other than "{clear}", don't copy the userPassword value into the SASL password property. This will ensure that someone can't do something such as use the SHA hashed password that is stored on the server to create the client side SASL credentials to authenticate. - The SASL logging callback was printing informational and trace messages at the LDAP_DEBUG_ANY level. This would cause spurious logging such as a message for each step of the DIGEST-MD5 authentication process. I changed the behavior so that these non-fatal SASL messages are logged at the LDAP_DEBUG_TRACE level instead.
Created attachment 142445 [details] CVS Diffs
Looks good! > if (pw == strstr( pw, "{clear}" )) { One silly question. :) I'd like to double check "{CLEAR}" never be used here... I quickly checked with RFC3112 "LDAP Authentication Password Schema", but I could not find the answer... :p Do you happen to know where I should check?
I'd also like to point out that the code will fail with a password with the following value: "{xxxxxxxxxxxx{clear}" I think we should use if (!strncmp(pw, "{clear}", 7)) { clear = pw + 7; } else { /* something else - just assume the whole thing is the clear text pw */ clear = pw; } If you need case insensitive, you can use PL_strncasecmp().
(In reply to comment #3) > I think we should use > if (!strncmp(pw, "{clear}", 7)) { > clear = pw + 7; > } else { > /* something else - just assume the whole thing is the clear text pw */ > clear = pw; > } The problem with this else block is that it will allow a client to use a password hash (assuming that's what is stored as the userPassword attribute value) to authenticate with SASL. This seems bad. The whole point of hashing the password is that the hashed password should not be usable for anything other than comparing another hash against it. This does cause passwords such as the example you mentioned to not work with SASL mechanisms such as DIGEST-MD5 and CRAM-MD5 though.
(In reply to comment #4) > The problem with this else block is that it will allow a client to use a > password hash (assuming that's what is stored as the userPassword attribute > value) to authenticate with SASL. This seems bad. The whole point of hashing > the password is that the hashed password should not be usable for anything other > than comparing another hash against it. This does cause passwords such as the > example you mentioned to not work with SASL mechanisms such as DIGEST-MD5 and > CRAM-MD5 though. > Ok. Then the original is fine, and you can use PL_strcasestr if you need to do a case insensitive comparison.
Created attachment 142641 [details] Revised Diffs After quite a bit of RFC reading (particularly 2256, 2307 and 3112) and discussion with Rich and Noriko, I've come to the conclusion that we shouldn't be adding the "{clear}" prefix to passwords. This new set of diffs fixes the issues with SASL being able to use password hash values to authenticate. WHen authenticating with SASL, the server will ensure that the password is a clear-text password. I've also made the server strip the "{clear}" prefix off of the userPassword attribute value if a client tries to add the prefix itself. This will ensure that any newly stored passwords will not contain the "{clear}" prefix.
diff -u -5 -t -r1.5 clear_pwd.c --- ldap/servers/plugins/pwdstorage/clear_pwd.c 10 Nov 2006 23:45:10 -0000 1.5 +++ ldap/servers/plugins/pwdstorage/clear_pwd.c 1 Dec 2006 23:50:44 -0000 @@ -58,7 +58,18 @@ } char * clear_pw_enc( char *pwd ) { - return( slapi_ch_strdup( pwd )); + /* Just return NULL if pwd is NULL */ + if (!pwd) + return NULL; + + /* If the modify operation specified the "{clear}" storage scheme + * prefix, we should strip it off. + */ + if ((*pwd == PWD_HASH_PREFIX_START) && PL_strcasestr( pwd, "{clear}" )) { ==> if ((*pwd == PWD_HASH_PREFIX_START) && pwd == PL_strcasestr( pwd, "{clear}" )) { I wonder if we strictly expect to have {<scheme>} as a prefix. If so, I'd feel safer if we check the position of "{clear}"... The rest looks good to me. And it'd be nice if you could open a new bug for implementing authpassword for 7.3 or later (if not yet ;).
Created attachment 142888 [details] Revised Diffs This set of revised diffs adds Noriko's suggestion from Comment#7.
Checked into ldapserver (HEAD). Thanks to Rich and Noriko for their reviews! Checking in servers/slapd/pw.c; /cvs/dirsec/ldapserver/ldap/servers/slapd/pw.c,v <-- pw.c new revision: 1.15; previous revision: 1.14 done Checking in servers/slapd/saslbind.c; /cvs/dirsec/ldapserver/ldap/servers/slapd/saslbind.c,v <-- saslbind.c new revision: 1.22; previous revision: 1.21 done
*** Bug 243792 has been marked as a duplicate of this bug. ***
Confirmed that the server no longer stores {clear} in with the password when stored as clear. configured the server by adding the following ldif: dn: cn=config changetype: modify add: passwordstoragescheme passwordstoragescheme: clear then after adding: dn: cn=Kerberos uid mapping,cn=mapping,cn=sasl,cn=config changetype: modify replace: nsSaslMapBaseDNTemplate nsSaslMapBaseDNTemplate: o=sasl.com dn: cn=rfc 2829 dn syntax,cn=mapping,cn=sasl,cn=config changetype: modify replace: nsSaslMapBaseDNTemplate nsSaslMapBaseDNTemplate: o=sasl.com dn: cn=rfc 2829 u syntax,cn=mapping,cn=sasl,cn=config changetype: modify replace: nsSaslMapBaseDNTemplate nsSaslMapBaseDNTemplate: o=sasl.com dn: cn=uid mapping,cn=mapping,cn=sasl,cn=config changetype: modify replace: nsSaslMapBaseDNTemplate nsSaslMapBaseDNTemplate: o=sasl.com I was able to auth with: /usr/lib/mozldap6/ldappasswd -h brandywine.dsqa.sjc2.redhat.com -p 1111 -s 9998 "uid=mgreggtest,ou=people,o=redhat" -a 1234 -m CRAM-M5 -o "mech=CRAM-MD5" -o "authid=ttest1" -w 1234 I was also able to dump the DB and verify that the entries are not stored with {clear} in front of them. Verified aginst: 1199305764 idm-console-framework-1.1.0-7.el4idm Wed Jan 02 2008 1199305765 redhat-ds-console-8.0.0-9.el4dsrv Wed Jan 02 2008 1199305766 redhat-admin-console-8.0.0-9.el4dsrv Wed Jan 02 2008 1199305768 redhat-idm-console-1.0.0-21.el4idm Wed Jan 02 2008 1199390834 net-snmp-libs-5.1.2-11.el4_6.11.1 Thu Jan 03 2008 1199390890 net-snmp-5.1.2-11.el4_6.11.1 Thu Jan 03 2008 1199391027 db4-4.2.52-7.3.el4 Thu Jan 03 2008 1199391030 db4-utils-4.2.52-7.3.el4 Thu Jan 03 2008 1199391031 db4-tcl-4.2.52-7.3.el4 Thu Jan 03 2008 1199391034 db4-java-4.2.52-7.3.el4 Thu Jan 03 2008 1199391044 db4-devel-4.2.52-7.3.el4 Thu Jan 03 2008 1199391134 cyrus-sasl-2.1.19-14 Thu Jan 03 2008 1199391137 cyrus-sasl-md5-2.1.19-14 Thu Jan 03 2008 1199391138 cyrus-sasl-gssapi-2.1.19-14 Thu Jan 03 2008 1199391138 cyrus-sasl-plain-2.1.19-14 Thu Jan 03 2008 1199391138 cyrus-sasl-sql-2.1.19-14 Thu Jan 03 2008 1199391139 cyrus-sasl-ntlm-2.1.19-14 Thu Jan 03 2008 1199391140 cyrus-sasl-devel-2.1.19-14 Thu Jan 03 2008