Re: [PATCH] Clarify that ssl_groups is for any key exchange groups
Hackorum builds and tests every patch posted to the lists, not only commitfest submissions. This is Hackorum's own CI rather than the PostgreSQL project's, and it is still under testing - please report anything that looks wrong.
You can run a PostgreSQL built from this patch straight from Docker, with no checkout and no build:
docker run --rm -p 5432:5432 ghcr.io/hackorum-dev/postgres-patch:t139835psql -h localhost -U postgresBuilt from patchset v1 (message #1), August 23, 2026 at 12:57 PM.
Every patchset is also pushed to a branch of our PostgreSQL fork, so you can check out the same tree CI built. Without a PostgreSQL checkout:
git clone --branch t139835_1 https://github.com/hackorum-dev/postgres.gitIn a checkout you already have, add the fork once:
git remote add hackorum https://github.com/hackorum-dev/postgres.gitthen, for this patchset and every later one:
git fetch hackorum t139835_1 && git checkout t139835_1Patchset v1 (message #1) is on t139835_1
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.
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
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.)