[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) 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; namespace CertificateTest var cert = new X509Certificate2(certificate, passphrase, X509KeyStorageFlags.Exportable); Console.WriteLine($"cert.HasPrivateKey: {cert.HasPrivateKey}"); "); "); "); "); "); Console.ReadLine(); Output: new X509Certificate2(cert.Export(X509ContentType.Cert)).HasPrivateKey: False new X509Certificate2(cert.Handle).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: |
| Comment by Githook User [ 07/Mar/17 ] |
|
Author: {u'name': u'Alexander Dawes', u'email': u'a.dawes@winton.com'}Message: |
| 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"); var clone1 = new X509Certificate2(origCert); Console.WriteLine(origCert.FriendlyName); origCert.FriendlyName = "Bar"; Console.WriteLine(origCert.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 = "bar"; Console.WriteLine($"cert.FriendlyName: {cert.FriendlyName} , clone.FriendlyName: {clone.FriendlyName}"); Output: |
| 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. |