Re: const time authentication in bozohttpd

   Date: Wed, 25 Jun 2014 07:35:12 +0000
   From: shm <[email protected]>

    bozohttpd currently checks password using strcmp, which may leak information
   about compared data, my patch [1] introduces following countermeasures:

A couple general comments first:

    - username / password is now compared using introduced timing safe function
      (which run time depends on the known string)

The consttime_strcmp you have written does not satisfy the properties
you documented it to have.  You say it should not leak anything about
the `unknown' string including its length, but you pass the `unknown'
string to strlen, which runs in time proportional to the length of the
string.

It also doesn't behave like strcmp, in that it returns equal/not-equal
rather than less/equal/greater.  For this reason, we chose to adopt
the name consttime_memequal rather than consttime_memcmp or
consttime_bcmp.

    - remove username/password from the memory as it's no longer needed

It seems to me there are likely to be other places in memory where
these get stored, particularly stdio buffers.  You could use a
separate process to do this, but you should first identify the threat
model that this is defending against, which is not clear to me.

For example, if your threat model includes heartbleed, then you almost
certainly need a separate process.  If your threat model includes
remote code execution in the bozohttpd process, even that won't help.

    - avoid username leak by checking all records in the auth file (previously
      when we found valid username but user sent invalid password we break from
      the loop, so other records from the .htpassword weren't checked, thus an
      attacker basing on response time could be able to figure out if username
      exists) by checking all records in the file.

This seems to be the part you're actually worried about: you want to
prevent an attacker who can make arbitrary HTTP requests over the
network from enumerating the valid usernames.

The loop, as you have written it, is not going to be constant-time,
because it has data-dependent branches in it (if (valid_pass == NULL),
for example).

Here's how you might rewrite it to avoid that.  Since you have a
maximum username length, you can use consttime_memequal here.  The
call to strlen should run in time independent of the requst assuming
that crypt(3) is sensible (which is an assumption worth scrutinizing,
since crypt(3) is not a shining paragon of crypto engineering).

char htpasswd_user[BUFSIZ], *htpasswd_pass, *hash;
char request_user[BUFSIZ];
unsigned ok = 0;

(void)memset(request_user, 0, sizeof request_user);
(void)strlcpy(request_user, request->hr_authuser, sizeof request_user);
while (fgets(htpasswd_user, sizeof htpasswd_user, fp) != NULL) {
	...parse into htpasswd_user and htpasswd_pass and pad htpasswd_user...
	hash = crypt(request->hr_authpass, htpasswd_pass);
	ok |= consttime_memequal(htpasswd_user, request_user, BUFSIZ) &
	    consttime_memequal(hash, htpasswd_pass, strlen(hash) + 1);
}

return ok? 0 : bozo_http_error(httpd, 401, "bad auth");

As a minor aside, this lets you have multiple passwords for one
username, which the previous code did not allow -- it required the
first password for any given username.  With a little extra work you
could restore the old semantics, but I'm not sure it matters.