Re: [libmicrohttpd] [PATCH] New option: MHD_OPTION_HTTPS_MEM_DHPARAMS

[Hani Benhabiles]

Hi Christian,

(Please CC me on further replies, as I am not subscribed to this specific
mailing-list.)

> Dear Hani,
> 
> First of all, thanks for the patch.
> 
> In principle, this is of course a reasonable idea, but the (1) unchecked
> and (2)
> leaking 'malloc' in the patch will need to be fixed before I'll take it.
> I would
> suggest simply adding the dhparms / 'gnutls_dh_params_t' to 'struct
> MHD_Daemon'
> so that we don't need a separate allocation/deallocation.
> 

Oh, I missed the leak part. For some reason I thought
gnutls_certificate_free_credentials() took care of it while writing this patch.

> Also, the TexInfo documentation and ChangeLog need to be updated.
> Naturally,
> I can do those changes myself, but it'll likely take me a few more days
> to find
> the time, so I wanted to let you know what the status is.  If you fix those
> minor issues, I (or some other committer) might of course be able to commit
> the changes before then.
> 

Please find below an updated patch.

And by the way, any rough estimate about the next stable release's date ?

---
 ChangeLog                 |  4 ++++
 doc/libmicrohttpd.texi    | 10 ++++++++++
 src/include/microhttpd.h  |  9 ++++++++-
 src/microhttpd/daemon.c   | 32 ++++++++++++++++++++++++++++++++
 src/microhttpd/internal.h |  5 +++++
 5 files changed, 59 insertions(+), 1 deletion(-)

diff --git a/ChangeLog b/ChangeLog
index 4b0049d..e934873 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,7 @@
+Thu Apr 03 22:52:04 CET 2014
+       Added MHD_OPTION_HTTPS_MEM_DHPARAMS to allow users to set Diffie-Hellman
+       parameters to use in TLS handshakes. -Hani Benhabiles
+
 Tue Apr 01 07:10:23 CET 2014
        Added usage of native mutex on W32. -EG
 
diff --git a/doc/libmicrohttpd.texi b/doc/libmicrohttpd.texi
index f670a83..a618750 100644
--- a/doc/libmicrohttpd.texi
+++ b/doc/libmicrohttpd.texi
@@ -700,6 +700,16 @@ are acceptable for the application.  The string is passed
 unchanged to gnutls_priority_init.  If this option is not
 specified, ``NORMAL'' is used.
 
address@hidden MHD_OPTION_HTTPS_MEM_DHPARAMS
address@hidden SSL
address@hidden TLS
+Memory pointer to the Diffie-Hellman parameters to be used
+by the HTTPS daemon to support ciphersuites using (EC)DHE
+for key exchange.
+This option should be followed by an "const char*" argument.
+Without this, the presence of values like DHE-RSA in the
+priority string will be ignored by the GnuTLS library.
+
 @item MHD_OPTION_HTTPS_CERT_CALLBACK
 @cindex SSL
 @cindex TLS
diff --git a/src/include/microhttpd.h b/src/include/microhttpd.h
index c6e2505..bccfae2 100644
--- a/src/include/microhttpd.h
+++ b/src/include/microhttpd.h
@@ -837,7 +837,14 @@ enum MHD_OPTION
    * resources for the SYN packet along with its DATA.  This option should be
    * followed by an `unsigned int` argument.
    */
-  MHD_OPTION_TCP_FASTOPEN_QUEUE_SIZE = 23
+  MHD_OPTION_TCP_FASTOPEN_QUEUE_SIZE = 23,
+
+  /**
+   * Memory pointer for the Diffie-Hellman parameters (dh.pem) to be used by 
the
+   * HTTPS daemon for key exchange.
+   * This option should be followed by a `const char *` argument.
+   */
+  MHD_OPTION_HTTPS_MEM_DHPARAMS = 24,
 
 };
 
diff --git a/src/microhttpd/daemon.c b/src/microhttpd/daemon.c
index cd6a0bd..8ad01ab 100644
--- a/src/microhttpd/daemon.c
+++ b/src/microhttpd/daemon.c
@@ -520,6 +520,25 @@ MHD_init_daemon_certificate (struct MHD_Daemon *daemon)
        }
     }
 
+  if (NULL != daemon->https_mem_dhparams)
+    {
+      gnutls_dh_params_t dhparams;
+      if (NULL == (dhparams = malloc (sizeof (gnutls_dh_params_t))))
+        return -1;
+
+      cert.data = (unsigned char *) daemon->https_mem_dhparams;
+      cert.size = strlen (daemon->https_mem_dhparams);
+      if (gnutls_dh_params_import_pkcs3 (dhparams, &cert,
+                                        GNUTLS_X509_FMT_PEM) < 0)
+       {
+#if HAVE_MESSAGES
+         MHD_DLOG(daemon, "Bad Diffie-Hellman parameters format\n");
+#endif
+         return -1;
+       }
+      gnutls_certificate_set_dh_params (daemon->x509_cred, dhparams);
+      daemon->dh_params = dhparams;
+    }
   /* certificate & key loaded from memory */
   if ( (NULL != daemon->https_mem_cert) &&
        (NULL != daemon->https_mem_key) )
@@ -2965,6 +2984,16 @@ parse_options_va (struct MHD_Daemon *daemon,
                      opt);
 #endif
           break;
+        case MHD_OPTION_HTTPS_MEM_DHPARAMS:
+         if (0 != (daemon->options & MHD_USE_SSL))
+           daemon->https_mem_dhparams = va_arg (ap, const char *);
+#if HAVE_MESSAGES
+         else
+           MHD_DLOG (daemon,
+                     "MHD HTTPS option %d passed to MHD but MHD_USE_SSL not 
set\n",
+                     opt);
+#endif
+          break;
        case MHD_OPTION_HTTPS_CRED_TYPE:
          daemon->cred_type = (gnutls_credentials_type_t) va_arg (ap, int);
          break;
@@ -3087,6 +3116,7 @@ parse_options_va (struct MHD_Daemon *daemon,
                case MHD_OPTION_HTTPS_MEM_CERT:
                case MHD_OPTION_HTTPS_MEM_TRUST:
                case MHD_OPTION_HTTPS_PRIORITIES:
+               case MHD_OPTION_HTTPS_MEM_DHPARAMS:
                case MHD_OPTION_ARRAY:
                 case MHD_OPTION_HTTPS_CERT_CALLBACK:
                  if (MHD_YES != parse_options (daemon,
@@ -4135,6 +4165,8 @@ MHD_stop_daemon (struct MHD_Daemon *daemon)
       gnutls_priority_deinit (daemon->priority_cache);
       if (daemon->x509_cred)
         gnutls_certificate_free_credentials (daemon->x509_cred);
+      if (daemon->dh_params)
+        gnutls_dh_params_deinit (daemon->dh_params);
     }
 #endif
 #if EPOLL_SUPPORT
diff --git a/src/microhttpd/internal.h b/src/microhttpd/internal.h
index 57c1aeb..3cdb24f 100644
--- a/src/microhttpd/internal.h
+++ b/src/microhttpd/internal.h
@@ -1194,6 +1194,11 @@ struct MHD_Daemon
   const char *https_mem_trust;
 
   /**
+   * Pointer to our Diffie-Hellman parameters in memory.
+   */
+  const char *https_mem_dhparams;
+
+  /**
    * For how many connections do we have 'tls_read_ready' set to MHD_YES?
    * Used to avoid O(n) traversal over all connections when determining
    * event-loop timeout (as it needs to be zero if there is any connection
-- 
1.8.3.2