From 2c781346b1538eb845139b407805fdd5b87a0fd3 Mon Sep 17 00:00:00 2001 From: Daniel Gustafsson Date: Tue, 22 Sep 2026 11:08:40 +0200 Subject: [PATCH v2] 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 not changed which could lead to connections trying to access non-existing configurations. Fix by reverting the ssl_sni GUC to the setting from the currently active SSL configuration on reload failures. The patch submitted by the reporter has been significantly reworked and expanded. Author: Anthropic OSS program Co-authored-by: Daniel Gustafsson Reported-by: Anthropic OSS program Discussion: https://postgr.es/m/20260920211619.9c.noahmisch@microsoft.com Backpatch-through: 19 --- src/backend/libpq/be-secure-openssl.c | 29 +++++++++++++-- src/test/ssl/t/004_sni.pl | 51 +++++++++++++++++++++++++-- src/test/ssl/t/SSL/Server.pm | 4 +++ 3 files changed, 78 insertions(+), 6 deletions(-) diff --git a/src/backend/libpq/be-secure-openssl.c b/src/backend/libpq/be-secure-openssl.c index 173623d1f6a..63c1fedc8b8 100644 --- a/src/backend/libpq/be-secure-openssl.c +++ b/src/backend/libpq/be-secure-openssl.c @@ -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 @@ -204,7 +210,7 @@ be_tls_init(bool isServerStart) * we set res to the state and continue with a new conditional instead of * duplicating logic and risk it diverging over time. */ - if (ssl_sni) + if (new_hosts->sni_enabled) { /* * The GUC check hook should have already blocked this but to be on @@ -565,15 +571,30 @@ 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 + * need to revert ssl_sni back to the previous setting to match the SSL + * configuration left in place. Log a WARNING to alert the user. + */ + if (SSL_hosts->sni_enabled != ssl_sni) + { + ssl_sni = SSL_hosts->sni_enabled; + ereport(WARNING, + errcode(ERRCODE_CONFIG_FILE_ERROR), + errmsg("SSL configuration not reloaded, SNI is still %s", ssl_sni ? "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 +1955,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; } diff --git a/src/test/ssl/t/004_sni.pl b/src/test/ssl/t/004_sni.pl index f1a135bb3c8..2a6d56ad169 100644 --- a/src/test/ssl/t/004_sni.pl +++ b/src/test/ssl/t/004_sni.pl @@ -263,6 +263,52 @@ $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 +# reverted back to "on" and connections must behave as before the reload. +my $node_loglocation = -s $node->logfile; +$node->append_conf( + 'postgresql.conf', qq{ +ssl_sni = off +ssl_cert_file = 'nonexistent.crt' +}); +$node->reload; + +$node->wait_for_log(qr/SSL configuration was not reloaded/, + $node_loglocation); +my $log = + PostgreSQL::Test::Utils::slurp_file($node->logfile, $node_loglocation); +like( + $log, + qr/SNI is still on/, + 'SSL reload triggered WARNING on ssl_sni state'); +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, 'on', 'ssl_sni remains enabled'); +$node_loglocation = -s $node->logfile; + +$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/); + +$node->append_conf( + 'postgresql.conf', qq{ +ssl_sni = on +ssl_cert_file = 'server-cn-only.crt' +}); +$node->reload; +$log = PostgreSQL::Test::Utils::slurp_file($node->logfile, $node_loglocation); +unlike($log, qr/WARNING/, 'No WARNING on correct configuration'); + # Reconfigure with broken configuration for the key passphrase, the server # should not start up ok(unlink($node->data_dir . '/pg_hosts.conf')); @@ -309,13 +355,12 @@ 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' ); -my $log = - PostgreSQL::Test::Utils::slurp_file($node->logfile, $node_loglocation); +$log = PostgreSQL::Test::Utils::slurp_file($node->logfile, $node_loglocation); unlike( $log, qr/cannot be reloaded because it requires a passphrase/, diff --git a/src/test/ssl/t/SSL/Server.pm b/src/test/ssl/t/SSL/Server.pm index 4400a432f42..a7646cf135f 100644 --- a/src/test/ssl/t/SSL/Server.pm +++ b/src/test/ssl/t/SSL/Server.pm @@ -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})) { -- 2.39.3 (Apple Git-146)