Fix Digest auth failure without algorithm=

Regression in 0.32.3 / 2b8e2e4e3568d10cdb3ef07b3ae4c699540479ea (issue #88)

* src/ne_auth.c (digest_challenge): Don't adjust the default algorithm
  here, fail if it is not set. Tweak test order.
  (auth_challenge): Initialize alg to MD5, leave unset for unknown values
  during parsing.

* test/auth.c (make_digest, verify_digest_header): Split qop= and
  algorithm= into two separate flags.  (digest): Add test case for
  2617-style auth without implicit algorithm.

2 files changed, 29 insertions, 21 deletions

diff --git a/src/ne_auth.c b/src/ne_auth.c
index d74d7ce..54b8fac 100644
--- a/src/ne_auth.c
+++ b/src/ne_auth.c
@@ -961,20 +961,17 @@ static int digest_challenge(auth_session *sess, int attempt,
 {
     char *p, *h_urp = NULL;
 
-    /* Handle 2069 legacy Digest case. */
-    if (parms->alg == NULL && !parms->got_qop) {
-        if ((parms->handler->protomask & NE_AUTH_LEGACY_DIGEST) == 0) {
-            challenge_error(errmsg, _("legacy Digest challenge not supported"));
-            return -1;
-        }
-        /* It can only be MD5. */
-        parms->alg = HASHALG_MD5;
-    }
-
     if (parms->alg == NULL) {
         challenge_error(errmsg, _("unknown algorithm in Digest challenge"));
         return -1;
     }
+
+    /* qop= is mandatory from 2617 onward, fail w/o LEGACY_DIGEST */
+    if (!parms->got_qop
+        && ((parms->handler->protomask & NE_AUTH_LEGACY_DIGEST) == 0)) {
+        challenge_error(errmsg, _("legacy Digest challenge not supported"));
+        return -1;
+    }
     else if (parms->alg->sess && !parms->qop_auth) {
         challenge_error(errmsg, _("incompatible algorithm in Digest challenge"));
         return -1;
@@ -1483,6 +1480,7 @@ static int auth_challenge(auth_session *sess, int attempt, const char *uri,
             chall = ne_calloc(sizeof *chall);
             chall->protocol = proto;
             chall->handler = hdl;
+            chall->alg = HASHALG_MD5; /* RFC default is MD5 */
 
             if ((proto->flags & AUTH_FLAG_OPAQUE_PARAM) && sep == ' ') {
                 /* Cope with the fact that the unquoted base64
@@ -1515,13 +1513,16 @@ static int auth_challenge(auth_session *sess, int attempt, const char *uri,
 	} else if (ne_strcasecmp(key, "algorithm") == 0) {
             unsigned int n;
 
-            chall->alg = NULL;
+            chall->alg = NULL; /* left unset for unknown algorithm. */
             for (n = 0; n < NUM_HASHALGS; n++) {
                 if (ne_strcasecmp(val, hashalgs[n].name) == 0) {
                     chall->alg = &hashalgs[n];
                     break;
                 }
             }
+
+            NE_DEBUG(NE_DBG_HTTPAUTH, "auth: Mapped '%s' to algorithm %s\n", val,
+                     chall->alg ? chall->alg->name : "[unknown]");
 	} else if (ne_strcasecmp(key, "qop") == 0) {
             /* iterate over each token in the value */
             do {
diff --git a/test/auth.c b/test/auth.c
index 96474e3..59b2ff8 100644
--- a/test/auth.c
+++ b/test/auth.c
@@ -400,13 +400,15 @@ static void dup_header(char *header)
 
 #define PARM_PROXY     (0x0001)
 #define PARM_NEXTNONCE (0x0002)
-#define PARM_RFC2617   (0x0004)
+#define PARM_ALG       (0x0004) /* use algorithm= */
 #define PARM_AINFO     (0x0008)
 #define PARM_USERHASH  (0x0010) /* userhash=true */
 #define PARM_UHFALSE   (0x0020) /* userhash=false */
 #define PARM_ALTUSER   (0x0040)
 #define PARM_LEGACY      (0x0080)
 #define PARM_LEGACY_ONLY (0x0100)
+#define PARM_QOP       (0x0200) /* use qop= */
+#define PARM_RFC2617   (0x0204) /* use algorithm= and qop= */
 
 struct digest_parms {
     const char *realm, *nonce, *opaque, *domain;
@@ -497,7 +499,7 @@ static char *make_digest(struct digest_state *state, struct digest_parms *parms,
 
     h_a2 = hash(parms, !auth_info ? state->method : "", ":", state->uri, NULL);
 
-    if (parms->flags & PARM_RFC2617) {
+    if (parms->flags & PARM_QOP) {
         rv = hash(parms,
                   h_a1, ":", state->nonce, ":",
                   state->ncval, ":", state->cnonce, ":", state->qop, ":",
@@ -606,7 +608,7 @@ static int verify_digest_header(struct digest_state *state,
     }
 
     ONN("cnonce param missing or short for 2617-style auth",
-        (parms->flags & PARM_RFC2617)
+        (parms->flags & PARM_QOP)
         && (newstate.cnonce == NULL
             || strlen(newstate.cnonce) < 32));
 
@@ -633,7 +635,7 @@ static int verify_digest_header(struct digest_state *state,
     DIGCMP(opaque);
     DIGCMP(algorithm);
 
-    if (parms->flags & PARM_RFC2617) {
+    if (parms->flags & PARM_QOP) {
         DIGCMP(qop);
     }
         
@@ -685,7 +687,7 @@ static char *make_authinfo_header(struct digest_state *state,
 
     ne_buffer_czappend(buf, "Authentication-Info: ");
 
-    if ((parms->flags & PARM_RFC2617) == 0) {
+    if ((parms->flags & PARM_QOP) == 0) {
         ne_buffer_concat(buf, "rspauth=\"", digest, "\"", NULL);
     } else {
         if (parms->failure != fail_ai_omit_nc) {
@@ -725,9 +727,12 @@ static char *make_digest_header(struct digest_state *state,
                      ": Digest "
                      "realm=\"", parms->realm, "\", ", NULL);
     
-    if (parms->flags & PARM_RFC2617) {
-        ne_buffer_concat(buf, "algorithm=\"", algorithm, "\", ",
-                         "qop=\"", state->qop, "\", ", NULL);
+    if (parms->flags & PARM_ALG) {
+        ne_buffer_concat(buf, "algorithm=\"", algorithm, "\", ", NULL);
+    }
+
+    if (parms->flags & PARM_QOP) {
+        ne_buffer_concat(buf, "qop=\"", state->qop, "\", ", NULL);
     }
 
     if (parms->opaque) {
@@ -904,7 +909,7 @@ static int test_digest(struct digest_parms *parms)
     NE_DEBUG(NE_DBG_HTTP, ">>>> Request sequence begins "
              "(reqs=%d, nonce=%s, rfc=%s, stale=%d, proxy=%d).\n",
              parms->num_requests,
-             parms->nonce, (parms->flags & PARM_RFC2617) ? "2617" : "2069",
+             parms->nonce, (parms->flags & PARM_QOP) ? "2617" : "2069",
              parms->stale, !!(parms->flags & PARM_PROXY));
 
     if ((parms->flags & PARM_PROXY)) {
@@ -933,6 +938,8 @@ static int digest(void)
     struct digest_parms parms[] = {
         /* RFC 2617-style */
         { "WallyWorld", "this-is-a-nonce", NULL, NULL, ALG_MD5, PARM_RFC2617, 1, 0, fail_not },
+        /* Leaving algorithm= optional. */
+        { "WallyWorld", "this-is-a-nonce", NULL, NULL, ALG_MD5, PARM_QOP, 1, 0, fail_not },
         { "WallyWorld", "this-is-also-a-nonce", "opaque-string", NULL, ALG_MD5, PARM_RFC2617, 1, 0, fail_not },
         /* ... with A-I */
         { "WallyWorld", "nonce-nonce-nonce", "opaque-string", NULL, ALG_MD5, PARM_RFC2617 | PARM_AINFO, 1, 0, fail_not },
@@ -1209,7 +1216,7 @@ static int fail_challenge(void)
         CALL(multi_session_server(&sess, "http", "localhost",
                                   2, single_serve_string, resp));
 
-        ne_set_server_auth(sess, fail_cb, buf);
+        ne_add_server_auth(sess, NE_AUTH_ALL|NE_AUTH_LEGACY_DIGEST, fail_cb, buf);
         
         ret = any_2xx_request(sess, "/fish");
         ONV(ret == NE_OK,