On Thu, Jul 17, 2025 at 05:28:09PM +0200, Peter Krempa via Devel wrote:
From: Peter Krempa <pkrempa(a)redhat.com>
Similarly to how we iterate the list of CAs in the concatenated bundle
there's a possibility of the server/client certificates to be
concatenated as well.
If for some case the first certificate is okay but the further one have
e.g. invalid signatures the validation code would not reject them but
we'd encounter failures later when gnutls tries to use them.
Iterate also the client/server certs rather than just the CAs.
Was there some bug that motivated this change ?
I'd like to keep libvirt's behaviour in sync with QEMU's
behaviour to the greatest extent possible. I've just been
finalizing changes to QEMU to cope with mutliple certs
in the server-cert.pem file, but the semantics there are
the certs are a chain of intermediate certs, all of which
must be valid.
ie, currently we allow
ca-cert.pem server-cert.pem
| |
|------+--------------------------| |---+-------|
Root CA -> Sub CA 1 -> Sub CA 2 -> Server Cert
but the intent is to support
ca-cert.pem server-cert.pem
| |
|------+--| |----------------------------+-------|
Root CA -> Sub CA 1 -> Sub CA 2 -> Server Cert
Or
ca-cert.pem server-cert.pem
| |
|------+-------------| |---------------+-------|
Root CA -> Sub CA 1 -> Sub CA 2 -> Server Cert
the rational is that these splits reflect how some CA will
give you your certs to begin with, and we want to allow
them to be used directly.
My intent was to copy the QEMU change into libvirt once it
was merged in QEMU, so from that POV I'm not a fan of making
the some of the changes in this series.
Beyond that I'm also working post-quantum crypto support,
which will require us to load multiple distinct server-cert-NNN.pem
files, each with an independant set of certs, which are selected
at runtime based on negotiated ciphers in the TLS handshake.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/rpc/virnettlscert.c | 29 +++++++++++++++++------------
1 file changed, 17 insertions(+), 12 deletions(-)
diff --git a/src/rpc/virnettlscert.c b/src/rpc/virnettlscert.c
index 3efc4f0716..2724f55bbe 100644
--- a/src/rpc/virnettlscert.c
+++ b/src/rpc/virnettlscert.c
@@ -442,38 +442,43 @@ int virNetTLSCertSanityCheck(bool isServer,
const char *cacertFile,
const char *certFile)
{
- gnutls_x509_crt_t cert = NULL;
+ gnutls_x509_crt_t certs[MAX_CERTS] = { 0 };
+ size_t ncerts = 0;
gnutls_x509_crt_t cacerts[MAX_CERTS] = { 0 };
size_t ncacerts = 0;
size_t i;
int ret = -1;
if ((access(certFile, R_OK) == 0) &&
- !(cert = virNetTLSCertLoadFromFile(certFile, isServer)))
+ virNetTLSCertLoadListFromFile(certFile, certs, MAX_CERTS, &ncerts) < 0)
goto cleanup;
+
if ((access(cacertFile, R_OK) == 0) &&
virNetTLSCertLoadListFromFile(cacertFile, cacerts,
MAX_CERTS, &ncacerts) < 0)
goto cleanup;
- if (cert &&
- virNetTLSCertCheck(cert, certFile, isServer, false) < 0)
- goto cleanup;
-
for (i = 0; i < ncacerts; i++) {
- if (virNetTLSCertCheck(cacerts[i], cacertFile, isServer, true) < 0)
+ g_autofree char *cacertid = g_strdup_printf("%s[%zu]", cacertFile,
i);
+ if (virNetTLSCertCheck(cacerts[i], cacertid, isServer, true) < 0)
goto cleanup;
}
- if (cert && ncacerts &&
- virNetTLSCertCheckPair(cert, certFile, cacerts, ncacerts, cacertFile, isServer)
< 0)
- goto cleanup;
+ for (i = 0; i < ncerts; i++) {
+ g_autofree char *certid = g_strdup_printf("%s[%zu]", certFile, i);
+ if (virNetTLSCertCheck(certs[i], certid, isServer, false) < 0)
+ goto cleanup;
+
+ if (ncacerts &&
+ virNetTLSCertCheckPair(certs[i], certid, cacerts, ncacerts, cacertFile,
isServer) < 0)
+ goto cleanup;
+ }
ret = 0;
cleanup:
- if (cert)
- gnutls_x509_crt_deinit(cert);
+ for (i = 0; i < ncerts; i++)
+ gnutls_x509_crt_deinit(certs[i]);
for (i = 0; i < ncacerts; i++)
gnutls_x509_crt_deinit(cacerts[i]);
return ret;
--
2.50.0
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 :|