fix digest authentication with escaped urls, as reported on mailinglist

3 files changed, 149 insertions, 139 deletions

diff --git a/ChangeLog b/ChangeLog
index 57dd440f..2555527e 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,7 @@
+Fri May 29 12:23:01 CEST 2015
+        Fixing digest authentication when used in combination
+        with escaped characters in URLs. -CG/AW
+
 Wed May 13 11:49:09 CEST 2015
         Releasing libmicrohttpd 0.9.42. -CG
 
diff --git a/src/microhttpd/digestauth.c b/src/microhttpd/digestauth.c
index 9c3fe8c5..4cc7f61b 100644
--- a/src/microhttpd/digestauth.c
+++ b/src/microhttpd/digestauth.c
@@ -1,6 +1,6 @@
 /*
      This file is part of libmicrohttpd
-     Copyright (C) 2010, 2011, 2012 Daniel Pittman and Christian Grothoff
+     Copyright (C) 2010, 2011, 2012, 2015 Daniel Pittman and Christian Grothoff
 
      This library is free software; you can redistribute it and/or
      modify it under the terms of the GNU Lesser General Public
@@ -472,8 +472,8 @@ test_header (struct MHD_Connection *connection,
  *
  * @param connection connections with headers to compare against
  * @param args argument URI string (after "?" in URI)
- * @return MHD_YES if the arguments match,
- *         MHD_NO if not
+ * @return #MHD_YES if the arguments match,
+ *         #MHD_NO if not
  */
 static int
 check_argument_match (struct MHD_Connection *connection,
@@ -632,10 +632,83 @@ MHD_digest_auth_check (struct MHD_Connection *connection,
        header value. */
     return MHD_NO;
   }
+  /* 8 = 4 hexadecimal numbers for the timestamp */
+  nonce_time = strtoul (nonce + len - 8, (char **)NULL, 16);
+  t = (uint32_t) MHD_monotonic_time();
+  /*
+   * First level vetting for the nonce validity: if the timestamp
+   * attached to the nonce exceeds `nonce_timeout', then the nonce is
+   * invalid.
+   */
+  if ( (t > nonce_time + nonce_timeout) ||
+       (nonce_time + nonce_timeout < nonce_time) )
+    {
+      /* too old */
+      return MHD_INVALID_NONCE;
+    }
+
+  calculate_nonce (nonce_time,
+                   connection->method,
+                   connection->daemon->digest_auth_random,
+                   connection->daemon->digest_auth_rand_size,
+                   connection->url,
+                   realm,
+                   noncehashexp);
+  /*
+   * Second level vetting for the nonce validity
+   * if the timestamp attached to the nonce is valid
+   * and possibly fabricated (in case of an attack)
+   * the attacker must also know the random seed to be
+   * able to generate a "sane" nonce, which if he does
+   * not, the nonce fabrication process going to be
+   * very hard to achieve.
+   */
+
+  if (0 != strcmp (nonce, noncehashexp))
+    {
+      return MHD_INVALID_NONCE;
+    }
+  if ( (0 == lookup_sub_value (cnonce,
+                               sizeof (cnonce),
+                               header, "cnonce")) ||
+       (0 == lookup_sub_value (qop, sizeof (qop), header, "qop")) ||
+       ( (0 != strcmp (qop, "auth")) &&
+         (0 != strcmp (qop, "")) ) ||
+       (0 == lookup_sub_value (nc, sizeof (nc), header, "nc"))  ||
+       (0 == lookup_sub_value (response, sizeof (response), header, "response")) )
+    {
+#if HAVE_MESSAGES
+      MHD_DLOG (connection->daemon,
+                "Authentication failed, invalid format.\n");
+#endif
+      return MHD_NO;
+    }
+  nci = strtoul (nc, &end, 16);
+  if ( ('\0' != *end) ||
+       ( (LONG_MAX == nci) &&
+         (ERANGE == errno) ) )
+    {
+#if HAVE_MESSAGES
+      MHD_DLOG (connection->daemon,
+                "Authentication failed, invalid format.\n");
+#endif
+      return MHD_NO; /* invalid nonce format */
+    }
+  /*
+   * Checking if that combination of nonce and nc is sound
+   * and not a replay attack attempt. Also adds the nonce
+   * to the nonce-nc map if it does not exist there.
+   */
+
+  if (MHD_YES != check_nonce_nc (connection, nonce, nci))
+    {
+      return MHD_NO;
+    }
+
   {
     char *uri;
-    
-    uri = malloc(left + 1);
+
+    uri = malloc (left + 1);
     if (NULL == uri)
     {
 #if HAVE_MESSAGES
@@ -648,24 +721,31 @@ MHD_digest_auth_check (struct MHD_Connection *connection,
                                left + 1,
                                header, "uri"))
     {
-      free(uri);
+      free (uri);
       return MHD_NO;
     }
 
-    /* 8 = 4 hexadecimal numbers for the timestamp */
-    nonce_time = strtoul (nonce + len - 8, (char **)NULL, 16);
-    t = (uint32_t) MHD_monotonic_time();
-    /*
-     * First level vetting for the nonce validity: if the timestamp
-     * attached to the nonce exceeds `nonce_timeout', then the nonce is
-     * invalid.
-     */
-    if ( (t > nonce_time + nonce_timeout) ||
-         (nonce_time + nonce_timeout < nonce_time) )
-    { 
-      free(uri);
-      return MHD_INVALID_NONCE;
-    }
+    digest_calc_ha1("md5",
+                    username,
+                    realm,
+                    password,
+                    nonce,
+                    cnonce,
+                    ha1);
+    digest_calc_response (ha1,
+                          nonce,
+                          nc,
+                          cnonce,
+                          qop,
+                          connection->method,
+                          uri,
+                          hentity,
+                          respexp);
+
+    /* Need to unescape URI before comparing with connection->url */
+    connection->daemon->unescape_callback (connection->daemon->unescape_callback_cls,
+                                           connection,
+                                           uri);
     if (0 != strncmp (uri,
                       connection->url,
                       strlen (connection->url)))
@@ -674,9 +754,10 @@ MHD_digest_auth_check (struct MHD_Connection *connection,
       MHD_DLOG (connection->daemon,
                 "Authentication failed, URI does not match.\n");
 #endif
-      free(uri);
+      free (uri);
       return MHD_NO;
     }
+
     {
       const char *args = strchr (uri, '?');
 
@@ -692,89 +773,11 @@ MHD_digest_auth_check (struct MHD_Connection *connection,
         MHD_DLOG (connection->daemon,
                   "Authentication failed, arguments do not match.\n");
 #endif
-       free(uri);
+       free (uri);
        return MHD_NO;
       }
     }
-    calculate_nonce (nonce_time,
-                     connection->method,
-                     connection->daemon->digest_auth_random,
-                     connection->daemon->digest_auth_rand_size,
-                     connection->url,
-                     realm,
-                     noncehashexp);
-    /*
-     * Second level vetting for the nonce validity
-     * if the timestamp attached to the nonce is valid
-     * and possibly fabricated (in case of an attack)
-     * the attacker must also know the random seed to be
-     * able to generate a "sane" nonce, which if he does
-     * not, the nonce fabrication process going to be
-     * very hard to achieve.
-     */
-
-    if (0 != strcmp (nonce, noncehashexp))
-    {
-      free(uri);
-      return MHD_INVALID_NONCE;
-    }
-    if ( (0 == lookup_sub_value (cnonce,
-                                 sizeof (cnonce),
-                                 header, "cnonce")) ||
-         (0 == lookup_sub_value (qop, sizeof (qop), header, "qop")) ||
-         ( (0 != strcmp (qop, "auth")) &&
-           (0 != strcmp (qop, "")) ) ||
-         (0 == lookup_sub_value (nc, sizeof (nc), header, "nc"))  ||
-         (0 == lookup_sub_value (response, sizeof (response), header, "response")) )
-    {
-#if HAVE_MESSAGES
-      MHD_DLOG (connection->daemon,
-                "Authentication failed, invalid format.\n");
-#endif
-      free(uri);
-      return MHD_NO;
-    }
-    nci = strtoul (nc, &end, 16);
-    if ( ('\0' != *end) ||
-         ( (LONG_MAX == nci) &&
-           (ERANGE == errno) ) )
-    {
-#if HAVE_MESSAGES
-      MHD_DLOG (connection->daemon,
-                "Authentication failed, invalid format.\n");
-#endif
-      free(uri);
-      return MHD_NO; /* invalid nonce format */
-    }
-    /*
-     * Checking if that combination of nonce and nc is sound
-     * and not a replay attack attempt. Also adds the nonce
-     * to the nonce-nc map if it does not exist there.
-     */
-
-    if (MHD_YES != check_nonce_nc (connection, nonce, nci))
-    {
-      free(uri);
-      return MHD_NO;
-    }
-
-    digest_calc_ha1("md5",
-                    username,
-                    realm,
-                    password,
-                    nonce,
-                    cnonce,
-                    ha1);
-    digest_calc_response (ha1,
-                          nonce,
-                          nc,
-                          cnonce,
-                          qop,
-                          connection->method,
-                          uri,
-                          hentity,
-                          respexp);
-    free(uri);
+    free (uri);
     return (0 == strcmp(response, respexp))
       ? MHD_YES
       : MHD_NO;
@@ -835,7 +838,7 @@ MHD_queue_auth_fail_response (struct MHD_Connection *connection,
                    : "");
   {
     char *header;
-    
+
     header = malloc(hlen + 1);
     if (NULL == header)
     {
diff --git a/src/testcurl/test_digestauth.c b/src/testcurl/test_digestauth.c
index 255e086a..fd579196 100644
--- a/src/testcurl/test_digestauth.c
+++ b/src/testcurl/test_digestauth.c
@@ -73,7 +73,8 @@ ahc_echo (void *cls,
           const char *url,
           const char *method,
           const char *version,
-          const char *upload_data, size_t *upload_data_size,
+          const char *upload_data,
+          size_t *upload_data_size,
           void **unused)
 {
   struct MHD_Response *response;
@@ -82,44 +83,47 @@ ahc_echo (void *cls,
   const char *realm = "test@example.com";
   int ret;
 
-  username = MHD_digest_auth_get_username(connection);
+  username = MHD_digest_auth_get_username (connection);
   if ( (username == NULL) ||
        (0 != strcmp (username, "testuser")) )
     {
-      response = MHD_create_response_from_buffer(strlen (DENIED), 
-                                                 DENIED,
-                                                 MHD_RESPMEM_PERSISTENT);  
+      response = MHD_create_response_from_buffer (strlen (DENIED),
+                                                  DENIED,
+                                                  MHD_RESPMEM_PERSISTENT);
       ret = MHD_queue_auth_fail_response(connection, realm,
                                          MY_OPAQUE,
                                          response,
-                                         MHD_NO);    
-      MHD_destroy_response(response);  
+                                         MHD_NO);
+      MHD_destroy_response(response);
       return ret;
     }
   ret = MHD_digest_auth_check(connection, realm,
-                              username, 
-                              password, 
+                              username,
+                              password,
                               300);
   free(username);
   if ( (ret == MHD_INVALID_NONCE) ||
        (ret == MHD_NO) )
     {
-      response = MHD_create_response_from_buffer(strlen (DENIED), 
+      response = MHD_create_response_from_buffer(strlen (DENIED),
                                                  DENIED,
-                                                 MHD_RESPMEM_PERSISTENT);  
-      if (NULL == response) 
+                                                 MHD_RESPMEM_PERSISTENT);
+      if (NULL == response)
         return MHD_NO;
       ret = MHD_queue_auth_fail_response(connection, realm,
                                          MY_OPAQUE,
                                          response,
-                                         (ret == MHD_INVALID_NONCE) ? MHD_YES : MHD_NO);  
-      MHD_destroy_response(response);  
+                                         (ret == MHD_INVALID_NONCE) ? MHD_YES : MHD_NO);
+      MHD_destroy_response(response);
       return ret;
     }
-  response = MHD_create_response_from_buffer(strlen(PAGE), PAGE,
-                                             MHD_RESPMEM_PERSISTENT);
-  ret = MHD_queue_response(connection, MHD_HTTP_OK, response);  
-  MHD_destroy_response(response);
+  response = MHD_create_response_from_buffer (strlen(PAGE),
+                                              PAGE,
+                                              MHD_RESPMEM_PERSISTENT);
+  ret = MHD_queue_response (connection,
+                            MHD_HTTP_OK,
+                            response);
+  MHD_destroy_response (response);
   return ret;
 }
 
@@ -144,24 +148,24 @@ testDigestAuth ()
   fd = open("/dev/urandom", O_RDONLY);
   if (-1 == fd)
     {
-          fprintf(stderr, "Failed to open `%s': %s\n",
-               "/dev/urandom",
-                   strerror(errno));
-          return 1;
-        }
+      fprintf(stderr, "Failed to open `%s': %s\n",
+              "/dev/urandom",
+              strerror(errno));
+      return 1;
+    }
   while (off < 8)
-        {
-          len = read(fd, rnd, 8);
-          if (len == -1)
-            {
-                  fprintf(stderr, "Failed to read `%s': %s\n",
-                       "/dev/urandom",
-                           strerror(errno));
-                  (void) close(fd);
-                  return 1;
-                }
-          off += len;
-        }
+    {
+      len = read(fd, rnd, 8);
+      if (len == -1)
+        {
+          fprintf(stderr, "Failed to read `%s': %s\n",
+                  "/dev/urandom",
+                  strerror(errno));
+          (void) close(fd);
+          return 1;
+        }
+      off += len;
+    }
   (void) close(fd);
 #else
   {
@@ -193,7 +197,7 @@ testDigestAuth ()
   if (d == NULL)
     return 1;
   c = curl_easy_init ();
-  curl_easy_setopt (c, CURLOPT_URL, "http://127.0.0.1:1337/");
+  curl_easy_setopt (c, CURLOPT_URL, "http://127.0.0.1:1337/bar%20 foo?a=bü%20");
   curl_easy_setopt (c, CURLOPT_WRITEFUNCTION, &copyBuffer);
   curl_easy_setopt (c, CURLOPT_WRITEDATA, &cbc);
   curl_easy_setopt (c, CURLOPT_HTTPAUTH, CURLAUTH_DIGEST);
@@ -225,7 +229,6 @@ testDigestAuth ()
 }
 
 
-
 int
 main (int argc, char *const *argv)
 {