Re: Is it worth accepting multiple CRLs? - Mailing list pgsql-hackers

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
From 9168e29fa6bd957171cfa53553ae406eb44bb936 Mon Sep 17 00:00:00 2001
From: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Date: Mon, 1 Feb 2021 11:16:41 +0900
Subject: [PATCH v4] Allow to specify CRL directory

We have the ssl_crl_file GUC variable and the sslcrl connection option
to specify a CRL file.  X509_STORE_load_locations accepts a directory,
which leads to on-demand loading method with which method only
relevant CRLs are loaded. Allow server and client to use the hashed
directory method. We could use the existing variable and option to
specify the direcotry name but allowing to use both methods at the
same time gives operation flexibility to users.
---
 .../postgres_fdw/expected/postgres_fdw.out    |  2 +-
 doc/src/sgml/config.sgml                      | 21 +++++++++++-
 doc/src/sgml/libpq.sgml                       | 20 +++++++++--
 doc/src/sgml/runtime.sgml                     | 33 +++++++++++++++++++
 src/backend/libpq/be-secure-openssl.c         | 26 +++++++++++++--
 src/backend/libpq/be-secure.c                 |  1 +
 src/backend/utils/misc/guc.c                  | 10 ++++++
 src/include/libpq/libpq.h                     |  1 +
 src/interfaces/libpq/fe-connect.c             |  6 ++++
 src/interfaces/libpq/fe-secure-openssl.c      | 24 ++++++++++----
 src/interfaces/libpq/libpq-int.h              |  1 +
 src/test/ssl/Makefile                         | 20 ++++++++++-
 src/test/ssl/ssl/client-crldir/9bb9e3c3.r0    | 11 +++++++
 .../ssl/ssl/root+client-crldir/9bb9e3c3.r0    | 11 +++++++
 .../ssl/ssl/root+client-crldir/a3d11bff.r0    | 11 +++++++
 .../ssl/ssl/root+server-crldir/a3d11bff.r0    | 11 +++++++
 .../ssl/ssl/root+server-crldir/a836cc2d.r0    | 11 +++++++
 src/test/ssl/ssl/server-crldir/a836cc2d.r0    | 11 +++++++
 src/test/ssl/t/001_ssltests.pl                | 31 +++++++++++++++--
 src/test/ssl/t/SSLServer.pm                   | 14 +++++++-
 20 files changed, 258 insertions(+), 18 deletions(-)
 create mode 100644 src/test/ssl/ssl/client-crldir/9bb9e3c3.r0
 create mode 100644 src/test/ssl/ssl/root+client-crldir/9bb9e3c3.r0
 create mode 100644 src/test/ssl/ssl/root+client-crldir/a3d11bff.r0
 create mode 100644 src/test/ssl/ssl/root+server-crldir/a3d11bff.r0
 create mode 100644 src/test/ssl/ssl/root+server-crldir/a836cc2d.r0
 create mode 100644 src/test/ssl/ssl/server-crldir/a836cc2d.r0

diff --git a/contrib/postgres_fdw/expected/postgres_fdw.out b/contrib/postgres_fdw/expected/postgres_fdw.out
index b09dce63f5..85d1d0dfab 100644
--- a/contrib/postgres_fdw/expected/postgres_fdw.out
+++ b/contrib/postgres_fdw/expected/postgres_fdw.out
@@ -8928,7 +8928,7 @@ DO $d$
     END;
 $d$;
 ERROR:  invalid option "password"
-HINT:  Valid options in this context are: service, passfile, channel_binding, connect_timeout, dbname, host, hostaddr,
port,options, application_name, keepalives, keepalives_idle, keepalives_interval, keepalives_count, tcp_user_timeout,
sslmode,sslcompression, sslcert, sslkey, sslrootcert, sslcrl, requirepeer, ssl_min_protocol_version,
ssl_max_protocol_version,gssencmode, krbsrvname, gsslib, target_session_attrs, use_remote_estimate, fdw_startup_cost,
fdw_tuple_cost,extensions, updatable, fetch_size, batch_size
 
