crypto: fix error reporting in cert chain checks

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.

Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>

diff --git a/crypto/tlscredsx509.c b/crypto/tlscredsx509.c
index 4169ad9a75e10811ed13eb8c233598baa448e3ba..2eccd71b3c049e42732912df668497dd06053990 100644 (file)
--- 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);
+    g_free(dn.data);
     return -1;
 }
 

diff --git a/tests/unit/test-crypto-tlscredsx509.c b/tests/unit/test-crypto-tlscredsx509.c
index 85f51aee1b517e06fb18c1133e163142b120a928..7c5df32bccfcbb6df587c8865a4d1783e783f25a 100644 (file)
--- 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(chainincomplete, true,
+                 WORKDIR "cacertchain-incomplete-ctx.pem",
+                 servercertlevel3areq.filename, true);
 
     /* Some missing certs - first two are fatal, the last
      * is ok