[PATCH] Relax X509 CA cert sanity checking

* [PATCH] Relax X509 CA cert sanity checking
@ 2021-12-14 11:39 Henry Kleynhans
  2021-12-14 13:04 ` Philippe Mathieu-Daudé
  2021-12-14 13:16 ` Daniel P. Berrangé
  0 siblings, 2 replies; 6+ messages in thread
From: Henry Kleynhans @ 2021-12-14 11:39 UTC (permalink / raw)
  To: qemu-devel; +Cc: Henry Kleynhans, henry.kleynhans

From: Henry Kleynhans <hkleynhans@fb.com>

The sanity checking function attempts to validate all the certificates
in the provided CA file.  These checks are performed on certificates
which may or may not be part of the signing chain and duplicates checks
that should be performed by the TLS library.

In real life this causes a problem if the certificate chain I want to
use is valid, but there exist another expired certificate in the CA
file.

This patch relaxes the sanity checks to only ensure we have at least one
valid certificate in the CA certificate file and leave the actual
validation to the TLS library.

Signed-off-by: Henry Kleynhans <hkleynhans@fb.com>
---
 crypto/tlscredsx509.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/crypto/tlscredsx509.c b/crypto/tlscredsx509.c
index 32948a6bdc..fb056f96a2 100644
--- a/crypto/tlscredsx509.c
+++ b/crypto/tlscredsx509.c
@@ -473,6 +473,7 @@ qcrypto_tls_creds_x509_sanity_check(QCryptoTLSCredsX509 *creds,
     gnutls_x509_crt_t cert = NULL;
     gnutls_x509_crt_t cacerts[MAX_CERTS];
     size_t ncacerts = 0;
+    size_t nvalidca = 0;
     size_t i;
     int ret = -1;
 
@@ -505,11 +506,15 @@ qcrypto_tls_creds_x509_sanity_check(QCryptoTLSCredsX509 *creds,
     for (i = 0; i < ncacerts; i++) {
         if (qcrypto_tls_creds_check_cert(creds,
                                          cacerts[i], cacertFile,
-                                         isServer, true, errp) < 0) {
-            goto cleanup;
+                                         isServer, true, errp) == 0) {
+            ++nvalidca;
         }
     }
 
+    if (nvalidca == 0) {
+        goto cleanup;
+    }
+
     if (cert && ncacerts &&
         qcrypto_tls_creds_check_cert_pair(cert, certFile, cacerts,
                                           ncacerts, cacertFile,
-- 
2.34.1



^ permalink raw reply related	[flat|nested] 6+ messages in thread