[mod_auth] fix Digest auth to be better than Basic (fixes #1844)

Make Digest authentication more compliant with RFC.

Excerpt from https://www.rfc-editor.org/rfc/rfc7616.txt Section 5.13:
    The bottom line is that any compliant implementation will be
    relatively weak by cryptographic standards, but any compliant
    implementation will be far superior to Basic Authentication.

x-ref:
  "Serious security problem in Digest Authentication"
  https://redmine.lighttpd.net/issues/1844
This commit is contained in:
Glenn Strauss 2016-07-16 23:25:53 -04:00
parent 052a049f29
commit 00cc4d7c0e
3 changed files with 72 additions and 21 deletions

View File

@ -887,6 +887,7 @@ int http_auth_digest_check(server *srv, connection *con, mod_auth_plugin_data *p
*(dkv[i].ptr) = c + dkv[i].key_len;
c += strlen(c) - 1;
}
break;
}
}
}
@ -981,6 +982,22 @@ int http_auth_digest_check(server *srv, connection *con, mod_auth_plugin_data *p
buffer_free(password);
/* detect if attacker is attempting to reuse valid digest for one uri
* on a different request uri. Might also happen if intermediate proxy
* altered client request line. (Altered request would not result in
* the same digest as that calculated by the client.) */
{
const size_t ulen = strlen(uri);
const size_t rlen = buffer_string_length(con->request.uri);
if (!buffer_is_equal_string(con->request.uri, uri, ulen)
&& !(rlen < ulen && 0 == memcmp(con->request.uri->ptr, uri, rlen) && uri[rlen] == '?')) {
log_error_write(srv, __FILE__, __LINE__, "sbssss",
"digest: auth failed: uri mismatch (", con->request.uri, "!=", uri, "), IP:", inet_ntop_cache_get_ip(srv, &(con->dst_addr)));
buffer_free(b);
return -1;
}
}
if (algorithm &&
strcasecmp(algorithm, "md5-sess") == 0) {
li_MD5_Init(&Md5Ctx);
@ -1054,6 +1071,28 @@ int http_auth_digest_check(server *srv, connection *con, mod_auth_plugin_data *p
return 0;
}
/* check age of nonce. Note that rand() is used in nonce generation
* in http_auth_digest_generate_nonce(). If that were replaced
* with nanosecond time, then nonce secret would remain unique enough
* for the purposes of Digest auth, and would be reproducible (and
* verifiable) if nanoseconds were inclued with seconds as part of the
* nonce "timestamp:secret". Since that is not done, timestamp in
* nonce could theoretically be modified and still produce same md5sum,
* but that is highly unlikely within a 10 min (moving) window of valid
* time relative to current time (now) */
{
time_t ts = 0;
const unsigned char * const nonce_uns = (unsigned char *)nonce;
for (i = 0; i < 8 && light_isxdigit(nonce_uns[i]); ++i) {
ts = (ts << 4) + hex2int(nonce_uns[i]);
}
if (i != 8 || nonce[8] != ':'
|| ts > srv->cur_ts || srv->cur_ts - ts > 600) { /*(10 mins)*/
buffer_free(b);
return -2; /* nonce is stale; have client regenerate digest */
} /*(future: might send nextnonce when expiration is imminent)*/
}
/* remember the username */
buffer_copy_string(p->auth_user, username);

View File

@ -287,7 +287,7 @@ static handler_t mod_auth_uri_handler(server *srv, connection *con, void *p_d) {
}
}
if (!auth_satisfied) {
if (1 != auth_satisfied) { /*(0 or -2)*/
data_string *method, *realm;
method = (data_string *)array_get_element(req, "method");
realm = (data_string *)array_get_element(req, "realm");
@ -311,8 +311,13 @@ static handler_t mod_auth_uri_handler(server *srv, connection *con, void *p_d) {
buffer_copy_string_len(p->tmp_buf, CONST_STR_LEN("Digest realm=\""));
buffer_append_string_buffer(p->tmp_buf, realm->value);
buffer_append_string_len(p->tmp_buf, CONST_STR_LEN("\", charset=\"UTF-8\", nonce=\""));
buffer_append_uint_hex(p->tmp_buf, (uintmax_t)srv->cur_ts);
buffer_append_string_len(p->tmp_buf, CONST_STR_LEN(":"));
buffer_append_string(p->tmp_buf, hh);
buffer_append_string_len(p->tmp_buf, CONST_STR_LEN("\", qop=\"auth\""));
if (-2 == auth_satisfied) {
buffer_append_string_len(p->tmp_buf, CONST_STR_LEN(", stale=true"));
}
response_header_insert(srv, con, CONST_STR_LEN("WWW-Authenticate"), CONST_BUF_LEN(p->tmp_buf));
} else {

View File

@ -8,7 +8,7 @@ BEGIN {
use strict;
use IO::Socket;
use Test::More tests => 19;
use Test::More tests => 20;
use LightyTest;
my $tf = LightyTest->new();
@ -133,6 +133,9 @@ EOF
$t->{RESPONSE} = [ { 'HTTP-Protocol' => 'HTTP/1.0', 'HTTP-Status' => 401 } ];
ok($tf->handle_http($t) == 0, 'Digest-Auth: missing qop, no crash');
# (Note: test case is invalid; mismatch between request line and uri="..."
# is not what is intended to be tested here, but that is what is invalid)
# https://redmine.lighttpd.net/issues/477
## this should not crash
$t->{REQUEST} = ( <<EOF
GET /server-status HTTP/1.0
@ -155,34 +158,38 @@ EOF
$t->{RESPONSE} = [ { 'HTTP-Protocol' => 'HTTP/1.0', 'HTTP-Status' => 401 } ];
ok($tf->handle_http($t) == 0, 'Basic-Auth: Invalid Base64');
$t->{REQUEST} = ( <<EOF
GET /server-status HTTP/1.0
User-Agent: Wget/1.9.1
Authorization: Digest username="jan", realm="jan",
nonce="b1d12348b4620437c43dd61c50ae4639", algorithm="md5-sess",
uri="/MJ-BONG.xm.mpc", qop=auth, noncecount=00000001",
cnonce="036FCA5B86F7E7C4965C7F9B8FE714B7",
nc="asd",
response="29B32C2953C763C6D033C8A49983B87E"
Authorization: Digest username="jan", realm="download archiv",
nonce="b3b26457000000003a9b34a3cd56d26e48a52a498ac9765d4b",
uri="/server-status", qop=auth, nc=00000001,
algorithm="md5-sess", response="049b000fb00ab51dddea6f093a96aa2e"
EOF
);
$t->{RESPONSE} = [ { 'HTTP-Protocol' => 'HTTP/1.0', 'HTTP-Status' => 401 } ];
$t->{RESPONSE} = [ { 'HTTP-Protocol' => 'HTTP/1.0', 'HTTP-Status' => 400 } ];
ok($tf->handle_http($t) == 0, 'Digest-Auth: md5-sess + missing cnonce');
$t->{REQUEST} = ( <<EOF
GET /server-status HTTP/1.0
Authorization: Digest username="jan", realm="download archiv",
nonce="b3b26457000000003a9b34a3cd56d26e48a52a498ac9765d4b",
uri="/server-status", qop=auth, nc=00000001, cnonce="65ee1b37",
algorithm="md5", response="049b000fb00ab51dddea6f093a96aa2e"
EOF
);
$t->{RESPONSE} = [ { 'HTTP-Protocol' => 'HTTP/1.0', 'HTTP-Status' => 401, 'WWW-Authenticate' => '/, stale=true$/' } ];
ok($tf->handle_http($t) == 0, 'Digest-Auth: stale nonce');
$t->{REQUEST} = ( <<EOF
GET /server-status HTTP/1.0
User-Agent: Wget/1.9.1
Authorization: Digest username="jan", realm="jan",
nonce="b1d12348b4620437c43dd61c50ae4639", algorithm="md5-sess",
uri="/MJ-BONG.xm.mpc", qop=auth, noncecount=00000001",
cnonce="036FCA5B86F7E7C4965C7F9B8FE714B7",
nc="asd",
response="29B32C2953C763C6D033C8A49983B87E"
Authorization: Digest username="jan", realm="download archiv",
nonce="b3b26457000000003a9b34a3cd56d26e48a52a498ac9765d4b",
uri="/server-status", qop=auth, nc=00000001, cnonce="65ee1b37",
algorithm="md5", response="049b000fb00ab51dddea6f093a96aa2e"
EOF
);
$t->{RESPONSE} = [ { 'HTTP-Protocol' => 'HTTP/1.0', 'HTTP-Status' => 401 } ];
ok($tf->handle_http($t) == 0, 'Digest-Auth: trailing WS');
); # note: trailing whitespace at end of request line above is intentional
$t->{RESPONSE} = [ { 'HTTP-Protocol' => 'HTTP/1.0', 'HTTP-Status' => 401, 'WWW-Authenticate' => '/, stale=true$/' } ];
ok($tf->handle_http($t) == 0, 'Digest-Auth: trailing WS, stale nonce');