On Thu, Nov 3, 2022 at 4:39 PM Jacob Champion <jchamp...@timescale.com> wrote:
> There is an additional test failure with LibreSSL, which doesn't appear
> to honor the SSL_CERT_FILE environment variable. This isn't a problem in
> production -- if you're using LibreSSL, you'd presumably understand that
> you can't use that envvar -- but it makes testing difficult, because I
> don't yet know a way to tell LibreSSL to use a different set of roots
> for the duration of a test. Has anyone dealt with this before?

Fixed in v3, with a large hammer (configure-time checks). Hopefully
I've missed a simpler solution.

> > If there are no valuable use cases for weaker checks, then we could go
> > even further than my 0002 and just reject any weaker sslmodes
> > outright. That'd be nice.

Done. sslrootcert=system now prevents you from explicitly setting a
weaker sslmode, to try to cement it as a Do What I Mean sort of
feature. If you need something weird then you can still jump through
the hoops by setting sslrootcert to a real file, same as today.

The macOS/OpenSSL 3.0.0 failure is still unfixed.

Thanks,
--Jacob
diff --git a/configure b/configure
index 3966368b8d..56166f1097 100755
--- a/configure
+++ b/configure
@@ -2095,6 +2095,56 @@ $as_echo "$ac_res" >&6; }
 
 } # 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
@@ -2388,56 +2438,6 @@ rm -f conftest.val
   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.
@@ -13035,6 +13035,107 @@ _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
 
@@ -16062,94 +16163,6 @@ 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
diff --git a/configure.ac b/configure.ac
index f76b7ee31f..5b26086430 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1366,6 +1366,8 @@ 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])
+  # 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])
diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml
index 1401b93e72..3ebbd45aa4 100644
--- a/doc/src/sgml/libpq.sgml
+++ b/doc/src/sgml/libpq.sgml
@@ -1733,15 +1733,16 @@ postgresql://%2Fvar%2Flib%2Fpostgresql/dbname
         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>
       </listitem>
      </varlistentry>
 
diff --git a/doc/src/sgml/runtime.sgml b/doc/src/sgml/runtime.sgml
index 8184a3d54e..c4c4874fd0 100644
--- a/doc/src/sgml/runtime.sgml
+++ b/doc/src/sgml/runtime.sgml
@@ -2008,9 +2008,9 @@ pg_dumpall -p 5432 | psql -d postgres -p 5433
    <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>
diff --git a/meson.build b/meson.build
index bfacbdc0af..ea658b23bc 100644
--- a/meson.build
+++ b/meson.build
@@ -1213,6 +1213,13 @@ if get_option('ssl') == 'openssl'
     endif
   endforeach
 
