From 21383dd7d5ac8f067fffc1feda345ac8f08750a8 Mon Sep 17 00:00:00 2001 From: Daniel Gustafsson Date: Fri, 2 Oct 2026 09:28:42 +0200 Subject: [PATCH v5 1/5] 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. Author: Anthropic OSS program Co-authored-by: Daniel Gustafsson Reported-by: Anthropic OSS program Reviewed-by: Zsolt Parragi Reviewed-by: Jacob Champion Discussion: https://postgr.es/m/20260920211619.9c.noahmisch@microsoft.com Backpatch-through: 19 --- src/backend/libpq/be-secure-openssl.c | 26 ++++++++- src/test/ssl/t/004_sni.pl | 80 ++++++++++++++++++++++++++- src/test/ssl/t/SSL/Server.pm | 4 ++ 3 files changed, 105 insertions(+), 5 deletions(-) diff --git a/src/backend/libpq/be-secure-openssl.c b/src/backend/libpq/be-secure-openssl.c index 173623d1f6a..c95e2ec62c2 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 @@ -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; } diff --git a/src/test/ssl/t/004_sni.pl b/src/test/ssl/t/004_sni.pl index f1a135bb3c8..46db8e94868 100644 --- a/src/test/ssl/t/004_sni.pl +++ b/src/test/ssl/t/004_sni.pl @@ -263,6 +263,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,13 +384,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); +my $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)