Skip to content

Commit 52134b4

Browse files
author
Tomas Urbonaitis
committed
Refactoring certificate loading into different methods, when loading from file and from store. Adding filtering and ordering, when loading from store
1 parent 0e635cc commit 52134b4

4 files changed

Lines changed: 116 additions & 32 deletions

File tree

‎GVFS/GVFS.Common/Git/GitSsl.cs‎

Lines changed: 77 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,20 @@
11
using System;
22
using System.Collections.Generic;
3-
using System.IO;
3+
using System.IO;
44
using System.Linq;
5-
using System.Security.Cryptography;
6-
using System.Security.Cryptography.X509Certificates;
7-
using GVFS.Common.Tracing;
8-
5+
using System.Security.Cryptography;
6+
using System.Security.Cryptography.X509Certificates;
7+
using System.Text.RegularExpressions;
8+
using GVFS.Common.Tracing;
9+
910
namespace GVFS.Common.Git
1011
{
1112
public class GitSsl : IDisposable
1213
{
1314
public readonly string CertificatePathOrSubjectCommonName;
1415
public readonly bool IsCertificatePasswordProtected;
15-
public readonly bool ShouldVerify;
16-
16+
public readonly bool ShouldVerify;
17+
1718
private readonly Lazy<X509Store> store = new Lazy<X509Store>(() =>
1819
{
1920
X509Store s = new X509Store();
@@ -47,8 +48,8 @@ public GitSsl(IDictionary<string, GitConfigSetting> configSettings) : this()
4748
this.ShouldVerify = sslVerify.Values.Select(bool.Parse).Single();
4849
}
4950
}
50-
}
51-
51+
}
52+
5253
public string GetCertificatePassword(ITracer tracer, GitProcess git)
5354
{
5455
if (git.TryGetCertificatePassword(tracer, this.CertificatePathOrSubjectCommonName, out string password, out string error))
@@ -63,11 +64,25 @@ public X509Certificate2 GetCertificate(ITracer tracer, string certificatePasswor
6364
{
6465
EventMetadata metadata = new EventMetadata
6566
{
66-
{ "certId", this.CertificatePathOrSubjectCommonName },
67+
{ "CertificatePathOrSubjectCommonName", this.CertificatePathOrSubjectCommonName },
6768
{ "isPasswordSpecified", string.IsNullOrEmpty(certificatePassword) },
6869
{ "shouldVerify", onlyLoadValidCertificateFromStore }
6970
};
7071

72+
var result =
73+
this.GetCertificateFromFile(tracer, metadata, certificatePassword, onlyLoadValidCertificateFromStore) ??
74+
this.GetCertificateFromStore(tracer, metadata, onlyLoadValidCertificateFromStore);
75+
76+
if (result == null)
77+
{
78+
tracer.RelatedError("Certificate {0} not found", this.CertificatePathOrSubjectCommonName);
79+
}
80+
81+
return result;
82+
}
83+
84+
private X509Certificate2 GetCertificateFromFile(ITracer tracer, EventMetadata metadata, string certificatePassword, bool onlyLoadValidCertificateFromStore)
85+
{
7186
if (File.Exists(this.CertificatePathOrSubjectCommonName))
7287
{
7388
try
@@ -89,12 +104,29 @@ public X509Certificate2 GetCertificate(ITracer tracer, string certificatePasswor
89104
}
90105
}
91106

107+
return null;
108+
}
109+
110+
private X509Certificate2 GetCertificateFromStore(ITracer tracer, EventMetadata metadata, bool onlyLoadValidCertificateFromStore)
111+
{
92112
try
93113
{
94114
X509Certificate2Collection findResults = this.store.Value.Certificates.Find(X509FindType.FindBySubjectName, this.CertificatePathOrSubjectCommonName, onlyLoadValidCertificateFromStore);
95115
if (findResults?.Count > 0)
96116
{
97-
return findResults[0];
117+
LogCertificateCounts(tracer, metadata, findResults.OfType<X509Certificate2>(), "Found {0} certificates by provided name. Matching DNs: {1}");
118+
119+
X509Certificate2[] certsWithMatchingCns = findResults
120+
.OfType<X509Certificate2>()
121+
.Where(x => x.HasPrivateKey && Regex.IsMatch(x.Subject, string.Format("(^|,\\s?)CN={0}(,|$)", this.CertificatePathOrSubjectCommonName))) // We only want certificates, that have private keys, as we need them. We also want a complete CN match
122+
.OrderByDescending(x => x.Verify()) // Ordering by validity in a descending order will bring valid certificates to the beginning
123+
.ThenBy(x => x.NotBefore) // We take the one, that was issued earliest, first
124+
.ThenByDescending(x => x.NotAfter) // We then take the one, that is valid for the longest period
125+
.ToArray();
126+
127+
LogCertificateCounts(tracer, metadata, certsWithMatchingCns, "Found {0} certificates with a private key and an exact CN match. DNs (sorted by priority, will take first): {1}");
128+
129+
return certsWithMatchingCns.FirstOrDefault();
98130
}
99131
}
100132
catch (CryptographicException cryptEx)
@@ -104,16 +136,43 @@ public X509Certificate2 GetCertificate(ITracer tracer, string certificatePasswor
104136
return null;
105137
}
106138

107-
tracer.RelatedError("Certificate {0} not found", this.CertificatePathOrSubjectCommonName);
108139
return null;
109-
}
110-
111-
public void Dispose()
112-
{
140+
}
141+
142+
private static void LogCertificateCounts(ITracer tracer, EventMetadata metadata, IEnumerable<X509Certificate2> certificates, string messageTemplate)
143+
{
144+
Action<EventMetadata, string> loggingFunction;
145+
int numberOfCertificates = certificates.Count();
146+
147+
switch (numberOfCertificates)
148+
{
149+
case 0:
150+
loggingFunction = tracer.RelatedError;
151+
break;
152+
case 1:
153+
loggingFunction = tracer.RelatedInfo;
154+
break;
155+
default:
156+
loggingFunction = tracer.RelatedWarning;
157+
break;
158+
}
159+
160+
loggingFunction(
161+
metadata,
162+
string.Format(
163+
messageTemplate,
164+
numberOfCertificates,
165+
string.Join(
166+
Environment.NewLine,
167+
certificates.Select(x => x.Subject))));
168+
}
169+
170+
public void Dispose()
171+
{
113172
if (this.store.IsValueCreated)
114173
{
115174
this.store.Value.Dispose();
116-
}
117-
}
175+
}
176+
}
118177
}
119178
}

