On Tue, Dec 21, 2021 at 03:22:59PM +0100, Michal Privoznik wrote: > As encryption norms get more strict it's easy to fall on the > insecure side. For instance, so far we are generating 2048 bits > long prime for Diffie-Hellman keys. Some systems consider this > not long enough. While we may just keep increasing the value > passed to the corresponding gnutls_* function, that is not well > maintainable. Instead, we may do what's recommended in the > gnutls_* manpage. From gnutls_dh_params_generate2(3): > > It is recommended not to set the number of bits directly, but > use gnutls_sec_param_to_pk_bits() instead. > > Looking into the gnutls_sec_param_to_pk_bits() then [1], 2048 > bits corresponds to parameter MEDIUM. Therefore, we want to chose > the next size (HIGH) to be future proof. > > 1: https://www.gnutls.org/manual/gnutls.html#tab_003akey_002dsizes > > Signed-off-by: Michal Privoznik <mprivozn@xxxxxxxxxx> > --- > src/rpc/virnettlscontext.c | 13 ++++++++++--- > 1 file changed, 10 insertions(+), 3 deletions(-) > > diff --git a/src/rpc/virnettlscontext.c b/src/rpc/virnettlscontext.c > index 3b6687e7f4..f0b1e8f9c1 100644 > --- a/src/rpc/virnettlscontext.c > +++ b/src/rpc/virnettlscontext.c > @@ -38,8 +38,6 @@ > #include "virthread.h" > #include "configmake.h" > > -#define DH_BITS 2048 > - > #define LIBVIRT_PKI_DIR SYSCONFDIR "/pki" > #define LIBVIRT_CACERT LIBVIRT_PKI_DIR "/CA/cacert.pem" > #define LIBVIRT_CACRL LIBVIRT_PKI_DIR "/CA/cacrl.pem" > @@ -718,6 +716,15 @@ static virNetTLSContext *virNetTLSContextNew(const char *cacert, > * security requirements. > */ > if (isServer) { > + unsigned int bits = 0; > + > + bits = gnutls_sec_param_to_pk_bits(GNUTLS_PK_DH, GNUTLS_SEC_PARAM_HIGH); > + 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, > @@ -725,7 +732,7 @@ static virNetTLSContext *virNetTLSContextNew(const char *cacert, > gnutls_strerror(err)); > goto error; > } > - err = gnutls_dh_params_generate2(ctxt->dhParams, DH_BITS); > + err = gnutls_dh_params_generate2(ctxt->dhParams, bits); > if (err < 0) { > virReportError(VIR_ERR_SYSTEM_ERROR, > _("Unable to generate diffie-hellman parameters: %s"), We shouldn't be introducing use of gnutls_sec_param_to_pk_bits at all IMHO, rather we should be removing use of gnutls_dh_params_generate2 instead. The recommendation is to use pre-generated DH parameters from the the FFDHE set of RFC7919. In gnutls >= 3.6.0 this happens automatically. In gnutls >= 3.5.6 && < 3.6.0 we can replace thegnutls_dh_params_generate2 + gnutls_certificate_set_dh_params pair of calls, with just gnutls_certificate_set_known_dh_params() 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 :|
