Give descriptive error messages for multiple-challenge authentication

failures:

* src/ne_auth.c (struct auth_class): Add error_noauth field.
(struct auth_protocol): Pass errmsg buffers to challenge callback; 
specify error message location for verify callback.
(challenge_error): New function.
(get_credentials, basic_challenge, continue_negotiate,
negotiate_challenge, digest_challenge): Take errmsg buffer; use
challenge_error on failure.
(verify_negotiate_response): Create errmsg buffer, pass to
continue_negotiate, and set session error string on failure.
(auth_challenge): Create error buffer and build error message during
challenge parsing.

* test/auth.c (digest_failures): Adjust for new error string.
(fail_cb, fail_challenge): Add tests for challenge parse failures.


git-svn-id: http://svn.webdav.org/repos/projects/neon/trunk@1124 61a7d7f5-40b7-0310-9c16-bb0ea8cb1845

1 files changed, 113 insertions, 54 deletions

diff --git a/src/ne_auth.c b/src/ne_auth.c
index 90e031e..d69ce8b 100644
--- a/src/ne_auth.c
+++ b/src/ne_auth.c
@@ -47,6 +47,7 @@
 #include <openssl/rand.h>
 #endif
 
+#include <errno.h>
 #include <time.h>
 
 #include "ne_md5.h"
@@ -116,15 +117,20 @@ struct auth_challenge {
 
 static const struct auth_class {
     const char *id, *req_hdr, *resp_hdr, *resp_info_hdr;
-    int status_code, fail_code;
+    int status_code; /* Response status-code to trap. */
+    int fail_code;   /* NE_* request to fail with. */
+    const char *error_noauth; /* Error message template use when
+                               * giving up authentication attempts. */
 } ah_server_class = {
     HOOK_SERVER_ID,
     "Authorization", "WWW-Authenticate", "Authentication-Info",
-    401, NE_AUTH
+    401, NE_AUTH,
+    N_("Could not authenticate to server: %s")
 }, ah_proxy_class = {
     HOOK_PROXY_ID,
     "Proxy-Authorization", "Proxy-Authenticate", "Proxy-Authentication-Info",
-    407, NE_PROXYAUTH 
+    407, NE_PROXYAUTH,
+    N_("Could not authenticate to proxy server: %s")
 };
 
 /* Authentication session state. */
@@ -210,22 +216,31 @@ struct auth_protocol {
     
     /* Parse the authentication challenge; returns zero on success, or
      * non-zero if this challenge be handled.  'attempt' is the number
-     * of times the request has been resent due to auth challenges. */
+     * of times the request has been resent due to auth challenges.
+     * On failure, challenge_error() should be used to append an error
+     * message to the error buffer 'errmsg'. */
     int (*challenge)(auth_session *sess, int attempt,
-                     struct auth_challenge *chall);
+                     struct auth_challenge *chall, ne_buffer **errmsg);
 
     /* Return the string to send in the -Authenticate request header:
      * (ne_malloc-allocated, NUL-terminated string) */
     char *(*response)(auth_session *sess, struct auth_request *req);
     
-    /* Parse a Authentication-Info response; returns NE_* error
-     * code. */
+    /* Parse a Authentication-Info response; returns NE_* error code
+     * on failure; on failure, the session error string must be
+     * set. */
     int (*verify)(struct auth_request *req, auth_session *sess,
                   const char *value);
     
     int flags; /* AUTH_FLAG_* flags */
 };
 
+/* Helper function to append an error to the buffer during challenge
+ * handling.  Pass printf-style string.  *errmsg may be NULL and is
+ * allocated if necessary.  errmsg must be non-NULL. */
+static void challenge_error(ne_buffer **errmsg, const char *fmt, ...)
+    ne_attribute((format(printf, 2, 3)));
+
 static void clean_session(auth_session *sess) 
 {
     if (sess->basic) ne_free(sess->basic);
@@ -310,22 +325,34 @@ static char *get_cnonce(void)
     return ne_strdup(ret);
 }
 