+HINT:  Valid options in this context are: service, passfile, channel_binding, connect_timeout, dbname, host, hostaddr,
port,options, application_name, keepalives, keepalives_idle, keepalives_interval, keepalives_count, tcp_user_timeout,
sslmode,sslcompression, sslcert, sslkey, sslrootcert, sslcrl, sslcrldir, requirepeer, ssl_min_protocol_version,
ssl_max_protocol_version,gssencmode, krbsrvname, gsslib, target_session_attrs, use_remote_estimate, fdw_startup_cost,
fdw_tuple_cost,extensions, updatable, fetch_size, batch_size
 
 CONTEXT:  SQL statement "ALTER SERVER loopback_nopw OPTIONS (ADD password 'dummypw')"
 PL/pgSQL function inline_code_block line 3 at EXECUTE
 -- If we add a password for our user mapping instead, we should get a different
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index e17cdcc816..67052bcbab 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -1216,7 +1216,26 @@ include_dir 'conf.d'
         Relative paths are relative to the data directory.
         This parameter can only be set in the <filename>postgresql.conf</filename>
         file or on the server command line.
-        The default is empty, meaning no CRL file is loaded.
+        The default is empty, meaning no CRL file is loaded unless
+        <xref linkend="guc-ssl-crl-dir"/> is set.
+       </para>
+      </listitem>
+     </varlistentry>
+
+     <varlistentry id="guc-ssl-crl-dir" xreflabel="ssl_crl_dir">
+      <term><varname>ssl_crl_dir</varname> (<type>string</type>)
+      <indexterm>
+       <primary><varname>ssl_crl_dir</varname> configuration parameter</primary>
+      </indexterm>
+      </term>
+      <listitem>
+       <para>
+        Specifies the name of the directory containing the SSL server
+        certificate revocation list (CRL).  Relative paths are relative to the
+        data directory.  This parameter can only be set in
+        the <filename>postgresql.conf</filename> file or on the server command
+        line.  The default is empty, meaning no CRL file is loaded unless
+        <xref linkend="guc-ssl-crl-file"/> is set.
        </para>
       </listitem>
      </varlistentry>
diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml
index b7a82453f0..7646756556 100644
--- a/doc/src/sgml/libpq.sgml
+++ b/doc/src/sgml/libpq.sgml
@@ -1723,8 +1723,24 @@ postgresql://%2Fvar%2Flib%2Fpostgresql/dbname
         This parameter specifies the file name of the SSL certificate
         revocation list (CRL).  Certificates listed in this file, if it
         exists, will be rejected while attempting to authenticate the
-        server's certificate.  The default is
-        <filename>~/.postgresql/root.crl</filename>.
+        server's certificate.  If both <xref linkend='libpq-connect-sslcrl'/>
+        and <xref linkend='libpq-connect-sslcrldir'/> are not set, this
+        setting is assumed to be
+        <filename>~/.postgresql/root.crl</filename>. See
+        <xref linkend="ssl-crl-files"/> for details.
+       </para>
+      </listitem>
+     </varlistentry>
+
+     <varlistentry id="libpq-connect-sslcrldir" xreflabel="sslcrldir">
+      <term><literal>sslcrldir</literal></term>
+      <listitem>
+       <para>
+        This parameter specifies the directory name of the SSL certificate
+        revocation list (CRL).  Certificates listed in the files in this
+        directory, if it exists, will be rejected while attempting to
+        authenticate the server's certificate.  See
+        <xref linkend="ssl-crl-files"/> for details.
        </para>
       </listitem>
      </varlistentry>
diff --git a/doc/src/sgml/runtime.sgml b/doc/src/sgml/runtime.sgml
index bf877c0e0c..4dc921779e 100644
--- a/doc/src/sgml/runtime.sgml
+++ b/doc/src/sgml/runtime.sgml
@@ -2552,6 +2552,39 @@ openssl x509 -req -in server.csr -text -days 365 \
    </para>
   </sect2>
 
+  <sect2 id="ssl-crl-files">
+    <title>Certification Revocation List files</title>
+
+    <para> The server setting <xref linkend="guc-ssl-crl-file"/> and
+    <xref linkend="guc-ssl-crl-dir"/>, and the connection option
+    <xref linkend="libpq-connect-sslcrl"/> and
+    <xref linkend="libpq-connect-sslcrldir"/> specify a file containing one or
+    more CRL, or a directory containing a separate file for every CRL
+    respectively. Settings for CRL file and CRL directory can be specified
+    together.  In the first method, file method, the all CRLs in the file is
+    loaded at server start time or by reloading config file (<command>pg_ctl
+    reload</command>). In the second method, hashed directory method, CRL
+    files are loaded on-demand, that is, only the relevant CRL files are
+    loaded at connection time.
+    </para>
+    <para>
+    The CRL file used for the file method can contain multiple CRLs, like
+    certificates, by just concatenated if it is in PEM format.  In the hashed
+    directory method, every file in the directory has the name
+    with <parameter>hash</parameter>.r<parameter>N</parameter> format,
+    where <parameter>hash</parameter> is the hash value of the issuer of the
+    CRL and <parameter>N</parameter> is a sequence number that starts at
+    zero. The hash value is calculated using openssl command. In both cases
+    the CRLs from all CAs involved in a certificate chain are needed to verify
+    a certificate, even if some or all of them are empty.
+<programlisting>
+$ openssl crl -hash -noout -in foo.crl
+98668507
+$ cp foo.crl $PGDATA/crldir/98668507.r0
+</programlisting>
+    </para>
+  </sect2>
+
  </sect1>
 
  <sect1 id="gssapi-enc">
