Added MHD_OPTION_DIGEST_AUTH_NONCE_BIND_TYPE to control how to generate and

check nonces for Digest Auth

5 files changed, 250 insertions, 75 deletions

diff --git a/src/include/microhttpd.h b/src/include/microhttpd.h
index 78a31f29..83006001 100644
--- a/src/include/microhttpd.h
+++ b/src/include/microhttpd.h
@@ -96,7 +96,7 @@ extern "C"
  * they are parsed as decimal numbers.
  * Example: 0x01093001 = 1.9.30-1.
  */
-#define MHD_VERSION 0x00097530
+#define MHD_VERSION 0x00097531
 
 /* If generic headers don't work on your platform, include headers
    which define 'va_list', 'size_t', 'ssize_t', 'intptr_t', 'off_t',
@@ -1541,6 +1541,69 @@ typedef int
                                     size_t *psk_size);
 
 /**
+ * Values for #MHD_OPTION_DIGEST_AUTH_NONCE_BIND_TYPE.
+ *
+ * These values can limit the scope of validity of MHD-generated nonces.
+ * Values can be combined with bitwise OR.
+ * Any value, except #MHD_DAUTH_BIND_NONCE_NONE, enforce function
+ * #MHD_digest_auth_check3() (and similar) to check nonce by re-generating
+ * it again with the same parameters, which is CPU-intensive operation.
+ * @note Available since #MHD_VERSION 0x00097531
+ */
+enum MHD_DAuthBindNonce
+{
+  /**
+   * Generated nonces are valid for any request from any client until expired.
+   * This is default and recommended value.
+   * #MHD_digest_auth_check3() (and similar function) would check only whether
+   * the nonce value that is used by client has been generated by MHD and not
+   * expired yet.
+   * It is recommended because RFC 7616 allows clients to use the same nonce
+   * for any request in the same "protection space".
+   * CPU is loaded less when this value is used when checking client's
+   * authorisation request.
+   * This value cannot be combined with other values.
+   */
+  MHD_DAUTH_BIND_NONCE_NONE = 0,
+
+  /**
+   * Generated nonces are valid only for the same realm.
+   */
+  MHD_DAUTH_BIND_NONCE_REALM = 1 << 0,
+
+  /**
+   * Generated nonces are valid only for the same URI (excluding parameters
+   * after '?' in URI) and request method (GET, POST etc).
+   * Not recommended unless "protection space" is limited to a single URI as
+   * RFC 7616 allows clients to re-use server-generated nonces for any URI
+   * in the same "protection space" which is by default consists of all server
+   * URIs.
+   * This was default (and only supported) value before #MHD_VERSION 0x00097518
+   */
+  MHD_DAUTH_BIND_NONCE_URI = 1 << 1,
+
+  /**
+   * Generated nonces are valid only for the same URI including URI parameters
+   * and request method (GET, POST etc).
+   * This value implies #MHD_DAUTH_BIND_NONCE_URI.
+   * Not recommended for that same reasons as #MHD_DAUTH_BIND_NONCE_URI.
+   */
+  MHD_DAUTH_BIND_NONCE_URI_PARAMS = 1 << 2,
+
+  /**
+   * Generated nonces are valid only for the single client's IP.
+   * While it looks like security improvement, in practice the same client may
+   * jump from one IP to another (mobile or Wi-Fi handover, DHCP re-assignment,
+   * Multi-NAT, different proxy chain and other reasons), while IP address
+   * spoofing could be used relatively easily.
+   * However, if server gets intensive requests with Digest Authentication
+   * this value helps to generate unique nonces for several requests, received
+   * exactly at the same time (within one millisecond) from different clients.
+   */
+  MHD_DAUTH_BIND_NONCE_CLIENT_IP = 1 << 3
+} _MHD_FLAGS_ENUM;
+
+/**
  * @brief MHD options.
  *
  * Passed in the varargs portion of #MHD_start_daemon.
@@ -1943,7 +2006,17 @@ enum MHD_OPTION
    * @sa #MHD_OPTION_DIGEST_AUTH_RANDOM
    * @note Available since #MHD_VERSION 0x00097529
    */
