Is it worth accepting multiple CRLs?

A CA may issue a CRL infrequently, but issue a delta-CRL frequently. Does the logic support this properly?

Personal email. hbhotz@oxy.edu

Greetings,

* Kyotaro Horiguchi (horikyota.ntt@gmail.com) wrote:

PostgreSQL server accepts only one CRL file. It is easy to expand
be_tls_init to accept a directory set in ssl_crl_file. But I'm not
sure CRL is actually even utilized in the field so that could ends
with just bloating the documentation.

Is it work doing?

Yes, CRLs are absolutely used in the field and having this would be
nice.

Thanks,

Stephen

At Fri, 31 Jul 2020 05:53:53 -0700, Henry B Hotz <hbhotz@oxy.edu> wrote in

A CA may issue a CRL infrequently, but issue a delta-CRL frequently. Does the logic support this properly?

If you are talking about regsitering new revokations while server is
running, it checks newer CRLs upon each lookup according to the
documentation [1]https://www.openssl.org/docs/man1.1.1/man3/X509_LOOKUP_hash_dir.html, so a new Delta-CRL can be added after server
start. If server restart is allowed, the CRL file specified by
ssl_crl_file can contain multiple CRLs by just concatenation.

[1]: https://www.openssl.org/docs/man1.1.1/man3/X509_LOOKUP_hash_dir.html

regards.

--
Kyotaro Horiguchi
NTT Open Source Software Center

At Fri, 31 Jul 2020 09:00:14 -0400, Stephen Frost <sfrost@snowman.net> wrote in

Greetings,

* Kyotaro Horiguchi (horikyota.ntt@gmail.com) wrote:

PostgreSQL server accepts only one CRL file. It is easy to expand
be_tls_init to accept a directory set in ssl_crl_file. But I'm not
sure CRL is actually even utilized in the field so that could ends
with just bloating the documentation.

Is it work doing?

Yes, CRLs are absolutely used in the field and having this would be
nice.

Thanks for the opinion. I'll continue working on this.

regards.

--
Kyotaro Horiguchi
NTT Open Source Software Center

Uggg.

At Mon, 03 Aug 2020 16:19:37 +0900 (JST), Kyotaro Horiguchi <horikyota.ntt@gmail.com> wrote in

At Fri, 31 Jul 2020 05:53:53 -0700, Henry B Hotz <hbhotz@oxy.edu> wrote in

A CA may issue a CRL infrequently, but issue a delta-CRL frequently. Does the logic support this properly?

If you are talking about regsitering new revokations while server is
running, it checks newer CRLs upon each lookup according to the
documentation [1], so a new Delta-CRL can be added after server
start. If server restart is allowed, the CRL file specified by

I didin't know that ssl files are reloaded by SIGHUP (pg_ctl
reload). So the ssl_crl_file is also reloaded on server reload.

ssl_crl_file can contain multiple CRLs by just concatenation.

[1]: https://www.openssl.org/docs/man1.1.1/man3/X509_LOOKUP_hash_dir.html

Still on-demand loading is the advantage of the hashed directory
method. I'll continue working..

regards.

--
Kyotaro Horiguchi
NTT Open Source Software Center

At Mon, 03 Aug 2020 16:20:40 +0900 (JST), Kyotaro Horiguchi <horikyota.ntt@gmail.com> wrote in

Thanks for the opinion. I'll continue working on this.

This is it, but..

Looking closer I realized that certificates are verified in each
backend so CRL cache doesn't work at all for the hashed directory
method. Therefore, all CRL files relevant to a certificate to be
verfied are loaded every time a backend starts.

The only advantage of this is avoiding irrelevant CRLs from being
loaded in exchange of loading relevant CRLs at every session
start. Session startup gets slower by many delta CRLs from the same
CA.

Seems far from promising.

regards.

--
Kyotaro Horiguchi
NTT Open Source Software Center

Greetings,

* Kyotaro Horiguchi (horikyota.ntt@gmail.com) wrote:

At Mon, 03 Aug 2020 16:20:40 +0900 (JST), Kyotaro Horiguchi <horikyota.ntt@gmail.com> wrote in

Thanks for the opinion. I'll continue working on this.

This is it, but..

Thanks!

Looking closer I realized that certificates are verified in each
backend so CRL cache doesn't work at all for the hashed directory
method. Therefore, all CRL files relevant to a certificate to be
verfied are loaded every time a backend starts.

The only advantage of this is avoiding irrelevant CRLs from being
loaded in exchange of loading relevant CRLs at every session
start. Session startup gets slower by many delta CRLs from the same
CA.

Seems far from promising.

