Re: [PATCH v2 6/6] crypto: fix error reporting in cert chain checks

["Daniel P. Berrangé" <berrange@redhat.com>]

On Thu, Oct 16, 2025 at 10:50:44AM -0500, Eric Blake wrote:
> On Fri, Sep 19, 2025 at 11:10:22AM +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
> > than is not present in the chain. In this case, the outer loop
> 
> s/than/that/
> 
> > '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 | 32 +++++++++++++++++++++-----------
> >  1 file changed, 21 insertions(+), 11 deletions(-)
> > 
> > diff --git a/crypto/tlscredsx509.c b/crypto/tlscredsx509.c
> > index 89a8e261d5..d42f2afaea 100644
> > --- a/crypto/tlscredsx509.c
> > +++ b/crypto/tlscredsx509.c
> > @@ -319,7 +319,6 @@ qcrypto_tls_creds_check_authority_chain(QCryptoTLSCredsX509 *creds,
> >                                          Error **errp)
> >  {
> >      gnutls_x509_crt_t cert_to_check = certs[ncerts - 1];
> > -    int checking_issuer = 1;
> 
> This was the line I mentioned in patch 1/6 as needing a bool.  Should
> this cleanup be squashed into that patch rather than having churn in
> the series?
> 
> >      int retval = 0;
> >      gnutls_datum_t dn = {}, dnissuer = {};
> >
> 
> Should there be a testsuite patch along with this to provoke that
> particular failure scenario?

Hmm, the test failure is introduced by patch #3 and this patch then
fixes it. IOW, I (reluctantly) need to squash this code into #3.

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|