[PATCH 0/2] virnettlscontext: Don't set DH parameters ourselves

Suggested by Dan here: https://listman.redhat.com/archives/libvir-list/2022-January/226681.html Since we can bump min gnutls version we don't have to care about pre-3.6.0 release he mentions in the e-mail. Michal Prívozník (2): meson: Require gnutls-3.6.0 or newer virnettlscontext: Don't set DH parameters ourselves meson.build | 2 +- src/rpc/virnettlscontext.c | 41 -------------------------------------- tests/virrandommock.c | 36 --------------------------------- 3 files changed, 1 insertion(+), 78 deletions(-) -- 2.35.1

Released almost 5 years ago, gnutls-3.6.0 brings some important features (which are utilized in next commit). Hence, require that version at least. Per repology, currently shipped versions are: RHEL-8: 3.6.16 RHEL-9: 3.7.3 Debian 11: 3.7.1 Debian 12: 3.7.6 openSUSE Leap 15.3: 3.6.7 Ubuntu LTS 20.04: 3.6.13 Ubuntu LTS 22.04: 3.7.3 FreeBSD 12: 3.7.6 Fedora 34: 3.7.4 Fedora 35: 3.7.6 Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- meson.build | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/meson.build b/meson.build index e03f330f3d..864462c6dc 100644 --- a/meson.build +++ b/meson.build @@ -982,7 +982,7 @@ conf.set('GLIB_VERSION_MAX_ALLOWED', glib_version_str) glusterfs_version = '3.4.1' glusterfs_dep = dependency('glusterfs-api', version: '>=' + glusterfs_version, required: get_option('glusterfs')) -gnutls_version = '3.2.0' +gnutls_version = '3.6.0' gnutls_dep = dependency('gnutls', version: '>=' + gnutls_version) # Check for BSD kvm (kernel memory interface) -- 2.35.1

