public inbox for [email protected]  
help / color / mirror / Atom feed
[PATCH v4] Allow to specify CRL directory
3+ messages / 3 participants
[nested] [flat]

* [PATCH v4] Allow to specify CRL directory
@ 2021-02-01 02:16 Kyotaro Horiguchi <[email protected]>
  0 siblings, 0 replies; 3+ messages in thread

From: Kyotaro Horiguchi @ 2021-02-01 02:16 UTC (permalink / raw)

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


----Next_Part(Mon_Feb__1_11_42_32_2021_967)----





^ permalink  raw  reply  [nested|flat] 3+ messages in thread

* Re: Relation bulk write facility
@ 2024-02-28 11:25 Heikki Linnakangas <[email protected]>
  2024-02-28 16:04 ` Re: Relation bulk write facility Tom Lane <[email protected]>
  0 siblings, 1 reply; 3+ messages in thread

From: Heikki Linnakangas @ 2024-02-28 11:25 UTC (permalink / raw)
  To: Andres Freund <[email protected]>; Thomas Munro <[email protected]>; Noah Misch <[email protected]>; +Cc: Michael Paquier <[email protected]>; Robert Haas <[email protected]>; Tom Lane <[email protected]>; Peter Smith <[email protected]>; vignesh C <[email protected]>; pgsql-hackers; Melanie Plageman <[email protected]>

Committed, after fixing the various little things you pointed out:

On 28/02/2024 00:30, Thomas Munro wrote:
> --- a/doc/src/sgml/installation.sgml
> +++ b/doc/src/sgml/installation.sgml
> @@ -3401,7 +3401,7 @@ export MANPATH
>     <para>
>      <productname>PostgreSQL</productname> can be expected to work on current
>      versions of these operating systems: Linux, Windows,
> -   FreeBSD, OpenBSD, NetBSD, DragonFlyBSD, macOS, AIX, Solaris, and illumos.
> +   FreeBSD, OpenBSD, NetBSD, DragonFlyBSD, macOS, Solaris, and illumos.
> 
> There is also a little roll-of-honour of operating systems we used to
> support, just a couple of paragraphs down, where AIX should appear.

Added.

On 28/02/2024 05:52, Noah Misch wrote:
> Regardless of how someone were to step up to maintain it, we'd be telling them
> such contributions have negative value and must stop.  We're expelling AIX due
> to low demand, compiler bugs, its ABI, and its shlib symbol export needs.

Reworded.

>> Apperently the "pg_attribute_aligned(a)" attribute doesn't work on AIX
>> (linker?) for values larger than PG_IO_ALIGN_SIZE.
> 
> No; see https://postgr.es/m/20240225194322.a5%40rfd.leadboat.com

Ok, reworded.

On 28/02/2024 11:26, Andres Freund wrote:
> On 2024-02-28 00:24:01 +0400, Heikki Linnakangas wrote:
> The problem is due to this hunk:
>> @@ -401,10 +376,6 @@ install-lib-static: $(stlib) installdirs-lib
>>   
>>   install-lib-shared: $(shlib) installdirs-lib
>>   ifdef soname
>> -# we don't install $(shlib) on AIX
>> -# (see http://archives.postgresql.org/message-id/[email protected]...)
>> -ifneq ($(PORTNAME), aix)
>> -	$(INSTALL_SHLIB) $< '$(DESTDIR)$(libdir)/$(shlib)'
>>   ifneq ($(PORTNAME), cygwin)
>>   ifneq ($(PORTNAME), win32)
>>   ifneq ($(shlib), $(shlib_major))
> 
> So the versioned name didn't end up getting installed anymore, leading to
> broken symlinks in the install directory.

Fixed, thanks!

>> diff --git a/src/bin/pg_basebackup/t/010_pg_basebackup.pl b/src/bin/pg_basebackup/t/010_pg_basebackup.pl
>> index 86cc01a640b..fc6b00224f6 100644
>> --- a/src/bin/pg_basebackup/t/010_pg_basebackup.pl
>> +++ b/src/bin/pg_basebackup/t/010_pg_basebackup.pl
>> @@ -400,9 +400,6 @@ is(scalar(@tblspc_tars), 1, 'one tablespace tar was created');
>>   SKIP:
>>   {
>>   	my $tar = $ENV{TAR};
>> -	# don't check for a working tar here, to accommodate various odd
>> -	# cases such as AIX. If tar doesn't work the init_from_backup below
>> -	# will fail.
>>   	skip "no tar program available", 1
>>   	  if (!defined $tar || $tar eq '');
> 
> Maybe better to not remove the whole comment, just the reference to AIX?

Ok, done

>> diff --git a/src/test/regress/sql/sanity_check.sql b/src/test/regress/sql/sanity_check.sql
>> index 7f338d191c6..2e9d5ebef3f 100644
>> --- a/src/test/regress/sql/sanity_check.sql
>> +++ b/src/test/regress/sql/sanity_check.sql
>> @@ -21,12 +21,15 @@ SELECT relname, relkind
>>          AND relfilenode <> 0;
>>   
>>   --
>> --- When ALIGNOF_DOUBLE==4 (e.g. AIX), the C ABI may impose 8-byte alignment on
>> +-- When MAXIMUM_ALIGNOF==8 but ALIGNOF_DOUBLE==4, the C ABI may impose 8-byte alignment
>>   -- some of the C types that correspond to TYPALIGN_DOUBLE SQL types.  To ensure
>>   -- catalog C struct layout matches catalog tuple layout, arrange for the tuple
>>   -- offset of each fixed-width, attalign='d' catalog column to be divisible by 8
>>   -- unconditionally.  Keep such columns before the first NameData column of the
>>   -- catalog, since packagers can override NAMEDATALEN to an odd number.
>> +-- (XXX: I'm not sure if any of the supported platforms have MAXIMUM_ALIGNOF==8 and
>> +-- ALIGNOF_DOUBLE==4.  Perhaps we should just require that
>> +-- ALIGNOF_DOUBLE==MAXIMUM_ALIGNOF)
>>   --
>>   WITH check_columns AS (
>>    SELECT relname, attname,
> 
> I agree, this should be an error, and we should then remove the test.

Done.

-- 
Heikki Linnakangas
Neon (https://neon.tech)






^ permalink  raw  reply  [nested|flat] 3+ messages in thread

* Re: Relation bulk write facility
  2024-02-28 11:25 Re: Relation bulk write facility Heikki Linnakangas <[email protected]>
@ 2024-02-28 16:04 ` Tom Lane <[email protected]>
  0 siblings, 0 replies; 3+ messages in thread

From: Tom Lane @ 2024-02-28 16:04 UTC (permalink / raw)
  To: Heikki Linnakangas <[email protected]>; +Cc: Andres Freund <[email protected]>; Thomas Munro <[email protected]>; Noah Misch <[email protected]>; Michael Paquier <[email protected]>; Robert Haas <[email protected]>; Peter Smith <[email protected]>; vignesh C <[email protected]>; pgsql-hackers; Melanie Plageman <[email protected]>

Heikki Linnakangas <[email protected]> writes:
> On 28/02/2024 00:30, Thomas Munro wrote:
>> I agree, this should be an error, and we should then remove the test.

> Done.

The commit that added that test added a support function
"get_columns_length" which is now unused.  Should we get rid of that
as well?

			regards, tom lane






^ permalink  raw  reply  [nested|flat] 3+ messages in thread


end of thread, other threads:[~2024-02-28 16:04 UTC | newest]

Thread overview: 3+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2021-02-01 02:16 [PATCH v4] Allow to specify CRL directory Kyotaro Horiguchi <[email protected]>
2024-02-28 11:25 Re: Relation bulk write facility Heikki Linnakangas <[email protected]>
2024-02-28 16:04 ` Re: Relation bulk write facility Tom Lane <[email protected]>

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox