On Wed, Jul 20, 2022 at 3:42 PM Tom Lane <t...@sss.pgh.pa.us> wrote:
> Jacob Champion <jchamp...@timescale.com> writes:
> > I'm currently hardcoding an elevel of ERROR on the new guc_strdup()s,
> > because that seems to be a common case for the check hooks.
>
> Really?  That's almost certainly NOT okay.  As an example, if you
> have a problem with a new value loaded from postgresql.conf during
> SIGHUP processing, throwing ERROR will cause the postmaster to exit.

v4 attempts to fix this by letting the check hooks pass
MCXT_ALLOC_NO_OOM to pg_clean_ascii(). (It's ignored in the frontend,
which just mallocs.)

> I wouldn't be too surprised if there are isolated cases where people
> didn't understand what they were doing and wrote that, but that
> needs to be fixed not emulated.

I might be missing something, but in guc.c at least it appears to be
the rule and not the exception.

Thanks,
--Jacob
diff --git a/src/backend/libpq/be-secure-openssl.c 
b/src/backend/libpq/be-secure-openssl.c
index 5318719acb..1394ecaa2b 100644
--- a/src/backend/libpq/be-secure-openssl.c
+++ b/src/backend/libpq/be-secure-openssl.c
@@ -1116,7 +1116,7 @@ prepare_cert_name(char *name)
 
 #undef MAXLEN
 
-       return pg_clean_ascii(truncated);
+       return pg_clean_ascii(truncated, 0);
 }
 
 /*
diff --git a/src/backend/postmaster/postmaster.c 
b/src/backend/postmaster/postmaster.c
index 5e8cd770c0..52fb2e52f1 100644
--- a/src/backend/postmaster/postmaster.c
+++ b/src/backend/postmaster/postmaster.c
@@ -2284,7 +2284,7 @@ retry1:
                                 */
                                if (strcmp(nameptr, "application_name") == 0)
                                {
-                                       port->application_name = 
pg_clean_ascii(valptr);
+                                       port->application_name = 
pg_clean_ascii(valptr, 0);
                                }
                        }
                        offset = valoffset + strlen(valptr) + 1;
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index 60400752e5..2f99fe9a6d 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -12479,9 +12479,18 @@ assign_maintenance_io_concurrency(int newval, void 
*extra)
 static bool
 check_application_name(char **newval, void **extra, GucSource source)
 {
+       char       *clean;
+
        /* Only allow clean ASCII chars in the application name */
-       *newval = guc_strdup(ERROR, pg_clean_ascii(*newval));
+       clean = pg_clean_ascii(*newval, MCXT_ALLOC_NO_OOM);
+       if (!clean)
+               return false;
+
+       clean = guc_strdup(WARNING, clean);
+       if (!clean)
+               return false;
 
+       *newval = clean;
        return true;
 }
 
@@ -12495,9 +12504,18 @@ assign_application_name(const char *newval, void 
*extra)
 static bool
 check_cluster_name(char **newval, void **extra, GucSource source)
 {
+       char       *clean;
+
        /* Only allow clean ASCII chars in the cluster name */
-       *newval = guc_strdup(ERROR, pg_clean_ascii(*newval));
+       clean = pg_clean_ascii(*newval, MCXT_ALLOC_NO_OOM);
+       if (!clean)
+               return false;
+
+       clean = guc_strdup(WARNING, clean);
+       if (!clean)
+               return false;
 
+       *newval = clean;
        return true;
 }
 
diff --git a/src/common/string.c b/src/common/string.c
index db15324c62..97b3d45d36 100644
--- a/src/common/string.c
+++ b/src/common/string.c
@@ -62,7 +62,10 @@ strtoint(const char *pg_restrict str, char **pg_restrict 
endptr, int base)
 /*
  * pg_clean_ascii -- Replace any non-ASCII chars with a "\xXX" string
  *
- * Makes a palloc'd copy of the string passed in, which must be 
'\0'-terminated.
+ * Makes a newly allocated copy of the string passed in, which must be
+ * '\0'-terminated. In the backend, additional alloc_flags may be provided and
+ * will be passed as-is to palloc_extended(); in the frontend, alloc_flags is
+ * ignored and the copy is malloc'd.
  *
  * This function exists specifically to deal with filtering out
  * non-ASCII characters in a few places where the client can provide an almost
@@ -80,23 +83,46 @@ strtoint(const char *pg_restrict str, char **pg_restrict 
endptr, int base)
  * for now.
  */
 char *
-pg_clean_ascii(const char *str)
+pg_clean_ascii(const char *str, int alloc_flags)
 {
-       StringInfoData buf;
-       const char   *p;
+       size_t          dstlen;
+       char       *dst;
+       const char *p;
+       size_t          i = 0;
 
-       initStringInfo(&buf);
+       /* Worst case, each byte can become four bytes, plus a null terminator. 
*/
+       dstlen = strlen(str) * 4 + 1;
+
+#ifdef FRONTEND
+       dst = malloc(dstlen);
+#else
+       dst = palloc_extended(dstlen, alloc_flags);
+#endif
+
+       if (!dst)
+               return NULL;
 
        for (p = str; *p != '\0'; p++)
        {
+
                /* Only allow clean ASCII chars in the string */
                if (*p < 32 || *p > 126)
-                       appendStringInfo(&buf, "\\x%02x", (unsigned char) *p);
+               {
+                       Assert(i < (dstlen - 3));
+                       snprintf(&dst[i], dstlen - i, "\\x%02x", (unsigned 
char) *p);
+                       i += 4;
+               }
                else
-                       appendStringInfoChar(&buf, *p);
+               {
+                       Assert(i < dstlen);
+                       dst[i] = *p;
+                       i++;
+               }
        }
 
-       return buf.data;
+       Assert(i < dstlen);
+       dst[i] = '\0';
+       return dst;
 }
 
 
