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

[Armin Burgmeier]

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?

Cheers,
Armin