I agree that it's not ideal, but I don't know that this is a reason to
not move forward with this feature..?

We could certainly have a later patch which improves this in some way
(though exactly how isn't clear... if we move the CRL loading into
postmaster then we'd have to load *all* of them, and then we'd still
need to check if they've changed since we loaded them, and presumably
have some way to signal the postmaster to update its set from time to
time..), but that can be a future effort.

I took a quick look through the patch and it seemed pretty straight
forward to me and a good improvement.

Would love to hear other thoughts. I hope you'll submit this for the
September CF and ping me when you do and I'll see if I can get it
committed.

Thanks!

Stephen

Hello.

At Sat, 15 Aug 2020 13:18:22 -0400, Stephen Frost <sfrost@snowman.net> wrote in

Looking closer I realized that certificates are verified in each
backend so CRL cache doesn't work at all for the hashed directory
method. Therefore, all CRL files relevant to a certificate to be
verfied are loaded every time a backend starts.

The only advantage of this is avoiding irrelevant CRLs from being
loaded in exchange of loading relevant CRLs at every session
start. Session startup gets slower by many delta CRLs from the same
CA.

Seems far from promising.

I agree that it's not ideal, but I don't know that this is a reason to
not move forward with this feature..?

Since one of the significant advantage of the directory method is
differential loading of new CRLs. But actually it has other advanges
like easier file handling and not needing server reload.

We could certainly have a later patch which improves this in some way
(though exactly how isn't clear... if we move the CRL loading into
postmaster then we'd have to load *all* of them, and then we'd still
need to check if they've changed since we loaded them, and presumably
have some way to signal the postmaster to update its set from time to
time..), but that can be a future effort.

I took a quick look through the patch and it seemed pretty straight
forward to me and a good improvement.

Would love to hear other thoughts. I hope you'll submit this for the
September CF and ping me when you do and I'll see if I can get it
committed.

Thank you very much. I'll do that after some polishing.

A near-by discussion about OpenSSL3.0 conflicts with this but it's
easy to follow.

regards.

--
Kyotaro Horiguchi
NTT Open Source Software Center

At Tue, 18 Aug 2020 16:43:47 +0900 (JST), Kyotaro Horiguchi <horikyota.ntt@gmail.com> wrote in

Thank you very much. I'll do that after some polishing.

A near-by discussion about OpenSSL3.0 conflicts with this but it's
easy to follow.

Rebased. Fixed bogus tests and strange tentative API change of
SSLServer.pm. Corrected a (maybe) spelling mistake. I'm going to
register this to the coming CF.

regards.

--
Kyotaro Horiguchi
NTT Open Source Software Center

On Mon, Aug 31, 2020 at 06:03:02PM +0900, Kyotaro Horiguchi wrote:

Rebased. Fixed bogus tests and strange tentative API change of
SSLServer.pm. Corrected a (maybe) spelling mistake. I'm going to
register this to the coming CF.

Stephen, are you planning to look at that? I know that you are not
registered as a reviewer, but you mentioned upthread that you may be
able to look at it.

The changes in libpq's backend/frontend are rather simple, but the
docs and the changes in the TAP tests require a careful lookup.
--
Michael

On 2020-08-31 11:03, Kyotaro Horiguchi wrote:

At Tue, 18 Aug 2020 16:43:47 +0900 (JST), Kyotaro Horiguchi <horikyota.ntt@gmail.com> wrote in

Thank you very much. I'll do that after some polishing.

A near-by discussion about OpenSSL3.0 conflicts with this but it's
easy to follow.

Rebased. Fixed bogus tests and strange tentative API change of
SSLServer.pm. Corrected a (maybe) spelling mistake. I'm going to
register this to the coming CF.

Other systems that offer both a CRL file and a CRL directory usually
specify those using two separate configuration settings. Examples:

https://dev.mysql.com/doc/refman/5.7/en/server-system-variables.html#sysvar_ssl_crlpath
https://httpd.apache.org/docs/current/mod/mod_ssl.html#sslcarevocationpath

These are then presumably both passed to X509_STORE_load_locations(),
which supports specifying a file and directory concurrently.

I think that would be a preferable approach. In practical terms, it
would allow a user to introduce the directory method gradually without
having to convert the existing CRL file at the same time.

At Fri, 15 Jan 2021 08:56:27 +0100, Peter Eisentraut <peter.eisentraut@enterprisedb.com> wrote in

On 2020-08-31 11:03, Kyotaro Horiguchi wrote:

At Tue, 18 Aug 2020 16:43:47 +0900 (JST), Kyotaro Horiguchi
<horikyota.ntt@gmail.com> wrote in

