On Mon, Apr 3, 2023 at 12:40 PM Daniel Gustafsson <[email protected]> wrote:
> Doh, sorry, my bad. I read and wrote 1.0.1 but was thinking about 1.0.2. You
> are right, in 1.0.1 that API does not exist. I'm not all too concerned with
> skipping this tests on OpenSSL versions that by the time 16 ships are 6 years
> EOL - and I'm not convinced that spending meson/autoconf cycles to include
> them
> is warranted.
Cool. v10 keys off of HAVE_SSL_CTX_SET_CERT_CB, instead.
> > We could maybe have them connect to a known host:
> >
> > $ echo Q | openssl s_client -connect postgresql.org:443
> > -verify_return_error
>
> Something along these lines is probably best, if we do it at all. Needs
> sleeping on.
Sounds good.
Thanks!
--Jacob
1: 18fd368e0e ! 1: 957a011364 libpq: add sslrootcert=system to use default CAs
@@ .cirrus.yml: task:
ccache_cache:
- ## configure ##
-@@ configure: $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
-@@ configure: 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.
-@@ configure: _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
-
-@@ configure: 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
-
- ## configure.ac ##
-@@ configure.ac: if test "$with_ssl" = openssl ; then
- 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])
-
## doc/src/sgml/libpq.sgml ##
@@ doc/src/sgml/libpq.sgml: postgresql://%2Fvar%2Flib%2Fpostgresql/dbname
to be signed by one of these authorities. The default is
@@ doc/src/sgml/runtime.sgml: pg_dumpall -p 5432 | psql -d postgres -p 5433
<para>
- ## meson.build ##
-@@ meson.build: if sslopt in ['auto', '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
-
-
- ## src/include/pg_config.h.in ##
-@@
- 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
-
## src/interfaces/libpq/fe-secure-openssl.c ##
@@ src/interfaces/libpq/fe-secure-openssl.c: initialize_SSL(PGconn *conn)
else
@@ src/test/ssl/t/001_ssltests.pl: 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");
++# Determine whether this build uses OpenSSL or LibreSSL. As a heuristic,
the
++# HAVE_SSL_CTX_SET_CERT_CB macro isn't defined for LibreSSL. (Nor for
OpenSSL
++# 1.0.1, but that's old enough that accomodating it isn't worth the cost.)
++my $libressl = not check_pg_config("#define HAVE_SSL_CTX_SET_CERT_CB 1");
+
#### Some configuration
2: ba09e1d83f = 2: c822c579ea libpq: force sslmode=verify-full for system CAs
From c822c579ea2c84c14a54c486328ac9ffed6184da Mon Sep 17 00:00:00 2001
From: Jacob Champion <[email protected]>
Date: Mon, 24 Oct 2022 15:24:11 -0700
Subject: [PATCH v10 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 3781a25e39..c8fbb3d06c 100644
--- a/src/test/ssl/t/001_ssltests.pl
+++ b/src/test/ssl/t/001_ssltests.pl
@@ -479,19 +479,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
From 957a011364f4ed73d043217eae8d18ac4d543573 Mon Sep 17 00:00:00 2001
From: Jacob Champion <[email protected]>
Date: Mon, 24 Oct 2022 15:30:25 -0700
Subject: [PATCH v10 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 ++++++-
doc/src/sgml/libpq.sgml | 25 ++++++++++++
doc/src/sgml/runtime.sgml | 6 ++-
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 | 31 +++++++++++++++
7 files changed, 142 insertions(+), 7 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/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/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..3781a25e39 100644
--- a/src/test/ssl/t/001_ssltests.pl
+++ b/src/test/ssl/t/001_ssltests.pl
@@ -31,10 +31,16 @@ sub sslkey
sub switch_server_cert
{
$ssl_server->switch_server_cert(@_);
}
+
+# Determine whether this build uses OpenSSL or LibreSSL. As a heuristic, the
+# HAVE_SSL_CTX_SET_CERT_CB macro isn't defined for LibreSSL. (Nor for OpenSSL
+# 1.0.1, but that's old enough that accomodating it isn't worth the cost.)
+my $libressl = not check_pg_config("#define HAVE_SSL_CTX_SET_CERT_CB 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 +465,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
