Re: [PATCH v3] Add ssl_cert_files/ssl_key_files for multi-certificate support

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

In response to

Responses

Browse pgsql-hackers by date

  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