diff --git a/src/include/common/string.h b/src/include/common/string.h
index d10d0c9cbf..3d59172151 100644
--- a/src/include/common/string.h
+++ b/src/include/common/string.h
@@ -24,7 +24,7 @@ typedef struct PromptInterruptContext
 extern bool pg_str_endswith(const char *str, const char *end);
 extern int     strtoint(const char *pg_restrict str, char **pg_restrict endptr,
                                         int base);
-extern char *pg_clean_ascii(const char *str);
+extern char *pg_clean_ascii(const char *str, int alloc_flags);
 extern int     pg_strip_crlf(char *str);
 extern bool pg_is_ascii(const char *str);
 
From 9d0330a7527ac2390b316db0915646137512109b Mon Sep 17 00:00:00 2001
From: Jacob Champion <jchamp...@timescale.com>
Date: Tue, 19 Jul 2022 10:48:58 -0700
Subject: [PATCH v4 1/2] pg_clean_ascii(): escape bytes rather than lose them

Rather than replace each unprintable byte with a '?' character, replace
it with a hex escape instead. The API now allocates a copy rather than
modifying the input in place.
---
 src/backend/postmaster/postmaster.c |  6 +---
 src/backend/utils/misc/guc.c        | 22 ++++++++++--
 src/common/string.c                 | 52 ++++++++++++++++++++++++-----
 src/include/common/string.h         |  2 +-
 4 files changed, 65 insertions(+), 17 deletions(-)

diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c
index 1c25457526..52fb2e52f1 100644
--- a/src/backend/postmaster/postmaster.c
+++ b/src/backend/postmaster/postmaster.c
@@ -2284,11 +2284,7 @@ retry1:
 				 */
 				if (strcmp(nameptr, "application_name") == 0)
 				{
-					char	   *tmp_app_name = pstrdup(valptr);
-
-					pg_clean_ascii(tmp_app_name);
-
-					port->application_name = tmp_app_name;
+					port->application_name = pg_clean_ascii(valptr, 0);
 				}
 			}
 			offset = valoffset + strlen(valptr) + 1;
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index 0328029d43..2f99fe9a6d 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -12479,9 +12479,18 @@ assign_maintenance_io_concurrency(int newval, void *extra)
 static bool
 check_application_name(char **newval, void **extra, GucSource source)
 {
+	char	   *clean;
+
 	/* Only allow clean ASCII chars in the application name */
-	pg_clean_ascii(*newval);
+	clean = pg_clean_ascii(*newval, MCXT_ALLOC_NO_OOM);
+	if (!clean)
+		return false;
+
+	clean = guc_strdup(WARNING, clean);
+	if (!clean)
+		return false;
 
+	*newval = clean;
 	return true;
 }
 
@@ -12495,9 +12504,18 @@ assign_application_name(const char *newval, void *extra)
 static bool
 check_cluster_name(char **newval, void **extra, GucSource source)
 {
+	char	   *clean;
+
 	/* Only allow clean ASCII chars in the cluster name */
-	pg_clean_ascii(*newval);
+	clean = pg_clean_ascii(*newval, MCXT_ALLOC_NO_OOM);
+	if (!clean)
+		return false;
+
+	clean = guc_strdup(WARNING, clean);
+	if (!clean)
+		return false;
 
+	*newval = clean;
 	return true;
 }
 
diff --git a/src/common/string.c b/src/common/string.c
index 16940d1fa7..97b3d45d36 100644
--- a/src/common/string.c
+++ b/src/common/string.c
@@ -22,6 +22,7 @@
 #endif
 
 #include "common/string.h"
+#include "lib/stringinfo.h"
 
 
 /*
@@ -59,9 +60,12 @@ strtoint(const char *pg_restrict str, char **pg_restrict endptr, int base)
 
 
 /*
- * pg_clean_ascii -- Replace any non-ASCII chars with a '?' char
+ * pg_clean_ascii -- Replace any non-ASCII chars with a "\xXX" string
  *
- * Modifies the string passed in which must be '\0'-terminated.
+ * Makes a newly allocated copy of the string passed in, which must be
+ * '\0'-terminated. In the backend, additional alloc_flags may be provided and
+ * will be passed as-is to palloc_extended(); in the frontend, alloc_flags is
+ * ignored and the copy is malloc'd.
  *
  * This function exists specifically to deal with filtering out
  * non-ASCII characters in a few places where the client can provide an almost
@@ -73,22 +77,52 @@ strtoint(const char *pg_restrict str, char **pg_restrict endptr, int base)
  * In general, this function should NOT be used- instead, consider how to handle
  * the string without needing to filter out the non-ASCII characters.
  *
- * Ultimately, we'd like to improve the situation to not require stripping out
- * all non-ASCII but perform more intelligent filtering which would allow UTF or
+ * Ultimately, we'd like to improve the situation to not require replacing all
+ * non-ASCII but perform more intelligent filtering which would allow UTF or
  * similar, but it's unclear exactly what we should allow, so stick to ASCII only
  * for now.
  */
