Don't use strcmp to compare passwords.

Add password_equal() which tries to avoid leaking the length of the secret
through timing side-channels.

Suggested-by: SUSE Security Team <security@suse.de>

darkhttpd.c

@@ -304,7 +304,7 @@ static char *pidfile_name = NULL;   /* NULL = no pidfile */
 static int want_chroot = 0, want_daemon = 0, want_accf = 0,
            want_keepalive = 1, want_server_id = 1;
 static char *server_hdr = NULL;
-static char *auth_key = NULL;
+static char *auth_key = NULL;       /* NULL or "Basic base64_of_password" */
 static char *custom_hdrs = NULL;
 static uint64_t num_requests = 0, total_in = 0, total_out = 0;
 static int accepting = 1;           /* set to 0 to stop accept()ing */
@@ -2291,6 +2291,33 @@ static void process_get(struct connection *conn) {
     }
 }
 
+/* Returns 1 if passwords are equal, runtime is proportional to the length of
+ * user_input to avoid leaking the secret's length and contents through timing
+ * information.
+ */
+int password_equal(const char *user_input, const char *secret) {
+    size_t i = 0;
+    size_t j = 0;
+    char out = 0;
+
+    while (1) {
+        /* Out stays zero if the strings are the same. */
+        out |= user_input[i] ^ secret[j];
+
+        /* Stop at end of user_input. */
+        if (user_input[i] == 0) break;
+        i++;
+
+        /* Don't go past end of secret. */
+        if (secret[j] != 0) j++;
+    }
+
+    /* Check length after loop, otherwise early exit would leak length. */
+    out |= (i != j); /* Secret was shorter. */
+    out |= (secret[j] != 0); /* Secret was longer; j is not the end. */
+    return out == 0;
+}
+
 /* Process a request: build the header and reply, advance state. */
 static void process_request(struct connection *conn) {
     num_requests++;
@@ -2305,8 +2332,7 @@ static void process_request(struct connection *conn) {
     /* fail if: (auth_enabled) AND (client supplied invalid credentials) */
     else if (auth_key != NULL &&
             (conn->authorization == NULL ||
-             strcmp(conn->authorization, auth_key)))
-    {
+             !password_equal(conn->authorization, auth_key))) {
         default_reply(conn, 401, "Unauthorized",
             "Access denied due to invalid credentials.");
     }

devel/Makefile

@@ -8,6 +8,7 @@ clean:
 		test.out.stdout \
 		test.pyc \
 		test_make_safe_uri \
+		test_password_equal \
 		a.out darkhttpd.gcda darkhttpd.gcno \
 		fuzz_darkhttpd.o fuzz_llvm_make_safe_uri fuzz_socket
 	rm -rf tmp.fuzz tmp.httpd.tests

devel/run-tests

@@ -153,7 +153,7 @@ runtests() {
 
 # --- main ---
 
-# Unit test.
+# Unit tests.
 echo "===> test_make_safe_uri"
 $CC -g -O2 -fsanitize=address -fsanitize=undefined \
   test_make_safe_uri.c -o test_make_safe_uri || exit 1
@@ -162,6 +162,14 @@ if ./test_make_safe_uri | egrep '^FAIL:'; then
   exit 1
 fi
 
+echo "===> test_password_equal"
+$CC -g -O2 -fsanitize=address -fsanitize=undefined \
+  test_password_equal.c -o test_password_equal || exit 1
+if ./test_password_equal | egrep '^FAIL:'; then
+  echo test_password_equal failed >&2
+  exit 1
+fi
+
 # Check that the code builds with various defines.
 echo "===> building without -DDEBUG"
 $CC -O2 -Wall ../darkhttpd.c || exit 1

devel/test_password_equal.c

@@ -0,0 +1,30 @@
+#define main _main_disabled_
+#include "../darkhttpd.c"
+#undef main
+
+static void
+test(int equal, const char *user_input, const char *secret)
+{
+    int out = password_equal(user_input, secret);
+    printf("%s: \"%s\" \"%s\"\n",
+        (equal == out) ? "PASS" : "FAIL",
+        user_input, secret);
+}
+
+int
+main(void)
+{
+    test(1, "", "");
+    test(1, "a", "a");
+    test(1, "abc", "abc");
+
+    test(0, "a", "");
+    test(0, "ab", "");
+    test(0, "", "a");
+    test(0, "", "ab");
+    test(0, "abcd", "abc");
+    test(0, "abc", "abcd");
+    return 0;
+}
+
+/* vim:set tabstop=4 shiftwidth=4 expandtab tw=78: */