mirror of
https://github.com/postgres/postgres.git
synced 2026-10-23 13:29:09 +03:00
Keep current ssl_sni setting on SSL config reload failure
When reloading the SSL configuration fails, the existing config is left in place to keep the server from being inaccessible. The GUC backing SNI, ssl_sni, was however changed. This could lead to new connections trying to access non-existing configurations. Fix by storing the ssl_sni setting at the time of initialization in the hosts context and only use the stored value during the handshake. The patch submitted by the reporter has been significantly reworked and expanded. Additional test cases written by Rui Zhao. Author: Anthropic OSS program Co-authored-by: Daniel Gustafsson <daniel@yesql.se> Co-authored-by: Rui Zhao <zhaorui126@gmail.com> Reported-by: Anthropic OSS program Reviewed-by: Zsolt Parragi <zsolt.parragi@percona.com> Reviewed-by: Jacob Champion <jacob.champion@enterprisedb.com> Reviewed-by: Rui Zhao <zhaorui126@gmail.com> Discussion: https://postgr.es/m/20260920211619.9c.noahmisch@microsoft.com Backpatch-through: 19
This commit is contained in:
@@ -125,6 +125,11 @@ static struct hosts
|
||||
* matches the supplied hostname in the SNI extension.
|
||||
*/
|
||||
HostsLine *default_host;
|
||||
|
||||
/*
|
||||
* Whether the configuration was loaded with ssl_sni enabled.
|
||||
*/
|
||||
bool sni_enabled;
|
||||
} *SSL_hosts;
|
||||
|
||||
static bool dummy_ssl_passwd_cb_called = false;
|
||||
@@ -177,6 +182,7 @@ be_tls_init(bool isServerStart)
|
||||
|
||||
/* Allocate a tentative replacement for SSL_hosts. */
|
||||
new_hosts = palloc0_object(struct hosts);
|
||||
new_hosts->sni_enabled = ssl_sni;
|
||||
|
||||
/*
|
||||
* Register a reset callback for the memory context which is responsible
|
||||
@@ -565,15 +571,29 @@ be_tls_init(bool isServerStart)
|
||||
|
||||
return 0;
|
||||
|
||||
error:
|
||||
|
||||
/*
|
||||
* Clean up by releasing working SSL contexts as well as allocations
|
||||
* performed during parsing. Since all our allocations are done in a
|
||||
* local memory context all we need to do is delete it.
|
||||
*/
|
||||
error:
|
||||
if (context)
|
||||
SSL_CTX_free(context);
|
||||
|
||||
/*
|
||||
* If the initialization failed, and the ssl_sni setting was changed, we
|
||||
* issue a WARNING to indicate that the ssl_sni setting won't match the
|
||||
* SSL configuration in use.
|
||||
*/
|
||||
if (SSL_context && SSL_hosts && SSL_hosts->sni_enabled != ssl_sni)
|
||||
{
|
||||
ereport(WARNING,
|
||||
errcode(ERRCODE_CONFIG_FILE_ERROR),
|
||||
errmsg("SSL configuration not reloaded, SNI remains %s", SSL_hosts->sni_enabled ? "on" : "off"),
|
||||
errdetail("The SSL configuration failed to reload, previous configuration and SNI state will remain active."));
|
||||
}
|
||||
|
||||
MemoryContextSwitchTo(oldcxt);
|
||||
MemoryContextDelete(host_memcxt);
|
||||
return -1;
|
||||
@@ -1934,9 +1954,11 @@ sni_clienthello_cb(SSL *ssl, int *al, void *arg)
|
||||
len;
|
||||
HostsLine *install_config = NULL;
|
||||
|
||||
if (!ssl_sni)
|
||||
if (!SSL_hosts->sni_enabled)
|
||||
{
|
||||
/* A configuration loaded without SNI must have a default host */
|
||||
install_config = SSL_hosts->default_host;
|
||||
Assert(install_config != NULL);
|
||||
goto found;
|
||||
}
|
||||
|
||||
|
||||
@@ -58,6 +58,86 @@ $ssl_server->switch_server_cert($node, certfile => 'server-cn-only');
|
||||
my $connstr =
|
||||
"user=ssltestuser dbname=trustdb hostaddr=$SERVERHOSTADDR sslsni=1";
|
||||
|
||||
# Enable SNI with an encrypted host key and a per-host passphrase command.
|
||||
{
|
||||
$ssl_server->switch_server_cert(
|
||||
$node,
|
||||
certfile => 'server-ip-cn-only',
|
||||
passphrase_cmd => '');
|
||||
my $verify_connstr =
|
||||
"$connstr sslmode=verify-full sslrootcert=ssl/root+server_ca.crt sslcert=invalid sslkey=invalid sslcrl=invalid sslcrldir=invalid";
|
||||
$node->connect_ok(
|
||||
"$verify_connstr host=192.0.2.1 sslsni=0",
|
||||
'global certificate before enabling SNI');
|
||||
$node->append_conf('pg_hosts.conf',
|
||||
'common-name.pg-ssltest.test server-cn-only.crt server-password.key root+client_ca.crt "echo secret1" on'
|
||||
);
|
||||
$node->append_conf('postgresql.conf', 'ssl_sni = on');
|
||||
my $log_offset = -s $node->logfile;
|
||||
$node->reload;
|
||||
$node->wait_for_log(qr/reloading configuration files/, $log_offset);
|
||||
$node->connect_ok(
|
||||
"$verify_connstr host=common-name.pg-ssltest.test",
|
||||
'reload enables SNI with the per-host passphrase command');
|
||||
|
||||
ok(unlink($node->data_dir . '/pg_hosts.conf'));
|
||||
$node->append_conf('pg_hosts.conf', '');
|
||||
$node->append_conf('postgresql.conf', 'ssl_sni = off');
|
||||
$ssl_server->switch_server_cert($node, certfile => 'server-cn-only');
|
||||
}
|
||||
|
||||
# Check certificate selection after a failed reload and a successful retry.
|
||||
SKIP:
|
||||
{
|
||||
skip 'SSL context retention requires forked backends', 11
|
||||
if $exec_backend =~ /on/;
|
||||
|
||||
my $verify_connstr =
|
||||
"$connstr sslmode=verify-full sslrootcert=ssl/root+server_ca.crt sslcert=invalid sslkey=invalid sslcrl=invalid sslcrldir=invalid";
|
||||
$node->append_conf(
|
||||
'pg_hosts.conf', qq{
|
||||
* server-ip-cn-only.crt server-ip-cn-only.key
|
||||
common-name.pg-ssltest.test server-cn-only.crt server-cn-only.key
|
||||
});
|
||||
$node->append_conf('postgresql.conf', 'ssl_sni = on');
|
||||
my $log_offset = -s $node->logfile;
|
||||
$node->reload;
|
||||
$node->wait_for_log(qr/reloading configuration files/, $log_offset);
|
||||
$node->connect_ok(
|
||||
"$verify_connstr host=common-name.pg-ssltest.test",
|
||||
'loaded SNI mode selects the named host certificate');
|
||||
|
||||
$node->append_conf('postgresql.conf', 'ssl_sni = off');
|
||||
$node->append_conf('sslconfig.conf',
|
||||
"ssl_cert_file = 'missing-server.crt'");
|
||||
$log_offset = -s $node->logfile;
|
||||
$node->reload;
|
||||
$node->wait_for_log(qr/SSL configuration was not reloaded/, $log_offset);
|
||||
$node->connect_ok(
|
||||
"$verify_connstr host=common-name.pg-ssltest.test",
|
||||
'failed reload retains the named host certificate with a default host configured'
|
||||
);
|
||||
|
||||
$ssl_server->switch_server_cert(
|
||||
$node,
|
||||
certfile => 'server-ip-cn-only',
|
||||
restart => 'no');
|
||||
$log_offset = -s $node->logfile;
|
||||
$node->reload;
|
||||
$node->wait_for_log(qr/reloading configuration files/, $log_offset);
|
||||
$node->connect_fails(
|
||||
"$verify_connstr host=common-name.pg-ssltest.test",
|
||||
'successful retry disables SNI host selection',
|
||||
expected_stderr =>
|
||||
qr/server certificate for "192\.0\.2\.1" does not match host name/);
|
||||
$node->connect_ok("$verify_connstr host=192.0.2.1 sslsni=0",
|
||||
'successful retry serves the configured certificate');
|
||||
|
||||
ok(unlink($node->data_dir . '/pg_hosts.conf'));
|
||||
$node->append_conf('pg_hosts.conf', '');
|
||||
$ssl_server->switch_server_cert($node, certfile => 'server-cn-only');
|
||||
}
|
||||
|
||||
##############################################################################
|
||||
# postgresql.conf
|
||||
##############################################################################
|
||||
@@ -263,6 +343,81 @@ $node->connect_fails(
|
||||
"pg_hosts.conf: connect to 'example' with sslmode=require",
|
||||
expected_stderr => qr/unrecognized name/);
|
||||
|
||||
# Turn off SNI while the postgresql.conf configuration cannot be loaded, such
|
||||
# that the reload fails to replace the SSL configuration. The pg_hosts.conf
|
||||
# configuration without a default host must remain in effect, ssl_sni will be
|
||||
# set to 'off' but the previous config - including ssl_sni setting - is what
|
||||
# will be used.
|
||||
my $bsession = $node->background_psql('trustdb',
|
||||
connstr =>
|
||||
"$connstr host=example.org sslrootcert=ssl/root_ca.crt sslmode=verify-ca"
|
||||
);
|
||||
$result = $bsession->query_safe('SHOW ssl_sni');
|
||||
is($result, 'on', 'SNI is enabled in active config for background session');
|
||||
|
||||
$node->append_conf(
|
||||
'postgresql.conf', qq{
|
||||
ssl_sni = off
|
||||
ssl_cert_file = 'nonexistent.crt'
|
||||
});
|
||||
my $node_loglocation = -s $node->logfile;
|
||||
$node->reload;
|
||||
|
||||
$node->wait_for_log(qr/SSL configuration was not reloaded/,
|
||||
$node_loglocation);
|
||||
$node->log_check('SSL reload triggered WARNING on ssl_sni state',
|
||||
$node_loglocation, log_like => [qr/SNI remains on/]);
|
||||
|
||||
$result = $bsession->query_safe('SHOW ssl_sni');
|
||||
is($result, 'off', 'SNI is reported as off in background session');
|
||||
$bsession->quit;
|
||||
$node_loglocation = -s $node->logfile;
|
||||
|
||||
# EXEC_BACKEND backends load the SSL configuration on each connection instead
|
||||
# of reusing the contexts in the postmaster
|
||||
SKIP:
|
||||
{
|
||||
skip 'SSL config reload is per connection in EXEC_BACKEND', 4
|
||||
if ($windows_os || $exec_backend =~ /on/);
|
||||
|
||||
my ($rc, $stdout, $stderr) = $node->psql(
|
||||
'trustdb',
|
||||
qq[SHOW ssl_sni;],
|
||||
connstr =>
|
||||
"$connstr sslrootcert=ssl/root+server_ca.crt sslmode=require host=example.org"
|
||||
);
|
||||
is($rc, 0,
|
||||
"pg_hosts.conf: connect to example.org after failed reload with ssl_sni off"
|
||||
);
|
||||
is($stdout, 'off', 'ssl_sni is set to off in a new connection');
|
||||
|
||||
$node->connect_fails(
|
||||
"$connstr sslrootcert=ssl/root+server_ca.crt sslmode=require sslsni=0",
|
||||
"pg_hosts.conf: connect to default after failed reload with ssl_sni off",
|
||||
expected_stderr => qr/handshake failure/);
|
||||
}
|
||||
|
||||
# Specifically test EXEC_BACKEND where a failed reload by definition cannot
|
||||
# retain the previous configuration. Here the server should not accept any
|
||||
# connections.
|
||||
SKIP:
|
||||
{
|
||||
skip 'Only testing EXEC_BACKEND behavior', 2 if ($exec_backend !~ /on/);
|
||||
$node->connect_fails(
|
||||
"$connstr sslrootcert=ssl/root+server_ca.crt sslmode=require sslsni=0",
|
||||
"pg_hosts.conf: connect to default after failed reload with ssl_sni off",
|
||||
expected_stderr => qr/server does not support SSL/);
|
||||
}
|
||||
|
||||
# Reset configuration for the next test
|
||||
$node->append_conf(
|
||||
'postgresql.conf', qq{
|
||||
ssl_sni = on
|
||||
ssl_cert_file = ''
|
||||
});
|
||||
$node_loglocation = -s $node->logfile;
|
||||
$node->reload;
|
||||
|
||||
# Reconfigure with broken configuration for the key passphrase, the server
|
||||
# should not start up
|
||||
ok(unlink($node->data_dir . '/pg_hosts.conf'));
|
||||
@@ -309,7 +464,7 @@ ok(unlink($node->data_dir . '/pg_hosts.conf'));
|
||||
$node->append_conf('pg_hosts.conf',
|
||||
'localhost server-cn-only.crt server-password.key root+client_ca.crt "echo secret1" off'
|
||||
);
|
||||
my $node_loglocation = -s $node->logfile;
|
||||
$node_loglocation = -s $node->logfile;
|
||||
$result = $node->restart(fail_ok => 1);
|
||||
is($result, 1,
|
||||
'pg_hosts.conf: restart succeeds with password-protected key when using the correct passphrase command'
|
||||
|
||||
@@ -166,6 +166,10 @@ sub configure_test_server_for_ssl
|
||||
$node->psql('postgres', "CREATE DATABASE $db");
|
||||
}
|
||||
|
||||
# Grant pg_read_all_settings to ssltestuser so that relevant GUCs can be
|
||||
# examined during tests
|
||||
$node->psql('postgres', "GRANT pg_read_all_settings TO ssltestuser");
|
||||
|
||||
# Update password of each user as needed.
|
||||
if (defined($params{password}))
|
||||
{
|
||||
|
||||
Reference in New Issue
Block a user