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