-  MHD_OPTION_DIGEST_AUTH_RANDOM_COPY = 35
+  MHD_OPTION_DIGEST_AUTH_RANDOM_COPY = 35,
+
+  /**
+   * Allow to controls the scope of validity of MHD-generated nonces.
+   * This regulates how "nonces" are generated and how "nonces" are checked by
+   * #MHD_digest_auth_check3() and similar functions.
+   * This option should be followed by an 'unsigned int` argument with value
+   * formed as bitwise OR combination of #MHD_DAuthBindNonce values.
+   * @note Available since #MHD_VERSION 0x00097531
+   */
+  MHD_OPTION_DIGEST_AUTH_NONCE_BIND_TYPE = 36
 } _MHD_FIXED_ENUM;
 
 
@@ -4906,7 +4979,7 @@ MHD_digest_auth_get_username3 (struct MHD_Connection *connection);
  *
  * All error values are zero or negative.
  *
- * @note Available since #MHD_VERSION 0x00097521
+ * @note Available since #MHD_VERSION 0x00097531
  */
 enum MHD_DigestAuthResult
 {
@@ -4968,6 +5041,20 @@ enum MHD_DigestAuthResult
   MHD_DAUTH_NONCE_STALE = -17,
 
   /**
+   * The 'nonce' was generated by MHD for other conditions.
+   * This value is only returned if #MHD_OPTION_DIGEST_AUTH_NONCE_BIND_TYPE
+   * is set to anything other than #MHD_DAUTH_BIND_NONCE_NONE.
+   * The interpretation of this code could be different. For example, if
+   * #MHD_DAUTH_BIND_NONCE_URI is set and client just used the same 'nonce' for
+   * another URI, the code could be handled as #MHD_DAUTH_NONCE_STALE as
+   * it is allowed to re-use nonces for other URIs in the same "protection
+   * space". However, if only #MHD_DAUTH_BIND_NONCE_CLIENT_IP bit is set and
+   * it is know that clients have fixed IP addresses, this return code could
+   * be handled like #MHD_DAUTH_NONCE_WRONG.
+   */
+  MHD_DAUTH_NONCE_OTHER_COND = -18,
+
+  /**
    * The 'nonce' is wrong. May indicate an attack attempt.
    */
   MHD_DAUTH_NONCE_WRONG = -33,
diff --git a/src/microhttpd/daemon.c b/src/microhttpd/daemon.c
index f548b434..6a92e733 100644
--- a/src/microhttpd/daemon.c
+++ b/src/microhttpd/daemon.c
@@ -6257,6 +6257,12 @@ parse_options_va (struct MHD_Daemon *daemon,
       daemon->nonce_nc_size = va_arg (ap,
                                       unsigned int);
       break;
+    case MHD_OPTION_DIGEST_AUTH_NONCE_BIND_TYPE:
+      daemon->dauth_bind_type = va_arg (ap,
+                                        unsigned int);
+      if (0 != (daemon->dauth_bind_type & MHD_DAUTH_BIND_NONCE_URI_PARAMS))
+        daemon->dauth_bind_type |= MHD_DAUTH_BIND_NONCE_URI;
+      break;
 #endif
     case MHD_OPTION_LISTEN_SOCKET:
       if (0 != (daemon->options & MHD_USE_NO_LISTEN_SOCKET))
@@ -6379,6 +6385,7 @@ parse_options_va (struct MHD_Daemon *daemon,
         case MHD_OPTION_LISTENING_ADDRESS_REUSE:
         case MHD_OPTION_LISTEN_BACKLOG_SIZE:
         case MHD_OPTION_SERVER_INSANITY:
+        case MHD_OPTION_DIGEST_AUTH_NONCE_BIND_TYPE:
           if (MHD_NO == parse_options (daemon,
                                        servaddr,
                                        opt,
diff --git a/src/microhttpd/digestauth.c b/src/microhttpd/digestauth.c
index 4b4eac7c..ba112bd6 100644
--- a/src/microhttpd/digestauth.c
+++ b/src/microhttpd/digestauth.c
@@ -23,7 +23,9 @@
  * @author Amr Ali
  * @author Matthieu Speder
  * @author Christian Grothoff (RFC 7616 support)
- * @author Karlson2k (Evgeny Grin)
+ * @author Karlson2k (Evgeny Grin) (fixes, new API, improvements, large rewrite,
+ *                                  many RFC 7616 features implementation,
+ *                                  old RFC 2069 support)
  */
 #include "digestauth.h"
 #include "gen_auth.h"
@@ -1314,85 +1316,140 @@ MHD_digest_auth_get_username (struct MHD_Connection *connection)
  * H(timestamp ":" method ":" random ":" uri ":" realm) + Hex(timestamp)
  *
  * @param nonce_time The amount of time in seconds for a nonce to be invalid
- * @param method HTTP method
- * @param rnd A pointer to a character array for the random seed
+ * @param mthd_e HTTP method as enum value
+ * @param method HTTP method as a string
+ * @param rnd the pointer to a character array for the random seed
  * @param rnd_size The size of the random seed array @a rnd
- * @param uri HTTP URI (in MHD, without the arguments ("?k=v")
+ * @param saddr the pointer to the socket address structure
+ * @param saddr_size the size of the socket address structure @a saddr
+ * @param uri the HTTP URI (in MHD, without the arguments ("?k=v")
+ * @param uri_len the length of the @a uri
+ * @param first_header the pointer to the first request's header
  * @param realm A string of characters that describes the realm of auth.
  * @param realm_len the length of the @a realm.
+ * @param bind_options the nonce bind options (#MHD_DAuthBindNonce values).
  * @param da digest algorithm to use
- * @param[out] nonce A pointer to a character array for the nonce to put in,
- *        must provide NONCE_STD_LEN(digest_get_size(da))+1 bytes
+ * @param[out] nonce the pointer to a character array for the nonce to put in,
+ *                   must provide NONCE_STD_LEN(digest_get_size(da)) bytes,
+ *                   result is NOT zero-terminated
  */
 static void
 calculate_nonce (uint64_t nonce_time,
+                 enum MHD_HTTP_Method mthd_e,
                  const char *method,
                  const char *rnd,
                  size_t rnd_size,
+                 const struct sockaddr_storage *saddr,
+                 size_t saddr_size,
                  const char *uri,
                  size_t uri_len,
-                 struct MHD_HTTP_Req_Header *first_header,
+                 const struct MHD_HTTP_Req_Header *first_header,
                  const char *realm,
                  size_t realm_len,
+                 unsigned int bind_options,
                  struct DigestAlgorithm *da,
                  char *nonce)
 {
-  uint8_t timestamp[TIMESTAMP_BIN_SIZE];
-  struct MHD_HTTP_Req_Header *h;
-
   digest_init (da);
-  /* If the nonce_time is milliseconds, then the same 48 bit value will repeat
-   * every 8 925 years, which is more than enough to mitigate a replay attack */
+  if (1)
+  {
+    uint8_t timestamp[TIMESTAMP_BIN_SIZE];
+    /* If the nonce_time is milliseconds, then the same 48 bit value will repeat
+     * every 8 925 years, which is more than enough to mitigate a replay attack */
 #if TIMESTAMP_BIN_SIZE != 6
 #error The code needs to be updated here
 #endif
-  timestamp[0] = (uint8_t) (nonce_time >> (8 * (TIMESTAMP_BIN_SIZE - 1 - 0)));
-  timestamp[1] = (uint8_t) (nonce_time >> (8 * (TIMESTAMP_BIN_SIZE - 1 - 1)));
-  timestamp[2] = (uint8_t) (nonce_time >> (8 * (TIMESTAMP_BIN_SIZE - 1 - 2)));
-  timestamp[3] = (uint8_t) (nonce_time >> (8 * (TIMESTAMP_BIN_SIZE - 1 - 3)));
-  timestamp[4] = (uint8_t) (nonce_time >> (8 * (TIMESTAMP_BIN_SIZE - 1 - 4)));
-  timestamp[5] = (uint8_t) (nonce_time >> (8 * (TIMESTAMP_BIN_SIZE - 1 - 5)));
-  digest_update (da,
-                 timestamp,
-                 sizeof (timestamp));
-  digest_update_with_colon (da);
-  digest_update_str (da, method);
-  digest_update_with_colon (da);
+    timestamp[0] = (uint8_t) (nonce_time >> (8 * (TIMESTAMP_BIN_SIZE - 1 - 0)));
+    timestamp[1] = (uint8_t) (nonce_time >> (8 * (TIMESTAMP_BIN_SIZE - 1 - 1)));
+    timestamp[2] = (uint8_t) (nonce_time >> (8 * (TIMESTAMP_BIN_SIZE - 1 - 2)));
+    timestamp[3] = (uint8_t) (nonce_time >> (8 * (TIMESTAMP_BIN_SIZE - 1 - 3)));
+    timestamp[4] = (uint8_t) (nonce_time >> (8 * (TIMESTAMP_BIN_SIZE - 1 - 4)));
+    timestamp[5] = (uint8_t) (nonce_time >> (8 * (TIMESTAMP_BIN_SIZE - 1 - 5)));
+    MHD_bin_to_hex (timestamp,
+                    sizeof (timestamp),
+                    nonce + digest_get_size (da) * 2);
+    digest_update (da,
+                   timestamp,
+                   sizeof (timestamp));
+    digest_update_with_colon (da);
+  }
   if (rnd_size > 0)
+  {
     digest_update (da,
                    rnd,
                    rnd_size);
-  digest_update_with_colon (da);
-  digest_update (da,
-                 uri,
-                 uri_len);
-  for (h = first_header; NULL != h; h = h->next)
+    digest_update_with_colon (da);
+  }
+  if ( (0 != (bind_options & MHD_DAUTH_BIND_NONCE_CLIENT_IP)) &&
+       (0 != saddr_size) )
+  {
+    if (AF_INET == saddr->ss_family)
+      digest_update (da,
+                     &((const struct sockaddr_in *) saddr)->sin_addr,
+                     sizeof(((const struct sockaddr_in *) saddr)->sin_addr));
+#ifdef HAVE_INET6
+    else if (AF_INET6 == saddr->ss_family)
+      digest_update (da,
+                     &((const struct sockaddr_in6 *) saddr)->sin6_addr,
+                     sizeof(((const struct sockaddr_in6 *) saddr)->sin6_addr));
+#endif /* HAVE_INET6 */
+    digest_update_with_colon (da);
+  }
+  if (0 != (bind_options & MHD_DAUTH_BIND_NONCE_URI))
+  {
+    if (MHD_HTTP_MTHD_OTHER != mthd_e)
+    {
+      uint8_t mthd_for_hash;
+      if (MHD_HTTP_MTHD_HEAD != mthd_e)
+        mthd_for_hash = (uint8_t) mthd_e;
+      else /* Treat HEAD method in the same way as GET method */
+        mthd_for_hash = (uint8_t) MHD_HTTP_MTHD_GET;
+      digest_update (da,
+                     &mthd_for_hash,
+                     sizeof(mthd_for_hash));
+    }
+    else
+      digest_update_str (da, method);
+
+    digest_update_with_colon (da);
+
+    digest_update (da,
+                   uri,
+                   uri_len);
+    digest_update_with_colon (da);
+  }
+  if (0 != (bind_options & MHD_DAUTH_BIND_NONCE_URI_PARAMS))
   {
-    if (MHD_GET_ARGUMENT_KIND != h->kind)
-      continue;
-    digest_update (da, "\0", 2);
-    if (0 != h->header_size)
-      digest_update (da, h->header, h->header_size);
-    digest_update (da, "", 1);
-    if (0 != h->value_size)
-      digest_update (da, h->value, h->value_size);
+    const struct MHD_HTTP_Req_Header *h;
+
+    for (h = first_header; NULL != h; h = h->next)
+    {
+      if (MHD_GET_ARGUMENT_KIND != h->kind)
+        continue;
+      digest_update (da, "\0", 2);
+      if (0 != h->header_size)
+        digest_update (da, h->header, h->header_size);
+      digest_update (da, "", 1);
+      if (0 != h->value_size)
+        digest_update (da, h->value, h->value_size);
+    }
+    digest_update_with_colon (da);
+  }
+  if (0 != (bind_options & MHD_DAUTH_BIND_NONCE_REALM))
+  {
+    digest_update (da,
+                   realm,
+                   realm_len);
+    digest_update_with_colon (da);
   }
-  digest_update_with_colon (da);
-  digest_update (da,
-                 realm,
-                 realm_len);
   if (1)
   {
-    const unsigned int digest_size = digest_get_size (da);
     uint8_t hash[MAX_DIGEST];
     digest_calc_hash (da, hash);
     MHD_bin_to_hex (hash,
-                    digest_size,
+                    digest_get_size (da),
                     nonce);
   }
-  MHD_bin_to_hex (timestamp,
-                  sizeof (timestamp),
-                  nonce + digest_get_size (da) * 2);
 }
 
 
@@ -1464,7 +1521,8 @@ is_slot_available (const struct MHD_NonceNc *const nn,
  * @param realm_len the length of the @a realm
  * @param da the digest algorithm to use
  * @param[out] nonce the pointer to a character array for the nonce to put in,
- *        must provide NONCE_STD_LEN(digest_get_size(da))+1 bytes
+ *                   must provide NONCE_STD_LEN(digest_get_size(da)) bytes,
+ *                   result is NOT zero-terminated
  * @return true if the new nonce has been added to the nonce-nc map array,
  *         false otherwise.
  */
@@ -1485,14 +1543,18 @@ calculate_add_nonce (struct MHD_Connection *const connection,
   mhd_assert (0 != nonce_size);
 
   calculate_nonce (timestamp,
+                   connection->rq.http_mthd,
                    connection->rq.method,
                    daemon->digest_auth_random,
                    daemon->digest_auth_rand_size,
+                   connection->addr,
+                   (size_t) connection->addr_len,
                    connection->rq.url,
                    connection->rq.url_len,
                    connection->rq.headers_received,
                    realm,
                    realm_len,
+                   daemon->dauth_bind_type,
                    da,
                    nonce);
 
@@ -1532,8 +1594,9 @@ calculate_add_nonce (struct MHD_Connection *const connection,
  * @param connection the MHD connection structure
  * @param realm A string of characters that describes the realm of auth.
  * @param da digest algorithm to use
- * @param[out] nonce A pointer to a character array for the nonce to put in,
- *        must provide NONCE_STD_LEN(digest_get_size(da))+1 bytes
+ * @param[out] nonce the pointer to a character array for the nonce to put in,
+ *                   must provide NONCE_STD_LEN(digest_get_size(da)) bytes,
+ *                   result is NOT zero-terminated
  */
 static bool
 calculate_add_nonce_with_retry (struct MHD_Connection *const connection,
@@ -2213,7 +2276,7 @@ digest_auth_check_all_inner (struct MHD_Connection *connection,
     digest_update_with_colon (&da);
     digest_update (&da, realm, realm_len);
     digest_calc_hash (&da, hash1_bin);
-    mhd_assert (sizeof (tmp1) >= (2 * digest_size + 1));
+    mhd_assert (sizeof (tmp1) >= (2 * digest_size));
     MHD_bin_to_hex (hash1_bin, digest_size, tmp1);
     if (! is_param_equal_caseless (&params->username, tmp1, 2 * digest_size))
       return MHD_DAUTH_WRONG_USERNAME;
@@ -2368,7 +2431,7 @@ digest_auth_check_all_inner (struct MHD_Connection *connection,
 
   digest_init (&da);
   /* Update digest with H(A1) */
-  mhd_assert (sizeof (tmp1) >= (digest_size * 2 + 1));
+  mhd_assert (sizeof (tmp1) >= (digest_size * 2));
   if (NULL == userdigest)
     MHD_bin_to_hex (hash1_bin, digest_size, tmp1);
   else
@@ -2432,25 +2495,33 @@ digest_auth_check_all_inner (struct MHD_Connection *connection,
   if (0 != memcmp (hash1_bin, hash2_bin, digest_size))
     return MHD_DAUTH_RESPONSE_WRONG;
 
-  mhd_assert (sizeof(tmp1) >= (NONCE_STD_LEN (digest_size) + 1));
-  /* It was already checked that 'nonce' (including timestamp) was generated
-     by MHD. The next check is mostly an overcaution. */
-  calculate_nonce (nonce_time,
-                   connection->rq.method,
-                   daemon->digest_auth_random,
-                   daemon->digest_auth_rand_size,
-                   connection->rq.url,
-                   connection->rq.url_len,
-                   connection->rq.headers_received,
-                   realm,
-                   realm_len,
-                   &da,
-                   tmp1);
-
-  if (! is_param_equal (&params->nonce, tmp1,
-                        NONCE_STD_LEN (digest_size)))
-    return MHD_DAUTH_NONCE_WRONG;
-  /* The 'nonce' was generated in the same conditions */
+  if (MHD_DAUTH_BIND_NONCE_NONE != daemon->dauth_bind_type)
+  {
+    mhd_assert (sizeof(tmp1) >= (NONCE_STD_LEN (digest_size) + 1));
+    /* It was already checked that 'nonce' (including timestamp) was generated
+       by MHD. */
+    calculate_nonce (nonce_time,
+                     connection->rq.http_mthd,
+                     connection->rq.method,
+                     daemon->digest_auth_random,
+                     daemon->digest_auth_rand_size,
+                     connection->addr,
+                     (size_t) connection->addr_len,
+                     connection->rq.url,
+                     connection->rq.url_len,
+                     connection->rq.headers_received,
+                     realm,
+                     realm_len,
+                     daemon->dauth_bind_type,
+                     &da,
+                     tmp1);
+
+
+    if (! is_param_equal (&params->nonce, tmp1,
+                          NONCE_STD_LEN (digest_size)))
+      return MHD_DAUTH_NONCE_OTHER_COND;
+    /* The 'nonce' was generated in the same conditions */
+  }
 
   return MHD_DAUTH_OK;
 }
@@ -2716,7 +2787,8 @@ MHD_digest_auth_check2 (struct MHD_Connection *connection,
                                 malgo3);
   if (MHD_DAUTH_OK == res)
     return MHD_YES;
-  else if ((MHD_DAUTH_NONCE_STALE == res) || (MHD_DAUTH_NONCE_WRONG == res))
+  else if ((MHD_DAUTH_NONCE_STALE == res) || (MHD_DAUTH_NONCE_WRONG == res) ||
+           (MHD_DAUTH_NONCE_OTHER_COND == res) )
     return MHD_INVALID_NONCE;
   return MHD_NO;
 
@@ -2773,7 +2845,8 @@ MHD_digest_auth_check_digest2 (struct MHD_Connection *connection,
                                        malgo3);
   if (MHD_DAUTH_OK == res)
     return MHD_YES;
-  else if ((MHD_DAUTH_NONCE_STALE == res) || (MHD_DAUTH_NONCE_WRONG == res))
+  else if ((MHD_DAUTH_NONCE_STALE == res) || (MHD_DAUTH_NONCE_WRONG == res) ||
+           (MHD_DAUTH_NONCE_OTHER_COND == res) )
     return MHD_INVALID_NONCE;
   return MHD_NO;
 }
diff --git a/src/microhttpd/internal.h b/src/microhttpd/internal.h
index 35620bff..3a81b2a2 100644
--- a/src/microhttpd/internal.h
+++ b/src/microhttpd/internal.h
@@ -2225,6 +2225,10 @@ struct MHD_Daemon
    */
   unsigned int nonce_nc_size;
 
+  /**
+   * Nonce bind type.
+   */
+  unsigned int dauth_bind_type;
 #endif
 
 #ifdef TCP_FASTOPEN
diff --git a/src/testcurl/test_digestauth2.c b/src/testcurl/test_digestauth2.c
index 6054af7d..4c3af9bf 100644
--- a/src/testcurl/test_digestauth2.c
+++ b/src/testcurl/test_digestauth2.c
@@ -694,6 +694,10 @@ ahc_echo (void *cls,
         mhdErrorExitDesc ("MHD_digest_auth_check[_digest]3()' returned " \
                           "MHD_DAUTH_NONCE_WRONG");
         break;
+      case MHD_DAUTH_NONCE_OTHER_COND:
+        mhdErrorExitDesc ("MHD_digest_auth_check[_digest]3()' returned " \
+                          "MHD_DAUTH_NONCE_OTHER_COND");
+        break;
       case MHD_DAUTH_ERROR:
         externalErrorExitDesc ("General error returned " \
                                "by 'MHD_digest_auth_check[_digest]3()'");