some KDF fixes; add checkpw() method

9 files changed, 137 insertions, 13 deletions

diff --git a/README b/README
index 5067c40..f2d97b7 100644
--- a/README
+++ b/README
@@ -40,8 +40,8 @@ A simple example that demonstrates most of the features:
 	hashed = bcrypt.hashpw(password, bcrypt.gensalt(10))
 
 	# Check that an unencrypted password matches one that has
-	# previously been hashed
-	if bcrypt.hashpw(plaintext, hashed) == hashed:
+	# previously been hashed.
+	if bcrypt.checkpw(plaintext, hashed):
 		print "It matches"
 	else:
 		print "It does not match"
diff --git a/bcrypt/__init__.py b/bcrypt/__init__.py
index 8f7ba2f..78d945d 100644
--- a/bcrypt/__init__.py
+++ b/bcrypt/__init__.py
@@ -21,6 +21,10 @@ gensalt() function:
 The parameter "log_rounds" defines the complexity of the hashing. The
 cost increases as 2**log_rounds.
 
+Passwords can be checked against a hashed copy using the checkpw() routine:
+
+  checkpw(password, hashed_password) -> boolean (true if matched)
+
 Passwords and salts for the hashpw and gensalt functions are text strings
 that must not contain embedded nul (ASCII 0) characters.
 
@@ -32,7 +36,9 @@ password and salt into bytes suitable for use as cryptographic key material:
 This will generate a key of "desired_length" in bytes (NB. not bits). For the
 KDF mode the "rounds" parameter is the literal rounds, not the logarithm as
 for gensalt. For the KDF case, "salt" and "password" may be binary strings
-containing embedded nul characters.
+containing embedded nul characters. Note also that the "salt" for the KDF
+should just be a random sequence of bytes (e.g. as generated by os.urandom)
+and not one prepared with gensalt().
 
 The KDF mode is recommended for generating symmetric cipher keys, IVs, hash
 and MAC keys, etc. It should not be used a keystream for encryption itself.
diff --git a/bcrypt/__pycache__/__init__.cpython-33.pyc b/bcrypt/__pycache__/__init__.cpython-33.pyc
deleted file mode 100644
index 2c0890e..0000000
--- a/bcrypt/__pycache__/__init__.cpython-33.pyc
+++ /dev/null
diff --git a/bcrypt/bcrypt_pbkdf.c b/bcrypt/bcrypt_pbkdf.c
index fd607e6..c41d5a6 100644
--- a/bcrypt/bcrypt_pbkdf.c
+++ b/bcrypt/bcrypt_pbkdf.c
@@ -61,20 +61,20 @@ bcrypt_hash(u_int8_t *sha2pass, u_int8_t *sha2salt, u_int8_t *out)
 	size_t shalen = PYBC_SHA512_DIGEST_LENGTH;
 
 	/* key expansion */
-	Blowfish_initstate(&state);
-	Blowfish_expandstate(&state, sha2salt, shalen, sha2pass, shalen);
+	pybc_Blowfish_initstate(&state);
+	pybc_Blowfish_expandstate(&state, sha2salt, shalen, sha2pass, shalen);
 	for (i = 0; i < 64; i++) {
-		Blowfish_expand0state(&state, sha2salt, shalen);
-		Blowfish_expand0state(&state, sha2pass, shalen);
+		pybc_Blowfish_expand0state(&state, sha2salt, shalen);
+		pybc_Blowfish_expand0state(&state, sha2pass, shalen);
 	}
 
 	/* encryption */
 	j = 0;
 	for (i = 0; i < BCRYPT_BLOCKS; i++)
-		cdata[i] = Blowfish_stream2word(ciphertext, sizeof(ciphertext),
+		cdata[i] = pybc_Blowfish_stream2word(ciphertext, sizeof(ciphertext),
 		    &j);
 	for (i = 0; i < 64; i++)
-		blf_enc(&state, cdata, sizeof(cdata) / sizeof(u_int64_t));
+		pybc_blf_enc(&state, cdata, sizeof(cdata) / sizeof(u_int64_t));
 
 	/* copy out */
 	for (i = 0; i < BCRYPT_BLOCKS; i++) {
diff --git a/bcrypt/bcrypt_python.c b/bcrypt/bcrypt_python.c
index 176f4bc..9875bdc 100644
--- a/bcrypt/bcrypt_python.c
+++ b/bcrypt/bcrypt_python.c
@@ -17,6 +17,8 @@
 #define PY_SSIZE_T_CLEAN
 #include "Python.h"
 
+#include "pybc_blf.h"
+
 #if PY_VERSION_HEX < 0x02050000
 typedef int Py_ssize_t;
 #define bzero(s,l) memset(s, '\0', l)
@@ -115,9 +117,11 @@ bcrypt_hashpw(PyObject *self, PyObject *args, PyObject *kw_args)
 		    "password must not contain nul characters");
 		return NULL;
 	}
