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

Daniel P. Berrangé posted 21 patches 2 weeks, 1 day ago
Maintainers: "Daniel P. Berrangé" <berrange@redhat.com>, "Marc-André Lureau" <marcandre.lureau@redhat.com>
[PATCH 16/21] crypto: deprecate use of external dh-params.pem file
Posted by Daniel P. Berrangé 2 weeks, 1 day ago
GNUTLS has deprecated use of externally provided diffie-hellman
parameters, since it will automatically negotiate DH params in
accordance with RFC7919.

Signed-off-by: Daniel P. Berrangé <berrange@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


Re: [PATCH 16/21] crypto: deprecate use of external dh-params.pem file
Posted by Marc-André Lureau 2 weeks ago
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
>
>
Re: [PATCH 16/21] crypto: deprecate use of external dh-params.pem file
Posted by Daniel P. Berrangé via Devel 2 weeks ago
On Fri, Oct 31, 2025 at 03:32:51PM +0400, Marc-André Lureau wrote:
> 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?

Oh whoops. I mis-read the meson.build rules. Our gnutls bump
to 3.5.18 was done in:

  commit d4c7ee330cd0ca05cc0c026f845af6711e37b0f7
  Author: Daniel P. Berrangé <berrange@redhat.com>
  Date:   Fri May 14 13:04:09 2021 +0100

    crypto: bump min gnutls to 3.5.18, dropping RHEL-7 support
    
    It has been over two years since RHEL-8 was released, and thus per the
    platform build policy, we no longer need to support RHEL-7 as a build
    target. This lets us increment the minimum required gnutls version
    
    Per repology, current shipping versions are:
    
                 RHEL-8: 3.6.14
          Debian Buster: 3.6.7
     openSUSE Leap 15.2: 3.6.7
       Ubuntu LTS 18.04: 3.5.18
       Ubuntu LTS 20.04: 3.6.13
                FreeBSD: 3.6.15
              Fedora 33: 3.6.16
              Fedora 34: 3.7.1
                OpenBSD: 3.6.15
         macOS HomeBrew: 3.6.15
    

the only one not already on 3.6 was Ubuntu 18.04 and that
is long outside our support matrix. IOW we can easily assume
at least 3.6 these days and this patch is safe on that basis.

I'll prepare another standalone patch to explicit increase
the min version though. Can probably bump gcrypt & nettle min
versions too.


With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|