> > }
> >
> > void
> > +make_empty_crl(struct tls_root_ctx *ctx)
> > +{
> > + if (ctx->crl == NULL)
> > + {
> > + ALLOC_OBJ_CLEAR(ctx->crl, mbedtls_x509_crl);
> > + }
> > + else
> > + {
> > + mbedtls_x509_crl_free(ctx->crl);
> > + }
> > +}
> > +
>
> This function is confsung me. This needs at least more documentation
> what it is doing as docstring etc. I would also expect to have
> mbedtls_x509_crl_init to init the struct and not just a malloc that set
> the whole structure to zero. And mbedtls_x509_crl_free does from its
> description doesn't guarantee that the object is left in a proper state.
All mbedtls_x509_crl_init() does is set the struct to 0, but I agree it's
better from a readability standpoint to call it anyway. Another function in
the file also uses ALLOC_OBJ_CLEAR instead of the init function; I'll fix
that too.
When OpenVPN fails to parse a CRL file, it also calls mbedtls_x509_crl_free()
and leaves it at that (see backend_tls_ctx_reload_crl()). This leaves it in
a state where all connection attempts are rejected. The same happens when you
initialize a CRL struct and then don't do anything further with it. So that's
what I did in this function. You're right, this needs more comments. If we want
to do it that way at all.
As far as I'm aware, none of that is documented. Actually, if I understand
correctly,
you're not even supposed to touch the mbedtls_ssl_config struct (which contains
a pointer to the CRL) after calling mbedtls_ssl_setup(). So far, reloading the
CRL
has worked anyway. It looks like the code in ssl_mbedtls.c needs some rewriting
if
we want to do it properly. I'll need to think about that some more.
_______________________________________________
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
