Re: Improve errors when setting incorrect bounds for SSL protocols
> On 30 Apr 2020, at 01:14, Michael Paquier wrote: > > On Wed, Apr 29, 2020 at 01:57:49PM +0200, Daniel Gustafsson wrote: >> Working in the TLS corners of the backend, I found while re-reviewing and >> re-testing for the release that this patch actually was a small, but vital, >> brick shy of a load. The error handling is always invoked due to a set of >> missing braces. Going into the check will cause the context to be freed and >> be_tls_open_server error out. The tests added narrowly escapes it by not >> setting the max version in the final test, but I'm not sure it's worth >> changing >> that now as not setting a value is an interesting testcase too. Sorry for >> missing that at the time of reviewing. > > Good catch, fixed. We would still have keep around the SSL old > context if both bounds were set. Testing this case would mean one > extra full restart of the server, and I am not sure either if that's > worth the extra cost here. Agreed. I don't think the cost is warranted given the low probability of new errors around here, so I think the commit as it stands is sufficient. Thanks. cheers ./daniel
Re: Improve errors when setting incorrect bounds for SSL protocols
On Wed, Apr 29, 2020 at 01:57:49PM +0200, Daniel Gustafsson wrote: > Working in the TLS corners of the backend, I found while re-reviewing and > re-testing for the release that this patch actually was a small, but vital, > brick shy of a load. The error handling is always invoked due to a set of > missing braces. Going into the check will cause the context to be freed and > be_tls_open_server error out. The tests added narrowly escapes it by not > setting the max version in the final test, but I'm not sure it's worth > changing > that now as not setting a value is an interesting testcase too. Sorry for > missing that at the time of reviewing. Good catch, fixed. We would still have keep around the SSL old context if both bounds were set. Testing this case would mean one extra full restart of the server, and I am not sure either if that's worth the extra cost here. -- Michael signature.asc Description: PGP signature
Re: Improve errors when setting incorrect bounds for SSL protocols
Working in the TLS corners of the backend, I found while re-reviewing and re-testing for the release that this patch actually was a small, but vital, brick shy of a load. The error handling is always invoked due to a set of missing braces. Going into the check will cause the context to be freed and be_tls_open_server error out. The tests added narrowly escapes it by not setting the max version in the final test, but I'm not sure it's worth changing that now as not setting a value is an interesting testcase too. Sorry for missing that at the time of reviewing. cheers ./daniel minmaxproto_guc.patch Description: Binary data
Re: Improve errors when setting incorrect bounds for SSL protocols
On Thu, Mar 19, 2020 at 10:54:35PM +0100, Daniel Gustafsson wrote:
> In this message we aren't quoting the TLS protocol setting:
> + (errmsg("%s setting %s not supported by this build",
> ..but in this detail we are:
> + errdetail("\"%s\" cannot be higher than \"%s\"",
> Perhaps we should be consistent across all ereports?
Right. Using quotes is a more popular style when it comes to GUC
parameters and their values, so switched to use that, and committed
the patch. Thanks for the review.
--
Michael
signature.asc
Description: PGP signature
Re: Improve errors when setting incorrect bounds for SSL protocols
> On 7 Feb 2020, at 01:33, Michael Paquier wrote:
>
> On Thu, Feb 06, 2020 at 11:30:40PM +0100, Daniel Gustafsson wrote:
>> Or change to the v1 patch in this thread, which avoids the problem by doing
>> it
>> in the OpenSSL code. It's a shame to have generic TLS functionality be
>> OpenSSL
>> specific when everything else TLS has been abstracted, but not working is
>> clearly a worse option.
>
> The v1 would work just fine considering that, as the code would be
> invoked in a context where all GUCs are already loaded. That's too
> late before the release though, so I have reverted 41aadee, and
> attached is a new patch to consider with improvements compared to v1
> mainly in the error messages.
Having gone back to look at this, I can't think of a better way to implement
this and I think we should go ahead with the proposed patch.
In this message we aren't quoting the TLS protocol setting:
+ (errmsg("%s setting %s not supported by this build",
..but in this detail we are:
+ errdetail("\"%s\" cannot be higher than \"%s\"",
Perhaps we should be consistent across all ereports?
Marking as ready for committer.
cheers ./daniel
Re: Improve errors when setting incorrect bounds for SSL protocols
On Thu, Feb 06, 2020 at 11:30:40PM +0100, Daniel Gustafsson wrote:
> Or change to the v1 patch in this thread, which avoids the problem by doing it
> in the OpenSSL code. It's a shame to have generic TLS functionality be
> OpenSSL
> specific when everything else TLS has been abstracted, but not working is
> clearly a worse option.
The v1 would work just fine considering that, as the code would be
invoked in a context where all GUCs are already loaded. That's too
late before the release though, so I have reverted 41aadee, and
attached is a new patch to consider with improvements compared to v1
mainly in the error messages.
--
Michael
diff --git a/src/backend/libpq/be-secure-openssl.c b/src/backend/libpq/be-secure-openssl.c
index 18d3fff068..5b772fd58c 100644
--- a/src/backend/libpq/be-secure-openssl.c
+++ b/src/backend/libpq/be-secure-openssl.c
@@ -68,8 +68,7 @@ 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,
- int loglevel);
+static int ssl_protocol_version_to_openssl(int v);
/* */
/* Public interface */
@@ -80,6 +79,8 @@ be_tls_init(bool isServerStart)
{
STACK_OF(X509_NAME) *root_cert_list = NULL;
SSL_CTX*context;
+ int ssl_ver_min = -1;
+ int ssl_ver_max = -1;
/* This stuff need be done only once. */
if (!SSL_initialized)
@@ -188,13 +189,19 @@ be_tls_init(bool isServerStart)
if (ssl_min_protocol_version)
{
- int ssl_ver = ssl_protocol_version_to_openssl(ssl_min_protocol_version,
- "ssl_min_protocol_version",
- isServerStart ? FATAL : LOG);
+ ssl_ver_min = ssl_protocol_version_to_openssl(ssl_min_protocol_version);
- if (ssl_ver == -1)
+ if (ssl_ver_min == -1)
+ {
+ ereport(isServerStart ? FATAL : LOG,
+ (errmsg("%s setting %s not supported by this build",
+ "ssl_min_protocol_version",
+ GetConfigOption("ssl_min_protocol_version",
+ false, false;
goto error;
- if (!SSL_CTX_set_min_proto_version(context, ssl_ver))
+ }
+
+ if (!SSL_CTX_set_min_proto_version(context, ssl_ver_min))
{
ereport(isServerStart ? FATAL : LOG,
(errmsg("could not set minimum SSL protocol version")));
@@ -204,13 +211,19 @@ be_tls_init(bool isServerStart)
if (ssl_max_protocol_version)
{
- int ssl_ver = ssl_protocol_version_to_openssl(ssl_max_protocol_version,
- "ssl_max_protocol_version",
- isServerStart ? FATAL : LOG);
+ ssl_ver_max = ssl_protocol_version_to_openssl(ssl_max_protocol_version);
- if (ssl_ver == -1)
+ if (ssl_ver_max == -1)
+ {
+ ereport(isServerStart ? FATAL : LOG,
+ (errmsg("%s setting %s not supported by this build",
+ "ssl_max_protocol_version",
+ GetConfigOption("ssl_max_protocol_version",
+ false, false;
goto error;
- if (!SSL_CTX_set_max_proto_version(context, ssl_ver))
+ }
+
+ if (!SSL_CTX_set_max_proto_version(context, ssl_ver_max))
{
ereport(isServerStart ? FATAL : LOG,
(errmsg("could not set maximum SSL protocol version")));
@@ -218,6 +231,23 @@ be_tls_init(bool isServerStart)
}
}
+ /* Check compatibility of min/max protocols */
+ if (ssl_min_protocol_version &&
+ ssl_max_protocol_version)
+ {
+ /*
+ * No need to check for invalid values (-1) for each protocol number
+ * as the code above would have already generated an error.
+ */
+ if (ssl_ver_min > ssl_ver_max)
+ ereport(isServerStart ? FATAL : LOG,
+ (errmsg("could not set SSL protocol version range"),
+ errdetail("\"%s\" cannot be higher than \"%s\"",
+ "ssl_min_protocol_version",
+ "ssl_max_protocol_version")));
+ goto error;
+ }
+
/* disallow SSL session tickets */
SSL_CTX_set_options(context, SSL_OP_NO_TICKET);
@@ -1271,15 +1301,14 @@ X509_NAME_to_cstring(X509_NAME *name)
* guc.c independent of OpenSSL availability and version.
*
* If a version is passed that is not supported by the current OpenSSL
- * version, then we log with the given loglevel and return (if we return) -1.
- * If a nonnegative value is returned, subsequent code can assume it's working
- * with a supported version.
+ * version, then we return -1. If a nonnegative value is returned,
+ * subsequent code can assume it's working with a supported version.
*
* Note: this is rather similar to libpq's routine in fe-secure-openssl.c,
* so make sure to update both routines if changing this one.
*/
static int
-ssl_protocol_version_to_openssl(int v, const char *guc_name, int loglevel)
+ssl_protocol_version_to_openssl(int v)
{
switch (v)
{
@@ -1307,9 +1336,5 @@ ssl_protocol_version_to_openssl(int v, const char *guc_name, int loglevel)
#endif
}
- ereport(loglevel,
- (errmsg("%s setting %s not supported by this build",
- guc_name,
- GetConfigOption(guc_name, false,
Re: Improve errors when setting incorrect bounds for SSL protocols
> On 6 Feb 2020, at 20:04, Tom Lane wrote: > I think this should be reverted. Perhaps there's a way to do it without > these problems, but we failed to find one in the past. Or change to the v1 patch in this thread, which avoids the problem by doing it in the OpenSSL code. It's a shame to have generic TLS functionality be OpenSSL specific when everything else TLS has been abstracted, but not working is clearly a worse option. cheers ./daniel
Re: Improve errors when setting incorrect bounds for SSL protocols
Michael Paquier writes: > On Thu, Jan 16, 2020 at 10:00:52AM +0900, Michael Paquier wrote: >> Thanks for the review. Let's wait a couple of days to see if others >> have objections or more comments about this patch, but I'd like to >> fix the issue and backpatch down to 12 where the parameters have been >> introduced. > And committed. I just happened to look at this patch while working on the release notes. I think this is a bad idea and very probably creates worse problems than it fixes. As we have learned painfully in the past, you can't have GUC check or assign hooks that look at other GUC variables, because that creates order-of-operations problems. If a postgresql.conf update is trying to change both values (hardly an unlikely scenario, for this pair of variables) then the checks are going to be comparing against the old values of the other variables, leading to either incorrect rejections of valid states or incorrect acceptances of invalid states. It's pure accident that the particular cases tested in the regression tests behave sanely. I think this should be reverted. Perhaps there's a way to do it without these problems, but we failed to find one in the past. regards, tom lane
Re: Improve errors when setting incorrect bounds for SSL protocols
On 2020-01-15 03:28, Michael Paquier wrote: Good points. And the get routines are not that portable in OpenSSL either even if HEAD supports 1.0.1 and newer versions... Attached is an updated patch which uses a GUC check for both parameters, and provides a hint on top of the original error message. The SSL context does not get reloaded if there is an error, so the errors from OpenSSL cannot be triggered as far as I checked (after mixing a couple of corrent and incorrect combinations manually). The reason this wasn't done originally is that it is not correct to have GUC check hooks that refer to other GUC variables, because otherwise you get inconsistent behavior depending on the order of processing of the assignments. In this case, I think it would work because you have symmetric checks for both variables, but in general it is a problematic strategy. -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Re: Improve errors when setting incorrect bounds for SSL protocols
On Thu, Jan 16, 2020 at 10:00:52AM +0900, Michael Paquier wrote: > Thanks for the review. Let's wait a couple of days to see if others > have objections or more comments about this patch, but I'd like to > fix the issue and backpatch down to 12 where the parameters have been > introduced. And committed. -- Michael signature.asc Description: PGP signature
Re: Improve errors when setting incorrect bounds for SSL protocols
On Wed, Jan 15, 2020 at 06:34:39PM +0100, Daniel Gustafsson wrote: > This is pretty much exactly the patch I was intending to write for this, so +1 > from me. Thanks for the review. Let's wait a couple of days to see if others have objections or more comments about this patch, but I'd like to fix the issue and backpatch down to 12 where the parameters have been introduced. -- Michael signature.asc Description: PGP signature
Re: Improve errors when setting incorrect bounds for SSL protocols
> On 15 Jan 2020, at 03:28, Michael Paquier wrote: > > On Tue, Jan 14, 2020 at 11:21:53AM +0100, Daniel Gustafsson wrote: >> On 14 Jan 2020, at 04:54, Michael Paquier wrote: >>> Please note that OpenSSL 1.1.0 has added two routines to be able to >>> get the min/max protocols set in a context, called >>> SSL_CTX_get_min/max_proto_version. Thinking about older versions of >>> OpenSSL I think that it is better to use >>> ssl_protocol_version_to_openssl to do the parsing work. I also found >>> that it is easier to check for compatible versions after setting both >>> bounds in the SSL context, so as there is no need to worry about >>> invalid values depending on the build of OpenSSL used. >> >> I'm not convinced that it's a good idea to check for incompatible protocol >> range in the OpenSSL backend. We've spent a lot of energy to make the TLS >> code >> library agnostic and pluggable, and since identifying a basic configuration >> error isn't OpenSSL specific I think it should be in the guc code. That >> would >> keep the layering as well as ensure that we don't mistakenly treat this >> differently should we get a second TLS backend. > > Good points. And the get routines are not that portable in OpenSSL > either even if HEAD supports 1.0.1 and newer versions... Attached is > an updated patch which uses a GUC check for both parameters, and > provides a hint on top of the original error message. The SSL context > does not get reloaded if there is an error, so the errors from OpenSSL > cannot be triggered as far as I checked (after mixing a couple of > corrent and incorrect combinations manually). This is pretty much exactly the patch I was intending to write for this, so +1 from me. cheers ./daniel
Re: Improve errors when setting incorrect bounds for SSL protocols
On Tue, Jan 14, 2020 at 11:21:53AM +0100, Daniel Gustafsson wrote:
> On 14 Jan 2020, at 04:54, Michael Paquier wrote:
>> Please note that OpenSSL 1.1.0 has added two routines to be able to
>> get the min/max protocols set in a context, called
>> SSL_CTX_get_min/max_proto_version. Thinking about older versions of
>> OpenSSL I think that it is better to use
>> ssl_protocol_version_to_openssl to do the parsing work. I also found
>> that it is easier to check for compatible versions after setting both
>> bounds in the SSL context, so as there is no need to worry about
>> invalid values depending on the build of OpenSSL used.
>
> I'm not convinced that it's a good idea to check for incompatible protocol
> range in the OpenSSL backend. We've spent a lot of energy to make the TLS
> code
> library agnostic and pluggable, and since identifying a basic configuration
> error isn't OpenSSL specific I think it should be in the guc code. That would
> keep the layering as well as ensure that we don't mistakenly treat this
> differently should we get a second TLS backend.
Good points. And the get routines are not that portable in OpenSSL
either even if HEAD supports 1.0.1 and newer versions... Attached is
an updated patch which uses a GUC check for both parameters, and
provides a hint on top of the original error message. The SSL context
does not get reloaded if there is an error, so the errors from OpenSSL
cannot be triggered as far as I checked (after mixing a couple of
corrent and incorrect combinations manually).
--
Michael
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index e5f8a1301f..d97fe3beb2 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -204,6 +204,10 @@ static const char *show_log_file_mode(void);
static const char *show_data_directory_mode(void);
static bool check_backtrace_functions(char **newval, void **extra, GucSource source);
static void assign_backtrace_functions(const char *newval, void *extra);
+static bool check_ssl_min_protocol_version(int *newval, void **extra,
+ GucSource source);
+static bool check_ssl_max_protocol_version(int *newval, void **extra,
+ GucSource source);
static bool check_recovery_target_timeline(char **newval, void **extra, GucSource source);
static void assign_recovery_target_timeline(const char *newval, void *extra);
static bool check_recovery_target(char **newval, void **extra, GucSource source);
@@ -4594,7 +4598,7 @@ static struct config_enum ConfigureNamesEnum[] =
_min_protocol_version,
PG_TLS1_2_VERSION,
ssl_protocol_versions_info + 1, /* don't allow PG_TLS_ANY */
- NULL, NULL, NULL
+ check_ssl_min_protocol_version, NULL, NULL
},
{
@@ -4606,7 +4610,7 @@ static struct config_enum ConfigureNamesEnum[] =
_max_protocol_version,
PG_TLS_ANY,
ssl_protocol_versions_info,
- NULL, NULL, NULL
+ check_ssl_max_protocol_version, NULL, NULL
},
/* End-of-list marker */
@@ -11603,6 +11607,49 @@ assign_backtrace_functions(const char *newval, void *extra)
backtrace_symbol_list = (char *) extra;
}
+static bool
+check_ssl_min_protocol_version(int *newval, void **extra, GucSource source)
+{
+ int new_ssl_min_protocol_version = *newval;
+
+ /* PG_TLS_ANY is not supported for the minimum bound */
+ Assert(new_ssl_min_protocol_version > PG_TLS_ANY);
+
+ if (ssl_max_protocol_version &&
+ new_ssl_min_protocol_version > ssl_max_protocol_version)
+ {
+ GUC_check_errhint("\"%s\" cannot be higher than \"%s\".",
+ "ssl_min_protocol_version",
+ "ssl_max_protocol_version");
+ GUC_check_errcode(ERRCODE_INVALID_PARAMETER_VALUE);
+ return false;
+ }
+
+ return true;
+}
+
+static bool
+check_ssl_max_protocol_version(int *newval, void **extra, GucSource source)
+{
+ int new_ssl_max_protocol_version = *newval;
+
+ /* if PG_TLS_ANY, there is no need to check the bounds */
+ if (new_ssl_max_protocol_version == PG_TLS_ANY)
+ return true;
+
+ if (ssl_min_protocol_version &&
+ ssl_min_protocol_version > new_ssl_max_protocol_version)
+ {
+ GUC_check_errhint("\"%s\" cannot be lower than \"%s\".",
+ "ssl_max_protocol_version",
+ "ssl_min_protocol_version");
+ GUC_check_errcode(ERRCODE_INVALID_PARAMETER_VALUE);
+ return false;
+ }
+
+ return true;
+}
+
static bool
check_recovery_target_timeline(char **newval, void **extra, GucSource source)
{
diff --git a/src/test/ssl/t/001_ssltests.pl b/src/test/ssl/t/001_ssltests.pl
index 83fcd5e839..7931ebc067 100644
--- a/src/test/ssl/t/001_ssltests.pl
+++ b/src/test/ssl/t/001_ssltests.pl
@@ -13,7 +13,7 @@ use SSLServer;
if ($ENV{with_openssl} eq 'yes')
{
- plan tests => 84;
+ plan tests => 86;
}
else
{
@@ -97,6 +97,22 @@ command_ok(
'restart succeeds with password-protected key file');
$node->_update_pid(1);
+# Test compatibility of SSL protocols.
+# TLSv1.1 is lower than TLSv1.2, so it won't work.
+$node->append_conf('postgresql.conf',
+ qq{ssl_min_protocol_version='TLSv1.2'
Re: Improve errors when setting incorrect bounds for SSL protocols
> On 14 Jan 2020, at 04:54, Michael Paquier wrote: > > Hi all, > (Daniel G. in CC.) > > As discussed on the thread to be able to set the min/max SSL protocols > with libpq, when mixing incorrect bounds the user experience is not > that good: > https://www.postgresql.org/message-id/9cfa34ee-f670-419d-b92c-cb7943a27...@yesql.se > > It happens that the error generated with incorrect combinations > depends solely on what OpenSSL thinks is fine, and that's the > following: > psql: error: could not connect to server: SSL error: tlsv1 alert > internal error > > It is hard for users to understand what such an error means and how to > act on it. Correct, it's an easy mistake to make but based on the error it might take some time to figure it out. > Please note that OpenSSL 1.1.0 has added two routines to be able to > get the min/max protocols set in a context, called > SSL_CTX_get_min/max_proto_version. Thinking about older versions of > OpenSSL I think that it is better to use > ssl_protocol_version_to_openssl to do the parsing work. I also found > that it is easier to check for compatible versions after setting both > bounds in the SSL context, so as there is no need to worry about > invalid values depending on the build of OpenSSL used. I'm not convinced that it's a good idea to check for incompatible protocol range in the OpenSSL backend. We've spent a lot of energy to make the TLS code library agnostic and pluggable, and since identifying a basic configuration error isn't OpenSSL specific I think it should be in the guc code. That would keep the layering as well as ensure that we don't mistakenly treat this differently should we get a second TLS backend. cheers ./daniel
Improve errors when setting incorrect bounds for SSL protocols
Hi all,
(Daniel G. in CC.)
As discussed on the thread to be able to set the min/max SSL protocols
with libpq, when mixing incorrect bounds the user experience is not
that good:
https://www.postgresql.org/message-id/9cfa34ee-f670-419d-b92c-cb7943a27...@yesql.se
It happens that the error generated with incorrect combinations
depends solely on what OpenSSL thinks is fine, and that's the
following:
psql: error: could not connect to server: SSL error: tlsv1 alert
internal error
It is hard for users to understand what such an error means and how to
act on it.
Please note that OpenSSL 1.1.0 has added two routines to be able to
get the min/max protocols set in a context, called
SSL_CTX_get_min/max_proto_version. Thinking about older versions of
OpenSSL I think that it is better to use
ssl_protocol_version_to_openssl to do the parsing work. I also found
that it is easier to check for compatible versions after setting both
bounds in the SSL context, so as there is no need to worry about
invalid values depending on the build of OpenSSL used.
So attached is a patch to improve the detection of incorrect
combinations. Once applied, we get a complain about an incorrect
version at server startup (FATAL) or reload (LOG). The patch includes
new regression tests.
Thoughts?
--
Michael
diff --git a/src/backend/libpq/be-secure-openssl.c b/src/backend/libpq/be-secure-openssl.c
index 62f1fcab2b..75fdb2e91b 100644
--- a/src/backend/libpq/be-secure-openssl.c
+++ b/src/backend/libpq/be-secure-openssl.c
@@ -67,8 +67,7 @@ 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,
- int loglevel);
+static int ssl_protocol_version_to_openssl(int v);
#ifndef SSL_CTX_set_min_proto_version
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);
@@ -84,6 +83,8 @@ be_tls_init(bool isServerStart)
{
STACK_OF(X509_NAME) *root_cert_list = NULL;
SSL_CTX*context;
+ int ssl_ver_min = -1;
+ int ssl_ver_max = -1;
/* This stuff need be done only once. */
if (!SSL_initialized)
@@ -192,13 +193,19 @@ be_tls_init(bool isServerStart)
if (ssl_min_protocol_version)
{
- int ssl_ver = ssl_protocol_version_to_openssl(ssl_min_protocol_version,
- "ssl_min_protocol_version",
- isServerStart ? FATAL : LOG);
+ ssl_ver_min = ssl_protocol_version_to_openssl(ssl_min_protocol_version);
- if (ssl_ver == -1)
+ if (ssl_ver_min == -1)
+ {
+ ereport(isServerStart ? FATAL : LOG,
+ (errmsg("%s setting %s not supported by this build",
+ "ssl_min_protocol_version",
+ GetConfigOption("ssl_min_protocol_version",
+ false, false;
goto error;
- if (!SSL_CTX_set_min_proto_version(context, ssl_ver))
+ }
+
+ if (!SSL_CTX_set_min_proto_version(context, ssl_ver_min))
{
ereport(isServerStart ? FATAL : LOG,
(errmsg("could not set minimum SSL protocol version")));
@@ -208,13 +215,19 @@ be_tls_init(bool isServerStart)
if (ssl_max_protocol_version)
{
- int ssl_ver = ssl_protocol_version_to_openssl(ssl_max_protocol_version,
- "ssl_max_protocol_version",
- isServerStart ? FATAL : LOG);
+ ssl_ver_max = ssl_protocol_version_to_openssl(ssl_max_protocol_version);
- if (ssl_ver == -1)
+ if (ssl_ver_max == -1)
+ {
+ ereport(isServerStart ? FATAL : LOG,
+ (errmsg("%s setting %s not supported by this build",
+ "ssl_max_protocol_version",
+ GetConfigOption("ssl_max_protocol_version",
+ false, false;
goto error;
- if (!SSL_CTX_set_max_proto_version(context, ssl_ver))
+ }
+
+ if (!SSL_CTX_set_max_proto_version(context, ssl_ver_max))
{
ereport(isServerStart ? FATAL : LOG,
(errmsg("could not set maximum SSL protocol version")));
@@ -222,6 +235,20 @@ be_tls_init(bool isServerStart)
}
}
+ /* Check compatibility of min/max protocols */
+ if (ssl_min_protocol_version &&
+ ssl_max_protocol_version)
+ {
+ /*
+ * No need to check for invalid values (-1) for each protocol number
+ * as the code above would have already generated an error.
+ */
+ if (ssl_ver_min > ssl_ver_max)
+ ereport(isServerStart ? FATAL : LOG,
+ (errmsg("incompatible min/max SSL protocol versions set")));
+ goto error;
+ }
+
/* disallow SSL session tickets */
SSL_CTX_set_options(context, SSL_OP_NO_TICKET);
@@ -1275,12 +1302,11 @@ X509_NAME_to_cstring(X509_NAME *name)
* guc.c independent of OpenSSL availability and version.
*
* If a version is passed that is not supported by the current OpenSSL
- * version, then we log with the given loglevel and return (if we return) -1.
- * If a nonnegative value is returned, subsequent code can assume it's working
- * with a supported version.
+ * version, then we return -1. If a nonnegative value is returned,
+ *
