On 3/31/23 02:14, Daniel Gustafsson wrote:
>> On 14 Mar 2023, at 20:20, Jacob Champion <jchamp...@timescale.com> wrote:
>
>> Rebased over yesterday's Meson changes in v8.
>
> I had a look at this and agree that it's something we should do.
Great, thanks for the review!
> + # Let tests differentiate between vanilla OpenSSL and LibreSSL.
> + AC_CHECK_DECLS([LIBRESSL_VERSION_NUMBER], [], [], [#include
> <openssl/opensslv.h>])
> We have a check for SSL_CTX_set_cert_cb which is specifically done since it's
> not present in Libressl. Rather than spending more cycles in autoconf/meson,
> couldn't we use HAVE_SSL_CTX_SET_CERT_CB for this test? (Longer term, maybe
> we
> should make the checks properly distinguish between OpenSSL and LibreSSL as
> they are diverging, but thats not for this patch to tackle.)
I can make that change; note that it'll also skip some of the new tests
with OpenSSL 1.0.1, where there's no SSL_CTX_set_cert_cb. If that's
acceptable, it should be an easy switch.
> I can agree with the comment that this seems brittle. How about moving the
> installation of openssl to after the brew cleanup stage to avoid the need for
> this? While that may leave more in the cache, it seems more palatable.
> Something like this essentially:
>
> brew install <everything but openssl>
> brew cleanup -s
> # Comment about why OpenSSL is kept separate
> brew install openssl@3
That looks much better to me, but it didn't work when I tried it. One or
more of the packages above it (and/or the previous cache?) has already
installed OpenSSL as one of its dependencies, so the last `brew install`
becomes a no-op. I tried an `install --force` as well, but that didn't
seem to do anything differently. :/
> + libpq_append_conn_error(conn, "weak sslmode \"%s\" may not be used
> with sslrootcert=system",
> + conn->sslmode);
> I think we should help the user by indicating which sslmode we allow in this
> message.
Added in v9.
> +
> + /*
> + * sslmode is not specified. Let it be filled in with the compiled
> + * default for now, but if sslrootcert=system, we'll override the
> + * default later before returning.
> + */
> + sslmode_default = option;
> As a not to self and other reviewers, "git am" misplaced this when applying
> the
> patch such that the result was syntactically correct but semantically wrong,
> causing very weird test errors.
Lovely... I've formatted v9 with a longer patch context.
> + sslmode_default->val = strdup("verify-full");
> This needs to be checked for OOM error.
Whoops, should be fixed now.
> - if (fnbuf[0] != '\0' &&
> - stat(fnbuf, &buf) == 0)
> + if (strcmp(fnbuf, "system") == 0)
> I'm not a fan of magic values, but sadly I don't have a better idea for this.
> We should however document that the keyword takes precedence over a file with
> the same name (even though the collision is unlikely).
Added a note to the docs.
> + if (SSL_CTX_set_default_verify_paths(SSL_context) != 1)
> OpenSSL documents this as "A missing default location is still treated as a
> success.", is that something we need to document or in any way deal with?
> (Skimming the OpenSSL code I'm not sure it's actually correct in v3+, but I
> might very well have missed something.)
I think it's still true in v3+, because that sounds exactly like the
brew issue we're working around in Cirrus. I'm not sure if there's much
for us to do in that case, short of reimplementing the OpenSSL defaults
logic and checking it ourselves. (And even that would look different
between OpenSSL and LibreSSL...)
Is there something we could document that's more helpful than "make sure
your installation isn't broken"?
--Jacob1: 84f67249e6 ! 1: 18fd368e0e libpq: add sslrootcert=system to use default CAs
@@ doc/src/sgml/libpq.sgml: postgresql://%2Fvar%2Flib%2Fpostgresql/dbname
+ they control, rendering the <literal>verify-ca</literal> mode
useless.
+ </para>
+ </warning>
++ <note>
++ <para>
++ The magic <literal>system</literal> value will take precedence
over a
++ local certificate file with the same name. If for some reason
you find
++ yourself in this situation, use an alternative path like
++ <literal>sslrootcert=./system</literal> instead.
++ </para>
++ </note>
</listitem>
</varlistentry>
2: 11b69d0bc0 ! 2: ba09e1d83f libpq: force sslmode=verify-full for system CAs
@@ doc/src/sgml/libpq.sgml: postgresql://%2Fvar%2Flib%2Fpostgresql/dbname
+ weaker modes useless.
</para>
- </warning>
-+ </note>
- </listitem>
- </varlistentry>
-
+- <note>
+ <para>
+ The magic <literal>system</literal> value will take precedence
over a
+ local certificate file with the same name. If for some reason
you find
## doc/src/sgml/runtime.sgml ##
@@ doc/src/sgml/runtime.sgml: pg_dumpall -p 5432 | psql -d postgres -p 5433
@@ src/interfaces/libpq/fe-connect.c: connectOptions2(PGconn *conn)
+ && strcmp(conn->sslmode, "verify-full") != 0)
+ {
+ conn->status = CONNECTION_BAD;
-+ libpq_append_conn_error(conn, "weak sslmode \"%s\" may not be
used with sslrootcert=system",
++ libpq_append_conn_error(conn, "weak sslmode \"%s\" may not be
used with sslrootcert=system (use verify-full)",
+ conn->sslmode);
+ return false;
+ }
@@ src/interfaces/libpq/fe-connect.c:
conninfo_add_defaults(PQconninfoOption *optio
+ if (sslrootcert->val && strcmp(sslrootcert->val, "system") == 0)
+ {
+ free(sslmode_default->val);
++
+ sslmode_default->val = strdup("verify-full");
++ if (!sslmode_default->val)
++ {
++ if (errorMessage)
++ libpq_append_error(errorMessage, "out
of memory");
++ return false;
++ }
+ }
+ }
+
From 18fd368e0e3f7008cd98bea49e1c701f63e3903d Mon Sep 17 00:00:00 2001
From: Jacob Champion <jchamp...@timescale.com>
Date: Mon, 24 Oct 2022 15:30:25 -0700
Subject: [PATCH v9 1/2] libpq: add sslrootcert=system to use default CAs
Based on a patch by Thomas Habets.
Note the workaround in .cirrus.yml for a strange interaction between
brew and the openssl@3 formula; hopefully this can be removed in the
near future.
Discussion: https://www.postgresql.org/message-id/flat/CA%2BkHd%2BcJwCUxVb-Gj_0ptr3_KZPwi3%2B67vK6HnLFBK9MzuYrLA%40mail.gmail.com
---
.cirrus.yml | 14 +-
configure | 289 +++++++++---------
configure.ac | 2 +
doc/src/sgml/libpq.sgml | 25 ++
doc/src/sgml/runtime.sgml | 6 +-
meson.build | 9 +
src/include/pg_config.h.in | 4 +
src/interfaces/libpq/fe-secure-openssl.c | 29 +-
src/test/ssl/ssl/server-cn-only+server_ca.crt | 38 +++
src/test/ssl/sslfiles.mk | 6 +-
src/test/ssl/t/001_ssltests.pl | 29 ++
11 files changed, 306 insertions(+), 145 deletions(-)
create mode 100644 src/test/ssl/ssl/server-cn-only+server_ca.crt
diff --git a/.cirrus.yml b/.cirrus.yml
index 04786174ed..d3f88821a8 100644
--- a/.cirrus.yml
+++ b/.cirrus.yml
@@ -475,16 +475,28 @@ task:
llvm \
lz4 \
make \
meson \
openldap \
- openssl \
+ openssl@3 \
python \
tcl-tk \
zstd
brew cleanup -s # to reduce cache size
+
+ # brew cleanup removes the empty certs directory in OPENSSLDIR, causing
+ # OpenSSL to report unexpected errors ("unregistered scheme") during
+ # verification failures. Put it back for now as a workaround.
+ #
+ # https://github.com/orgs/Homebrew/discussions/4030
+ #
+ # Note that $(brew --prefix openssl) will give us the opt/ prefix but not
+ # the etc/ prefix, so we hardcode the full path here. openssl@3 is pinned
+ # above to try to minimize the chances of this changing beneath us, but it's
+ # brittle...
+ mkdir -p "/opt/homebrew/etc/openssl@3/certs"
upload_caches: homebrew
ccache_cache:
folder: $CCACHE_DIR
configure_script: |
diff --git a/configure b/configure
index 905be9568b..ae08c2fc5c 100755
--- a/configure
+++ b/configure
@@ -2092,10 +2092,60 @@ eval ac_res=\$$3
$as_echo "$ac_res" >&6; }
eval $as_lineno_stack; ${as_lineno_stack:+:} unset as_lineno
} # ac_fn_c_check_func
+# ac_fn_c_check_decl LINENO SYMBOL VAR INCLUDES
+# ---------------------------------------------
+# Tests whether SYMBOL is declared in INCLUDES, setting cache variable VAR
+# accordingly.
+ac_fn_c_check_decl ()
+{
+ as_lineno=${as_lineno-"$1"} as_lineno_stack=as_lineno_stack=$as_lineno_stack
+ # Initialize each $ac_[]_AC_LANG_ABBREV[]_decl_warn_flag once.
+ as_decl_name=`echo $2|sed 's/ *(.*//'`
+ as_decl_use=`echo $2|sed -e 's/(/((/' -e 's/)/) 0&/' -e 's/,/) 0& (/g'`
+ { $as_echo "$as_me:${as_lineno-$LINENO}: checking whether $as_decl_name is declared" >&5
+$as_echo_n "checking whether $as_decl_name is declared... " >&6; }
+if eval \${$3+:} false; then :
+ $as_echo_n "(cached) " >&6
+else
+ ac_save_werror_flag=$ac_c_werror_flag
+ ac_c_werror_flag="$ac_c_decl_warn_flag$ac_c_werror_flag"
+ cat confdefs.h - <<_ACEOF >conftest.$ac_ext
+/* end confdefs.h. */
+$4
+int
+main ()
+{
+#ifndef $as_decl_name
+#ifdef __cplusplus
+ (void) $as_decl_use;
+#else
+ (void) $as_decl_name;
+#endif
+#endif
+
+ ;
+ return 0;
+}
+_ACEOF
+if ac_fn_c_try_compile "$LINENO"; then :
+ eval "$3=yes"
+else
+ eval "$3=no"
+fi
+rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext
+ ac_c_werror_flag=$ac_save_werror_flag
+fi
+eval ac_res=\$$3
+ { $as_echo "$as_me:${as_lineno-$LINENO}: result: $ac_res" >&5
+$as_echo "$ac_res" >&6; }
+ eval $as_lineno_stack; ${as_lineno_stack:+:} unset as_lineno
+
+} # ac_fn_c_check_decl
+
# ac_fn_c_check_type LINENO TYPE VAR INCLUDES
# -------------------------------------------
# Tests whether TYPE exists after having included INCLUDES, setting cache
# variable VAR accordingly.
ac_fn_c_check_type ()
@@ -2385,60 +2435,10 @@ rm -f conftest.val
fi
eval $as_lineno_stack; ${as_lineno_stack:+:} unset as_lineno
as_fn_set_status $ac_retval
} # ac_fn_c_compute_int
-
-# ac_fn_c_check_decl LINENO SYMBOL VAR INCLUDES
-# ---------------------------------------------
-# Tests whether SYMBOL is declared in INCLUDES, setting cache variable VAR
-# accordingly.
-ac_fn_c_check_decl ()
-{
- as_lineno=${as_lineno-"$1"} as_lineno_stack=as_lineno_stack=$as_lineno_stack
- # Initialize each $ac_[]_AC_LANG_ABBREV[]_decl_warn_flag once.
- as_decl_name=`echo $2|sed 's/ *(.*//'`
- as_decl_use=`echo $2|sed -e 's/(/((/' -e 's/)/) 0&/' -e 's/,/) 0& (/g'`
- { $as_echo "$as_me:${as_lineno-$LINENO}: checking whether $as_decl_name is declared" >&5
-$as_echo_n "checking whether $as_decl_name is declared... " >&6; }
-if eval \${$3+:} false; then :
- $as_echo_n "(cached) " >&6
-else
- ac_save_werror_flag=$ac_c_werror_flag
- ac_c_werror_flag="$ac_c_decl_warn_flag$ac_c_werror_flag"
- cat confdefs.h - <<_ACEOF >conftest.$ac_ext
-/* end confdefs.h. */
-$4
-int
-main ()
-{
-#ifndef $as_decl_name
-#ifdef __cplusplus
- (void) $as_decl_use;
-#else
- (void) $as_decl_name;
-#endif
-#endif
-
- ;
- return 0;
-}
-_ACEOF
-if ac_fn_c_try_compile "$LINENO"; then :
- eval "$3=yes"
-else
- eval "$3=no"
-fi
-rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext
- ac_c_werror_flag=$ac_save_werror_flag
-fi
-eval ac_res=\$$3
- { $as_echo "$as_me:${as_lineno-$LINENO}: result: $ac_res" >&5
-$as_echo "$ac_res" >&6; }
- eval $as_lineno_stack; ${as_lineno_stack:+:} unset as_lineno
-
-} # ac_fn_c_check_decl
cat >config.log <<_ACEOF
This file contains any messages produced by compilers while
running configure, to aid debugging if configure makes a mistake.
It was created by PostgreSQL $as_me 16devel, which was
@@ -13028,10 +13028,111 @@ if test "x$ac_cv_func_X509_get_signature_info" = xyes; then :
_ACEOF
fi
done
+ # Let tests differentiate between vanilla OpenSSL and LibreSSL.
+ # The Clang compiler raises a warning for an undeclared identifier that matches
+# a compiler builtin function. All extant Clang versions are affected, as of
+# Clang 3.6.0. Test a builtin known to every version. This problem affects the
+# C and Objective C languages, but Clang does report an error under C++ and
+# Objective C++.
+#
+# Passing -fno-builtin to the compiler would suppress this problem. That
+# strategy would have the advantage of being insensitive to stray warnings, but
+# it would make tests less realistic.
+{ $as_echo "$as_me:${as_lineno-$LINENO}: checking how $CC reports undeclared, standard C functions" >&5
+$as_echo_n "checking how $CC reports undeclared, standard C functions... " >&6; }
+if ${ac_cv_c_decl_report+:} false; then :
+ $as_echo_n "(cached) " >&6
+else
+ cat confdefs.h - <<_ACEOF >conftest.$ac_ext
+/* end confdefs.h. */
+
+int
+main ()
+{
+(void) strchr;
+ ;
+ return 0;
+}
+_ACEOF
+if ac_fn_c_try_compile "$LINENO"; then :
+ if test -s conftest.err; then :
+ # For AC_CHECK_DECL to react to warnings, the compiler must be silent on
+ # valid AC_CHECK_DECL input. No library function is consistently available
+ # on freestanding implementations, so test against a dummy declaration.
+ # Include always-available headers on the off chance that they somehow
+ # elicit warnings.
+ cat confdefs.h - <<_ACEOF >conftest.$ac_ext
+/* end confdefs.h. */
+#include <float.h>
+#include <limits.h>
+#include <stdarg.h>
+#include <stddef.h>
+extern void ac_decl (int, char *);
+int
+main ()
+{
+#ifdef __cplusplus
+ (void) ac_decl ((int) 0, (char *) 0);
+ (void) ac_decl;
+#else
+ (void) ac_decl;
+#endif
+
+ ;
+ return 0;
+}
+_ACEOF
+if ac_fn_c_try_compile "$LINENO"; then :
+ if test -s conftest.err; then :
+ { { $as_echo "$as_me:${as_lineno-$LINENO}: error: in \`$ac_pwd':" >&5
+$as_echo "$as_me: error: in \`$ac_pwd':" >&2;}
+as_fn_error $? "cannot detect from compiler exit status or warnings
+See \`config.log' for more details" "$LINENO" 5; }
+else
+ ac_cv_c_decl_report=warning
+fi
+else
+ { { $as_echo "$as_me:${as_lineno-$LINENO}: error: in \`$ac_pwd':" >&5
+$as_echo "$as_me: error: in \`$ac_pwd':" >&2;}
+as_fn_error $? "cannot compile a simple declaration test
+See \`config.log' for more details" "$LINENO" 5; }
+fi
+rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext
+else
+ { { $as_echo "$as_me:${as_lineno-$LINENO}: error: in \`$ac_pwd':" >&5
+$as_echo "$as_me: error: in \`$ac_pwd':" >&2;}
+as_fn_error $? "compiler does not report undeclared identifiers
+See \`config.log' for more details" "$LINENO" 5; }
+fi
+else
+ ac_cv_c_decl_report=error
+fi
+rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext
+fi
+{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $ac_cv_c_decl_report" >&5
+$as_echo "$ac_cv_c_decl_report" >&6; }
+
+case $ac_cv_c_decl_report in
+ warning) ac_c_decl_warn_flag=yes ;;
+ *) ac_c_decl_warn_flag= ;;
+esac
+
+ac_fn_c_check_decl "$LINENO" "LIBRESSL_VERSION_NUMBER" "ac_cv_have_decl_LIBRESSL_VERSION_NUMBER" "#include <openssl/opensslv.h>
+"
+if test "x$ac_cv_have_decl_LIBRESSL_VERSION_NUMBER" = xyes; then :
+ ac_have_decl=1
+else
+ ac_have_decl=0
+fi
+
+cat >>confdefs.h <<_ACEOF
+#define HAVE_DECL_LIBRESSL_VERSION_NUMBER $ac_have_decl
+_ACEOF
+
$as_echo "#define USE_OPENSSL 1" >>confdefs.h
elif test "$with_ssl" != no ; then
as_fn_error $? "--with-ssl must specify openssl" "$LINENO" 5
@@ -16041,98 +16142,10 @@ fi
# posix_fadvise() is a no-op on Solaris, so don't incur function overhead
# by calling it, 2009-04-02
# http://src.opensolaris.org/source/xref/onnv/onnv-gate/usr/src/lib/libc/port/gen/posix_fadvise.c
-# The Clang compiler raises a warning for an undeclared identifier that matches
-# a compiler builtin function. All extant Clang versions are affected, as of
-# Clang 3.6.0. Test a builtin known to every version. This problem affects the
-# C and Objective C languages, but Clang does report an error under C++ and
-# Objective C++.
-#
-# Passing -fno-builtin to the compiler would suppress this problem. That
-# strategy would have the advantage of being insensitive to stray warnings, but
-# it would make tests less realistic.
-{ $as_echo "$as_me:${as_lineno-$LINENO}: checking how $CC reports undeclared, standard C functions" >&5
-$as_echo_n "checking how $CC reports undeclared, standard C functions... " >&6; }
-if ${ac_cv_c_decl_report+:} false; then :
- $as_echo_n "(cached) " >&6
-else
- cat confdefs.h - <<_ACEOF >conftest.$ac_ext
-/* end confdefs.h. */
-
-int
-main ()
-{
-(void) strchr;
- ;
- return 0;
-}
-_ACEOF
-if ac_fn_c_try_compile "$LINENO"; then :
- if test -s conftest.err; then :
- # For AC_CHECK_DECL to react to warnings, the compiler must be silent on
- # valid AC_CHECK_DECL input. No library function is consistently available
- # on freestanding implementations, so test against a dummy declaration.
- # Include always-available headers on the off chance that they somehow
- # elicit warnings.
- cat confdefs.h - <<_ACEOF >conftest.$ac_ext
-/* end confdefs.h. */
-#include <float.h>
-#include <limits.h>
-#include <stdarg.h>
-#include <stddef.h>
-extern void ac_decl (int, char *);
-int
-main ()
-{
-#ifdef __cplusplus
- (void) ac_decl ((int) 0, (char *) 0);
- (void) ac_decl;
-#else
- (void) ac_decl;
-#endif
-
- ;
- return 0;
-}
-_ACEOF
-if ac_fn_c_try_compile "$LINENO"; then :
- if test -s conftest.err; then :
- { { $as_echo "$as_me:${as_lineno-$LINENO}: error: in \`$ac_pwd':" >&5
-$as_echo "$as_me: error: in \`$ac_pwd':" >&2;}
-as_fn_error $? "cannot detect from compiler exit status or warnings
-See \`config.log' for more details" "$LINENO" 5; }
-else
- ac_cv_c_decl_report=warning
-fi
-else
- { { $as_echo "$as_me:${as_lineno-$LINENO}: error: in \`$ac_pwd':" >&5
-$as_echo "$as_me: error: in \`$ac_pwd':" >&2;}
-as_fn_error $? "cannot compile a simple declaration test
-See \`config.log' for more details" "$LINENO" 5; }
-fi
-rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext
-else
- { { $as_echo "$as_me:${as_lineno-$LINENO}: error: in \`$ac_pwd':" >&5
-$as_echo "$as_me: error: in \`$ac_pwd':" >&2;}
-as_fn_error $? "compiler does not report undeclared identifiers
-See \`config.log' for more details" "$LINENO" 5; }
-fi
-else
- ac_cv_c_decl_report=error
-fi
-rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext
-fi
-{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $ac_cv_c_decl_report" >&5
-$as_echo "$ac_cv_c_decl_report" >&6; }
-
-case $ac_cv_c_decl_report in
- warning) ac_c_decl_warn_flag=yes ;;
- *) ac_c_decl_warn_flag= ;;
-esac
-
if test "$PORTNAME" != "solaris"; then :
for ac_func in posix_fadvise
do :
ac_fn_c_check_func "$LINENO" "posix_fadvise" "ac_cv_func_posix_fadvise"
diff --git a/configure.ac b/configure.ac
index 8095dfcf1d..2d53399f06 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1386,10 +1386,12 @@ if test "$with_ssl" = openssl ; then
# thread-safety. In 1.1.0, it's no longer required, and CRYPTO_lock()
# function was removed.
AC_CHECK_FUNCS([CRYPTO_lock])
# Function introduced in OpenSSL 1.1.1.
AC_CHECK_FUNCS([X509_get_signature_info])
+ # Let tests differentiate between vanilla OpenSSL and LibreSSL.
+ AC_CHECK_DECLS([LIBRESSL_VERSION_NUMBER], [], [], [#include <openssl/opensslv.h>])
AC_DEFINE([USE_OPENSSL], 1, [Define to 1 to build with OpenSSL support. (--with-ssl=openssl)])
elif test "$with_ssl" != no ; then
AC_MSG_ERROR([--with-ssl must specify openssl])
fi
AC_SUBST(with_ssl)
diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml
index 9f72dd29d8..29ef0ae75d 100644
--- a/doc/src/sgml/libpq.sgml
+++ b/doc/src/sgml/libpq.sgml
@@ -1874,10 +1874,35 @@ postgresql://%2Fvar%2Flib%2Fpostgresql/dbname
certificate authority (<acronym>CA</acronym>) certificate(s).
If the file exists, the server's certificate will be verified
to be signed by one of these authorities. The default is
<filename>~/.postgresql/root.crt</filename>.
</para>
+ <para>
+ The special value <literal>system</literal> may be specified instead, in
+ which case the system's trusted CA roots will be loaded. The exact
+ locations of these root certificates differ by SSL implementation and
+ platform. For <productname>OpenSSL</productname> in particular, the
+ locations may be further modified by the <envar>SSL_CERT_DIR</envar>
+ and <envar>SSL_CERT_FILE</envar> environment variables.
+ </para>
+ <warning>
+ <para>
+ When using <literal>sslrootcert=system</literal>, it is critical to
+ also use the strongest certificate verification method,
+ <literal>sslmode=verify-full</literal>. In most cases it is trivial for
+ anyone to obtain a certificate trusted by the system for a hostname
+ they control, rendering the <literal>verify-ca</literal> mode useless.
+ </para>
+ </warning>
+ <note>
+ <para>
+ The magic <literal>system</literal> value will take precedence over a
+ local certificate file with the same name. If for some reason you find
+ yourself in this situation, use an alternative path like
+ <literal>sslrootcert=./system</literal> instead.
+ </para>
+ </note>
</listitem>
</varlistentry>
<varlistentry id="libpq-connect-sslcrl" xreflabel="sslcrl">
<term><literal>sslcrl</literal></term>
diff --git a/doc/src/sgml/runtime.sgml b/doc/src/sgml/runtime.sgml
index 149e9b33d4..b93184537a 100644
--- a/doc/src/sgml/runtime.sgml
+++ b/doc/src/sgml/runtime.sgml
@@ -2005,11 +2005,15 @@ pg_dumpall -p 5432 | psql -d postgres -p 5433
must be configured to accept only <literal>hostssl</literal> connections (<xref
linkend="auth-pg-hba-conf"/>) and have SSL key and certificate files
(<xref linkend="ssl-tcp"/>). The TCP client must connect using
<literal>sslmode=verify-ca</literal> or
<literal>verify-full</literal> and have the appropriate root certificate
- file installed (<xref linkend="libq-ssl-certificates"/>).
+ file installed (<xref linkend="libq-ssl-certificates"/>). Alternatively the
+ system CA pool can be used using <literal>sslrootcert=system</literal>; in
+ this case, <literal>sslmode=verify-full</literal> must be used for safety,
+ since it is generally trivial to obtain certificates which are signed by a
+ public CA.
</para>
<para>
To prevent spoofing with GSSAPI, the server must be configured to accept
only <literal>hostgssenc</literal> connections
diff --git a/meson.build b/meson.build
index 84b60c8933..a04475f83a 100644
--- a/meson.build
+++ b/meson.build
@@ -1271,10 +1271,19 @@ if sslopt in ['auto', 'openssl']
description: '''Define to the OpenSSL API version in use. This avoids deprecation warnings from newer OpenSSL versions.''')
ssl_library = 'openssl'
else
ssl = not_found_dep
endif
+
+ if ssl.found()
+ # Let tests differentiate between vanilla OpenSSL and LibreSSL.
+ sym = 'LIBRESSL_VERSION_NUMBER'
+ found = cc.has_header_symbol('openssl/opensslv.h', sym, dependencies: ssl_int)
+ cdata.set10('HAVE_DECL_' + sym, found, description:
+'''Define to 1 if you have the declaration of `@0@', and to 0 if you
+ don't.'''.format(sym))
+ endif
endif
endif
if sslopt == 'auto' and auto_features.enabled() and not ssl.found()
error('no SSL library found')
diff --git a/src/include/pg_config.h.in b/src/include/pg_config.h.in
index 3665e799e7..547d447696 100644
--- a/src/include/pg_config.h.in
+++ b/src/include/pg_config.h.in
@@ -97,10 +97,14 @@
/* Define to 1 if you have the declaration of `F_FULLFSYNC', and to 0 if you
don't. */
#undef HAVE_DECL_F_FULLFSYNC
+/* Define to 1 if you have the declaration of `LIBRESSL_VERSION_NUMBER', and
+ to 0 if you don't. */
+#undef HAVE_DECL_LIBRESSL_VERSION_NUMBER
+
/* Define to 1 if you have the declaration of
`LLVMCreateGDBRegistrationListener', and to 0 if you don't. */
#undef HAVE_DECL_LLVMCREATEGDBREGISTRATIONLISTENER
/* Define to 1 if you have the declaration of
diff --git a/src/interfaces/libpq/fe-secure-openssl.c b/src/interfaces/libpq/fe-secure-openssl.c
index 4d1e4009ef..ac0c27a926 100644
--- a/src/interfaces/libpq/fe-secure-openssl.c
+++ b/src/interfaces/libpq/fe-secure-openssl.c
@@ -1058,12 +1058,33 @@ initialize_SSL(PGconn *conn)
else if (have_homedir)
snprintf(fnbuf, sizeof(fnbuf), "%s/%s", homedir, ROOT_CERT_FILE);
else
fnbuf[0] = '\0';
- if (fnbuf[0] != '\0' &&
- stat(fnbuf, &buf) == 0)
+ if (strcmp(fnbuf, "system") == 0)
+ {
+ /*
+ * The "system" sentinel value indicates that we should load whatever
+ * root certificates are installed for use by OpenSSL; these locations
+ * differ by platform. Note that the default system locations may be
+ * further overridden by the SSL_CERT_DIR and SSL_CERT_FILE environment
+ * variables.
+ */
+ if (SSL_CTX_set_default_verify_paths(SSL_context) != 1)
+ {
+ char *err = SSLerrmessage(ERR_get_error());
+
+ libpq_append_conn_error(conn, "could not load system root certificate paths: %s",
+ err);
+ SSLerrfree(err);
+ SSL_CTX_free(SSL_context);
+ return -1;
+ }
+ have_rootcert = true;
+ }
+ else if (fnbuf[0] != '\0' &&
+ stat(fnbuf, &buf) == 0)
{
X509_STORE *cvstore;
if (SSL_CTX_load_verify_locations(SSL_context, fnbuf, NULL) != 1)
{
@@ -1120,14 +1141,14 @@ initialize_SSL(PGconn *conn)
* pqGetHomeDirectory failed. That's a sufficiently unusual case
* that it seems worth having a specialized error message for it.
*/
if (fnbuf[0] == '\0')
libpq_append_conn_error(conn, "could not get home directory to locate root certificate file\n"
- "Either provide the file or change sslmode to disable server certificate verification.");
+ "Either provide the file, use the system's trusted roots with sslrootcert=system, or change sslmode to disable server certificate verification.");
else
libpq_append_conn_error(conn, "root certificate file \"%s\" does not exist\n"
- "Either provide the file or change sslmode to disable server certificate verification.", fnbuf);
+ "Either provide the file, use the system's trusted roots with sslrootcert=system, or change sslmode to disable server certificate verification.", fnbuf);
SSL_CTX_free(SSL_context);
return -1;
}
have_rootcert = false;
}
diff --git a/src/test/ssl/ssl/server-cn-only+server_ca.crt b/src/test/ssl/ssl/server-cn-only+server_ca.crt
new file mode 100644
index 0000000000..9870e8c17a
--- /dev/null
+++ b/src/test/ssl/ssl/server-cn-only+server_ca.crt
@@ -0,0 +1,38 @@
+-----BEGIN CERTIFICATE-----
+MIIDAzCCAesCCCAhAwMUEgcBMA0GCSqGSIb3DQEBCwUAMEIxQDA+BgNVBAMMN1Rl
+c3QgQ0EgZm9yIFBvc3RncmVTUUwgU1NMIHJlZ3Jlc3Npb24gdGVzdCBzZXJ2ZXIg
+Y2VydHMwHhcNMjEwMzAzMjIxMjA3WhcNNDgwNzE5MjIxMjA3WjBGMR4wHAYDVQQL
+DBVQb3N0Z3JlU1FMIHRlc3Qgc3VpdGUxJDAiBgNVBAMMG2NvbW1vbi1uYW1lLnBn
+LXNzbHRlc3QudGVzdDCCASIwDQYJKoZIhvcNAQEBBQADggEPADCCAQoCggEBANWz
+VPMk7i5f+W0eEadRE+TTAtsIK08CkLMUnjs7zJkxnnm6RGBXPx6vK3AkAIi+wG4Y
+mXjYP3GuMiXaLjnWh2kzBSfIRQyNbTThnhSu3nDjAVkPexsSrPyiKimFuNgDfkGe
+5dQKa9Ag2SuVU4vd9SYxOMAiIFIC4ts4MLWWJf5D/PehdSuc0e5Me+91Nnbz90nl
+ds4lHvuDR+aKnZlTHmch3wfhXv7lNQImIBzfwl36Kd/bWB0fAEVFse3iZWmigaI/
+9FKh//WIq43TNLxn68OCQoyMe/HGjZDR/Xwo3rE6jg6/iAwSWib9yabfYPKbqq2G
+oFy6aYmmEquaDgLuX7kCAwEAATANBgkqhkiG9w0BAQsFAAOCAQEA2AZrD9cTQXTW
+4j2tT8N/TTc6WK2ncN4h22NTte6vK7MVwsZJCtw5ndYkmxcWkXAqiclzWyMdayds
+WOa12CEH7jKAhivF4Hcw3oO3JHM5BA6KzLWBVz9uZksOM6mPqn29DTKvA/Y1V8tj
+mxK/KUA68h/u6inu3mo4ywBpb/tqHxxg2cjyR0faCmM0pwRM0HBr/16fUMfO83nj
+QG8g9J/bybu5sYso/aSoC5nUNp4XjmDMdVLdqg/nTe/ejS8IfFr0WQxBlqooqFgx
+MSE+kX2e2fHsuOWSU/9eClt6FpQrwoC2C8F+/4g1Uz7Liqc4yMHPwjgeP9ewrrLO
+iIhlNNPqpQ==
+-----END CERTIFICATE-----
+-----BEGIN CERTIFICATE-----
+MIIDFDCCAfygAwIBAgIIICEDAxQSBwAwDQYJKoZIhvcNAQELBQAwQDE+MDwGA1UE
+Aww1VGVzdCByb290IENBIGZvciBQb3N0Z3JlU1FMIFNTTCByZWdyZXNzaW9uIHRl
+c3Qgc3VpdGUwHhcNMjEwMzAzMjIxMjA3WhcNNDgwNzE5MjIxMjA3WjBCMUAwPgYD
+VQQDDDdUZXN0IENBIGZvciBQb3N0Z3JlU1FMIFNTTCByZWdyZXNzaW9uIHRlc3Qg
+c2VydmVyIGNlcnRzMIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEA4kp2
+GW5nPb6QrSrtbClfZeutyQnHrm4TMPBoNepFdIVxBX/04BguM5ImDRze/huOWA+z
+atJAQqt3R7dsDwnOnPKUKCOuHX6a1aj5L86HtVgaWTXrZFE5NtL9PIzXkWu13UW0
+UesHtbPVRv6a6fB7Npph6hHy7iPZb009A8/lTJnxSPC39u/K/sPqjrVZaAJF+wDs
+qCxCZTUtAUFvWFnR/TeXLWlFzBupS1djgI7PltbJqSn6SKTAgNZTxpRJbu9Icp6J
+/50ELwT++0n0KWVXNHrDNfI5Gaa+SxClAsPsei2jLKpgR6QFC3wn38Z9q9LjAVuC
++FWhoN1uhYeoricEXwIDAQABoxAwDjAMBgNVHRMEBTADAQH/MA0GCSqGSIb3DQEB
+CwUAA4IBAQCdCA/EoXrustoV4jJGbkdXDuOUkBurwggSNBAqUBSDvCohRoD77Ecb
+QVuzPNxWKG+E4PwfUq2ha+2yPONEJ28ZgsbHq5qlJDMJ43wlcjn6wmmAJNeSpO8F
+0V9d2X/4wNZty9/zbwTnw26KChgDHumQ0WIbCoBtdqy8KDswYOvpgws6dqc021I7
+UrFo6vZek7VoApbJgkDL6qYADa6ApfW43ThH4sViFITeYt/kSHgmy2Udhs34jMM8
+xsFP/uYpRi1b1glenwSIKiHjD4/C9vnWQt5K3gRBvYukEj2Bw9VkNRpBVCi0cOoA
+OuwX3bwzNYNbZQv4K66oRpvuoEjCNeHg
+-----END CERTIFICATE-----
diff --git a/src/test/ssl/sslfiles.mk b/src/test/ssl/sslfiles.mk
index e63342469d..f7ababe42c 100644
--- a/src/test/ssl/sslfiles.mk
+++ b/src/test/ssl/sslfiles.mk
@@ -59,11 +59,12 @@ COMBINATIONS := \
ssl/both-cas-2.crt \
ssl/root+server_ca.crt \
ssl/root+server.crl \
ssl/root+client_ca.crt \
ssl/root+client.crl \
- ssl/client+client_ca.crt
+ ssl/client+client_ca.crt \
+ ssl/server-cn-only+server_ca.crt
CERTIFICATES := root_ca server_ca client_ca $(SERVERS) $(CLIENTS)
STANDARD_CERTS := $(CERTIFICATES:%=ssl/%.crt)
STANDARD_KEYS := $(CERTIFICATES:%=ssl/%.key)
CRLS := ssl/root.crl \
@@ -148,10 +149,13 @@ ssl/root+server_ca.crt: ssl/root_ca.crt ssl/server_ca.crt
ssl/root+client_ca.crt: ssl/root_ca.crt ssl/client_ca.crt
# and for the client, to present to the server
ssl/client+client_ca.crt: ssl/client.crt ssl/client_ca.crt
+# for the server, to present to a client that only knows the root
+ssl/server-cn-only+server_ca.crt: ssl/server-cn-only.crt ssl/server_ca.crt
+
# If a CRL is used, OpenSSL requires a CRL file for *all* the CAs in the
# chain, even if some of them are empty.
ssl/root+server.crl: ssl/root.crl ssl/server.crl
ssl/root+client.crl: ssl/root.crl ssl/client.crl
diff --git a/src/test/ssl/t/001_ssltests.pl b/src/test/ssl/t/001_ssltests.pl
index dc43b8f81a..7c5080b632 100644
--- a/src/test/ssl/t/001_ssltests.pl
+++ b/src/test/ssl/t/001_ssltests.pl
@@ -31,10 +31,14 @@ sub sslkey
sub switch_server_cert
{
$ssl_server->switch_server_cert(@_);
}
+
+# Determine whether this build uses OpenSSL or LibreSSL.
+my $libressl = check_pg_config("#define HAVE_DECL_LIBRESSL_VERSION_NUMBER 1");
+
#### Some configuration
# This is the hostname used to connect to the server. This cannot be a
# hostname, because the server certificate is always for the domain
# postgresql-ssl-regression.test.
@@ -459,10 +463,35 @@ $node->connect_fails(
. "sslmode=verify-full host=common-name.pg-ssltest.test",
"server certificate without CN or SANs sslmode=verify-full",
expected_stderr =>
qr/could not get server's host name from server certificate/);
+# Test system trusted roots.
+switch_server_cert($node,
+ certfile => 'server-cn-only+server_ca',
+ keyfile => 'server-cn-only',
+ cafile => 'root_ca');
+$common_connstr =
+ "$default_ssl_connstr user=ssltestuser dbname=trustdb sslrootcert=system hostaddr=$SERVERHOSTADDR";
+
+# By default our custom-CA-signed certificate should not be trusted.
+$node->connect_fails(
+ "$common_connstr sslmode=verify-full host=common-name.pg-ssltest.test",
+ "sslrootcert=system does not connect with private CA",
+ expected_stderr => qr/SSL error: certificate verify failed/);
+
+SKIP:
+{
+ skip "SSL_CERT_FILE is not supported with LibreSSL" if $libressl;
+
+ # We can modify the definition of "system" to get it trusted again.
+ local $ENV{SSL_CERT_FILE} = $node->data_dir . "/root_ca.crt";
+ $node->connect_ok(
+ "$common_connstr sslmode=verify-full host=common-name.pg-ssltest.test",
+ "sslrootcert=system connects with overridden SSL_CERT_FILE");
+}
+
# Test that the CRL works
switch_server_cert($node, certfile => 'server-revoked');
$common_connstr =
"$default_ssl_connstr user=ssltestuser dbname=trustdb hostaddr=$SERVERHOSTADDR host=common-name.pg-ssltest.test";
--
2.25.1
From ba09e1d83fb65e6f9bc769572920cda45534b112 Mon Sep 17 00:00:00 2001
From: Jacob Champion <jchamp...@timescale.com>
Date: Mon, 24 Oct 2022 15:24:11 -0700
Subject: [PATCH v9 2/2] libpq: force sslmode=verify-full for system CAs
Weaker verification modes don't make much sense for public CAs.
---
doc/src/sgml/libpq.sgml | 15 ++++----
doc/src/sgml/runtime.sgml | 6 +--
src/interfaces/libpq/fe-connect.c | 63 +++++++++++++++++++++++++++++++
src/interfaces/libpq/t/001_uri.pl | 30 ++++++++++++++-
src/test/ssl/t/001_ssltests.pl | 12 ++++++
5 files changed, 113 insertions(+), 13 deletions(-)
diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml
index 29ef0ae75d..faa8aa3187 100644
--- a/doc/src/sgml/libpq.sgml
+++ b/doc/src/sgml/libpq.sgml
@@ -1882,20 +1882,19 @@ postgresql://%2Fvar%2Flib%2Fpostgresql/dbname
locations of these root certificates differ by SSL implementation and
platform. For <productname>OpenSSL</productname> in particular, the
locations may be further modified by the <envar>SSL_CERT_DIR</envar>
and <envar>SSL_CERT_FILE</envar> environment variables.
</para>
- <warning>
+ <note>
<para>
- When using <literal>sslrootcert=system</literal>, it is critical to
- also use the strongest certificate verification method,
- <literal>sslmode=verify-full</literal>. In most cases it is trivial for
- anyone to obtain a certificate trusted by the system for a hostname
- they control, rendering the <literal>verify-ca</literal> mode useless.
+ When using <literal>sslrootcert=system</literal>, the default
+ <literal>sslmode</literal> is changed to <literal>verify-full</literal>,
+ and any weaker setting will result in an error. In most cases it is
+ trivial for anyone to obtain a certificate trusted by the system for a
+ hostname they control, rendering <literal>verify-ca</literal> and all
+ weaker modes useless.
</para>
- </warning>
- <note>
<para>
The magic <literal>system</literal> value will take precedence over a
local certificate file with the same name. If for some reason you find
yourself in this situation, use an alternative path like
<literal>sslrootcert=./system</literal> instead.
diff --git a/doc/src/sgml/runtime.sgml b/doc/src/sgml/runtime.sgml
index b93184537a..dbe23db54f 100644
--- a/doc/src/sgml/runtime.sgml
+++ b/doc/src/sgml/runtime.sgml
@@ -2007,13 +2007,13 @@ pg_dumpall -p 5432 | psql -d postgres -p 5433
(<xref linkend="ssl-tcp"/>). The TCP client must connect using
<literal>sslmode=verify-ca</literal> or
<literal>verify-full</literal> and have the appropriate root certificate
file installed (<xref linkend="libq-ssl-certificates"/>). Alternatively the
system CA pool can be used using <literal>sslrootcert=system</literal>; in
- this case, <literal>sslmode=verify-full</literal> must be used for safety,
- since it is generally trivial to obtain certificates which are signed by a
- public CA.
+ this case, <literal>sslmode=verify-full</literal> is forced for safety, since
+ it is generally trivial to obtain certificates which are signed by a public
+ CA.
</para>
<para>
To prevent spoofing with GSSAPI, the server must be configured to accept
only <literal>hostgssenc</literal> connections
diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c
index bb7347cb0c..16a5105d8b 100644
--- a/src/interfaces/libpq/fe-connect.c
+++ b/src/interfaces/libpq/fe-connect.c
@@ -1463,10 +1463,26 @@ connectOptions2(PGconn *conn)
conn->channel_binding = strdup(DefaultChannelBinding);
if (!conn->channel_binding)
goto oom_error;
}
+#ifndef USE_SSL
+ /*
+ * sslrootcert=system is not supported. Since setting this changes the
+ * default sslmode, check this _before_ we validate sslmode, to avoid
+ * confusing the user with errors for an option they may not have set.
+ */
+ if (conn->sslrootcert
+ && strcmp(conn->sslrootcert, "system") == 0)
+ {
+ conn->status = CONNECTION_BAD;
+ libpq_append_conn_error(conn, "sslrootcert value \"%s\" invalid when SSL support is not compiled in",
+ conn->sslrootcert);
+ return false;
+ }
+#endif
+
/*
* validate sslmode option
*/
if (conn->sslmode)
{
@@ -1509,10 +1525,25 @@ connectOptions2(PGconn *conn)
conn->sslmode = strdup(DefaultSSLMode);
if (!conn->sslmode)
goto oom_error;
}
+#ifdef USE_SSL
+ /*
+ * If sslrootcert=system, make sure our chosen sslmode is compatible.
+ */
+ if (conn->sslrootcert
+ && strcmp(conn->sslrootcert, "system") == 0
+ && strcmp(conn->sslmode, "verify-full") != 0)
+ {
+ conn->status = CONNECTION_BAD;
+ libpq_append_conn_error(conn, "weak sslmode \"%s\" may not be used with sslrootcert=system (use verify-full)",
+ conn->sslmode);
+ return false;
+ }
+#endif
+
/*
* Validate TLS protocol versions for ssl_min_protocol_version and
* ssl_max_protocol_version.
*/
if (!sslVerifyProtocolVersion(conn->ssl_min_protocol_version))
@@ -6234,10 +6265,11 @@ conninfo_array_parse(const char *const *keywords, const char *const *values,
*/
static bool
conninfo_add_defaults(PQconninfoOption *options, PQExpBuffer errorMessage)
{
PQconninfoOption *option;
+ PQconninfoOption *sslmode_default = NULL, *sslrootcert = NULL;
char *tmp;
/*
* If there's a service spec, use it to obtain any not-explicitly-given
* parameters. Ignore error if no error message buffer is passed because
@@ -6250,10 +6282,13 @@ conninfo_add_defaults(PQconninfoOption *options, PQExpBuffer errorMessage)
* Get the fallback resources for parameters not specified in the conninfo
* string nor the service.
*/
for (option = options; option->keyword != NULL; option++)
{
+ if (strcmp(option->keyword, "sslrootcert") == 0)
+ sslrootcert = option; /* save for later */
+
if (option->val != NULL)
continue; /* Value was in conninfo or service */
/*
* Try to get the environment variable fallback
@@ -6292,10 +6327,17 @@ conninfo_add_defaults(PQconninfoOption *options, PQExpBuffer errorMessage)
libpq_append_error(errorMessage, "out of memory");
return false;
}
continue;
}
+
+ /*
+ * sslmode is not specified. Let it be filled in with the compiled
+ * default for now, but if sslrootcert=system, we'll override the
+ * default later before returning.
+ */
+ sslmode_default = option;
}
/*
* No environment variable specified or the variable isn't set - try
* compiled-in default
@@ -6324,10 +6366,31 @@ conninfo_add_defaults(PQconninfoOption *options, PQExpBuffer errorMessage)
option->val = pg_fe_getauthname(NULL);
continue;
}
}
+ /*
+ * Special handling for sslrootcert=system with no sslmode explicitly
+ * defined. In this case we want to strengthen the default sslmode to
+ * verify-full.
+ */
+ if (sslmode_default && sslrootcert)
+ {
+ if (sslrootcert->val && strcmp(sslrootcert->val, "system") == 0)
+ {
+ free(sslmode_default->val);
+
+ sslmode_default->val = strdup("verify-full");
+ if (!sslmode_default->val)
+ {
+ if (errorMessage)
+ libpq_append_error(errorMessage, "out of memory");
+ return false;
+ }
+ }
+ }
+
return true;
}
/*
* Subroutine for parse_connection_string
diff --git a/src/interfaces/libpq/t/001_uri.pl b/src/interfaces/libpq/t/001_uri.pl
index 2ab537f97f..cd659bc1b0 100644
--- a/src/interfaces/libpq/t/001_uri.pl
+++ b/src/interfaces/libpq/t/001_uri.pl
@@ -6,11 +6,13 @@ use PostgreSQL::Test::Utils;
use Test::More;
use IPC::Run;
# List of URIs tests. For each test the first element is the input string, the
-# second the expected stdout and the third the expected stderr.
+# second the expected stdout and the third the expected stderr. Optionally,
+# additional arguments may specify key/value pairs which will override
+# environment variables for the duration of the test.
my @tests = (
[
q{postgresql://uri-user:secret@host:12345/db},
q{user='uri-user' password='secret' dbname='db' host='host' port='12345' (inet)},
q{},
@@ -207,24 +209,48 @@ my @tests = (
],
[
q{postgres://%2Fvar%2Flib%2Fpostgresql/dbname},
q{dbname='dbname' host='/var/lib/postgresql' (local)},
q{},
+ ],
+ # Usually the default sslmode is 'prefer' (for libraries with SSL) or
+ # 'disable' (for those without). This default changes to 'verify-full' if
+ # the system CA store is in use.
+ [
+ q{postgresql://host?sslmode=disable},
+ q{host='host' sslmode='disable' (inet)},
+ q{},
+ PGSSLROOTCERT => "system",
+ ],
+ [
+ q{postgresql://host?sslmode=prefer},
+ q{host='host' sslmode='prefer' (inet)},
+ q{},
+ PGSSLROOTCERT => "system",
+ ],
+ [
+ q{postgresql://host?sslmode=verify-full},
+ q{host='host' (inet)},
+ q{},
+ PGSSLROOTCERT => "system",
]);
# test to run for each of the above test definitions
sub test_uri
{
local $Test::Builder::Level = $Test::Builder::Level + 1;
+ local %ENV = %ENV;
my $uri;
my %expect;
+ my %envvars;
my %result;
- ($uri, $expect{stdout}, $expect{stderr}) = @$_;
+ ($uri, $expect{stdout}, $expect{stderr}, %envvars) = @$_;
$expect{'exit'} = $expect{stderr} eq '';
+ %ENV = (%ENV, %envvars);
my $cmd = [ 'libpq_uri_regress', $uri ];
$result{exit} = IPC::Run::run $cmd, '>', \$result{stdout}, '2>',
\$result{stderr};
diff --git a/src/test/ssl/t/001_ssltests.pl b/src/test/ssl/t/001_ssltests.pl
index 7c5080b632..ae53bb946b 100644
--- a/src/test/ssl/t/001_ssltests.pl
+++ b/src/test/ssl/t/001_ssltests.pl
@@ -477,19 +477,31 @@ $common_connstr =
$node->connect_fails(
"$common_connstr sslmode=verify-full host=common-name.pg-ssltest.test",
"sslrootcert=system does not connect with private CA",
expected_stderr => qr/SSL error: certificate verify failed/);
+# Modes other than verify-full cannot be mixed with sslrootcert=system.
+$node->connect_fails(
+ "$common_connstr sslmode=verify-ca host=common-name.pg-ssltest.test",
+ "sslrootcert=system only accepts sslmode=verify-full",
+ expected_stderr => qr/weak sslmode "verify-ca" may not be used with sslrootcert=system/);
+
SKIP:
{
skip "SSL_CERT_FILE is not supported with LibreSSL" if $libressl;
# We can modify the definition of "system" to get it trusted again.
local $ENV{SSL_CERT_FILE} = $node->data_dir . "/root_ca.crt";
$node->connect_ok(
"$common_connstr sslmode=verify-full host=common-name.pg-ssltest.test",
"sslrootcert=system connects with overridden SSL_CERT_FILE");
+
+ # verify-full mode should be the default for system CAs.
+ $node->connect_fails(
+ "$common_connstr host=common-name.pg-ssltest.test.bad",
+ "sslrootcert=system defaults to sslmode=verify-full",
+ expected_stderr => qr/server certificate for "common-name.pg-ssltest.test" does not match host name "common-name.pg-ssltest.test.bad"/);
}
# Test that the CRL works
switch_server_cert($node, certfile => 'server-revoked');
--
2.25.1
