Re: [PATCH 16/21] crypto: deprecate use of external dh-params.pem file

["Marc-André Lureau" <marcandre.lureau@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 7157 bytes --]

Hi

On Thu, Oct 30, 2025 at 6:50 PM Daniel P. Berrangé <berrange@redhat.com>
wrote:

> GNUTLS has deprecated use of externally provided diffie-hellman
> parameters, since it will automatically negotiate DH params in
> accordance with RFC7919.
>

The doc says:
 Since 3.6.0, DH parameters are negotiated following RFC7919.

But QEMU doesn't require >= 3.6. Add a preliminary patch?


> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
>

Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>


> ---
>  crypto/tlscreds.c         | 24 ++++++++----------------
>  crypto/tlscredsanon.c     |  6 ++++--
>  crypto/tlscredspsk.c      |  6 ++++--
>  crypto/tlscredsx509.c     |  4 +++-
>  docs/about/deprecated.rst |  9 +++++++++
>  docs/system/tls.rst       | 12 +++++++-----
>  6 files changed, 35 insertions(+), 26 deletions(-)
>
> diff --git a/crypto/tlscreds.c b/crypto/tlscreds.c
> index 798c9712fb..85268f3b57 100644
> --- a/crypto/tlscreds.c
> +++ b/crypto/tlscreds.c
> @@ -22,6 +22,7 @@
>  #include "qapi/error.h"
>  #include "qapi-types-crypto.h"
>  #include "qemu/module.h"
> +#include "qemu/error-report.h"
>  #include "tlscredspriv.h"
>  #include "trace.h"
>
> @@ -38,22 +39,7 @@ qcrypto_tls_creds_get_dh_params_file(QCryptoTLSCreds
> *creds,
>
>      trace_qcrypto_tls_creds_load_dh(creds, filename ? filename :
> "<generated>");
>
> -    if (filename == NULL) {
> -        ret = gnutls_dh_params_init(dh_params);
> -        if (ret < 0) {
> -            error_setg(errp, "Unable to initialize DH parameters: %s",
> -                       gnutls_strerror(ret));
> -            return -1;
> -        }
> -        ret = gnutls_dh_params_generate2(*dh_params, DH_BITS);
> -        if (ret < 0) {
> -            gnutls_dh_params_deinit(*dh_params);
> -            *dh_params = NULL;
> -            error_setg(errp, "Unable to generate DH parameters: %s",
> -                       gnutls_strerror(ret));
> -            return -1;
> -        }
> -    } else {
> +    if (filename != NULL) {
>          GError *gerr = NULL;
>          gchar *contents;
>          gsize len;
> @@ -67,6 +53,10 @@ qcrypto_tls_creds_get_dh_params_file(QCryptoTLSCreds
> *creds,
>              g_error_free(gerr);
>              return -1;
>          }
> +        warn_report_once("Use of an external DH parameters file '%s' is "
> +                         "deprecated and will be removed in a future
> release",
> +                         filename);
> +
>          data.data = (unsigned char *)contents;
>          data.size = len;
>          ret = gnutls_dh_params_init(dh_params);
> @@ -87,6 +77,8 @@ qcrypto_tls_creds_get_dh_params_file(QCryptoTLSCreds
> *creds,
>                         filename, gnutls_strerror(ret));
>              return -1;
>          }
> +    } else {
> +        *dh_params = NULL;
>      }
>
>      return 0;
> diff --git a/crypto/tlscredsanon.c b/crypto/tlscredsanon.c
> index 69ed1d792a..777cc4f5bb 100644
> --- a/crypto/tlscredsanon.c
> +++ b/crypto/tlscredsanon.c
> @@ -68,8 +68,10 @@ qcrypto_tls_creds_anon_load(QCryptoTLSCredsAnon *creds,
>              return -1;
>          }
>
> -        gnutls_anon_set_server_dh_params(box->data.anonserver,
> -                                         box->dh_params);
> +        if (box->dh_params) {
> +            gnutls_anon_set_server_dh_params(box->data.anonserver,
> +                                             box->dh_params);
> +        }
>      } else {
>          ret =
> gnutls_anon_allocate_client_credentials(&box->data.anonclient);
>          if (ret < 0) {
> diff --git a/crypto/tlscredspsk.c b/crypto/tlscredspsk.c
> index e437985260..801da50625 100644
> --- a/crypto/tlscredspsk.c
> +++ b/crypto/tlscredspsk.c
> @@ -129,8 +129,10 @@ qcrypto_tls_creds_psk_load(QCryptoTLSCredsPSK *creds,
>                         gnutls_strerror(ret));
>              goto cleanup;
>          }
> -        gnutls_psk_set_server_dh_params(box->data.pskserver,
> -                                        box->dh_params);
> +        if (box->dh_params) {
> +            gnutls_psk_set_server_dh_params(box->data.pskserver,
> +                                            box->dh_params);
> +        }
>      } else {
>          box = qcrypto_tls_creds_box_new_client(GNUTLS_CRD_PSK);
>
> diff --git a/crypto/tlscredsx509.c b/crypto/tlscredsx509.c
> index 2fc0872627..7e79af4266 100644
> --- a/crypto/tlscredsx509.c
> +++ b/crypto/tlscredsx509.c
> @@ -684,7 +684,9 @@ qcrypto_tls_creds_x509_load(QCryptoTLSCredsX509 *creds,
>                                                   errp) < 0) {
>              return -1;
>          }
> -        gnutls_certificate_set_dh_params(box->data.cert, box->dh_params);
> +        if (box->dh_params) {
> +            gnutls_certificate_set_dh_params(box->data.cert,
> box->dh_params);
> +        }
>      }
>      creds->parent_obj.box = g_steal_pointer(&box);
>
> diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
> index ca6b3769b5..694a69da64 100644
> --- a/docs/about/deprecated.rst
> +++ b/docs/about/deprecated.rst
> @@ -365,6 +365,15 @@ Options are:
>      - move backing file to NVDIMM storage and keep ``pmem=on``
>        (to have NVDIMM with persistence guaranties).
>
> +Using an external DH (Diffie-Hellman) parameters file (since 10.2)
> +''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''
> +
> +Loading of external Diffie-Hellman parameters from a 'dh-params.pem'
> +file is deprecated and will be removed with no replacement in a
> +future release. Where no 'dh-params.pem' file is provided, the DH
> +parameters will be automatically negotiated in accordance with
> +RFC7919.
> +
>  Device options
>  --------------
>
> diff --git a/docs/system/tls.rst b/docs/system/tls.rst
> index a4f6781d62..44c4bf04e9 100644
> --- a/docs/system/tls.rst
> +++ b/docs/system/tls.rst
> @@ -251,11 +251,13 @@ When specifying the object, the ``dir`` parameters
> specifies which
>  directory contains the credential files. This directory is expected to
>  contain files with the names mentioned previously, ``ca-cert.pem``,
>  ``server-key.pem``, ``server-cert.pem``, ``client-key.pem`` and
> -``client-cert.pem`` as appropriate. It is also possible to include a set
> -of pre-generated Diffie-Hellman (DH) parameters in a file
> -``dh-params.pem``, which can be created using the
> -``certtool --generate-dh-params`` command. If omitted, QEMU will
> -dynamically generate DH parameters when loading the credentials.
> +``client-cert.pem`` as appropriate.
> +
> +While it is possible to include a set of pre-generated Diffie-Hellman
> +(DH) parameters in a file ``dh-params.pem``, this facility is now
> +deprecated and will be removed in a future release. When omitted the
> +DH parameters will be automatically negotiated in accordance with
> +RFC7919.
>
>  The ``endpoint`` parameter indicates whether the credentials will be
>  used for a network client or server, and determines which PEM files are
> --
> 2.51.1
>
>

[-- Attachment #2: Type: text/html, Size: 9239 bytes --]