[gnutls-devel] [PATCH] Check for all error conditions when verifying a certificate

[Nikos Mavrogiannopoulos]

On Fri, 2014-09-12 at 14:39 -0400, Armin Burgmeier wrote:
> This is an attempt at a patch for what we discussed in the other email
> thread. I have made sure that the tests still pass (with minor
> modifications where additional failure reasons are now expected). Since
> this is in a quite sensitive part of the code it would be good to have
> this reviewed carefully...
> 
> > @@ -1504,10 +1488,10 @@ gnutls_x509_crl_verify(gnutls_x509_crl_t crl,
> >  		*verify |= GNUTLS_CERT_REVOCATION_DATA_SUPERSEDED;
> >  
> > 
> > -      cleanup:
> >  	if (verify)
> >  		*verify |= GNUTLS_CERT_INVALID;
> >  
> > +      cleanup:
> >  	_gnutls_free_datum(&crl_signed_data);
> >  	_gnutls_free_datum(&crl_signature);
> 
> I have two comments to this hunk.
> 
>  1) the question where to put the cleanup label is basically what should
> happen with the *verify output parameter when the function itself does
> not return GNUTLS_E_SUCCESS. Probably it does not matter because when
> the function does not run successfully, the verification output is of no
> good use anyway.
> 
>  2) If I understand the code correctly, then the verify result will
> *always* have the GNUTLS_CERT_INVALID flag set, even for perfectly valid
> CRLs. I have the impression the check should read "if(verify && result !
> = 0)" instead of just "if(verify)"... Is there a unit test for this
> function?

That is indeed correct. The check should have been if (verify &&
*verify) ... It seems we need some unit tests for that one.

regards,
Nikos


> 
> Cheers,
> Armin
> 
> 
> _______________________________________________
> Gnutls-devel mailing list
> Gnutls-devel at lists.gnutls.org
> http://lists.gnupg.org/mailman/listinfo/gnutls-devel