Jim Meyering <jim@meyering.net>
To:
libmicrohttpd@gnu.org
Date:
Today 10:17:27
I sent this change before, along with the s/are/is/ change,
but I guess you didn't see it:
Index: doc/microhttpd.texi
===================================================================
--- doc/microhttpd.texi (revision 15482)
+++ doc/microhttpd.texi (working copy)
@@ -269,7 +269,7 @@
@deftp {Enumeration} MHD_FLAG
Options for the @mhd{} daemon.
-Note that if neither @code{MHD_USER_THREAD_PER_CONNECTION} nor
+Note that if neither @code{MHD_USE_THREAD_PER_CONNECTION} nor
@code{MHD_USE_SELECT_INTERNALLY} is used, the client wants control over
the process and will call the appropriate microhttpd callbacks.
Index: src/include/microhttpd.h
===================================================================
--- src/include/microhttpd.h (revision 15482)
+++ src/include/microhttpd.h (working copy)
@@ -306,7 +306,7 @@
/**
* Options for the MHD daemon. Note that if neither
- * MHD_USER_THREAD_PER_CONNECTION nor MHD_USE_SELECT_INTERNALLY is
+ * MHD_USE_THREAD_PER_CONNECTION nor MHD_USE_SELECT_INTERNALLY is
* used, the client wants control over the process and will call the
* appropriate microhttpd callbacks.<p>
From:
Michael Wiegand <michael.wiegand@greenbone.net> (Greenbone Networks GmbH)
To:
christian@grothoff.org
Date:
Today 12:39:11 pm
Attachments:
microhttpd.texi-typo-fixes.patch
Hallo Christian,
ich habe ein paar Typos in der Doku von libmicrohttpd gefunden, siehe
angehängter Patch.
Oder nimmst du Patches lieber über die Liste? Habe mich jetzt auch endlich
angemeldet. :)
Grüße aus Osnabrück
Michael
From:
Jim Meyering <jim@meyering.net>
To:
libmicrohttpd@gnu.org
Date:
Today 12:42:34
Spam Status: Spamassassin 0% probability of being spam.
Full report:
Probability=No, score=-3.7 required=7.0 tests=AWL,BAYES_00 autolearn=ham version=3.2.5-tuminfo_1
Here is a patch to fix two typo fixes (and grammar nit).
I didn't change microhttpd.info, since it's generated.
(personally, I would not version-control a generated .info file)
Index: ChangeLog
This patch isn't purely about the subject matter, but rather have some tiny
small stuff that concerns other things so keep an eye for those. Also have to
say that documentation is included, including updated example and update unit test.
As for the performance part to it, its so complex to fully test it performance
wise, but it seems to work just fine, although keep an eye on how it does search
for the nonce to see if it is an attack or not, or if it is a whole new nonce
that needs to be added to the array, also have an eye for the rw-locks, I think
they are very useful and add a lot to the performance.
Thanks,
Amr Ali
End of signed message
latest_nc_mhd.patch
Note that the actual changes do not include the rw-locks (not as portable, no real
benefit in this context anyway) and I also made some other minor changes. Original
patch follows.
diff --git a/doc/microhttpd.texi b/doc/microhttpd.texi
index a86a59a..cc7f7a8 100644
--- a/doc/microhttpd.texi
+++ b/doc/microhttpd.texi
@@ -458,12 +458,31 @@ specified, ``NORMAL'' is used.
@cindex random
Digest Authentication nonce's seed.
This option must be followed by an "const char *" argument
-specifying a NULL terminated array of randomly generated values
-to be used in generating the server nonce when using digest authentication.
-It is a MUST to supply these values before utilizing any of MHD
-digest authentication functions, as otherwise, it will read from
-an arbitrary address in memory which results in an undefined behavior.
-
+an array of randomly generated values to be used in generating
+the server nonce when using digest authentication.
+It is a MUST to supply these values if MHD is compiled with
+Digest Authentication enabled (default).
+
+@item MHD_OPTION_DIGEST_AUTH_RAND_SIZE
+@cindex digest auth
+@cindex random
+Digest Authentication random seed size.
+This option must be followed by a "unsigned int" argument
+that have the size (number of elements) of the random seed
+array. This helps so that the seed might have zero bytes
+so it won't have a problem with string functions that
+depend on null terminated character arrays.
+
+@item MHD_OPTION_NONCE_NC_SIZE
+@cindex digest auth
+@cindex replay attack
+Size of an array of nonce and nonce counter map.
+This option must be followed by an "unsigned int" argument
+that have the size (number of elements) of a map of
+a nonce and a nonce-counter. This option MUST be
+specified if MHD is compiled with
+Digest Authentication enabled (default).
+
@item MHD_OPTION_LISTEN_SOCKET
@cindex systemd
Listen socket to use. Pass a listen socket for MHD to use
diff --git a/src/daemon/daemon.c b/src/daemon/daemon.c
index f68b9fb..8f0583c 100644
--- a/src/daemon/daemon.c
+++ b/src/daemon/daemon.c
@@ -1376,6 +1376,12 @@ parse_options_va (struct MHD_Daemon *daemon,
case MHD_OPTION_DIGEST_AUTH_RANDOM:
daemon->digest_auth_random = va_arg (ap, const char *);
break;
+ case MHD_OPTION_DIGEST_AUTH_RAND_SIZE:
+ daemon->digest_auth_rand_size = va_arg (ap, unsigned int);
+ break;
+ case MHD_OPTION_NONCE_NC_SIZE:
+ daemon->nonce_nc_size = va_arg (ap, unsigned int);
+ break;
#endif
case MHD_OPTION_LISTEN_SOCKET:
daemon->socket_fd = va_arg (ap, int);
@@ -1407,6 +1413,8 @@ parse_options_va (struct MHD_Daemon *daemon,
return MHD_NO;
break;
/* all options taking 'unsigned int' */
+ case MHD_OPTION_NONCE_NC_SIZE:
+ case MHD_OPTION_DIGEST_AUTH_RAND_SIZE:
case MHD_OPTION_CONNECTION_LIMIT:
case MHD_OPTION_CONNECTION_TIMEOUT:
case MHD_OPTION_PER_IP_CONNECTION_LIMIT:
@@ -1578,6 +1586,63 @@ MHD_start_daemon_va (unsigned int options,
return NULL;
}
+#ifdef DAUTH_SUPPORT
+ if (retVal->nonce_nc_size == 0)
+ {
+#if HAVE_MESSAGES
+ fprintf (stderr,
+ "MHD nonce-nc map size cannot be 0\n");
+#endif
+ free (retVal);
+ return NULL;
+ }
+ else
+ {
+ retVal->nnc = calloc(retVal->nonce_nc_size, sizeof(struct MHD_NonceNc));
+
+ if (!retVal->nnc)
+ {
+#if HAVE_MESSAGES
+ fprintf (stderr,
+ "An error has occured while allocating nonce-nc map\n");
+#endif
+ free (retVal);
+ return NULL;
+ }
+ }
+
+ if (retVal->digest_auth_rand_size == 0)
+ {
+#if HAVE_MESSAGES
+ fprintf(stderr,
+ "MHD Digest Auth random seed array size must be > 0\n");
+#endif
+ free (retVal);
+ return NULL;
+ }
+
+ if (retVal->digest_auth_random == NULL)
+ {
+#if HAVE_MESSAGES
+ fprintf (stderr,
+ "MHD requires `digest_auth_random' to point to a char array"
+ " of random values\n");
+#endif
+ free (retVal);
+ return NULL;
+ }
+
+ if (pthread_rwlock_init(&retVal->nnc_lock, NULL))
+ {
+#if HAVE_MESSAGES
+ fprintf (stderr,
+ "Failed to initialize nonce-nc lock\n");
+#endif
+ free (retVal);
+ return NULL;
+ }
+#endif
+
/* poll support currently only works with MHD_USE_THREAD_PER_CONNECTION */
if ( (0 != (options & MHD_USE_POLL)) &&
(0 == (options & MHD_USE_THREAD_PER_CONNECTION)) ) {
@@ -2012,6 +2077,11 @@ MHD_stop_daemon (struct MHD_Daemon *daemon)
}
}
#endif
+
+#ifdef DAUTH_SUPPORT
+ free (daemon->nnc);
+ pthread_rwlock_destroy (&daemon->nnc_lock);
+#endif
pthread_mutex_destroy (&daemon->per_ip_connection_mutex);
free (daemon);
}
diff --git a/src/daemon/digestauth.c b/src/daemon/digestauth.c
index 591538e..ad63b79 100644
--- a/src/daemon/digestauth.c
+++ b/src/daemon/digestauth.c
@@ -23,6 +23,7 @@
* @author Amr Ali
*/
+#include "platform.h"
#include "internal.h"
#include "md5.h"
@@ -82,12 +83,12 @@ cvthex(const unsigned char *bin,
* calculate H(A1) as per RFC2617 spec and store the
* result in 'sessionkey'.
*
- * @param alg FIXME: document
- * @param username FIXME: document
- * @param realm FIXME: document
- * @param password FIXME: document
- * @param nonce FIXME: document
- * @param cnonce FIXME: document
+ * @param alg The hash algorithm used, can be "md5" or "md5-sess"
+ * @param username A `char *' pointer to the username value
+ * @param realm A `char *' pointer to the realm value
+ * @param password A `char *' pointer to the password value
+ * @param nonce A `char *' pointer to the nonce value
+ * @param cnonce A `char *' pointer to the cnonce value
* @param sessionkey pointer to buffer of HASH_MD5_HEX_LEN+1 bytes
*/
static void
@@ -275,6 +276,69 @@ lookup_sub_value(char *dest,
return 0;
}
+/**
+ * Check nonce-nc map array with either new nonce counter
+ * or a whole new nonce.
+ *
+ * @param connection The MHD connection structure
+ * @param nonce A pointer that referenced a zero-terminated array of nonce
+ * @param nc The nonce counter
+ * @return 1 if successful, 0 if invalid
+ */
+static int
+check_nonce_nc(
+ struct MHD_Connection *connection,
+ const char *nonce,
+ unsigned int nc)
+{
+ static unsigned int pos = 0;
+ int i;
+
+ /*
+ * 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.
+ */
+
+ pthread_rwlock_rdlock(&connection->daemon->nnc_lock);
+
+ for (i = 0; i < connection->daemon->nonce_nc_size; ++i) {
+ if (0 == strcmp(connection->daemon->nnc[i].nonce, nonce)) {
+ if (nc - 1 == connection->daemon->nnc[i].nc) {
+ pthread_rwlock_unlock(&connection->daemon->nnc_lock);
+ pthread_rwlock_wrlock(&connection->daemon->nnc_lock);
+ ++connection->daemon->nnc[i].nc;
+ pthread_rwlock_unlock(&connection->daemon->nnc_lock);
+ return 1;
+ } else {
+ /*
+ * Nonce is found but its nonce counter is invalid.
+ */
+ pthread_rwlock_unlock(&connection->daemon->nnc_lock);
+ pthread_rwlock_wrlock(&connection->daemon->nnc_lock);
+ connection->daemon->nnc[i].nc = -1;
+ pthread_rwlock_unlock(&connection->daemon->nnc_lock);
+ return 0;
+ }
+ }
+ }
+
+ pthread_rwlock_unlock(&connection->daemon->nnc_lock);
+
+ /*
+ * nonce is not found and will be added to the array.
+ */
+
+ pthread_rwlock_wrlock(&connection->daemon->nnc_lock);
+ strncpy(connection->daemon->nnc[pos].nonce, nonce, strlen(nonce));
+ connection->daemon->nnc[pos].nc = 1;
+
+ pos = pos >= connection->daemon->nonce_nc_size ? 0 : pos + 1;
+
+ pthread_rwlock_unlock(&connection->daemon->nnc_lock);
+
+ return 1;
+}
/**
* Get the username from the authorization header sent by the client
@@ -316,6 +380,7 @@ MHD_digest_auth_get_username(struct MHD_Connection *connection)
* @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 rnd_size The size of the random seed array
* @param uri HTTP URI
* @param realm A string of characters that describes the realm of auth.
* @param nonce A pointer to a character array for the nonce to put in
@@ -324,6 +389,7 @@ static void
calculate_nonce (uint32_t nonce_time,
const char *method,
const char *rnd,
+ unsigned int rnd_size,
const char *uri,
const char *realm,
char *nonce)
@@ -342,7 +408,7 @@ calculate_nonce (uint32_t nonce_time,
MD5Update(&md5, ":", 1);
MD5Update(&md5, method, strlen(method));
MD5Update(&md5, ":", 1);
- MD5Update(&md5, rnd, strlen(rnd));
+ MD5Update(&md5, rnd, rnd_size);
MD5Update(&md5, ":", 1);
MD5Update(&md5, uri, strlen(uri));
MD5Update(&md5, ":", 1);
@@ -436,7 +502,7 @@ MHD_digest_auth_check(struct MHD_Connection *connection,
return MHD_NO;
/* 8 = 4 hexadecimal numbers for the timestamp */
- nonce_time = strtoul(nonce + len - 8, 0, 16);
+ nonce_time = strtoul(nonce + len - 8, (char **)NULL, 16);
t = (uint32_t) time(NULL);
/*
* First level vetting for the nonce validity
@@ -449,14 +515,15 @@ MHD_digest_auth_check(struct MHD_Connection *connection,
calculate_nonce (nonce_time,
connection->method,
connection->daemon->digest_auth_random,
+ connection->daemon->digest_auth_rand_size,
uri,
realm,
noncehashexp);
/*
* Second level vetting for the nonce validity
* if the timestamp attached to the nonce is valid
- * and possibility fabricated (in case of an attack)
- * the attacker must also know the password to be
+ * 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.
@@ -471,6 +538,15 @@ MHD_digest_auth_check(struct MHD_Connection *connection,
(0 == lookup_sub_value(nc, sizeof (nc), header, "nc")) ||
(0 == lookup_sub_value(response, sizeof (response), header, "response")) )
return MHD_NO;
+
+ /*
+ * 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 (! check_nonce_nc(connection, nonce, strtoul(nc, (char **)NULL, 16)) )
+ return MHD_NO;
digest_calc_ha1("md5",
username,
@@ -519,6 +595,7 @@ MHD_queue_auth_fail_response(struct MHD_Connection *connection,
calculate_nonce ((uint32_t) time(NULL),
connection->method,
connection->daemon->digest_auth_random,
+ connection->daemon->digest_auth_rand_size,
connection->url,
realm,
nonce);
diff --git a/src/daemon/internal.h b/src/daemon/internal.h
index 3887b27..9308355 100644
--- a/src/daemon/internal.h
+++ b/src/daemon/internal.h
@@ -93,6 +93,24 @@ struct MHD_Pollfd {
enum MHD_PollActions events;
};
+#ifdef DAUTH_SUPPORT
+/**
+ * A structure representing the internal holder of the
+ * nonce-nc map.
+ */
+struct MHD_NonceNc {
+ /**
+ * Nonce value: 32(MD5 Hex) + 8(Timestamp Hex) + 1(NULL)
+ */
+ char nonce[41];
+
+ /**
+ * Nonce counter, a value that increases for each subsequent
+ * request for the same nonce.
+ */
+ int nc;
+};
+#endif
#if HAVE_MESSAGES
/**
@@ -866,12 +884,34 @@ struct MHD_Daemon
#endif
#ifdef DAUTH_SUPPORT
+
/**
* Character array of random values.
*/
const char *digest_auth_random;
+ /**
+ * Size of `digest_auth_random.
+ */
+ unsigned int digest_auth_rand_size;
+
+ /**
+ * Size of the nonce-nc array.
+ */
+ unsigned int nonce_nc_size;
+
+ /**
+ * An array that contains the map nonce-nc.
+ */
+ struct MHD_NonceNc *nnc;
+
+ /**
+ * A rw-lock for synchronizing access to `nnc'.
+ */
+ pthread_rwlock_t nnc_lock;
+
#endif
+
/**
* Pointer to master daemon (NULL if this is the master)
*/
diff --git a/src/examples/digest_auth_example.c b/src/examples/digest_auth_example.c
index 51c81b8..9ead210 100644
--- a/src/examples/digest_auth_example.c
+++ b/src/examples/digest_auth_example.c
@@ -90,7 +90,7 @@ int
main (int argc, char *const *argv)
{
int fd;
- char rnd[9];
+ char rnd[8];
size_t len;
size_t off;
struct MHD_Daemon *d;
@@ -123,11 +123,12 @@ main (int argc, char *const *argv)
off += len;
}
(void) close(fd);
- rnd[8] = '\0';
d = MHD_start_daemon (MHD_USE_THREAD_PER_CONNECTION | MHD_USE_DEBUG,
atoi (argv[1]),
NULL, NULL, &ahc_echo, PAGE,
MHD_OPTION_DIGEST_AUTH_RANDOM, rnd,
+ MHD_OPTION_DIGEST_AUTH_RAND_SIZE, sizeof(rnd),
+ MHD_OPTION_NONCE_NC_SIZE, 300,
MHD_OPTION_END);
if (d == NULL)
return 1;
diff --git a/src/include/microhttpd.h b/src/include/microhttpd.h
index a74bf55..45d49f0 100644
--- a/src/include/microhttpd.h
+++ b/src/include/microhttpd.h
@@ -527,8 +527,22 @@ enum MHD_OPTION
* Auth module. This option should be followed by an "const char *"
* argument.
*/
- MHD_OPTION_DIGEST_AUTH_RANDOM = 17
+ MHD_OPTION_DIGEST_AUTH_RANDOM = 17,
+ /**
+ * Size of Digest Auth random seed. This option should be followed
+ * by an "unsigned int" argument. It is needed as the random seed array
+ * may contain values of zero, which would cause problems for
+ * string functions.
+ */
+ MHD_OPTION_DIGEST_AUTH_RAND_SIZE = 18,
+
+ /**
+ * Size of the internal array holding the map of the nonce and
+ * the nonce counter. This option should be followed by a "unsigend int"
+ * argument.
+ */
+ MHD_OPTION_NONCE_NC_SIZE = 19
};
diff --git a/src/testcurl/daemontest_digestauth.c b/src/testcurl/daemontest_digestauth.c
index e05d857..86ea305 100644
--- a/src/testcurl/daemontest_digestauth.c
+++ b/src/testcurl/daemontest_digestauth.c
@@ -129,7 +129,7 @@ testDigestAuth ()
size_t len;
size_t off = 0;
char buf[2048];
- char rnd[9];
+ char rnd[8];
cbc.buf = buf;
cbc.size = 2048;
@@ -156,10 +156,11 @@ testDigestAuth ()
off += len;
}
(void) close(fd);
- rnd[8] = '\0';
d = MHD_start_daemon (MHD_USE_SELECT_INTERNALLY | MHD_USE_DEBUG,
1337, NULL, NULL, &ahc_echo, PAGE,
MHD_OPTION_DIGEST_AUTH_RANDOM, rnd,
+ MHD_OPTION_DIGEST_AUTH_RAND_SIZE, sizeof(rnd),
+ MHD_OPTION_NONCE_NC_SIZE, 300,
MHD_OPTION_END);
if (d == NULL)
return 1;
Christian Grothoff
Matthias Wachs
Nils Durner