diff --git a/src/backend/libpq/be-secure-openssl.c b/src/backend/libpq/be-secure-openssl.c
index 1e2ecc6e7a..ac471f3eeb 100644
--- a/src/backend/libpq/be-secure-openssl.c
+++ b/src/backend/libpq/be-secure-openssl.c
@@ -285,19 +285,22 @@ be_tls_init(bool isServerStart)
      * http://searchsecurity.techtarget.com/sDefinition/0,,sid14_gci803160,00.html
      *----------
      */
-    if (ssl_crl_file[0])
+    if (ssl_crl_file[0] || ssl_crl_dir[0])
     {
         X509_STORE *cvstore = SSL_CTX_get_cert_store(context);
 
         if (cvstore)
         {
             /* Set the flags to check against the complete CRL chain */
-            if (X509_STORE_load_locations(cvstore, ssl_crl_file, NULL) == 1)
+            if (X509_STORE_load_locations(cvstore,
+                                          ssl_crl_file[0] ? ssl_crl_file : NULL,
+                                          ssl_crl_dir[0]  ? ssl_crl_dir : NULL)
+                == 1)
             {
                 X509_STORE_set_flags(cvstore,
                                      X509_V_FLAG_CRL_CHECK | X509_V_FLAG_CRL_CHECK_ALL);
             }
-            else
+            else if (ssl_crl_dir[0] == 0)
             {
                 ereport(isServerStart ? FATAL : LOG,
                         (errcode(ERRCODE_CONFIG_FILE_ERROR),
@@ -305,6 +308,23 @@ be_tls_init(bool isServerStart)
                                 ssl_crl_file, SSLerrmessage(ERR_get_error()))));
                 goto error;
             }
+            else if (ssl_crl_file[0] == 0)
+            {
+                ereport(isServerStart ? FATAL : LOG,
+                        (errcode(ERRCODE_CONFIG_FILE_ERROR),
+                         errmsg("could not load SSL certificate revocation list directory \"%s\": %s",
+                                ssl_crl_dir, SSLerrmessage(ERR_get_error()))));
+                goto error;
+            }
+            else
+            {
+                ereport(isServerStart ? FATAL : LOG,
+                        (errcode(ERRCODE_CONFIG_FILE_ERROR),
+                         errmsg("could not load SSL certificate revocation list file \"%s\" and/or directory \"%s\":
%s",
+                                ssl_crl_file, ssl_crl_dir,
+                                SSLerrmessage(ERR_get_error()))));
+                goto error;
+            }
         }
     }
 
diff --git a/src/backend/libpq/be-secure.c b/src/backend/libpq/be-secure.c
index 4cf139a223..3ad6890f70 100644
--- a/src/backend/libpq/be-secure.c
+++ b/src/backend/libpq/be-secure.c
@@ -42,6 +42,7 @@ char       *ssl_cert_file;
 char       *ssl_key_file;
 char       *ssl_ca_file;
 char       *ssl_crl_file;
+char       *ssl_crl_dir;
 char       *ssl_dh_params_file;
 char       *ssl_passphrase_command;
 bool        ssl_passphrase_command_supports_reload;
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index eafdb1118e..00018abb7d 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -4355,6 +4355,16 @@ static struct config_string ConfigureNamesString[] =
         NULL, NULL, NULL
     },
 
