settings to control SSL/TLS protocol version
There have been some requests to be able to select the TLS versions
PostgreSQL is using. We currently only hardcode that SSLv2 and SSLv3
are disabled, but there is also some interest now in disabling TLSv1.0
and TLSv1.1. Also, I've had some issues in some combinations with the
new TLSv1.3, so there is perhaps also some use for disabling at the top end.
Attached is a patch that implements this. For example:
ssl_min_protocol_version = 'TLSv1'
ssl_max_protocol_version = 'any'
For reference, here is similar functionality implemented elsewhere:
https://nginx.org/en/docs/http/ngx_http_ssl_module.html#ssl_protocols
https://httpd.apache.org/docs/2.4/mod/mod_ssl.html#sslprotocol
Unlike those two, which offer a list of protocols to use, I have gone
with min and max settings. I think that is easier to use, and it also
maps better to the newer OpenSSL API (SSL_CTX_set_min_proto_version()
etc.). The older SSL_CTX_set_options()-based approach is deprecated and
has some very weird behaviors that would make it complicated to use for
anything more than a min/max.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Attachments:
0001-Add-settings-to-control-SSL-TLS-protocol-version.patchtext/plain; charset=UTF-8; name=0001-Add-settings-to-control-SSL-TLS-protocol-version.patch; x-mac-creator=0; x-mac-type=0Download
From 768ccf861b6904baa25a601a09c5e440f3f62cca Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter_e@gmx.net>
Date: Mon, 1 Oct 2018 21:43:30 +0200
Subject: [PATCH] Add settings to control SSL/TLS protocol version
For example:
ssl_min_protocol_version = 'TLSv1'
ssl_max_protocol_version = 'any'
---
doc/src/sgml/config.sgml | 44 +++++++
src/backend/libpq/be-secure-openssl.c | 123 +++++++++++++++++-
src/backend/libpq/be-secure.c | 3 +
src/backend/utils/misc/guc.c | 33 +++++
src/backend/utils/misc/postgresql.conf.sample | 2 +
src/include/libpq/libpq.h | 11 ++
6 files changed, 214 insertions(+), 2 deletions(-)
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index f11b8f724c..efdf8a2849 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -1290,6 +1290,50 @@ <title>SSL</title>
</listitem>
</varlistentry>
+ <varlistentry id="guc-ssl-min-protocol-version" xreflabel="ssl_min_protocol_version">
+ <term><varname>ssl_min_protocol_version</varname> (<type>enum</type>)
+ <indexterm>
+ <primary><varname>ssl_min_protocol_version</varname> configuration parameter</primary>
+ </indexterm>
+ </term>
+ <listitem>
+ <para>
+ Sets the minimum SSL/TLS protocol version to use. Valid values are
+ currently: <literal>TLSv1</literal>, <literal>TLSv1.1</literal>,
+ <literal>TLSv1.2</literal>, <literal>TLSv1.3</literal>. Older
+ versions of the <productname>OpenSSL</productname> library do not
+ support all values. An error will be raised if an unsupported setting
+ is chosen. Protocol versions before TLS 1.0, namely SSL version 2 and
+ 3, are always disabled.
+ </para>
+
+ <para>
+ The default is <literal>TLSv1</literal>, mainly to support older
+ versions of the <productname>OpenSSL</productname> library. You might
+ want to set this to a higher value if all software components can
+ support the newer protocol versions.
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry id="guc-ssl-max-protocol-version" xreflabel="ssl_max_protocol_version">
+ <term><varname>ssl_max_protocol_version</varname> (<type>enum</type>)
+ <indexterm>
+ <primary><varname>ssl_max_protocol_version</varname> configuration parameter</primary>
+ </indexterm>
+ </term>
+ <listitem>
+ <para>
+ Sets the maximum SSL/TLS protocol version to use. Valid values are as
+ for <xref linkend="guc-ssl-min-protocol-version"/>, with addition of
+ <literal>any</literal> which allows any protocol version. The default is
+ <literal>any</literal>. Setting the maximum protocol version is
+ mainly useful for testing or if some component has issues working with
+ a newer protocol.
+ </para>
+ </listitem>
+ </varlistentry>
+
<varlistentry id="guc-ssl-dh-params-file" xreflabel="ssl_dh_params_file">
<term><varname>ssl_dh_params_file</varname> (<type>string</type>)
<indexterm>
diff --git a/src/backend/libpq/be-secure-openssl.c b/src/backend/libpq/be-secure-openssl.c
index 6a576572bb..b2b0cccdae 100644
--- a/src/backend/libpq/be-secure-openssl.c
+++ b/src/backend/libpq/be-secure-openssl.c
@@ -67,6 +67,12 @@ static bool SSL_initialized = false;
static bool dummy_ssl_passwd_cb_called = false;
static bool ssl_is_server_start;
+static int ssl_protocol_version_to_openssl(int v, const char *guc_name);
+#if (OPENSSL_VERSION_NUMBER < 0x10100000L)
+static int SSL_CTX_set_min_proto_version(SSL_CTX *ctx, int version);
+static int SSL_CTX_set_max_proto_version(SSL_CTX *ctx, int version);
+#endif
+
/* ------------------------------------------------------------ */
/* Public interface */
@@ -183,8 +189,14 @@ be_tls_init(bool isServerStart)
goto error;
}
- /* disallow SSL v2/v3 */
- SSL_CTX_set_options(context, SSL_OP_NO_SSLv2 | SSL_OP_NO_SSLv3);
+ if (ssl_min_protocol_version)
+ SSL_CTX_set_min_proto_version(context,
+ ssl_protocol_version_to_openssl(ssl_min_protocol_version,
+ "ssl_min_protocol_version"));
+ if (ssl_max_protocol_version)
+ SSL_CTX_set_max_proto_version(context,
+ ssl_protocol_version_to_openssl(ssl_max_protocol_version,
+ "ssl_max_protocol_version"));
/* disallow SSL session tickets */
#ifdef SSL_OP_NO_TICKET /* added in OpenSSL 0.9.8f */
@@ -1209,3 +1221,110 @@ X509_NAME_to_cstring(X509_NAME *name)
return result;
}
+
+/*
+ * Convert TLS protocol version GUC enum to OpenSSL values
+ *
+ * This is a straightforward one-to-one mapping, but doing it this way makes
+ * guc.c independent of OpenSSL availability and version.
+ *
+ * If a version is passed that is not supported by the current OpenSSL
+ * version, then we throw an error, so that subsequent code can assume it's
+ * working with a supported version.
+ */
+static int
+ssl_protocol_version_to_openssl(int v, const char *guc_name)
+{
+ switch (v)
+ {
+ case PG_TLS_ANY:
+ return 0;
+ case PG_TLS1_VERSION:
+ return TLS1_VERSION;
+ case PG_TLS1_1_VERSION:
+#ifdef TLS1_1_VERSION
+ return TLS1_1_VERSION;
+#else
+ goto error;
+#endif
+ case PG_TLS1_2_VERSION:
+#ifdef TLS1_2_VERSION
+ return TLS1_2_VERSION;
+#else
+ goto error;
+#endif
+ case PG_TLS1_3_VERSION:
+#ifdef TLS1_3_VERSION
+ return TLS1_3_VERSION;
+#else
+ goto error;
+#endif
+ }
+
+error:
+ pg_attribute_unused();
+ ereport(ERROR,
+ (errmsg("%s setting %s not supported by this build",
+ guc_name,
+ GetConfigOption(guc_name, false, false))));
+ return -1;
+}
+
+/*
+ * Replacements for APIs present in newer versions of OpenSSL
+ */
+#if (OPENSSL_VERSION_NUMBER < 0x10100000L)
+
+/*
+ * OpenSSL versions that support TLS 1.3 shouldn't get here because they
+ * already have these functions. So we don't have to keep updating the below
+ * code for every new TLS version, and eventually it can go away. But let's
+ * just check this to make sure ...
+ */
+#ifdef TLS1_3_VERSION
+#error OpenSSL version mismatch
+#endif
+
+static int
+SSL_CTX_set_min_proto_version(SSL_CTX *ctx, int version)
+{
+ int ssl_options = SSL_OP_NO_SSLv2 | SSL_OP_NO_SSLv3;
+
+ if (version > TLS1_VERSION)
+ ssl_options |= SSL_OP_NO_TLSv1;
+#ifdef TLS1_1_VERSION
+ if (version > TLS1_1_VERSION)
+ ssl_options |= SSL_OP_NO_TLSv1_1;
+#endif
+#ifdef TLS1_2_VERSION
+ if (version > TLS1_2_VERSION)
+ ssl_options |= SSL_OP_NO_TLSv1_2;
+#endif
+
+ SSL_CTX_set_options(ctx, ssl_options);
+
+ return 1; /* success */
+}
+
+static int
+SSL_CTX_set_max_proto_version(SSL_CTX *ctx, int version)
+{
+ int ssl_options = 0;
+
+ AssertArg(version != 0);
+
+#ifdef TLS1_1_VERSION
+ if (version < TLS1_1_VERSION)
+ ssl_options |= SSL_OP_NO_TLSv1_1;
+#endif
+#ifdef TLS1_2_VERSION
+ if (version < TLS1_2_VERSION)
+ ssl_options |= SSL_OP_NO_TLSv1_2;
+#endif
+
+ SSL_CTX_set_options(ctx, ssl_options);
+
+ return 1; /* success */
+}
+
+#endif /* OPENSSL_VERSION_NUMBER */
diff --git a/src/backend/libpq/be-secure.c b/src/backend/libpq/be-secure.c
index d349d7c2c7..0cf97cb9b0 100644
--- a/src/backend/libpq/be-secure.c
+++ b/src/backend/libpq/be-secure.c
@@ -60,6 +60,9 @@ char *SSLECDHCurve;
/* GUC variable: if false, prefer client ciphers */
bool SSLPreferServerCiphers;
+int ssl_min_protocol_version;
+int ssl_max_protocol_version;
+
/* ------------------------------------------------------------ */
/* Procedures common to all secure sessions */
/* ------------------------------------------------------------ */
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index 0bec3914f8..43d4efaaae 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -429,6 +429,15 @@ static const struct config_enum_entry password_encryption_options[] = {
{NULL, 0, false}
};
+const struct config_enum_entry ssl_protocol_versions_info[] = {
+ {"any", PG_TLS_ANY, false},
+ {"TLSv1", PG_TLS1_VERSION, false},
+ {"TLSv1.1", PG_TLS1_1_VERSION, false},
+ {"TLSv1.2", PG_TLS1_2_VERSION, false},
+ {"TLSv1.3", PG_TLS1_3_VERSION, false},
+ {NULL, 0, false}
+};
+
/*
* Options for enum values stored in other modules
*/
@@ -4187,6 +4196,30 @@ static struct config_enum ConfigureNamesEnum[] =
NULL, NULL, NULL
},
+ {
+ {"ssl_min_protocol_version", PGC_SIGHUP, CONN_AUTH_SSL,
+ gettext_noop("Sets the minimum SSL/TLS protocol version to use."),
+ NULL,
+ GUC_SUPERUSER_ONLY
+ },
+ &ssl_min_protocol_version,
+ PG_TLS1_VERSION,
+ ssl_protocol_versions_info + 1 /* don't allow 'any' */,
+ NULL, NULL, NULL
+ },
+
+ {
+ {"ssl_max_protocol_version", PGC_SIGHUP, CONN_AUTH_SSL,
+ gettext_noop("Sets the maximum SSL/TLS protocol version to use."),
+ NULL,
+ GUC_SUPERUSER_ONLY
+ },
+ &ssl_max_protocol_version,
+ PG_TLS_ANY,
+ ssl_protocol_versions_info,
+ NULL, NULL, NULL
+ },
+
/* End-of-list marker */
{
{NULL, 0, 0, NULL, NULL}, NULL, 0, NULL, NULL, NULL, NULL
diff --git a/src/backend/utils/misc/postgresql.conf.sample b/src/backend/utils/misc/postgresql.conf.sample
index 4e61bc6521..1e71e7196d 100644
--- a/src/backend/utils/misc/postgresql.conf.sample
+++ b/src/backend/utils/misc/postgresql.conf.sample
@@ -103,6 +103,8 @@
#ssl_ciphers = 'HIGH:MEDIUM:+3DES:!aNULL' # allowed SSL ciphers
#ssl_prefer_server_ciphers = on
#ssl_ecdh_curve = 'prime256v1'
+#ssl_min_protocol_version = 'TLSv1'
+#ssl_max_protocol_version = 'any'
#ssl_dh_params_file = ''
#ssl_passphrase_command = ''
#ssl_passphrase_command_supports_reload = off
diff --git a/src/include/libpq/libpq.h b/src/include/libpq/libpq.h
index 36baf6b919..d3a12358ca 100644
--- a/src/include/libpq/libpq.h
+++ b/src/include/libpq/libpq.h
@@ -102,6 +102,17 @@ extern WaitEventSet *FeBeWaitSet;
extern char *SSLCipherSuites;
extern char *SSLECDHCurve;
extern bool SSLPreferServerCiphers;
+extern int ssl_min_protocol_version;
+extern int ssl_max_protocol_version;
+
+enum ssl_protocol_versions
+{
+ PG_TLS_ANY = 0,
+ PG_TLS1_VERSION,
+ PG_TLS1_1_VERSION,
+ PG_TLS1_2_VERSION,
+ PG_TLS1_3_VERSION
+};
/*
* prototypes for functions in be-secure-common.c
--
2.19.0
On 1 Oct 2018, at 22:21, Peter Eisentraut <peter.eisentraut@2ndquadrant.com> wrote:
There have been some requests to be able to select the TLS versions
PostgreSQL is using. We currently only hardcode that SSLv2 and SSLv3
are disabled, but there is also some interest now in disabling TLSv1.0
and TLSv1.1. Also, I've had some issues in some combinations with the
new TLSv1.3, so there is perhaps also some use for disabling at the top end.Attached is a patch that implements this. For example:
ssl_min_protocol_version = 'TLSv1'
ssl_max_protocol_version = ‘any'
I don’t think ‘any’ is a clear name for a setting which means “the highest
supported version”. How about ‘max_supported’ or something similar?
For reference, here is similar functionality implemented elsewhere:
https://nginx.org/en/docs/http/ngx_http_ssl_module.html#ssl_protocols
https://httpd.apache.org/docs/2.4/mod/mod_ssl.html#sslprotocolUnlike those two, which offer a list of protocols to use, I have gone
with min and max settings.
FWIW, libcurl also supports a min/max approach with CURLOPT_SSLVERSION:
https://curl.haxx.se/libcurl/c/CURLOPT_SSLVERSION.html
+1 for using a min/max approach for setting the version, and it should be
trivial to add support for in the pending GnuTLS and Secure Transport patches.
cheers ./daniel
On 01/10/2018 23:30, Daniel Gustafsson wrote:
ssl_min_protocol_version = 'TLSv1'
ssl_max_protocol_version = ‘any'I don’t think ‘any’ is a clear name for a setting which means “the highest
supported version”. How about ‘max_supported’ or something similar?
I can see the argument for an alternative, but your suggestion is a
mouthful.
+1 for using a min/max approach for setting the version, and it should be
trivial to add support for in the pending GnuTLS and Secure Transport patches.
AFAICT, in GnuTLS this is done via the "priorities" setting that also
sets the ciphers. There is no separate API for just the TLS version.
It would be interesting to see how Secure Transport can do it.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On 2 Oct 2018, at 14:23, Peter Eisentraut <peter.eisentraut@2ndquadrant.com> wrote:
On 01/10/2018 23:30, Daniel Gustafsson wrote:
ssl_min_protocol_version = 'TLSv1'
ssl_max_protocol_version = ‘any'I don’t think ‘any’ is a clear name for a setting which means “the highest
supported version”. How about ‘max_supported’ or something similar?I can see the argument for an alternative, but your suggestion is a
mouthful.
Agreed, but I can’t think of a better wording. Perhaps just ‘tls_max’?
+1 for using a min/max approach for setting the version, and it should be
trivial to add support for in the pending GnuTLS and Secure Transport patches.AFAICT, in GnuTLS this is done via the "priorities" setting that also
sets the ciphers. There is no separate API for just the TLS version.
It would be interesting to see how Secure Transport can do it.
Secure Transport has a fairly neat API for this, SSLSetProtocolVersionMax() and
SSLSetProtocolVersionMin() (available since Lion).
cheers ./daniel
The following review has been posted through the commitfest application:
make installcheck-world: tested, passed
Implements feature: tested, passed
Spec compliant: not tested
Documentation: tested, passed
I've reviewed the patch and here are my comments.
The feature seems useful a lot of application servers are implementing minimal TLS protocol versions.
I don't see a way to restrict libpq to only connect with certain protocol versions. Maybe that is a separate patch but it would make this feature harder to test in the future.
I tested with a server configured to via the options to only TLS1.3 and clients without TLSv1.3 support and confirmed that I couldn't connect with SSL. This is fine
I tested with options to restrict the max version to TLSv1 and verified that the clients connected with TLSv1. This is fine
I tested with a min protocol version greater than the max. The server started up (Do we want this to be an warning on startup?) but I wasn't able to connect with SSL. The following was in the server log
could not accept SSL connection: unknown protocol
I tested with a max protocol version set to any. This is fine.
I tested putting TLSv1.3 in the config file when my openssl library did not support 1.3. This is fine.
I am updating the patch status to ready for committer.
The new status of this patch is: Ready for Committer
On Mon, Oct 1, 2018 at 4:21 PM Peter Eisentraut
<peter.eisentraut@2ndquadrant.com> wrote:
There have been some requests to be able to select the TLS versions
PostgreSQL is using. We currently only hardcode that SSLv2 and SSLv3
are disabled, but there is also some interest now in disabling TLSv1.0
and TLSv1.1. Also, I've had some issues in some combinations with the
new TLSv1.3, so there is perhaps also some use for disabling at the top end.Attached is a patch that implements this. For example:
ssl_min_protocol_version = 'TLSv1'
ssl_max_protocol_version = 'any'
+1. Maybe it would make sense to spell 'any' as the empty string.
Intuitively, it makes more sense to me to think about there being no
maximum than to think about the maximum being anything.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
On Mon, Nov 05, 2018 at 03:01:58PM -0500, Robert Haas wrote:
On Mon, Oct 1, 2018 at 4:21 PM Peter Eisentraut
<peter.eisentraut@2ndquadrant.com> wrote:There have been some requests to be able to select the TLS versions
PostgreSQL is using. We currently only hardcode that SSLv2 and SSLv3
are disabled, but there is also some interest now in disabling TLSv1.0
and TLSv1.1. Also, I've had some issues in some combinations with the
new TLSv1.3, so there is perhaps also some use for disabling at the top end.Attached is a patch that implements this. For example:
ssl_min_protocol_version = 'TLSv1'
ssl_max_protocol_version = 'any'+1. Maybe it would make sense to spell 'any' as the empty string.
Intuitively, it makes more sense to me to think about there being no
maximum than to think about the maximum being anything.
..and now, I'm finally beginning to see the reasoning that led Oracle
to conflate NULL and empty string.
Best,
David.
--
David Fetter <david(at)fetter(dot)org> http://fetter.org/
Phone: +1 415 235 3778
Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate
On Monday, November 5, 2018, David Fetter <david@fetter.org> wrote:
On Mon, Nov 05, 2018 at 03:01:58PM -0500, Robert Haas wrote:
On Mon, Oct 1, 2018 at 4:21 PM Peter Eisentraut
<peter.eisentraut@2ndquadrant.com> wrote:Attached is a patch that implements this. For example:
ssl_min_protocol_version = 'TLSv1'
ssl_max_protocol_version = 'any'+1. Maybe it would make sense to spell 'any' as the empty string.
Intuitively, it makes more sense to me to think about there being no
maximum than to think about the maximum being anything...and now, I'm finally beginning to see the reasoning that led Oracle
to conflate NULL and empty string.
Seems like a situation for ‘n/a’ though maybe that’s too English-centric...
I’m a bit uncertain about the mix of name and number in something that
purports to be a version and thus should be numeric only. SSLv3 and TLSv2
would not be comparable in terms of min/max...but I haven’t delved deeply
into the feature either.
David J.
On Mon, Nov 05, 2018 at 03:01:58PM -0500, Robert Haas wrote:
+1. Maybe it would make sense to spell 'any' as the empty string.
Intuitively, it makes more sense to me to think about there being no
maximum than to think about the maximum being anything.
I have looked at the code a bit yesterday and the implementation as well
as how things are handled with OpenSSL looked sane to me. The
suggestion of using an empty string as the default instead of 'any' also
makes sense per your argument
--
Michael
On 04/11/2018 04:24, Steve Singer wrote:
The feature seems useful a lot of application servers are implementing minimal TLS protocol versions.
I don't see a way to restrict libpq to only connect with certain protocol versions. Maybe that is a separate patch but it would make this feature harder to test in the future.
Client-side support could be separate patch, yes. It seems less important.
I am updating the patch status to ready for committer.
The new status of this patch is: Ready for Committer
Committed with the change 'any' -> '' that was discussed elsewhere in
the thread. Thanks.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services