Re: Is it worth accepting multiple CRLs?

At Sat, 30 Jan 2021 22:20:19 +0100, Peter Eisentraut <peter.eisentraut@enterprisedb.com> wrote in 
> On 2021-01-19 09:32, Kyotaro Horiguchi wrote:
> > At Tue, 19 Jan 2021 09:17:34 +0900 (JST), Kyotaro Horiguchi
> > <horikyota.ntt@gmail.com> wrote in
> >> By the way we can do the same thing on CA file/dir, but I personally
> >> think that the benefit from the specify-by-directory for CA files is
> >> far less than CRL files. So I'm not going to do this for CA files for
> >> now.
> > This is it. A new guc ssl_crl_dir and connection option crldir are
> > added.
> 
> This looks pretty good to me overall.

Thanks!

> You need to update the expected result of the postgres_fdw test.

Oops. Fixed.

> Also check your patch for whitespace errors with git diff --check or
> similar.

Sorry for forgetting that. I found an extra new line in
be-secure-openssl.c and remved it.

> > One problem raised upthread is the footprint for test is quite large
> > because all certificate and key files are replaced by this patch. I
> > think we can shrink the footprint by generating that files on-demand
> > but that needs openssl frontend to be installed on the development
> > environment.
> 
> I don't understand why you need to recreate all these files.  All your
> patch should contain are the new *.r0 files that are computed from the
> existing *.crl files.  Nothing else should change, AIUI.

Ah. If I ran make with this patch, it complains of
ssl/root_ca-certindex lacking and I ran "make clean" to avoid the
complaint.  Instead, I created the additional crl directories by
manually executing the recipes of the additional rules.

v3:  41 files changed, 496 insertions(+), 255 deletions(-)
v4:  21 files changed, 258 insertions(+), 18 deletions(-)

I checked that 001_ssltests.pl succedds both with the preexisting ssl/
files and with the files created by "make sslfiles" after "make
sslfiles-clean".

> Some of the makefile rules for generating the CRL files need some
> refinement.  In
> 
> +ssl/root+server-crldir: ssl/server.crl
> +   mkdir ssl/root+server-crldir
> + cp ssl/server.crl ssl/root+server-crldir/`openssl crl -hash -noout
> -in ssl/server.crl`.r0
> + cp ssl/root.crl ssl/root+server-crldir/`openssl crl -hash -noout -in
> ssl/root.crl`.r0
> +ssl/root+client-crldir: ssl/client.crl
> +   mkdir ssl/root+client-crldir
> + cp ssl/client.crl ssl/root+client-crldir/`openssl crl -hash -noout
> -in ssl/client.crl`.r0
> + cp ssl/root.crl ssl/root+client-crldir/`openssl crl -hash -noout -in
> ssl/root.crl`.r0
> 
> the rules should also have a dependency on ssl/root.crl in addition to
> ssl/server.crl.

Right. Added.

> By the way:
> 
> -   print $sslconf "ssl_crl_file='root+client.crl'\n";
> +   print $sslconf "ssl_crl_file='$crlfile'\n" if (defined $crlfile);
> +   print $sslconf "ssl_crl_dir='$crldir'\n" if (defined $crldir);
> 
> Trailing "if" doesn't need parentheses.

I know. However I preferred to have them at the time, I don't have a
strong opinion about how it should be. Ripped off them.


regards.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center