On Thu, Oct 30, 2025 at 6:50 PM Daniel P. Berrangé <[email protected]> wrote:
> The x509 TLS credentials code will load the CA certs once to perform > sanity chcking on the certs, then discard the certificate objects > and let gnutls load them a second time. > > This introduces a new QCryptoTLSCredsX509Files struct which will > hold the CA certificates loaded for sanity checking and pass them on > to gnutls, avoiding the duplicated loading. > > Signed-off-by: Daniel P. Berrangé <[email protected]> > Reviewed-by: Marc-André Lureau <[email protected]> > --- > crypto/tlscredsx509.c | 141 ++++++++++++++++++++++++++---------------- > 1 file changed, 87 insertions(+), 54 deletions(-) > > diff --git a/crypto/tlscredsx509.c b/crypto/tlscredsx509.c > index 7e79af4266..6a830af50d 100644 > --- a/crypto/tlscredsx509.c > +++ b/crypto/tlscredsx509.c > @@ -40,6 +40,35 @@ struct QCryptoTLSCredsX509 { > }; > > > +typedef struct QCryptoTLSCredsX509Files QCryptoTLSCredsX509Files; > +struct QCryptoTLSCredsX509Files { > + char *cacertpath; > + gnutls_x509_crt_t *cacerts; > + unsigned int ncacerts; > +}; > + > +static QCryptoTLSCredsX509Files * > +qcrypto_tls_creds_x509_files_new(void) > +{ > + return g_new0(QCryptoTLSCredsX509Files, 1); > +} > + > + > +static void > +qcrypto_tls_creds_x509_files_free(QCryptoTLSCredsX509Files *files) > +{ > + size_t i; > + for (i = 0; i < files->ncacerts; i++) { > + gnutls_x509_crt_deinit(files->cacerts[i]); > + } > + g_free(files->cacerts); > + g_free(files->cacertpath); > + g_free(files); > +} > + > +G_DEFINE_AUTOPTR_CLEANUP_FUNC(QCryptoTLSCredsX509Files, > + qcrypto_tls_creds_x509_files_free); > + > static int > qcrypto_tls_creds_check_cert_times(gnutls_x509_crt_t cert, > const char *certFile, > @@ -315,11 +344,9 @@ qcrypto_tls_creds_check_cert(QCryptoTLSCredsX509 > *creds, > > static int > qcrypto_tls_creds_check_authority_chain(QCryptoTLSCredsX509 *creds, > + QCryptoTLSCredsX509Files *files, > gnutls_x509_crt_t *certs, > unsigned int ncerts, > - gnutls_x509_crt_t *cacerts, > - unsigned int ncacerts, > - const char *cacertFile, > bool isServer, > Error **errp) > { > @@ -360,13 +387,13 @@ > qcrypto_tls_creds_check_authority_chain(QCryptoTLSCredsX509 *creds, > * reached the root of trust. > */ > return qcrypto_tls_creds_check_cert( > - creds, cert_to_check, cacertFile, > + creds, cert_to_check, files->cacertpath, > isServer, true, errp); > } > - for (int i = 0; i < ncacerts; i++) { > + for (int i = 0; i < files->ncacerts; i++) { > if (gnutls_x509_crt_check_issuer(cert_to_check, > - cacerts[i])) { > - cert_issuer = cacerts[i]; > + files->cacerts[i])) { > + cert_issuer = files->cacerts[i]; > break; > } > } > @@ -374,7 +401,7 @@ > qcrypto_tls_creds_check_authority_chain(QCryptoTLSCredsX509 *creds, > break; > } > > - if (qcrypto_tls_creds_check_cert(creds, cert_issuer, cacertFile, > + if (qcrypto_tls_creds_check_cert(creds, cert_issuer, > files->cacertpath, > isServer, true, errp) < 0) { > return -1; > } > @@ -394,19 +421,17 @@ > qcrypto_tls_creds_check_authority_chain(QCryptoTLSCredsX509 *creds, > } > > static int > -qcrypto_tls_creds_check_cert_pair(gnutls_x509_crt_t *certs, > +qcrypto_tls_creds_check_cert_pair(QCryptoTLSCredsX509Files *files, > + gnutls_x509_crt_t *certs, > size_t ncerts, > const char *certFile, > - gnutls_x509_crt_t *cacerts, > - size_t ncacerts, > - const char *cacertFile, > bool isServer, > Error **errp) > { > unsigned int status; > > if (gnutls_x509_crt_list_verify(certs, ncerts, > - cacerts, ncacerts, > + files->cacerts, files->ncacerts, > NULL, 0, > 0, &status) < 0) { > error_setg(errp, isServer ? > @@ -414,7 +439,7 @@ qcrypto_tls_creds_check_cert_pair(gnutls_x509_crt_t > *certs, > "CA certificate %s" : > "Unable to verify client certificate %s against " > "CA certificate %s", > - certFile, cacertFile); > + certFile, files->cacertpath); > return -1; > } > > @@ -439,7 +464,7 @@ qcrypto_tls_creds_check_cert_pair(gnutls_x509_crt_t > *certs, > > error_setg(errp, > "Our own certificate %s failed validation against %s: > %s", > - certFile, cacertFile, reason); > + certFile, files->cacertpath, reason); > return -1; > } > > @@ -490,15 +515,13 @@ qcrypto_tls_creds_load_cert_list(QCryptoTLSCredsX509 > *creds, > > static int > qcrypto_tls_creds_x509_sanity_check(QCryptoTLSCredsX509 *creds, > + QCryptoTLSCredsX509Files *files, > bool isServer, > - const char *cacertFile, > const char *certFile, > Error **errp) > { > gnutls_x509_crt_t *certs = NULL; > unsigned int ncerts = 0; > - gnutls_x509_crt_t *cacerts = NULL; > - unsigned int ncacerts = 0; > size_t i; > int ret = -1; > > @@ -514,16 +537,6 @@ > qcrypto_tls_creds_x509_sanity_check(QCryptoTLSCredsX509 *creds, > } > } > > - if (qcrypto_tls_creds_load_cert_list(creds, > - cacertFile, > - &cacerts, > - &ncacerts, > - isServer, > - true, > - errp) < 0) { > - goto cleanup; > - } > - > for (i = 0; i < ncerts; i++) { > if (qcrypto_tls_creds_check_cert(creds, > certs[i], certFile, > @@ -533,17 +546,14 @@ > qcrypto_tls_creds_x509_sanity_check(QCryptoTLSCredsX509 *creds, > } > > if (ncerts && > - qcrypto_tls_creds_check_authority_chain(creds, > + qcrypto_tls_creds_check_authority_chain(creds, files, > certs, ncerts, > - cacerts, ncacerts, > - cacertFile, isServer, > - errp) < 0) { > + isServer, errp) < 0) { > goto cleanup; > } > > - if (ncerts && ncacerts && > - qcrypto_tls_creds_check_cert_pair(certs, ncerts, certFile, > - cacerts, ncacerts, cacertFile, > + if (ncerts && > + qcrypto_tls_creds_check_cert_pair(files, certs, ncerts, certFile, > isServer, errp) < 0) { > goto cleanup; > } > @@ -555,21 +565,53 @@ > qcrypto_tls_creds_x509_sanity_check(QCryptoTLSCredsX509 *creds, > gnutls_x509_crt_deinit(certs[i]); > } > g_free(certs); > - for (i = 0; i < ncacerts; i++) { > - gnutls_x509_crt_deinit(cacerts[i]); > - } > - g_free(cacerts); > > return ret; > } > > > +static int > +qcrypto_tls_creds_x509_load_ca(QCryptoTLSCredsX509 *creds, > + QCryptoTLSCredsBox *box, > + QCryptoTLSCredsX509Files *files, > + bool isServer, > + Error **errp) > +{ > + int ret; > + > + if (qcrypto_tls_creds_get_path(&creds->parent_obj, > + QCRYPTO_TLS_CREDS_X509_CA_CERT, > + true, &files->cacertpath, errp) < 0) { > + return -1; > + } > + > + if (qcrypto_tls_creds_load_cert_list(creds, > + files->cacertpath, > + &files->cacerts, > + &files->ncacerts, > + isServer, > + true, > + errp) < 0) { > + return -1; > + } > + > + ret = gnutls_certificate_set_x509_trust(box->data.cert, > + files->cacerts, > files->ncacerts); > + if (ret < 0) { > + error_setg(errp, "Cannot set CA certificate '%s': %s", > + files->cacertpath, gnutls_strerror(ret)); > + return -1; > + } > + > + return 0; > +} > + > static int > qcrypto_tls_creds_x509_load(QCryptoTLSCredsX509 *creds, > Error **errp) > { > g_autoptr(QCryptoTLSCredsBox) box = NULL; > - g_autofree char *cacert = NULL; > + g_autoptr(QCryptoTLSCredsX509Files) files = NULL; > g_autofree char *cacrl = NULL; > g_autofree char *cert = NULL; > g_autofree char *key = NULL; > @@ -598,9 +640,9 @@ qcrypto_tls_creds_x509_load(QCryptoTLSCredsX509 *creds, > return -1; > } > > - if (qcrypto_tls_creds_get_path(&creds->parent_obj, > - QCRYPTO_TLS_CREDS_X509_CA_CERT, > - true, &cacert, errp) < 0) { > + files = qcrypto_tls_creds_x509_files_new(); > + > + if (qcrypto_tls_creds_x509_load_ca(creds, box, files, isServer, errp) > < 0) { > return -1; > } > > @@ -631,17 +673,8 @@ qcrypto_tls_creds_x509_load(QCryptoTLSCredsX509 > *creds, > } > > if (creds->sanityCheck && > - qcrypto_tls_creds_x509_sanity_check(creds, isServer, > - cacert, cert, errp) < 0) { > - return -1; > - } > - > - ret = gnutls_certificate_set_x509_trust_file(box->data.cert, > - cacert, > - GNUTLS_X509_FMT_PEM); > - if (ret < 0) { > - error_setg(errp, "Cannot load CA certificate '%s': %s", > - cacert, gnutls_strerror(ret)); > + qcrypto_tls_creds_x509_sanity_check(creds, files, isServer, > + cert, errp) < 0) { > return -1; > } > > -- > 2.51.1 > >
