crypto: fix lifecycle handling of gnutls credentials objects

As described in the previous commit, the gnutls credentials need to
be kept alive for as long as the gnutls session object exists. Convert
the QCryptoTLSCreds objects to use QCryptoTLSCredsBox and holding the
gnutls credential objects. When loading the credentials into a gnutls
session, store a reference to the box into the QCryptoTLSSession object.

This has the useful side effect that the QCryptoTLSSession code no
longer needs to know about all the different credential types, it can
use the generic pointer stored in the box.

Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
This commit is contained in:
Daniel P. Berrangé 2025-10-29 09:50:02 +00:00
parent 51e24d46e0
commit 70f9fd8dbf
6 changed files with 76 additions and 196 deletions

View file

@ -247,11 +247,8 @@ qcrypto_tls_creds_finalize(Object *obj)
QCryptoTLSCreds *creds = QCRYPTO_TLS_CREDS(obj);
#ifdef CONFIG_GNUTLS
if (creds->dh_params) {
gnutls_dh_params_deinit(creds->dh_params);
}
qcrypto_tls_creds_box_unref(creds->box);
#endif
g_free(creds->dir);
g_free(creds->priority);
}

View file

@ -36,6 +36,7 @@ static int
qcrypto_tls_creds_anon_load(QCryptoTLSCredsAnon *creds,
Error **errp)
{
g_autoptr(QCryptoTLSCredsBox) box = NULL;
g_autofree char *dhparams = NULL;
int ret;
@ -43,6 +44,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) {
box = qcrypto_tls_creds_box_new_server(GNUTLS_CRD_ANON);
if (creds->parent_obj.dir &&
qcrypto_tls_creds_get_path(&creds->parent_obj,
QCRYPTO_TLS_CREDS_DH_PARAMS,
@ -50,7 +53,7 @@ qcrypto_tls_creds_anon_load(QCryptoTLSCredsAnon *creds,
return -1;
}
ret = gnutls_anon_allocate_server_credentials(&creds->data.server);
ret = gnutls_anon_allocate_server_credentials(&box->data.anonserver);
if (ret < 0) {
error_setg(errp, "Cannot allocate credentials: %s",
gnutls_strerror(ret));
@ -58,42 +61,26 @@ qcrypto_tls_creds_anon_load(QCryptoTLSCredsAnon *creds,
}
if (qcrypto_tls_creds_get_dh_params_file(&creds->parent_obj, dhparams,
&creds->parent_obj.dh_params,
errp) < 0) {
&box->dh_params, errp) < 0) {
return -1;
}
gnutls_anon_set_server_dh_params(creds->data.server,
creds->parent_obj.dh_params);
gnutls_anon_set_server_dh_params(box->data.anonserver,
box->dh_params);
} else {
ret = gnutls_anon_allocate_client_credentials(&creds->data.client);
ret = gnutls_anon_allocate_client_credentials(&box->data.anonclient);
if (ret < 0) {
error_setg(errp, "Cannot allocate credentials: %s",
gnutls_strerror(ret));
return -1;
}
}
creds->parent_obj.box = g_steal_pointer(&box);
return 0;
}
static void
qcrypto_tls_creds_anon_unload(QCryptoTLSCredsAnon *creds)
{
if (creds->parent_obj.endpoint == QCRYPTO_TLS_CREDS_ENDPOINT_CLIENT) {
if (creds->data.client) {
gnutls_anon_free_client_credentials(creds->data.client);
creds->data.client = NULL;
}
} else {
if (creds->data.server) {
gnutls_anon_free_server_credentials(creds->data.server);
creds->data.server = NULL;
}
}
}
#else /* ! CONFIG_GNUTLS */
@ -105,13 +92,6 @@ qcrypto_tls_creds_anon_load(QCryptoTLSCredsAnon *creds G_GNUC_UNUSED,
}
static void
qcrypto_tls_creds_anon_unload(QCryptoTLSCredsAnon *creds G_GNUC_UNUSED)
{
/* nada */
}
#endif /* ! CONFIG_GNUTLS */
@ -124,15 +104,6 @@ qcrypto_tls_creds_anon_complete(UserCreatable *uc, Error **errp)
}
static void
qcrypto_tls_creds_anon_finalize(Object *obj)
{
QCryptoTLSCredsAnon *creds = QCRYPTO_TLS_CREDS_ANON(obj);
qcrypto_tls_creds_anon_unload(creds);
}
static void
qcrypto_tls_creds_anon_class_init(ObjectClass *oc, const void *data)
{
@ -148,7 +119,6 @@ static const TypeInfo qcrypto_tls_creds_anon_info = {
.parent = TYPE_QCRYPTO_TLS_CREDS,
.name = TYPE_QCRYPTO_TLS_CREDS_ANON,
.instance_size = sizeof(QCryptoTLSCredsAnon),
.instance_finalize = qcrypto_tls_creds_anon_finalize,
.class_size = sizeof(QCryptoTLSCredsAnonClass),
.class_init = qcrypto_tls_creds_anon_class_init,
.interfaces = (const InterfaceInfo[]) {

View file

@ -22,6 +22,7 @@
#define QCRYPTO_TLSCREDSPRIV_H
#include "crypto/tlscreds.h"
#include "crypto/tlscredsbox.h"
#ifdef CONFIG_GNUTLS
#include <gnutls/gnutls.h>
@ -31,39 +32,22 @@ struct QCryptoTLSCreds {
Object parent_obj;
char *dir;
QCryptoTLSCredsEndpoint endpoint;
#ifdef CONFIG_GNUTLS
gnutls_dh_params_t dh_params;
#endif
bool verifyPeer;
char *priority;
QCryptoTLSCredsBox *box;
};
struct QCryptoTLSCredsAnon {
QCryptoTLSCreds parent_obj;
#ifdef CONFIG_GNUTLS
union {
gnutls_anon_server_credentials_t server;
gnutls_anon_client_credentials_t client;
} data;
#endif
};
struct QCryptoTLSCredsPSK {
QCryptoTLSCreds parent_obj;
char *username;
#ifdef CONFIG_GNUTLS
union {
gnutls_psk_server_credentials_t server;
gnutls_psk_client_credentials_t client;
} data;
#endif
};
struct QCryptoTLSCredsX509 {
QCryptoTLSCreds parent_obj;
#ifdef CONFIG_GNUTLS
gnutls_certificate_credentials_t data;
#endif
bool sanityCheck;
char *passwordid;
};

View file

@ -71,6 +71,7 @@ static int
qcrypto_tls_creds_psk_load(QCryptoTLSCredsPSK *creds,
Error **errp)
{
g_autoptr(QCryptoTLSCredsBox) box = NULL;
g_autofree char *pskfile = NULL;
g_autofree char *dhparams = NULL;
const char *username;
@ -87,6 +88,8 @@ qcrypto_tls_creds_psk_load(QCryptoTLSCredsPSK *creds,
}
if (creds->parent_obj.endpoint == QCRYPTO_TLS_CREDS_ENDPOINT_SERVER) {
box = qcrypto_tls_creds_box_new_server(GNUTLS_CRD_PSK);
if (creds->username) {
error_setg(errp, "username should not be set when endpoint=server");
goto cleanup;
@ -101,7 +104,7 @@ qcrypto_tls_creds_psk_load(QCryptoTLSCredsPSK *creds,
goto cleanup;
}
ret = gnutls_psk_allocate_server_credentials(&creds->data.server);
ret = gnutls_psk_allocate_server_credentials(&box->data.pskserver);
if (ret < 0) {
error_setg(errp, "Cannot allocate credentials: %s",
gnutls_strerror(ret));
@ -109,20 +112,23 @@ qcrypto_tls_creds_psk_load(QCryptoTLSCredsPSK *creds,
}
if (qcrypto_tls_creds_get_dh_params_file(&creds->parent_obj, dhparams,
&creds->parent_obj.dh_params,
&box->dh_params,
errp) < 0) {
goto cleanup;
}
ret = gnutls_psk_set_server_credentials_file(creds->data.server, pskfile);
ret = gnutls_psk_set_server_credentials_file(box->data.pskserver,
pskfile);
if (ret < 0) {
error_setg(errp, "Cannot set PSK server credentials: %s",
gnutls_strerror(ret));
goto cleanup;
}
gnutls_psk_set_server_dh_params(creds->data.server,
creds->parent_obj.dh_params);
gnutls_psk_set_server_dh_params(box->data.pskserver,
box->dh_params);
} else {
box = qcrypto_tls_creds_box_new_client(GNUTLS_CRD_PSK);
if (qcrypto_tls_creds_get_path(&creds->parent_obj,
QCRYPTO_TLS_CREDS_PSKFILE,
true, &pskfile, errp) < 0) {
@ -138,14 +144,14 @@ qcrypto_tls_creds_psk_load(QCryptoTLSCredsPSK *creds,
goto cleanup;
}
ret = gnutls_psk_allocate_client_credentials(&creds->data.client);
ret = gnutls_psk_allocate_client_credentials(&box->data.pskclient);
if (ret < 0) {
error_setg(errp, "Cannot allocate credentials: %s",
gnutls_strerror(ret));
goto cleanup;
}
ret = gnutls_psk_set_client_credentials(creds->data.client,
ret = gnutls_psk_set_client_credentials(box->data.pskclient,
username, &key, GNUTLS_PSK_KEY_HEX);
if (ret < 0) {
error_setg(errp, "Cannot set PSK client credentials: %s",
@ -153,6 +159,7 @@ qcrypto_tls_creds_psk_load(QCryptoTLSCredsPSK *creds,
goto cleanup;
}
}
creds->parent_obj.box = g_steal_pointer(&box);
rv = 0;
cleanup:
@ -160,23 +167,6 @@ qcrypto_tls_creds_psk_load(QCryptoTLSCredsPSK *creds,
return rv;
}
static void
qcrypto_tls_creds_psk_unload(QCryptoTLSCredsPSK *creds)
{
if (creds->parent_obj.endpoint == QCRYPTO_TLS_CREDS_ENDPOINT_CLIENT) {
if (creds->data.client) {
gnutls_psk_free_client_credentials(creds->data.client);
creds->data.client = NULL;
}
} else {
if (creds->data.server) {
gnutls_psk_free_server_credentials(creds->data.server);
creds->data.server = NULL;
}
}
}
#else /* ! CONFIG_GNUTLS */
@ -188,13 +178,6 @@ qcrypto_tls_creds_psk_load(QCryptoTLSCredsPSK *creds G_GNUC_UNUSED,
}
static void
qcrypto_tls_creds_psk_unload(QCryptoTLSCredsPSK *creds G_GNUC_UNUSED)
{
/* nada */
}
#endif /* ! CONFIG_GNUTLS */
@ -212,7 +195,6 @@ qcrypto_tls_creds_psk_finalize(Object *obj)
{
QCryptoTLSCredsPSK *creds = QCRYPTO_TLS_CREDS_PSK(obj);
qcrypto_tls_creds_psk_unload(creds);
g_free(creds->username);
}

View file

@ -562,6 +562,7 @@ static int
qcrypto_tls_creds_x509_load(QCryptoTLSCredsX509 *creds,
Error **errp)
{
g_autoptr(QCryptoTLSCredsBox) box = NULL;
g_autofree char *cacert = NULL;
g_autofree char *cacrl = NULL;
g_autofree char *cert = NULL;
@ -578,6 +579,19 @@ qcrypto_tls_creds_x509_load(QCryptoTLSCredsX509 *creds,
trace_qcrypto_tls_creds_x509_load(creds, creds->parent_obj.dir);
if (creds->parent_obj.endpoint == QCRYPTO_TLS_CREDS_ENDPOINT_SERVER) {
box = qcrypto_tls_creds_box_new_server(GNUTLS_CRD_CERTIFICATE);
} else {
box = qcrypto_tls_creds_box_new_client(GNUTLS_CRD_CERTIFICATE);
}
ret = gnutls_certificate_allocate_credentials(&box->data.cert);
if (ret < 0) {
error_setg(errp, "Cannot allocate credentials: '%s'",
gnutls_strerror(ret));
return -1;
}
if (qcrypto_tls_creds_get_path(&creds->parent_obj,
QCRYPTO_TLS_CREDS_X509_CA_CERT,
true, &cacert, errp) < 0) {
@ -616,14 +630,7 @@ qcrypto_tls_creds_x509_load(QCryptoTLSCredsX509 *creds,
return -1;
}
ret = gnutls_certificate_allocate_credentials(&creds->data);
if (ret < 0) {
error_setg(errp, "Cannot allocate credentials: '%s'",
gnutls_strerror(ret));
return -1;
}
ret = gnutls_certificate_set_x509_trust_file(creds->data,
ret = gnutls_certificate_set_x509_trust_file(box->data.cert,
cacert,
GNUTLS_X509_FMT_PEM);
if (ret < 0) {
@ -641,7 +648,7 @@ qcrypto_tls_creds_x509_load(QCryptoTLSCredsX509 *creds,
return -1;
}
}
ret = gnutls_certificate_set_x509_key_file2(creds->data,
ret = gnutls_certificate_set_x509_key_file2(box->data.cert,
cert, key,
GNUTLS_X509_FMT_PEM,
password,
@ -654,7 +661,7 @@ qcrypto_tls_creds_x509_load(QCryptoTLSCredsX509 *creds,
}
if (cacrl != NULL) {
ret = gnutls_certificate_set_x509_crl_file(creds->data,
ret = gnutls_certificate_set_x509_crl_file(box->data.cert,
cacrl,
GNUTLS_X509_FMT_PEM);
if (ret < 0) {
@ -666,28 +673,18 @@ qcrypto_tls_creds_x509_load(QCryptoTLSCredsX509 *creds,
if (isServer) {
if (qcrypto_tls_creds_get_dh_params_file(&creds->parent_obj, dhparams,
&creds->parent_obj.dh_params,
&box->dh_params,
errp) < 0) {
return -1;
}
gnutls_certificate_set_dh_params(creds->data,
creds->parent_obj.dh_params);
gnutls_certificate_set_dh_params(box->data.cert, box->dh_params);
}
creds->parent_obj.box = g_steal_pointer(&box);
return 0;
}
static void
qcrypto_tls_creds_x509_unload(QCryptoTLSCredsX509 *creds)
{
if (creds->data) {
gnutls_certificate_free_credentials(creds->data);
creds->data = NULL;
}
}
#else /* ! CONFIG_GNUTLS */
@ -699,13 +696,6 @@ qcrypto_tls_creds_x509_load(QCryptoTLSCredsX509 *creds G_GNUC_UNUSED,
}
static void
qcrypto_tls_creds_x509_unload(QCryptoTLSCredsX509 *creds G_GNUC_UNUSED)
{
/* nada */
}
#endif /* ! CONFIG_GNUTLS */
@ -768,29 +758,17 @@ qcrypto_tls_creds_x509_reload(QCryptoTLSCreds *creds, Error **errp)
{
QCryptoTLSCredsX509 *x509_creds = QCRYPTO_TLS_CREDS_X509(creds);
Error *local_err = NULL;
gnutls_certificate_credentials_t creds_data = x509_creds->data;
gnutls_dh_params_t creds_dh_params = creds->dh_params;
QCryptoTLSCredsBox *creds_box = creds->box;
x509_creds->data = NULL;
creds->dh_params = NULL;
creds->box = NULL;
qcrypto_tls_creds_x509_load(x509_creds, &local_err);
if (local_err) {
qcrypto_tls_creds_x509_unload(x509_creds);
if (creds->dh_params) {
gnutls_dh_params_deinit(creds->dh_params);
}
x509_creds->data = creds_data;
creds->dh_params = creds_dh_params;
creds->box = creds_box;
error_propagate(errp, local_err);
return false;
}
if (creds_data) {
gnutls_certificate_free_credentials(creds_data);
}
if (creds_dh_params) {
gnutls_dh_params_deinit(creds_dh_params);
}
qcrypto_tls_creds_box_unref(creds_box);
return true;
}
@ -823,7 +801,6 @@ qcrypto_tls_creds_x509_finalize(Object *obj)
QCryptoTLSCredsX509 *creds = QCRYPTO_TLS_CREDS_X509(obj);
g_free(creds->passwordid);
qcrypto_tls_creds_x509_unload(creds);
}

View file

@ -38,6 +38,7 @@
struct QCryptoTLSSession {
QCryptoTLSCreds *creds;
QCryptoTLSCredsBox *credsbox;
gnutls_session_t handle;
char *hostname;
char *authzid;
@ -78,6 +79,7 @@ qcrypto_tls_session_free(QCryptoTLSSession *session)
g_free(session->hostname);
g_free(session->peername);
g_free(session->authzid);
qcrypto_tls_creds_box_unref(session->credsbox);
object_unref(OBJECT(session->creds));
qemu_mutex_destroy(&session->lock);
g_free(session);
@ -206,65 +208,33 @@ qcrypto_tls_session_new(QCryptoTLSCreds *creds,
goto error;
}
if (object_dynamic_cast(OBJECT(creds),
TYPE_QCRYPTO_TLS_CREDS_ANON)) {
QCryptoTLSCredsAnon *acreds = QCRYPTO_TLS_CREDS_ANON(creds);
if (creds->endpoint == QCRYPTO_TLS_CREDS_ENDPOINT_SERVER) {
ret = gnutls_credentials_set(session->handle,
GNUTLS_CRD_ANON,
acreds->data.server);
} else {
ret = gnutls_credentials_set(session->handle,
GNUTLS_CRD_ANON,
acreds->data.client);
}
if (ret < 0) {
error_setg(errp, "Cannot set session credentials: %s",
gnutls_strerror(ret));
goto error;
}
} else if (object_dynamic_cast(OBJECT(creds),
TYPE_QCRYPTO_TLS_CREDS_PSK)) {
QCryptoTLSCredsPSK *pcreds = QCRYPTO_TLS_CREDS_PSK(creds);
if (creds->endpoint == QCRYPTO_TLS_CREDS_ENDPOINT_SERVER) {
ret = gnutls_credentials_set(session->handle,
GNUTLS_CRD_PSK,
pcreds->data.server);
} else {
ret = gnutls_credentials_set(session->handle,
GNUTLS_CRD_PSK,
pcreds->data.client);
}
if (ret < 0) {
error_setg(errp, "Cannot set session credentials: %s",
gnutls_strerror(ret));
goto error;
}
} else if (object_dynamic_cast(OBJECT(creds),
TYPE_QCRYPTO_TLS_CREDS_X509)) {
QCryptoTLSCredsX509 *tcreds = QCRYPTO_TLS_CREDS_X509(creds);
ret = gnutls_credentials_set(session->handle,
GNUTLS_CRD_CERTIFICATE,
tcreds->data);
if (ret < 0) {
error_setg(errp, "Cannot set session credentials: %s",
gnutls_strerror(ret));
goto error;
}
if (creds->endpoint == QCRYPTO_TLS_CREDS_ENDPOINT_SERVER) {
/* This requests, but does not enforce a client cert.
* The cert checking code later does enforcement */
gnutls_certificate_server_set_request(session->handle,
GNUTLS_CERT_REQUEST);
}
} else {
error_setg(errp, "Unsupported TLS credentials type %s",
object_get_typename(OBJECT(creds)));
ret = gnutls_credentials_set(session->handle,
creds->box->type,
creds->box->data.any);
if (ret < 0) {
error_setg(errp, "Cannot set session credentials: %s",
gnutls_strerror(ret));
goto error;
}
/*
* creds->box->data.any must be kept alive for as long
* as the gnutls_session_t is alive, so acquire a ref
*/
qcrypto_tls_creds_box_ref(creds->box);
session->credsbox = creds->box;
if (object_dynamic_cast(OBJECT(creds),
TYPE_QCRYPTO_TLS_CREDS_X509) &&
creds->endpoint == QCRYPTO_TLS_CREDS_ENDPOINT_SERVER) {
/*
* This requests, but does not enforce a client cert.
* The cert checking code later does enforcement
*/
gnutls_certificate_server_set_request(session->handle,
GNUTLS_CERT_REQUEST);
}
gnutls_transport_set_ptr(session->handle, session);
gnutls_transport_set_push_function(session->handle,
qcrypto_tls_session_push);