-void
-pg_clean_ascii(char *str)
+char *
+pg_clean_ascii(const char *str, int alloc_flags)
 {
-	/* Only allow clean ASCII chars in the string */
-	char	   *p;
+	size_t		dstlen;
+	char	   *dst;
+	const char *p;
+	size_t		i = 0;
+
+	/* Worst case, each byte can become four bytes, plus a null terminator. */
+	dstlen = strlen(str) * 4 + 1;
+
+#ifdef FRONTEND
+	dst = malloc(dstlen);
+#else
+	dst = palloc_extended(dstlen, alloc_flags);
+#endif
+
+	if (!dst)
+		return NULL;
 
 	for (p = str; *p != '\0'; p++)
 	{
+
+		/* Only allow clean ASCII chars in the string */
 		if (*p < 32 || *p > 126)
-			*p = '?';
+		{
+			Assert(i < (dstlen - 3));
+			snprintf(&dst[i], dstlen - i, "\\x%02x", (unsigned char) *p);
+			i += 4;
+		}
+		else
+		{
+			Assert(i < dstlen);
+			dst[i] = *p;
+			i++;
+		}
 	}
+
+	Assert(i < dstlen);
+	dst[i] = '\0';
+	return dst;
 }
 
 
diff --git a/src/include/common/string.h b/src/include/common/string.h
index cf00fb53cd..3d59172151 100644
--- a/src/include/common/string.h
+++ b/src/include/common/string.h
@@ -24,7 +24,7 @@ typedef struct PromptInterruptContext
 extern bool pg_str_endswith(const char *str, const char *end);
 extern int	strtoint(const char *pg_restrict str, char **pg_restrict endptr,
 					 int base);
-extern void pg_clean_ascii(char *str);
+extern char *pg_clean_ascii(const char *str, int alloc_flags);
 extern int	pg_strip_crlf(char *str);
 extern bool pg_is_ascii(const char *str);
 
-- 
2.25.1

From e7cf0ab92f4f8d8405d8ca9735de4d83f126a3f9 Mon Sep 17 00:00:00 2001
From: Jacob Champion <jchamp...@timescale.com>
Date: Mon, 18 Jul 2022 15:01:19 -0700
Subject: [PATCH v4 2/2] Don't reflect unescaped cert data to the logs

Commit 3a0e385048 introduced a new path for unauthenticated bytes from
the client certificate to be printed unescaped to the logs. There are a
handful of these already, but it doesn't make sense to keep making the
problem worse. \x-escape any unprintable bytes.

The test case introduces a revoked UTF-8 certificate. This requires the
addition of the `-utf8` flag to `openssl req`. Since the existing
certificates all use an ASCII subset, this won't modify the existing
certificates' subjects if/when they get regenerated; this was verified
experimentally with

    $ make sslfiles-clean
    $ make sslfiles

Unfortunately the test can't be run in the CI yet due to a test timing
issue; see 55828a6b60.
---
 src/backend/libpq/be-secure-openssl.c         | 57 ++++++++++---------
 src/test/ssl/conf/client-revoked-utf8.config  | 13 +++++
 src/test/ssl/ssl/client-crldir/9bb9e3c3.r0    | 19 ++++---
 src/test/ssl/ssl/client-revoked-utf8.crt      | 18 ++++++
 src/test/ssl/ssl/client-revoked-utf8.key      | 27 +++++++++
 src/test/ssl/ssl/client.crl                   | 19 ++++---
 .../ssl/ssl/root+client-crldir/9bb9e3c3.r0    | 19 ++++---
 src/test/ssl/ssl/root+client.crl              | 19 ++++---
 src/test/ssl/sslfiles.mk                      | 10 ++--
 src/test/ssl/t/001_ssltests.pl                | 13 +++++
 src/test/ssl/t/SSL/Backend/OpenSSL.pm         |  3 +-
 11 files changed, 150 insertions(+), 67 deletions(-)
 create mode 100644 src/test/ssl/conf/client-revoked-utf8.config
 create mode 100644 src/test/ssl/ssl/client-revoked-utf8.crt
 create mode 100644 src/test/ssl/ssl/client-revoked-utf8.key

diff --git a/src/backend/libpq/be-secure-openssl.c b/src/backend/libpq/be-secure-openssl.c
index 9cec6866a3..1394ecaa2b 100644
--- a/src/backend/libpq/be-secure-openssl.c
+++ b/src/backend/libpq/be-secure-openssl.c
@@ -29,12 +29,14 @@
 #include <arpa/inet.h>
 #endif
 
+#include "common/string.h"
 #include "libpq/libpq.h"
 #include "miscadmin.h"
 #include "pgstat.h"
 #include "storage/fd.h"
 #include "storage/latch.h"
 #include "tcop/tcopprot.h"
+#include "utils/builtins.h"
 #include "utils/memutils.h"
 
 /*
@@ -1082,16 +1084,16 @@ dummy_ssl_passwd_cb(char *buf, int size, int rwflag, void *userdata)
 }
 
 /*
- * Examines the provided certificate name, and if it's too long to log, modifies
- * and truncates it. The return value is NULL if no truncation was needed; it
- * otherwise points into the middle of the input string, and should not be
- * freed.
+ * Examines the provided certificate name, and if it's too long to log or
+ * contains unprintable ASCII, escapes and truncates it. The return value is
+ * always a new palloc'd string. (The input string is still modified in place,
+ * for ease of implementation.)
  */
 static char *
-truncate_cert_name(char *name)
+prepare_cert_name(char *name)
 {
 	size_t		namelen = strlen(name);
-	char	   *truncated;
+	char	   *truncated = name;
 
 	/*
 	 * Common Names are 64 chars max, so for a common case where the CN is the
@@ -1101,19 +1103,20 @@ truncate_cert_name(char *name)
 	 */
 #define MAXLEN 71
 
-	if (namelen <= MAXLEN)
-		return NULL;
-
-	/*
-	 * Keep the end of the name, not the beginning, since the most specific
-	 * field is likely to give users the most information.
-	 */
-	truncated = name + namelen - MAXLEN;
-	truncated[0] = truncated[1] = truncated[2] = '.';
+	if (namelen > MAXLEN)
+	{
+		/*
+		 * Keep the end of the name, not the beginning, since the most specific
+		 * field is likely to give users the most information.
+		 */
+		truncated = name + namelen - MAXLEN;
+		truncated[0] = truncated[1] = truncated[2] = '.';
+		namelen = MAXLEN;
+	}
 
 #undef MAXLEN
 
-	return truncated;
+	return pg_clean_ascii(truncated, 0);
 }
 
 /*
@@ -1156,21 +1159,24 @@ verify_cb(int ok, X509_STORE_CTX *ctx)
 	{
 		char	   *subject,
 				   *issuer;
-		char	   *sub_truncated,
-				   *iss_truncated;
+		char	   *sub_prepared,
+				   *iss_prepared;
 		char	   *serialno;
 		ASN1_INTEGER *sn;
 		BIGNUM	   *b;
 
 		/*
 		 * Get the Subject and Issuer for logging, but don't let maliciously
-		 * huge certs flood the logs.
+		 * huge certs flood the logs, and don't reflect non-ASCII bytes into it
+		 * either.
 		 */
 		subject = X509_NAME_to_cstring(X509_get_subject_name(cert));
-		sub_truncated = truncate_cert_name(subject);
+		sub_prepared = prepare_cert_name(subject);
+		pfree(subject);
 
 		issuer = X509_NAME_to_cstring(X509_get_issuer_name(cert));
-		iss_truncated = truncate_cert_name(issuer);
+		iss_prepared = prepare_cert_name(issuer);
+		pfree(issuer);
 
 		/*
 		 * Pull the serial number, too, in case a Subject is still ambiguous.
@@ -1183,14 +1189,13 @@ verify_cb(int ok, X509_STORE_CTX *ctx)
 		appendStringInfoChar(&str, '\n');
 		appendStringInfo(&str,
 						 _("Failed certificate data (unverified): subject \"%s\", serial number %s, issuer \"%s\"."),
-						 sub_truncated ? sub_truncated : subject,
-						 serialno ? serialno : _("unknown"),
-						 iss_truncated ? iss_truncated : issuer);
+						 sub_prepared, serialno ? serialno : _("unknown"),
+						 iss_prepared);
 
 		BN_free(b);
 		OPENSSL_free(serialno);
-		pfree(issuer);
-		pfree(subject);
+		pfree(iss_prepared);
+		pfree(sub_prepared);
 	}
 
 	/* Store our detail message to be logged later. */
diff --git a/src/test/ssl/conf/client-revoked-utf8.config b/src/test/ssl/conf/client-revoked-utf8.config
new file mode 100644
index 0000000000..ff57e6de8a
--- /dev/null
+++ b/src/test/ssl/conf/client-revoked-utf8.config
@@ -0,0 +1,13 @@
+# An OpenSSL format CSR config file for creating a client certificate.
+#
+# The certificate contains a non-ASCII CN encoded in UTF-8. It is revoked by the
+# client CA.
+
+[ req ]
+distinguished_name     = req_distinguished_name
+prompt                 = no
+
+[ req_distinguished_name ]
+CN                     = Οδυσσέας
+
+# no extensions in client certs
diff --git a/src/test/ssl/ssl/client-crldir/9bb9e3c3.r0 b/src/test/ssl/ssl/client-crldir/9bb9e3c3.r0
index d93791b88a..f75eb1c0bc 100644
--- a/src/test/ssl/ssl/client-crldir/9bb9e3c3.r0
+++ b/src/test/ssl/ssl/client-crldir/9bb9e3c3.r0
@@ -1,11 +1,12 @@
 -----BEGIN X509 CRL-----
-MIIBpTCBjjANBgkqhkiG9w0BAQsFADBCMUAwPgYDVQQDDDdUZXN0IENBIGZvciBQ
-b3N0Z3JlU1FMIFNTTCByZWdyZXNzaW9uIHRlc3QgY2xpZW50IGNlcnRzFw0yMTAz
-MDMyMjEyMDdaFw00ODA3MTkyMjEyMDdaMBswGQIIICEDAxQSBwEXDTIxMDMwMzIy
-MTIwN1owDQYJKoZIhvcNAQELBQADggEBAC1AJ+HhHg74uXNXdoXLnqDhowdx1y3z
-GKSTPH4iW6jvGp7mMeJhq7cx5kzC+Rqtjui7FjkXbvGd4f6ZVKf30tDD/LvVLxLU
-Up7TmwZjYHbB4NPMyMyqUxtusjYm6HFhbfJwf11TQFwF9yRN3MI4os3J9KTzvhY1
-AvfyEqhBdeygkc1cDduZD+cx7QFYtaeD316q4lz8yfegtxwng8/JDlThu72zdpWV
-w0LuzLei1A9cPXoXfMxRGVEOrDt5z3ArNqdD0bnXTTYqm1IX8ZRHDNeUi4NuFCCu
-tKWT4j9ad4mMcJ6TY/8MiJ14mSJmWSR8115QT69rrQIdDu0sA/sBJX0=
+MIIBwDCBqTANBgkqhkiG9w0BAQsFADBCMUAwPgYDVQQDDDdUZXN0IENBIGZvciBQ
+b3N0Z3JlU1FMIFNTTCByZWdyZXNzaW9uIHRlc3QgY2xpZW50IGNlcnRzFw0yMjA3
+MTgyMjI4MTVaFw00OTEyMDMyMjI4MTVaMDYwGQIIICEDAxQSBwEXDTIyMDcxODIy
+MjgxNVowGQIIICIHGBUoFQAXDTIyMDcxODIyMjgxNVowDQYJKoZIhvcNAQELBQAD
+ggEBAFDH3m9AHpDjkEFjO6svnLJ2bTliGeKZaJW8/RAN4mWvWDhXDQfzqGcFHN2a
+SIL57Xc4PdwTiXuU4QEP4RvWW90LYKdcrcT8uh0AN3i7ShMwcV7I7owzF5+CBuT7
+Ev0MU4QIz0PjXoybXP6b3wHhZbEjYTLYdnYdqjrsAchUpyDQn6fiC0C7FgjCi4HL
+rNm2kMchFpzd6K9e41kxWVp7xCPXgqUK8OrxlW56ObkX8UpBIZzyU6RisJKOZJAn
+/+lwT43yTtU739atdXdSMvGHT9Y7LsrSDz9zgp2/iMTmfctnPcp81J/6jQZEP8kx
+OyPyZz4xy/EShWy+KUklfOoKRo8=
 -----END X509 CRL-----