+	password_len = 0;
 	if ((salt_copy = checkdup(salt, salt_len)) == NULL) {
 		PyErr_SetString(PyExc_ValueError,
 		    "salt must not contain nul characters");
+		bzero(password_copy, strlen(password_copy));
 		free(password_copy);
 		return NULL;
 	}
@@ -129,7 +133,7 @@ bcrypt_hashpw(PyObject *self, PyObject *args, PyObject *kw_args)
 	free(password_copy);
 	bzero(salt_copy, strlen(salt_copy));
 	free(salt_copy);
-	if (ret != 0 || strcmp(hashed, ":") == 0) {
+	if (ret != 0 || strlen(hashed) < 32) {
 		PyErr_SetString(PyExc_ValueError, "Invalid salt");
 		return NULL;
 	}
@@ -140,6 +144,75 @@ bcrypt_hashpw(PyObject *self, PyObject *args, PyObject *kw_args)
 #endif
 }
 
+PyDoc_STRVAR(bcrypt_checkpw_doc,
+"checkpw(password, hashed_password) -> boolean\n\
+    Verify that the plaintext password matches hashed_password. Returns true\n\
+    if it matches or false otherwise.\n");
+
+static PyObject *
+bcrypt_checkpw(PyObject *self, PyObject *args, PyObject *kw_args)
+{
+	static char *keywords[] = { "password", "hashed_password", NULL };
+	char *password = NULL, *expected = NULL;
+	char hashed[128], *password_copy, *expected_copy;
+	Py_ssize_t password_len = -1, expected_len = -1, hashed_len;
+	int ret;
+
+	if (!PyArg_ParseTupleAndKeywords(args, kw_args, "s#s#:checkpw",
+	    keywords,
+	    &password, &password_len, &expected, &expected_len))
+                return NULL;
+
+	if (password_len < 0 || password_len > 65535) {
+		PyErr_SetString(PyExc_ValueError,
+		    "unsupported password length");
+		return NULL;
+	}
+	if (expected_len < 0 || expected_len > 65535) {
+		PyErr_SetString(PyExc_ValueError,
+		    "unsupported hashed_password length");
+		return NULL;
+	}
+	if ((password_copy = checkdup(password, password_len)) == NULL) {
+		PyErr_SetString(PyExc_ValueError,
+		    "password must not contain nul characters");
+		return NULL;
+	}
+	password_len = 0;
+	if ((expected_copy = checkdup(expected, expected_len)) == NULL) {
+		PyErr_SetString(PyExc_ValueError,
+		    "hashed_password must not contain nul characters");
+		bzero(password_copy, strlen(password_copy));
+		free(password_copy);
+		return NULL;
+	}
+
+	Py_BEGIN_ALLOW_THREADS;
+	ret = pybc_bcrypt(password_copy, expected_copy, hashed, sizeof(hashed));
+	Py_END_ALLOW_THREADS;
+
+	bzero(password_copy, strlen(password_copy));
+	free(password_copy);
+	hashed_len = strlen(hashed);
+	if (ret != 0 || hashed_len < 32) {
+		PyErr_SetString(PyExc_ValueError,
+		    "Invalid hashed_password salt");
+		bzero(expected_copy, strlen(expected_copy));
+		free(expected_copy);
+		return NULL;
+	}
+	ret = 1; /* fail unless timingsafe_bcmp runs and succeeds */
+	if (hashed_len == strlen(expected_copy))
+		ret = pybc_timingsafe_bcmp(expected_copy, hashed, hashed_len);
+	bzero(hashed, sizeof(hashed));
+	bzero(expected_copy, strlen(expected_copy));
+	free(expected_copy);
+	if (ret == 0)
+		Py_RETURN_TRUE;
+	else
+		Py_RETURN_FALSE;
+}
+
 PyDoc_STRVAR(bcrypt_kdf_doc,
 "kdf(password, salt, desired_key_bytes, rounds) -> key\n\
     Derive \"desired_key_bytes\" (up to 512) cryptographic key material from\n\
@@ -210,6 +283,8 @@ bcrypt_kdf(PyObject *self, PyObject *args, PyObject *kw_args)
 }
 
 static PyMethodDef bcrypt_methods[] = {
+	{	"checkpw",	(PyCFunction)bcrypt_checkpw,
+		METH_VARARGS|METH_KEYWORDS,	bcrypt_checkpw_doc	},
 	{	"hashpw",	(PyCFunction)bcrypt_hashpw,
 		METH_VARARGS|METH_KEYWORDS,	bcrypt_hashpw_doc	},
 	{	"encode_salt",	(PyCFunction)bcrypt_encode_salt,
diff --git a/bcrypt/pybc_blf.h b/bcrypt/pybc_blf.h
index e18f7b6..dd0ce46 100644
--- a/bcrypt/pybc_blf.h
+++ b/bcrypt/pybc_blf.h
@@ -34,13 +34,14 @@
 #ifndef _PYBC_BLF_H_
 #define _PYBC_BLF_H_
 
+#include <sys/types.h>
 #if defined(_WIN32)
 typedef unsigned __int8		u_int8_t;
 typedef unsigned __int16	u_int16_t;
 typedef unsigned __int32	u_int32_t;
 typedef unsigned __int64	u_int64_t;
 #elif __STDC_VERSION__ >= 199901L /* C99 or later */
-include <stdint.h>
+#include <stdint.h>
 typedef uint8_t u_int8_t;
 typedef uint16_t u_int16_t;
 typedef uint32_t u_int32_t;
@@ -86,4 +87,7 @@ int bcrypt_pbkdf(const char *pass, size_t passlen,
     const u_int8_t *salt, size_t saltlen,
     u_int8_t *key, size_t keylen, unsigned int rounds);
 
+/* timingsafe_bcmp */
+int pybc_timingsafe_bcmp(const void *b1, const void *b2, size_t n);
+
 #endif
diff --git a/bcrypt/timingsafe_bcmp.c b/bcrypt/timingsafe_bcmp.c
new file mode 100644
index 0000000..2785f84
--- /dev/null
+++ b/bcrypt/timingsafe_bcmp.c
@@ -0,0 +1,29 @@
+/*	$OpenBSD: timingsafe_bcmp.c,v 1.1 2010/09/24 13:33:00 matthew Exp $	*/
+/*
+ * Copyright (c) 2010 Damien Miller.  All rights reserved.
+ *
+ * Permission to use, copy, modify, and distribute this software for any
+ * purpose with or without fee is hereby granted, provided that the above
+ * copyright notice and this permission notice appear in all copies.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
+ * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
+ * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
+ * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
+ * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
+ * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
+ * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+ */
+
+#include "pybc_blf.h"
+
+int
+pybc_timingsafe_bcmp(const void *b1, const void *b2, size_t n)
+{
+	const unsigned char *p1 = b1, *p2 = b2;
+	int ret = 0;
+
+	for (; n > 0; n--)
+		ret |= *p1++ ^ *p2++;
+	return (ret != 0);
+}
diff --git a/setup.py b/setup.py
index be61b94..191d186 100755
--- a/setup.py
+++ b/setup.py
@@ -27,11 +27,12 @@ VERSION = "0.3"
 if __name__ == '__main__':
 	bcrypt = Extension('bcrypt._bcrypt',
 		sources = [
+			'bcrypt/bcrypt.c',
+			'bcrypt/bcrypt_pbkdf.c',
 			'bcrypt/bcrypt_python.c',
 			'bcrypt/blowfish.c',
-			'bcrypt/bcrypt.c',
 			'bcrypt/sha2.c',
-			'bcrypt/bcrypt_pbkdf.c',
+			'bcrypt/timingsafe_bcmp.c',
 		],
 	)
 	setup(	name = "py-bcrypt",
diff --git a/test/test.py b/test/test.py
index 947e625..4b89a43 100755
--- a/test/test.py
+++ b/test/test.py
@@ -100,6 +100,15 @@ class TestBcrypt(unittest.TestCase):
 				crypted2 = bcrypt.hashpw(plain, crypted)
 				self.assertEqual(crypted, crypted2)
 
+	def test_02__checkpw_success(self):
+		for plain, salt, expected in test_vectors:
+			self.assertTrue(bcrypt.checkpw(plain, expected))
+
+	def test_03__checkpw_fail(self):
+		for plain, salt, expected in test_vectors:
+			self.assertFalse(bcrypt.checkpw("foo", expected))
+
+
 # rounds, password, salt, expected_key
 kdf_test_vectors = [
 	[ 4, "password", "salt",