From 9a767a901154c4fd56d3a70933aefe03cb594126 Mon Sep 17 00:00:00 2001 From: Joe Orton Date: Mon, 6 Jul 2020 21:31:27 +0100 Subject: [PATCH] Handle Digest challenges in algorithm strength order (issue #26): * src/ne_auth.c (insert_challenge): Take the challenge as an argument and adjust to sort by Digest algorithm as well as challenge strength. (auth_challenge): Adjust to allocate challenge first, and insert later as above. * test/auth.c (init): New function. (multi_rfc7616): New test case. --- src/ne_auth.c | 53 ++++++++++++++++++++-------------- test/auth.c | 80 +++++++++++++++++++++++++++++++++++++++++++++------ 2 files changed, 102 insertions(+), 31 deletions(-) diff --git a/src/ne_auth.c b/src/ne_auth.c index a5411333..e989faff 100644 --- a/src/ne_auth.c +++ b/src/ne_auth.c @@ -1275,33 +1275,31 @@ static const struct auth_protocol protocols[] = { { 0 } }; -/* Insert a new auth challenge for protocol 'proto' in list of - * challenges 'list'. The challenge list is kept in sorted order of - * strength, with highest strength first. */ -static struct auth_challenge *insert_challenge(struct auth_challenge **list, - const struct auth_protocol *proto) +/* Insert a new auth challenge 'chall' into list of challenges 'list'. + * The challenge list is kept in sorted order of strength, with + * highest strength first. */ +static void insert_challenge(struct auth_challenge **list, struct auth_challenge *chall) { - struct auth_challenge *ret = ne_calloc(sizeof *ret); - struct auth_challenge *chall, *prev; - - for (chall = *list, prev = NULL; chall != NULL; - prev = chall, chall = chall->next) { - if (proto->strength > chall->protocol->strength) { + struct auth_challenge *cur, *prev; + + for (cur = *list, prev = NULL; cur != NULL; + prev = cur, cur = cur->next) { + if (chall->protocol->strength > cur->protocol->strength + || (cur->protocol->id == NE_AUTH_DIGEST + && chall->protocol->id == NE_AUTH_DIGEST + && chall->alg > cur->alg)) { break; } } if (prev) { - ret->next = prev->next; - prev->next = ret; - } else { - ret->next = *list; - *list = ret; + chall->next = prev->next; + prev->next = chall; + } + else { + chall->next = *list; + *list = chall; } - - ret->protocol = proto; - - return ret; } static void challenge_error(ne_buffer **errbuf, const char *fmt, ...) @@ -1342,10 +1340,18 @@ static int auth_challenge(auth_session *sess, int attempt, while (!tokenize(&pnt, &key, &val, &sep, 1)) { if (val == NULL) { + /* Special case, challenge token, not key=value pair: */ const struct auth_protocol *proto = NULL; struct auth_handler *hdl; size_t n; + /* Accumulated challenge is now completed and can be + * inserted into the list. */ + if (chall) { + insert_challenge(&challenges, chall); + chall = NULL; + } + for (hdl = sess->handlers; hdl; hdl = hdl->next) { for (n = 0; protocols[n].id; n++) { if (protocols[n].id & hdl->protomask @@ -1359,13 +1365,13 @@ static int auth_challenge(auth_session *sess, int attempt, if (proto == NULL) { /* Ignore this challenge. */ - chall = NULL; challenge_error(&errmsg, _("ignored %s challenge"), key); continue; } NE_DEBUG(NE_DBG_HTTPAUTH, "auth: Got '%s' challenge.\n", proto->name); - chall = insert_challenge(&challenges, proto); + chall = ne_calloc(sizeof *chall); + chall->protocol = proto; chall->handler = hdl; if ((proto->flags & AUTH_FLAG_OPAQUE_PARAM) && sep == ' ') { @@ -1442,6 +1448,9 @@ static int auth_challenge(auth_session *sess, int attempt, NE_DEBUG(NE_DBG_HTTPAUTH, "auth: Ignored bogus userhash value '%s'\n", val); } } + + /* Insert the in-flight challenge (if any). */ + if (chall) insert_challenge(&challenges, chall); sess->protocol = NULL; diff --git a/test/auth.c b/test/auth.c index b535757d..bbc8dadf 100644 --- a/test/auth.c +++ b/test/auth.c @@ -44,6 +44,8 @@ static const char *alt_username, *alt_username_star; static int auth_failed; +static int has_sha256 = 0, has_sha512_256 = 0; + #define BASIC_WALLY "Basic realm=WallyWorld" #define CHAL_WALLY "WWW-Authenticate: " BASIC_WALLY @@ -115,6 +117,21 @@ static int auth_serve(ne_socket *sock, void *userdata) return 0; } +static int init(void) +{ + char *p; + + p = ne_strhash(NE_HASH_SHA256, "", NULL); + has_sha256 = p != NULL; + if (p) ne_free(p); + + p = ne_strhash(NE_HASH_SHA512_256, "", NULL); + has_sha512_256 = p != NULL; + if (p) ne_free(p); + + return lookup_localhost(); +} + /* Test that various Basic auth challenges are correctly handled. */ static int basic(void) { @@ -935,7 +952,7 @@ static int digest(void) { NULL } }; size_t n; - + for (n = 0; parms[n].realm; n++) { CALL(test_digest(&parms[n])); @@ -955,14 +972,12 @@ static int digest_sha256(void) { NULL }, }; size_t n; - char *p = ne_strhash(NE_HASH_SHA256, "", NULL); - if (p == NULL) { + if (!has_sha256) { t_context("SHA-256 not supported"); return SKIP; } - ne_free(p); - + for (n = 0; parms[n].realm; n++) { CALL(test_digest(&parms[n])); @@ -981,13 +996,11 @@ static int digest_sha512_256(void) { NULL }, }; size_t n; - char *p = ne_strhash(NE_HASH_SHA512_256, "", NULL); - if (p == NULL) { + if (!has_sha512_256) { t_context("SHA-512/256 not supported"); return SKIP; } - ne_free(p); for (n = 0; parms[n].realm; n++) { CALL(test_digest(&parms[n])); @@ -1240,6 +1253,54 @@ static int multi_handler(void) return await_server(); } +static int multi_rfc7616(void) +{ + ne_session *sess; + struct multi_context c[2]; + unsigned n; + ne_buffer *buf, *exp; + + buf = ne_buffer_create(); + CALL(make_session(&sess, single_serve_string, + "HTTP/1.1 401 Auth Denied\r\n" + "WWW-Authenticate: " + "Digest realm='sha512-realm', algorithm=SHA-512-256, qop=auth, nonce=gaga, " + "Basic realm='basic-realm', " + "Digest realm='md5-realm', algorithm=MD5, qop=auth, nonce=gaga, " + "Digest realm='sha256-realm', algorithm=SHA-256, qop=auth, nonce=gaga\r\n" + "Content-Length: 0\r\n" "\r\n")); + + for (n = 0; n < 2; n++) { + c[n].buf = buf; + c[n].id = n + 1; + } + + ne_add_server_auth(sess, NE_AUTH_BASIC, multi_cb, &c[0]); + ne_add_server_auth(sess, NE_AUTH_DIGEST, multi_cb, &c[1]); + + any_request(sess, "/fish"); + + exp = ne_buffer_create(); + n = 0; + if (has_sha512_256) + ne_buffer_snprintf(exp, 100, "[id=2, realm=sha512-realm, tries=%u]", n++); + if (has_sha256) + ne_buffer_snprintf(exp, 100, "[id=2, realm=sha256-realm, tries=%u]", n++); + ne_buffer_snprintf(exp, 100, + "[id=2, realm=md5-realm, tries=%u]" + "[id=1, realm=basic-realm, tries=0]", n); + ONV(strcmp(exp->data, buf->data), + ("unexpected callback ordering.\n" + "expected: %s\n" + "actual: %s\n", + exp->data, buf->data)); + + ne_session_destroy(sess); + ne_buffer_destroy(buf); + + return await_server(); +} + static int domains(void) { ne_session *sess; @@ -1370,7 +1431,7 @@ static int forget(void) /* proxy auth, proxy AND origin */ ne_test tests[] = { - T(lookup_localhost), + T(init), T(basic), T(retries), T(forget_regress), @@ -1383,6 +1444,7 @@ ne_test tests[] = { T(digest_username_star), T(fail_challenge), T(multi_handler), + T(multi_rfc7616), T(domains), T(defaults), T(CVE_2008_3746),