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é <[email protected]> > > --- > > 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 :|
