[PATCH v3 4/7] crypto: fix error reporting in cert chain checks

Daniel P. Berrangé posted 7 patches 3 weeks, 4 days ago
Maintainers: "Daniel P. Berrangé" <berrange@redhat.com>
[PATCH v3 4/7] crypto: fix error reporting in cert chain checks
Posted by Daniel P. Berrangé 3 weeks, 4 days ago
The loop that checks the CA certificate chain can fail to report
an error message if one of the certs in the chain has an issuer
that is not present in the chain. In this case, the outer loop
'while (checking_issuer)' will terminate after failing to find
the issuer, and no error message will be reported.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 crypto/tlscredsx509.c                 | 33 ++++++++++++++++++---------
 tests/unit/test-crypto-tlscredsx509.c | 12 ++++++++++
 2 files changed, 34 insertions(+), 11 deletions(-)

diff --git a/crypto/tlscredsx509.c b/crypto/tlscredsx509.c
index 4169ad9a75..e1f82f1cdc 100644
--- a/crypto/tlscredsx509.c
+++ b/crypto/tlscredsx509.c
@@ -326,11 +326,11 @@ qcrypto_tls_creds_check_authority_chain(QCryptoTLSCredsX509 *creds,
                                         Error **errp)
 {
     gnutls_x509_crt_t cert_to_check = cert;
-    bool checking_issuer = true;
     int retval = 0;
+    gnutls_datum_t dn = {};
 
-    while (checking_issuer) {
-        checking_issuer = false;
+    for (;;) {
+        gnutls_x509_crt_t cert_issuer = NULL;
 
         if (gnutls_x509_crt_check_issuer(cert_to_check,
                                          cert_to_check)) {
@@ -345,19 +345,30 @@ qcrypto_tls_creds_check_authority_chain(QCryptoTLSCredsX509 *creds,
         for (int i = 0; i < ncacerts; i++) {
             if (gnutls_x509_crt_check_issuer(cert_to_check,
                                              cacerts[i])) {
-                retval = qcrypto_tls_creds_check_cert(
-                    creds, cacerts[i], cacertFile,
-                    isServer, isCA, errp);
-                if (retval < 0) {
-                    return retval;
-                }
-                cert_to_check = cacerts[i];
-                checking_issuer = true;
+                cert_issuer = cacerts[i];
                 break;
             }
         }
+        if (!cert_issuer) {
+            break;
+        }
+
+        if (qcrypto_tls_creds_check_cert(creds, cert_issuer, cacertFile,
+                                         isServer, isCA, errp) < 0) {
+            return -1;
+        }
+
+        cert_to_check = cert_issuer;
     }
 
+    retval = gnutls_x509_crt_get_dn2(cert_to_check, &dn);
+    if (retval < 0) {
+        error_setg(errp, "Unable to fetch cert DN: %s",
+                   gnutls_strerror(retval));
+        return -1;
+    }
+    error_setg(errp, "Cert '%s' has no issuer in CA chain", dn.data);
+    gnutls_free(dn.data);
     return -1;
 }
 
diff --git a/tests/unit/test-crypto-tlscredsx509.c b/tests/unit/test-crypto-tlscredsx509.c
index 85f51aee1b..5f78fe7d6f 100644
--- a/tests/unit/test-crypto-tlscredsx509.c
+++ b/tests/unit/test-crypto-tlscredsx509.c
@@ -643,6 +643,15 @@ int main(int argc, char **argv)
                               certchain_with_invalid,
                               G_N_ELEMENTS(certchain_with_invalid));
 
+    gnutls_x509_crt_t certchain_incomplete[] = {
+        cacertrootreq.crt,
+        cacertlevel2areq.crt,
+    };
+
+    test_tls_write_cert_chain(WORKDIR "cacertchain-incomplete-ctx.pem",
+                              certchain_incomplete,
+                              G_N_ELEMENTS(certchain_incomplete));
+
     TLS_TEST_REG(chain1, true,
                  WORKDIR "cacertchain-ctx.pem",
                  servercertlevel3areq.filename, false);
@@ -652,6 +661,9 @@ int main(int argc, char **argv)
     TLS_TEST_REG(certchainwithexpiredcert, false,
                  WORKDIR "cacertchain-with-invalid-ctx.pem",
                  clientcertlevel2breq.filename, false);
+    TLS_TEST_REG(chainincomlpete, true,
+                 WORKDIR "cacertchain-incomplete-ctx.pem",
+                 servercertlevel3areq.filename, true);
 
     /* Some missing certs - first two are fatal, the last
      * is ok
-- 
2.50.1


Re: [PATCH v3 4/7] crypto: fix error reporting in cert chain checks
Posted by Eric Blake 3 weeks, 3 days ago
On Mon, Oct 20, 2025 at 03:12:34PM +0100, Daniel P. Berrangé wrote:
> The loop that checks the CA certificate chain can fail to report
> an error message if one of the certs in the chain has an issuer
> that is not present in the chain. In this case, the outer loop
> 'while (checking_issuer)' will terminate after failing to find
> the issuer, and no error message will be reported.
> 
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>  crypto/tlscredsx509.c                 | 33 ++++++++++++++++++---------
>  tests/unit/test-crypto-tlscredsx509.c | 12 ++++++++++
>  2 files changed, 34 insertions(+), 11 deletions(-)
> 

> diff --git a/tests/unit/test-crypto-tlscredsx509.c b/tests/unit/test-crypto-tlscredsx509.c
> index 85f51aee1b..5f78fe7d6f 100644
> --- a/tests/unit/test-crypto-tlscredsx509.c

> @@ -652,6 +661,9 @@ int main(int argc, char **argv)
>      TLS_TEST_REG(certchainwithexpiredcert, false,
>                   WORKDIR "cacertchain-with-invalid-ctx.pem",
>                   clientcertlevel2breq.filename, false);
> +    TLS_TEST_REG(chainincomlpete, true,

chainincomplete

> +                 WORKDIR "cacertchain-incomplete-ctx.pem",
> +                 servercertlevel3areq.filename, true);

With the typo fixed,

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.
Virtualization:  qemu.org | libguestfs.org