Thank you very much. I'll do that after some polishing.

A near-by discussion about OpenSSL3.0 conflicts with this but it's
easy to follow.

Rebased. Fixed bogus tests and strange tentative API change of
SSLServer.pm. Corrected a (maybe) spelling mistake. I'm going to
register this to the coming CF.

Other systems that offer both a CRL file and a CRL directory usually
specify those using two separate configuration settings. Examples:

https://dev.mysql.com/doc/refman/5.7/en/server-system-variables.html#sysvar_ssl_crlpath
https://httpd.apache.org/docs/current/mod/mod_ssl.html#sslcarevocationpath

These are then presumably both passed to X509_STORE_load_locations(),
which supports specifying a file and directory concurrently.

I think that would be a preferable approach. In practical terms, it
would allow a user to introduce the directory method gradually without
having to convert the existing CRL file at the same time.

Thank you for the information. The only reason for sharing the same
variable for both file and directory is to avoid additional variable
only for this reason. I'll post a new version where new GUC
ssl_crl_path is added.

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.

regards.

--
Kyotaro Horiguchi
NTT Open Source Software Center

On 2021-01-19 01:17, Kyotaro Horiguchi wrote:

Thank you for the information. The only reason for sharing the same
variable for both file and directory is to avoid additional variable
only for this reason. I'll post a new version where new GUC
ssl_crl_path is added.

Okay, I look forward to that patch.

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.

Yeah, that seems not so commonly used.

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.

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.

If we agree that requirement, I'm going to go that direction.

regards.

--
Kyotaro Horiguchi
NTT Open Source Software Center

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.

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

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

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.

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.

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.

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

The commit fe61df7f82 shot down this.

This patch allows a new GUC ssl_crl_dir and a new libpq connection
option sslcrldir to specify CRL directory, which stores multiple files
that contains one CRL. With that method server loads only CRLs for the
CA of the certificate being validated.

Along with rebasing, the documentation is slightly reworded.

revocation list (CRL).  Certificates listed in this file, if it
         exists, will be rejected while attempting to authenticate the
-        server's certificate.  If both sslcrl and sslcrldir are not set,
-        this setting is assumed to be
+        server's certificate.  If neither sslcrl sslcrldir is set, this
+        setting is assumed to be
         <filename>~/.postgresql/root.crl</filename>. See

And added a line for the new variable in postgresql.conf.sample.

regards.

--
Kyotaro Horiguchi
NTT Open Source Software Center

On 2021-02-17 05:05, Kyotaro Horiguchi wrote:

The commit fe61df7f82 shot down this.

This patch allows a new GUC ssl_crl_dir and a new libpq connection
option sslcrldir to specify CRL directory, which stores multiple files
that contains one CRL. With that method server loads only CRLs for the
CA of the certificate being validated.

Along with rebasing, the documentation is slightly reworded.

Committed this.

I changed the documentation a bit. Instead of having a separate section
describing the CRL options, I put that information directly into the
libpq and GUC sections. Some of the information, such as that the
directory files are loaded on demand, isn't so obviously useful in the
libpq case, so I found that a bit confusing. Also, I got the impression
that the hashed directory format is sort of internal to OpenSSL, and
there are several versions of that format, so I didn't want to copy over
the description of these internals. Instead, I referred to the openssl
rehash/c_rehash commands for information. If we get support for
non-OpenSSL providers, we'll probably have to revisit this.

Thanks for committing this!

At Thu, 18 Feb 2021 08:24:23 +0100, Peter Eisentraut <peter.eisentraut@enterprisedb.com> wrote in

On 2021-02-17 05:05, Kyotaro Horiguchi wrote:

The commit fe61df7f82 shot down this.
This patch allows a new GUC ssl_crl_dir and a new libpq connection
option sslcrldir to specify CRL directory, which stores multiple files
that contains one CRL. With that method server loads only CRLs for the
CA of the certificate being validated.
Along with rebasing, the documentation is slightly reworded.

Committed this.

I changed the documentation a bit. Instead of having a separate
section describing the CRL options, I put that information directly
into the libpq and GUC sections. Some of the information, such as
that the directory files are loaded on demand, isn't so obviously
useful in the libpq case, so I found that a bit confusing. Also, I

Agreed.

got the impression that the hashed directory format is sort of
internal to OpenSSL, and there are several versions of that format, so
I didn't want to copy over the description of these internals.
Instead, I referred to the openssl rehash/c_rehash commands for
information. If we get support for non-OpenSSL providers, we'll
probably have to revisit this.

Thanks. I'm fine with that, either.

regards.

--
Kyotaro Horiguchi
NTT Open Source Software Center