diff --git a/src/test/ssl/ssl/client-revoked-utf8.crt b/src/test/ssl/ssl/client-revoked-utf8.crt
new file mode 100644
index 0000000000..de471d1e60
--- /dev/null
+++ b/src/test/ssl/ssl/client-revoked-utf8.crt
@@ -0,0 +1,18 @@
+-----BEGIN CERTIFICATE-----
+MIIC2DCCAcACCCAiBxgVKBUAMA0GCSqGSIb3DQEBCwUAMEIxQDA+BgNVBAMMN1Rl
+c3QgQ0EgZm9yIFBvc3RncmVTUUwgU1NMIHJlZ3Jlc3Npb24gdGVzdCBjbGllbnQg
+Y2VydHMwHhcNMjIwNzE4MjIyODE1WhcNNDkxMjAzMjIyODE1WjAbMRkwFwYDVQQD
+DBDOn860z4XPg8+Dzq3Osc+CMIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKC
+AQEAvBiL1mVjTrzZ6sbrvfu746dzh+EEyuJNkCwPeJTtpva2wqqRUMYw05cV5kzi
+YQ3UikMP5Yz0FXTeWoahSpJAWeR5XsFx3wOQvRzwi1KWm2CHr/rb7KbPvoZQdXuV
+8UeKrQ6PrEvjoarHAUZuWyUC6EnEAGuiKl5yuax5mkTcK5F8pig2/SS/UonX5ar5
+58rOUEaIdyZmXtrO86cm5S5Oz3G2naQB3PPPOhtkoGBHikRHiqBPVRpX3w9TIpBL
+BZbT4MIZ+fCjZ9wXj4aiDUzPglu6/Tfx9sNcxc6Ilz/XHfPuBVyyjgrny2SrW0W4
+KlhU09y+m5gKL358z8tj599DowIDAQABMA0GCSqGSIb3DQEBCwUAA4IBAQAE47ns
+wfceztieaRQtoF+gPcCuImOJqaB7jTE6cQ+aoW/q+sUlOh7AD0IZqhS4o0A4O+ny
+MD7kHkpYP+ctHNomsSQRkFTDZ2ZJGcRgxbwMOSvsKcgNOTMGqpXQiP0x0m7QMBGl
+EHeu5MqG/IK/ZlH9aOTvSnHegB6ztct/7wXMeFCflsWLp6wvnv9YpddaaXf95Oms
+9kwbVYkI1wxaBsAO8VGbJw1YtdErgd65qKTJa45xndtm61i1Jeig5asSNQPwjfZ5
+aNHZ9GsSwsc31Q/6iiezbPwgdAi3ih//uB2hznkMhObnqzR3n8Sw9zgL7DdFr2y9
+2R7kJuGq6DvlWFYS
+-----END CERTIFICATE-----
diff --git a/src/test/ssl/ssl/client-revoked-utf8.key b/src/test/ssl/ssl/client-revoked-utf8.key
new file mode 100644
index 0000000000..86d9697096
--- /dev/null
+++ b/src/test/ssl/ssl/client-revoked-utf8.key
@@ -0,0 +1,27 @@
+-----BEGIN RSA PRIVATE KEY-----
+MIIEpAIBAAKCAQEAvBiL1mVjTrzZ6sbrvfu746dzh+EEyuJNkCwPeJTtpva2wqqR
+UMYw05cV5kziYQ3UikMP5Yz0FXTeWoahSpJAWeR5XsFx3wOQvRzwi1KWm2CHr/rb
+7KbPvoZQdXuV8UeKrQ6PrEvjoarHAUZuWyUC6EnEAGuiKl5yuax5mkTcK5F8pig2
+/SS/UonX5ar558rOUEaIdyZmXtrO86cm5S5Oz3G2naQB3PPPOhtkoGBHikRHiqBP
+VRpX3w9TIpBLBZbT4MIZ+fCjZ9wXj4aiDUzPglu6/Tfx9sNcxc6Ilz/XHfPuBVyy
+jgrny2SrW0W4KlhU09y+m5gKL358z8tj599DowIDAQABAoIBAQCpdePmUInb0kDy
+SCzziOyJ+b8YWB4dOy1uCoQVuvcxSWz2jP1GrIgo2SGdzv7VOcSWnDyiLw9olVYO
+cOS3bnQTiMfgGqAgr/Gir4P3wXx2l80nOvcQimj32cJ7VdCNBEtoBopiTCzU5itM
+dsvNydaIuIyhZFdBnL33kfAskIVbqbgNyMCuDvhEMGFh7T193j3cKnvcfoHsSoNK
+65MT53764P404avgH9+C0W41GvXoMY5BUphUiCwi5TKIvalNP9Gu2LI3R4J3tAE1
+QSR/3Jtaunb5izCyi23MZC+mdz9EALeYRmLpXsspbHaPXDpUA67xifCKnX4JUPVf
+Op5XcXjhAoGBAN/Jt7TFPypRtbW200zx6F3RmToWRnvgn5vaNTmiy0ivcHJu4MLS
+o0yiV3VWksf1PCInK19C1yFo6H2lkhKhvipL62MoRkspOUJcMh42DPPf/RDMhYnF
+8MVQ1TlMdg/I4YXGzsrfl93eFERRjWiAt8b58D9OVpWcQNZMPmGztes3AoGBANcr
+n8ZmGZ5JDzbc+N9l1bGJuRT6PvH0rpoKjWOyaVMDedAUnCbfq01j42zXMfQLR1nE
+67Z6oWrBNNdEJFBhTzTZ+ZYXxpJP/FYJQ19dOCTKN1LQ79OAbSsU0NBLkss4a903
+9JQ+zhrEIEaXCTV9sEnp10KrEo6ctuaqMOkVCBj1AoGBAJy/Xb1wq12o/e3ZsQck
+Ke4M8ZaOI7CBFUrE/KLyNBElUU3V+/h6MYdr7nZxvT3xt7z0UpzW5HiyUqYvYrFK
+OTjHFIjPnOzoYwLoMPKYSVpIealal+54hryucatAszE7MzvQlOfk1SrCcs+nj7Sy
+9Aaa6nxtEpiYaZGwtcEZb0LhAoGAJYODjbGLUd9m+ae49CnrAdMDI7cldkW0k0K3
+t+QJHOIEQNT3DIf+c7Wwlu9F1EiLHgmJFv12WwhoUAefVSxCBPLj4tkuU6ACXHWs
++1ljSna/An9O8M75OYOdjFNAupGRrLXuvFHe2SfMgMIgZuUM8TYFw6fTym1kLf8K
+G/kAumkCgYBBD0TXDDAmVCYECSG1Uz35vm9GitbIe++o2ykO2sdB5mPRiMsfVJw4
+bVInkvV6Y2u4ltsNsS/0Y3A2xq/CnYhc7PeIIWFnfoyuHaIM4TIAflpM6qf4lOWE
+8Ot31P8Mt5U0cvCBuKpu0r9by66xX6yqKCqTPMSvbL7MCx5ukGYY7g==
+-----END RSA PRIVATE KEY-----
diff --git a/src/test/ssl/ssl/client.crl b/src/test/ssl/ssl/client.crl
index d93791b88a..f75eb1c0bc 100644
--- a/src/test/ssl/ssl/client.crl
+++ b/src/test/ssl/ssl/client.crl
@@ -1,11 +1,12 @@
 -----BEGIN X509 CRL-----
