Add new auth credentials callback which unifies server/proxy auth,

takes only UTF-8 usernames, takes a larger password buffer, and
has improved semantics for the attempt counter.

* src/ne_auth.h: Drop NE_AUTH_UTF8, add NE_AUTH_PROXY.
  Add ne_add_auth, ne_auth_provide type.

* src/ne_auth.c (struct auth_handler): Add new_creds field.
  (ABUFSIZE): Define constant.
  (auth_session): Use ABUFSIZE for username buffer.
  (get_credentials): Invoke new_creds callback if available,
  using passed-in attempt counter.
  (basic_challenge): Use ABUFSIZE for password buffer.
  (digest_challenge): Reject non-ASCII usernames if new_creds
  callback is not used.
  (auth_register): Take new_creds callback.
  (ne_set_server_auth, ne_set_proxy_auth,
   ne_add_server_auth, ne_add_proxy_auth): Adjust for above.
  (ne_add_auth): New function.

* test/auth.c (multi_provider_cb, serve_provider, multi_provider):
  New test case.

6 files changed, 206 insertions, 50 deletions

diff --git a/NEWS b/NEWS
index d71bf81..1d27418 100644
--- a/NEWS
+++ b/NEWS
@@ -4,18 +4,19 @@ Changes in release 0.32.0:
  - NE_AUTH_DIGEST now only enables RFC 2617/7616 auth by default;
    to enable weaker RFC 2069 Digest, use NE_AUTH_LEGACY_DIGEST
    (this is a security enhancement, not an API/ABI break)
-* API clarification:
- - for ne_auth.h, the character encoding of the username provided
-   by a credentials callback was unspecified.  Callers providing
-   non-alphanumeric usernames for Basic and Digest challenges must
-   now use the NE_AUTH_UTF8 flag, or the auth challenge will fail
-   if the username cannot be sent safely.
+* Interface clarifications:
+ - ne_auth.h: use of non-ASCII usernames with the ne_auth_creds
+   callback type is now rejected for Digest auth since the
+   encoding is not known.  ne_add_auth() can be used instead.
 * New interfaces and features:
- - ne_string.h: added ne_strhash, ne_vstrhash, ne_strparam
+ - ne_string.h: added ne_strhash(), ne_vstrhash(), ne_strparam()
  - ne_auth.h: added RFC 7616 (Digest authentication) support,
    including userhash=, username*= and SHA-2-256/512-256 algorithms
    (SHA-2 requires GnuTLS/OpenSSL).  added NE_AUTH_WEAK_DIGEST
    to re-enable RFC 2069 Digest support
+ - ne_auth.h: added ne_add_auth() unified auth callback interface,
+   accepts (only) UTF-8 usernames, uses a larger password buffer,
+   and has different/improved attempt counter semantics.
 
 Changes in release 0.31.0:
 * Interface changes:
diff --git a/src/ne_auth.c b/src/ne_auth.c
index fb1f103..1b397f5 100644
--- a/src/ne_auth.c
+++ b/src/ne_auth.c
@@ -1,6 +1,6 @@
 /* 
    HTTP Authentication routines
-   Copyright (C) 1999-2011, Joe Orton <joe@manyfish.co.uk>
+   Copyright (C) 1999-2020, Joe Orton <joe@manyfish.co.uk>
 
    This library is free software; you can redistribute it and/or
    modify it under the terms of the GNU Library General Public
@@ -129,7 +129,8 @@ typedef enum {
 struct auth_handler {
     unsigned protomask; 
 
-    ne_auth_creds creds;
+    ne_auth_creds old_creds;
+    ne_auth_provide new_creds;
     void *userdata;
     int attempt; /* number of invocations of this callback for
                   * current request. */
@@ -168,6 +169,9 @@ static const struct auth_class {
     N_("Could not authenticate to proxy server: %s")
 };
 