‎GVFS/GVFS.Common/Tracing/ITracer.cs‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,16 +13,20 @@ public interface ITracer : IDisposable
1313

1414
void RelatedEvent(EventLevel level, string eventName, EventMetadata metadata, Keywords keywords);
1515

16+
void RelatedInfo(string message);
17+
1618
void RelatedInfo(string format, params object[] args);
17-
19+
20+
void RelatedInfo(EventMetadata metadata, string message);
21+
1822
void RelatedWarning(EventMetadata metadata, string message);
1923

2024
void RelatedWarning(EventMetadata metadata, string message, Keywords keywords);
2125

2226
void RelatedWarning(string message);
2327

24-
void RelatedWarning(string format, params object[] args);
25-
28+
void RelatedWarning(string format, params object[] args);
29+
2630
void RelatedError(EventMetadata metadata, string message);
2731

2832
void RelatedError(EventMetadata metadata, string message, Keywords keywords);

‎GVFS/GVFS.Common/Tracing/JsonTracer.cs‎

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -120,11 +120,21 @@ public virtual void RelatedEvent(EventLevel level, string eventName, EventMetada
120120

121121
public virtual void RelatedInfo(string format, params object[] args)
122122
{
123-
EventMetadata metadata = new EventMetadata();
124-
metadata.Add(TracingConstants.MessageKey.InfoMessage, string.Format(format, args));
125-
this.RelatedEvent(EventLevel.Informational, "Information", metadata);
123+
this.RelatedInfo(string.Format(format, args));
124+
}
125+
126+
public virtual void RelatedInfo(string message)
127+
{
128+
this.RelatedInfo(new EventMetadata(), message);
129+
}
130+
131+
public virtual void RelatedInfo(EventMetadata metadata, string message)
132+
{
133+
metadata = metadata ?? new EventMetadata();
134+
metadata.Add(TracingConstants.MessageKey.InfoMessage, message);
135+
this.RelatedEvent(EventLevel.Informational, "Information", metadata);
126136
}
127-
137+
128138
public virtual void RelatedWarning(EventMetadata metadata, string message)
129139
{
130140
this.RelatedWarning(metadata, message, Keywords.None);
@@ -139,7 +149,7 @@ public virtual void RelatedWarning(EventMetadata metadata, string message, Keywo
139149

140150
public virtual void RelatedWarning(string message)
141151
{
142-
EventMetadata metadata = new EventMetadata();
152+
EventMetadata metadata = new EventMetadata();
143153
this.RelatedWarning(metadata, message);
144154
}
145155

‎GVFS/GVFS.UnitTests/Mock/Common/MockTracer.cs‎

Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -45,11 +45,22 @@ public void RelatedEvent(EventLevel error, string eventName, EventMetadata metad
4545
}
4646
}
4747

48-
public void RelatedInfo(string format, params object[] args)
48+
public void RelatedInfo(string message)
4949
{
50-
this.RelatedInfoEvents.Add(string.Format(format, args));
50+
this.RelatedInfoEvents.Add(message);
51+
}
52+
53+
public void RelatedInfo(EventMetadata metadata, string message)
54+
{
55+
metadata[TracingConstants.MessageKey.InfoMessage] = message;
56+
this.RelatedWarningEvents.Add(JsonConvert.SerializeObject(metadata));
5157
}
52-
58+
59+
public void RelatedInfo(string format, params object[] args)
60+
{
61+
this.RelatedInfo(string.Format(format, args));
62+
}
63+
5364
public void RelatedWarning(EventMetadata metadata, string message)
5465
{
5566
metadata[TracingConstants.MessageKey.WarningMessage] = message;
@@ -69,8 +80,8 @@ public void RelatedWarning(string message)
6980
public void RelatedWarning(string format, params object[] args)
7081
{
7182
this.RelatedWarningEvents.Add(string.Format(format, args));
72-
}
73-
83+
}
84+
7485
public void RelatedError(EventMetadata metadata, string message)
7586
{
7687
metadata[TracingConstants.MessageKey.ErrorMessage] = message;
@@ -128,6 +139,6 @@ protected void Dispose(bool disposing)
128139
this.waitEvent = null;
129140
}
130141
}
131-
}
142+
}
132143
}
133144
}

0 commit comments

Comments
 (0)