[Spice-devel] [spice-common 2/3] ssl: Log an error when peer certificate verification failed

[Uri Lublin]

On 09/20/2013 06:07 PM, Christophe Fergeau wrote:
> We currently log an error when openssl_verify() is called with
> preverify_ok set to 0 for all certificates in the certificate chain
> except for the peer certificate (when 'depth' is 0).
> This commit logs an error in the latter case as well.
> ---
>   common/ssl_verify.c | 10 +++++++++-
>   1 file changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/common/ssl_verify.c b/common/ssl_verify.c
> index d4b89f0..7af78bc 100644
> --- a/common/ssl_verify.c
> +++ b/common/ssl_verify.c
> @@ -456,8 +456,16 @@ static int openssl_verify(int preverify_ok, X509_STORE_CTX *ctx)
>               failed_verifications |= SPICE_SSL_VERIFY_OP_PUBKEY;
>       }
>   
> -    if (!v->all_preverify_ok || !preverify_ok)
> +    if (!preverify_ok) {
> +        err = X509_STORE_CTX_get_error(ctx);
> +        depth = X509_STORE_CTX_get_error_depth(ctx);
> +        spice_warning("Error in server certificate verification: %s (num=%d:depth%d:%s)",
> +                      X509_verify_cert_error_string(err), err, depth, buf);
>           return 0;
> +    }
> +    if (!v->all_preverify_ok) {
> +        return 0;
> +    }

Hi Christophe,

Nitpick1: if !all_preverfiy_ok then something has already failed and 
reported. Maybe it's better to
switch those new ifs (to not report error twice).
Nitpick2: err and depth are already set in the beginning of the function.
Nitpick3: Maybe it's better to move this code above the check for 
verify_pubkey

Thanks,
     Uri.