Re: [Openvpn-devel] [PATCH] Log serial number of revoked certificate

[Lev S. <lst...@gm...>]

Hi,

Compiled and tested on Ubuntu 20.04, looks good.

A few nit-picks:

> +verify_check_crl_dir(const char *crl_dir, int cert_depth, openvpn_x509_cert_t *cert, char *subject)

The last parameter could benefit from const to indicate that function
is not going to modify it.


> -        msg(D_HANDSHAKE, "VERIFY CRL: certificate serial number %s is revoked", serial);
> +        msg(D_HANDSHAKE, "VERIFY CRL: depth=%d, %s, serial=%s is revoked",
> +            cert_depth, subject, serial);

Since you are modifying this line, could you add a NULL check to
serial to and pass
something like "<not available>" in this case?


> +            msg(D_TLS_ERRORS, "VERIFY ERROR: depth=%d, subject=%s, serial=%s: %s",
> +                cert_depth, subject, serial ? serial : "", errstr);

I would use "<not available>" in NULL case, otherwise the error
message becomes funny.


> +        msg(D_TLS_ERRORS, "VERIFY ERROR: depth=%d, error=%s: %s, serial=%s",
>              X509_STORE_CTX_get_error_depth(ctx),
>              X509_verify_cert_error_string(X509_STORE_CTX_get_error(ctx)),
> -            subject);
> +            subject, serial ? serial : "");

Same as above.