Re: [PATCH v2] Add ssl_alt_cert_file/ssl_alt_key_file for dual RSA+ECDSA certificate support

+ 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.