[mod_auth] Fix authentication (has been "disabled")! Implement apr-md5 crypt, add test cases

include/lighttpd/utils.h

@@ -99,6 +99,7 @@ LI_API void li_string_append_int(GString *dest, gint64 val);
 LI_API gsize li_dirent_buf_size(DIR * dirp);
 
 LI_API void li_apr_sha1_base64(GString *dest, const GString *passwd);
+LI_API void li_apr_md5_crypt(GString *dest, const GString *password, const GString *salt);
 
 INLINE GString* _li_g_string_append_len(GString *s, const gchar *val, gssize len);
 INLINE void li_g_string_clear(GString *s);

src/common/utils.c

@@ -862,6 +862,109 @@ void li_apr_sha1_base64(GString *dest, const GString *passwd) {
 	g_free(digest_base64);
 }
 
+/*  The basic algorithm for this "apr-md5-crypt" comes from
+ *  the FreeBSD 3.0 MD5 crypt() function, and was licensed as
+ *  "BEER-WARE" from Poul-Henning Kamp.
+ *
+ *  This is a complete rewrite to use glib functions.
+ *
+ *  Note: security by obscurity is not real security - this
+ *    still is "just" md5, don't trust it.
+ */
+
+#define APR1_MAGIC "$apr1$"
+
+static void md5_crypt_to64(GString *dest, guint number, guint len) {
+	static const gchar code[] = "./0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz";
+	for ( ; len-- > 0; ) {
+		g_string_append_len(dest, code + (number & 63), 1);
+		number /= 64;
+	}
+}
+
+void li_apr_md5_crypt(GString *dest, const GString *password, const GString *salt) {
+	guint i;
+	GChecksum *md5sum;
+	gsize digestlen = g_checksum_type_get_length(G_CHECKSUM_MD5);
+	guint8 digest[digestlen];
+
+	GString rsalt = { GSTR_LEN(salt), 0 };
+	if (li_string_prefix(&rsalt, CONST_STR_LEN(APR1_MAGIC))) {
+		rsalt.str += sizeof(APR1_MAGIC)-1;
+		rsalt.len -= sizeof(APR1_MAGIC)-1;
+	}
+	if (rsalt.len > 8) rsalt.len = 8;
+	for (i = 0; i < rsalt.len && rsalt.str[i] != '$'; i++) ;
+	rsalt.len = i;
+
+	md5sum = g_checksum_new(G_CHECKSUM_MD5);
+
+	g_checksum_update(md5sum, GUSTR_LEN(password));
+	g_checksum_update(md5sum, (guchar*) rsalt.str, rsalt.len);
+	g_checksum_update(md5sum, GUSTR_LEN(password));
+	g_checksum_get_digest(md5sum, digest, &digestlen);
+
+	g_checksum_reset(md5sum);
+
+	g_checksum_update(md5sum, GUSTR_LEN(password));
+	g_checksum_update(md5sum, CONST_USTR_LEN(APR1_MAGIC));
+	g_checksum_update(md5sum, (guchar*) rsalt.str, rsalt.len);
+
+	for (i = password->len / 16; i-- > 0; ) {
+		g_checksum_update(md5sum, digest, digestlen);
+	}
+	g_checksum_update(md5sum, digest, password->len % 16);
+
+	for (i = password->len; i != 0; i /= 2) {
+		if (i % 2) {
+			g_checksum_update(md5sum, (guchar*) "", 1);
+		} else {
+			g_checksum_update(md5sum, (guchar*) password->str, 1);
+		}
+	}
+	g_checksum_get_digest(md5sum, digest, &digestlen);
+
+	for (i = 0; i < 1000; i++) {
+		g_checksum_reset(md5sum);
+
+		if (i % 2) {
+			g_checksum_update(md5sum, GUSTR_LEN(password));
+		} else {
+			g_checksum_update(md5sum, digest, digestlen);
+		}
+
+		if (i % 3) {
+			g_checksum_update(md5sum, (guchar*) rsalt.str, rsalt.len);
+		}
+
+		if (i % 7) {
+			g_checksum_update(md5sum, GUSTR_LEN(password));
+		}
+
+		if (i % 2) {
+			g_checksum_update(md5sum, digest, digestlen);
+		} else {
+			g_checksum_update(md5sum, GUSTR_LEN(password));
+		}
+
+		g_checksum_get_digest(md5sum, digest, &digestlen);
+	}
+
+	g_checksum_free(md5sum);
+
+	li_g_string_clear(dest);
+	g_string_append_len(dest, CONST_STR_LEN(APR1_MAGIC));
+	g_string_append_len(dest, rsalt.str, rsalt.len);
+	g_string_append_len(dest, CONST_STR_LEN("$"));
+	md5_crypt_to64(dest, (digest[ 0] << 16) | (digest[ 6] << 8) | digest[12], 4);
+	md5_crypt_to64(dest, (digest[ 1] << 16) | (digest[ 7] << 8) | digest[13], 4);
+	md5_crypt_to64(dest, (digest[ 2] << 16) | (digest[ 8] << 8) | digest[14], 4);
+	md5_crypt_to64(dest, (digest[ 3] << 16) | (digest[ 9] << 8) | digest[15], 4);
+	md5_crypt_to64(dest, (digest[ 4] << 16) | (digest[10] << 8) | digest[ 5], 4);
+	md5_crypt_to64(dest,                       digest[11]                   , 2);
+}
+
+
 void li_g_queue_merge(GQueue *dest, GQueue *src) {
 	assert(dest != src);
 	if (g_queue_is_empty(src)) return; /* nothing to do */

src/modules/mod_auth.c

@@ -75,7 +75,7 @@ LI_API gboolean mod_auth_free(liModules *mods, liModule *mod);
 typedef struct AuthBasicData AuthBasicData;
 
 /* GStrings may be fake, only use ->str and ->len; but they are \0 terminated */
-typedef gboolean (*AuthBasicBackend)(liVRequest *vr, const GString *username, const GString *password, AuthBasicData *bdata);
+typedef gboolean (*AuthBasicBackend)(liVRequest *vr, const GString *username, const GString *password, AuthBasicData *bdata, gboolean debug);
 
 struct AuthBasicData {
 	liPlugin *p;
@@ -245,7 +245,7 @@ static AuthFile* auth_file_new(liWorker *wrk, const GString *path, gboolean has_
 	return f;
 }
 
-static gboolean auth_backend_plain(liVRequest *vr, const GString *username, const GString *password, AuthBasicData *bdata) {
+static gboolean auth_backend_plain(liVRequest *vr, const GString *username, const GString *password, AuthBasicData *bdata, gboolean debug) {
 	const char *pass;
 	AuthFileData *afd = auth_file_get_data(vr->wrk, bdata->data);
 	gboolean res = FALSE;
@@ -254,11 +254,18 @@ static gboolean auth_backend_plain(liVRequest *vr, const GString *username, cons
 
 	/* unknown user? */
 	if (!(pass = g_hash_table_lookup(afd->users, username->str))) {
+		if (debug) {
+			VR_DEBUG(vr, "User \"%s\" not found", username->str);
+		}
 		goto out;
 	}
 
 	/* wrong password? */
-	if (!g_str_equal(password->str, pass)) {
+	if (0 != g_strcmp0(password->str, pass)) {
+		if (debug) {
+			VR_DEBUG(vr, "Password \"%s\" doesn't match \"%s\" for user \"%s\"", password->str, pass, username->str);
+		}
+
 		goto out;
 	}
 
@@ -270,7 +277,7 @@ out:
 	return res;
 }
 
-static gboolean auth_backend_htpasswd(liVRequest *vr, const GString *username, const GString *password, AuthBasicData *bdata) {
+static gboolean auth_backend_htpasswd(liVRequest *vr, const GString *username, const GString *password, AuthBasicData *bdata, gboolean debug) {
 	const char *pass;
 	AuthFileData *afd = auth_file_get_data(vr->wrk, bdata->data);
 	gboolean res = FALSE;
@@ -279,17 +286,30 @@ static gboolean auth_backend_htpasswd(liVRequest *vr, const GString *username, c
 
 	/* unknown user? */
 	if (!(pass = g_hash_table_lookup(afd->users, username->str))) {
+		if (debug) {
+			VR_DEBUG(vr, "User \"%s\" not found", username->str);
+		}
 		goto out;
 	}
 
 	if (g_str_has_prefix(pass, "$apr1$")) {
-		/* We don't support this stupid method. Run around your house 1000 times and use sha1 next time */
-		goto out;
+		const GString salt = { (gchar*) pass, strlen(pass), 0 };
+		li_apr_md5_crypt(vr->wrk->tmp_str, password, &salt);
+
+		if (0 != g_strcmp0(pass, vr->wrk->tmp_str->str)) {
+			if (debug) {
+				VR_DEBUG(vr, "Password apr-md5 crypt \"%s\" doesn't match \"%s\" for user \"%s\"", vr->wrk->tmp_str->str, pass, username->str);
+			}
+			goto out;
+		}
 	} else
 	if (g_str_has_prefix(pass, "{SHA}")) {
 		li_apr_sha1_base64(vr->wrk->tmp_str, password);
 
-		if (g_str_equal(password->str, vr->wrk->tmp_str->str)) {
+		if (0 != g_strcmp0(pass, vr->wrk->tmp_str->str)) {
+			if (debug) {
+				VR_DEBUG(vr, "Password apr-sha1 crypt \"%s\" doesn't match \"%s\" for user \"%s\"", vr->wrk->tmp_str->str, pass, username->str);
+			}
 			goto out;
 		}
 	}
@@ -301,7 +321,10 @@ static gboolean auth_backend_htpasswd(liVRequest *vr, const GString *username, c
 		memset(&buffer, 0, sizeof(buffer));
 		crypted = crypt_r(password->str, pass, &buffer);
 
-		if (g_str_equal(pass, crypted)) {
+		if (0 != g_strcmp0(pass, crypted)) {
+			if (debug) {
+				VR_DEBUG(vr, "Password crypt \"%s\" doesn't match \"%s\" for user \"%s\"", crypted, pass, username->str);
+			}
 			goto out;
 		}
 	}
@@ -315,7 +338,7 @@ out:
 	return res;
 }
 
-static gboolean auth_backend_htdigest(liVRequest *vr, const GString *username, const GString *password, AuthBasicData *bdata) {
+static gboolean auth_backend_htdigest(liVRequest *vr, const GString *username, const GString *password, AuthBasicData *bdata, gboolean debug) {
 	const char *pass, *realm;
 	AuthFileData *afd = auth_file_get_data(vr->wrk, bdata->data);
 	GChecksum *md5sum;
@@ -325,6 +348,9 @@ static gboolean auth_backend_htdigest(liVRequest *vr, const GString *username, c
 
 	/* unknown user? */
 	if (!(pass = g_hash_table_lookup(afd->users, username->str))) {
+		if (debug) {
+			VR_DEBUG(vr, "User \"%s\" not found", username->str);
+		}
 		goto out;
 	}
 
@@ -333,6 +359,9 @@ static gboolean auth_backend_htdigest(liVRequest *vr, const GString *username, c
 
 	/* no realm/wrong realm? */
 	if (NULL == pass || 0 != strncmp(realm, bdata->realm->str, bdata->realm->len)) {
+		if (debug) {
+			VR_DEBUG(vr, "Realm for user \"%s\" doesn't match", username->str);
+		}
 		goto out;
 	}
 	pass++;
@@ -347,6 +376,10 @@ static gboolean auth_backend_htdigest(liVRequest *vr, const GString *username, c
 	/* wrong password? */
 	if (g_str_equal(pass, g_checksum_get_string(md5sum))) {
 		res = TRUE;
+	} else {
+		if (debug) {
+			VR_DEBUG(vr, "Password digest \"%s\" doesn't match \"%s\" for user \"%s\"", g_checksum_get_string(md5sum), pass, username->str);
+		}
 	}
 
 	g_checksum_free(md5sum);
@@ -401,14 +434,14 @@ static liHandlerResult auth_basic(liVRequest *vr, gpointer param, gpointer *cont
 		} else {
 			GString user = li_const_gstring(username, password - username - 1);
 			GString pass = li_const_gstring(password, len - (password - username));
-			if (bdata->backend(vr, &user, &pass, bdata)) {
+			if (bdata->backend(vr, &user, &pass, bdata, debug)) {
 				auth_ok = TRUE;
 
 				li_environment_set(&vr->env, CONST_STR_LEN("REMOTE_USER"), username, password - username - 1);
 				li_environment_set(&vr->env, CONST_STR_LEN("AUTH_TYPE"), CONST_STR_LEN("Basic"));
 			} else {
 				if (debug) {
-					VR_DEBUG(vr, "wrong authorization info from client for realm \"%s\"", bdata->realm->str);
+					VR_DEBUG(vr, "wrong authorization info from client on realm \"%s\" (user: \"%s\")", bdata->realm->str, username);
 				}
 			}
 			g_free(decoded);

src/unittests/test-utils.c

@@ -39,7 +39,7 @@ static void test_send_fd(void) {
 	close(rfd);
 }
 
-static void test_apr_sha1_base64(void) {
+static void test_apr_sha1_base64_1(void) {
 	GString *dest = g_string_sized_new(0);
 	GString pass = li_const_gstring(CONST_STR_LEN("bar"));
 
@@ -48,11 +48,32 @@ static void test_apr_sha1_base64(void) {
 	g_assert_cmpstr(dest->str, ==, "{SHA}Ys23Ag/5IOWqZCw9QGaVDdHwH00=");
 }
 
+static void test_apr_sha1_base64_2(void) {
+	GString *dest = g_string_sized_new(0);
+	GString pass = li_const_gstring(CONST_STR_LEN("pass4"));
+
+	li_apr_sha1_base64(dest, &pass);
+
+	g_assert_cmpstr(dest->str, ==, "{SHA}LbTBgR9CRYKpD41+53mVzwGNlEM=");
+}
+
+static void test_apr_md5_crypt(void) {
+	GString *dest = g_string_sized_new(0);
+	GString hash = li_const_gstring(CONST_STR_LEN("$apr1$mhpONdUp$xSRcAbK2F6hLFUzW59tzW/"));
+	GString pass = li_const_gstring(CONST_STR_LEN("pass1"));
+
+	li_apr_md5_crypt(dest, &pass, &hash);
+
+	g_assert_cmpstr(dest->str, ==, hash.str);
+}
+
 int main(int argc, char **argv) {
 	g_test_init(&argc, &argv, NULL);
 
 	g_test_add_func("/utils/send_fd", test_send_fd);
-	g_test_add_func("/utils/apr_sha1_base64", test_apr_sha1_base64);
+	g_test_add_func("/utils/apr_sha1_base64/1", test_apr_sha1_base64_1);
+	g_test_add_func("/utils/apr_sha1_base64/2", test_apr_sha1_base64_2);
+	g_test_add_func("/utils/apr_md5_crypt", test_apr_md5_crypt);
 
 	return g_test_run();
 }

tests/base.py

@@ -7,7 +7,7 @@ import traceback
 
 from service import *
 
-__all__ = [ "Env", "Tests", "TestBase" ]
+__all__ = [ "Env", "Tests", "TestBase", "GroupTest" ]
 
 class Dict(object):
 	pass
@@ -34,7 +34,7 @@ def vhostname(testname):
 
 # basic interface
 class TestBase(object):
-	config = "defaultaction;"
+	config = None
 	name = None
 	vhost = None
 	runnable = True
@@ -89,6 +89,8 @@ var.vhosts = var.vhosts + [ "%s" : ${
 			self._cleanupFile(f)
 		for d in self._test_cleanup_dirs:
 			self._cleanupDir(d)
+		self._test_cleanup_files = []
+		self._test_cleanup_dirs = []
 
 	def _cleanupFile(self, fname):
 		self.tests.CleanupFile(fname)
@@ -123,6 +125,36 @@ var.vhosts = var.vhosts + [ "%s" : ${
 	def Cleanup(self):
 		pass
 
+def class2testname(name):
+	if name.startswith("Test"): name = name[4:]
+	return name
+
+class GroupTest(TestBase):
+	runnable = False
+
+	def __init__(self):
+		super(GroupTest, self).__init__()
+		self.subtests = []
+		for c in self.group:
+			t = c()
+			self.subtests.append(t)
+
+	def _register(self, tests):
+		super(GroupTest, self)._register(tests)
+		for t in self.subtests:
+			if None == t.name:
+				t.name = self.name + class2testname(t.__class__.__name__) + '/'
+			if None == t.vhost:
+				t.vhost = self.vhost
+			t._register(tests)
+
+	def _cleanup(self):
+		for t in self.subtests:
+			if t._test_failed:
+				self._test_failed = True
+		super(GroupTest, self)._cleanup()
+
+
 class Tests(object):
 	def __init__(self):
 		self.tests_filter = []
@@ -295,7 +327,11 @@ allow-listen {{ ip "127.0.0.1:{Env.port}"; }}
 
 	def _cleanupfile(self, fname):
 		if self.prepared_files.has_key(fname):
-			os.remove(os.path.join(Env.dir, fname))
+			try:
+				os.remove(os.path.join(Env.dir, fname))
+			except Exception as e:
+				print >>sys.stderr, "Couldn't delete file '%s': %s" % (fname, e)
+				return False
 			return True
 		else:
 			return False
@@ -310,7 +346,10 @@ allow-listen {{ ip "127.0.0.1:{Env.port}"; }}
 	def _cleanupdir(self, dirname):
 		self.prepared_dirs[dirname] -= 1
 		if 0 == self.prepared_dirs[dirname]:
-			os.rmdir(os.path.join(Env.dir, dirname))
+			try:
+				os.rmdir(os.path.join(Env.dir, dirname))
+			except Exception as e:
+				print >>sys.stderr, "Couldn't delete directory '%s': %s" % (dirname, e)
 
 	def PrepareFile(self, fname, content):
 		path = filter(lambda x: x != '', fname.split('/'))

tests/logfile.py

@@ -41,7 +41,10 @@ class LogFile(object):
 	def close(self, *args, **kwargs): return self.file.close(*args, **kwargs)
 	def fileno(self, *args, **kwargs):
 		pass
-	def flush(self, *args, **kwargs): return self.file.flush(*args, **kwargs)
+	def flush(self, *args, **kwargs):
+		for (p, f) in self.clones.items():
+			f.flush(*args, **kwargs)
+		return self.file.flush(*args, **kwargs)
 	def isatty(self, *args, **kwargs): return False
 	def next(self, *args, **kwargs): return self.file.next(*args, **kwargs)
 

tests/requests.py

@@ -15,6 +15,7 @@ class CurlRequest(TestBase):
 	URL = None
 	SCHEME = "http"
 	PORT = 0 # offset to Env.port
+	AUTH = None
 
 	EXPECT_RESPONSE_BODY = None
 	EXPECT_RESPONSE_CODE = None
@@ -56,6 +57,11 @@ class CurlRequest(TestBase):
 		c.setopt(pycurl.WRITEFUNCTION, b.write)
 		c.setopt(pycurl.HEADERFUNCTION, self._recv_header)
 
+		if None != self.AUTH:
+			c.setopt(pycurl.USERPWD, self.AUTH)
+			c.setopt(pycurl.FOLLOWLOCATION, 1)
+			c.setopt(pycurl.MAXREDIRS, 5)
+
 		self.curl = c
 		self.buffer = b
 
@@ -78,6 +84,8 @@ class CurlRequest(TestBase):
 
 	def dump(self):
 		c = self.curl
+		sys.stdout.flush()
+		print >> sys.stdout, "Dumping request for test '%s'" % self.name
 		print >> sys.stdout, "Curl request: URL = %s://%s:%i%s" % (self.SCHEME, self.vhost, Env.port + self.PORT, self.URL)
 		print >> sys.stdout, "Curl response code: %i " % (c.getinfo(pycurl.RESPONSE_CODE))
 		print >> sys.stdout, "Curl response headers:"
@@ -85,6 +93,7 @@ class CurlRequest(TestBase):
 			print >> sys.stdout, "  %s: %s" % (k, v)
 		print >> sys.stdout, "Curl response body:"
 		print >> sys.stdout, self.buffer.getvalue()
+		sys.stdout.flush()
 
 	def _checkResponse(self):
 		c = self.curl

tests/t-basic-gets.py

@@ -1,7 +1,5 @@
 # -*- coding: utf-8 -*-
 
-import sys
-
 from base import *
 from requests import *
 

tests/t-mod-auth.py

@@ -0,0 +1,94 @@
+# -*- coding: utf-8 -*-
+
+from base import *
+from requests import *
+
+#userI:passI for I in [1..4] with [apr-md5, crypt, plain and apr-sha]
+PASSWORDS="""user1:$apr1$mhpONdUp$xSRcAbK2F6hLFUzW59tzW/
+user2:JTMoqfZHCS0aI
+user3:pass3
+user4:{SHA}LbTBgR9CRYKpD41+53mVzwGNlEM=
+"""
+
+# user5:pass5 in realm 'Realm1'
+DIGESTPASSWORDS="""user5:Realm1:b0590e8c95605dd708226b552fc86a22
+"""
+
+class TestAprMd5Fail(CurlRequest):
+	URL = "/test.txt"
+	EXPECT_RESPONSE_CODE = 401
+	AUTH = "user1:test1"
+
+class TestAprMd5Success(CurlRequest):
+	URL = "/test.txt"
+	EXPECT_RESPONSE_CODE = 200
+	AUTH = "user1:pass1"
+
+class TestCryptFail(CurlRequest):
+	URL = "/test.txt"
+	EXPECT_RESPONSE_CODE = 401
+	AUTH = "user2:test2"
+
+class TestCryptSuccess(CurlRequest):
+	URL = "/test.txt"
+	EXPECT_RESPONSE_CODE = 200
+	AUTH = "user2:pass2"
+
+class TestPlainFail(CurlRequest):
+	URL = "/test.txt?plain"
+	EXPECT_RESPONSE_CODE = 401
+	AUTH = "user3:test3"
+
+class TestPlainSuccess(CurlRequest):
+	URL = "/test.txt?plain"
+	EXPECT_RESPONSE_CODE = 200
+	AUTH = "user3:pass3"
+
+class TestAprSha1Fail(CurlRequest):
+	URL = "/test.txt"
+	EXPECT_RESPONSE_CODE = 401
+	AUTH = "user4:test4"
+
+class TestAprSha1Success(CurlRequest):
+	URL = "/test.txt"
+	EXPECT_RESPONSE_CODE = 200
+	AUTH = "user4:pass4"
+
+class TestDigestFail(CurlRequest):
+	URL = "/test.txt?digest"
+	EXPECT_RESPONSE_CODE = 401
+	AUTH = "user5:test5"
+
+class TestDigestSuccess(CurlRequest):
+	URL = "/test.txt?digest"
+	EXPECT_RESPONSE_CODE = 200
+	AUTH = "user5:pass5"
+
+class Test(GroupTest):
+	vhost = "mod-auth"
+	group = [
+		TestAprMd5Fail, TestAprMd5Success,
+		TestCryptFail, TestCryptSuccess,
+		TestPlainFail, TestPlainSuccess,
+		TestAprSha1Fail, TestAprSha1Success,
+		TestDigestFail, TestDigestSuccess,
+	]
+
+	def Prepare(self):
+		passwdfile = self.PrepareFile("conf/mod-auth.htpasswd", PASSWORDS)
+		digestfile = self.PrepareFile("conf/mod-auth.htdigest", DIGESTPASSWORDS)
+
+		self.config = """
+			setup {{ module_load ( "mod_auth" ); }}
+
+			auth.debug = true;
+			if req.query == "plain" {{
+				auth.plain ["method": "basic", "realm": "Basic Auth Realm", "file": "{passwdfile}", "ttl": 10];
+			}} else if req.query == "digest" {{
+				auth.htdigest ["method": "basic", "realm": "Realm1", "file": "{digestfile}", "ttl": 10];
+			}} else {{
+				auth.htpasswd ["method": "basic", "realm": "Basic Auth Realm", "file": "{passwdfile}", "ttl": 10];
+			}}
+
+			defaultaction;
+		""".format(passwdfile = passwdfile, digestfile = digestfile)