-MIIBpTCBjjANBgkqhkiG9w0BAQsFADBCMUAwPgYDVQQDDDdUZXN0IENBIGZvciBQ
-b3N0Z3JlU1FMIFNTTCByZWdyZXNzaW9uIHRlc3QgY2xpZW50IGNlcnRzFw0yMTAz
-MDMyMjEyMDdaFw00ODA3MTkyMjEyMDdaMBswGQIIICEDAxQSBwEXDTIxMDMwMzIy
-MTIwN1owDQYJKoZIhvcNAQELBQADggEBAC1AJ+HhHg74uXNXdoXLnqDhowdx1y3z
-GKSTPH4iW6jvGp7mMeJhq7cx5kzC+Rqtjui7FjkXbvGd4f6ZVKf30tDD/LvVLxLU
-Up7TmwZjYHbB4NPMyMyqUxtusjYm6HFhbfJwf11TQFwF9yRN3MI4os3J9KTzvhY1
-AvfyEqhBdeygkc1cDduZD+cx7QFYtaeD316q4lz8yfegtxwng8/JDlThu72zdpWV
-w0LuzLei1A9cPXoXfMxRGVEOrDt5z3ArNqdD0bnXTTYqm1IX8ZRHDNeUi4NuFCCu
-tKWT4j9ad4mMcJ6TY/8MiJ14mSJmWSR8115QT69rrQIdDu0sA/sBJX0=
+MIIBwDCBqTANBgkqhkiG9w0BAQsFADBCMUAwPgYDVQQDDDdUZXN0IENBIGZvciBQ
+b3N0Z3JlU1FMIFNTTCByZWdyZXNzaW9uIHRlc3QgY2xpZW50IGNlcnRzFw0yMjA3
+MTgyMjI4MTVaFw00OTEyMDMyMjI4MTVaMDYwGQIIICEDAxQSBwEXDTIyMDcxODIy
+MjgxNVowGQIIICIHGBUoFQAXDTIyMDcxODIyMjgxNVowDQYJKoZIhvcNAQELBQAD
+ggEBAFDH3m9AHpDjkEFjO6svnLJ2bTliGeKZaJW8/RAN4mWvWDhXDQfzqGcFHN2a
+SIL57Xc4PdwTiXuU4QEP4RvWW90LYKdcrcT8uh0AN3i7ShMwcV7I7owzF5+CBuT7
+Ev0MU4QIz0PjXoybXP6b3wHhZbEjYTLYdnYdqjrsAchUpyDQn6fiC0C7FgjCi4HL
+rNm2kMchFpzd6K9e41kxWVp7xCPXgqUK8OrxlW56ObkX8UpBIZzyU6RisJKOZJAn
+/+lwT43yTtU739atdXdSMvGHT9Y7LsrSDz9zgp2/iMTmfctnPcp81J/6jQZEP8kx
+OyPyZz4xy/EShWy+KUklfOoKRo8=
 -----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
