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

[Christian Grothoff]

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.

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.

Happy hacking!

Christian

On 03/31/2014 05:16 PM, Hani Benhabiles wrote:
> List,
> 
> In order to activate ciphersuites with Perfect Forward Secrecy property, we 
> need
> to provide Diffie-Hellman parameters to gnutls to enable (EC)DHE key exchange
> algorithm.
> 
> The patch below adds a new option to do so: MHD_OPTION_HTTPS_MEM_DHPARAMS 
> which,
> like other HTTPS_MEM_* options takes a memory pointer as the following 
> argument.
> 
> In consequence, a gnutls priority string such as "NORMAL:+DHE-RSA" would be
> effective, and not equivalent to "NORMAL" as is the case now.
> 
> Cheers,
> 
> Hani.
> 
> ---
>  src/include/microhttpd.h  |  9 ++++++++-
>  src/microhttpd/daemon.c   | 26 ++++++++++++++++++++++++++
>  src/microhttpd/internal.h |  5 +++++
>  3 files changed, 39 insertions(+), 1 deletion(-)
> 
> diff --git a/src/include/microhttpd.h b/src/include/microhttpd.h
> index d486543..4f43578 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 5fee5c9..3444eb3 100644
> --- a/src/microhttpd/daemon.c
> +++ b/src/microhttpd/daemon.c
> @@ -520,6 +520,22 @@ MHD_init_daemon_certificate (struct MHD_Daemon *daemon)
>         }
>      }
>  
> +  if (NULL != daemon->https_mem_dhparams)
> +    {
> +      gnutls_dh_params_t dhparams;
> +      cert.data = (unsigned char *) daemon->https_mem_dhparams;
> +      cert.size = strlen (daemon->https_mem_dhparams);
> +      dhparams = malloc (sizeof (gnutls_dh_params_t));
> +      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);
> +    }
>    /* certificate & key loaded from memory */
>    if ( (NULL != daemon->https_mem_cert) &&
>         (NULL != daemon->https_mem_key) )
> @@ -2967,6 +2983,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;
> diff --git a/src/microhttpd/internal.h b/src/microhttpd/internal.h
> index 53d1f85..62818fd 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
>

0x48426C7E.asc
Description: application/pgp-keys