From:
Andreas Wehrmann <a.wehrmann@centersystems.com>
To:
libmicrohttpd@gnu.org
Date:
Today 08:58:43 am
Spam Status: Spamassassin 0% probability of being spam.
Full report:
Probability=No, score=-3.2 required=7.0 tests=AWL,BAYES_00 autolearn=ham version=3.2.5-tuminfo_1
Hello!
I wrote a little testpage that I deliver using libmicrohttpd using
digest authentication.
The testpage consists of four files (framed page + image file).
When I initially connected to the webserver via the browser it correctly
challenged me
for my credentials. However, after entering the username and password
the index file
got loaded but it happened that the browser then challenged me again for
each
additional file to be loaded.
Since this is very annoying I tried increasing the nonce table size to 3000
(was default) but it was no good.
I then dug a little deeper and found out, that the hash algorithm to
determine the index
for a given nonce always returned zero thus overwriting other nonces.
The offending line is at check_nonce_nc() in digestauth.c:313:
off = (off << 8) | (*np & (off >> 24));
whereas is should be:
off = (off << 8) | (*np ^ (off >> 24));
Since "off" is initialized with zero and an unsigned integer
a logical AND returns zero which is not right obviously.
After this fix, the server challenged me only once and I got "random"
indices.
I found the problem in libmicrohttpd 0.9.5.
Best regards,
Andreas Wehrmann
--
Dipl.-Ing. (FH) Andreas Wehrmann
Software Development
--------------------------------------------------------------
Center Communication Systems GmbH
A-1210 Wien, Ignaz-Köck-Straße 19
Sitz in Wien
FN 796 88p, Firmenbuchgericht Wien
www.centersystems.com
Tel.: +43 (0) 190 199 - 3616
Mobile: +43 (0) 664 884 75916
Fax: +43 (0) 190 199 - 2110
E-Mail: a.wehrmann@centersystems.com
From:
Erik Slagter <erik@slagter.name>
To:
Christian Grothoff <grothoff@net.in.tum.de>
CC:
libmicrohttpd@gnu.org
Date:
Yesterday 17:31:46
Spam Status: Spamassassin 0% probability of being spam.
Full report:
Probability=No, score=-2.6 required=7.0 tests=AWL,BAYES_00,DKIM_SIGNED, DKIM_VERIFIED autolearn=ham version=3.2.5-tuminfo_1
> >> I think I found & fixed the issue in SVN 12778. Please try SVN HEAD and
> > > report if it fixes the problem.
> > Okay, will do so. But first I'll be on vacation for two weeks.
> Sure.
The issue indeed has been solved. It's quite a pity the distribution's
packages lag behind by ages, so again I'll have to make my own packages
now.
I've run into another problem and also I think I have found the cause.
When a connection is closed by the peer, my application tends to
segfault.
GDB says:
Program received signal SIGSEGV, Segmentation fault.
[Switching to Thread 0x7ffff6a6c710 (LWP 27005)]
0x00007ffff77a4db0 in pthread_mutex_lock () from /lib64/libpthread.so.0
Missing separate debuginfos, use: debuginfo-install glibc-2.11.2-1.x86_64 libgcc-4.4.4-10.fc12.x86_64 libid3tag-0.15.1b-9.fc12.x86_64 libstdc++-4.4.4-10.fc12.x86_64 zlib-1.2.3-23.fc12.x86_64
(gdb) where
#0 0x00007ffff77a4db0 in pthread_mutex_lock () from /lib64/libpthread.so.0
#1 0x00007ffff7bd97ce in MHD_destroy_response (response=0x7ffff7ed5010) at response.c:341
#2 0x00007ffff7bd760f in MHD_cleanup_connections (daemon=0x62eb60) at daemon.c:964
#3 0x00007ffff7bd8fbc in MHD_select_thread (cls=0x62eb60) at daemon.c:1205
#4 0x00007ffff77a2a3a in start_thread () from /lib64/libpthread.so.0
#5 0x00007ffff6d6077d in clone () from /lib64/libc.so.6
#6 0x0000000000000000 in ?? ()
It looks like pos->response is being "destroyed" twice. If I remove one of the instances, the segfault is gone.
Index: src/daemon/daemon.c
===================================================================
--- src/daemon/daemon.c (revision 12985)
+++ src/daemon/daemon.c (working copy)
@@ -952,7 +952,7 @@
abort();
}
}
- MHD_destroy_response (pos->response);
+ // MHD_destroy_response (pos->response);
MHD_pool_destroy (pos->pool);
#if HTTPS_SUPPORT
if (pos->tls_session != NULL)
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;
From:
Gerrit Telkamp <g.telkamp@domologic.de>
To:
libmicrohttpd@gnu.org
Date:
Today 11:42:30
Attachments:
libmicrohttpd-windows.patch
It was a bit tricky to build libmicrohttpd (SVN trunk) for Windows.
I'm using the newest MSYS/MinGW environment (GCC 4.5.1).
I've optimized the autoconf feature, so this should be easier in future.
Please find enclosed my patches.
In detail:
1- libmicrohttpd needs the "PlibC" library under Windows (libplibc.a).
Methods of PlibC are referenced by src/daemon/daemon.c.
autoconf assumes that PlibC is installed on the Build environment and
does not check this.
If PlibC is not installed, the pthreads check (see "acinclude.m4") will
fail and stops with the wrong error message "Your system is not
supporting pthreads". This is very confusing.
The patch includes an additional check for PlibC, before pthreads is
used. The check is only done under cygwin and mingw environments.
Furthermore, an older version of plibc.h was in the include directory.
This gives some problems if you are using a newer version of the PlibC
library. So I propose to exclude src/include/plibc.h from the SVN
Files affected:
acinclude.m4 (modified)
configure.ac (modified)
src/include/plibc.h (deleted)
2- setsockopt() uses a different parameter type under Windows ("optval"
is const char). This caused an error when src/daemon/daemon.c was compiled.
The patch enclosed includes a different call of the setsockopt() on
Windows machines.
Important: IPV6_V6ONLY is not available on Windows machines < Windows 7!
So this parameter is "0". But there is a TODO comment, maybe someone has
an idea how to solve this on Windows XP machines.
Files affected:
src/daemon/daemon.c (modified)
I have tested the patch on a MinGW / MSYS environment only.
Best regards
Gerrit.
libmicrohttpd-windows.patch
Christian Grothoff