From: Peter Krempa <pkrempa@redhat.com>
Key encipherment is required only for RSA key exchange algorithm. With
TLS 1.3 this is not even used as RSA is used only for authentication.
Since we can't really check when it's required ahead of time drop the
check completely. GnuTLS will moan if it will not be able to use RSA
key exchange.
In commit 11867b0224a2 I tried to relax the check for some eliptic
curve algorithm that explicitly forbid it. Based on the above the proper
solution is to completely remove it.
Resolves: https://issues.redhat.com/browse/RHEL-100711
Fixes: 11867b0224a2b8dc34755ff0ace446b6842df1c1
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
---
src/rpc/virnettlscert.c | 28 ----------------------------
1 file changed, 28 deletions(-)
diff --git a/src/rpc/virnettlscert.c b/src/rpc/virnettlscert.c
index f197995633..7024e858f0 100644
--- a/src/rpc/virnettlscert.c
+++ b/src/rpc/virnettlscert.c
@@ -162,34 +162,6 @@ static int virNetTLSCertCheckKeyUsage(gnutls_x509_crt_t cert,
certFile);
}
}
- if (!(usage & GNUTLS_KEY_KEY_ENCIPHERMENT)) {
- int alg = gnutls_x509_crt_get_pk_algorithm(cert, NULL);
-
- /* Per RFC8813 [1] which amends RFC5580 [2] ECDSA, ECDH, and ECMQV
- * algorithms must not have 'keyEncipherment' present.
- *
- * [1] https://datatracker.ietf.org/doc/rfc8813/
- * [2] https://datatracker.ietf.org/doc/rfc5480
- */
-
- switch (alg) {
- case GNUTLS_PK_ECDSA:
- case GNUTLS_PK_ECDH_X25519:
- case GNUTLS_PK_ECDH_X448:
- break;
-
- default:
- if (critical) {
- virReportError(VIR_ERR_SYSTEM_ERROR,
- _("Certificate %1$s usage does not permit key encipherment"),
- certFile);
- return -1;
- } else {
- VIR_WARN("Certificate %s usage does not permit key encipherment",
- certFile);
- }
- }
- }
}
return 0;
--
2.49.0On Mon, Jun 30, 2025 at 07:25:05PM +0200, Peter Krempa via Devel wrote: > From: Peter Krempa <pkrempa@redhat.com> > > Key encipherment is required only for RSA key exchange algorithm. With > TLS 1.3 this is not even used as RSA is used only for authentication. > > Since we can't really check when it's required ahead of time drop the > check completely. GnuTLS will moan if it will not be able to use RSA > key exchange. GNUTLS only reports problems at runtime, while the libvirt code is used at system startup. This greatly improves the debuggability of sysadmin config screwups, so we don't really want to delegate to GNUTLS for this. > In commit 11867b0224a2 I tried to relax the check for some eliptic > curve algorithm that explicitly forbid it. Based on the above the proper > solution is to completely remove it. We need to invert the check - instead of excluding just ECDSA, we need to include only DSA and GHOST algorithms. 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 :|
On Tue, Jul 01, 2025 at 09:49:57 +0100, Daniel P. Berrangé wrote: > On Mon, Jun 30, 2025 at 07:25:05PM +0200, Peter Krempa via Devel wrote: > > From: Peter Krempa <pkrempa@redhat.com> > > > > Key encipherment is required only for RSA key exchange algorithm. With > > TLS 1.3 this is not even used as RSA is used only for authentication. > > > > Since we can't really check when it's required ahead of time drop the > > check completely. GnuTLS will moan if it will not be able to use RSA > > key exchange. > > GNUTLS only reports problems at runtime, while the libvirt code is > used at system startup. This greatly improves the debuggability of > sysadmin config screwups, so we don't really want to delegate to > GNUTLS for this. > > > In commit 11867b0224a2 I tried to relax the check for some eliptic > > curve algorithm that explicitly forbid it. Based on the above the proper > > solution is to completely remove it. > > We need to invert the check - instead of excluding just ECDSA, we > need to include only DSA and GHOST algorithms. Originally I did the same but then I read (and verified; see my followup) that with TLS 1.3 the RSA key exchange algorithm isn't even used so keyEncipherment capability isn't even needed. The pre-verification doesn't really allow us checking which protocol will be used. If you thing it's important to stay compatible in the pre-check also with older protocols then I can keep it but to me it didn't make too much sense.
On Tue, Jul 01, 2025 at 10:59:06AM +0200, Peter Krempa wrote: > On Tue, Jul 01, 2025 at 09:49:57 +0100, Daniel P. Berrangé wrote: > > On Mon, Jun 30, 2025 at 07:25:05PM +0200, Peter Krempa via Devel wrote: > > > From: Peter Krempa <pkrempa@redhat.com> > > > > > > Key encipherment is required only for RSA key exchange algorithm. With > > > TLS 1.3 this is not even used as RSA is used only for authentication. > > > > > > Since we can't really check when it's required ahead of time drop the > > > check completely. GnuTLS will moan if it will not be able to use RSA > > > key exchange. > > > > GNUTLS only reports problems at runtime, while the libvirt code is > > used at system startup. This greatly improves the debuggability of > > sysadmin config screwups, so we don't really want to delegate to > > GNUTLS for this. > > > > > In commit 11867b0224a2 I tried to relax the check for some eliptic > > > curve algorithm that explicitly forbid it. Based on the above the proper > > > solution is to completely remove it. > > > > We need to invert the check - instead of excluding just ECDSA, we > > need to include only DSA and GHOST algorithms. > > Originally I did the same but then I read (and verified; see my > followup) that with TLS 1.3 the RSA key exchange algorithm isn't even > used so keyEncipherment capability isn't even needed. Ok, yeah, I've found that now too. If we're removing this entirely from the impl, we should also update the docs/kbase/tlscerts.rst. IIRC, we need to remove the 'encryption_key' flag to stop gnutls adding 'key encipherment' to DSA certs. > If you thing it's important to stay compatible in the pre-check also > with older protocols then I can keep it but to me it didn't make too > much sense. Yeah, TLS 1.3 is effectively the baseline for libvirt unless talking to very old systems, as gnutls will preferentially negotiate it, we only need care about libvirt clients. 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 :|
On Tue, Jul 01, 2025 at 11:38:37 +0100, Daniel P. Berrangé wrote: > On Tue, Jul 01, 2025 at 10:59:06AM +0200, Peter Krempa wrote: > > On Tue, Jul 01, 2025 at 09:49:57 +0100, Daniel P. Berrangé wrote: > > > On Mon, Jun 30, 2025 at 07:25:05PM +0200, Peter Krempa via Devel wrote: > > > > From: Peter Krempa <pkrempa@redhat.com> > > > > > > > > Key encipherment is required only for RSA key exchange algorithm. With > > > > TLS 1.3 this is not even used as RSA is used only for authentication. > > > > > > > > Since we can't really check when it's required ahead of time drop the > > > > check completely. GnuTLS will moan if it will not be able to use RSA > > > > key exchange. > > > > > > GNUTLS only reports problems at runtime, while the libvirt code is > > > used at system startup. This greatly improves the debuggability of > > > sysadmin config screwups, so we don't really want to delegate to > > > GNUTLS for this. > > > > > > > In commit 11867b0224a2 I tried to relax the check for some eliptic > > > > curve algorithm that explicitly forbid it. Based on the above the proper > > > > solution is to completely remove it. > > > > > > We need to invert the check - instead of excluding just ECDSA, we > > > need to include only DSA and GHOST algorithms. > > > > Originally I did the same but then I read (and verified; see my > > followup) that with TLS 1.3 the RSA key exchange algorithm isn't even > > used so keyEncipherment capability isn't even needed. > > Ok, yeah, I've found that now too. > > If we're removing this entirely from the impl, we should also update > the docs/kbase/tlscerts.rst. > > IIRC, we need to remove the 'encryption_key' flag to stop gnutls > adding 'key encipherment' to DSA certs. I've found that removing the flag isn't necessary. gnutls just doesn't use it with keys that don't support it. I've actually used my very old script built on the examples from the above page and just added use of one of the fancy new "post quantum" signature algorithms and it simply worked ven with "encryption_key" being present in the 'info' file. > > > If you thing it's important to stay compatible in the pre-check also > > with older protocols then I can keep it but to me it didn't make too > > much sense. > > Yeah, TLS 1.3 is effectively the baseline for libvirt unless talking to > very old systems, as gnutls will preferentially negotiate it, we only > need care about libvirt clients. Based on this I guess we could drop it from the examples. I think that it'd be much more beneficial to hint to better ciphers (but that'd also effectively make it obsolete).
On Tue, Jul 01, 2025 at 01:13:36PM +0200, Peter Krempa wrote: > On Tue, Jul 01, 2025 at 11:38:37 +0100, Daniel P. Berrangé wrote: > > On Tue, Jul 01, 2025 at 10:59:06AM +0200, Peter Krempa wrote: > > > On Tue, Jul 01, 2025 at 09:49:57 +0100, Daniel P. Berrangé wrote: > > > > On Mon, Jun 30, 2025 at 07:25:05PM +0200, Peter Krempa via Devel wrote: > > > > > From: Peter Krempa <pkrempa@redhat.com> > > > > > > > > > > Key encipherment is required only for RSA key exchange algorithm. With > > > > > TLS 1.3 this is not even used as RSA is used only for authentication. > > > > > > > > > > Since we can't really check when it's required ahead of time drop the > > > > > check completely. GnuTLS will moan if it will not be able to use RSA > > > > > key exchange. > > > > > > > > GNUTLS only reports problems at runtime, while the libvirt code is > > > > used at system startup. This greatly improves the debuggability of > > > > sysadmin config screwups, so we don't really want to delegate to > > > > GNUTLS for this. > > > > > > > > > In commit 11867b0224a2 I tried to relax the check for some eliptic > > > > > curve algorithm that explicitly forbid it. Based on the above the proper > > > > > solution is to completely remove it. > > > > > > > > We need to invert the check - instead of excluding just ECDSA, we > > > > need to include only DSA and GHOST algorithms. > > > > > > Originally I did the same but then I read (and verified; see my > > > followup) that with TLS 1.3 the RSA key exchange algorithm isn't even > > > used so keyEncipherment capability isn't even needed. > > > > Ok, yeah, I've found that now too. > > > > If we're removing this entirely from the impl, we should also update > > the docs/kbase/tlscerts.rst. > > > > IIRC, we need to remove the 'encryption_key' flag to stop gnutls > > adding 'key encipherment' to DSA certs. > > I've found that removing the flag isn't necessary. gnutls just doesn't > use it with keys that don't support it. > > I've actually used my very old script built on the examples from the > above page and just added use of one of the fancy new "post quantum" > signature algorithms and it simply worked ven with "encryption_key" > being present in the 'info' file. Yes, gnutls simply ignores the 'encryption_key' setting in most cases, but we should remove it from docs given that it is redundant & potentially misleading. > > > If you thing it's important to stay compatible in the pre-check also > > > with older protocols then I can keep it but to me it didn't make too > > > much sense. > > > > Yeah, TLS 1.3 is effectively the baseline for libvirt unless talking to > > very old systems, as gnutls will preferentially negotiate it, we only > > need care about libvirt clients. > > Based on this I guess we could drop it from the examples. I think that > it'd be much more beneficial to hint to better ciphers (but that'd also > effectively make it obsolete). We've generally assumed that gnutls will pick the best ciphers itself based on crypto priority config on the host. 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 :|
On Tue, Jul 01, 2025 at 11:38:37AM +0100, Daniel P. Berrangé via Devel wrote: > On Tue, Jul 01, 2025 at 10:59:06AM +0200, Peter Krempa wrote: > > On Tue, Jul 01, 2025 at 09:49:57 +0100, Daniel P. Berrangé wrote: > > > On Mon, Jun 30, 2025 at 07:25:05PM +0200, Peter Krempa via Devel wrote: > > > > From: Peter Krempa <pkrempa@redhat.com> > > > > > > > > Key encipherment is required only for RSA key exchange algorithm. With > > > > TLS 1.3 this is not even used as RSA is used only for authentication. > > > > > > > > Since we can't really check when it's required ahead of time drop the > > > > check completely. GnuTLS will moan if it will not be able to use RSA > > > > key exchange. > > > > > > GNUTLS only reports problems at runtime, while the libvirt code is > > > used at system startup. This greatly improves the debuggability of > > > sysadmin config screwups, so we don't really want to delegate to > > > GNUTLS for this. > > > > > > > In commit 11867b0224a2 I tried to relax the check for some eliptic > > > > curve algorithm that explicitly forbid it. Based on the above the proper > > > > solution is to completely remove it. > > > > > > We need to invert the check - instead of excluding just ECDSA, we > > > need to include only DSA and GHOST algorithms. > > > > Originally I did the same but then I read (and verified; see my > > followup) that with TLS 1.3 the RSA key exchange algorithm isn't even > > used so keyEncipherment capability isn't even needed. > > Ok, yeah, I've found that now too. > > If we're removing this entirely from the impl, we should also update > the docs/kbase/tlscerts.rst. > > IIRC, we need to remove the 'encryption_key' flag to stop gnutls > adding 'key encipherment' to DSA certs. Also our test suite uses GNUTLS_KEY_KEY_ENCIPHERMENT when creating test certs and so that can go. 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 :|
On Mon, Jun 30, 2025 at 19:25:05 +0200, Peter Krempa wrote: > From: Peter Krempa <pkrempa@redhat.com> > > Key encipherment is required only for RSA key exchange algorithm. With > TLS 1.3 this is not even used as RSA is used only for authentication. > > Since we can't really check when it's required ahead of time drop the > check completely. GnuTLS will moan if it will not be able to use RSA > key exchange. > > In commit 11867b0224a2 I tried to relax the check for some eliptic > curve algorithm that explicitly forbid it. Based on the above the proper > solution is to completely remove it. > > Resolves: https://issues.redhat.com/browse/RHEL-100711 > Fixes: 11867b0224a2b8dc34755ff0ace446b6842df1c1 > Signed-off-by: Peter Krempa <pkrempa@redhat.com> > --- I've tested this by creating a client certificate with following info: organization = test cn = HOST tls_www_server signing_key (thus missing 'encryption_key' field) Attempting to use currently relased libvirt results in: $ virsh -c qemu+tls://speedmetal/system list error: failed to connect to the hypervisor error: Certificate /etc/pki/libvirt/clientcert.pem usage does not permit key encipherment Whereas with patched libvirt: $ ./build/libvirt/gcc/tools/virsh -c qemu+tls://speedmetal/system list Id Name State ---------------------- 1 ha running
© 2016 - 2025 Red Hat, Inc.