+  # 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))
+
   cdata.set('USE_OPENSSL', 1,
             description: 'Define to 1 to build with OpenSSL support. 
(-Dssl=openssl)')
   cdata.set('OPENSSL_API_COMPAT', '0x10001000L',
diff --git a/src/include/pg_config.h.in b/src/include/pg_config.h.in
index c5a80b829e..34055c0639 100644
--- a/src/include/pg_config.h.in
+++ b/src/include/pg_config.h.in
@@ -99,6 +99,10 @@
    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
diff --git a/src/interfaces/libpq/fe-connect.c 
b/src/interfaces/libpq/fe-connect.c
index 92b87d3318..437d530ba1 100644
--- a/src/interfaces/libpq/fe-connect.c
+++ b/src/interfaces/libpq/fe-connect.c
@@ -1263,6 +1263,23 @@ connectOptions2(PGconn *conn)
                        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;
+               appendPQExpBuffer(&conn->errorMessage,
+                                                 libpq_gettext("sslrootcert 
value \"%s\" invalid when SSL support is not compiled in\n"),
+                                                 conn->sslrootcert);
+               return false;
+       }
+#endif
+
        /*
         * validate sslmode option
         */
@@ -1311,6 +1328,22 @@ connectOptions2(PGconn *conn)
                        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;
+               appendPQExpBuffer(&conn->errorMessage,
+                                                 libpq_gettext("weak sslmode 
\"%s\" may not be used with sslrootcert=system\n"),
+                                                 conn->sslmode);
+               return false;
+       }
+#endif
+
        /*
         * Validate TLS protocol versions for ssl_min_protocol_version and
         * ssl_max_protocol_version.
diff --git a/src/test/ssl/t/001_ssltests.pl b/src/test/ssl/t/001_ssltests.pl
index 4272c04a82..4747e72318 100644
--- a/src/test/ssl/t/001_ssltests.pl
+++ b/src/test/ssl/t/001_ssltests.pl
@@ -33,6 +33,10 @@ 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
@@ -455,8 +459,16 @@ $node->connect_fails(
        "sslrootcert=system does not connect with private CA",
        expected_stderr => qr/SSL error: certificate verify failed/);
 
-if ($ENV{with_ssl} eq 'openssl')
+# 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(
From d19b0bfc95da83e066e17b66812315290c97e653 Mon Sep 17 00:00:00 2001
From: Jacob Champion <jchamp...@timescale.com>
Date: Mon, 24 Oct 2022 15:30:25 -0700
Subject: [PATCH v3 1/2] libpq: add sslrootcert=system to use default CAs

Based on a patch by Thomas Habets.

Discussion: https://www.postgresql.org/message-id/flat/CA%2BkHd%2BcJwCUxVb-Gj_0ptr3_KZPwi3%2B67vK6HnLFBK9MzuYrLA%40mail.gmail.com
---
 configure                                     | 289 +++++++++---------
 configure.ac                                  |   2 +
 doc/src/sgml/libpq.sgml                       |  17 ++
 doc/src/sgml/runtime.sgml                     |   6 +-
 meson.build                                   |   7 +
 src/include/pg_config.h.in                    |   4 +
 src/interfaces/libpq/fe-secure-openssl.c      |  26 +-
 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 ++
 10 files changed, 282 insertions(+), 142 deletions(-)
 create mode 100644 src/test/ssl/ssl/server-cn-only+server_ca.crt

diff --git a/configure b/configure
index 3966368b8d..56166f1097 100755
--- a/configure
+++ b/configure
@@ -2095,6 +2095,56 @@ $as_echo "$ac_res" >&6; }
 
 } # 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
@@ -2388,56 +2438,6 @@ rm -f conftest.val
   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.
@@ -13035,6 +13035,107 @@ _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
 
@@ -16062,94 +16163,6 @@ 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
diff --git a/configure.ac b/configure.ac
index f76b7ee31f..5b26086430 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1366,6 +1366,8 @@ 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])
+  # 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])
diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml
index 3c9bd3d673..1401b93e72 100644
--- a/doc/src/sgml/libpq.sgml
+++ b/doc/src/sgml/libpq.sgml
@@ -1725,6 +1725,23 @@ postgresql://%2Fvar%2Flib%2Fpostgresql/dbname
         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>
       </listitem>
      </varlistentry>
 
diff --git a/doc/src/sgml/runtime.sgml b/doc/src/sgml/runtime.sgml
index 66367587b2..8184a3d54e 100644
--- a/doc/src/sgml/runtime.sgml
+++ b/doc/src/sgml/runtime.sgml
@@ -2006,7 +2006,11 @@ 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"/>).
+   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>
diff --git a/meson.build b/meson.build
index ce2f223a40..fcaeb8609d 100644
--- a/meson.build
+++ b/meson.build
@@ -1213,6 +1213,13 @@ if get_option('ssl') == 'openssl'
     endif
   endforeach
 
+  # 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))
+
   cdata.set('USE_OPENSSL', 1,
             description: 'Define to 1 to build with OpenSSL support. (-Dssl=openssl)')
   cdata.set('OPENSSL_API_COMPAT', '0x10001000L',
diff --git a/src/include/pg_config.h.in b/src/include/pg_config.h.in
index c5a80b829e..34055c0639 100644
--- a/src/include/pg_config.h.in
+++ b/src/include/pg_config.h.in
@@ -99,6 +99,10 @@
    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
diff --git a/src/interfaces/libpq/fe-secure-openssl.c b/src/interfaces/libpq/fe-secure-openssl.c
index b42a908733..4371cc7b91 100644
--- a/src/interfaces/libpq/fe-secure-openssl.c
+++ b/src/interfaces/libpq/fe-secure-openssl.c
@@ -1050,8 +1050,30 @@ initialize_SSL(PGconn *conn)
 	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());
+
+			appendPQExpBuffer(&conn->errorMessage,
+							  libpq_gettext("could not load system root certificate paths: %s\n"),
+							  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;
 
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 54ada01d46..d093ebc24a 100644
--- a/src/test/ssl/sslfiles.mk
+++ b/src/test/ssl/sslfiles.mk
@@ -57,7 +57,8 @@ COMBINATIONS := \
 	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)
@@ -132,6 +133,9 @@ 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
diff --git a/src/test/ssl/t/001_ssltests.pl b/src/test/ssl/t/001_ssltests.pl
index fe42161a0f..b34a6f2a81 100644
--- a/src/test/ssl/t/001_ssltests.pl
+++ b/src/test/ssl/t/001_ssltests.pl
@@ -33,6 +33,10 @@ 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
@@ -441,6 +445,31 @@ $node->connect_fails(
 	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');
 
-- 
2.25.1

From 87a324efcfe347265439dc24c9780eb07de7781b Mon Sep 17 00:00:00 2001
From: Jacob Champion <jchamp...@timescale.com>
Date: Mon, 24 Oct 2022 15:24:11 -0700
Subject: [PATCH v3 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 | 58 +++++++++++++++++++++++++++++++
 src/interfaces/libpq/t/001_uri.pl | 30 ++++++++++++++--
 src/test/ssl/t/001_ssltests.pl    | 12 +++++++
 5 files changed, 109 insertions(+), 12 deletions(-)

diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml
index 1401b93e72..3ebbd45aa4 100644
--- a/doc/src/sgml/libpq.sgml
+++ b/doc/src/sgml/libpq.sgml
@@ -1733,15 +1733,16 @@ postgresql://%2Fvar%2Flib%2Fpostgresql/dbname
         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>
       </listitem>
      </varlistentry>
 
diff --git a/doc/src/sgml/runtime.sgml b/doc/src/sgml/runtime.sgml
index 8184a3d54e..c4c4874fd0 100644
--- a/doc/src/sgml/runtime.sgml
+++ b/doc/src/sgml/runtime.sgml
@@ -2008,9 +2008,9 @@ pg_dumpall -p 5432 | psql -d postgres -p 5433
    <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>
diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c
index 746e9b4f1e..437d530ba1 100644
--- a/src/interfaces/libpq/fe-connect.c
+++ b/src/interfaces/libpq/fe-connect.c
@@ -1263,6 +1263,23 @@ connectOptions2(PGconn *conn)
 			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;
+		appendPQExpBuffer(&conn->errorMessage,
+						  libpq_gettext("sslrootcert value \"%s\" invalid when SSL support is not compiled in\n"),
+						  conn->sslrootcert);
+		return false;
+	}
+#endif
+
 	/*
 	 * validate sslmode option
 	 */
