From 5fdb9effcb4ad4d6110b36255e73ec59b7a47994 Mon Sep 17 00:00:00 2001 From: "Evgeny Grin (Karlson2k)" Date: Fri, 13 May 2022 15:15:04 +0300 Subject: digestauth: added detection for possibly fabricated nonces --- src/microhttpd/digestauth.c | 219 ++++++++++++++++++++++++++++++-------------- 1 file changed, 152 insertions(+), 67 deletions(-) (limited to 'src/microhttpd/digestauth.c') diff --git a/src/microhttpd/digestauth.c b/src/microhttpd/digestauth.c index 3154088f..c7d13866 100644 --- a/src/microhttpd/digestauth.c +++ b/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) -- cgit v1.2.3