+    {
+        {"ssl_crl_dir", PGC_SIGHUP, CONN_AUTH_SSL,
+            gettext_noop("Location of the SSL certificate revocation list directory."),
+            NULL
+        },
+        &ssl_crl_dir,
+        "",
+        NULL, NULL, NULL
+    },
+
     {
         {"stats_temp_directory", PGC_SIGHUP, STATS_COLLECTOR,
             gettext_noop("Writes temporary statistics files to the specified directory."),
diff --git a/src/include/libpq/libpq.h b/src/include/libpq/libpq.h
index a55898c85a..b41b10620a 100644
--- a/src/include/libpq/libpq.h
+++ b/src/include/libpq/libpq.h
@@ -82,6 +82,7 @@ extern char *ssl_cert_file;
 extern char *ssl_key_file;
 extern char *ssl_ca_file;
 extern char *ssl_crl_file;
+extern char *ssl_crl_dir;
 extern char *ssl_dh_params_file;
 extern PGDLLIMPORT char *ssl_passphrase_command;
 extern PGDLLIMPORT bool ssl_passphrase_command_supports_reload;
diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c
index 8ca0583aa9..db71fea169 100644
--- a/src/interfaces/libpq/fe-connect.c
+++ b/src/interfaces/libpq/fe-connect.c
@@ -317,6 +317,10 @@ static const internalPQconninfoOption PQconninfoOptions[] = {
         "SSL-Revocation-List", "", 64,
     offsetof(struct pg_conn, sslcrl)},
 
+    {"sslcrldir", "PGSSLCRLDIR", NULL, NULL,
+        "SSL-Revocation-List-Dir", "", 64,
+    offsetof(struct pg_conn, sslcrldir)},
+
     {"requirepeer", "PGREQUIREPEER", NULL, NULL,
         "Require-Peer", "", 10,
     offsetof(struct pg_conn, requirepeer)},
@@ -3998,6 +4002,8 @@ freePGconn(PGconn *conn)
         free(conn->sslrootcert);
     if (conn->sslcrl)
         free(conn->sslcrl);
+    if (conn->sslcrldir)
+        free(conn->sslcrldir);
     if (conn->sslcompression)
         free(conn->sslcompression);
     if (conn->requirepeer)
diff --git a/src/interfaces/libpq/fe-secure-openssl.c b/src/interfaces/libpq/fe-secure-openssl.c
index 5b4a4157d5..0fa10a23b4 100644
--- a/src/interfaces/libpq/fe-secure-openssl.c
+++ b/src/interfaces/libpq/fe-secure-openssl.c
@@ -794,7 +794,8 @@ initialize_SSL(PGconn *conn)
     if (!(conn->sslcert && strlen(conn->sslcert) > 0) ||
         !(conn->sslkey && strlen(conn->sslkey) > 0) ||
         !(conn->sslrootcert && strlen(conn->sslrootcert) > 0) ||
-        !(conn->sslcrl && strlen(conn->sslcrl) > 0))
+        !((conn->sslcrl && strlen(conn->sslcrl) > 0) ||
+          (conn->sslcrldir && strlen(conn->sslcrldir) > 0)))
         have_homedir = pqGetHomeDirectory(homedir, sizeof(homedir));
     else                        /* won't need it */
         have_homedir = false;
@@ -936,20 +937,29 @@ initialize_SSL(PGconn *conn)
 
         if ((cvstore = SSL_CTX_get_cert_store(SSL_context)) != NULL)
         {
+            char   *fname = NULL;
+            char   *dname = NULL;
+
             if (conn->sslcrl && strlen(conn->sslcrl) > 0)
-                strlcpy(fnbuf, conn->sslcrl, sizeof(fnbuf));
-            else if (have_homedir)
+                fname = conn->sslcrl;
+            if (conn->sslcrldir && strlen(conn->sslcrldir) > 0)
+                dname = conn->sslcrldir;
+
+            /* defaults to use the default CRL file */
+            if (!fname && !dname && have_homedir)
+            {
                 snprintf(fnbuf, sizeof(fnbuf), "%s/%s", homedir, ROOT_CRL_FILE);
-            else
-                fnbuf[0] = '\0';
+                fname = fnbuf;
+            }
 
             /* Set the flags to check against the complete CRL chain */
-            if (fnbuf[0] != '\0' &&
-                X509_STORE_load_locations(cvstore, fnbuf, NULL) == 1)
+            if ((fname || dname) &&
+                X509_STORE_load_locations(cvstore, fname, dname) == 1)
             {
                 X509_STORE_set_flags(cvstore,
                                      X509_V_FLAG_CRL_CHECK | X509_V_FLAG_CRL_CHECK_ALL);
             }
+
             /* if not found, silently ignore;  we do not require CRL */
             ERR_clear_error();
         }
diff --git a/src/interfaces/libpq/libpq-int.h b/src/interfaces/libpq/libpq-int.h
index 4db498369c..ce36aabd25 100644
--- a/src/interfaces/libpq/libpq-int.h
+++ b/src/interfaces/libpq/libpq-int.h
@@ -362,6 +362,7 @@ struct pg_conn
     char       *sslpassword;    /* client key file password */
     char       *sslrootcert;    /* root certificate filename */
     char       *sslcrl;            /* certificate revocation list filename */
+    char       *sslcrldir;        /* certificate revocation list directory name */
     char       *requirepeer;    /* required peer credentials for local sockets */
     char       *gssencmode;        /* GSS mode (require,prefer,disable) */
     char       *krbsrvname;        /* Kerberos service name */
diff --git a/src/test/ssl/Makefile b/src/test/ssl/Makefile
index 93335b1ea2..bc953a0bec 100644
--- a/src/test/ssl/Makefile
+++ b/src/test/ssl/Makefile
@@ -30,12 +30,15 @@ SSLFILES := $(CERTIFICATES:%=ssl/%.key) $(CERTIFICATES:%=ssl/%.crt) \
     ssl/client+client_ca.crt ssl/client-der.key \
     ssl/client-encrypted-pem.key ssl/client-encrypted-der.key
 
+SSLDIRS := ssl/client-crldir ssl/server-crldir \
+    ssl/root+client-crldir ssl/root+server-crldir
+
 # This target re-generates all the key and certificate files. Usually we just
 # use the ones that are committed to the tree without rebuilding them.
 #
 # This target will fail unless preceded by sslfiles-clean.
 #
-sslfiles: $(SSLFILES)
+sslfiles: $(SSLFILES) $(SSLDIRS)
 
 # OpenSSL requires a directory to put all generated certificates in. We don't
 # use this for anything, but we need a location.
@@ -146,10 +149,25 @@ ssl/root+server.crl: ssl/root.crl ssl/server.crl
     cat $^ > $@
 ssl/root+client.crl: ssl/root.crl ssl/client.crl
     cat $^ > $@
+ssl/root+server-crldir: ssl/server.crl ssl/root.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 ssl/root.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
+ssl/server-crldir: ssl/server.crl
+    mkdir ssl/server-crldir
+    cp ssl/server.crl ssl/server-crldir/`openssl crl -hash -noout -in ssl/server.crl`.r0
+ssl/client-crldir: ssl/client.crl
+    mkdir ssl/client-crldir
+    cp ssl/client.crl ssl/client-crldir/`openssl crl -hash -noout -in ssl/client.crl`.r0
 
 .PHONY: sslfiles-clean
 sslfiles-clean:
     rm -f $(SSLFILES) ssl/client_ca.srl ssl/server_ca.srl ssl/client_ca-certindex* ssl/server_ca-certindex*
ssl/root_ca-certindex*ssl/root_ca.srl ssl/temp_ca.crt ssl/temp_ca_signed.crt
 
+    rm -rf $(SSLDIRS)
 
 clean distclean maintainer-clean:
     rm -rf tmp_check
diff --git a/src/test/ssl/ssl/client-crldir/9bb9e3c3.r0 b/src/test/ssl/ssl/client-crldir/9bb9e3c3.r0
new file mode 100644
index 0000000000..a667680e04
--- /dev/null
+++ b/src/test/ssl/ssl/client-crldir/9bb9e3c3.r0
@@ -0,0 +1,11 @@
+-----BEGIN X509 CRL-----
+MIIBnjCBhzANBgkqhkiG9w0BAQsFADBCMUAwPgYDVQQDDDdUZXN0IENBIGZvciBQ
+b3N0Z3JlU1FMIFNTTCByZWdyZXNzaW9uIHRlc3QgY2xpZW50IGNlcnRzFw0xODEx
+MjcxMzQwNTVaFw00NjA0MTQxMzQwNTVaMBQwEgIBAhcNMTgxMTI3MTM0MDU1WjAN
+BgkqhkiG9w0BAQsFAAOCAQEAXjLxA9Qc6gAudwUHBxMIq5EHBcuNEX5e3GNlkyNf
+8I0DtHTPfJPvmAG+i6lYz//hHmmjxK0dR2ucg79XgXI/6OpDqlxS/TG1Xv52wA1p
+xz6GaJ2hC8Lk4/vbJo/Rrzme2QsI7xqBWya0JWVrehttqhFxPzWA5wID8X7G4Kb4
+pjVnzqYzn8A9FBiV9t10oZg60aVLqt3kbyy+U3pefvjhj8NmQc7uyuVjWvYZA0vG
+nnDUo4EKJzHNIYLk+EfpzKWO2XAWBLOT9SyyNCeMuQ5p/2pdAt9jtWHenms2ajo9
+2iUsHS91e3TooP9yNYuNcN8/wXY6H2Xm+dCLcEnkcr7EEw==
+-----END X509 CRL-----
diff --git a/src/test/ssl/ssl/root+client-crldir/9bb9e3c3.r0 b/src/test/ssl/ssl/root+client-crldir/9bb9e3c3.r0
new file mode 100644
index 0000000000..a667680e04
--- /dev/null
+++ b/src/test/ssl/ssl/root+client-crldir/9bb9e3c3.r0
@@ -0,0 +1,11 @@
+-----BEGIN X509 CRL-----
+MIIBnjCBhzANBgkqhkiG9w0BAQsFADBCMUAwPgYDVQQDDDdUZXN0IENBIGZvciBQ
+b3N0Z3JlU1FMIFNTTCByZWdyZXNzaW9uIHRlc3QgY2xpZW50IGNlcnRzFw0xODEx
+MjcxMzQwNTVaFw00NjA0MTQxMzQwNTVaMBQwEgIBAhcNMTgxMTI3MTM0MDU1WjAN
+BgkqhkiG9w0BAQsFAAOCAQEAXjLxA9Qc6gAudwUHBxMIq5EHBcuNEX5e3GNlkyNf
+8I0DtHTPfJPvmAG+i6lYz//hHmmjxK0dR2ucg79XgXI/6OpDqlxS/TG1Xv52wA1p
+xz6GaJ2hC8Lk4/vbJo/Rrzme2QsI7xqBWya0JWVrehttqhFxPzWA5wID8X7G4Kb4
+pjVnzqYzn8A9FBiV9t10oZg60aVLqt3kbyy+U3pefvjhj8NmQc7uyuVjWvYZA0vG
+nnDUo4EKJzHNIYLk+EfpzKWO2XAWBLOT9SyyNCeMuQ5p/2pdAt9jtWHenms2ajo9
+2iUsHS91e3TooP9yNYuNcN8/wXY6H2Xm+dCLcEnkcr7EEw==
+-----END X509 CRL-----
diff --git a/src/test/ssl/ssl/root+client-crldir/a3d11bff.r0 b/src/test/ssl/ssl/root+client-crldir/a3d11bff.r0
new file mode 100644
index 0000000000..e879cf25a7
--- /dev/null
+++ b/src/test/ssl/ssl/root+client-crldir/a3d11bff.r0
@@ -0,0 +1,11 @@
+-----BEGIN X509 CRL-----
+MIIBhTBvMA0GCSqGSIb3DQEBCwUAMEAxPjA8BgNVBAMMNVRlc3Qgcm9vdCBDQSBm
+b3IgUG9zdGdyZVNRTCBTU0wgcmVncmVzc2lvbiB0ZXN0IHN1aXRlFw0xODExMjcx
+MzQwNTVaFw00NjA0MTQxMzQwNTVaMA0GCSqGSIb3DQEBCwUAA4IBAQB8OSDym4/a
+qbZOrZvOOhmKrd7AJSTgAadtdK0CX3v58Ym3EmZK7gQFdBuFCXnvbue/x6avZHgz
+4pYFlJmL0IiD4QuTzsoo+LzifrmTzteO9oEJNLd2bjfEnpE5Wdaw6Yuy2Xb5edy5
+lQhNZdc8w3FiXhPOEUAi7EbdfDwn4G/fvEjpzyVb2wCujDUUePUGGayjKIM4PUu4
+pixM6gt9FFL27l47lQ3g0PbvB3TnU3oqcB3Y17FjbxjFc6AsGXholNetoEE2/49E
+PEYzOH7/PtxlZUtoCqZM+741LuI6Q7z4/P2X/IY33lMy6Iiyc41C94l/P7fCkMLG
+AlO+O0a4SpYS
+-----END X509 CRL-----
diff --git a/src/test/ssl/ssl/root+server-crldir/a3d11bff.r0 b/src/test/ssl/ssl/root+server-crldir/a3d11bff.r0
new file mode 100644
index 0000000000..e879cf25a7
--- /dev/null
+++ b/src/test/ssl/ssl/root+server-crldir/a3d11bff.r0
@@ -0,0 +1,11 @@
+-----BEGIN X509 CRL-----
+MIIBhTBvMA0GCSqGSIb3DQEBCwUAMEAxPjA8BgNVBAMMNVRlc3Qgcm9vdCBDQSBm
+b3IgUG9zdGdyZVNRTCBTU0wgcmVncmVzc2lvbiB0ZXN0IHN1aXRlFw0xODExMjcx
+MzQwNTVaFw00NjA0MTQxMzQwNTVaMA0GCSqGSIb3DQEBCwUAA4IBAQB8OSDym4/a
+qbZOrZvOOhmKrd7AJSTgAadtdK0CX3v58Ym3EmZK7gQFdBuFCXnvbue/x6avZHgz
+4pYFlJmL0IiD4QuTzsoo+LzifrmTzteO9oEJNLd2bjfEnpE5Wdaw6Yuy2Xb5edy5
+lQhNZdc8w3FiXhPOEUAi7EbdfDwn4G/fvEjpzyVb2wCujDUUePUGGayjKIM4PUu4
+pixM6gt9FFL27l47lQ3g0PbvB3TnU3oqcB3Y17FjbxjFc6AsGXholNetoEE2/49E
+PEYzOH7/PtxlZUtoCqZM+741LuI6Q7z4/P2X/IY33lMy6Iiyc41C94l/P7fCkMLG
+AlO+O0a4SpYS
+-----END X509 CRL-----
diff --git a/src/test/ssl/ssl/root+server-crldir/a836cc2d.r0 b/src/test/ssl/ssl/root+server-crldir/a836cc2d.r0
new file mode 100644
index 0000000000..717951c26a
--- /dev/null
+++ b/src/test/ssl/ssl/root+server-crldir/a836cc2d.r0
@@ -0,0 +1,11 @@
+-----BEGIN X509 CRL-----
+MIIBnjCBhzANBgkqhkiG9w0BAQsFADBCMUAwPgYDVQQDDDdUZXN0IENBIGZvciBQ
+b3N0Z3JlU1FMIFNTTCByZWdyZXNzaW9uIHRlc3Qgc2VydmVyIGNlcnRzFw0xODEx
+MjcxMzQwNTVaFw00NjA0MTQxMzQwNTVaMBQwEgIBBhcNMTgxMTI3MTM0MDU1WjAN
+BgkqhkiG9w0BAQsFAAOCAQEAbVuJXemxM6HLlIHGWlQvVmsmG4ZTQWiDnZjfmrND
+xB4XsvZNPXnFkjdBENDROrbDRwm60SJDW73AbDbfq1IXAzSpuEyuRz61IyYKo0wq
+nmObJtVdIu3bVlWIlDXaP5Emk3d7ouCj5f8Kyeb8gm4pL3N6e0eI63hCaS39hhE6
+RLGh9HU9ht1kKfgcTwmB5b2HTPb4M6z1AmSIaMVqZTjIspsUgNF2+GBm3fOnOaiZ
+SEXWtgjMRXiIHbtU0va3LhSH5OSW0mh+L9oGUQDYnyuudnWGpulhqIp4qVkJRDDu
+41HpD83dV2uRtBLvc25AFHj7kXBflbO3gvGZVPYf1zVghQ==
+-----END X509 CRL-----
diff --git a/src/test/ssl/ssl/server-crldir/a836cc2d.r0 b/src/test/ssl/ssl/server-crldir/a836cc2d.r0
new file mode 100644
index 0000000000..717951c26a
--- /dev/null
+++ b/src/test/ssl/ssl/server-crldir/a836cc2d.r0
@@ -0,0 +1,11 @@
+-----BEGIN X509 CRL-----
+MIIBnjCBhzANBgkqhkiG9w0BAQsFADBCMUAwPgYDVQQDDDdUZXN0IENBIGZvciBQ
+b3N0Z3JlU1FMIFNTTCByZWdyZXNzaW9uIHRlc3Qgc2VydmVyIGNlcnRzFw0xODEx
+MjcxMzQwNTVaFw00NjA0MTQxMzQwNTVaMBQwEgIBBhcNMTgxMTI3MTM0MDU1WjAN
+BgkqhkiG9w0BAQsFAAOCAQEAbVuJXemxM6HLlIHGWlQvVmsmG4ZTQWiDnZjfmrND
+xB4XsvZNPXnFkjdBENDROrbDRwm60SJDW73AbDbfq1IXAzSpuEyuRz61IyYKo0wq
+nmObJtVdIu3bVlWIlDXaP5Emk3d7ouCj5f8Kyeb8gm4pL3N6e0eI63hCaS39hhE6
+RLGh9HU9ht1kKfgcTwmB5b2HTPb4M6z1AmSIaMVqZTjIspsUgNF2+GBm3fOnOaiZ
+SEXWtgjMRXiIHbtU0va3LhSH5OSW0mh+L9oGUQDYnyuudnWGpulhqIp4qVkJRDDu
+41HpD83dV2uRtBLvc25AFHj7kXBflbO3gvGZVPYf1zVghQ==
+-----END X509 CRL-----
diff --git a/src/test/ssl/t/001_ssltests.pl b/src/test/ssl/t/001_ssltests.pl
index fd2727b568..4f36bf9dc0 100644
--- a/src/test/ssl/t/001_ssltests.pl
+++ b/src/test/ssl/t/001_ssltests.pl
@@ -13,7 +13,7 @@ use SSLServer;
 
 if ($ENV{with_openssl} eq 'yes')
 {
-    plan tests => 93;
+    plan tests => 100;
 }
 else
 {
@@ -214,6 +214,12 @@ test_connect_fails(
     "sslrootcert=ssl/root+server_ca.crt sslmode=verify-ca sslcrl=ssl/client.crl",
     qr/SSL error/,
     "CRL belonging to a different CA");
+# The same for CRL directory, fails
+test_connect_fails(
+    $common_connstr,
+    "sslrootcert=ssl/root+server_ca.crt sslmode=verify-ca sslcrldir=ssl/client-crldir",
+    qr/SSL error/,
+    "directory CRL belonging to a different CA");
 
 # With the correct CRL, succeeds (this cert is not revoked)
 test_connect_ok(
@@ -221,6 +227,12 @@ test_connect_ok(
     "sslrootcert=ssl/root+server_ca.crt sslmode=verify-ca sslcrl=ssl/root+server.crl",
     "CRL with a non-revoked cert");
 
+# With the correct server CRL directory, succeeds (this cert is not revoked)
+test_connect_ok(
+    $common_connstr,
+    "sslrootcert=ssl/root+server_ca.crt sslmode=verify-ca sslcrldir=ssl/root+server-crldir",
+    "directory CRL with a non-revoked cert");
+
 # Check that connecting with verify-full fails, when the hostname doesn't
 # match the hostname in the server's certificate.
 $common_connstr =
@@ -346,7 +358,12 @@ test_connect_fails(
     $common_connstr,
     "sslrootcert=ssl/root+server_ca.crt sslmode=verify-ca sslcrl=ssl/root+server.crl",
     qr/SSL error/,
-    "does not connect with client-side CRL");
+    "does not connect with client-side CRL file");
+test_connect_fails(
+    $common_connstr,
+    "sslrootcert=ssl/root+server_ca.crt sslmode=verify-ca sslcrldir=ssl/root+server-crldir",
+    qr/SSL error/,
+    "does not connect with client-side CRL directory");
 
 # pg_stat_ssl
 command_like(
@@ -545,6 +562,16 @@ test_connect_ok(
 test_connect_fails($common_connstr, "sslmode=require sslcert=ssl/client.crt",
     qr/SSL error/, "intermediate client certificate is missing");
 
+# test server-side CRL directory
+switch_server_cert($node, 'server-cn-only', undef, undef, 'root+client-crldir');
+
+# revoked client cert
+test_connect_fails(
+    $common_connstr,
+    "user=ssltestuser sslcert=ssl/client-revoked.crt sslkey=ssl/client-revoked_tmp.key",
+    qr/SSL error/,
+    "certificate authorization fails with revoked client cert with server-side CRL directory");
+
 # clean up
 foreach my $key (@keys)
 {
diff --git a/src/test/ssl/t/SSLServer.pm b/src/test/ssl/t/SSLServer.pm
index f5987a003e..5ec5e0dac8 100644
--- a/src/test/ssl/t/SSLServer.pm
+++ b/src/test/ssl/t/SSLServer.pm
@@ -150,6 +150,8 @@ sub configure_test_server_for_ssl
     copy_files("ssl/root+client_ca.crt", $pgdata);
     copy_files("ssl/root_ca.crt",        $pgdata);
     copy_files("ssl/root+client.crl",    $pgdata);
+    mkdir("$pgdata/root+client-crldir");
+    copy_files("ssl/root+client-crldir/*", "$pgdata/root+client-crldir/");
 
     # Stop and restart server to load new listen_addresses.
     $node->restart;
@@ -167,14 +169,24 @@ sub switch_server_cert
     my $node     = $_[0];
     my $certfile = $_[1];
     my $cafile   = $_[2] || "root+client_ca";
+    my $crlfile  = "root+client.crl";
+    my $crldir;
     my $pgdata   = $node->data_dir;
 
+    # defaults to use crl file
+    if (defined $_[3] || defined $_[4])
+    {
+        $crlfile = $_[3];
+        $crldir = $_[4];
+    }
+
     open my $sslconf, '>', "$pgdata/sslconfig.conf";
     print $sslconf "ssl=on\n";
     print $sslconf "ssl_ca_file='$cafile.crt'\n";
     print $sslconf "ssl_cert_file='$certfile.crt'\n";
     print $sslconf "ssl_key_file='$certfile.key'\n";
-    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;
     close $sslconf;
 
     $node->restart;
-- 
2.27.0