index d93791b88a..f75eb1c0bc 100644
--- a/src/test/ssl/ssl/root+client-crldir/9bb9e3c3.r0
+++ b/src/test/ssl/ssl/root+client-crldir/9bb9e3c3.r0
@@ -1,11 +1,12 @@
 -----BEGIN X509 CRL-----
-MIIBpTCBjjANBgkqhkiG9w0BAQsFADBCMUAwPgYDVQQDDDdUZXN0IENBIGZvciBQ
-b3N0Z3JlU1FMIFNTTCByZWdyZXNzaW9uIHRlc3QgY2xpZW50IGNlcnRzFw0yMTAz
-MDMyMjEyMDdaFw00ODA3MTkyMjEyMDdaMBswGQIIICEDAxQSBwEXDTIxMDMwMzIy
-MTIwN1owDQYJKoZIhvcNAQELBQADggEBAC1AJ+HhHg74uXNXdoXLnqDhowdx1y3z
-GKSTPH4iW6jvGp7mMeJhq7cx5kzC+Rqtjui7FjkXbvGd4f6ZVKf30tDD/LvVLxLU
-Up7TmwZjYHbB4NPMyMyqUxtusjYm6HFhbfJwf11TQFwF9yRN3MI4os3J9KTzvhY1
-AvfyEqhBdeygkc1cDduZD+cx7QFYtaeD316q4lz8yfegtxwng8/JDlThu72zdpWV
-w0LuzLei1A9cPXoXfMxRGVEOrDt5z3ArNqdD0bnXTTYqm1IX8ZRHDNeUi4NuFCCu
-tKWT4j9ad4mMcJ6TY/8MiJ14mSJmWSR8115QT69rrQIdDu0sA/sBJX0=
+MIIBwDCBqTANBgkqhkiG9w0BAQsFADBCMUAwPgYDVQQDDDdUZXN0IENBIGZvciBQ
+b3N0Z3JlU1FMIFNTTCByZWdyZXNzaW9uIHRlc3QgY2xpZW50IGNlcnRzFw0yMjA3
+MTgyMjI4MTVaFw00OTEyMDMyMjI4MTVaMDYwGQIIICEDAxQSBwEXDTIyMDcxODIy
+MjgxNVowGQIIICIHGBUoFQAXDTIyMDcxODIyMjgxNVowDQYJKoZIhvcNAQELBQAD
+ggEBAFDH3m9AHpDjkEFjO6svnLJ2bTliGeKZaJW8/RAN4mWvWDhXDQfzqGcFHN2a
+SIL57Xc4PdwTiXuU4QEP4RvWW90LYKdcrcT8uh0AN3i7ShMwcV7I7owzF5+CBuT7
+Ev0MU4QIz0PjXoybXP6b3wHhZbEjYTLYdnYdqjrsAchUpyDQn6fiC0C7FgjCi4HL
+rNm2kMchFpzd6K9e41kxWVp7xCPXgqUK8OrxlW56ObkX8UpBIZzyU6RisJKOZJAn
+/+lwT43yTtU739atdXdSMvGHT9Y7LsrSDz9zgp2/iMTmfctnPcp81J/6jQZEP8kx
+OyPyZz4xy/EShWy+KUklfOoKRo8=
 -----END X509 CRL-----
diff --git a/src/test/ssl/ssl/root+client.crl b/src/test/ssl/ssl/root+client.crl
index 02eff4d326..459f48da43 100644
--- a/src/test/ssl/ssl/root+client.crl
+++ b/src/test/ssl/ssl/root+client.crl
@@ -10,13 +10,14 @@ SBNr2rpYp7Coc3GeCoWPcClgSrABD3Z5GY1YAdLGiXVKaH3CmdJTznhEPagE4z5R
 +GrJP3XxJ1OC
 -----END X509 CRL-----
 -----BEGIN X509 CRL-----
-MIIBpTCBjjANBgkqhkiG9w0BAQsFADBCMUAwPgYDVQQDDDdUZXN0IENBIGZvciBQ
-b3N0Z3JlU1FMIFNTTCByZWdyZXNzaW9uIHRlc3QgY2xpZW50IGNlcnRzFw0yMTAz
-MDMyMjEyMDdaFw00ODA3MTkyMjEyMDdaMBswGQIIICEDAxQSBwEXDTIxMDMwMzIy
-MTIwN1owDQYJKoZIhvcNAQELBQADggEBAC1AJ+HhHg74uXNXdoXLnqDhowdx1y3z
-GKSTPH4iW6jvGp7mMeJhq7cx5kzC+Rqtjui7FjkXbvGd4f6ZVKf30tDD/LvVLxLU
-Up7TmwZjYHbB4NPMyMyqUxtusjYm6HFhbfJwf11TQFwF9yRN3MI4os3J9KTzvhY1
-AvfyEqhBdeygkc1cDduZD+cx7QFYtaeD316q4lz8yfegtxwng8/JDlThu72zdpWV
-w0LuzLei1A9cPXoXfMxRGVEOrDt5z3ArNqdD0bnXTTYqm1IX8ZRHDNeUi4NuFCCu
-tKWT4j9ad4mMcJ6TY/8MiJ14mSJmWSR8115QT69rrQIdDu0sA/sBJX0=
+MIIBwDCBqTANBgkqhkiG9w0BAQsFADBCMUAwPgYDVQQDDDdUZXN0IENBIGZvciBQ
+b3N0Z3JlU1FMIFNTTCByZWdyZXNzaW9uIHRlc3QgY2xpZW50IGNlcnRzFw0yMjA3
+MTgyMjI4MTVaFw00OTEyMDMyMjI4MTVaMDYwGQIIICEDAxQSBwEXDTIyMDcxODIy
+MjgxNVowGQIIICIHGBUoFQAXDTIyMDcxODIyMjgxNVowDQYJKoZIhvcNAQELBQAD
+ggEBAFDH3m9AHpDjkEFjO6svnLJ2bTliGeKZaJW8/RAN4mWvWDhXDQfzqGcFHN2a
+SIL57Xc4PdwTiXuU4QEP4RvWW90LYKdcrcT8uh0AN3i7ShMwcV7I7owzF5+CBuT7
+Ev0MU4QIz0PjXoybXP6b3wHhZbEjYTLYdnYdqjrsAchUpyDQn6fiC0C7FgjCi4HL
+rNm2kMchFpzd6K9e41kxWVp7xCPXgqUK8OrxlW56ObkX8UpBIZzyU6RisJKOZJAn
+/+lwT43yTtU739atdXdSMvGHT9Y7LsrSDz9zgp2/iMTmfctnPcp81J/6jQZEP8kx
+OyPyZz4xy/EShWy+KUklfOoKRo8=
 -----END X509 CRL-----
