From 2f4733993a967ce7f89d1a02c9f12988e9b7ff6f Mon Sep 17 00:00:00 2001 From: Michael Paquier Date: Fri, 7 Feb 2020 08:11:01 +0900 Subject: [PATCH] Revert "Add GUC checks for ssl_min_protocol_version and ssl_max_protocol_version" This reverts commit 41aadee, as the GUC checks could run on older values with the new values used, and result in incorrect errors if both parameters are changed at the same time. Per complaint from Tom Lane. Discussion: https://postgr.es/m/27574.1581015893@sss.pgh.pa.us Backpatch-through: 12 --- src/backend/utils/misc/guc.c | 51 ++-------------------------------- src/test/ssl/t/001_ssltests.pl | 20 +------------ src/test/ssl/t/SSLServer.pm | 2 +- 3 files changed, 4 insertions(+), 69 deletions(-) diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c index 00520fe43f..a70e79c489 100644 --- a/src/backend/utils/misc/guc.c +++ b/src/backend/utils/misc/guc.c @@ -201,10 +201,6 @@ static bool check_cluster_name(char **newval, void **extra, GucSource source); static const char *show_unix_socket_permissions(void); static const char *show_log_file_mode(void); static const char *show_data_directory_mode(void); -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); @@ -4526,7 +4522,7 @@ static struct config_enum ConfigureNamesEnum[] = &ssl_min_protocol_version, PG_TLS1_VERSION, ssl_protocol_versions_info + 1, /* don't allow PG_TLS_ANY */ - check_ssl_min_protocol_version, NULL, NULL + NULL, NULL, NULL }, { @@ -4538,7 +4534,7 @@ static struct config_enum ConfigureNamesEnum[] = &ssl_max_protocol_version, PG_TLS_ANY, ssl_protocol_versions_info, - check_ssl_max_protocol_version, NULL, NULL + NULL, NULL, NULL }, /* End-of-list marker */ @@ -11442,49 +11438,6 @@ show_data_directory_mode(void) return buf; } -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 66278381bd..67a3a28db6 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 => 77; + plan tests => 75; } else { @@ -87,24 +87,6 @@ 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' -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 diff --git a/src/test/ssl/t/SSLServer.pm b/src/test/ssl/t/SSLServer.pm index 228cddf3a2..d25c38dbbc 100644 --- a/src/test/ssl/t/SSLServer.pm +++ b/src/test/ssl/t/SSLServer.pm @@ -128,7 +128,7 @@ sub configure_test_server_for_ssl print $conf "log_statement=all\n"; # enable SSL and set up server key - print $conf "include 'sslconfig.conf'\n"; + print $conf "include 'sslconfig.conf'"; close $conf;