On Fri, Jan 20, 2023 at 09:13:04PM +0000, Job Snijders wrote:
> On Fri, Jan 20, 2023 at 09:35:08PM +0100, Theo Buehler wrote:
> > On Fri, Jan 20, 2023 at 08:06:00PM +0000, Job Snijders wrote:
> > > While studying why X509_check_ca() is the ugly thing it is, tb@
> > > suggested x509v3_cache_extensions() might benefit from a wrapper to
> > > avoid duplication of locking and checking the stupid EXFLAG_INVALID
> > > flag. x509v3_cache_extensions() isn't a public function anyway.
> > >
> > > Passes regress & rpki-client.
> > >
> > > OK?
> >
> > Cool, thanks. I think it's better to pull the EXFLAG_SET check into the
> > new function like beck has done in x509_verify_cert_cache_extensions()
> > which then becomes a simple wrapper (it's nice to keep the 'namespacing'
> > clean in x509_verify.c).
> >
> > Details inline.
>
> Thanks for the feedback!
>
> I think all your points have been addressed in the below changeset.
Yes that's great, thank you.
ok tb
with the two small nits below.
> +/*
> + * 1 is success, 0 is failure.
> + */
Please remove this comment. This is the default in libcrypto.
> +int
> +x509v3_cache_extensions(X509 *x)
> +{
> + if (!(x->ex_flags & EXFLAG_SET)) {
This is not really a Boolean, so please check explicitly against 0 like
a few lines down.
if ((x->ex_flags & EXFLAG_SET) == 0) {
> + CRYPTO_w_lock(CRYPTO_LOCK_X509);
> + x509v3_cache_extensions_internal(x);
> + CRYPTO_w_unlock(CRYPTO_LOCK_X509);
> + }
> +
> + return (x->ex_flags & EXFLAG_INVALID) == 0;
> +}
