public inbox for [email protected]
help / color / mirror / Atom feedFrom: Si, Evan <[email protected]>
To: Ewan Young <[email protected]>
Cc: [email protected] <[email protected]>
Subject: Re: [PATCH] Clarify that ssl_groups is for any key exchange groups
Date: Wed, 3 Jun 2026 17:29:30 +0000
Message-ID: <[email protected]> (raw)
On 6/2/26, 11:32 PM, "Ewan Young" <[email protected] <mailto:[email protected]>> wrote:
>
> +1 for the idea. (I'm fairly new here, so please take my comments with
> a grain of salt.)
Thanks for the review!
> 1. The comment just above the renamed call in be_tls_init() still
> says "set up ephemeral DH and ECDH keys". Maybe it should be
> updated to match?
Right, that makes sense. I did a larger grep and updated comments where I found stale references to curves and (EC)DH.
> 2. The SSLECDHCurve variable (and its "GUC variable for default ECDH
> curve" comment in be-secure.c) still uses the old naming. I wasn't
> sure if that was left out intentionally to keep the patch small --
> if not, would it make sense to rename it too, for consistency with
> the initialize_groups() rename?
This also seems reasonable. I didn't find usage of this extern outside of Postgres itself in the wild from a brief search.
Attached a revision.
Evan
Attachments:
[application/octet-stream] v2-0001-Clarify-that-ssl_groups-is-for-any-key-exchange-g.patch (6.7K, 2-v2-0001-Clarify-that-ssl_groups-is-for-any-key-exchange-g.patch)
download | inline diff:
From 090fca6246c166170d6b5ab819a4680712745124 Mon Sep 17 00:00:00 2001
From: Evan Si <[email protected]>
Date: Mon, 1 Jun 2026 18:13:35 +0000
Subject: [PATCH v2] Clarify that ssl_groups is for any key exchange groups
The current wording seems to suggest that the parameter is only meant for DH.
This introduces minor wording tweaks across comments, short_desc, and the docs to
reflect that it accepts any group.
---
doc/src/sgml/config.sgml | 12 ++++++------
src/backend/libpq/be-secure-openssl.c | 16 ++++++++--------
src/backend/libpq/be-secure.c | 4 ++--
src/backend/utils/misc/guc_parameters.dat | 4 ++--
src/include/libpq/libpq.h | 2 +-
src/test/ssl/t/SSL/Server.pm | 2 +-
6 files changed, 20 insertions(+), 20 deletions(-)
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index cebae4b6d1b..8a94ecd6221 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -1573,11 +1573,11 @@ include_dir 'conf.d'
</term>
<listitem>
<para>
- Specifies the name of the curve to use in <acronym>ECDH</acronym> key
- exchange. It needs to be supported by all clients that connect.
- Multiple curves can be specified by using a colon-separated list.
- It does not need to be the same curve used by the server's Elliptic
- Curve key. This parameter can only be set in the
+ Specifies the named group to use for <acronym>TLS</acronym> key
+ exchange. It needs to be supported by all clients that
+ connect. Multiple groups can be specified by using a colon-separated
+ list. It does not need to match the key type used by the server
+ certificate. This parameter can only be set in the
<filename>postgresql.conf</filename> file or on the server command line.
The default is <literal>X25519:prime256v1</literal>.
</para>
@@ -1592,7 +1592,7 @@ include_dir 'conf.d'
</note>
<para>
- <productname>OpenSSL</productname> names for the most common curves
+ <productname>OpenSSL</productname> names for the most common groups
are:
<literal>prime256v1</literal> (NIST P-256),
<literal>secp384r1</literal> (NIST P-384),
diff --git a/src/backend/libpq/be-secure-openssl.c b/src/backend/libpq/be-secure-openssl.c
index 7890e6c2de2..6172726bfbc 100644
--- a/src/backend/libpq/be-secure-openssl.c
+++ b/src/backend/libpq/be-secure-openssl.c
@@ -95,7 +95,7 @@ static int alpn_cb(SSL *ssl,
unsigned int inlen,
void *userdata);
static bool initialize_dh(SSL_CTX *context, bool isServerStart);
-static bool initialize_ecdh(SSL_CTX *context, bool isServerStart);
+static bool initialize_groups(SSL_CTX *context, bool isServerStart);
static const char *SSLerrmessageExt(unsigned long ecode, const char *replacement);
static const char *SSLerrmessage(unsigned long ecode);
static bool init_host_context(HostsLine *host, bool isServerStart);
@@ -513,10 +513,10 @@ be_tls_init(bool isServerStart)
SSL_CTX_set_options(context, SSL_OP_NO_CLIENT_RENEGOTIATION);
#endif
- /* set up ephemeral DH and ECDH keys */
+ /* set up DH parameters and TLS named groups */
if (!initialize_dh(context, isServerStart))
goto error;
- if (!initialize_ecdh(context, isServerStart))
+ if (!initialize_groups(context, isServerStart))
goto error;
/* set up the allowed cipher list for TLSv1.2 and below */
@@ -2106,14 +2106,14 @@ initialize_dh(SSL_CTX *context, bool isServerStart)
}
/*
- * Set ECDH parameters for generating ephemeral Elliptic Curve DH
- * keys. This is much simpler than the DH parameters, as we just
- * need to provide the name of the curve to OpenSSL.
+ * Set the group(s) to use for TLS key exchange. This is much simpler
+ * than the static DH parameters, as we just need to provide the
+ * colon-separated list of group names to OpenSSL.
*/
static bool
-initialize_ecdh(SSL_CTX *context, bool isServerStart)
+initialize_groups(SSL_CTX *context, bool isServerStart)
{
- if (SSL_CTX_set1_groups_list(context, SSLECDHCurve) != 1)
+ if (SSL_CTX_set1_groups_list(context, SSLNamedGroups) != 1)
{
/*
* OpenSSL 3.3.0 introduced proper error messages for group parsing
diff --git a/src/backend/libpq/be-secure.c b/src/backend/libpq/be-secure.c
index 86ceea72e64..d69106e6545 100644
--- a/src/backend/libpq/be-secure.c
+++ b/src/backend/libpq/be-secure.c
@@ -52,8 +52,8 @@ bool ssl_loaded_verify_locations = false;
char *SSLCipherSuites = NULL;
char *SSLCipherList = NULL;
-/* GUC variable for default ECDH curve. */
-char *SSLECDHCurve;
+/* GUC variable for the named groups to use for TLS key exchange. */
+char *SSLNamedGroups;
/* GUC variable: if false, prefer client ciphers */
bool SSLPreferServerCiphers;
diff --git a/src/backend/utils/misc/guc_parameters.dat b/src/backend/utils/misc/guc_parameters.dat
index afaa058b046..e5e01ce1cd6 100644
--- a/src/backend/utils/misc/guc_parameters.dat
+++ b/src/backend/utils/misc/guc_parameters.dat
@@ -2790,10 +2790,10 @@
},
{ name => 'ssl_groups', type => 'string', context => 'PGC_SIGHUP', group => 'CONN_AUTH_SSL',
- short_desc => 'Sets the group(s) to use for Diffie-Hellman key exchange.',
+ short_desc => 'Sets the named groups to use for TLS key exchange.',
long_desc => 'Multiple groups can be specified using a colon-separated list.',
flags => 'GUC_SUPERUSER_ONLY',
- variable => 'SSLECDHCurve',
+ variable => 'SSLNamedGroups',
boot_val => 'DEFAULT_SSL_GROUPS',
},
diff --git a/src/include/libpq/libpq.h b/src/include/libpq/libpq.h
index d15073a0a93..906ac742b83 100644
--- a/src/include/libpq/libpq.h
+++ b/src/include/libpq/libpq.h
@@ -116,7 +116,7 @@ extern PGDLLIMPORT char *ssl_dh_params_file;
extern PGDLLIMPORT bool ssl_sni;
extern PGDLLIMPORT char *SSLCipherSuites;
extern PGDLLIMPORT char *SSLCipherList;
-extern PGDLLIMPORT char *SSLECDHCurve;
+extern PGDLLIMPORT char *SSLNamedGroups;
extern PGDLLIMPORT bool SSLPreferServerCiphers;
#ifdef USE_SSL
extern PGDLLIMPORT bool ssl_loaded_verify_locations;
diff --git a/src/test/ssl/t/SSL/Server.pm b/src/test/ssl/t/SSL/Server.pm
index 4400a432f42..624d36ff7ea 100644
--- a/src/test/ssl/t/SSL/Server.pm
+++ b/src/test/ssl/t/SSL/Server.pm
@@ -322,7 +322,7 @@ sub switch_server_cert
ok(unlink($node->data_dir . '/sslconfig.conf'));
$node->append_conf('sslconfig.conf', 'ssl=on');
$node->append_conf('sslconfig.conf', $backend->set_server_cert(\%params));
- # use lists of ECDH curves and cipher suites for syntax testing
+ # use lists of TLS groups and cipher suites for syntax testing
$node->append_conf('sslconfig.conf', 'ssl_groups=prime256v1:secp521r1');
$node->append_conf('sslconfig.conf',
'ssl_tls13_ciphers=TLS_AES_256_GCM_SHA384:TLS_AES_128_GCM_SHA256');
--
2.47.3
view thread (6+ 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]
Subject: Re: [PATCH] Clarify that ssl_groups is for any key exchange groups
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