-static int get_credentials(auth_session *sess, int attempt,
+/* Callback to retrieve user credentials for given session on given
+ * attempt (pre request) for given challenge.  Password is written to
+ * pwbuf (of size NE_ABUFSIZ.  On error, challenge_error() is used
+ * with errmsg. */
+static int get_credentials(auth_session *sess, ne_buffer **errmsg, int attempt,
                            struct auth_challenge *chall, char *pwbuf) 
 {
-    return chall->handler->creds(chall->handler->userdata, sess->realm, 
-                                 attempt, sess->username, pwbuf);
+    if (chall->handler->creds(chall->handler->userdata, sess->realm, 
+                              attempt, sess->username, pwbuf) == 0) {
+        return 0;
+    } else {
+        challenge_error(errmsg, _("rejected %s challenge"), 
+                        chall->protocol->name);
+        return -1;
+    }
 }
 
 /* Examine a Basic auth challenge.
  * Returns 0 if an valid challenge, else non-zero. */
 static int basic_challenge(auth_session *sess, int attempt,
-                           struct auth_challenge *parms) 
+                           struct auth_challenge *parms,
+                           ne_buffer **errmsg) 
 {
     char *tmp, password[NE_ABUFSIZ];
 
     /* Verify challenge... must have a realm */
     if (parms->realm == NULL) {
+        challenge_error(errmsg, _("missing realm in Basic challenge"));
 	return -1;
     }
 
@@ -333,7 +360,7 @@ static int basic_challenge(auth_session *sess, int attempt,
     
     sess->realm = ne_strdup(parms->realm);
 
-    if (get_credentials(sess, attempt, parms, password)) {
+    if (get_credentials(sess, errmsg, attempt, parms, password)) {
 	/* Failed to get credentials */
 	return -1;
     }
@@ -407,7 +434,8 @@ static void make_gss_error(ne_buffer *buf, int *flag,
 
 /* Continue a GSS-API Negotiate exchange, using input TOKEN if
  * non-NULL.  Returns non-zero on error. */
-static int continue_negotiate(auth_session *sess, const char *token)
+static int continue_negotiate(auth_session *sess, const char *token,
+                              ne_buffer **errmsg)
 {
     unsigned int major, minor;
     gss_buffer_desc input = GSS_C_EMPTY_BUFFER;
@@ -418,8 +446,7 @@ static int continue_negotiate(auth_session *sess, const char *token)
     if (token) {
         input.length = ne_unbase64(token, &bintoken);
         if (input.length == 0) {
-            NE_DEBUG(NE_DBG_HTTPAUTH, "gssapi: Invalid input [%s].\n",
-                     token);
+            challenge_error(errmsg, _("invalid Negotiate token"));
             return -1;
         }
         input.value = bintoken;
@@ -440,15 +467,12 @@ static int continue_negotiate(auth_session *sess, const char *token)
     if (bintoken) ne_free(bintoken);
 
     if (GSS_ERROR(major)) {
-        ne_buffer *err = ne_buffer_create();
         int flag = 0;
 
-        make_gss_error(err, &flag, major, GSS_C_GSS_CODE);
-        make_gss_error(err, &flag, minor, GSS_C_MECH_CODE);
-        NE_DEBUG(NE_DBG_HTTPAUTH, "gssapi: Error: %s\n", err->data);
-        ne_set_error(sess->sess, _("GSSAPI authentication error (%s)"), 
-                     err->data);
-        ne_buffer_destroy(err);
+        challenge_error(errmsg, _("GSSAPI authentication error: "));
+        make_gss_error(*errmsg, &flag, major, GSS_C_GSS_CODE);
+        make_gss_error(*errmsg, &flag, minor, GSS_C_MECH_CODE);
+
         return -1;
     }
 
@@ -458,7 +482,7 @@ static int continue_negotiate(auth_session *sess, const char *token)
         ret = 0;
     } 
     else {
-        NE_DEBUG(NE_DBG_HTTPAUTH, "gssapi: Init failure %d.\n", major);
+        challenge_error(errmsg, _("GSSAPI failure (code %u)"), major);
         ret = -1;
     }
 
@@ -482,18 +506,18 @@ static int continue_negotiate(auth_session *sess, const char *token)
 /* Process a Negotiate challange CHALL in session SESS; returns zero
  * if challenge is accepted. */
 static int negotiate_challenge(auth_session *sess, int attempt,
-                               struct auth_challenge *chall) 
+                               struct auth_challenge *chall,
+                               ne_buffer **errmsg) 
 {
     const char *token = chall->opaque;
 
     /* Respect an initial challenge - which must have no input token,
      * or a continuation - which must have an input token. */
     if (attempt == 0 || token) {
-        return continue_negotiate(sess, token);
+        return continue_negotiate(sess, token, errmsg);
     }
     else {
-        NE_DEBUG(NE_DBG_HTTPAUTH, "gssapi: Ignoring empty Negotiate "
-                 "challenge (attempt=%d).\n", attempt);
+        challenge_error(errmsg, _("ignoring empty Negotiate continuation"));
         return -1;
     }
 }
@@ -505,6 +529,7 @@ static int verify_negotiate_response(struct auth_request *req, auth_session *ses
     char *duphdr = ne_strdup(hdr);
     char *sep, *ptr = strchr(duphdr, ' ');
     int ret;
+    ne_buffer *errmsg;
 
     if (strncmp(hdr, "Negotiate", ptr - hdr) != 0) {
         NE_DEBUG(NE_DBG_HTTPAUTH, "gssapi: Not a Negotiate response!\n");
@@ -526,10 +551,12 @@ static int verify_negotiate_response(struct auth_request *req, auth_session *ses
         *sep = '\0';
 
     NE_DEBUG(NE_DBG_HTTPAUTH, "gssapi: Negotiate response token [%s]\n", ptr);
-    ret = continue_negotiate(sess, ptr);
+    ret = continue_negotiate(sess, ptr, &errmsg);
     if (ret) {
-        NE_DEBUG(NE_DBG_HTTPAUTH, "gssapi: Mutual auth failed.\n");
+        ne_set_error(sess->sess, _("Negotiate response verification failure: %s"),
+                     errmsg->data);
     }
+    ne_buffer_destroy(errmsg);
     ne_free(duphdr);
     return ret ? NE_ERROR : NE_OK;
 }
@@ -542,7 +569,8 @@ static char *request_sspi(auth_session *sess, struct auth_request *request)
 }
 
 static int sspi_challenge(auth_session *sess, int attempt,
-                          struct auth_challenge *parms) 
+                          struct auth_challenge *parms,
+                          ne_buffer **errmsg) 
 {
     int ntlm = ne_strcasecmp(parms->protocol->name, "NTLM") == 0;
     int status;
@@ -580,24 +608,21 @@ static int sspi_challenge(auth_session *sess, int attempt,
 /* Examine a digest challenge: return 0 if it is a valid Digest challenge,
  * else non-zero. */
 static int digest_challenge(auth_session *sess, int attempt,
-                            struct auth_challenge *parms) 
+                            struct auth_challenge *parms,
+                            ne_buffer **errmsg) 
 {
     char password[NE_ABUFSIZ];
 
     if (parms->alg == auth_alg_unknown) {
-        ne_set_error(sess->sess, _("Unknown algorithm in Digest "
-                                   "authentication challenge"));
+        challenge_error(errmsg, _("unknown algorithm in Digest challenge"));
         return -1;
     }
     else if (parms->alg == auth_alg_md5_sess && !parms->qop_auth) {
-        ne_set_error(sess->sess, _("Incompatible algorithm in Digest "
-                                   "authentication challenge"));
+        challenge_error(errmsg, _("incompatible algorithm in Digest challenge"));
         return -1;
     }
     else if (parms->realm == NULL || parms->nonce == NULL) {
-	NE_DEBUG(NE_DBG_HTTPAUTH, "auth: Digest challenge missing parms.\n");
-        ne_set_error(sess->sess, _("Missing nonce or realm in Digest "
-                                   "authentication challenge"));
+        challenge_error(errmsg, _("missing parameter in Digest challenge"));
 	return -1;
     }
 
@@ -608,7 +633,7 @@ static int digest_challenge(auth_session *sess, int attempt,
 	sess->realm = ne_strdup(parms->realm);
 
 	/* Not a stale response: really need user authentication */
-	if (get_credentials(sess, attempt, parms, password)) {
+	if (get_credentials(sess, errmsg, attempt, parms, password)) {
 	    /* Failed to get credentials */
 	    return -1;
 	}
@@ -861,12 +886,7 @@ static int verify_digest_response(struct auth_request *req, auth_session *sess,
 	    cnonce = val;
 	} else if (ne_strcasecmp(key, "nc") == 0) { 
 	    nc = val;
-	    if (sscanf(val, "%x", &nonce_count) != 1) {
-		NE_DEBUG(NE_DBG_HTTPAUTH, "Couldn't find nonce count.\n");
-	    } else {
-		NE_DEBUG(NE_DBG_HTTPAUTH, "Got nonce_count: %u\n", nonce_count);
-	    }
-	}
+        }
     }
 
     if (qop == auth_qop_none) {
@@ -884,14 +904,27 @@ static int verify_digest_response(struct auth_request *req, auth_session *sess,
         ne_set_error(sess->sess, _("Digest mutual authentication failure: "
                                    "client nonce mismatch"));
     }
-    else if (nonce_count != sess->nonce_count) {
-        ret = NE_ERROR;
-        ne_set_error(sess->sess, _("Digest mutual authentication failure: "
-                                   "nonce count mismatch (%u not %u)"),
-                     nonce_count, sess->nonce_count);
+    else if (nc) {
+        char *ptr;
+        
+        errno = 0;
+        nonce_count = strtoul(nc, &ptr, 16);
+        if (*ptr != '\0' || errno) {
+            ret = NE_ERROR;
+            ne_set_error(sess->sess, _("Digest mutual authentication failure: "
+                                       "could not parse nonce count"));
+        }
+        else if (nonce_count != sess->nonce_count) {
+            ret = NE_ERROR;
+            ne_set_error(sess->sess, _("Digest mutual authentication failure: "
+                                       "nonce count mismatch (%u not %u)"),
+                         nonce_count, sess->nonce_count);
+        }
     }
-    else {
-        /* Verify the response-digest field */
+
+    /* Finally, for qop=auth cases, if everything else is OK, verify
+     * the response-digest field. */    
+    if (qop == auth_qop_auth && ret == NE_OK) {
         struct ne_md5_ctx *a2;
         char a2_md5_ascii[33], rdig_md5_ascii[33];
 
@@ -996,6 +1029,25 @@ static struct auth_challenge *insert_challenge(struct auth_challenge **list,
     return ret;
 }
 
+static void challenge_error(ne_buffer **errbuf, const char *fmt, ...)
+{
+    char err[128];
+    va_list ap;
+    size_t len;
+    
+    va_start(ap, fmt);
+    len = ne_vsnprintf(err, sizeof err, fmt, ap);
+    va_end(ap);
+    
+    if (*errbuf == NULL) {
+        *errbuf = ne_buffer_create();
+        ne_buffer_append(*errbuf, err, len);
+    }
+    else {
+        ne_buffer_concat(*errbuf, _(", "), err, NULL);
+    }
+}
+
 /* Passed the value of a "(Proxy,WWW)-Authenticate: " header field.
  * Returns 0 if valid challenge was accepted; non-zero if no valid
  * challenge was found. */
@@ -1004,6 +1056,7 @@ static int auth_challenge(auth_session *sess, int attempt,
 {
     char *pnt, *key, *val, *hdr, sep;
     struct auth_challenge *chall = NULL, *challenges = NULL;
+    ne_buffer *errmsg = NULL;
 
     pnt = hdr = ne_strdup(value); 
 
@@ -1030,8 +1083,9 @@ static int auth_challenge(auth_session *sess, int attempt,
             }
 
             if (proto == NULL) {
-		NE_DEBUG(NE_DBG_HTTPAUTH, "auth: Ignoring '%s' challenge.\n", key);
+                /* Ignore this challenge. */
                 chall = NULL;
+                challenge_error(&errmsg, _("ignored %s challenge"), key);
                 continue;
 	    }
             
@@ -1096,7 +1150,9 @@ static int auth_challenge(auth_session *sess, int attempt,
     for (chall = challenges; chall != NULL; chall = chall->next) {
         NE_DEBUG(NE_DBG_HTTPAUTH, "auth: Trying %s challenge...\n",
                  chall->protocol->name);
-        if (chall->protocol->challenge(sess, attempt, chall) == 0) {
+        if (chall->protocol->challenge(sess, attempt, chall, &errmsg) == 0) {
+            NE_DEBUG(NE_DBG_HTTPAUTH, "auth: Accepted %s challenge.\n", 
+                     chall->protocol->name);
             sess->protocol = chall->protocol;
             break;
         }
@@ -1104,6 +1160,8 @@ static int auth_challenge(auth_session *sess, int attempt,
 
     if (!sess->protocol) {
         NE_DEBUG(NE_DBG_HTTPAUTH, "auth: No challenges accepted.\n");
+        ne_set_error(sess->sess, _(sess->spec->error_noauth),
+                     errmsg ? errmsg->data : _("could not parse challenge"));
     }
 
     while (challenges != NULL) {
@@ -1113,6 +1171,7 @@ static int auth_challenge(auth_session *sess, int attempt,
     }
 
     ne_free(hdr);
+    if (errmsg) ne_buffer_destroy(errmsg);
 
     return !(sess->protocol != NULL);
 }