[PATCH 00/10] remote: support multiple certificate identities
This series adds support for multiple certificate identities. This is intended to aid in the transition to post-quantum cryptography by allowing use of certs with RSA in parallel with certs using MLDSA algorithms. Daniel P. Berrangé (10): remote: use g_strfreev for free()ing lists of strings rpc: change 'isServer' parameter from 'int' to 'bool' rpc: refactor TLS sanity checking to support many cert files rpc: add support for loading multiple certs & keys remote: support specifying multiple keys/certs in libvirtd.conf rpc: skip fallback when using custom PKI path rpc: move file access checks into TLS config API rpc: reduce duplication when locating credentials rpc: support loading multiple certificate identities docs: describe support for multiple certs & PQC config docs/kbase/tlscerts.rst | 88 +++++++++ po/POTFILES | 1 + src/libvirt_probes.d | 3 +- src/remote/libvirtd.aug.in | 2 + src/remote/libvirtd.conf.in | 16 ++ src/remote/remote_daemon.c | 24 +-- src/remote/remote_daemon_config.c | 66 ++++--- src/remote/remote_daemon_config.h | 4 +- src/remote/test_libvirtd.aug.in | 8 + src/rpc/virnettlscert.c | 35 ++-- src/rpc/virnettlscert.h | 2 +- src/rpc/virnettlsconfig.c | 302 +++++++++++++++++++++++++----- src/rpc/virnettlsconfig.h | 44 +++-- src/rpc/virnettlscontext.c | 231 +++++++++++------------ src/rpc/virnettlscontext.h | 26 +-- tests/virnettlscontexttest.c | 10 +- tests/virnettlssessiontest.c | 9 +- tools/virt-pki-validate.c | 3 +- 18 files changed, 612 insertions(+), 262 deletions(-) -- 2.51.1
From: Daniel P. Berrangé <berrange@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- src/remote/remote_daemon_config.c | 23 +++-------------------- 1 file changed, 3 insertions(+), 20 deletions(-) diff --git a/src/remote/remote_daemon_config.c b/src/remote/remote_daemon_config.c index 2a41c27dfc..c1e75444e1 100644 --- a/src/remote/remote_daemon_config.c +++ b/src/remote/remote_daemon_config.c @@ -168,8 +168,6 @@ daemonConfigNew(bool privileged G_GNUC_UNUSED) void daemonConfigFree(struct daemonConfig *data) { - char **tmp; - if (!data) return; @@ -179,12 +177,7 @@ daemonConfigFree(struct daemonConfig *data) g_free(data->tcp_port); #endif /* ! WITH_IP */ - tmp = data->access_drivers; - while (tmp && *tmp) { - g_free(*tmp); - tmp++; - } - g_free(data->access_drivers); + g_strfreev(data->access_drivers); g_free(data->unix_sock_admin_perms); g_free(data->unix_sock_ro_perms); @@ -192,20 +185,10 @@ daemonConfigFree(struct daemonConfig *data) g_free(data->unix_sock_group); g_free(data->unix_sock_dir); - tmp = data->sasl_allowed_username_list; - while (tmp && *tmp) { - g_free(*tmp); - tmp++; - } - g_free(data->sasl_allowed_username_list); + g_strfreev(data->sasl_allowed_username_list); #ifdef WITH_IP - tmp = data->tls_allowed_dn_list; - while (tmp && *tmp) { - g_free(*tmp); - tmp++; - } - g_free(data->tls_allowed_dn_list); + g_strfreev(data->tls_allowed_dn_list); g_free(data->tls_priority); -- 2.51.1
From: Daniel P. Berrangé <berrange@redhat.com> The callers are all passing in a 'bool' value, and this type should be maintained rather than cast to 'int' and then inpreted as a bool again later. Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- src/rpc/virnettlsconfig.c | 14 +++++++------- src/rpc/virnettlsconfig.h | 13 +++++++------ 2 files changed, 14 insertions(+), 13 deletions(-) diff --git a/src/rpc/virnettlsconfig.c b/src/rpc/virnettlsconfig.c index d020083d6a..ffab3b4fc8 100644 --- a/src/rpc/virnettlsconfig.c +++ b/src/rpc/virnettlsconfig.c @@ -49,7 +49,7 @@ static void virNetTLSConfigTrust(const char *cacertdir, VIR_DEBUG("TLS CA CRL %s", *cacrl); } -static void virNetTLSConfigIdentity(int isServer, +static void virNetTLSConfigIdentity(bool isServer, const char *certdir, const char *keydir, char **cert, @@ -102,7 +102,7 @@ void virNetTLSConfigSystemTrust(char **cacert, } void virNetTLSConfigCustomIdentity(const char *pkipath, - int isServer, + bool isServer, char **cert, char **key) { @@ -114,7 +114,7 @@ void virNetTLSConfigCustomIdentity(const char *pkipath, key); } -void virNetTLSConfigUserIdentity(int isServer, +void virNetTLSConfigUserIdentity(bool isServer, char **cert, char **key) { @@ -129,7 +129,7 @@ void virNetTLSConfigUserIdentity(int isServer, key); } -void virNetTLSConfigSystemIdentity(int isServer, +void virNetTLSConfigSystemIdentity(bool isServer, char **cert, char **key) { @@ -143,7 +143,7 @@ void virNetTLSConfigSystemIdentity(int isServer, } void virNetTLSConfigCustomCreds(const char *pkipath, - int isServer, + bool isServer, char **cacert, char **cacrl, char **cert, @@ -161,7 +161,7 @@ void virNetTLSConfigCustomCreds(const char *pkipath, key); } -void virNetTLSConfigUserCreds(int isServer, +void virNetTLSConfigUserCreds(bool isServer, char **cacert, char **cacrl, char **cert, @@ -182,7 +182,7 @@ void virNetTLSConfigUserCreds(int isServer, key); } -void virNetTLSConfigSystemCreds(int isServer, +void virNetTLSConfigSystemCreds(bool isServer, char **cacert, char **cacrl, char **cert, diff --git a/src/rpc/virnettlsconfig.h b/src/rpc/virnettlsconfig.h index 797b3e3ac5..a9378c18b7 100644 --- a/src/rpc/virnettlsconfig.h +++ b/src/rpc/virnettlsconfig.h @@ -20,6 +20,7 @@ #pragma once +#include "internal.h" #include "configmake.h" #define LIBVIRT_PKI_DIR SYSCONFDIR "/pki" @@ -39,29 +40,29 @@ void virNetTLSConfigSystemTrust(char **cacert, char **cacrl); void virNetTLSConfigCustomIdentity(const char *pkipath, - int isServer, + bool isServer, char **cert, char **key); -void virNetTLSConfigUserIdentity(int isServer, +void virNetTLSConfigUserIdentity(bool isServer, char **cert, char **key); -void virNetTLSConfigSystemIdentity(int isServer, +void virNetTLSConfigSystemIdentity(bool isServer, char **cert, char **key); void virNetTLSConfigCustomCreds(const char *pkipath, - int isServer, + bool isServer, char **cacert, char **cacrl, char **cert, char **key); -void virNetTLSConfigUserCreds(int isServer, +void virNetTLSConfigUserCreds(bool isServer, char **cacert, char **cacrl, char **cert, char **key); -void virNetTLSConfigSystemCreds(int isServer, +void virNetTLSConfigSystemCreds(bool isServer, char **cacert, char **cacrl, char **cert, -- 2.51.1
From: Daniel P. Berrangé <berrange@redhat.com> Future patches will make it possible to load multiple certificate files. This prepares the sanity checking code to support that by taking a NUL terminated array of cert filenames. Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- src/rpc/virnettlscert.c | 35 ++++++++++++++++++++++------------- src/rpc/virnettlscert.h | 2 +- src/rpc/virnettlscontext.c | 6 ++++-- tools/virt-pki-validate.c | 3 ++- 4 files changed, 29 insertions(+), 17 deletions(-) diff --git a/src/rpc/virnettlscert.c b/src/rpc/virnettlscert.c index 3efc4f0716..6f20b2601b 100644 --- a/src/rpc/virnettlscert.c +++ b/src/rpc/virnettlscert.c @@ -440,40 +440,49 @@ int virNetTLSCertLoadListFromFile(const char *certFile, #define MAX_CERTS 16 int virNetTLSCertSanityCheck(bool isServer, const char *cacertFile, - const char *certFile) + const char *const *certFiles) { - gnutls_x509_crt_t cert = NULL; + gnutls_x509_crt_t *certs = NULL; 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))) - goto cleanup; + certs = g_new0(gnutls_x509_crt_t, g_strv_length((gchar **)certFiles)); + for (i = 0; certFiles[i] != NULL; i++) { + if ((access(certFiles[i], R_OK) == 0) && + !(certs[i] = virNetTLSCertLoadFromFile(certFiles[i], isServer))) + 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; certFiles[i] != NULL; i++) { + if (certs[i] && + virNetTLSCertCheck(certs[i], certFiles[i], isServer, false) < 0) + goto cleanup; + } for (i = 0; i < ncacerts; i++) { if (virNetTLSCertCheck(cacerts[i], cacertFile, isServer, true) < 0) goto cleanup; } - if (cert && ncacerts && - virNetTLSCertCheckPair(cert, certFile, cacerts, ncacerts, cacertFile, isServer) < 0) - goto cleanup; + for (i = 0; certFiles[i] != NULL && ncacerts; i++) { + if (certs[i] && ncacerts && + virNetTLSCertCheckPair(certs[i], certFiles[i], cacerts, ncacerts, cacertFile, isServer) < 0) + goto cleanup; + } ret = 0; cleanup: - if (cert) - gnutls_x509_crt_deinit(cert); + for (i = 0; certFiles[i] != NULL; i++) { + if (certs[i]) + gnutls_x509_crt_deinit(certs[i]); + } for (i = 0; i < ncacerts; i++) gnutls_x509_crt_deinit(cacerts[i]); return ret; diff --git a/src/rpc/virnettlscert.h b/src/rpc/virnettlscert.h index a2f591d172..086d8dc7d6 100644 --- a/src/rpc/virnettlscert.h +++ b/src/rpc/virnettlscert.h @@ -28,7 +28,7 @@ int virNetTLSCertSanityCheck(bool isServer, const char *cacertFile, - const char *certFile); + const char *const *certFiles); int virNetTLSCertValidateCA(gnutls_x509_crt_t cert, bool isServer); diff --git a/src/rpc/virnettlscontext.c b/src/rpc/virnettlscontext.c index f857bb2339..bb9db90dff 100644 --- a/src/rpc/virnettlscontext.c +++ b/src/rpc/virnettlscontext.c @@ -199,6 +199,7 @@ static virNetTLSContext *virNetTLSContextNew(const char *cacert, { virNetTLSContext *ctxt; int err; + const char *certs[] = { cert, NULL }; if (virNetTLSContextInitialize() < 0) return NULL; @@ -224,7 +225,7 @@ static virNetTLSContext *virNetTLSContextNew(const char *cacert, } if (sanityCheckCert && - virNetTLSCertSanityCheck(isServer, cacert, cert) < 0) + virNetTLSCertSanityCheck(isServer, cacert, certs) < 0) goto error; if (virNetTLSContextLoadCredentials(ctxt, isServer, cacert, cacrl, cert, key) < 0) @@ -367,6 +368,7 @@ int virNetTLSContextReloadForServer(virNetTLSContext *ctxt, g_autofree char *cacrl = NULL; g_autofree char *cert = NULL; g_autofree char *key = NULL; + const char *certs[] = { cert, NULL }; x509credBak = g_steal_pointer(&ctxt->x509cred); @@ -382,7 +384,7 @@ int virNetTLSContextReloadForServer(virNetTLSContext *ctxt, goto error; } - if (virNetTLSCertSanityCheck(true, cacert, cert)) + if (virNetTLSCertSanityCheck(true, cacert, certs)) goto error; if (virNetTLSContextLoadCredentials(ctxt, true, cacert, cacrl, cert, key)) diff --git a/tools/virt-pki-validate.c b/tools/virt-pki-validate.c index e693ffaed6..0df289256e 100644 --- a/tools/virt-pki-validate.c +++ b/tools/virt-pki-validate.c @@ -163,6 +163,7 @@ virPKIValidateIdentity(bool isServer, bool system, const char *path) { g_autofree char *cacert = NULL, *cacrl = NULL; g_autofree char *cert = NULL, *key = NULL; + const char *certs[] = { cert, NULL }; bool ok = true; const char *scope = isServer ? "SERVER" : "CLIENT"; @@ -274,7 +275,7 @@ virPKIValidateIdentity(bool isServer, bool system, const char *path) if (virNetTLSCertSanityCheck(isServer, cacert, - cert) < 0) { + certs) < 0) { virValidateFail(VIR_VALIDATE_FAIL, "%s", virGetLastErrorMessage()); ok = false; -- 2.51.1
On 11/6/25 15:50, Daniel P. Berrangé via Devel wrote:
From: Daniel P. Berrangé <berrange@redhat.com>
Future patches will make it possible to load multiple certificate files. This prepares the sanity checking code to support that by taking a NUL terminated array of cert filenames.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- src/rpc/virnettlscert.c | 35 ++++++++++++++++++++++------------- src/rpc/virnettlscert.h | 2 +- src/rpc/virnettlscontext.c | 6 ++++-- tools/virt-pki-validate.c | 3 ++- 4 files changed, 29 insertions(+), 17 deletions(-)
diff --git a/src/rpc/virnettlscert.c b/src/rpc/virnettlscert.c index 3efc4f0716..6f20b2601b 100644 --- a/src/rpc/virnettlscert.c +++ b/src/rpc/virnettlscert.c @@ -440,40 +440,49 @@ int virNetTLSCertLoadListFromFile(const char *certFile, #define MAX_CERTS 16 int virNetTLSCertSanityCheck(bool isServer, const char *cacertFile, - const char *certFile) + const char *const *certFiles) { - gnutls_x509_crt_t cert = NULL; + gnutls_x509_crt_t *certs = NULL;
This ^^ needs to be g_autofree so that it doesn't leak.
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))) - goto cleanup; + certs = g_new0(gnutls_x509_crt_t, g_strv_length((gchar **)certFiles)); + for (i = 0; certFiles[i] != NULL; i++) { + if ((access(certFiles[i], R_OK) == 0) && + !(certs[i] = virNetTLSCertLoadFromFile(certFiles[i], isServer))) + 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; certFiles[i] != NULL; i++) { + if (certs[i] && + virNetTLSCertCheck(certs[i], certFiles[i], isServer, false) < 0) + goto cleanup; + }
for (i = 0; i < ncacerts; i++) { if (virNetTLSCertCheck(cacerts[i], cacertFile, isServer, true) < 0) goto cleanup; }
- if (cert && ncacerts && - virNetTLSCertCheckPair(cert, certFile, cacerts, ncacerts, cacertFile, isServer) < 0) - goto cleanup; + for (i = 0; certFiles[i] != NULL && ncacerts; i++) { + if (certs[i] && ncacerts && + virNetTLSCertCheckPair(certs[i], certFiles[i], cacerts, ncacerts, cacertFile, isServer) < 0) + goto cleanup; + }
ret = 0;
cleanup: - if (cert) - gnutls_x509_crt_deinit(cert); + for (i = 0; certFiles[i] != NULL; i++) { + if (certs[i]) + gnutls_x509_crt_deinit(certs[i]); + } for (i = 0; i < ncacerts; i++) gnutls_x509_crt_deinit(cacerts[i]); return ret;
Michal
On Fri, Nov 07, 2025 at 10:03:30AM +0100, Michal Prívozník wrote:
On 11/6/25 15:50, Daniel P. Berrangé via Devel wrote:
From: Daniel P. Berrangé <berrange@redhat.com>
Future patches will make it possible to load multiple certificate files. This prepares the sanity checking code to support that by taking a NUL terminated array of cert filenames.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- src/rpc/virnettlscert.c | 35 ++++++++++++++++++++++------------- src/rpc/virnettlscert.h | 2 +- src/rpc/virnettlscontext.c | 6 ++++-- tools/virt-pki-validate.c | 3 ++- 4 files changed, 29 insertions(+), 17 deletions(-)
diff --git a/src/rpc/virnettlscert.c b/src/rpc/virnettlscert.c index 3efc4f0716..6f20b2601b 100644 --- a/src/rpc/virnettlscert.c +++ b/src/rpc/virnettlscert.c @@ -440,40 +440,49 @@ int virNetTLSCertLoadListFromFile(const char *certFile, #define MAX_CERTS 16 int virNetTLSCertSanityCheck(bool isServer, const char *cacertFile, - const char *certFile) + const char *const *certFiles) { - gnutls_x509_crt_t cert = NULL; + gnutls_x509_crt_t *certs = NULL;
This ^^ needs to be g_autofree so that it doesn't leak.
I'll do a manual free for this one - since we have to manually free the elements, IMHO it would be confusing to have a mix of manual free and auto-free.
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))) - goto cleanup; + certs = g_new0(gnutls_x509_crt_t, g_strv_length((gchar **)certFiles)); + for (i = 0; certFiles[i] != NULL; i++) { + if ((access(certFiles[i], R_OK) == 0) && + !(certs[i] = virNetTLSCertLoadFromFile(certFiles[i], isServer))) + 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; certFiles[i] != NULL; i++) { + if (certs[i] && + virNetTLSCertCheck(certs[i], certFiles[i], isServer, false) < 0) + goto cleanup; + }
for (i = 0; i < ncacerts; i++) { if (virNetTLSCertCheck(cacerts[i], cacertFile, isServer, true) < 0) goto cleanup; }
- if (cert && ncacerts && - virNetTLSCertCheckPair(cert, certFile, cacerts, ncacerts, cacertFile, isServer) < 0) - goto cleanup; + for (i = 0; certFiles[i] != NULL && ncacerts; i++) { + if (certs[i] && ncacerts && + virNetTLSCertCheckPair(certs[i], certFiles[i], cacerts, ncacerts, cacertFile, isServer) < 0) + goto cleanup; + }
ret = 0;
cleanup: - if (cert) - gnutls_x509_crt_deinit(cert); + for (i = 0; certFiles[i] != NULL; i++) { + if (certs[i]) + gnutls_x509_crt_deinit(certs[i]); + } for (i = 0; i < ncacerts; i++) gnutls_x509_crt_deinit(cacerts[i]); return ret;
Michal
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 :|
From: Daniel P. Berrangé <berrange@redhat.com> In the transition to Post-Quantum Cryptography, it will often be desirable to load multiple sets of certificates, some with RSA/ECC and some with MLDSA. This extends the TLS context code to support the loading of many certs, passed as a NULL terminated array. Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- src/libvirt_probes.d | 3 ++- src/remote/remote_daemon.c | 6 +++-- src/rpc/virnettlscontext.c | 51 ++++++++++++++++++++---------------- src/rpc/virnettlscontext.h | 26 +++++++++--------- tests/virnettlscontexttest.c | 10 ++++--- tests/virnettlssessiontest.c | 9 ++++--- 6 files changed, 58 insertions(+), 47 deletions(-) diff --git a/src/libvirt_probes.d b/src/libvirt_probes.d index 6fac10a2bf..d9e75d9797 100644 --- a/src/libvirt_probes.d +++ b/src/libvirt_probes.d @@ -54,7 +54,8 @@ provider libvirt { # file: src/rpc/virnettlscontext.c # prefix: rpc probe rpc_tls_context_new(void *ctxt, const char *cacert, const char *cacrl, - const char *cert, const char *key, int sanityCheckCert, int requireValidCert, int isServer); + const char **cert, const char **keys, + int sanityCheckCert, int requireValidCert, int isServer); probe rpc_tls_context_dispose(void *ctxt); probe rpc_tls_context_session_allow(void *ctxt, void *sess, const char *dname); probe rpc_tls_context_session_deny(void *ctxt, void *sess, const char *dname); diff --git a/src/remote/remote_daemon.c b/src/remote/remote_daemon.c index 2973813548..e7c8f587c4 100644 --- a/src/remote/remote_daemon.c +++ b/src/remote/remote_daemon.c @@ -327,6 +327,9 @@ daemonSetupNetworking(virNetServer *srv, if (config->ca_file || config->cert_file || config->key_file) { + const char *certs[] = { config->cert_file, NULL }; + const char *keys[] = { config->key_file, NULL }; + if (!config->ca_file) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("No CA certificate path set to match server key/cert")); @@ -346,8 +349,7 @@ daemonSetupNetworking(virNetServer *srv, config->ca_file, config->cert_file, config->key_file); if (!(ctxt = virNetTLSContextNewServer(config->ca_file, config->crl_file, - config->cert_file, - config->key_file, + certs, keys, (const char *const*)config->tls_allowed_dn_list, config->tls_priority, config->tls_no_sanity_certificate ? false : true, diff --git a/src/rpc/virnettlscontext.c b/src/rpc/virnettlscontext.c index bb9db90dff..5e9c262b48 100644 --- a/src/rpc/virnettlscontext.c +++ b/src/rpc/virnettlscontext.c @@ -115,10 +115,11 @@ static int virNetTLSContextLoadCredentials(virNetTLSContext *ctxt, bool isServer, const char *cacert, const char *cacrl, - const char *cert, - const char *key) + const char *const *certs, + const char *const *keys) { int err; + size_t i; if (cacert && cacert[0] != '\0') { if (virNetTLSContextCheckCertFile("CA certificate", cacert, false) < 0) @@ -157,29 +158,29 @@ static int virNetTLSContextLoadCredentials(virNetTLSContext *ctxt, } } - if (cert && cert[0] != '\0' && key && key[0] != '\0') { + for (i = 0; certs[i] != NULL && keys[i] != NULL; i++) { int rv; - if ((rv = virNetTLSContextCheckCertFile("certificate", cert, !isServer)) < 0) + if ((rv = virNetTLSContextCheckCertFile("certificate", certs[i], !isServer)) < 0) return -1; if (rv == 0 && - (rv = virNetTLSContextCheckCertFile("private key", key, !isServer)) < 0) + (rv = virNetTLSContextCheckCertFile("private key", keys[i], !isServer)) < 0) return -1; if (rv == 0) { - VIR_DEBUG("loading cert and key from %s and %s", cert, key); + VIR_DEBUG("loading cert and key from %s and %s", certs[i], keys[i]); err = gnutls_certificate_set_x509_key_file(ctxt->x509cred, - cert, key, + certs[i], keys[i], GNUTLS_X509_FMT_PEM); if (err < 0) { virReportError(VIR_ERR_SYSTEM_ERROR, _("Unable to set x509 key and certificate: %1$s, %2$s: %3$s"), - key, cert, gnutls_strerror(err)); + keys[i], certs[i], gnutls_strerror(err)); return -1; } } else { VIR_DEBUG("Skipping non-existent cert %s key %s on client", - cert, key); + certs[i], keys[i]); } } @@ -189,8 +190,8 @@ static int virNetTLSContextLoadCredentials(virNetTLSContext *ctxt, static virNetTLSContext *virNetTLSContextNew(const char *cacert, const char *cacrl, - const char *cert, - const char *key, + const char *const *certs, + const char *const *keys, const char *const *x509dnACL, const char *priority, bool sanityCheckCert, @@ -199,7 +200,6 @@ static virNetTLSContext *virNetTLSContextNew(const char *cacert, { virNetTLSContext *ctxt; int err; - const char *certs[] = { cert, NULL }; if (virNetTLSContextInitialize() < 0) return NULL; @@ -228,7 +228,8 @@ static virNetTLSContext *virNetTLSContextNew(const char *cacert, virNetTLSCertSanityCheck(isServer, cacert, certs) < 0) goto error; - if (virNetTLSContextLoadCredentials(ctxt, isServer, cacert, cacrl, cert, key) < 0) + if (virNetTLSContextLoadCredentials(ctxt, isServer, cacert, cacrl, + certs, keys) < 0) goto error; ctxt->requireValidCert = requireValidCert; @@ -236,8 +237,8 @@ static virNetTLSContext *virNetTLSContextNew(const char *cacert, ctxt->isServer = isServer; PROBE(RPC_TLS_CONTEXT_NEW, - "ctxt=%p cacert=%s cacrl=%s cert=%s key=%s sanityCheckCert=%d requireValidCert=%d isServer=%d", - ctxt, cacert, NULLSTR(cacrl), cert, key, sanityCheckCert, requireValidCert, isServer); + "ctxt=%p cacert=%s cacrl=%s cert=%p key=%p sanityCheckCert=%d requireValidCert=%d isServer=%d", + ctxt, cacert, NULLSTR(cacrl), certs, keys, sanityCheckCert, requireValidCert, isServer); return ctxt; @@ -313,12 +314,14 @@ static virNetTLSContext *virNetTLSContextNewPath(const char *pkipath, g_autofree char *cacrl = NULL; g_autofree char *key = NULL; g_autofree char *cert = NULL; + const char *certs[] = { cert, NULL }; + const char *keys[] = { key, NULL }; if (virNetTLSContextLocateCredentials(pkipath, tryUserPkiPath, isServer, &cacert, &cacrl, &cert, &key) < 0) return NULL; - return virNetTLSContextNew(cacert, cacrl, cert, key, + return virNetTLSContextNew(cacert, cacrl, certs, keys, x509dnACL, priority, sanityCheckCert, requireValidCert, isServer); } @@ -347,14 +350,14 @@ virNetTLSContext *virNetTLSContextNewClientPath(const char *pkipath, virNetTLSContext *virNetTLSContextNewServer(const char *cacert, const char *cacrl, - const char *cert, - const char *key, + const char *const *certs, + const char *const *keys, const char *const *x509dnACL, const char *priority, bool sanityCheckCert, bool requireValidCert) { - return virNetTLSContextNew(cacert, cacrl, cert, key, x509dnACL, priority, + return virNetTLSContextNew(cacert, cacrl, certs, keys, x509dnACL, priority, sanityCheckCert, requireValidCert, true); } @@ -369,6 +372,7 @@ int virNetTLSContextReloadForServer(virNetTLSContext *ctxt, g_autofree char *cert = NULL; g_autofree char *key = NULL; const char *certs[] = { cert, NULL }; + const char *keys[] = { key, NULL }; x509credBak = g_steal_pointer(&ctxt->x509cred); @@ -387,7 +391,8 @@ int virNetTLSContextReloadForServer(virNetTLSContext *ctxt, if (virNetTLSCertSanityCheck(true, cacert, certs)) goto error; - if (virNetTLSContextLoadCredentials(ctxt, true, cacert, cacrl, cert, key)) + if (virNetTLSContextLoadCredentials(ctxt, true, cacert, cacrl, + certs, keys)) goto error; gnutls_certificate_free_credentials(x509credBak); @@ -404,13 +409,13 @@ int virNetTLSContextReloadForServer(virNetTLSContext *ctxt, virNetTLSContext *virNetTLSContextNewClient(const char *cacert, const char *cacrl, - const char *cert, - const char *key, + const char *const *certs, + const char *const *keys, const char *priority, bool sanityCheckCert, bool requireValidCert) { - return virNetTLSContextNew(cacert, cacrl, cert, key, NULL, priority, + return virNetTLSContextNew(cacert, cacrl, certs, keys, NULL, priority, sanityCheckCert, requireValidCert, false); } diff --git a/src/rpc/virnettlscontext.h b/src/rpc/virnettlscontext.h index 11c954ce4b..1e67171e3e 100644 --- a/src/rpc/virnettlscontext.h +++ b/src/rpc/virnettlscontext.h @@ -44,21 +44,21 @@ virNetTLSContext *virNetTLSContextNewClientPath(const char *pkipath, bool requireValidCert); virNetTLSContext *virNetTLSContextNewServer(const char *cacert, - const char *cacrl, - const char *cert, - const char *key, - const char *const *x509dnACL, - const char *priority, - bool sanityCheckCert, - bool requireValidCert); + const char *cacrl, + const char *const *certs, + const char *const *keys, + const char *const *x509dnACL, + const char *priority, + bool sanityCheckCert, + bool requireValidCert); virNetTLSContext *virNetTLSContextNewClient(const char *cacert, - const char *cacrl, - const char *cert, - const char *key, - const char *priority, - bool sanityCheckCert, - bool requireValidCert); + const char *cacrl, + const char *const *certs, + const char *const *keys, + const char *priority, + bool sanityCheckCert, + bool requireValidCert); int virNetTLSContextReloadForServer(virNetTLSContext *ctxt, bool tryUserPkiPath); diff --git a/tests/virnettlscontexttest.c b/tests/virnettlscontexttest.c index 48bdefdd76..47675bffd0 100644 --- a/tests/virnettlscontexttest.c +++ b/tests/virnettlscontexttest.c @@ -56,12 +56,14 @@ static int testTLSContextInit(const void *opaque) struct testTLSContextData *data = (struct testTLSContextData *)opaque; virNetTLSContext *ctxt = NULL; int ret = -1; + const char *certs[] = { data->crt, NULL }; + const char *keys[] = { KEYFILE, NULL }; if (data->isServer) { ctxt = virNetTLSContextNewServer(data->cacrt, NULL, - data->crt, - KEYFILE, + certs, + keys, NULL, "NORMAL", true, @@ -69,8 +71,8 @@ static int testTLSContextInit(const void *opaque) } else { ctxt = virNetTLSContextNewClient(data->cacrt, NULL, - data->crt, - KEYFILE, + certs, + keys, "NORMAL", true, true); diff --git a/tests/virnettlssessiontest.c b/tests/virnettlssessiontest.c index 459e17c52c..e8d64c7da0 100644 --- a/tests/virnettlssessiontest.c +++ b/tests/virnettlssessiontest.c @@ -81,6 +81,9 @@ static int testTLSSessionInit(const void *opaque) int channel[2]; bool clientShake = false; bool serverShake = false; + const char *keys[] = { KEYFILE, NULL }; + const char *clientcerts[] = { data->clientcrt, NULL }; + const char *servercerts[] = { data->servercrt, NULL }; /* We'll use this for our fake client-server connection */ @@ -102,8 +105,7 @@ static int testTLSSessionInit(const void *opaque) */ serverCtxt = virNetTLSContextNewServer(data->servercacrt, NULL, - data->servercrt, - KEYFILE, + servercerts, keys, data->wildcards, "NORMAL", false, @@ -111,8 +113,7 @@ static int testTLSSessionInit(const void *opaque) clientCtxt = virNetTLSContextNewClient(data->clientcacrt, NULL, - data->clientcrt, - KEYFILE, + clientcerts, keys, "NORMAL", false, true); -- 2.51.1
From: Daniel P. Berrangé <berrange@redhat.com> The 'cert_file' and 'key_file' parameters in libvirtd.conf only permit a single cert/key. To support hybrid deployments for PQC, we need to be able to request multiple certs/keys. This involves new 'cert_files' and 'key_files' config parameters that accept a list of filenames. The new parameters are mutually exclusive with the old parameters. Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- src/remote/libvirtd.aug.in | 2 ++ src/remote/libvirtd.conf.in | 16 ++++++++++++ src/remote/remote_daemon.c | 26 +++++++++---------- src/remote/remote_daemon_config.c | 43 +++++++++++++++++++++++++++---- src/remote/remote_daemon_config.h | 4 +-- src/remote/test_libvirtd.aug.in | 8 ++++++ 6 files changed, 79 insertions(+), 20 deletions(-) diff --git a/src/remote/libvirtd.aug.in b/src/remote/libvirtd.aug.in index d744548f41..1f3bb5d0e2 100644 --- a/src/remote/libvirtd.aug.in +++ b/src/remote/libvirtd.aug.in @@ -47,6 +47,8 @@ module @DAEMON_NAME_UC@ = let certificate_entry = str_entry "key_file" | str_entry "cert_file" + | str_array_entry "key_files" + | str_array_entry "cert_files" | str_entry "ca_file" | str_entry "crl_file" diff --git a/src/remote/libvirtd.conf.in b/src/remote/libvirtd.conf.in index 32a680317a..e4460e61ef 100644 --- a/src/remote/libvirtd.conf.in +++ b/src/remote/libvirtd.conf.in @@ -244,12 +244,28 @@ # Override the default server key file path # +# This parameter is mutually exclusive with 'key_files' +# #key_file = "@sysconfdir@/pki/libvirt/private/serverkey.pem" +# Override the default server key file path(s) +# +# This parameter is mutually exclusive with 'key_file' +# +#key_files = ["@sysconfdir@/pki/libvirt/private/serverkey-0.pem", "@sysconfdir@/pki/libvirt/private/serverkey-1.pem"] + # Override the default server certificate file path # +# This parameter is mutually exclusive with 'cert_files' +# #cert_file = "@sysconfdir@/pki/libvirt/servercert.pem" +# Override the default server certificate file path(s) +# +# This parameter is mutually exclusive with 'cert_file' +# +#cert_files = ["@sysconfdir@/pki/libvirt/servercert-0.pem", "@sysconfdir@/pki/libvirt/servercert-1.pem"] + # Override the default CA certificate path # #ca_file = "@sysconfdir@/pki/CA/cacert.pem" diff --git a/src/remote/remote_daemon.c b/src/remote/remote_daemon.c index e7c8f587c4..ee3d10bc23 100644 --- a/src/remote/remote_daemon.c +++ b/src/remote/remote_daemon.c @@ -325,31 +325,31 @@ daemonSetupNetworking(virNetServer *srv, virNetTLSContext *ctxt = NULL; if (config->ca_file || - config->cert_file || - config->key_file) { - const char *certs[] = { config->cert_file, NULL }; - const char *keys[] = { config->key_file, NULL }; - + config->cert_files || + config->key_files) { + g_autofree char *certs = g_strjoinv(", ", config->cert_files); + g_autofree char *keys = g_strjoinv(", ", config->key_files); if (!config->ca_file) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("No CA certificate path set to match server key/cert")); + _("No CA certificate path set to match server key(s)/cert(s)")); return -1; } - if (!config->cert_file) { + if (!config->cert_files) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("No server certificate path set to match server key")); + _("No server certificate path(s) set to match server key(s)")); return -1; } - if (!config->key_file) { + if (!config->key_files) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("No server key path set to match server cert")); + _("No server key path(s) set to match server cert(s)")); return -1; } - VIR_DEBUG("Using CA='%s' cert='%s' key='%s'", - config->ca_file, config->cert_file, config->key_file); + VIR_DEBUG("Using CA='%s' certs='%s' keys='%s'", + config->ca_file, certs, keys); if (!(ctxt = virNetTLSContextNewServer(config->ca_file, config->crl_file, - certs, keys, + (const char *const*)config->cert_files, + (const char *const*)config->key_files, (const char *const*)config->tls_allowed_dn_list, config->tls_priority, config->tls_no_sanity_certificate ? false : true, diff --git a/src/remote/remote_daemon_config.c b/src/remote/remote_daemon_config.c index c1e75444e1..bb6078967f 100644 --- a/src/remote/remote_daemon_config.c +++ b/src/remote/remote_daemon_config.c @@ -192,9 +192,9 @@ daemonConfigFree(struct daemonConfig *data) g_free(data->tls_priority); - g_free(data->key_file); + g_strfreev(data->key_files); g_free(data->ca_file); - g_free(data->cert_file); + g_strfreev(data->cert_files); g_free(data->crl_file); #endif /* ! WITH_IP */ @@ -212,8 +212,12 @@ daemonConfigLoadOptions(struct daemonConfig *data, virConf *conf) { int rc G_GNUC_UNUSED; - #ifdef WITH_IP + g_autofree char *cert_file = NULL; + g_autofree char *key_file = NULL; + size_t ncerts; + size_t nkeys; + if (virConfGetValueBool(conf, "listen_tcp", &data->listen_tcp) < 0) return -1; if (virConfGetValueBool(conf, "listen_tls", &data->listen_tls) < 0) @@ -269,10 +273,39 @@ daemonConfigLoadOptions(struct daemonConfig *data, if (virConfGetValueBool(conf, "tls_no_verify_certificate", &data->tls_no_verify_certificate) < 0) return -1; - if (virConfGetValueString(conf, "key_file", &data->key_file) < 0) + if (virConfGetValueString(conf, "key_file", &key_file) < 0) + return -1; + if (virConfGetValueString(conf, "cert_file", &cert_file) < 0) + return -1; + if (virConfGetValueStringList(conf, "key_files", false, &data->key_files) < 0) + return -1; + if (virConfGetValueStringList(conf, "cert_files", false, &data->cert_files) < 0) + return -1; + if ((cert_file && data->cert_files) || + (key_file && data->key_files)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("cert_file/key_file are mutually exclusive with cert_files/key_files")); return -1; - if (virConfGetValueString(conf, "cert_file", &data->cert_file) < 0) + } + if (cert_file) { + data->cert_files = g_new0(char *, 2); + data->cert_files[0] = g_steal_pointer(&cert_file); + data->cert_files[1] = NULL; + } + if (key_file) { + data->key_files = g_new0(char *, 2); + data->key_files[0] = g_steal_pointer(&key_file); + data->key_files[1] = NULL; + } + ncerts = data->cert_files ? g_strv_length(data->cert_files) : 0; + nkeys = data->key_files ? g_strv_length(data->key_files) : 0; + if (ncerts != nkeys) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Number of certificates (%1$zu) must match number of keys (%2$zu)"), + ncerts, nkeys); return -1; + } + if (virConfGetValueString(conf, "ca_file", &data->ca_file) < 0) return -1; if (virConfGetValueString(conf, "crl_file", &data->crl_file) < 0) diff --git a/src/remote/remote_daemon_config.h b/src/remote/remote_daemon_config.h index 9f9e54e838..e699889191 100644 --- a/src/remote/remote_daemon_config.h +++ b/src/remote/remote_daemon_config.h @@ -58,8 +58,8 @@ struct daemonConfig { char *tls_priority; unsigned int tcp_min_ssf; - char *key_file; - char *cert_file; + char **key_files; + char **cert_files; char *ca_file; char *crl_file; #endif /* ! WITH_IP */ diff --git a/src/remote/test_libvirtd.aug.in b/src/remote/test_libvirtd.aug.in index c27680e130..a37b0daa55 100644 --- a/src/remote/test_libvirtd.aug.in +++ b/src/remote/test_libvirtd.aug.in @@ -26,7 +26,15 @@ module Test_@DAEMON_NAME@ = } @CUT_ENABLE_IP@ { "key_file" = "@sysconfdir@/pki/libvirt/private/serverkey.pem" } + { "key_files" + { "1" = "@sysconfdir@/pki/libvirt/private/serverkey-0.pem" } + { "2" = "@sysconfdir@/pki/libvirt/private/serverkey-1.pem" } + } { "cert_file" = "@sysconfdir@/pki/libvirt/servercert.pem" } + { "cert_files" + { "1" = "@sysconfdir@/pki/libvirt/servercert-0.pem" } + { "2" = "@sysconfdir@/pki/libvirt/servercert-1.pem" } + } { "ca_file" = "@sysconfdir@/pki/CA/cacert.pem" } { "crl_file" = "@sysconfdir@/pki/CA/crl.pem" } { "tls_no_sanity_certificate" = "1" } -- 2.51.1
From: Daniel P. Berrangé <berrange@redhat.com> The virNetTLSConfigCustomCreds will always set the cert paths to non-NULL strings. This in turn means that the later call to virNetTLSConfigSystemCreds will be a no-op aside from duplicating log information. Refactor the conditions so that the call to find system credentials is skipped when using custom credentials. While this patch could have just done an early "return 0" after the virNetTLSConfigCustomCreds call, an "} else {" branch is instead added, since this will facilitate a later patch in this series which prefers a common return path. Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- src/rpc/virnettlscontext.c | 50 ++++++++++++++++++++------------------ 1 file changed, 26 insertions(+), 24 deletions(-) diff --git a/src/rpc/virnettlscontext.c b/src/rpc/virnettlscontext.c index 5e9c262b48..37f635f47f 100644 --- a/src/rpc/virnettlscontext.c +++ b/src/rpc/virnettlscontext.c @@ -271,32 +271,34 @@ static int virNetTLSContextLocateCredentials(const char *pkipath, virNetTLSConfigCustomCreds(pkipath, isServer, cacert, cacrl, cert, key); - } else if (tryUserPkiPath) { - virNetTLSConfigUserCreds(isServer, - cacert, cacrl, - cert, key); - - /* - * If some of the files can't be found, fallback - * to the global location for them - */ - if (!virFileExists(*cacert)) - VIR_FREE(*cacert); - if (!virFileExists(*cacrl)) - VIR_FREE(*cacrl); - - /* Check these as a pair, since it they are - * mutually dependent - */ - if (!virFileExists(*key) || !virFileExists(*cert)) { - VIR_FREE(*key); - VIR_FREE(*cert); + } else { + if (tryUserPkiPath) { + virNetTLSConfigUserCreds(isServer, + cacert, cacrl, + cert, key); + + /* + * If some of the files can't be found, fallback + * to the global location for them + */ + if (!virFileExists(*cacert)) + VIR_FREE(*cacert); + if (!virFileExists(*cacrl)) + VIR_FREE(*cacrl); + + /* Check these as a pair, since it they are + * mutually dependent + */ + if (!virFileExists(*key) || !virFileExists(*cert)) { + VIR_FREE(*key); + VIR_FREE(*cert); + } } - } - virNetTLSConfigSystemCreds(isServer, - cacert, cacrl, - cert, key); + virNetTLSConfigSystemCreds(isServer, + cacert, cacrl, + cert, key); + } return 0; } -- 2.51.1
From: Daniel P. Berrangé <berrange@redhat.com> A future patch will require fule access checks to be done as part of locating the certificate files, as we will have the ability to load many more files, most of which will be optional. Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- po/POTFILES | 1 + src/rpc/virnettlsconfig.c | 168 +++++++++++++++++++++++++++++++++---- src/rpc/virnettlsconfig.h | 37 ++++---- src/rpc/virnettlscontext.c | 161 ++++++++++++++--------------------- 4 files changed, 236 insertions(+), 131 deletions(-) diff --git a/po/POTFILES b/po/POTFILES index 23da794f84..f0aad35c8c 100644 --- a/po/POTFILES +++ b/po/POTFILES @@ -230,6 +230,7 @@ src/rpc/virnetserverservice.c src/rpc/virnetsocket.c src/rpc/virnetsshsession.c src/rpc/virnettlscert.c +src/rpc/virnettlsconfig.c src/rpc/virnettlscontext.c src/secret/secret_driver.c src/security/security_apparmor.c diff --git a/src/rpc/virnettlsconfig.c b/src/rpc/virnettlsconfig.c index ffab3b4fc8..1479eb01ae 100644 --- a/src/rpc/virnettlsconfig.c +++ b/src/rpc/virnettlsconfig.c @@ -21,12 +21,15 @@ #include <config.h> #include "virnettlsconfig.h" +#include "viralloc.h" #include "virlog.h" #include "virutil.h" +#include "virfile.h" +#include "virerror.h" #define VIR_FROM_THIS VIR_FROM_RPC -VIR_LOG_INIT("rpc.nettlscontext"); +VIR_LOG_INIT("rpc.nettlsconfig"); char *virNetTLSConfigUserPKIBaseDir(void) { @@ -142,30 +145,143 @@ void virNetTLSConfigSystemIdentity(bool isServer, key); } -void virNetTLSConfigCustomCreds(const char *pkipath, - bool isServer, - char **cacert, - char **cacrl, - char **cert, - char **key) + +int virNetTLSConfigCheckTrust(const char *cacert, const char *cacrl, + bool *cacertExists, bool *cacrlExists, + bool allowMissingCA) +{ + if (cacertExists) + *cacertExists = true; + if (cacrlExists) + *cacrlExists = true; + VIR_DEBUG("Checking CA certificate '%s' and CRL '%s'", cacert, NULLSTR(cacrl)); + if (!virFileExists(cacert)) { + if (allowMissingCA) { + VIR_DEBUG("CA certificate '%s' does not exist", cacert); + if (cacertExists) + *cacertExists = false; + } else { + virReportSystemError(errno, _("CA certificate '%1$s' does not exist"), + cacert); + return -1; + } + } + if (cacrl != NULL && !virFileExists(cacrl)) { + VIR_DEBUG("CA CRL '%s' does not exist", cacrl); + if (cacrlExists) + *cacrlExists = false; + } + return 0; +} + +static int virNetTLSConfigEnsureTrust(char **cacert, char **cacrl, + bool allowMissingCA) +{ + bool cacertExists, cacrlExists; + + if (virNetTLSConfigCheckTrust(*cacert, *cacrl, + &cacertExists, &cacrlExists, + allowMissingCA) < 0) + return -1; + + if (!cacertExists) + VIR_FREE(*cacert); + if (!cacrlExists) + VIR_FREE(*cacrl); + + return 0; +} + +int virNetTLSConfigCheckIdentity(const char *cert, const char *key, + bool *identityExists, bool allowMissing) +{ + if (identityExists) + *identityExists = true; + VIR_DEBUG("Checking certificate '%s' and key '%s'", cert, key); + if (!virFileExists(cert)) { + int saved_errno = errno; + if (allowMissing) { + if (virFileExists(key)) { + virReportSystemError( + saved_errno, + _("Certificate '%1$s' does not exist, but key '%2$s' does"), + cert, key); + return -1; + } + if (identityExists) + *identityExists = false; + VIR_DEBUG("Missing cert '%s' / key '%s'", cert, key); + return 0; + } else { + virReportSystemError(saved_errno, _("Certificate '%1$s' does not exist"), + cert); + return -1; + } + } else { + if (!virFileExists(key)) { + virReportSystemError(errno, + _("Key '%1$s' does not exist, but certificate '%2$s' does"), + key, cert); + return -1; + } + } + + return 0; +} + + +static int virNetTLSConfigEnsureIdentity(char **cert, char **key, + bool allowMissing) +{ + bool identityExists; + + if (virNetTLSConfigCheckIdentity(*cert, *key, &identityExists, + allowMissing) < 0) + return -1; + + if (!identityExists) { + VIR_FREE(*cert); + VIR_FREE(*key); + } + + return 0; +} + + +int virNetTLSConfigCustomCreds(const char *pkipath, + bool isServer, + char **cacert, + char **cacrl, + char **cert, + char **key) { VIR_DEBUG("Locating creds in custom dir %s", pkipath); virNetTLSConfigTrust(pkipath, pkipath, cacert, cacrl); + + if (virNetTLSConfigEnsureTrust(cacert, cacrl, false) < 0) + return -1; + virNetTLSConfigIdentity(isServer, pkipath, pkipath, cert, key); + + + if (virNetTLSConfigEnsureIdentity(cert, key, !isServer) < 0) + return -1; + + return 0; } -void virNetTLSConfigUserCreds(bool isServer, - char **cacert, - char **cacrl, - char **cert, - char **key) +int virNetTLSConfigUserCreds(bool isServer, + char **cacert, + char **cacrl, + char **cert, + char **key) { g_autofree char *pkipath = virNetTLSConfigUserPKIBaseDir(); @@ -175,18 +291,27 @@ void virNetTLSConfigUserCreds(bool isServer, pkipath, cacert, cacrl); + + if (virNetTLSConfigEnsureTrust(cacert, cacrl, true) < 0) + return -1; + virNetTLSConfigIdentity(isServer, pkipath, pkipath, cert, key); + + if (virNetTLSConfigEnsureIdentity(cert, key, true) < 0) + return -1; + + return 0; } -void virNetTLSConfigSystemCreds(bool isServer, - char **cacert, - char **cacrl, - char **cert, - char **key) +int virNetTLSConfigSystemCreds(bool isServer, + char **cacert, + char **cacrl, + char **cert, + char **key) { VIR_DEBUG("Locating creds in system dir %s", LIBVIRT_PKI_DIR); @@ -194,9 +319,18 @@ void virNetTLSConfigSystemCreds(bool isServer, LIBVIRT_CACRL_DIR, cacert, cacrl); + + if (virNetTLSConfigEnsureTrust(cacert, cacrl, false) < 0) + return -1; + virNetTLSConfigIdentity(isServer, LIBVIRT_CERT_DIR, LIBVIRT_KEY_DIR, cert, key); + + if (virNetTLSConfigEnsureIdentity(cert, key, !isServer) < 0) + return -1; + + return 0; } diff --git a/src/rpc/virnettlsconfig.h b/src/rpc/virnettlsconfig.h index a9378c18b7..9ad213fe06 100644 --- a/src/rpc/virnettlsconfig.h +++ b/src/rpc/virnettlsconfig.h @@ -50,20 +50,25 @@ void virNetTLSConfigSystemIdentity(bool isServer, char **cert, char **key); +int virNetTLSConfigCheckIdentity(const char *cert, const char *key, + bool *identityExists, bool allowMissing); +int virNetTLSConfigCheckTrust(const char *cacert, const char *cacrl, + bool *cacertExists, bool *cacrlExists, + bool allowMissingCA); -void virNetTLSConfigCustomCreds(const char *pkipath, - bool isServer, - char **cacert, - char **cacrl, - char **cert, - char **key); -void virNetTLSConfigUserCreds(bool isServer, - char **cacert, - char **cacrl, - char **cert, - char **key); -void virNetTLSConfigSystemCreds(bool isServer, - char **cacert, - char **cacrl, - char **cert, - char **key); +int virNetTLSConfigCustomCreds(const char *pkipath, + bool isServer, + char **cacert, + char **cacrl, + char **cert, + char **key); +int virNetTLSConfigUserCreds(bool isServer, + char **cacert, + char **cacrl, + char **cert, + char **key); +int virNetTLSConfigSystemCreds(bool isServer, + char **cacert, + char **cacrl, + char **cert, + char **key); diff --git a/src/rpc/virnettlscontext.c b/src/rpc/virnettlscontext.c index 37f635f47f..7061eb5953 100644 --- a/src/rpc/virnettlscontext.c +++ b/src/rpc/virnettlscontext.c @@ -31,7 +31,6 @@ #include "virnettlscert.h" #include "virstring.h" -#include "viralloc.h" #include "virerror.h" #include "virfile.h" #include "virutil.h" @@ -88,22 +87,6 @@ static int virNetTLSContextOnceInit(void) VIR_ONCE_GLOBAL_INIT(virNetTLSContext); -static int -virNetTLSContextCheckCertFile(const char *type, const char *file, bool allowMissing) -{ - if (!virFileExists(file)) { - if (allowMissing) - return 1; - - virReportSystemError(errno, - _("Cannot read %1$s '%2$s'"), - type, file); - return -1; - } - return 0; -} - - static void virNetTLSLog(int level G_GNUC_UNUSED, const char *str G_GNUC_UNUSED) { @@ -112,7 +95,6 @@ static void virNetTLSLog(int level G_GNUC_UNUSED, static int virNetTLSContextLoadCredentials(virNetTLSContext *ctxt, - bool isServer, const char *cacert, const char *cacrl, const char *const *certs, @@ -121,66 +103,42 @@ static int virNetTLSContextLoadCredentials(virNetTLSContext *ctxt, int err; size_t i; - if (cacert && cacert[0] != '\0') { - if (virNetTLSContextCheckCertFile("CA certificate", cacert, false) < 0) - return -1; + VIR_DEBUG("loading CA cert from %s", cacert); + err = gnutls_certificate_set_x509_trust_file(ctxt->x509cred, + cacert, + GNUTLS_X509_FMT_PEM); + if (err < 0) { + virReportError(VIR_ERR_SYSTEM_ERROR, + _("Unable to set x509 CA certificate: %1$s: %2$s"), + cacert, gnutls_strerror(err)); + return -1; + } - VIR_DEBUG("loading CA cert from %s", cacert); - err = gnutls_certificate_set_x509_trust_file(ctxt->x509cred, - cacert, - GNUTLS_X509_FMT_PEM); + if (cacrl) { + VIR_DEBUG("loading CRL from %s", cacrl); + err = gnutls_certificate_set_x509_crl_file(ctxt->x509cred, + cacrl, + GNUTLS_X509_FMT_PEM); if (err < 0) { virReportError(VIR_ERR_SYSTEM_ERROR, - _("Unable to set x509 CA certificate: %1$s: %2$s"), - cacert, gnutls_strerror(err)); - return -1; - } - } - - if (cacrl && cacrl[0] != '\0') { - int rv; - if ((rv = virNetTLSContextCheckCertFile("CA revocation list", cacrl, true)) < 0) + _("Unable to set x509 certificate revocation list: %1$s: %2$s"), + cacrl, gnutls_strerror(err)); return -1; - - if (rv == 0) { - VIR_DEBUG("loading CRL from %s", cacrl); - err = gnutls_certificate_set_x509_crl_file(ctxt->x509cred, - cacrl, - GNUTLS_X509_FMT_PEM); - if (err < 0) { - virReportError(VIR_ERR_SYSTEM_ERROR, - _("Unable to set x509 certificate revocation list: %1$s: %2$s"), - cacrl, gnutls_strerror(err)); - return -1; - } - } else { - VIR_DEBUG("Skipping non-existent CA CRL %s", cacrl); } + } else { + VIR_DEBUG("no CRL file to load"); } for (i = 0; certs[i] != NULL && keys[i] != NULL; i++) { - int rv; - if ((rv = virNetTLSContextCheckCertFile("certificate", certs[i], !isServer)) < 0) - return -1; - if (rv == 0 && - (rv = virNetTLSContextCheckCertFile("private key", keys[i], !isServer)) < 0) + VIR_DEBUG("loading cert and key from %s and %s", certs[i], keys[i]); + err = gnutls_certificate_set_x509_key_file(ctxt->x509cred, + certs[i], keys[i], + GNUTLS_X509_FMT_PEM); + if (err < 0) { + virReportError(VIR_ERR_SYSTEM_ERROR, + _("Unable to set x509 key and certificate: %1$s, %2$s: %3$s"), + keys[i], certs[i], gnutls_strerror(err)); return -1; - - if (rv == 0) { - VIR_DEBUG("loading cert and key from %s and %s", certs[i], keys[i]); - err = - gnutls_certificate_set_x509_key_file(ctxt->x509cred, - certs[i], keys[i], - GNUTLS_X509_FMT_PEM); - if (err < 0) { - virReportError(VIR_ERR_SYSTEM_ERROR, - _("Unable to set x509 key and certificate: %1$s, %2$s: %3$s"), - keys[i], certs[i], gnutls_strerror(err)); - return -1; - } - } else { - VIR_DEBUG("Skipping non-existent cert %s key %s on client", - certs[i], keys[i]); } } @@ -200,6 +158,15 @@ static virNetTLSContext *virNetTLSContextNew(const char *cacert, { virNetTLSContext *ctxt; int err; + g_autofree char *certlist = certs ? g_strjoinv(", ", (char **)certs) : NULL; + g_autofree char *keylist = keys ? g_strjoinv(", ", (char **)keys) : NULL; + g_autofree char *acllist = x509dnACL ? g_strjoinv(", ", (char **)x509dnACL) : NULL; + + VIR_DEBUG("CA cert=%s CRL=%s certs='%s' keys='%s' ACL='%s' " + "priority=%s sanity-check=%d require-valid=%d is-server=%d", + cacert, NULLSTR(cacrl), NULLSTR(certlist), NULLSTR(keylist), + NULLSTR(acllist), priority, sanityCheckCert, requireValidCert, + isServer); if (virNetTLSContextInitialize() < 0) return NULL; @@ -228,7 +195,7 @@ static virNetTLSContext *virNetTLSContextNew(const char *cacert, virNetTLSCertSanityCheck(isServer, cacert, certs) < 0) goto error; - if (virNetTLSContextLoadCredentials(ctxt, isServer, cacert, cacrl, + if (virNetTLSContextLoadCredentials(ctxt, cacert, cacrl, certs, keys) < 0) goto error; @@ -268,38 +235,22 @@ static int virNetTLSContextLocateCredentials(const char *pkipath, * files actually exist there */ if (pkipath) { - virNetTLSConfigCustomCreds(pkipath, isServer, - cacert, cacrl, - cert, key); + if (virNetTLSConfigCustomCreds(pkipath, isServer, + cacert, cacrl, + cert, key) < 0) + return -1; } else { - if (tryUserPkiPath) { + if (tryUserPkiPath && virNetTLSConfigUserCreds(isServer, cacert, cacrl, - cert, key); - - /* - * If some of the files can't be found, fallback - * to the global location for them - */ - if (!virFileExists(*cacert)) - VIR_FREE(*cacert); - if (!virFileExists(*cacrl)) - VIR_FREE(*cacrl); - - /* Check these as a pair, since it they are - * mutually dependent - */ - if (!virFileExists(*key) || !virFileExists(*cert)) { - VIR_FREE(*key); - VIR_FREE(*cert); - } - } + cert, key) < 0) + return -1; - virNetTLSConfigSystemCreds(isServer, - cacert, cacrl, - cert, key); + if (virNetTLSConfigSystemCreds(isServer, + cacert, cacrl, + cert, key) < 0) + return -1; } - return 0; } @@ -359,6 +310,13 @@ virNetTLSContext *virNetTLSContextNewServer(const char *cacert, bool sanityCheckCert, bool requireValidCert) { + size_t i; + if (virNetTLSConfigCheckTrust(cacert, cacrl, NULL, NULL, false) < 0) + return NULL; + for (i = 0; certs[i] != NULL && keys[i] != NULL; i++) { + if (virNetTLSConfigCheckIdentity(certs[i], keys[i], NULL, false) < 0) + return NULL; + } return virNetTLSContextNew(cacert, cacrl, certs, keys, x509dnACL, priority, sanityCheckCert, requireValidCert, true); } @@ -393,7 +351,7 @@ int virNetTLSContextReloadForServer(virNetTLSContext *ctxt, if (virNetTLSCertSanityCheck(true, cacert, certs)) goto error; - if (virNetTLSContextLoadCredentials(ctxt, true, cacert, cacrl, + if (virNetTLSContextLoadCredentials(ctxt, cacert, cacrl, certs, keys)) goto error; @@ -417,6 +375,13 @@ virNetTLSContext *virNetTLSContextNewClient(const char *cacert, bool sanityCheckCert, bool requireValidCert) { + size_t i; + if (virNetTLSConfigCheckTrust(cacert, cacrl, NULL, NULL, false) < 0) + return NULL; + for (i = 0; certs[i] != NULL && keys[i] != NULL; i++) { + if (virNetTLSConfigCheckIdentity(certs[i], keys[i], NULL, false) < 0) + return NULL; + } return virNetTLSContextNew(cacert, cacrl, certs, keys, NULL, priority, sanityCheckCert, requireValidCert, false); } -- 2.51.1
From: Daniel P. Berrangé <berrange@redhat.com> The three different APIs for locating credentials differ only in what directories they search and their policy for missing files. Their code can be collapsed onto a single helper method. This will greatly facilitate the subsequent patch that expands the logic to locate many certificate files. Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- src/rpc/virnettlsconfig.c | 103 ++++++++++++++++++++------------------ 1 file changed, 53 insertions(+), 50 deletions(-) diff --git a/src/rpc/virnettlsconfig.c b/src/rpc/virnettlsconfig.c index 1479eb01ae..59cb8c2566 100644 --- a/src/rpc/virnettlsconfig.c +++ b/src/rpc/virnettlsconfig.c @@ -248,35 +248,58 @@ static int virNetTLSConfigEnsureIdentity(char **cert, char **key, } -int virNetTLSConfigCustomCreds(const char *pkipath, - bool isServer, - char **cacert, - char **cacrl, - char **cert, - char **key) +static int virNetTLSConfigCreds(const char *cacertdir, + const char *cacrldir, + const char *certdir, + const char *keydir, + bool isServer, + bool allowMissingCA, + bool allowMissingIdentity, + char **cacert, + char **cacrl, + char **cert, + char **key) { - VIR_DEBUG("Locating creds in custom dir %s", pkipath); - virNetTLSConfigTrust(pkipath, - pkipath, + virNetTLSConfigTrust(cacertdir, + cacrldir, cacert, cacrl); - if (virNetTLSConfigEnsureTrust(cacert, cacrl, false) < 0) + if (virNetTLSConfigEnsureTrust(cacert, cacrl, allowMissingCA) < 0) return -1; virNetTLSConfigIdentity(isServer, - pkipath, - pkipath, + certdir, + keydir, cert, key); - - if (virNetTLSConfigEnsureIdentity(cert, key, !isServer) < 0) + if (virNetTLSConfigEnsureIdentity(cert, key, allowMissingIdentity) < 0) return -1; return 0; } + +int virNetTLSConfigCustomCreds(const char *pkipath, + bool isServer, + char **cacert, + char **cacrl, + char **cert, + char **key) +{ + VIR_DEBUG("Locating creds in custom dir %s", pkipath); + + return virNetTLSConfigCreds(pkipath, pkipath, + pkipath, pkipath, + isServer, + false, + !isServer, + cacert, cacrl, + cert, key); +} + + int virNetTLSConfigUserCreds(bool isServer, char **cacert, char **cacrl, @@ -287,24 +310,13 @@ int virNetTLSConfigUserCreds(bool isServer, VIR_DEBUG("Locating creds in user dir %s", pkipath); - virNetTLSConfigTrust(pkipath, - pkipath, - cacert, - cacrl); - - if (virNetTLSConfigEnsureTrust(cacert, cacrl, true) < 0) - return -1; - - virNetTLSConfigIdentity(isServer, - pkipath, - pkipath, - cert, - key); - - if (virNetTLSConfigEnsureIdentity(cert, key, true) < 0) - return -1; - - return 0; + return virNetTLSConfigCreds(pkipath, pkipath, + pkipath, pkipath, + isServer, + true, + true, + cacert, cacrl, + cert, key); } int virNetTLSConfigSystemCreds(bool isServer, @@ -315,22 +327,13 @@ int virNetTLSConfigSystemCreds(bool isServer, { VIR_DEBUG("Locating creds in system dir %s", LIBVIRT_PKI_DIR); - virNetTLSConfigTrust(LIBVIRT_CACERT_DIR, - LIBVIRT_CACRL_DIR, - cacert, - cacrl); - - if (virNetTLSConfigEnsureTrust(cacert, cacrl, false) < 0) - return -1; - - virNetTLSConfigIdentity(isServer, - LIBVIRT_CERT_DIR, - LIBVIRT_KEY_DIR, - cert, - key); - - if (virNetTLSConfigEnsureIdentity(cert, key, !isServer) < 0) - return -1; - - return 0; + return virNetTLSConfigCreds(LIBVIRT_CACERT_DIR, + LIBVIRT_CACRL_DIR, + LIBVIRT_CERT_DIR, + LIBVIRT_KEY_DIR, + isServer, + false, + !isServer, + cacert, cacrl, + cert, key); } -- 2.51.1
From: Daniel P. Berrangé <berrange@redhat.com> In addition to servercert.pem / serverkey.pem, we now also support loading servercert{N}.pem / serverkey{N}.pem, for values of {N} between 0 and 3 inclusive. If servercert0.pem is provided, then using servercert.pem becomes optional. The first missing index terminates the loading process. eg if servercert1.pem is NOT present, then it will NOT attempt to look for servercert2.pem / servercert3.pem. This also applies to clientcert.pem / clientkey.pem. This facilitates the transition to post-quantum cryptography by allowing loading of certificates with different algorithms, eg traditional RSA based cert, and optional ECC based cert or MLDSA based cert for PQC. The use of CA cert files is unchanged with only a single cacert.pem loaded. WHen multiple CAs are needed they must be concatenated in the single cacert.pem file. Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- src/rpc/virnettlsconfig.c | 111 +++++++++++++++++++++++++++++++------ src/rpc/virnettlsconfig.h | 12 ++-- src/rpc/virnettlscontext.c | 51 ++++++++++------- 3 files changed, 130 insertions(+), 44 deletions(-) diff --git a/src/rpc/virnettlsconfig.c b/src/rpc/virnettlsconfig.c index 59cb8c2566..eec20cf6b7 100644 --- a/src/rpc/virnettlsconfig.c +++ b/src/rpc/virnettlsconfig.c @@ -31,6 +31,9 @@ VIR_LOG_INIT("rpc.nettlsconfig"); + +#define VIR_NET_TLS_CONFIG_MAX_INDEXED 4 + char *virNetTLSConfigUserPKIBaseDir(void) { g_autofree char *userdir = virGetUserDirectory(); @@ -69,6 +72,24 @@ static void virNetTLSConfigIdentity(bool isServer, VIR_DEBUG("TLS cert %s", *cert); } +static void virNetTLSConfigIdentityIndexed(bool isServer, + const char *certdir, + const char *keydir, + size_t idx, + char **cert, + char **key) +{ + if (!*key) + *key = g_strdup_printf("%s/%skey%zu.pem", keydir, + (isServer ? "server" : "client"), idx); + if (!*cert) + *cert = g_strdup_printf("%s/%scert%zu.pem", certdir, + (isServer ? "server" : "client"), idx); + + VIR_DEBUG("TLS key %s", *key); + VIR_DEBUG("TLS cert %s", *cert); +} + void virNetTLSConfigCustomTrust(const char *pkipath, char **cacert, char **cacrl) @@ -257,8 +278,8 @@ static int virNetTLSConfigCreds(const char *cacertdir, bool allowMissingIdentity, char **cacert, char **cacrl, - char **cert, - char **key) + char ***certs, + char ***keys) { virNetTLSConfigTrust(cacertdir, cacrldir, @@ -268,14 +289,68 @@ static int virNetTLSConfigCreds(const char *cacertdir, if (virNetTLSConfigEnsureTrust(cacert, cacrl, allowMissingCA) < 0) return -1; - virNetTLSConfigIdentity(isServer, - certdir, - keydir, - cert, - key); + if (!*certs && !*keys) { + g_auto(GStrv) certlist = + g_new0(char *, VIR_NET_TLS_CONFIG_MAX_INDEXED + 2); + g_auto(GStrv) keylist = + g_new0(char *, VIR_NET_TLS_CONFIG_MAX_INDEXED + 2); + size_t i; + + /* + * When searching for indexed certs/keys, the first + * missing index terminates the search, so we don't + * get gaps in the returned array. All indexed files + * are optional, as if they're all missing, we'll + * still honour the traditional file names + */ + for (i = 0; i < VIR_NET_TLS_CONFIG_MAX_INDEXED; i++) { + virNetTLSConfigIdentityIndexed(isServer, + certdir, + keydir, + i, + &(certlist[i + 1]), + &(keylist[i + 1])); + + if (virNetTLSConfigEnsureIdentity(&(certlist[i + 1]), + &(keylist[i + 1]), true) < 0) + return -1; - if (virNetTLSConfigEnsureIdentity(cert, key, allowMissingIdentity) < 0) - return -1; + if (certlist[i + 1] == NULL) { + break; + } + } + + /* + * If we find index 0, then allow the traditional + * default files to be optional + */ + if (certlist[1] != NULL) + allowMissingIdentity = true; + + virNetTLSConfigIdentity(isServer, + certdir, + keydir, + &(certlist[0]), + &(keylist[0])); + + if (virNetTLSConfigEnsureIdentity(&(certlist[0]), + &(keylist[0]), allowMissingIdentity) < 0) + return -1; + + if (certlist[0] == NULL) { + memmove(certlist, certlist + 1, + VIR_NET_TLS_CONFIG_MAX_INDEXED * sizeof(char *)); + memmove(keylist, keylist + 1, + VIR_NET_TLS_CONFIG_MAX_INDEXED * sizeof(char *)); + certlist[VIR_NET_TLS_CONFIG_MAX_INDEXED] = NULL; + keylist[VIR_NET_TLS_CONFIG_MAX_INDEXED] = NULL; + } + + if (certlist[0] != NULL) { + *certs = g_steal_pointer(&certlist); + *keys = g_steal_pointer(&keylist); + } + } return 0; } @@ -285,8 +360,8 @@ int virNetTLSConfigCustomCreds(const char *pkipath, bool isServer, char **cacert, char **cacrl, - char **cert, - char **key) + char ***certs, + char ***keys) { VIR_DEBUG("Locating creds in custom dir %s", pkipath); @@ -296,15 +371,15 @@ int virNetTLSConfigCustomCreds(const char *pkipath, false, !isServer, cacert, cacrl, - cert, key); + certs, keys); } int virNetTLSConfigUserCreds(bool isServer, char **cacert, char **cacrl, - char **cert, - char **key) + char ***certs, + char ***keys) { g_autofree char *pkipath = virNetTLSConfigUserPKIBaseDir(); @@ -316,14 +391,14 @@ int virNetTLSConfigUserCreds(bool isServer, true, true, cacert, cacrl, - cert, key); + certs, keys); } int virNetTLSConfigSystemCreds(bool isServer, char **cacert, char **cacrl, - char **cert, - char **key) + char ***certs, + char ***keys) { VIR_DEBUG("Locating creds in system dir %s", LIBVIRT_PKI_DIR); @@ -335,5 +410,5 @@ int virNetTLSConfigSystemCreds(bool isServer, false, !isServer, cacert, cacrl, - cert, key); + certs, keys); } diff --git a/src/rpc/virnettlsconfig.h b/src/rpc/virnettlsconfig.h index 9ad213fe06..c0bf604950 100644 --- a/src/rpc/virnettlsconfig.h +++ b/src/rpc/virnettlsconfig.h @@ -60,15 +60,15 @@ int virNetTLSConfigCustomCreds(const char *pkipath, bool isServer, char **cacert, char **cacrl, - char **cert, - char **key); + char ***certs, + char ***keys); int virNetTLSConfigUserCreds(bool isServer, char **cacert, char **cacrl, - char **cert, - char **key); + char ***certs, + char ***keys); int virNetTLSConfigSystemCreds(bool isServer, char **cacert, char **cacrl, - char **cert, - char **key); + char ***certs, + char ***keys); diff --git a/src/rpc/virnettlscontext.c b/src/rpc/virnettlscontext.c index 7061eb5953..dfe793e5db 100644 --- a/src/rpc/virnettlscontext.c +++ b/src/rpc/virnettlscontext.c @@ -220,13 +220,13 @@ static int virNetTLSContextLocateCredentials(const char *pkipath, bool isServer, char **cacert, char **cacrl, - char **cert, - char **key) + char ***certs, + char ***keys) { *cacert = NULL; *cacrl = NULL; - *key = NULL; - *cert = NULL; + *keys = NULL; + *certs = NULL; VIR_DEBUG("pkipath=%s isServer=%d tryUserPkiPath=%d", pkipath, isServer, tryUserPkiPath); @@ -237,20 +237,31 @@ static int virNetTLSContextLocateCredentials(const char *pkipath, if (pkipath) { if (virNetTLSConfigCustomCreds(pkipath, isServer, cacert, cacrl, - cert, key) < 0) + certs, keys) < 0) return -1; } else { if (tryUserPkiPath && virNetTLSConfigUserCreds(isServer, cacert, cacrl, - cert, key) < 0) + certs, keys) < 0) return -1; if (virNetTLSConfigSystemCreds(isServer, cacert, cacrl, - cert, key) < 0) + certs, keys) < 0) return -1; } + + /* + * Ensure the cert list is always non-NULL, even + * if it is an empty list, so that callers don't + * need to have repeated checks for a NULL array. + */ + if (*certs == NULL) + *certs = g_new0(char *, 1); + if (*keys == NULL) + *keys = g_new0(char *, 1); + return 0; } @@ -265,16 +276,16 @@ static virNetTLSContext *virNetTLSContextNewPath(const char *pkipath, { g_autofree char *cacert = NULL; g_autofree char *cacrl = NULL; - g_autofree char *key = NULL; - g_autofree char *cert = NULL; - const char *certs[] = { cert, NULL }; - const char *keys[] = { key, NULL }; + g_auto(GStrv) keys = NULL; + g_auto(GStrv) certs = NULL; if (virNetTLSContextLocateCredentials(pkipath, tryUserPkiPath, isServer, - &cacert, &cacrl, &cert, &key) < 0) + &cacert, &cacrl, &certs, &keys) < 0) return NULL; - return virNetTLSContextNew(cacert, cacrl, certs, keys, + return virNetTLSContextNew(cacert, cacrl, + (const char *const *)certs, + (const char *const *)keys, x509dnACL, priority, sanityCheckCert, requireValidCert, isServer); } @@ -329,15 +340,13 @@ int virNetTLSContextReloadForServer(virNetTLSContext *ctxt, int err; g_autofree char *cacert = NULL; g_autofree char *cacrl = NULL; - g_autofree char *cert = NULL; - g_autofree char *key = NULL; - const char *certs[] = { cert, NULL }; - const char *keys[] = { key, NULL }; + g_auto(GStrv) certs = NULL; + g_auto(GStrv) keys = NULL; x509credBak = g_steal_pointer(&ctxt->x509cred); if (virNetTLSContextLocateCredentials(NULL, tryUserPkiPath, true, - &cacert, &cacrl, &cert, &key)) + &cacert, &cacrl, &certs, &keys)) goto error; err = gnutls_certificate_allocate_credentials(&ctxt->x509cred); @@ -348,11 +357,13 @@ int virNetTLSContextReloadForServer(virNetTLSContext *ctxt, goto error; } - if (virNetTLSCertSanityCheck(true, cacert, certs)) + if (virNetTLSCertSanityCheck(true, cacert, + (const char *const *)certs)) goto error; if (virNetTLSContextLoadCredentials(ctxt, cacert, cacrl, - certs, keys)) + (const char *const *)certs, + (const char *const *)keys)) goto error; gnutls_certificate_free_credentials(x509credBak); -- 2.51.1
From: Daniel P. Berrangé <berrange@redhat.com> This describes the new index based certificate naming scheme, and how to create & deploy certificates for post-quantum cryptography. Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- docs/kbase/tlscerts.rst | 88 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 88 insertions(+) diff --git a/docs/kbase/tlscerts.rst b/docs/kbase/tlscerts.rst index c10ab11b7f..db962d0f46 100644 --- a/docs/kbase/tlscerts.rst +++ b/docs/kbase/tlscerts.rst @@ -75,6 +75,25 @@ in the path specified, otherwise the connection will fail with a fatal error. If - For the root user, the global default locations will always be used. +Multiple parallel certificate identities +---------------------------------------- + +Any scenario that requires a certificate identify (``servercert.pem`` / +``serverkey.pem`` and ``clientcert.pem`` / ``clientkey.pem``) can optionally +provide multiple parallel identities via a new indexed file naming +scheme. The new filenames are ``servercertNN.pem`` / ``serverkeyNN.pem`` +and ``clientcertNN.pem`` / ``clientkeyNN.pem``, for values of ``NN`` between +0 and 3 inclusive. + +The new naming can be used instead of the old naming, or concurrently +with the old naming. The old file names will be loaded first (if +present), followed by the indexed file names. Loading will stop at +the first missing index value. ie if ``servercert1.pem`` is not present, +then no attempt will be made to load ``servercert2.pem`` or ``servercert3.pem``. + +If multiple CA certificates are required they must all be concatenated +into the single ``cacert.pem`` file. + Background to TLS certificates ------------------------------ @@ -326,6 +345,75 @@ briefly cover the steps. cp clientkey.pem /etc/pki/libvirt/private/clientkey.pem cp clientcert.pem /etc/pki/libvirt/clientcert.pem +Configuring for Post-Quantum Cryptography +----------------------------------------- + +Given a new enough gnutls release, suitably integrated & configured with the +operating system crypto policies, libvirt is able to support post-quantum +crytography on TLS enabled services, either exclusively or in a hybrid mode. + +In exclusive mode, only a single set of certificates need to be configured +for libvirt, with PQC compliant algorithms. Such a libvirt configuration will +only be able to interoperate with other libvirt daemons that also have PQC +enabled. This can result in compatibility concerns during the period of +transition over to PQC compliant algorithms. + +In hybrid mode, multiple sets of certificates need to be configured for libvirt, +at least one set with traditional (non-PQC compliant) algorithms, and at least +one other set with modern (PQC compliant) algorithms. At time of the TLS +handshake, the GNUTLS algorithm priorities should ensure that PQC compliant +algorithms are negotiated if both sides of the connection support PQC. If one +side lacks PQC, the TLS handshake should fallback to the non-PQC algorithms. +This can assist with interoperability during the transition to PQC, but has a +potential weakness wrt downgrade attacks forcing use of non-PQC algorithms. +Exclusive PQC mode should be preferred where both peers in the TLS connections +are known to support PQC. + +Key generation parameters +^^^^^^^^^^^^^^^^^^^^^^^^^ + +To create certificates with PQC compliant algorithms, the ``--key-type`` +argument must be passed to ``certtool`` when creating private keys. No +extra arguments are required for the other ``certtool`` commands, as +their behaviour will be determined by the private key type. + +The typical PQC compliant algorithms to use are ``ML-DSA-44``, ``ML-DSA-65`` +and ``ML-DSA-87``, with ``ML-DSA-65`` being a suitable default choice in +the absence of explicit requirements. + +Taking the example earlier, for creating a key for a client certificate, +to use ``ML-DSA-65`` the command line would be modified to look like:: + + # certtool --generate-privkey --key-type=mldsa65 > clientkey.pem + +The equivalent modification applies to the creation of the private keys +used for server certs, or root/intermediate CA certs. + +For hybrid mode, the additional indexed certificate naming must be used. +If multiple configured certificates are compatible with the mutually +supported crypto algorithms between the client and server, then the +first matching certificate will be used. + +IOW, to ensure that PQC certificates are preferred, they must use a +non-index based filename, or use an index that is smaller than any +non-PQC certificates. ie, ``servercert.pem`` for PQC and ``servercert0.pem`` +for non-PQC, or ``servercert0.pem`` for PQC and ``servercert1.pem`` for +non-PQC. + +Force disabling PQC via crypto priority +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +If the OS configuration for system crypto algorithm priorities has +enabled PQC, this can (optionally) be overriden in libvirt server +configuration. To disable use of PQC set the ``tls_priority`` +parameter in the ``libvirtd.conf`` / ``virtproxyd.conf`` files: + + tls_priority = "@SYSTEM:-SIGN-ML-DSA-65:-SIGN-ML-DSA-44:-SIGN-ML-DSA-87:-GROUP-X25519-MLKEM768:-GROUP-SECP256R1-MLKEM768:-GROUP-SECP384R1-MLKEM1024" + +On the client side this can be overriden using the ``tls_priority`` +URI parameter in the libvirt connection address. + + Troubleshooting TLS certificate problems ---------------------------------------- -- 2.51.1
On 11/6/25 15:50, Daniel P. Berrangé via Devel wrote:
This series adds support for multiple certificate identities. This is intended to aid in the transition to post-quantum cryptography by allowing use of certs with RSA in parallel with certs using MLDSA algorithms.
Daniel P. Berrangé (10): remote: use g_strfreev for free()ing lists of strings rpc: change 'isServer' parameter from 'int' to 'bool' rpc: refactor TLS sanity checking to support many cert files rpc: add support for loading multiple certs & keys remote: support specifying multiple keys/certs in libvirtd.conf rpc: skip fallback when using custom PKI path rpc: move file access checks into TLS config API rpc: reduce duplication when locating credentials rpc: support loading multiple certificate identities docs: describe support for multiple certs & PQC config
docs/kbase/tlscerts.rst | 88 +++++++++ po/POTFILES | 1 + src/libvirt_probes.d | 3 +- src/remote/libvirtd.aug.in | 2 + src/remote/libvirtd.conf.in | 16 ++ src/remote/remote_daemon.c | 24 +-- src/remote/remote_daemon_config.c | 66 ++++--- src/remote/remote_daemon_config.h | 4 +- src/remote/test_libvirtd.aug.in | 8 + src/rpc/virnettlscert.c | 35 ++-- src/rpc/virnettlscert.h | 2 +- src/rpc/virnettlsconfig.c | 302 +++++++++++++++++++++++++----- src/rpc/virnettlsconfig.h | 44 +++-- src/rpc/virnettlscontext.c | 231 +++++++++++------------ src/rpc/virnettlscontext.h | 26 +-- tests/virnettlscontexttest.c | 10 +- tests/virnettlssessiontest.c | 9 +- tools/virt-pki-validate.c | 3 +- 18 files changed, 612 insertions(+), 262 deletions(-)
Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Michal
participants (2)
-
Daniel P. Berrangé -
Michal Prívozník