Skip to content

Add support for multiple client certificates#6162

Merged
vonzshik merged 2 commits intomainfrom
6152-support-multiple-client-certs
Oct 5, 2025
Merged

Add support for multiple client certificates#6162
vonzshik merged 2 commits intomainfrom
6152-support-multiple-client-certs

Conversation

@vonzshik
Copy link
Contributor

Fixes #6152

@vonzshik vonzshik requested a review from roji as a code owner July 11, 2025 11:02
Copy link
Member

@roji roji left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, just see question about .NET limitation below.

else
{
var cert = string.IsNullOrEmpty(password)
? X509Certificate2.CreateFromPemFile(certPath, keyPath)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So there's no way to read multiple certificates (as above with X509Certificate2Collection.ImportFromPemFile()) if the PEM file happens to be encrypted with a password or key? While with PKCS12 it's possible (below we do X509CertificateLoader.LoadPkcs12CollectionFromFile)?

Is this a weird/arbitrary .NET limitation, is it tracked somewhere?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems to be intentional dotnet/runtime#31944 (comment)

To quote:

Having the collection import support key matching requires a whole lot more work and a whole lot of failure cases, and I'd really like to avoid having a simple-looking method that requires pages and pages of documentation of what it does with unmatched keys, what happens if the same cert is specified twice and a key matches it, are multiple keys even supported, et cetera, at such a low level.

I'm still investigating a few things in regards to how pg actually behaves so there might be some other changes in this pr.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, it's a mess.

A bit of context: you can send certificates from a client to the server which then can validate them either for auth or just in general. That's so called client certificates.
One requirement for them is that client certificate has to have private key. Just having a certificate with public key would not do.
The basic validation pg can do is that that certificate is signed by some other trusted certificate (CA). Here there is no issue, we already support this from our side.
The problem is when there are multiple certificates in between the trusted (CA) and the client (leaf). Let's say we have a CA->intermediate->leaf chain. PG only has CA while we have both intermediate and leaf certificates. In this case we have to send both of them, as otherwise PG can't establish the whole chain and rejects our connection. That's the original issue this pr is fixing.
Now, here's the fun part.

  1. There is no explicit API in .NET to load the first certificate from the chain with key/password and other with only public keys. You either load them all together with whatever they had in that file, or you somehow parse that file and load each certificate separately. I've managed to work around this thanks to the quirk X509Certificate2.CreateFromPemFile has: it loads only the first certificate, ignoring everything else. This way we can load the first certificate with both key and password and then load all other certificates via X509Certificate2Collection.ImportFromPemFile. This still makes it so we have a duplicate of the first certificate, but it's easy to remove it since we know it's the first one.
  2. SslClientAuthenticationOptions has a property ClientCertificates which we're using to send client certificates. The problem is, it doesn't work for intermediate certificates. If they're not trusted or they don't have a private key, SslStream seems to completely ignore them, making the chain incomplete. From my understanding, that's due to the way SChannel works (and this behavior was copied to other platforms). Which means that we have to use SslStreamCertificateContext.Create and explicitly specify which certificate is leaf and which are additional. The main issue is that SslStreamCertificateContext doesn't exist for .NET Framework, so that means we can't backport that fix to npgsql 8, but I'm personally OK with leaving it to 9 and 10.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is no explicit API in .NET to load the first certificate from the chain with key/password and other with only public keys.

OK. I just want to confirm that the thing we're supporting here (loading first certificate with key/password, the rest of the certificates with public key only) is "standard" - or just supported by libpq (if libpq supports it ideally so should we). If so, seems quite smelly that we need to hack around like this in .NET rather than having a more 1st-class API (is it worth raising the problem more explicitly?).

Otherwise at this point doing something that can't be backported to netfx seems completely fine to me.

@vonzshik vonzshik force-pushed the 6152-support-multiple-client-certs branch from 7342196 to 66da7b8 Compare September 10, 2025 11:38
@vonzshik vonzshik merged commit 5d073da into main Oct 5, 2025
16 checks passed
@vonzshik vonzshik deleted the 6152-support-multiple-client-certs branch October 5, 2025 18:45
vonzshik added a commit that referenced this pull request Oct 5, 2025
@vonzshik
Copy link
Contributor Author

vonzshik commented Oct 5, 2025

Backported to 9.0.4 via 5d073da (but should work only when targeting .NET 8 or higher due to API limitations)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bug: PGSSLCERT does not pass full certificate chain for client authentication

3 participants