@@ -1311,6 +1328,22 @@ connectOptions2(PGconn *conn)
 			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;
+		appendPQExpBuffer(&conn->errorMessage,
+						  libpq_gettext("weak sslmode \"%s\" may not be used with sslrootcert=system\n"),
+						  conn->sslmode);
+		return false;
+	}
+#endif
+
 	/*
 	 * Validate TLS protocol versions for ssl_min_protocol_version and
 	 * ssl_max_protocol_version.
@@ -5876,6 +5909,7 @@ static bool
 conninfo_add_defaults(PQconninfoOption *options, PQExpBuffer errorMessage)
 {
 	PQconninfoOption *option;
+	PQconninfoOption *sslmode_default = NULL, *sslrootcert = NULL;
 	char	   *tmp;
 
 	/*
@@ -5892,6 +5926,9 @@ conninfo_add_defaults(PQconninfoOption *options, PQExpBuffer errorMessage)
 	 */
 	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 */
 
@@ -5936,6 +5973,13 @@ conninfo_add_defaults(PQconninfoOption *options, PQExpBuffer errorMessage)
 				}
 				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;
 		}
 
 		/*
@@ -5969,6 +6013,20 @@ conninfo_add_defaults(PQconninfoOption *options, PQExpBuffer errorMessage)
 		}
 	}
 
+	/*
+	 * 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");
+		}
+	}
+
 	return true;
 }
 
diff --git a/src/interfaces/libpq/t/001_uri.pl b/src/interfaces/libpq/t/001_uri.pl
index beaf13b49c..ccfcd77680 100644
--- a/src/interfaces/libpq/t/001_uri.pl
+++ b/src/interfaces/libpq/t/001_uri.pl
@@ -8,7 +8,9 @@ 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},
@@ -209,20 +211,44 @@ 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>',
diff --git a/src/test/ssl/t/001_ssltests.pl b/src/test/ssl/t/001_ssltests.pl
index b34a6f2a81..4747e72318 100644
--- a/src/test/ssl/t/001_ssltests.pl
+++ b/src/test/ssl/t/001_ssltests.pl
@@ -459,6 +459,12 @@ $node->connect_fails(
 	"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;
@@ -468,6 +474,12 @@ SKIP:
 	$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
-- 
2.25.1

Reply via email to