* [PATCH 01/21] crypto: remove redundant parameter checking CA certs
2025-10-30 14:47 [PATCH 00/21] crypto: support multiple parallel certificate identities Daniel P. Berrangé
@ 2025-10-30 14:47 ` Daniel P. Berrangé
2025-10-30 19:30 ` Marc-André Lureau
2025-10-30 14:47 ` [PATCH 02/21] crypto: add missing free of certs array Daniel P. Berrangé
` (5 subsequent siblings)
6 siblings, 1 reply; 17+ messages in thread
From: Daniel P. Berrangé @ 2025-10-30 14:47 UTC (permalink / raw)
To: qemu-devel; +Cc: Daniel P. Berrangé, Marc-André Lureau, devel
The only caller of qcrypto_tls_creds_check_authority_chain always
passes 'true' for the 'isCA' parameter. The point of this method
is to check the CA chani, so no other value would ever make sense.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
crypto/tlscredsx509.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/crypto/tlscredsx509.c b/crypto/tlscredsx509.c
index db2b74bafa..847fd4d9fa 100644
--- a/crypto/tlscredsx509.c
+++ b/crypto/tlscredsx509.c
@@ -315,7 +315,6 @@ qcrypto_tls_creds_check_authority_chain(QCryptoTLSCredsX509 *creds,
unsigned int ncacerts,
const char *cacertFile,
bool isServer,
- bool isCA,
Error **errp)
{
gnutls_x509_crt_t cert_to_check = certs[ncerts - 1];
@@ -356,7 +355,7 @@ qcrypto_tls_creds_check_authority_chain(QCryptoTLSCredsX509 *creds,
*/
return qcrypto_tls_creds_check_cert(
creds, cert_to_check, cacertFile,
- isServer, isCA, errp);
+ isServer, true, errp);
}
for (int i = 0; i < ncacerts; i++) {
if (gnutls_x509_crt_check_issuer(cert_to_check,
@@ -370,7 +369,7 @@ qcrypto_tls_creds_check_authority_chain(QCryptoTLSCredsX509 *creds,
}
if (qcrypto_tls_creds_check_cert(creds, cert_issuer, cacertFile,
- isServer, isCA, errp) < 0) {
+ isServer, true, errp) < 0) {
return -1;
}
@@ -534,7 +533,7 @@ qcrypto_tls_creds_x509_sanity_check(QCryptoTLSCredsX509 *creds,
certs, ncerts,
cacerts, ncacerts,
cacertFile, isServer,
- true, errp) < 0) {
+ errp) < 0) {
goto cleanup;
}
--
2.51.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH 01/21] crypto: remove redundant parameter checking CA certs
2025-10-30 14:47 ` [PATCH 01/21] crypto: remove redundant parameter checking CA certs Daniel P. Berrangé
@ 2025-10-30 19:30 ` Marc-André Lureau
0 siblings, 0 replies; 17+ messages in thread
From: Marc-André Lureau @ 2025-10-30 19:30 UTC (permalink / raw)
To: Daniel P. Berrangé; +Cc: qemu-devel, devel
[-- Attachment #1: Type: text/plain, Size: 2473 bytes --]
On Thu, Oct 30, 2025 at 6:48 PM Daniel P. Berrangé <berrange@redhat.com>
wrote:
> The only caller of qcrypto_tls_creds_check_authority_chain always
> passes 'true' for the 'isCA' parameter. The point of this method
> is to check the CA chani, so no other value would ever make sense.
>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
> crypto/tlscredsx509.c | 7 +++----
> 1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/crypto/tlscredsx509.c b/crypto/tlscredsx509.c
> index db2b74bafa..847fd4d9fa 100644
> --- a/crypto/tlscredsx509.c
> +++ b/crypto/tlscredsx509.c
> @@ -315,7 +315,6 @@
> qcrypto_tls_creds_check_authority_chain(QCryptoTLSCredsX509 *creds,
> unsigned int ncacerts,
> const char *cacertFile,
> bool isServer,
> - bool isCA,
> Error **errp)
> {
> gnutls_x509_crt_t cert_to_check = certs[ncerts - 1];
> @@ -356,7 +355,7 @@
> qcrypto_tls_creds_check_authority_chain(QCryptoTLSCredsX509 *creds,
> */
> return qcrypto_tls_creds_check_cert(
> creds, cert_to_check, cacertFile,
> - isServer, isCA, errp);
> + isServer, true, errp);
> }
> for (int i = 0; i < ncacerts; i++) {
> if (gnutls_x509_crt_check_issuer(cert_to_check,
> @@ -370,7 +369,7 @@
> qcrypto_tls_creds_check_authority_chain(QCryptoTLSCredsX509 *creds,
> }
>
> if (qcrypto_tls_creds_check_cert(creds, cert_issuer, cacertFile,
> - isServer, isCA, errp) < 0) {
> + isServer, true, errp) < 0) {
> return -1;
> }
>
> @@ -534,7 +533,7 @@
> qcrypto_tls_creds_x509_sanity_check(QCryptoTLSCredsX509 *creds,
> certs, ncerts,
> cacerts, ncacerts,
> cacertFile, isServer,
> - true, errp) < 0) {
> + errp) < 0) {
> goto cleanup;
> }
>
> --
> 2.51.1
>
>
[-- Attachment #2: Type: text/html, Size: 3597 bytes --]
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 02/21] crypto: add missing free of certs array
2025-10-30 14:47 [PATCH 00/21] crypto: support multiple parallel certificate identities Daniel P. Berrangé
2025-10-30 14:47 ` [PATCH 01/21] crypto: remove redundant parameter checking CA certs Daniel P. Berrangé
@ 2025-10-30 14:47 ` Daniel P. Berrangé
2025-10-30 19:31 ` Marc-André Lureau
2025-10-30 14:47 ` [PATCH 03/21] crypto: replace stat() with access() for credential checks Daniel P. Berrangé
` (4 subsequent siblings)
6 siblings, 1 reply; 17+ messages in thread
From: Daniel P. Berrangé @ 2025-10-30 14:47 UTC (permalink / raw)
To: qemu-devel; +Cc: Daniel P. Berrangé, Marc-André Lureau, devel
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
crypto/tlscredsx509.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/crypto/tlscredsx509.c b/crypto/tlscredsx509.c
index 847fd4d9fa..75c70af522 100644
--- a/crypto/tlscredsx509.c
+++ b/crypto/tlscredsx509.c
@@ -550,6 +550,7 @@ qcrypto_tls_creds_x509_sanity_check(QCryptoTLSCredsX509 *creds,
for (i = 0; i < ncerts; i++) {
gnutls_x509_crt_deinit(certs[i]);
}
+ g_free(certs);
for (i = 0; i < ncacerts; i++) {
gnutls_x509_crt_deinit(cacerts[i]);
}
--
2.51.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH 02/21] crypto: add missing free of certs array
2025-10-30 14:47 ` [PATCH 02/21] crypto: add missing free of certs array Daniel P. Berrangé
@ 2025-10-30 19:31 ` Marc-André Lureau
0 siblings, 0 replies; 17+ messages in thread
From: Marc-André Lureau @ 2025-10-30 19:31 UTC (permalink / raw)
To: Daniel P. Berrangé; +Cc: qemu-devel, devel
[-- Attachment #1: Type: text/plain, Size: 794 bytes --]
Hi
On Thu, Oct 30, 2025 at 6:48 PM Daniel P. Berrangé <berrange@redhat.com>
wrote:
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
> crypto/tlscredsx509.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/crypto/tlscredsx509.c b/crypto/tlscredsx509.c
> index 847fd4d9fa..75c70af522 100644
> --- a/crypto/tlscredsx509.c
> +++ b/crypto/tlscredsx509.c
> @@ -550,6 +550,7 @@
> qcrypto_tls_creds_x509_sanity_check(QCryptoTLSCredsX509 *creds,
> for (i = 0; i < ncerts; i++) {
> gnutls_x509_crt_deinit(certs[i]);
> }
> + g_free(certs);
> for (i = 0; i < ncacerts; i++) {
> gnutls_x509_crt_deinit(cacerts[i]);
> }
> --
> 2.51.1
>
>
[-- Attachment #2: Type: text/html, Size: 1483 bytes --]
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 03/21] crypto: replace stat() with access() for credential checks
2025-10-30 14:47 [PATCH 00/21] crypto: support multiple parallel certificate identities Daniel P. Berrangé
2025-10-30 14:47 ` [PATCH 01/21] crypto: remove redundant parameter checking CA certs Daniel P. Berrangé
2025-10-30 14:47 ` [PATCH 02/21] crypto: add missing free of certs array Daniel P. Berrangé
@ 2025-10-30 14:47 ` Daniel P. Berrangé
2025-10-30 19:31 ` Marc-André Lureau
2025-10-30 14:47 ` [PATCH 04/21] crypto: remove redundant access() checks before loading certs Daniel P. Berrangé
` (3 subsequent siblings)
6 siblings, 1 reply; 17+ messages in thread
From: Daniel P. Berrangé @ 2025-10-30 14:47 UTC (permalink / raw)
To: qemu-devel; +Cc: Daniel P. Berrangé, Marc-André Lureau, devel
Readability of the credential files is what matters for our usage,
so access() is more appropriate than stat().
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
crypto/tlscreds.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/crypto/tlscreds.c b/crypto/tlscreds.c
index 9e59594d67..208a7e6d8f 100644
--- a/crypto/tlscreds.c
+++ b/crypto/tlscreds.c
@@ -100,7 +100,6 @@ qcrypto_tls_creds_get_path(QCryptoTLSCreds *creds,
char **cred,
Error **errp)
{
- struct stat sb;
int ret = -1;
if (!creds->dir) {
@@ -114,7 +113,7 @@ qcrypto_tls_creds_get_path(QCryptoTLSCreds *creds,
*cred = g_strdup_printf("%s/%s", creds->dir, filename);
- if (stat(*cred, &sb) < 0) {
+ if (access(*cred, R_OK) < 0) {
if (errno == ENOENT && !required) {
ret = 0;
} else {
--
2.51.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH 03/21] crypto: replace stat() with access() for credential checks
2025-10-30 14:47 ` [PATCH 03/21] crypto: replace stat() with access() for credential checks Daniel P. Berrangé
@ 2025-10-30 19:31 ` Marc-André Lureau
0 siblings, 0 replies; 17+ messages in thread
From: Marc-André Lureau @ 2025-10-30 19:31 UTC (permalink / raw)
To: Daniel P. Berrangé; +Cc: qemu-devel, devel
[-- Attachment #1: Type: text/plain, Size: 1164 bytes --]
Hi
On Thu, Oct 30, 2025 at 6:48 PM Daniel P. Berrangé <berrange@redhat.com>
wrote:
> Readability of the credential files is what matters for our usage,
> so access() is more appropriate than stat().
>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
> crypto/tlscreds.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/crypto/tlscreds.c b/crypto/tlscreds.c
> index 9e59594d67..208a7e6d8f 100644
> --- a/crypto/tlscreds.c
> +++ b/crypto/tlscreds.c
> @@ -100,7 +100,6 @@ qcrypto_tls_creds_get_path(QCryptoTLSCreds *creds,
> char **cred,
> Error **errp)
> {
> - struct stat sb;
> int ret = -1;
>
> if (!creds->dir) {
> @@ -114,7 +113,7 @@ qcrypto_tls_creds_get_path(QCryptoTLSCreds *creds,
>
> *cred = g_strdup_printf("%s/%s", creds->dir, filename);
>
> - if (stat(*cred, &sb) < 0) {
> + if (access(*cred, R_OK) < 0) {
> if (errno == ENOENT && !required) {
> ret = 0;
> } else {
> --
> 2.51.1
>
>
[-- Attachment #2: Type: text/html, Size: 1949 bytes --]
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 04/21] crypto: remove redundant access() checks before loading certs
2025-10-30 14:47 [PATCH 00/21] crypto: support multiple parallel certificate identities Daniel P. Berrangé
` (2 preceding siblings ...)
2025-10-30 14:47 ` [PATCH 03/21] crypto: replace stat() with access() for credential checks Daniel P. Berrangé
@ 2025-10-30 14:47 ` Daniel P. Berrangé
2025-10-30 19:31 ` Marc-André Lureau
2025-10-30 14:47 ` [PATCH 05/21] crypto: move check for TLS creds 'dir' property Daniel P. Berrangé
` (2 subsequent siblings)
6 siblings, 1 reply; 17+ messages in thread
From: Daniel P. Berrangé @ 2025-10-30 14:47 UTC (permalink / raw)
To: qemu-devel; +Cc: Daniel P. Berrangé, Marc-André Lureau, devel
The qcrypto_tls_creds_get_path method will perform an access()
check on the file and return a NULL path if it fails. By the
time we get to loading the cert files we know they must exist
on disk and thus the second access() check is redundant.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
crypto/tlscredsx509.c | 22 ++++++++++------------
1 file changed, 10 insertions(+), 12 deletions(-)
diff --git a/crypto/tlscredsx509.c b/crypto/tlscredsx509.c
index 75c70af522..0acb17b6ec 100644
--- a/crypto/tlscredsx509.c
+++ b/crypto/tlscredsx509.c
@@ -496,8 +496,7 @@ qcrypto_tls_creds_x509_sanity_check(QCryptoTLSCredsX509 *creds,
size_t i;
int ret = -1;
- if (certFile &&
- access(certFile, R_OK) == 0) {
+ if (certFile) {
if (qcrypto_tls_creds_load_cert_list(creds,
certFile,
&certs,
@@ -508,16 +507,15 @@ qcrypto_tls_creds_x509_sanity_check(QCryptoTLSCredsX509 *creds,
goto cleanup;
}
}
- if (access(cacertFile, R_OK) == 0) {
- if (qcrypto_tls_creds_load_cert_list(creds,
- cacertFile,
- &cacerts,
- &ncacerts,
- isServer,
- true,
- errp) < 0) {
- goto cleanup;
- }
+
+ if (qcrypto_tls_creds_load_cert_list(creds,
+ cacertFile,
+ &cacerts,
+ &ncacerts,
+ isServer,
+ true,
+ errp) < 0) {
+ goto cleanup;
}
for (i = 0; i < ncerts; i++) {
--
2.51.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH 04/21] crypto: remove redundant access() checks before loading certs
2025-10-30 14:47 ` [PATCH 04/21] crypto: remove redundant access() checks before loading certs Daniel P. Berrangé
@ 2025-10-30 19:31 ` Marc-André Lureau
0 siblings, 0 replies; 17+ messages in thread
From: Marc-André Lureau @ 2025-10-30 19:31 UTC (permalink / raw)
To: Daniel P. Berrangé; +Cc: qemu-devel, devel
[-- Attachment #1: Type: text/plain, Size: 2296 bytes --]
Hi
On Thu, Oct 30, 2025 at 6:48 PM Daniel P. Berrangé <berrange@redhat.com>
wrote:
> The qcrypto_tls_creds_get_path method will perform an access()
> check on the file and return a NULL path if it fails. By the
> time we get to loading the cert files we know they must exist
> on disk and thus the second access() check is redundant.
>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
> crypto/tlscredsx509.c | 22 ++++++++++------------
> 1 file changed, 10 insertions(+), 12 deletions(-)
>
> diff --git a/crypto/tlscredsx509.c b/crypto/tlscredsx509.c
> index 75c70af522..0acb17b6ec 100644
> --- a/crypto/tlscredsx509.c
> +++ b/crypto/tlscredsx509.c
> @@ -496,8 +496,7 @@
> qcrypto_tls_creds_x509_sanity_check(QCryptoTLSCredsX509 *creds,
> size_t i;
> int ret = -1;
>
> - if (certFile &&
> - access(certFile, R_OK) == 0) {
> + if (certFile) {
> if (qcrypto_tls_creds_load_cert_list(creds,
> certFile,
> &certs,
> @@ -508,16 +507,15 @@
> qcrypto_tls_creds_x509_sanity_check(QCryptoTLSCredsX509 *creds,
> goto cleanup;
> }
> }
> - if (access(cacertFile, R_OK) == 0) {
> - if (qcrypto_tls_creds_load_cert_list(creds,
> - cacertFile,
> - &cacerts,
> - &ncacerts,
> - isServer,
> - true,
> - errp) < 0) {
> - goto cleanup;
> - }
> +
> + if (qcrypto_tls_creds_load_cert_list(creds,
> + cacertFile,
> + &cacerts,
> + &ncacerts,
> + isServer,
> + true,
> + errp) < 0) {
> + goto cleanup;
> }
>
> for (i = 0; i < ncerts; i++) {
> --
> 2.51.1
>
>
[-- Attachment #2: Type: text/html, Size: 3430 bytes --]
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 05/21] crypto: move check for TLS creds 'dir' property
2025-10-30 14:47 [PATCH 00/21] crypto: support multiple parallel certificate identities Daniel P. Berrangé
` (3 preceding siblings ...)
2025-10-30 14:47 ` [PATCH 04/21] crypto: remove redundant access() checks before loading certs Daniel P. Berrangé
@ 2025-10-30 14:47 ` Daniel P. Berrangé
2025-10-30 19:31 ` Marc-André Lureau
2025-10-30 14:47 ` [PATCH 06/21] crypto: use g_autofree when loading x509 credentials Daniel P. Berrangé
2025-10-30 15:30 ` [PATCH 00/21] crypto: support multiple parallel certificate identities Daniel P. Berrangé
6 siblings, 1 reply; 17+ messages in thread
From: Daniel P. Berrangé @ 2025-10-30 14:47 UTC (permalink / raw)
To: qemu-devel; +Cc: Daniel P. Berrangé, Marc-André Lureau, devel
The check for the 'dir' property is being repeated for every
credential file to be loaded, but this results in incorrect
logic for optional credentials. The 'dir' property is mandatory
for PSK and x509 creds, even if some individual files are
optional. Address this by separating the check for the 'dir'
property.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
crypto/tlscreds.c | 9 ---------
crypto/tlscredsanon.c | 3 ++-
crypto/tlscredspsk.c | 5 +++++
crypto/tlscredsx509.c | 8 ++++++--
4 files changed, 13 insertions(+), 12 deletions(-)
diff --git a/crypto/tlscreds.c b/crypto/tlscreds.c
index 208a7e6d8f..65e97ddd11 100644
--- a/crypto/tlscreds.c
+++ b/crypto/tlscreds.c
@@ -102,15 +102,6 @@ qcrypto_tls_creds_get_path(QCryptoTLSCreds *creds,
{
int ret = -1;
- if (!creds->dir) {
- if (required) {
- error_setg(errp, "Missing 'dir' property value");
- return -1;
- } else {
- return 0;
- }
- }
-
*cred = g_strdup_printf("%s/%s", creds->dir, filename);
if (access(*cred, R_OK) < 0) {
diff --git a/crypto/tlscredsanon.c b/crypto/tlscredsanon.c
index 44af9e6c9a..bc3351b5d6 100644
--- a/crypto/tlscredsanon.c
+++ b/crypto/tlscredsanon.c
@@ -43,7 +43,8 @@ qcrypto_tls_creds_anon_load(QCryptoTLSCredsAnon *creds,
creds->parent_obj.dir ? creds->parent_obj.dir : "<nodir>");
if (creds->parent_obj.endpoint == QCRYPTO_TLS_CREDS_ENDPOINT_SERVER) {
- if (qcrypto_tls_creds_get_path(&creds->parent_obj,
+ if (creds->parent_obj.dir &&
+ qcrypto_tls_creds_get_path(&creds->parent_obj,
QCRYPTO_TLS_CREDS_DH_PARAMS,
false, &dhparams, errp) < 0) {
return -1;
diff --git a/crypto/tlscredspsk.c b/crypto/tlscredspsk.c
index 5b68a6b7ba..545d3e45db 100644
--- a/crypto/tlscredspsk.c
+++ b/crypto/tlscredspsk.c
@@ -81,6 +81,11 @@ qcrypto_tls_creds_psk_load(QCryptoTLSCredsPSK *creds,
trace_qcrypto_tls_creds_psk_load(creds,
creds->parent_obj.dir ? creds->parent_obj.dir : "<nodir>");
+ if (!creds->parent_obj.dir) {
+ error_setg(errp, "Missing 'dir' property value");
+ goto cleanup;
+ }
+
if (creds->parent_obj.endpoint == QCRYPTO_TLS_CREDS_ENDPOINT_SERVER) {
if (creds->username) {
error_setg(errp, "username should not be set when endpoint=server");
diff --git a/crypto/tlscredsx509.c b/crypto/tlscredsx509.c
index 0acb17b6ec..8fe6cc8e93 100644
--- a/crypto/tlscredsx509.c
+++ b/crypto/tlscredsx509.c
@@ -567,8 +567,12 @@ qcrypto_tls_creds_x509_load(QCryptoTLSCredsX509 *creds,
int ret;
int rv = -1;
- trace_qcrypto_tls_creds_x509_load(creds,
- creds->parent_obj.dir ? creds->parent_obj.dir : "<nodir>");
+ if (!creds->parent_obj.dir) {
+ error_setg(errp, "Missing 'dir' property value");
+ return -1;
+ }
+
+ trace_qcrypto_tls_creds_x509_load(creds, creds->parent_obj.dir);
if (creds->parent_obj.endpoint == QCRYPTO_TLS_CREDS_ENDPOINT_SERVER) {
if (qcrypto_tls_creds_get_path(&creds->parent_obj,
--
2.51.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH 05/21] crypto: move check for TLS creds 'dir' property
2025-10-30 14:47 ` [PATCH 05/21] crypto: move check for TLS creds 'dir' property Daniel P. Berrangé
@ 2025-10-30 19:31 ` Marc-André Lureau
0 siblings, 0 replies; 17+ messages in thread
From: Marc-André Lureau @ 2025-10-30 19:31 UTC (permalink / raw)
To: Daniel P. Berrangé; +Cc: qemu-devel, devel
[-- Attachment #1: Type: text/plain, Size: 3590 bytes --]
Hi
On Thu, Oct 30, 2025 at 6:48 PM Daniel P. Berrangé <berrange@redhat.com>
wrote:
> The check for the 'dir' property is being repeated for every
> credential file to be loaded, but this results in incorrect
> logic for optional credentials. The 'dir' property is mandatory
> for PSK and x509 creds, even if some individual files are
> optional. Address this by separating the check for the 'dir'
> property.
>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
> crypto/tlscreds.c | 9 ---------
> crypto/tlscredsanon.c | 3 ++-
> crypto/tlscredspsk.c | 5 +++++
> crypto/tlscredsx509.c | 8 ++++++--
> 4 files changed, 13 insertions(+), 12 deletions(-)
>
> diff --git a/crypto/tlscreds.c b/crypto/tlscreds.c
> index 208a7e6d8f..65e97ddd11 100644
> --- a/crypto/tlscreds.c
> +++ b/crypto/tlscreds.c
> @@ -102,15 +102,6 @@ qcrypto_tls_creds_get_path(QCryptoTLSCreds *creds,
> {
> int ret = -1;
>
> - if (!creds->dir) {
> - if (required) {
> - error_setg(errp, "Missing 'dir' property value");
> - return -1;
> - } else {
> - return 0;
> - }
> - }
> -
> *cred = g_strdup_printf("%s/%s", creds->dir, filename);
>
> if (access(*cred, R_OK) < 0) {
> diff --git a/crypto/tlscredsanon.c b/crypto/tlscredsanon.c
> index 44af9e6c9a..bc3351b5d6 100644
> --- a/crypto/tlscredsanon.c
> +++ b/crypto/tlscredsanon.c
> @@ -43,7 +43,8 @@ qcrypto_tls_creds_anon_load(QCryptoTLSCredsAnon *creds,
> creds->parent_obj.dir ? creds->parent_obj.dir : "<nodir>");
>
> if (creds->parent_obj.endpoint == QCRYPTO_TLS_CREDS_ENDPOINT_SERVER) {
> - if (qcrypto_tls_creds_get_path(&creds->parent_obj,
> + if (creds->parent_obj.dir &&
> + qcrypto_tls_creds_get_path(&creds->parent_obj,
> QCRYPTO_TLS_CREDS_DH_PARAMS,
> false, &dhparams, errp) < 0) {
> return -1;
> diff --git a/crypto/tlscredspsk.c b/crypto/tlscredspsk.c
> index 5b68a6b7ba..545d3e45db 100644
> --- a/crypto/tlscredspsk.c
> +++ b/crypto/tlscredspsk.c
> @@ -81,6 +81,11 @@ qcrypto_tls_creds_psk_load(QCryptoTLSCredsPSK *creds,
> trace_qcrypto_tls_creds_psk_load(creds,
> creds->parent_obj.dir ? creds->parent_obj.dir : "<nodir>");
>
> + if (!creds->parent_obj.dir) {
> + error_setg(errp, "Missing 'dir' property value");
> + goto cleanup;
> + }
> +
> if (creds->parent_obj.endpoint == QCRYPTO_TLS_CREDS_ENDPOINT_SERVER) {
> if (creds->username) {
> error_setg(errp, "username should not be set when
> endpoint=server");
> diff --git a/crypto/tlscredsx509.c b/crypto/tlscredsx509.c
> index 0acb17b6ec..8fe6cc8e93 100644
> --- a/crypto/tlscredsx509.c
> +++ b/crypto/tlscredsx509.c
> @@ -567,8 +567,12 @@ qcrypto_tls_creds_x509_load(QCryptoTLSCredsX509
> *creds,
> int ret;
> int rv = -1;
>
> - trace_qcrypto_tls_creds_x509_load(creds,
> - creds->parent_obj.dir ? creds->parent_obj.dir : "<nodir>");
> + if (!creds->parent_obj.dir) {
> + error_setg(errp, "Missing 'dir' property value");
> + return -1;
> + }
> +
> + trace_qcrypto_tls_creds_x509_load(creds, creds->parent_obj.dir);
>
> if (creds->parent_obj.endpoint == QCRYPTO_TLS_CREDS_ENDPOINT_SERVER) {
> if (qcrypto_tls_creds_get_path(&creds->parent_obj,
> --
> 2.51.1
>
>
[-- Attachment #2: Type: text/html, Size: 4825 bytes --]
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 06/21] crypto: use g_autofree when loading x509 credentials
2025-10-30 14:47 [PATCH 00/21] crypto: support multiple parallel certificate identities Daniel P. Berrangé
` (4 preceding siblings ...)
2025-10-30 14:47 ` [PATCH 05/21] crypto: move check for TLS creds 'dir' property Daniel P. Berrangé
@ 2025-10-30 14:47 ` Daniel P. Berrangé
2025-10-30 19:23 ` Marc-André Lureau
2025-10-30 15:30 ` [PATCH 00/21] crypto: support multiple parallel certificate identities Daniel P. Berrangé
6 siblings, 1 reply; 17+ messages in thread
From: Daniel P. Berrangé @ 2025-10-30 14:47 UTC (permalink / raw)
To: qemu-devel; +Cc: Daniel P. Berrangé, Marc-André Lureau, devel
This allows removal of goto jumps during loading of the credentials
and will simplify the diff in following commits.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
crypto/tlscredsx509.c | 35 +++++++++++++++--------------------
1 file changed, 15 insertions(+), 20 deletions(-)
diff --git a/crypto/tlscredsx509.c b/crypto/tlscredsx509.c
index 8fe6cc8e93..e5b869a35f 100644
--- a/crypto/tlscredsx509.c
+++ b/crypto/tlscredsx509.c
@@ -562,10 +562,12 @@ static int
qcrypto_tls_creds_x509_load(QCryptoTLSCredsX509 *creds,
Error **errp)
{
- char *cacert = NULL, *cacrl = NULL, *cert = NULL,
- *key = NULL, *dhparams = NULL;
+ g_autofree char *cacert = NULL;
+ g_autofree char *cacrl = NULL;
+ g_autofree char *cert = NULL;
+ g_autofree char *key = NULL;
+ g_autofree char *dhparams = NULL;
int ret;
- int rv = -1;
if (!creds->parent_obj.dir) {
error_setg(errp, "Missing 'dir' property value");
@@ -590,7 +592,7 @@ qcrypto_tls_creds_x509_load(QCryptoTLSCredsX509 *creds,
qcrypto_tls_creds_get_path(&creds->parent_obj,
QCRYPTO_TLS_CREDS_DH_PARAMS,
false, &dhparams, errp) < 0) {
- goto cleanup;
+ return -1;
}
} else {
if (qcrypto_tls_creds_get_path(&creds->parent_obj,
@@ -602,7 +604,7 @@ qcrypto_tls_creds_x509_load(QCryptoTLSCredsX509 *creds,
qcrypto_tls_creds_get_path(&creds->parent_obj,
QCRYPTO_TLS_CREDS_X509_CLIENT_KEY,
false, &key, errp) < 0) {
- goto cleanup;
+ return -1;
}
}
@@ -610,14 +612,14 @@ qcrypto_tls_creds_x509_load(QCryptoTLSCredsX509 *creds,
qcrypto_tls_creds_x509_sanity_check(creds,
creds->parent_obj.endpoint == QCRYPTO_TLS_CREDS_ENDPOINT_SERVER,
cacert, cert, errp) < 0) {
- goto cleanup;
+ return -1;
}
ret = gnutls_certificate_allocate_credentials(&creds->data);
if (ret < 0) {
error_setg(errp, "Cannot allocate credentials: '%s'",
gnutls_strerror(ret));
- goto cleanup;
+ return -1;
}
ret = gnutls_certificate_set_x509_trust_file(creds->data,
@@ -626,7 +628,7 @@ qcrypto_tls_creds_x509_load(QCryptoTLSCredsX509 *creds,
if (ret < 0) {
error_setg(errp, "Cannot load CA certificate '%s': %s",
cacert, gnutls_strerror(ret));
- goto cleanup;
+ return -1;
}
if (cert != NULL && key != NULL) {
@@ -635,7 +637,7 @@ qcrypto_tls_creds_x509_load(QCryptoTLSCredsX509 *creds,
password = qcrypto_secret_lookup_as_utf8(creds->passwordid,
errp);
if (!password) {
- goto cleanup;
+ return -1;
}
}
ret = gnutls_certificate_set_x509_key_file2(creds->data,
@@ -647,7 +649,7 @@ qcrypto_tls_creds_x509_load(QCryptoTLSCredsX509 *creds,
if (ret < 0) {
error_setg(errp, "Cannot load certificate '%s' & key '%s': %s",
cert, key, gnutls_strerror(ret));
- goto cleanup;
+ return -1;
}
}
@@ -658,7 +660,7 @@ qcrypto_tls_creds_x509_load(QCryptoTLSCredsX509 *creds,
if (ret < 0) {
error_setg(errp, "Cannot load CRL '%s': %s",
cacrl, gnutls_strerror(ret));
- goto cleanup;
+ return -1;
}
}
@@ -666,20 +668,13 @@ qcrypto_tls_creds_x509_load(QCryptoTLSCredsX509 *creds,
if (qcrypto_tls_creds_get_dh_params_file(&creds->parent_obj, dhparams,
&creds->parent_obj.dh_params,
errp) < 0) {
- goto cleanup;
+ return -1;
}
gnutls_certificate_set_dh_params(creds->data,
creds->parent_obj.dh_params);
}
- rv = 0;
- cleanup:
- g_free(cacert);
- g_free(cacrl);
- g_free(cert);
- g_free(key);
- g_free(dhparams);
- return rv;
+ return 0;
}
--
2.51.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH 06/21] crypto: use g_autofree when loading x509 credentials
2025-10-30 14:47 ` [PATCH 06/21] crypto: use g_autofree when loading x509 credentials Daniel P. Berrangé
@ 2025-10-30 19:23 ` Marc-André Lureau
2025-10-31 15:06 ` Daniel P. Berrangé
0 siblings, 1 reply; 17+ messages in thread
From: Marc-André Lureau @ 2025-10-30 19:23 UTC (permalink / raw)
To: Daniel P. Berrangé; +Cc: qemu-devel, devel
[-- Attachment #1: Type: text/plain, Size: 4855 bytes --]
Hi
On Thu, Oct 30, 2025 at 6:48 PM Daniel P. Berrangé <berrange@redhat.com>
wrote:
> This allows removal of goto jumps during loading of the credentials
> and will simplify the diff in following commits.
>
>
If you want, you could also g_autofree password.
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
> crypto/tlscredsx509.c | 35 +++++++++++++++--------------------
> 1 file changed, 15 insertions(+), 20 deletions(-)
>
> diff --git a/crypto/tlscredsx509.c b/crypto/tlscredsx509.c
> index 8fe6cc8e93..e5b869a35f 100644
> --- a/crypto/tlscredsx509.c
> +++ b/crypto/tlscredsx509.c
> @@ -562,10 +562,12 @@ static int
> qcrypto_tls_creds_x509_load(QCryptoTLSCredsX509 *creds,
> Error **errp)
> {
> - char *cacert = NULL, *cacrl = NULL, *cert = NULL,
> - *key = NULL, *dhparams = NULL;
> + g_autofree char *cacert = NULL;
> + g_autofree char *cacrl = NULL;
> + g_autofree char *cert = NULL;
> + g_autofree char *key = NULL;
> + g_autofree char *dhparams = NULL;
> int ret;
> - int rv = -1;
>
> if (!creds->parent_obj.dir) {
> error_setg(errp, "Missing 'dir' property value");
> @@ -590,7 +592,7 @@ qcrypto_tls_creds_x509_load(QCryptoTLSCredsX509 *creds,
> qcrypto_tls_creds_get_path(&creds->parent_obj,
> QCRYPTO_TLS_CREDS_DH_PARAMS,
> false, &dhparams, errp) < 0) {
> - goto cleanup;
> + return -1;
> }
> } else {
> if (qcrypto_tls_creds_get_path(&creds->parent_obj,
> @@ -602,7 +604,7 @@ qcrypto_tls_creds_x509_load(QCryptoTLSCredsX509 *creds,
> qcrypto_tls_creds_get_path(&creds->parent_obj,
> QCRYPTO_TLS_CREDS_X509_CLIENT_KEY,
> false, &key, errp) < 0) {
> - goto cleanup;
> + return -1;
> }
> }
>
> @@ -610,14 +612,14 @@ qcrypto_tls_creds_x509_load(QCryptoTLSCredsX509
> *creds,
> qcrypto_tls_creds_x509_sanity_check(creds,
> creds->parent_obj.endpoint ==
> QCRYPTO_TLS_CREDS_ENDPOINT_SERVER,
> cacert, cert, errp) < 0) {
> - goto cleanup;
> + return -1;
> }
>
> ret = gnutls_certificate_allocate_credentials(&creds->data);
> if (ret < 0) {
> error_setg(errp, "Cannot allocate credentials: '%s'",
> gnutls_strerror(ret));
> - goto cleanup;
> + return -1;
> }
>
> ret = gnutls_certificate_set_x509_trust_file(creds->data,
> @@ -626,7 +628,7 @@ qcrypto_tls_creds_x509_load(QCryptoTLSCredsX509 *creds,
> if (ret < 0) {
> error_setg(errp, "Cannot load CA certificate '%s': %s",
> cacert, gnutls_strerror(ret));
> - goto cleanup;
> + return -1;
> }
>
> if (cert != NULL && key != NULL) {
> @@ -635,7 +637,7 @@ qcrypto_tls_creds_x509_load(QCryptoTLSCredsX509 *creds,
> password = qcrypto_secret_lookup_as_utf8(creds->passwordid,
> errp);
> if (!password) {
> - goto cleanup;
> + return -1;
> }
> }
> ret = gnutls_certificate_set_x509_key_file2(creds->data,
> @@ -647,7 +649,7 @@ qcrypto_tls_creds_x509_load(QCryptoTLSCredsX509 *creds,
> if (ret < 0) {
> error_setg(errp, "Cannot load certificate '%s' & key '%s':
> %s",
> cert, key, gnutls_strerror(ret));
> - goto cleanup;
> + return -1;
> }
> }
>
> @@ -658,7 +660,7 @@ qcrypto_tls_creds_x509_load(QCryptoTLSCredsX509 *creds,
> if (ret < 0) {
> error_setg(errp, "Cannot load CRL '%s': %s",
> cacrl, gnutls_strerror(ret));
> - goto cleanup;
> + return -1;
> }
> }
>
> @@ -666,20 +668,13 @@ qcrypto_tls_creds_x509_load(QCryptoTLSCredsX509
> *creds,
> if (qcrypto_tls_creds_get_dh_params_file(&creds->parent_obj,
> dhparams,
>
> &creds->parent_obj.dh_params,
> errp) < 0) {
> - goto cleanup;
> + return -1;
> }
> gnutls_certificate_set_dh_params(creds->data,
> creds->parent_obj.dh_params);
> }
>
> - rv = 0;
> - cleanup:
> - g_free(cacert);
> - g_free(cacrl);
> - g_free(cert);
> - g_free(key);
> - g_free(dhparams);
> - return rv;
> + return 0;
> }
>
>
> --
> 2.51.1
>
>
[-- Attachment #2: Type: text/html, Size: 6678 bytes --]
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH 06/21] crypto: use g_autofree when loading x509 credentials
2025-10-30 19:23 ` Marc-André Lureau
@ 2025-10-31 15:06 ` Daniel P. Berrangé
0 siblings, 0 replies; 17+ messages in thread
From: Daniel P. Berrangé @ 2025-10-31 15:06 UTC (permalink / raw)
To: Marc-André Lureau; +Cc: qemu-devel, devel
On Thu, Oct 30, 2025 at 11:23:44PM +0400, Marc-André Lureau wrote:
> Hi
>
> On Thu, Oct 30, 2025 at 6:48 PM Daniel P. Berrangé <berrange@redhat.com>
> wrote:
>
> > This allows removal of goto jumps during loading of the credentials
> > and will simplify the diff in following commits.
> >
> >
> If you want, you could also g_autofree password.
Yes, I'll add that.
> > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> >
>
> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>
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 :|
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 00/21] crypto: support multiple parallel certificate identities
2025-10-30 14:47 [PATCH 00/21] crypto: support multiple parallel certificate identities Daniel P. Berrangé
` (5 preceding siblings ...)
2025-10-30 14:47 ` [PATCH 06/21] crypto: use g_autofree when loading x509 credentials Daniel P. Berrangé
@ 2025-10-30 15:30 ` Daniel P. Berrangé
6 siblings, 0 replies; 17+ messages in thread
From: Daniel P. Berrangé @ 2025-10-30 15:30 UTC (permalink / raw)
To: qemu-devel; +Cc: Marc-André Lureau, devel
Sorry, this series aborted during mail sending so is incomplete.
See the immediately following re-post for the full set of 21 patches
On Thu, Oct 30, 2025 at 02:47:44PM +0000, Daniel P. Berrangé wrote:
> This series aims to improve the support for post-quantum cryptography
> in TLS connections by allowing multiple sets of certificates to be
> loaded. The idea is that during a transition period servers will have
> a traditional RSA based certificate in parallel with an MLDSA based
> certificate for PQC, and the right one will be dynamically determined
> during the TLS handshake.
>
> The first 12 patches are trivial cleanups.
>
> The next 3 patches fix a potential use-after-free problem
>
> The last patches introduce support for multiple certificates.
>
> NB, in terms of testing this will require either CentOS Stream 10,
> or Fedora 43. Most other distros will not support PQC out of the
> box at this time even if they have new enough gnutls, since they
> don't make use of the crypto-policies package which is needed to
> enable PQC by default.
>
> Daniel P. Berrangé (21):
> crypto: remove redundant parameter checking CA certs
> crypto: add missing free of certs array
> crypto: replace stat() with access() for credential checks
> crypto: remove redundant access() checks before loading certs
> crypto: move check for TLS creds 'dir' property
> crypto: use g_autofree when loading x509 credentials
> crypto: remove needless indirection via parent_obj field
> crypto: move release of DH parameters into TLS creds parent
> crypto: shorten the endpoint == server check in TLS creds
> crypto: remove duplication loading x509 CA cert
> crypto: reduce duplication in handling TLS priority strings
> crypto: introduce method for reloading TLS creds
> crypto: introduce a wrapper around gnutls credentials
> crypto: fix lifecycle handling of gnutls credentials objects
> crypto: make TLS credentials structs private
> crypto: deprecate use of external dh-params.pem file
> crypto: avoid loading the CA certs twice
> crypto: avoid loading the identity certs twice
> crypto: expand logic to cope with multiple certificate identities
> crypto: support upto 5 parallel certificate identities
> docs: creation of x509 certs compliant with post-quantum crypto
>
> crypto/meson.build | 5 +-
> crypto/tlscreds.c | 77 ++--
> crypto/tlscredsanon.c | 62 +--
> crypto/tlscredsbox.c | 101 +++++
> crypto/tlscredsbox.h | 46 ++
> crypto/tlscredspriv.h | 36 +-
> crypto/tlscredspsk.c | 64 ++-
> crypto/tlscredsx509.c | 592 +++++++++++++++++---------
> crypto/tlssession.c | 139 ++----
> crypto/trace-events | 1 +
> docs/about/deprecated.rst | 9 +
> docs/system/tls.rst | 134 +++++-
> include/crypto/tlscreds.h | 26 ++
> include/crypto/tlscredsx509.h | 6 +
> tests/unit/test-crypto-tlscredsx509.c | 8 +-
> ui/vnc.c | 9 +-
> 16 files changed, 849 insertions(+), 466 deletions(-)
> create mode 100644 crypto/tlscredsbox.c
> create mode 100644 crypto/tlscredsbox.h
>
> --
> 2.51.1
>
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 :|
^ permalink raw reply [flat|nested] 17+ messages in thread