digestauth: added detection for possibly fabricated nonces

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 0x00097511
+#define MHD_VERSION 0x00097512
 
 /* If generic headers don't work on your platform, include headers
    which define 'va_list', 'size_t', 'ssize_t', 'intptr_t', 'off_t',

src/microhttpd/digestauth.c

@@ -210,6 +210,30 @@ enum MHD_DigestAuthResult
   MHD_DAUTH_RESPONSE_WRONG = -33,
 };
 
+/**
+ * The result of nonce-nc map array check.
+ */
+enum MHD_CheckNonceNC_
+{
+  /**
+   * The nonce and NC are OK (valid and NC was not used before).
+   */
+  MHD_DAUTH_NONCENC_OK = MHD_DAUTH_OK,
+
+  /**
+   * The 'nonce' was overwritten with newer 'nonce' in the same slot or
+   * NC was already used.
+   * The validity of the 'nonce' was not be checked.
+   */
+  MHD_DAUTH_NONCENC_STALE = MHD_DAUTH_NONCE_STALE,
+
+  /**
+   * The 'nonce' is wrong, it was not generated before.
+   */
+  MHD_DAUTH_NONCENC_WRONG = MHD_DAUTH_NONCE_WRONG,
+};
+
+
 /**
  * Context passed to functions that need to calculate
  * a digest but are orthogonal to the specific
@@ -591,6 +615,36 @@ lookup_sub_value (char *dest,
 }
 
 
+/**
+ * Extract timestamp from the given nonce.
+ * @param nonce the nonce to check
+ * @param noncelen the lenght of the nonce, zero for autodetect
+ * @param[out] ptimestamp the pointer to store extracted timestamp
+ * @return true if timestamp was extracted,
+ *         false if nonce does not have valid timestamp.
+ */
+static bool
+get_nonce_timestamp (const char *const nonce,
+                     size_t noncelen,
+                     uint64_t *const ptimestamp)
+{
+  mhd_assert ((0 == noncelen) || (strlen (nonce) == noncelen));
+  if (0 == noncelen)
+    noncelen = strlen (nonce);
+
+  if ( (NONCE_STD_LEN (SHA256_DIGEST_SIZE) != noncelen) &&
+       (NONCE_STD_LEN (MD5_DIGEST_SIZE) != noncelen) )
+    return false;
+
+  if (TIMESTAMP_CHARS_LEN !=
+      MHD_strx_to_uint64_n_ (nonce + noncelen - TIMESTAMP_CHARS_LEN,
+                             TIMESTAMP_CHARS_LEN,
+                             ptimestamp))
+    return false;
+  return true;
+}
+
+
 /**
  * Super-fast xor-based "hash" function
  *
@@ -638,51 +692,96 @@ get_nonce_nc_idx (size_t arr_size,
 
 
 /**
- * Check nonce-nc map array with either new nonce counter
- * or a whole new nonce.
+ * Check nonce-nc map array with the new nonce counter.
  *
  * @param connection The MHD connection structure
  * @param nonce A pointer that referenced a zero-terminated array of nonce
- * @param noncelen the lenth of @a nonce, in characters
- * @param nc The nonce counter, zero to add the nonce to the array
- * @return #MHD_YES if successful, #MHD_NO if invalid (or we have no NC array)
+ * @param noncelen the length of @a nonce, in characters
+ * @param nc The nonce counter
+ * @return #MHD_DAUTH_NONCENC_OK if successful,
+ *         #MHD_DAUTH_NONCENC_STALE if nonce is stale (or no nonce-nc array
+ *         is available),
+ *         #MHD_DAUTH_NONCENC_WRONG if nonce was not recodered in nonce-nc map
+ *         array, while it should.
  */