+/* Internal buffer size, which must be >= NE_ABUFSIZ. */
+#define ABUFSIZE (NE_ABUFSIZ * 2)
+
 /* Authentication session state. */
 typedef struct {
     ne_session *sess;
@@ -190,7 +194,7 @@ typedef struct {
     /*** Session details ***/
 
     /* The username and password we are using to authenticate with */
-    char username[NE_ABUFSIZ];
+    char username[ABUFSIZE];
 
     /* This used for Basic auth */
     char *basic; 
@@ -397,19 +401,29 @@ static char *get_cnonce(void)
 
 /* 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
+ * pwbuf (of size ABUFSIZE).  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) 
+                           struct auth_challenge *chall, char *pwbuf)
 {
-    if (chall->handler->creds(chall->handler->userdata, sess->realm, 
-                              chall->handler->attempt++, sess->username, pwbuf) == 0) {
+    int rv;
+
+    if (chall->handler->new_creds)
+        rv = chall->handler->new_creds(chall->handler->userdata,
+                                       attempt,
+                                       chall->protocol->id, sess->realm,
+                                       sess->username, pwbuf,
+                                       ABUFSIZE);
+    else
+        rv = chall->handler->old_creds(chall->handler->userdata, sess->realm,
+                                       chall->handler->attempt++, sess->username, pwbuf);
+
+    if (rv == 0)
         return 0;
-    } else {
-        challenge_error(errmsg, _("rejected %s challenge"), 
-                        chall->protocol->name);
-        return -1;
-    }
+
+    challenge_error(errmsg, _("rejected %s challenge"),
+                    chall->protocol->name);
+    return -1;
 }
 
 /* Examine a Basic auth challenge.
@@ -418,7 +432,7 @@ static int basic_challenge(auth_session *sess, int attempt,
                            struct auth_challenge *parms,
                            ne_buffer **errmsg) 
 {
-    char *tmp, password[NE_ABUFSIZ];
+    char *tmp, password[ABUFSIZE];
 
     /* Verify challenge... must have a realm */
     if (parms->realm == NULL) {
@@ -795,7 +809,7 @@ static int ntlm_challenge(auth_session *sess, int attempt,
     NE_DEBUG(NE_DBG_HTTPAUTH, "auth: NTLM challenge.\n");
     
     if (!parms->opaque && (!sess->ntlm_context || (attempt > 1))) {
-        char password[NE_ABUFSIZ];
+        char password[ABUFSIZE];
 
         if (get_credentials(sess, errmsg, attempt, parms, password)) {
             /* Failed to get credentials */
@@ -825,7 +839,7 @@ static int digest_challenge(auth_session *sess, int attempt,
                             struct auth_challenge *parms,
                             ne_buffer **errmsg) 
 {
-    char password[NE_ABUFSIZ];
+    char password[ABUFSIZE];
     unsigned int hash;
     char *p;
 
@@ -907,7 +921,7 @@ static int digest_challenge(auth_session *sess, int attempt,
 
             if (esc) {
                 if (parms->userhash == userhash_none
-                    || (parms->handler->protomask & NE_AUTH_UTF8) == 0) {
+                    || parms->handler->new_creds == NULL) {
                     ne_free(esc);
                     challenge_error(errmsg, _("could not handle non-ASCII "
                                               "username in Digest challenge"));
@@ -1661,8 +1675,9 @@ static void free_auth(void *cookie)
 }
 
 static void auth_register(ne_session *sess, int isproxy, unsigned protomask,
-			  const struct auth_class *ahc, const char *id, 
-			  ne_auth_creds creds, void *userdata) 
+                          const struct auth_class *ahc, const char *id,
+                          ne_auth_creds old_creds, ne_auth_provide new_creds,
+                          void *userdata)
 {
     auth_session *ahs;
     struct auth_handler **hdl;
@@ -1757,7 +1772,8 @@ static void auth_register(ne_session *sess, int isproxy, unsigned protomask,
         
     *hdl = ne_malloc(sizeof **hdl);
     (*hdl)->protomask = protomask;
-    (*hdl)->creds = creds;
+    (*hdl)->old_creds = old_creds;
+    (*hdl)->new_creds = new_creds;
     (*hdl)->userdata = userdata;
     (*hdl)->next = NULL;
     (*hdl)->attempt = 0;
@@ -1766,27 +1782,37 @@ static void auth_register(ne_session *sess, int isproxy, unsigned protomask,
 void ne_set_server_auth(ne_session *sess, ne_auth_creds creds, void *userdata)
 {
     auth_register(sess, 0, NE_AUTH_DEFAULT, &ah_server_class, HOOK_SERVER_ID,
-                  creds, userdata);
+                  creds, NULL, userdata);
 }
 
 void ne_set_proxy_auth(ne_session *sess, ne_auth_creds creds, void *userdata)
 {
     auth_register(sess, 1, NE_AUTH_DEFAULT, &ah_proxy_class, HOOK_PROXY_ID,
-                  creds, userdata);
+                  creds, NULL, userdata);
 }
 
 void ne_add_server_auth(ne_session *sess, unsigned protocol, 
                         ne_auth_creds creds, void *userdata)
 {
     auth_register(sess, 0, protocol, &ah_server_class, HOOK_SERVER_ID,
-                  creds, userdata);
+                  creds, NULL, userdata);
 }
 
 void ne_add_proxy_auth(ne_session *sess, unsigned protocol, 
                        ne_auth_creds creds, void *userdata)
 {
     auth_register(sess, 1, protocol, &ah_proxy_class, HOOK_PROXY_ID,
-                  creds, userdata);
+                  creds, NULL, userdata);
+}
+
+void ne_add_auth(ne_session *sess, unsigned protocol,
+                 ne_auth_provide new_creds, void *userdata)
+{
+    if (protocol & NE_AUTH_PROXY)
+        auth_register(sess, 0, protocol, &ah_proxy_class, HOOK_PROXY_ID,
+                      NULL, new_creds, userdata);
+    auth_register(sess, 0, protocol, &ah_server_class, HOOK_SERVER_ID,
+                  NULL, new_creds, userdata);
 }
 
 void ne_forget_auth(ne_session *sess)
diff --git a/src/ne_auth.h b/src/ne_auth.h
index e825616..487ac0b 100644
--- a/src/ne_auth.h
+++ b/src/ne_auth.h
@@ -33,15 +33,8 @@ NE_BEGIN_DECLS
 /* The callback used to request the username and password in the given
  * realm. The username and password must be copied into the buffers
  * which are both of size NE_ABUFSIZ.  The 'attempt' parameter is zero
- * on the first call to the callback, and increases by one each time
- * an attempt to authenticate fails.
- *
- * With Basic and Digest authentication, a non-alphanumeric username
- * can be provided as a UTF-8-encoded string only if the NE_AUTH_UTF8
- * flag is used in the protocol mask to configure the callback via
- * ne_add_*_auth.  Otherwise the username must be an alphanumeric
- * ASCII username string.  Non-alphanumeric usernames are only
- * accepted in some cases.
+ * on the first call to the callback, and increases by one for each
+ * invocation of the callback during an attempt to authenticate.
  *
  * The callback must return zero to indicate that authentication
  * should be attempted with the username/password, or non-zero to
@@ -126,10 +119,9 @@ void ne_set_proxy_auth(ne_session *sess, ne_auth_creds creds, void *userdata);
  * flag may change across versions. */
 #define NE_AUTH_ALL (0x2000)
 
-/* This flag may be used in combination with another auth protocol
- * specifier. If present, it indicates that the username passed back
- * by the credentials callback is in UTF-8 encoding. */
-#define NE_AUTH_UTF8 (0x4000)
+/* If present in the protocol mask passed to ne_auth_provide,
+ * indicates that proxy authentication is requested. */
+#define NE_AUTH_PROXY (0x4000)
 
 /* Add a callback to provide credentials for server and proxy
  * authentication using a particular auth protocol or set of
@@ -157,6 +149,43 @@ void ne_add_server_auth(ne_session *sess, unsigned protocol,
 void ne_add_proxy_auth(ne_session *sess, unsigned protocol, 
                        ne_auth_creds creds, void *userdata);
 
+/* Alternative credentials provider callback, invoked when credentials
+ * are required to authenticate the client to either a server or
+ * proxy.  'protocol' is the authentication protocol number
+ * (NE_AUTH_*) of the challenge, bitwise-ORed with NE_AUTH_PROXY when
+ * the auth challenge is made by an HTTP proxy.
+ *
+ * 'realm' is the realm name.  The 'attempt' counter reflects the
+ * number of attempts to provide credentials to the server
+ * (i.e. retried requests sent with a challenge response), NOT the
+ * number of times the callback is invoked, unlike the ne_auth_creds
+ * callback.
+ *
+ * The callback must return zero to indicate that authentication
+ * should be attempted with the username/password, or non-zero to
+ * cancel the request. (if non-zero, username and password are
+ * ignored.)
+ *
+ * The username and password buffers have length 'buflen', which is
+ * guaranteed to be >= NE_ABUFSIZ.  The username must be provided as a
+ * NUL-terminated UTF-8 encoding only.  The password must be provided
+ * as a NUL-terminated string.  Additional protocol-specific
+ * restrictions apply, e.g. username cannot contain a colon for Basic
+ * auth.
+ *
+ * IMPORTANT NOTE: The callback will be invoked repeatedly until
+ * either it returns non-zero, or authentication is successful.
+ *
+ * Hint: if you just wish to attempt authentication just once (even if
+ * the user gets the username/password wrong), have the callback
+ * function use 'attempt' value as the function return value. */
+typedef int (*ne_auth_provide)(void *userdata, int attempt,
+                               unsigned protocol, const char *realm,
+                               char *username, char *password, size_t buflen);
+
+void ne_add_auth(ne_session *sess, unsigned protocol,
+                 ne_auth_provide creds, void *userdata);
+
 /* Clear any cached authentication credentials for the given
  * session. */
 void ne_forget_auth(ne_session *sess);
diff --git a/test/auth.c b/test/auth.c
index 3b01f8e..8186c91 100644
--- a/test/auth.c
+++ b/test/auth.c
@@ -63,6 +63,19 @@ static int auth_cb(void *userdata, const char *realm, int tries,
     return tries;
 }		   
 
+static int auth_provide_cb(void *userdata, int attempt,
+                           unsigned protocol, const char *realm,
+                           char *un, char *pw, size_t buflen)
+{
+    if (strcmp(realm, "WallyWorld")) {
+        NE_DEBUG(NE_DBG_HTTP, "Got wrong realm '%s'!\n", realm);
+        return -1;
+    }
+    strcpy(un, alt_username);
+    strcpy(pw, password);
+    return attempt;
+}
+
 static void auth_hdr(char *value)
 {
 #define B "Basic QWxhZGRpbjpvcGVuIHNlc2FtZQ=="
@@ -888,12 +901,9 @@ static int serve_digest(ne_socket *sock, void *userdata)
 static int test_digest(struct digest_parms *parms)
 {
     ne_session *sess;
-    void *auth_userdata = (parms->flags & PARM_ALTUSER) ? (void *)alt_username : NULL;
     unsigned proto = NE_AUTH_DIGEST;
 
-    if ((parms->flags & PARM_ALTUSER))
-        proto |= NE_AUTH_UTF8;
-    else if ((parms->flags & PARM_LEGACY))
+    if ((parms->flags & PARM_LEGACY))
         proto |= NE_AUTH_LEGACY_DIGEST;
     else if ((parms->flags & PARM_LEGACY_ONLY))
         proto = NE_AUTH_LEGACY_DIGEST;
@@ -907,11 +917,14 @@ static int test_digest(struct digest_parms *parms)
     if ((parms->flags & PARM_PROXY)) {
         CALL(proxied_session_server(&sess, "http", "www.example.com", 80,
                                     serve_digest, parms));
-        ne_set_proxy_auth(sess, auth_cb, auth_userdata);
+        ne_set_proxy_auth(sess, auth_cb, NULL);
     } 
     else {
         CALL(session_server(&sess, serve_digest, parms));
-        ne_add_server_auth(sess, proto, auth_cb, auth_userdata);
+        if ((parms->flags & PARM_ALTUSER))
+            ne_add_auth(sess, proto, auth_provide_cb, NULL);
+        else
+            ne_add_server_auth(sess, proto, auth_cb, NULL);
     }
 
     do {
@@ -1321,6 +1334,90 @@ static int multi_rfc7616(void)
     return await_server();
 }
 
+static int multi_provider_cb(void *userdata, int attempt,
+                             unsigned protocol, const char *realm,
+                             char *un, char *pw, size_t buflen)
+{
+    ne_buffer *buf = userdata;
+
+    if (buflen == NE_ABUFSIZ) {
+        NE_DEBUG(NE_DBG_HTTPAUTH, "auth: FAILED for short buffer length.\n");
+        return -1;
+    }
+
+    ne_buffer_snprintf(buf, 128, "[proto=%u, realm=%s, attempt=%d]",
+                       protocol, realm, attempt);
+
+    ne_strnzcpy(un, "foo", buflen);
+    ne_strnzcpy(pw, "bar", buflen);
+
+    return protocol == NE_AUTH_BASIC ? 0 : -1;
+}
+
+static int serve_provider(ne_socket *s, void *userdata)
+{
+    CALL(serve_response(s,
+                        "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"));
+    CALL(serve_response(s,
+                        "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'\r\n"
+                        "Content-Length: 0\r\n" "\r\n"));
+    return serve_response(s,
+                          "HTTP/1.1 200 OK\r\n"
+                          "Content-Length: 0\r\n" "\r\n");
+}
+
+static int multi_provider(void)
+{
+    ne_session *sess;
+    ne_buffer *buf = ne_buffer_create(), *exp;
+
+    CALL(make_session(&sess, serve_provider, NULL));
+
+    ne_add_auth(sess, NE_AUTH_DIGEST|NE_AUTH_BASIC, multi_provider_cb, buf);
+
+    ONREQ(any_request(sess, "/fish"));
+
+    exp = ne_buffer_create();
+    if (has_sha512_256)
+        ne_buffer_snprintf(exp, 100, "[proto=%u, realm=sha512-realm, attempt=0]",
+                           NE_AUTH_DIGEST);
+    if (has_sha256)
+        ne_buffer_snprintf(exp, 100, "[proto=%u, realm=sha256-realm, attempt=0]",
+                           NE_AUTH_DIGEST);
+    ne_buffer_snprintf(exp, 100,
+                       "[proto=%u, realm=md5-realm, attempt=0]"
+                       "[proto=%u, realm=basic-realm, attempt=0]",
+                       NE_AUTH_DIGEST, NE_AUTH_BASIC);
+
+    if (has_sha512_256)
+        ne_buffer_snprintf(exp, 100, "[proto=%u, realm=sha512-realm, attempt=1]",
+                           NE_AUTH_DIGEST);
+    ne_buffer_snprintf(exp, 100, "[proto=%u, realm=basic-realm, attempt=1]",
+                       NE_AUTH_BASIC);
+
+    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);
+    ne_buffer_destroy(exp);
+
+    return await_server();
+}
+
+
 static int domains(void)
 {
     ne_session *sess;
@@ -1465,6 +1562,7 @@ ne_test tests[] = {
     T(fail_challenge),
     T(multi_handler),
     T(multi_rfc7616),
+    T(multi_provider),
     T(domains),
     T(defaults),
     T(CVE_2008_3746),
diff --git a/test/utils.c b/test/utils.c
index 9d8e54b..ddb950c 100644
--- a/test/utils.c
+++ b/test/utils.c
@@ -34,7 +34,7 @@
 #include "tests.h"
 #include "utils.h"
 
-static int serve_response(ne_socket *s, const char *response)
+int serve_response(ne_socket *s, const char *response)
 {
     CALL(discard_request(s));
     CALL(discard_body(s));
diff --git a/test/utils.h b/test/utils.h
index c51df8c..117338e 100644
--- a/test/utils.h
+++ b/test/utils.h
@@ -29,6 +29,8 @@
 
 int single_serve_string(ne_socket *s, void *userdata);
 
+int serve_response(ne_socket *s, const char *response);
+
 struct many_serve_args {
     int count;
     const char *str;