diff --git a/src/test/ssl/sslfiles.mk b/src/test/ssl/sslfiles.mk
index 5f67aba795..a843a21d42 100644
--- a/src/test/ssl/sslfiles.mk
+++ b/src/test/ssl/sslfiles.mk
@@ -33,7 +33,8 @@ SERVERS := server-cn-and-alt-names \
 	server-multiple-alt-names \
 	server-no-names \
 	server-revoked
-CLIENTS := client client-dn client-revoked client_ext client-long
+CLIENTS := client client-dn client-revoked client_ext client-long \
+	client-revoked-utf8
 
 #
 # To add a new non-standard key, add it to SPECIAL_KEYS and then add a recipe
@@ -175,7 +176,7 @@ $(CLIENT_CERTS): ssl/%.crt: ssl/%.csr conf/%.config conf/cas.config ssl/client_c
 # The CSRs don't need to persist after a build.
 .INTERMEDIATE: $(CERTIFICATES:%=ssl/%.csr)
 ssl/%.csr: ssl/%.key conf/%.config
-	openssl req -new -key $< -out $@ -config conf/$*.config
+	openssl req -new -utf8 -key $< -out $@ -config conf/$*.config
 
 #
 # CA State
@@ -215,8 +216,9 @@ ssl/server.crl: ssl/server-revoked.crt ssl/server_ca.crt | $(server_ca_state_fil
 	openssl ca -config conf/cas.config -name server_ca -revoke $<
 	openssl ca -config conf/cas.config -name server_ca -gencrl -out $@
 
-ssl/client.crl: ssl/client-revoked.crt ssl/client_ca.crt | $(client_ca_state_files)
-	openssl ca -config conf/cas.config -name client_ca -revoke $<
+ssl/client.crl: ssl/client-revoked.crt ssl/client-revoked-utf8.crt ssl/client_ca.crt | $(client_ca_state_files)
+	openssl ca -config conf/cas.config -name client_ca -revoke ssl/client-revoked.crt
+	openssl ca -config conf/cas.config -name client_ca -revoke ssl/client-revoked-utf8.crt
 	openssl ca -config conf/cas.config -name client_ca -gencrl -out $@
 
 #
diff --git a/src/test/ssl/t/001_ssltests.pl b/src/test/ssl/t/001_ssltests.pl
index fa26212de9..efab2ac194 100644
--- a/src/test/ssl/t/001_ssltests.pl
+++ b/src/test/ssl/t/001_ssltests.pl
@@ -793,4 +793,17 @@ $node->connect_fails(
 #	]
 );
 
+# revoked client cert, non-ASCII subject
+$node->connect_fails(
+	"$common_connstr user=ssltestuser sslcert=ssl/client-revoked-utf8.crt "
+	  . sslkey('client-revoked-utf8.key'),
+	"certificate authorization fails with revoked UTF-8 client cert with server-side CRL directory",
+	expected_stderr => qr/SSL error: sslv3 alert certificate revoked/,
+	# temporarily(?) skip this check due to timing issue
+#	log_like => [
+#		qr{Client certificate verification failed at depth 0: certificate revoked},
+#		qr{Failed certificate data \(unverified\): subject "/CN=\\xce\\x9f\\xce\\xb4\\xcf\\x85\\xcf\\x83\\xcf\\x83\\xce\\xad\\xce\\xb1\\xcf\\x82", serial number 2315420958437414144, issuer "/CN=Test CA for PostgreSQL SSL regression test client certs"},
+#	]
+);
+
 done_testing();
diff --git a/src/test/ssl/t/SSL/Backend/OpenSSL.pm b/src/test/ssl/t/SSL/Backend/OpenSSL.pm
index a43e64c04f..6386a25323 100644
--- a/src/test/ssl/t/SSL/Backend/OpenSSL.pm
+++ b/src/test/ssl/t/SSL/Backend/OpenSSL.pm
@@ -88,7 +88,8 @@ sub init
 		"client.key",               "client-revoked.key",
 		"client-der.key",           "client-encrypted-pem.key",
 		"client-encrypted-der.key", "client-dn.key",
-		"client_ext.key",           "client-long.key");
+		"client_ext.key",           "client-long.key",
+		"client-revoked-utf8.key");
 	foreach my $keyfile (@keys)
 	{
 		copy("ssl/$keyfile", "$cert_tempdir/$keyfile")
-- 
2.25.1

Reply via email to