Skip to content

Add support for multiple client certificates #6162

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
vonzshik merged 2 commits into from
Oct 5, 2025
Merged

Add support for multiple client certificates #6162

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
97b9c4b
Add support for multiple client certificates
vonzshik Jul 11, 2025
66da7b8
Allow sending multiple client certificated with separate key and pass…
vonzshik Jul 16, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
Add support for multiple client certificates
  • Loading branch information
@vonzshik
vonzshik committed Sep 10, 2025
commit 97b9c4b88f884ea77e15ad6b41a5810cb93a7dd2
64 changes: 42 additions & 22 deletions src/Npgsql/Internal/NpgsqlConnector.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -283,7 +283,7 @@ internal bool PostgresCancellationPerformed
#pragma warning disable CA1859
// We're casting to IDisposable to not explicitly reference X509Certificate2 for NativeAOT
// TODO: probably pointless now, needs to be rechecked
IDisposable? _certificate;
List<IDisposable>? _certificates;
#pragma warning restore CA1859

internal NpgsqlLoggingConfiguration LoggingConfiguration { get; }
Expand Down Expand Up @@ -1063,36 +1063,59 @@ internal async Task NegotiateEncryption(SslMode sslMode, NpgsqlTimeout timeout,
{
var password = Settings.SslPassword;

X509Certificate2? cert = null;
if (!string.Equals(Path.GetExtension(certPath), ".pfx", StringComparison.OrdinalIgnoreCase))
{
// It's PEM time
var keyPath = Settings.SslKey ?? PostgresEnvironment.SslKey ?? PostgresEnvironment.SslKeyDefault;
cert = string.IsNullOrEmpty(password)
? X509Certificate2.CreateFromPemFile(certPath, keyPath)
: X509Certificate2.CreateFromEncryptedPemFile(certPath, password, keyPath);
if (string.IsNullOrEmpty(password) && string.IsNullOrEmpty(keyPath))
{
// If there is no password or key provided, most likely there are multiple certificates
// Where one is leaf and others are intermediate
clientCertificates.ImportFromPemFile(certPath);
}
else
{
var cert = string.IsNullOrEmpty(password)
? X509Certificate2.CreateFromPemFile(certPath, keyPath)
Copy link
Copy Markdown
Member

@roji roji Jul 15, 2025

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
Copy Markdown
Contributor Author

@vonzshik vonzshik Jul 15, 2025

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
Copy Markdown
Contributor Author

@vonzshik vonzshik Jul 16, 2025

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
Copy Markdown
Member

@roji roji Oct 5, 2025

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.

: X509Certificate2.CreateFromEncryptedPemFile(certPath, password, keyPath);
clientCertificates.Add(cert);
}

if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows))
{
// Windows crypto API has a bug with pem certs
// See #3650
using var previousCert = cert;
for (var i = 0; i < clientCertificates.Count; i++)
{
var cert = clientCertificates[i];

// Windows crypto API has a bug with pem certs
// See #3650
using var previousCert = cert;
#if NET9_0_OR_GREATER
cert = X509CertificateLoader.LoadPkcs12(cert.Export(X509ContentType.Pkcs12), null);
cert = X509CertificateLoader.LoadPkcs12(cert.Export(X509ContentType.Pkcs12), null);
#else
cert = new X509Certificate2(cert.Export(X509ContentType.Pkcs12));
cert = new X509Certificate2(cert.Export(X509ContentType.Pkcs12));
#endif
clientCertificates[i] = cert;
}
}
}

// If it's empty, it's probably PFX
if (clientCertificates.Count == 0)
{
#if NET9_0_OR_GREATER
// If it's null, it's probably PFX
cert ??= X509CertificateLoader.LoadPkcs12FromFile(certPath, password);
var certs = X509CertificateLoader.LoadPkcs12CollectionFromFile(certPath, password);
clientCertificates.AddRange(certs);
#else
cert ??= new X509Certificate2(certPath, password);
var cert = new X509Certificate2(certPath, password);
clientCertificates.Add(cert);
#endif
clientCertificates.Add(cert);
}

_certificate = cert;
var certificates = new List<IDisposable>();
foreach (var certificate in clientCertificates)
certificates.Add(certificate);
_certificates = certificates;
}

try
Expand Down Expand Up @@ -1181,8 +1204,8 @@ internal async Task NegotiateEncryption(SslMode sslMode, NpgsqlTimeout timeout,
}
catch
{
_certificate?.Dispose();
_certificate = null;
_certificates?.ForEach(x => x.Dispose());
_certificates = null;

throw;
}
Expand Down Expand Up @@ -2522,11 +2545,8 @@ void Cleanup()
PostgresParameters.Clear();
_currentCommand = null;

if (_certificate is not null)
{
_certificate.Dispose();
_certificate = null;
}
_certificates?.ForEach(x => x.Dispose());
_certificates = null;
}

void GenerateResetMessage()
Expand Down