[CSHARP-1914] Certificate Private Keys being dropped Created: 10/Feb/17  Updated: 07/Mar/17  Resolved: 07/Mar/17

Status: Closed
Project: C# Driver
Component/s: Connectivity, Security
Affects Version/s: 2.3
Fix Version/s: 2.4.3

Type: Bug Priority: Major - P3
Reporter: Alex Dawes Assignee: Robert Stam
Resolution: Done Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Backwards Compatibility: Fully Compatible

 Description   

The use of certificates for authentication appears to be currently broken.

We recently updated from 2.2.3 to 2.4.1, after which our clients saw connection issues against all clusters using certificates.

Specifically, on the client side we encountered timeouts of the form:

{{System.TimeoutException: A timeout occured after 30000ms selecting a server using CompositeServerSelector{ Selectors = ReadPreferenceServerSelector{ ReadPreference =

{ Mode : Primary }

}, LatencyLimitingServerSelector

{ AllowedLatencyRange = 00:00:00.0150000 }

}. Client view of cluster state is { ClusterId : "1", ConnectionMode : "Automatic", Type : "Unknown", State : "Disconnected", Servers : [{ ServerId: "

{ ClusterId : 1, EndPoint : "Unspecified/wint-dev-vm102:27017" }

", EndPoint: "Unspecified/wint-dev-vm102:27017", State: "Disconnected", Type: "Unknown", HeartbeatException: "MongoDB.Driver.MongoConnectionException: An exception occurred while opening a connection to the server. ---> System.InvalidOperationException: Invalid BinaryConnection state transition from 2 to Failed.}}

On the server side:

{{[initandlisten] connection accepted from 10.105.0.229:53377 #673429 (425 connections now open)
[conn673429] no SSL certificate provided by peer; connection rejected
[conn673429] end connection 10.105.0.229:53377 (424 connections now open)}}

Having looked into it, it appears that since 2.3.0 the private key is dropped on all X509Certificate2 certificates. This occurs in the SslSettings.CloneCertificate method at https://github.com/mongodb/mongo-csharp-driver/blob/master/src/MongoDB.Driver/SslSettings.cs#L260 and can be replicated using the following program:

{{using System;
using System.Security.Cryptography.X509Certificates;

namespace CertificateTest
{
public class Program
{
public static void Main(string[] args)
{
string certificate = @"C:/Certificates/testcert.pfx";
//string certificate = @"C:\Certificates\testcert.pfx";
string passphrase = "password";

var cert = new X509Certificate2(certificate, passphrase, X509KeyStorageFlags.Exportable);

Console.WriteLine($"cert.HasPrivateKey:

{cert.HasPrivateKey}

");
Console.WriteLine();
Console.WriteLine($"new X509Certificate2(cert.Export(X509ContentType.Cert)).HasPrivateKey:

{new X509Certificate2(cert.Export(X509ContentType.Cert)).HasPrivateKey}

");
Console.WriteLine($"new X509Certificate2(cert.RawData).HasPrivateKey:

{new X509Certificate2(cert.RawData).HasPrivateKey}

");
Console.WriteLine();
Console.WriteLine($"new X509Certificate2(cert.Handle).HasPrivateKey:

{new X509Certificate2(cert.Handle).HasPrivateKey}

");
Console.WriteLine($"new X509Certificate2(cert.Export(X509ContentType.Pfx)).HasPrivateKey:

{new X509Certificate2(cert.Export(X509ContentType.Pfx)).HasPrivateKey}

");
Console.WriteLine($"new X509Certificate2(cert.Export(X509ContentType.Pkcs12)).HasPrivateKey:

{new X509Certificate2(cert.Export(X509ContentType.Pkcs12)).HasPrivateKey}

");

Console.ReadLine();
}
}
}}}

Output:
{{cert.HasPrivateKey: True

new X509Certificate2(cert.Export(X509ContentType.Cert)).HasPrivateKey: False
new X509Certificate2(cert.RawData).HasPrivateKey: False

new X509Certificate2(cert.Handle).HasPrivateKey: True
new X509Certificate2(cert.Export(X509ContentType.Pfx)).HasPrivateKey: True
new X509Certificate2(cert.Export(X509ContentType.Pkcs12)).HasPrivateKey: True}}



 Comments   
Comment by Githook User [ 07/Mar/17 ]

Author:

{u'username': u'rstam', u'name': u'rstam', u'email': u'robert@robertstam.org'}

Message: CSHARP-1914: Added Alex Dawes to the contributor list.
Branch: master
https://github.com/mongodb/mongo-csharp-driver/commit/90698a173a07bbbc8b4efc8d39d66d708390b645

Comment by Githook User [ 07/Mar/17 ]

Author:

{u'name': u'Alexander Dawes', u'email': u'a.dawes@winton.com'}

Message: CSHARP-1914 Remove cloning of the certificates, as this was problematic as the private key is not included in the RawData of a cert and a cert cannot always be cloned in a deep manner (if it is non exportable)
Branch: master
https://github.com/mongodb/mongo-csharp-driver/commit/53383f27ded6cccd8a0f631b9736d2a7422f9ec2

Comment by Alex Dawes [ 07/Mar/17 ]

Created PR: https://github.com/mongodb/mongo-csharp-driver/pull/271

Comment by Robert Stam [ 07/Mar/17 ]

Go ahead and submit a pull request that removes cloning the X509 certificates.

Thanks.

Comment by Alex Dawes [ 07/Mar/17 ]

As another nail in the coffin against trying to perform a shallow clone, I have been unable to get this to work on a Linux machine. Works fine on Windows, but I could not get the driver to create a successful connection after cloning using the IntPtr handle.

Comment by Alex Dawes [ 07/Mar/17 ]

Just testing my changes on a Linux machine now (works fine on Windows). I've tried two separate solutions, firstly not cloning and secondly doing a shallow clone using the X509Certificate2(IntPtr) constructor (which is closer to the original functionality using X509Certificate2(X509Certificate2) constructor which did only a shallow clone anyway). Im happy to put in a PR for either option - whichever you would prefer..

Comment by Robert Stam [ 07/Mar/17 ]

I'm starting to think that the best solution might be to not clone the certificates.

While the intention is good (i.e. preserve the immutability of the SslSettings class), if we can't be sure that we can clone the certificates reliably it is better not to.

Better for SslSettings to have partial mutability than for it to have reliability problems.

This would then become a documentation task: we would document that once a certificate has been attached to an SslSettings instance the application should not modify that certificate in any way.

Comment by Alex Dawes [ 07/Mar/17 ]

Apologies - been distracted the last week but I intend to get this fixed today. Looks like the X509Certificate2(X509Certificate2) constructor originally used only performs a shallow clone anyway, so I am going to simply remove it.

var origCert = new X509Certificate2(@"J:\GIT\GITHUB\mongo-csharp-driver\tests\MongoDB.Driver.Tests\testcert.pfx", "password");
origCert.FriendlyName = "Foo";

var clone1 = new X509Certificate2(origCert);
var clone2 = new X509Certificate2(origCert.Handle);

Console.WriteLine(origCert.FriendlyName);
Console.WriteLine(clone1.FriendlyName);
Console.WriteLine(clone2.FriendlyName);

origCert.FriendlyName = "Bar";

Console.WriteLine(origCert.FriendlyName);
Console.WriteLine(clone1.FriendlyName);
Console.WriteLine(clone2.FriendlyName);

Console.ReadLine();

Comment by Alex Dawes [ 10/Feb/17 ]

Possibly you could do the following:

try

{ //If the original certificate is not exportable, calling export will fail. return new X509Certificate2(certificate2.Export(X509ContentType.Pfx), (string)null, X509KeyStorageFlags.Exportable); }

catch

{ return certificate2; }

This isnt very nice though, as you are not always cloning the certificate (if it is not exportable and cant be cloned, then the returned cert is not a copy).

Comment by Alex Dawes [ 10/Feb/17 ]

Unfortunately the above isn't true - the handle references the entire certificate in memory, so the clone has the same underlying memory for all properties:

var cert = new X509Certificate2(certificate, passphrase, X509KeyStorageFlags.Exportable);
cert.FriendlyName = "foo";
var clone = new X509Certificate2(cert.Handle);
Console.WriteLine($"cert.FriendlyName:

{cert.FriendlyName}, clone.FriendlyName: {clone.FriendlyName}");
cert.FriendlyName = "bar";
Console.WriteLine($"cert.FriendlyName: {cert.FriendlyName}

, clone.FriendlyName:

{clone.FriendlyName}

");
Console.ReadLine();

Output:
cert.FriendlyName: foo, clone.FriendlyName: foo
cert.FriendlyName: bar, clone.FriendlyName: bar

Comment by Alex Dawes [ 10/Feb/17 ]

I dont know enough about X509Certificate2, but there is a X509Certificate2(IntPtr) constructor. I wonder if the memory pointed to by cert.Handle is the raw data (incl private key) and other mutable properties are then stored elsewhere. If so, then new X509Certificate2(cert.Handle) might provide a way to get a clone with the same private key but..

Comment by Robert Stam [ 10/Feb/17 ]

Cloning the certificates is not strictly necessary.

The reason they are being cloned is because SslSettings is intended to be frozen once it has been fully initialized, but X509Certificate2 is mutable. So to protect itself from external code mutating the certificate the SslSettings class only hands out clones of the certificate.

If a certificates cannot be successfully cloned we may have to drop our freeze protection a bit.

Comment by Alex Dawes [ 10/Feb/17 ]

Before I submit a fix for this, can you please explain why cloning the certificates is necessary? Note that using new X509Certificate2(cert.Export(X509ContentType.Pfx)) to clone the certificate is not a perfect fix as if the certificate is not marked as exportable this will break. Not sure if this will also affect X509Certificate usage.

Generated at Wed Feb 07 21:41:03 UTC 2024 using Jira 9.7.1#970001-sha1:2222b88b221c4928ef0de3161136cc90c8356a66.