From f33722c129cab1af5b9ac0931563d24ab92251d3 Mon Sep 17 00:00:00 2001 From: Daniel Gustafsson Date: Tue, 22 Sep 2026 23:35:07 +0200 Subject: [PATCH v3] 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 storing the ssl_sni setting at the time of initialization in the hosts context and only use the stored value during handshakes. 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 Reviewed-by: Zsolt Parragi Discussion: https://postgr.es/m/20260920211619.9c.noahmisch@microsoft.com Backpatch-through: 19 --- src/backend/libpq/be-secure-openssl.c | 30 ++++++++++-- src/test/ssl/t/004_sni.pl | 70 +++++++++++++++++++++++++-- src/test/ssl/t/SSL/Server.pm | 4 ++ 3 files changed, 97 insertions(+), 7 deletions(-) diff --git a/src/backend/libpq/be-secure-openssl.c b/src/backend/libpq/be-secure-openssl.c index 173623d1f6a..72e77a305dd 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,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 is still %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; @@ -640,7 +660,7 @@ init_host_context(HostsLine *host, bool isServerStart, bool *hasWarned) * * If SNI is enabled, we set password callback based what was configured. */ - if (!ssl_sni) + if ((SSL_hosts && !SSL_hosts->sni_enabled) || !ssl_sni) (*openssl_tls_init_hook) (ctx, isServerStart); else { @@ -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; } diff --git a/src/test/ssl/t/004_sni.pl b/src/test/ssl/t/004_sni.pl index f1a135bb3c8..22160f89633 100644 --- a/src/test/ssl/t/004_sni.pl +++ b/src/test/ssl/t/004_sni.pl @@ -263,6 +263,71 @@ $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); +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'); +$result = $bsession->query_safe('SHOW ssl_sni'); +is($result, 'off', 'SNI is reported as off in background session'); +$node_loglocation = -s $node->logfile; + +# EXEC_BACKEND backends loads 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', 3 + 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/); +} + +# Reset configuration for the next test +$node->append_conf( + 'postgresql.conf', qq{ +ssl_sni = on +ssl_cert_file = '' +}); +$node->reload; +$node->wait_for_log(qr/reloading configuration files/); + # Reconfigure with broken configuration for the key passphrase, the server # should not start up ok(unlink($node->data_dir . '/pg_hosts.conf')); @@ -309,13 +374,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)