Bugs

[satleeixn]

1. In function b64_to_ab64(): the function does not work as expected, if the first character is a '+'. This results in the creation of invalid hashes, if the base64 of the salt or the derived key starts with a '+'.
2. In function init_module(): The lines 684-691 (starting with SECStatus rv; ) are specific for Mozilla NSS and prevent the compilation of the plugin without Mozilla NSS.
3. The Makefile does not mention libssl, which may be required by the plugin, if compiled against OpenSSL only.

[satleeixn]

Since it seems unclear, what problems the bug (1.) causes, what problems a bug-fix will cause and whether a bug-fix is worth the effort, maybe make a branch of the project where you quick-fix the bug. [pw-pbkdf2.c: new line 70: for(;*p;p++){ ] Remind users, that this branch requires corrected hashes (in hashes using the PBKDF2-schemes all '+'-chars must be replaced by '.'-chars.) New OpenLDAP instances can then start without the bug. And (test-)users can decide, whether the bug-fix is worth the effort. Once you get enough feedback from users, you can then decide how to fix the problem in the master branch.

[hamano]

@satleeixn Could you make PR? I just created issue12 branch. thanks

[satleeixn]

@hamano Sorry, I'm not a security researcher. I'm just a developer with a dead-line. Maybe you should ask the people from the OpenLDAP project for help. In case you have not much experience with making errors/error fixing, here are some more suggestions what you can do:

• Make sure that the plugin from the new branch creates only correct hashes.
• Document the issue and all research results in the README.md of the new branch.
• Check, whether the old/new plugin verifies passwords against the hashes from the other plugin. (I didn't check this, I stopped after seeing the undocumented '+'-signs.)
• Consider renaming the schema names for the new plugin (if the old plugin does not work with the correct hashes). (Even the bcrypt prefixes were changed from $2a$ to $2b$/$2y$).
• Since the '+'-sign is used by LDAP for concatenation of array values, some LDAP tools may have problems with the '+'-signs of the bad hashes. Maybe check, whether the LDAP export/import tools handle the bad hashes correctly.
• Maybe provide helper scripts to detect bad hashes in an existing OpenLDAP instance and for generation of (bad) example hashes.
• Milestone: The plugin from the new branch should detect/log bad hashes and verify passwords against both bad and correct hashes.

[satleeixn]

We tested the plugin with the one-line-fix above. So far we have no problems:

• The plugin verifies both incorrect(hash contains '+' instead of '.') and correct hashes against a password with correct result.
• Even the current/unfixed plugin should behave like this, since hash verification does not involve the function b64_to_ab64(). So there seems to be no reason to change the schema names.
• The fixed plugin now generates correct hashes.
• The bad/incorrect hashes may still cause problems, if exported/imported out of/into the OpenLDAP. This depends on the usage scenario.

All in all the issue seems to be mostly harmless if not cosmetic.

[hamano]

I'm sorry for the late response. just merged upstream patch. many thanks.