-static bool
+static enum MHD_CheckNonceNC_
 check_nonce_nc (struct MHD_Connection *connection,
                 const char *nonce,
                 size_t noncelen,
+                uint64_t nonce_time,
                 uint64_t nc)
 {
   struct MHD_Daemon *daemon = MHD_get_master (connection->daemon);
   struct MHD_NonceNc *nn;
   uint32_t mod;
-  bool ret;
+  enum MHD_CheckNonceNC_ ret;
 
-  mhd_assert (noncelen == strlen (nonce));
+  mhd_assert (strlen (nonce) == noncelen);
   mhd_assert (0 != nc);
   if (MAX_NONCE_LENGTH < noncelen)
-    return false; /* This should be impossible, but static analysis
+    return MHD_DAUTH_NONCENC_WRONG; /* This should be impossible, but static analysis
                       tools have a hard time with it *and* this also
                       protects against unsafe modifications that may
                       happen in the future... */
   mod = daemon->nonce_nc_size;
   if (0 == mod)
-    return false;  /* no array! */
+    return MHD_DAUTH_NONCENC_STALE;  /* no array! */
   if (nc + 64 < nc)
-    return false;  /* Overflow, unrealistically high value */
+    return MHD_DAUTH_NONCENC_STALE;  /* Overflow, unrealistically high value */
 
-  /*
-   * Look for the nonce, if it does exist and its corresponding
-   * nonce counter is less than the current nonce counter by 1,
-   * then only increase the nonce counter by one.
-   */
   nn = &daemon->nnc[get_nonce_nc_idx (mod, nonce, noncelen)];
 
   MHD_mutex_lock_chk_ (&daemon->nnc_lock);
 
+  mhd_assert (0 == nn->nonce[noncelen]); /* The old value must be valid */
+
   if ( (0 != memcmp (nn->nonce, nonce, noncelen)) ||
        (0 != nn->nonce[noncelen]) )
-    ret = false;     /* Nonce does not match, fail */
+  { /* The nonce in the slot does not match nonce from the client */
+    if (0 == nn->nonce[0])
+    { /* The slot was never used, while the client's nonce value should be
+       * recorded when it was generated by MHD */
+      ret = MHD_DAUTH_NONCENC_WRONG;
+    }
+    else if (0 != nn->nonce[noncelen])
+    { /* The value is the slot is wrong */
+      ret =  MHD_DAUTH_NONCENC_STALE;
+    }
+    else
+    {
+      uint64_t slot_ts; /**< The timestamp in the slot */
+      if (! get_nonce_timestamp (nn->nonce, 0, &slot_ts))
+      {
+        mhd_assert (0); /* The value is the slot is wrong */
+        ret = MHD_DAUTH_NONCENC_STALE;
+      }
+      else
+      {
+        /* Unsigned value, will be large if nonce_time is less than slot_ts */
+        const uint64_t ts_diff = TRIM_TO_TIMESTAMP (nonce_time - slot_ts);
+        if ((REUSE_TIMEOUT * 1000) >= ts_diff)
+        {
+          /* The nonce from the client may not have been placed in the slot
+           * because another nonce in that slot has not yet expired. */
+          ret = MHD_DAUTH_NONCENC_STALE;
+        }
+        else if (TRIM_TO_TIMESTAMP (UINT64_MAX) / 2 >= ts_diff)
+        {
+          /* Too large value means that nonce_time is less than slot_ts.
+           * The nonce from the client may have been overwritten by the newer
+           * nonce. */
+          ret = MHD_DAUTH_NONCENC_STALE;
+        }
+        else
+        {
+          /* The nonce from the client should be generated after the nonce
+           * in the slot has been expired, the nonce must be recorded, but
+           * it's not. */
+          ret = MHD_DAUTH_NONCENC_WRONG;
+        }
+      }
+    }
+  }
   else if (nc > nn->nc)
   {
     /* 'nc' is larger, shift bitmask and bump limit */
@@ -699,7 +798,7 @@ check_nonce_nc (struct MHD_Connection *connection,
     else
       nn->nmask = 0;                /* big jump, unset all bits in the mask */
     nn->nc = nc;
-    ret = true;
+    ret = MHD_DAUTH_NONCENC_OK;
   }
   else if (nc < nn->nc)
   {
@@ -710,23 +809,18 @@ check_nonce_nc (struct MHD_Connection *connection,
     {
       /* Out-of-order nonce, but within 64-bit bitmask, set bit */
       nn->nmask |= (UINT64_C (1) << (nn->nc - nc - 1));
-      ret = true;
+      ret = MHD_DAUTH_NONCENC_OK;
     }
     else
       /* 'nc' was already used or too old (more then 64 values ago) */
-      ret = false;
+      ret = MHD_DAUTH_NONCENC_STALE;
   }
   else /* if (nc == nn->nc) */
     /* 'nc' was already used */
-    ret = false;
+    ret = MHD_DAUTH_NONCENC_STALE;
 
   MHD_mutex_unlock_chk_ (&daemon->nnc_lock);
-#ifdef HAVE_MESSAGES
-  if (! ret)
-    MHD_DLOG (daemon,
-              _ ("Stale nonce received. If this happens a lot, you should "
-                 "probably increase the size of the nonce array.\n"));
-#endif
+
   return ret;
 }
 
@@ -851,36 +945,6 @@ calculate_nonce (uint64_t nonce_time,
 }
 
 
-/**
- * Extract timestamp from the given nonce.
- * @param nonce the nonce to check
- * @param noncelen the lenght of the nonce, zero for autodetect
- * @param[out] ptimestamp the pointer to store extracted timestamp
- * @return true if timestamp was extracted,
- *         false if nonce does not have valid timestamp.
- */
-static bool
-get_nonce_timestamp (const char *const nonce,
-                     size_t noncelen,
-                     uint64_t *const ptimestamp)
-{
-  mhd_assert ((0 == noncelen) || (strlen (nonce) == noncelen));
-  if (0 == noncelen)
-    noncelen = strlen (nonce);
-
-  if ( (NONCE_STD_LEN (SHA256_DIGEST_SIZE) != noncelen) &&
-       (NONCE_STD_LEN (MD5_DIGEST_SIZE) != noncelen) )
-    return false;
-
-  if (TIMESTAMP_CHARS_LEN !=
-      MHD_strx_to_uint64_n_ (nonce + noncelen - TIMESTAMP_CHARS_LEN,
-                             TIMESTAMP_CHARS_LEN,
-                             ptimestamp))
-    return false;
-  return true;
-}
-
-
 /**
  * Check whether it is possible to use slot in nonce-nc map array.
  *
@@ -1380,17 +1444,38 @@ digest_auth_check_all (struct MHD_Connection *connection,
     return MHD_DAUTH_WRONG_HEADER;   /* invalid nc value */
   }
 
-  /*
-   * Checking if that combination of nonce and nc is sound
-   * and not a replay attack attempt. Refuse if nonce was not
-   * generated previously.
-   */
-  if (! check_nonce_nc (connection,
-                        nonce,
-                        nonce_len,
-                        nci))
+  if (1)
   {
-    return MHD_DAUTH_NONCE_STALE;
+    enum MHD_CheckNonceNC_ nonce_nc_check;
+    /*
+     * Checking if that combination of nonce and nc is sound
+     * and not a replay attack attempt. Refuse if nonce was not
+     * generated previously.
+     */
+    nonce_nc_check = check_nonce_nc (connection,
+                                     nonce,
+                                     nonce_len,
+                                     nonce_time,
+                                     nci);
+    if (MHD_DAUTH_NONCENC_STALE == nonce_nc_check)
+    {
+#ifdef HAVE_MESSAGES
+      MHD_DLOG (daemon,
+                _ ("Stale nonce received. If this happens a lot, you should "
+                   "probably increase the size of the nonce array.\n"));
+#endif
+      return MHD_DAUTH_NONCE_STALE;
+    }
+    else if (MHD_DAUTH_NONCENC_WRONG == nonce_nc_check)
+    {
+#ifdef HAVE_MESSAGES
+      MHD_DLOG (daemon,
+                _ ("Received nonce that technically valid, but was not "
+                   "generated by MHD. This may indicate an attack attempt.\n"));
+#endif
+      return MHD_DAUTH_NONCE_WRONG;
+    }
+    mhd_assert (MHD_DAUTH_NONCENC_OK == nonce_nc_check);
   }
 
   if (1)