public inbox for [email protected]  

help / color / mirror / Atom feed

From: Chao Li <[email protected]>
To: Daniel Gustafsson <[email protected]>
Cc: Michael Paquier <[email protected]>
Cc: Andres Freund <[email protected]>
Cc: Jacob Champion <[email protected]>
Cc: Pgsql Hackers <[email protected]>
Subject: Re: Serverside SNI support in libpq
Date: Tue, 11 Nov 2025 17:06:49 +0800
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>
References: <[email protected]>
	<CAOYmi+nYV6Rr9BY4YfYyVdiQ5TzMZray6QPXwiO3pYSaow+-Tg@mail.gmail.com>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<CAOYmi+mSrV8hRaQkvGDf1Df4cmpv5SeTbTxppyxeonMe6MW8nA@mail.gmail.com>
	<[email protected]>
	<aa7gx3mychf3m2g67mbslzbxjy3if4enpcflstoa5pol3432x5@ugqz45gsvurq>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>

Hi Daniel,

I just reviewed the patch and got a few comments:

> On Nov 11, 2025, at 06:32, Daniel Gustafsson <[email protected]> wrote:
> 
> Attached is a cleaned up rebase with improved memory handling, additional code
> documentation, removed passphrase test (sent as a separate thread), and some
> general cleanup and additional testing.
> 
> --
> Daniel Gustafsson
> 
> <v9-0001-Serverside-SNI-support-for-libpq.patch>

1 - commit message
```
Experimental support for serverside SNI support in libpq, a new config
file $datadir/pg_hosts.conf is used for configuring which certicate and
```

Typo: certicate -> certificate

2 - be-secure-common.c
```
+run_ssl_passphrase_command(const char *prompt, bool is_server_start, char *buf, int size, void *userdata)
 {
 	int			loglevel = is_server_start ? ERROR : LOG;
 	char	   *command;
 	FILE	   *fh;
 	int			pclose_rc;
 	size_t		len = 0;
+	char	   *cmd = (char *) userdata;
```

Cmd is only passed to replace_percent_placeholders(), and the function take a "const char *” argument, so we can define cmd as “const char *”.

2 - be-secure-common.c
```
+	tokenize_auth_file(HostsFileName, file, &hosts_lines, LOG, 0);
+
+	foreach(line, hosts_lines)
+	{
+		TokenizedAuthLine *tok_line = (TokenizedAuthLine *) lfirst(line);
+
+		if (tok_line->err_msg != NULL)
+		{
+			ok = false;
+			continue;
+		}
+
+		if ((newline = parse_hosts_line(tok_line, LOG)) == NULL)
+		{
+			ok = false;
+			continue;
+		}
+
+		parsed_lines = lappend(parsed_lines, newline);
+	}
+
+	free_auth_file(file, 0);
```

When I read this function, I thought to raise a comment for freeing hosts_lines. However, then I read be-secure-openssl.c, I saw the load_hosts() is called within a transient hostctx, so it doesn’t have to free memory pieces. Can we explain that in the function comment? Otherwise other reviewers and future code readers may have the same confusion.

3 - be-secure-openssl.c
```
 int
@@ -759,6 +933,9 @@ be_tls_close(Port *port)
 		pfree(port->peer_dn);
 		port->peer_dn = NULL;
 	}
+
+	Host_context = NULL;
+	SSL_context = NULL;
 }
```

Do we need to free_contexts() here? When be_tls_init() is called again, contexts will anyway be freed, so I guess earlier free might be better?

4 - guc_parameters.dat
```
+{ name => 'hosts_file', type => 'string', context => 'PGC_POSTMASTER', group => 'FILE_LOCATIONS',
+  short_desc => 'Sets the server\'s "hosts" configuration file.',
+  flags => 'GUC_SUPERUSER_ONLY',
+  variable => 'HostsFileName',
+  boot_val => 'NULL',
+},

+{ name => 'ssl_snimode', type => 'enum', context => 'PGC_SIGHUP', group => 'CONN_AUTH_SSL',
+  short_desc => 'Sets the SNI mode to use for the server.',
+  flags => 'GUC_SUPERUSER_ONLY',
+  variable => 'ssl_snimode',
+  boot_val => 'SSL_SNIMODE_DEFAULT',
+  options => 'ssl_snimode_options',
+},
```
 
If ssl_snimode is PGC_SIGHUP that allows to reload without a server reset, why hosts_file cannot?

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/

view thread (58+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
  Subject: Re: Serverside SNI support in libpq
  In-Reply-To: <[email protected]>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox