From: "Daniel P. Berrangé" <berrange@redhat.com>
To: qemu-devel@nongnu.org
Cc: "Marc-André Lureau" <marcandre.lureau@redhat.com>,
"Daniel P. Berrangé" <berrange@redhat.com>,
devel@lists.libvirt.org
Subject: [PATCH 18/21] crypto: avoid loading the identity certs twice
Date: Thu, 30 Oct 2025 14:49:24 +0000 [thread overview]
Message-ID: <20251030144927.2241109-19-berrange@redhat.com> (raw)
In-Reply-To: <20251030144927.2241109-1-berrange@redhat.com>
The x509 TLS credentials code will load the identity certs once to
perform sanity chcking on the certs, then discard the certificate
objects and let gnutls load them a second time.
This extends the previous QCryptoTLSCredsX509Files struct to also
hold the identity certificates & key loaded for sanity checking
and pass them on to gnutls, avoiding the duplicated loading.
The unit tests need updating because we now correctly diagnose the
error scenario where the cert PEM file exists, without its matching
key PEM file. Previously that error was mistakenly ignored.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
crypto/tlscredsx509.c | 247 +++++++++++++++++---------
tests/unit/test-crypto-tlscredsx509.c | 8 +-
2 files changed, 164 insertions(+), 91 deletions(-)
diff --git a/crypto/tlscredsx509.c b/crypto/tlscredsx509.c
index 6a830af50d..3cb0a6c31f 100644
--- a/crypto/tlscredsx509.c
+++ b/crypto/tlscredsx509.c
@@ -45,6 +45,12 @@ struct QCryptoTLSCredsX509Files {
char *cacertpath;
gnutls_x509_crt_t *cacerts;
unsigned int ncacerts;
+
+ char *certpath;
+ char *keypath;
+ gnutls_x509_crt_t *certs;
+ unsigned int ncerts;
+ gnutls_x509_privkey_t key;
};
static QCryptoTLSCredsX509Files *
@@ -63,6 +69,13 @@ qcrypto_tls_creds_x509_files_free(QCryptoTLSCredsX509Files *files)
}
g_free(files->cacerts);
g_free(files->cacertpath);
+ for (i = 0; i < files->ncerts; i++) {
+ gnutls_x509_crt_deinit(files->certs[i]);
+ }
+ gnutls_x509_privkey_deinit(files->key);
+ g_free(files->certs);
+ g_free(files->certpath);
+ g_free(files->keypath);
g_free(files);
}
@@ -477,14 +490,13 @@ qcrypto_tls_creds_load_cert_list(QCryptoTLSCredsX509 *creds,
const char *certFile,
gnutls_x509_crt_t **certs,
unsigned int *ncerts,
- bool isServer,
- bool isCA,
Error **errp)
{
gnutls_datum_t data;
g_autofree char *buf = NULL;
gsize buflen;
GError *gerr = NULL;
+ int ret;
*ncerts = 0;
trace_qcrypto_tls_creds_x509_load_cert_list(creds, certFile);
@@ -499,13 +511,60 @@ qcrypto_tls_creds_load_cert_list(QCryptoTLSCredsX509 *creds,
data.data = (unsigned char *)buf;
data.size = strlen(buf);
- if (gnutls_x509_crt_list_import2(certs, ncerts, &data,
- GNUTLS_X509_FMT_PEM, 0) < 0) {
- error_setg(errp,
- isCA ? "Unable to import CA certificate list %s" :
- (isServer ? "Unable to import server certificate %s" :
- "Unable to import client certificate %s"),
- certFile);
+ ret = gnutls_x509_crt_list_import2(certs, ncerts, &data,
+ GNUTLS_X509_FMT_PEM, 0);
+ if (ret < 0) {
+ error_setg(errp, "Unable to import certificate %s: %s",
+ certFile, gnutls_strerror(ret));
+ return -1;
+ }
+
+ return 0;
+}
+
+
+static int
+qcrypto_tls_creds_load_privkey(QCryptoTLSCredsX509 *creds,
+ const char *keyFile,
+ gnutls_x509_privkey_t *key,
+ Error **errp)
+{
+ gnutls_datum_t data;
+ g_autofree char *buf = NULL;
+ g_autofree char *password = NULL;
+ gsize buflen;
+ GError *gerr = NULL;
+ int ret;
+
+ ret = gnutls_x509_privkey_init(key);
+ if (ret < 0) {
+ error_setg(errp, "Unable to initialize private key: %s",
+ gnutls_strerror(ret));
+ return -1;
+ }
+
+ if (!g_file_get_contents(keyFile, &buf, &buflen, &gerr)) {
+ error_setg(errp, "Cannot load private key %s: %s",
+ keyFile, gerr->message);
+ g_error_free(gerr);
+ return -1;
+ }
+
+ data.data = (unsigned char *)buf;
+ data.size = strlen(buf);
+
+ if (creds->passwordid) {
+ password = qcrypto_secret_lookup_as_utf8(creds->passwordid,
+ errp);
+ if (!password) {
+ return -1;
+ }
+ }
+
+ if (gnutls_x509_privkey_import2(*key, &data,
+ GNUTLS_X509_FMT_PEM,
+ password, 0) < 0) {
+ error_setg(errp, "Unable to import private key %s", keyFile);
return -1;
}
@@ -517,56 +576,34 @@ static int
qcrypto_tls_creds_x509_sanity_check(QCryptoTLSCredsX509 *creds,
QCryptoTLSCredsX509Files *files,
bool isServer,
- const char *certFile,
Error **errp)
{
- gnutls_x509_crt_t *certs = NULL;
- unsigned int ncerts = 0;
size_t i;
- int ret = -1;
-
- if (certFile) {
- if (qcrypto_tls_creds_load_cert_list(creds,
- certFile,
- &certs,
- &ncerts,
- isServer,
- false,
- errp) < 0) {
- goto cleanup;
- }
- }
- for (i = 0; i < ncerts; i++) {
+ for (i = 0; i < files->ncerts; i++) {
if (qcrypto_tls_creds_check_cert(creds,
- certs[i], certFile,
+ files->certs[i], files->certpath,
isServer, i != 0, errp) < 0) {
- goto cleanup;
+ return -1;
}
}
- if (ncerts &&
+ if (files->ncerts &&
qcrypto_tls_creds_check_authority_chain(creds, files,
- certs, ncerts,
+ files->certs, files->ncerts,
isServer, errp) < 0) {
- goto cleanup;
- }
-
- if (ncerts &&
- qcrypto_tls_creds_check_cert_pair(files, certs, ncerts, certFile,
- isServer, errp) < 0) {
- goto cleanup;
+ return -1;
}
- ret = 0;
-
- cleanup:
- for (i = 0; i < ncerts; i++) {
- gnutls_x509_crt_deinit(certs[i]);
+ if (files->ncerts &&
+ qcrypto_tls_creds_check_cert_pair(files,
+ files->certs, files->ncerts,
+ files->certpath, isServer,
+ errp) < 0) {
+ return -1;
}
- g_free(certs);
- return ret;
+ return 0;
}
@@ -589,8 +626,6 @@ qcrypto_tls_creds_x509_load_ca(QCryptoTLSCredsX509 *creds,
files->cacertpath,
&files->cacerts,
&files->ncacerts,
- isServer,
- true,
errp) < 0) {
return -1;
}
@@ -606,6 +641,79 @@ qcrypto_tls_creds_x509_load_ca(QCryptoTLSCredsX509 *creds,
return 0;
}
+
+static int
+qcrypto_tls_creds_x509_load_identity(QCryptoTLSCredsX509 *creds,
+ QCryptoTLSCredsBox *box,
+ QCryptoTLSCredsX509Files *files,
+ bool isServer,
+ Error **errp)
+{
+ int ret;
+
+ if (isServer) {
+ if (qcrypto_tls_creds_get_path(&creds->parent_obj,
+ QCRYPTO_TLS_CREDS_X509_SERVER_CERT,
+ true, &files->certpath, errp) < 0 ||
+ qcrypto_tls_creds_get_path(&creds->parent_obj,
+ QCRYPTO_TLS_CREDS_X509_SERVER_KEY,
+ true, &files->keypath, errp) < 0) {
+ return -1;
+ }
+ } else {
+ if (qcrypto_tls_creds_get_path(&creds->parent_obj,
+ QCRYPTO_TLS_CREDS_X509_CLIENT_CERT,
+ false, &files->certpath, errp) < 0 ||
+ qcrypto_tls_creds_get_path(&creds->parent_obj,
+ QCRYPTO_TLS_CREDS_X509_CLIENT_KEY,
+ false, &files->keypath, errp) < 0) {
+ return -1;
+ }
+ }
+
+ if (!files->certpath &&
+ !files->keypath) {
+ return 0;
+ }
+ if (files->certpath && !files->keypath) {
+ error_setg(errp, "Cert '%s' without corresponding key",
+ files->certpath);
+ return -1;
+ }
+ if (!files->certpath && files->keypath) {
+ error_setg(errp, "Key '%s' without corresponding cert",
+ files->keypath);
+ return -1;
+ }
+
+ if (qcrypto_tls_creds_load_cert_list(creds,
+ files->certpath,
+ &files->certs,
+ &files->ncerts,
+ errp) < 0) {
+ return -1;
+ }
+
+ if (qcrypto_tls_creds_load_privkey(creds,
+ files->keypath,
+ &files->key,
+ errp) < 0) {
+ return -1;
+ }
+
+ ret = gnutls_certificate_set_x509_key(box->data.cert,
+ files->certs,
+ files->ncerts,
+ files->key);
+ if (ret < 0) {
+ error_setg(errp, "Cannot set certificate '%s' & key '%s': %s",
+ files->certpath, files->keypath, gnutls_strerror(ret));
+ return -1;
+ }
+ return 0;
+}
+
+
static int
qcrypto_tls_creds_x509_load(QCryptoTLSCredsX509 *creds,
Error **errp)
@@ -613,8 +721,6 @@ qcrypto_tls_creds_x509_load(QCryptoTLSCredsX509 *creds,
g_autoptr(QCryptoTLSCredsBox) box = NULL;
g_autoptr(QCryptoTLSCredsX509Files) files = NULL;
g_autofree char *cacrl = NULL;
- g_autofree char *cert = NULL;
- g_autofree char *key = NULL;
g_autofree char *dhparams = NULL;
bool isServer = (creds->parent_obj.endpoint ==
QCRYPTO_TLS_CREDS_ENDPOINT_SERVER);
@@ -646,60 +752,27 @@ qcrypto_tls_creds_x509_load(QCryptoTLSCredsX509 *creds,
return -1;
}
+ if (qcrypto_tls_creds_x509_load_identity(creds, box, files,
+ isServer, errp) < 0) {
+ return -1;
+ }
+
if (isServer) {
if (qcrypto_tls_creds_get_path(&creds->parent_obj,
QCRYPTO_TLS_CREDS_X509_CA_CRL,
false, &cacrl, errp) < 0 ||
- qcrypto_tls_creds_get_path(&creds->parent_obj,
- QCRYPTO_TLS_CREDS_X509_SERVER_CERT,
- true, &cert, errp) < 0 ||
- qcrypto_tls_creds_get_path(&creds->parent_obj,
- QCRYPTO_TLS_CREDS_X509_SERVER_KEY,
- true, &key, errp) < 0 ||
qcrypto_tls_creds_get_path(&creds->parent_obj,
QCRYPTO_TLS_CREDS_DH_PARAMS,
false, &dhparams, errp) < 0) {
return -1;
}
- } else {
- if (qcrypto_tls_creds_get_path(&creds->parent_obj,
- QCRYPTO_TLS_CREDS_X509_CLIENT_CERT,
- false, &cert, errp) < 0 ||
- qcrypto_tls_creds_get_path(&creds->parent_obj,
- QCRYPTO_TLS_CREDS_X509_CLIENT_KEY,
- false, &key, errp) < 0) {
- return -1;
- }
}
if (creds->sanityCheck &&
- qcrypto_tls_creds_x509_sanity_check(creds, files, isServer,
- cert, errp) < 0) {
+ qcrypto_tls_creds_x509_sanity_check(creds, files, isServer, errp) < 0) {
return -1;
}
- if (cert != NULL && key != NULL) {
- char *password = NULL;
- if (creds->passwordid) {
- password = qcrypto_secret_lookup_as_utf8(creds->passwordid,
- errp);
- if (!password) {
- return -1;
- }
- }
- ret = gnutls_certificate_set_x509_key_file2(box->data.cert,
- cert, key,
- GNUTLS_X509_FMT_PEM,
- password,
- 0);
- g_free(password);
- if (ret < 0) {
- error_setg(errp, "Cannot load certificate '%s' & key '%s': %s",
- cert, key, gnutls_strerror(ret));
- return -1;
- }
- }
-
if (cacrl != NULL) {
ret = gnutls_certificate_set_x509_crl_file(box->data.cert,
cacrl,
diff --git a/tests/unit/test-crypto-tlscredsx509.c b/tests/unit/test-crypto-tlscredsx509.c
index a5f21728d4..b1ad7d5c0d 100644
--- a/tests/unit/test-crypto-tlscredsx509.c
+++ b/tests/unit/test-crypto-tlscredsx509.c
@@ -95,16 +95,16 @@ static void test_tls_creds(const void *opaque)
if (access(data->crt, R_OK) == 0) {
g_assert(link(data->crt,
CERT_DIR QCRYPTO_TLS_CREDS_X509_SERVER_CERT) == 0);
+ g_assert(link(KEYFILE,
+ CERT_DIR QCRYPTO_TLS_CREDS_X509_SERVER_KEY) == 0);
}
- g_assert(link(KEYFILE,
- CERT_DIR QCRYPTO_TLS_CREDS_X509_SERVER_KEY) == 0);
} else {
if (access(data->crt, R_OK) == 0) {
g_assert(link(data->crt,
CERT_DIR QCRYPTO_TLS_CREDS_X509_CLIENT_CERT) == 0);
+ g_assert(link(KEYFILE,
+ CERT_DIR QCRYPTO_TLS_CREDS_X509_CLIENT_KEY) == 0);
}
- g_assert(link(KEYFILE,
- CERT_DIR QCRYPTO_TLS_CREDS_X509_CLIENT_KEY) == 0);
}
creds = test_tls_creds_create(
--
2.51.1
next prev parent reply other threads:[~2025-10-30 14:53 UTC|newest]
Thread overview: 44+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-10-30 14:49 [PATCH 00/21] crypto: support multiple parallel certificate identities Daniel P. Berrangé
2025-10-30 14:49 ` [PATCH 01/21] crypto: remove redundant parameter checking CA certs Daniel P. Berrangé
2025-10-31 13:57 ` Philippe Mathieu-Daudé
2025-10-30 14:49 ` [PATCH 02/21] crypto: add missing free of certs array Daniel P. Berrangé
2025-10-30 14:49 ` [PATCH 03/21] crypto: replace stat() with access() for credential checks Daniel P. Berrangé
2025-10-31 13:58 ` Philippe Mathieu-Daudé
2025-10-30 14:49 ` [PATCH 04/21] crypto: remove redundant access() checks before loading certs Daniel P. Berrangé
2025-10-30 14:49 ` [PATCH 05/21] crypto: move check for TLS creds 'dir' property Daniel P. Berrangé
2025-10-30 14:49 ` [PATCH 06/21] crypto: use g_autofree when loading x509 credentials Daniel P. Berrangé
2025-10-31 13:59 ` Philippe Mathieu-Daudé
2025-10-30 14:49 ` [PATCH 07/21] crypto: remove needless indirection via parent_obj field Daniel P. Berrangé
2025-10-30 19:31 ` Marc-André Lureau
2025-10-31 14:00 ` Philippe Mathieu-Daudé
2025-10-30 14:49 ` [PATCH 08/21] crypto: move release of DH parameters into TLS creds parent Daniel P. Berrangé
2025-10-30 19:31 ` Marc-André Lureau
2025-10-30 14:49 ` [PATCH 09/21] crypto: shorten the endpoint == server check in TLS creds Daniel P. Berrangé
2025-10-30 19:32 ` Marc-André Lureau
2025-10-30 14:49 ` [PATCH 10/21] crypto: remove duplication loading x509 CA cert Daniel P. Berrangé
2025-10-30 19:31 ` Marc-André Lureau
2025-10-30 14:49 ` [PATCH 11/21] crypto: reduce duplication in handling TLS priority strings Daniel P. Berrangé
2025-10-30 19:40 ` Marc-André Lureau
2025-10-30 14:49 ` [PATCH 12/21] crypto: introduce method for reloading TLS creds Daniel P. Berrangé
2025-10-30 19:43 ` Marc-André Lureau
2025-10-31 11:31 ` Daniel P. Berrangé
2025-10-30 14:49 ` [PATCH 13/21] crypto: introduce a wrapper around gnutls credentials Daniel P. Berrangé
2025-10-31 11:23 ` Marc-André Lureau
2025-10-31 11:27 ` Daniel P. Berrangé
2025-10-30 14:49 ` [PATCH 14/21] crypto: fix lifecycle handling of gnutls credentials objects Daniel P. Berrangé
2025-10-31 11:24 ` Marc-André Lureau
2025-10-30 14:49 ` [PATCH 15/21] crypto: make TLS credentials structs private Daniel P. Berrangé
2025-10-31 11:25 ` Marc-André Lureau
2025-10-30 14:49 ` [PATCH 16/21] crypto: deprecate use of external dh-params.pem file Daniel P. Berrangé
2025-10-31 11:32 ` Marc-André Lureau
2025-10-31 11:38 ` Daniel P. Berrangé
2025-10-30 14:49 ` [PATCH 17/21] crypto: avoid loading the CA certs twice Daniel P. Berrangé
2025-10-31 15:08 ` Marc-André Lureau
2025-10-30 14:49 ` Daniel P. Berrangé [this message]
2025-11-01 9:25 ` [PATCH 18/21] crypto: avoid loading the identity " Marc-André Lureau
2025-10-30 14:49 ` [PATCH 19/21] crypto: expand logic to cope with multiple certificate identities Daniel P. Berrangé
2025-11-02 8:11 ` Marc-André Lureau
2025-10-30 14:49 ` [PATCH 20/21] crypto: support upto 5 parallel " Daniel P. Berrangé
2025-11-02 8:25 ` Marc-André Lureau
2025-10-30 14:49 ` [PATCH 21/21] docs: creation of x509 certs compliant with post-quantum crypto Daniel P. Berrangé
2025-11-02 11:40 ` Marc-André Lureau
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20251030144927.2241109-19-berrange@redhat.com \
--to=berrange@redhat.com \
--cc=devel@lists.libvirt.org \
--cc=marcandre.lureau@redhat.com \
--cc=qemu-devel@nongnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).