Re: [PATCH] Clarify that ssl_groups is for any key exchange groups

On Thu, Jun 4, 2026 at 1:29 AM Si, Evan <evsi@amazon.com> wrote:

On 6/2/26, 11:32 PM, "Ewan Young" <kdbase.hack@gmail.com <mailto:kdbase.hack@gmail.com>> 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.

Thanks! I re-did the grep on v2 and found no remaining stale references.

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

I tested v2 on top of current master:
- applies cleanly, builds without warnings (--with-openssl)
- src/test/ssl TAP suite passes

v2 looks good to me, and I have nothing further.

Best regards,
Ewan Young

On 4 Jun 2026, at 05:00, Ewan Young <kdbase.hack@gmail.com> wrote:

v2 looks good to me, and I have nothing further.

I have applied the docs portion of this patch to master and REL_18_STABLE, but
given where we are in the release cycle I don't think the renames qualify as
fixes during a freeze. Please feel free to register this patch in the upcoming
commitfest to get the remaining portion into v20. You can add me as a reviewer
on that entry to make sure it stays on my radar.

--
Daniel Gustafsson

On 6/5/2026 1:22 PM, Daniel Gustafsson wrote:

On 4 Jun 2026, at 05:00, Ewan Young <kdbase.hack@gmail.com> wrote:

v2 looks good to me, and I have nothing further.

I have applied the docs portion of this patch to master and REL_18_STABLE, but
given where we are in the release cycle I don't think the renames qualify as
fixes during a freeze. Please feel free to register this patch in the upcoming
commitfest to get the remaining portion into v20. You can add me as a reviewer
on that entry to make sure it stays on my radar.

Thanks! It looks like you found the commitfest entry for it right after
the email.

Otherwise for reference: https://commitfest.postgresql.org/patch/6843/

(Same person, different email.)