Index: auth-bozo.c =================================================================== RCS file: /cvsroot/src/libexec/httpd/auth-bozo.c,v retrieving revision 1.13 diff -u -r1.13 auth-bozo.c --- auth-bozo.c 8 Jul 2014 14:01:21 -0000 1.13 +++ auth-bozo.c 16 Nov 2014 08:49:55 -0000 @@ -49,6 +49,9 @@ static ssize_t base64_decode(const unsigned char *, size_t, unsigned char *, size_t); +void *consttime_memcpy_if(void *dst, const void *src, int condition, + size_t len); + /* * Check if HTTP authentication is required */ @@ -58,9 +61,14 @@ bozohttpd_t *httpd = request->hr_httpd; struct stat sb; char dir[MAXPATHLEN], authfile[MAXPATHLEN], *basename; - char user[BUFSIZ], *pass; + char request_user[BUFSIZ], tmp_user[BUFSIZ], buf[BUFSIZ], + tmp_pass[BUFSIZ]; + char *c, *hash; + /* dummy password filled in case of no valid user found */ + char pass[BUFSIZ] = "$sha1$23859$DUMMYDYM$_INVALID_HASH_INVALID_HASH__"; + size_t i; FILE *fp; - int len; + int ret=1, found=0, match; /* get dir=dirname(file) */ snprintf(dir, sizeof(dir), "%s", file); @@ -74,7 +82,7 @@ } request->hr_authrealm = bozostrdup(httpd, dir); - if ((size_t)snprintf(authfile, sizeof(authfile), "%s/%s", dir, AUTH_FILE) >= + if ((size_t)snprintf(authfile, sizeof(authfile), "%s/%s", dir, AUTH_FILE) >= sizeof(authfile)) { return bozo_http_error(httpd, 404, request, "authfile path too long"); @@ -91,30 +99,65 @@ debug((httpd, DEBUG_NORMAL, "bozo_auth_check realm `%s' dir `%s' authfile `%s' open", dir, file, authfile)); + if (request->hr_authuser && request->hr_authpass) { - while (fgets(user, sizeof(user), fp) != NULL) { - len = strlen(user); - if (len > 0 && user[len-1] == '\n') - user[--len] = '\0'; - if ((pass = strchr(user, ':')) == NULL) - continue; - *pass++ = '\0'; + + /* Idea behind all this masquarade is to make authentication independent + * of outside input in order to protect against potential time-based + * info leak. We always take the same amount of time to + * + * (a) scan through the entrie user database + * (b) select a matching password hash, using arithmetic instead of + * branches (see: consttime_memcpy_if function). + * (c) compare a password hash + * + * If there is no matching password hash, we compare the hash of the + * input password to a dummy password hash so that an attacker cannot + * use timings to distinguish a valid username from an invalid username. + * + * The text of each username and the size and text of each password is + * kept secret from an attacker. We do not keep the size of the user + * database secret. + */ + + explicit_memset(request_user, 0, sizeof(request_user)); + (void)strlcpy(request_user, request->hr_authuser, sizeof(request_user)); + + explicit_memset(buf, 0, sizeof(buf)); + while (fgets(buf, sizeof(buf), fp) != NULL) { + explicit_memset(tmp_user, 0, sizeof(tmp_user)); + explicit_memset(tmp_pass, 0, sizeof(tmp_pass)); + for (i = 0, c = tmp_user; i < sizeof(buf); i++) { + if (buf[i] == ':') { + *c = '\0'; + c = tmp_pass; + continue; + } + *c++ = buf[i] == '\n' ? '\0' : buf[i]; + } + debug((httpd, DEBUG_NORMAL, "bozo_auth_check authfile `%s':`%s' " - "client `%s':`%s'", - user, pass, request->hr_authuser, + "client `%s':`%s'", + tmp_user, tmp_pass, request->hr_authuser, request->hr_authpass)); - if (strcmp(request->hr_authuser, user) != 0) - continue; - if (strcmp(crypt(request->hr_authpass, pass), - pass) != 0) - break; - fclose(fp); - return 0; + + /* found and match are both either 0 or 1 */ + match = consttime_memequal(tmp_user, request_user, + sizeof(tmp_user)); + match &= ~found; /* ignore match if we already found valid user */ + consttime_memcpy_if(pass, tmp_pass, match, sizeof(pass)); + found |= match; /* mark that we've alredy found a match */ + + explicit_memset(buf, 0, sizeof(buf)); } + hash = crypt(request->hr_authpass, pass); + if (consttime_memequal(hash, pass, strlen(hash))) + ret = 0; } + fclose(fp); - return bozo_http_error(httpd, 401, request, "bad auth"); + return ret ? bozo_http_error(httpd, 401, request, "bad auth") : 0; } void @@ -212,14 +255,14 @@ * Written by Luke Mewburn */ const unsigned char decodetable[] = { - 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, - 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, - 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 62, 255, 255, 255, 63, - 52, 53, 54, 55, 56, 57, 58, 59, 60, 61, 255, 255, 255, 0, 255, 255, - 255, 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, - 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 255, 255, 255, 255, 255, - 255, 26, 27, 28, 29, 30, 31, 32, 33, 34, 35, 36, 37, 38, 39, 40, - 41, 42, 43, 44, 45, 46, 47, 48, 49, 50, 51, 255, 255, 255, 255, 255, + 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, + 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, + 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 62, 255, 255, 255, 63, + 52, 53, 54, 55, 56, 57, 58, 59, 60, 61, 255, 255, 255, 0, 255, 255, + 255, 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, + 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 255, 255, 255, 255, 255, + 255, 26, 27, 28, 29, 30, 31, 32, 33, 34, 35, 36, 37, 38, 39, 40, + 41, 42, 43, 44, 45, 46, 47, 48, 49, 50, 51, 255, 255, 255, 255, 255, }; static ssize_t @@ -254,4 +297,30 @@ cp--,i--; return (cp - out); } + +/* + * consttime_memcpy_if - function takes condition which indicates if memory + * stored under src should be copied. Function copies len bytes. Returns + * condition value. Time taken by this function depends only on len parameter. + * + * WARNING: condition MUST be 1 or 0. Function may return incorrect results + * for other values. + */ + +void * +consttime_memcpy_if(void *dst, const void *src, int condition, size_t len) +{ + unsigned char *dp = dst; + const unsigned char *sp = src; + unsigned char mask; + size_t i; + + /* set mask to 0xFF if condition is true and to 0 if it's false */ + mask = 0-condition; + + for (i = 0; i < len; i++) + dp[i] = (sp[i]&mask) | (dp[i]&(~mask)); + + return dst; +} #endif /* DO_HTPASSWD */