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.

2 files changed, 102 insertions, 31 deletions

diff --git a/src/ne_auth.c b/src/ne_auth.c
index a541133..e989faf 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 b535757..bbc8dad 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),