rdelaage
/
postgresql
mirror of https://git.postgresql.org/git/postgresql.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Add bound checks for ssl_min_protocol_version and ssl_max_protocol_version 

Mixing incorrect bounds in the SSL context leads to confusing error
messages generated by OpenSSL which are hard to act on.  New range
checks are added when both min/max parameters are loaded in the context
of a SSL reload to improve the error reporting.  Note that this does not
make use of the GUC hook machinery contrary to 41aadee, as there is no
way to ensure a consistent range check (except if there is a way one day
to define range types for GUC parameters?).  Hence, this patch applies
only to OpenSSL, and uses a logic similar to other parameters to trigger
an error when reloading the SSL context in a session.

Author: Michael Paquier
Reviewed-by: Daniel Gustafsson
Discussion: https://postgr.es/m/20200114035420.GE1515@paquier.xyz
This commit is contained in:
Michael Paquier 2020-03-23 11:01:41 +09:00
parent de9396326e
commit 79dfa8afb2
2 changed files with 62 additions and 21 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

65
src/backend/libpq/be-secure-openssl.c
View File

@ -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, false))));
return -1;
}

18
src/test/ssl/t/001_ssltests.pl
View File

@ -13,7 +13,7 @@ use SSLServer;
if ($ENV{with_openssl} eq 'yes')
{
plan tests => 91;
plan tests => 93;
}
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'
ssl_max_protocol_version='TLSv1.1'});
command_fails(
[ 'pg_ctl', '-D', $node->data_dir, '-l', $node->logfile, 'restart' ],
'restart fails with incorrect SSL protocol bounds');
# Go back to the defaults, this works.
$node->append_conf('postgresql.conf',
qq{ssl_min_protocol_version='TLSv1.2'
ssl_max_protocol_version=''});
command_ok(
[ 'pg_ctl', '-D', $node->data_dir, '-l', $node->logfile, 'restart' ],
'restart succeeds with correct SSL protocol bounds');
### Run client-side tests.
###
### Test that libpq accepts/rejects the connection correctly, depending