| From: | Renaud Métrich <rmetrich(at)redhat(dot)com> |
|---|---|
| To: | pgsql-hackers(at)lists(dot)postgresql(dot)org |
| Subject: | Re: [PATCH v3] Add ssl_cert_files/ssl_key_files for multi-certificate support |
| Date: | 2026-06-22 14:53:09 |
| Message-ID: | c0d91918-b02a-42e2-9b70-a30c152379d6@redhat.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
Hi,
Here is v3, redesigned based on Zsolt's v2 feedback and the proxy
workaround discussion with Jacob.
The main change: the ssl_alt_cert_file/ssl_alt_key_file approach is
dropped entirely. Instead, v3 introduces two new list-valued GUCs:
ssl_cert_files = 'server-rsa.crt, server-ecdsa.crt'
ssl_key_files = 'server-rsa.key, server-ecdsa.key'
This follows the same pattern as unix_socket_directories and
shared_preload_libraries (GUC_LIST_INPUT | GUC_LIST_QUOTE). When set,
ssl_cert_files takes precedence over ssl_cert_file. Each entry is
paired positionally with the corresponding entry in ssl_key_files.
Certificates are loaded via SSL_CTX_use_certificate_chain_file() (full
chain) and OpenSSL handles key-type matching internally — no same-type
detection needed, no ordering validation, and natural support for all
three key types (RSA, ECDSA, EdDSA).
Addressing each v2 review point:
1+4) Same-type detection / EVP_PKEY_get_base_id — dropped entirely.
With ssl_cert_files, OpenSSL handles duplicate key types internally
(last wins), so no application-level detection is needed. This
also eliminates the fragile SSL_CTX_set_current_cert(NEXT) logic.
2) TLS 1.3 HRR test — added a proper test that forces HelloRetryRequest
by setting ssl_groups='secp384r1' on the server and connecting with
-groups X25519:secp384r1. The ssl_update_ssl() fix (override=1
always) is carried over from v2.
3) Test in meson.build — the new test is t/005_ssl_multi_cert.pl,
added to both Makefile and src/test/ssl/meson.build.
5) SNI limitation — documented that ssl_cert_files applies to the
default SSL configuration from postgresql.conf only. Per-host
support in pg_hosts.conf is left as future work per Zsolt's
suggestion.
6) Multi-valued GUC design — implemented as new ssl_cert_files /
ssl_key_files GUCs rather than making ssl_cert_file list-valued,
avoiding any dump/restore implications. Added to
variable_is_guc_list_quote() in dumputils.c. Verified with
pg_dumpall: the GUCs are PGC_SIGHUP context so they never appear
in dumps (only in postgresql.conf / postgresql.auto.conf).
The observability functions (ssl_server_cert_type/ssl_server_cert_types
via sslinfo extension) that were in v1/v2 have been split out for a
separate submission, to keep this patch focused on the core multi-cert
loading.
Test results: 30 subtests in 005_ssl_multi_cert.pl covering dual cert
negotiation (TLS 1.2 + 1.3), HelloRetryRequest, mismatched list
lengths, missing cert/key files, bad certificate path, cert/key type
mismatch, single cert regression, ssl_cert_files precedence over
ssl_cert_file, and SIGHUP reload add/remove. Full SSL suite passes
with no regressions. RHEL 9.8 / OpenSSL 3.5.5. LibreSSL fallback
paths verified via #undef SSL_CERT_SET_FIRST build.
10 files changed, 522 insertions(+), 11 deletions(-)
Thanks,
Renaud Métrich
Red Hat
Le 16/06/2026 à 2:16 PM, Renaud Métrich a écrit :
> Hi Zsolt,
>
> Thanks for the thorough review, I will take a look at all this.
>
> Best regards,
>
> Renaud Métrich
> Red Hat
>
> Le 15/06/2026 à 11:13 PM, Zsolt Parragi a écrit :
>> + primary_type =
>> EVP_PKEY_get_base_id(X509_get0_pubkey(primary_cert));
>> + alt_type =
>> EVP_PKEY_get_base_id(X509_get0_pubkey(alt_cert));
>> +
>>
>> Isn't EVP_PKEY_get_base_id also a function introduced by OpenSSL 3.0?
>>
>> -# Test 5: Verify ssl_server_cert_type() returns correct type per
>> connection
>> +# Test 5: Verify TLS 1.3 connection works (exercises
>> HelloRetryRequest path)
>> +note "testing TLS 1.3 connection with dual certs";
>> +$node->connect_ok(
>> + "$common_connstr sslcert=invalid",
>> + "connect via TLS 1.3 with dual certs (default negotiation)",
>> + sql => "SELECT 1");
>> +
>> +# Test 6: Verify ssl_server_cert_type() returns correct type per
>> connection
>>
>> I don't think this properly tests HelloRetryRequest, as there's no
>> key-share group mismatch in the testcase.
>>
>> Also, the testcase file should be included in
>> src/test/ssl/meson.build, currently it is only executed by make.
>>
>> +
>> + /*
>> + * Verify that the alternate certificate uses a different
>> key type
>> + * than the primary. If both are the same type (e.g. both
>> RSA),
>> + * the alternate silently replaces the primary, which is not
>> useful.
>> + */
>> + {
>> + X509 *primary_cert;
>> + X509 *alt_cert;
>> + int primary_type;
>> + int alt_type;
>> +
>> + SSL_CTX_set_current_cert(ctx, SSL_CERT_SET_FIRST);
>> + primary_cert = SSL_CTX_get0_certificate(ctx);
>> +
>> + SSL_CTX_set_current_cert(ctx, SSL_CERT_SET_NEXT);
>> + alt_cert = SSL_CTX_get0_certificate(ctx);
>> +
>> + if (primary_cert && alt_cert)
>> + {
>> + primary_type =
>> EVP_PKEY_get_base_id(X509_get0_pubkey(primary_cert));
>> + alt_type =
>> EVP_PKEY_get_base_id(X509_get0_pubkey(alt_cert));
>> +
>> + if (primary_type == alt_type)
>> + {
>> + ereport(isServerStart ? FATAL : LOG,
>> + (errcode(ERRCODE_CONFIG_FILE_ERROR),
>> + errmsg("alternate certificate has the
>> same key type (%s) as
>> the primary certificate",
>> + evp_pkey_type_name(alt_type))));
>> + goto error;
>> + }
>> + }
>> + }
>>
>> Are you sure about this code? The comment says "the alternate silently
>> replaces the primary, which is not useful." - which was also my
>> observation, but replacing means that there's no alt certificate
>> really. I think this check works by accident because if there's no
>> NEXT certificate, OpenSSL returns the first certificate again, but
>> this behavior is undocumented.
>>
>>> 3) Per-host SNI support: documented that ssl_alt_cert_file applies only
>>> to the default SSL configuration from postgresql.conf.
>> I don't think documenting the limitation is a good approach for this
>> feature, it should be supported uniformly everywhere. The question of
>> how it could fit into pg_hosts is a different question, so I am not
>> suggesting that you should simply add a few more columns there, but I
>> would at least keep it as a TODO item. The format/extensibility of
>> hosts and other configuration files is a bigger question I want to
>> start a discussion about soon, and this could fit there.
>>
>>> For PostgreSQL, since GUCs are strings, this could take the form of
>>> comma-separated paths:
>>>
>>> ssl_cert_file = 'server-rsa.crt, server-ecdsa.crt'
>>> ssl_key_file = 'server-rsa.key, server-ecdsa.key'
>> List style GUCs already exist (for example unix_socket_directories),
>> so there's a good precedent for this. This could also fit into
>> pg_hosts, where the host part already accepts a list of hosts.
>>
>> Also I'm not sure if changing the single string GUC to a list could
>> cause any issues with dump/restore. I didn't check this in detail, but
>> there's a comment about this in
>> src/backend/utils/misc/guc_parameters.dat and dumputils.c.
>>
>> Another option would be to add new GUCs ending with _files, and make
>> them mutually exclusive?
>>
>>> and let OpenSSL sort out the type matching — no same-type detection
>>> needed, no "alt" naming, and natural support for all three key types.
>> I'm quite sure we would still have to do some validation, but that's a
>> minor detail.
>>
>>
| Attachment | Content-Type | Size |
|---|---|---|
| v3-0001-Add-ssl_cert_files-ssl_key_files-for-multi-certifica.patch | text/x-patch | 27.2 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Marcos Pegoraro | 2026-06-22 14:54:38 | Re: [PATCH] Add pg_get_table_ddl() to reconstruct CREATE TABLE statements |
| Previous Message | Akshay Joshi | 2026-06-22 14:11:18 | Re: [PATCH] Add pg_get_table_ddl() to reconstruct CREATE TABLE statements |