commit 2f8eefada8be541c82c7fb4ed9bca7e88e720cd8
parent 25863e1c897b63eb56d248fde9634d0477ca8830
Author: Evgeny Grin (Karlson2k) <k2k@narod.ru>
Date: Sun, 14 Aug 2022 14:10:03 +0300
Added MHD_OPTION_DIGEST_AUTH_NONCE_BIND_TYPE to control how to generate and
check nonces for Digest Auth
Diffstat:
5 files changed, 250 insertions(+), 75 deletions(-)
diff --git 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
@@ -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
@@ -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 (¶ms->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 (¶ms->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 (¶ms->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
@@ -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
@@ -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()'");