According to [1]: Prior to GnuTLS 3.6.0 for the ephemeral or anonymous Diffie-Hellman (DH) TLS ciphersuites the application was required to generate or provide DH parameters. That is no longer necessary as GnuTLS utilizes DH parameters and negotiation from [RFC7919]. This allows us to: a) drop the code that's setting DH params, b) drop @dhParams member from _virNetTLSContext struct. and c) drop gnutls_dh_params_generate2() mock. 1: https://www.gnutls.org/manual/html_node/Parameter-generation.html Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/rpc/virnettlscontext.c | 41 -------------------------------------- tests/virrandommock.c | 36 --------------------------------- 2 files changed, 77 deletions(-) diff --git a/src/rpc/virnettlscontext.c b/src/rpc/virnettlscontext.c index bdbf01855d..acfc4f9323 100644 --- a/src/rpc/virnettlscontext.c +++ b/src/rpc/virnettlscontext.c @@ -54,7 +54,6 @@ struct _virNetTLSContext { virObjectLockable parent; gnutls_certificate_credentials_t x509cred; - gnutls_dh_params_t dhParams; bool isServer; bool requireValidCert; @@ -709,40 +708,6 @@ static virNetTLSContext *virNetTLSContextNew(const char *cacert, if (virNetTLSContextLoadCredentials(ctxt, isServer, cacert, cacrl, cert, key) < 0) goto error; - /* Generate Diffie Hellman parameters - for use with DHE - * kx algorithms. These should be discarded and regenerated - * once a day, once a week or once a month. Depending on the - * security requirements. - */ - if (isServer) { - unsigned int bits = 0; - - bits = gnutls_sec_param_to_pk_bits(GNUTLS_PK_DH, GNUTLS_SEC_PARAM_MEDIUM); - if (bits == 0) { - virReportError(VIR_ERR_SYSTEM_ERROR, "%s", - _("Unable to get key length for diffie-hellman parameters")); - goto error; - } - - err = gnutls_dh_params_init(&ctxt->dhParams); - if (err < 0) { - virReportError(VIR_ERR_SYSTEM_ERROR, - _("Unable to initialize diffie-hellman parameters: %s"), - gnutls_strerror(err)); - goto error; - } - err = gnutls_dh_params_generate2(ctxt->dhParams, bits); - if (err < 0) { - virReportError(VIR_ERR_SYSTEM_ERROR, - _("Unable to generate diffie-hellman parameters: %s"), - gnutls_strerror(err)); - goto error; - } - - gnutls_certificate_set_dh_params(ctxt->x509cred, - ctxt->dhParams); - } - ctxt->requireValidCert = requireValidCert; ctxt->x509dnACL = x509dnACL; ctxt->isServer = isServer; @@ -754,8 +719,6 @@ static virNetTLSContext *virNetTLSContextNew(const char *cacert, return ctxt; error: - if (isServer) - gnutls_dh_params_deinit(ctxt->dhParams); virObjectUnref(ctxt); return NULL; } @@ -950,9 +913,6 @@ int virNetTLSContextReloadForServer(virNetTLSContext *ctxt, if (virNetTLSContextLoadCredentials(ctxt, true, cacert, cacrl, cert, key)) goto error; - gnutls_certificate_set_dh_params(ctxt->x509cred, - ctxt->dhParams); - gnutls_certificate_free_credentials(x509credBak); return 0; @@ -1156,7 +1116,6 @@ void virNetTLSContextDispose(void *obj) "ctxt=%p", ctxt); g_free(ctxt->priority); - gnutls_dh_params_deinit(ctxt->dhParams); gnutls_certificate_free_credentials(ctxt->x509cred); } diff --git a/tests/virrandommock.c b/tests/virrandommock.c index e295f74446..2673230cf7 100644 --- a/tests/virrandommock.c +++ b/tests/virrandommock.c @@ -20,8 +20,6 @@ #ifndef WIN32 -# include <gnutls/gnutls.h> - # include "internal.h" # include "virrandom.h" # include "virmock.h" @@ -57,40 +55,6 @@ int virRandomGenerateWWN(char **wwn, return 0; } - -static int (*real_gnutls_dh_params_generate2)(gnutls_dh_params_t dparams, - unsigned int bits); - -static gnutls_dh_params_t params_cache; -static unsigned int cachebits; - -int -gnutls_dh_params_generate2(gnutls_dh_params_t dparams, - unsigned int bits) -{ - int rc = 0; - - VIR_MOCK_REAL_INIT(gnutls_dh_params_generate2); - - if (!params_cache) { - if (gnutls_dh_params_init(¶ms_cache) < 0) { - fprintf(stderr, "Error initializing params cache"); - abort(); - } - rc = real_gnutls_dh_params_generate2(params_cache, bits); - - if (rc < 0) - return rc; - cachebits = bits; - } - - if (cachebits != bits) { - fprintf(stderr, "Requested bits do not match the cached value"); - abort(); - } - - return gnutls_dh_params_cpy(dparams, params_cache); -} #else /* WIN32 */ /* Can't mock on WIN32 */ #endif -- 2.35.1

Reviewed-by: Ján Tomko <jtomko@redhat.com> On Thu, Jun 30, 2022 at 10:56 AM Michal Privoznik <mprivozn@redhat.com> wrote:
Suggested by Dan here:
https://listman.redhat.com/archives/libvir-list/2022-January/226681.html
Since we can bump min gnutls version we don't have to care about pre-3.6.0 release he mentions in the e-mail.
Michal Prívozník (2): meson: Require gnutls-3.6.0 or newer virnettlscontext: Don't set DH parameters ourselves
meson.build | 2 +- src/rpc/virnettlscontext.c | 41 -------------------------------------- tests/virrandommock.c | 36 --------------------------------- 3 files changed, 1 insertion(+), 78 deletions(-)
-- 2.35.1
participants (2)
-
Jano Tomko
-
Michal Privoznik