Skip to content

Commit 4e38f0d

Browse files
author
Tomas Urbonaitis
committed
Refactoring GitSslSettings to be GitSsl and handle all the certificate loading. Extracting interaction with GitSsl from HttpRequestor to GitAuthentication
1 parent 38867ef commit 4e38f0d

5 files changed

Lines changed: 167 additions & 167 deletions

File tree

‎GVFS/GVFS.Common/Git/GitAuthentication.cs‎

Lines changed: 46 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,10 @@
11
using GVFS.Common.Http;
22
using GVFS.Common.Tracing;
33
using System;
4+
using System.Collections.Generic;
45
using System.Net;
6+
using System.Net.Http;
7+
using System.Security.Cryptography.X509Certificates;
58
using System.Text;
69

710
namespace GVFS.Common.Git
@@ -26,18 +29,16 @@ public GitAuthentication(GitProcess git, string repoUrl)
2629
this.git = git;
2730
this.repoUrl = repoUrl;
2831

29-
if (git.TryGetConfigUrlMatch("http", this.repoUrl, out var configSettings))
32+
if (git.TryGetConfigUrlMatch("http", this.repoUrl, out Dictionary<string, GitConfigSetting> configSettings))
3033
{
31-
this.GitSslSettings = new GitSslSettings(configSettings);
34+
this.GitSsl = new GitSsl(configSettings);
3235
}
3336
else
3437
{
35-
this.GitSslSettings = new GitSslSettings();
38+
this.GitSsl = new GitSsl();
3639
}
3740
}
3841

39-
public GitSslSettings GitSslSettings { get; }
40-
4142
public bool IsBackingOff
4243
{
4344
get
@@ -48,6 +49,8 @@ public bool IsBackingOff
4849

4950
public bool IsAnonymous { get; private set; } = true;
5051

52+
private GitSsl GitSsl { get; }
53+
5154
public void ConfirmCredentialsWorked(string usedCredential)
5255
{
5356
lock (this.gitAuthLock)
@@ -163,6 +166,44 @@ public bool TryInitializeAndRequireAuth(ITracer tracer, out string errorMessage)
163166
return false;
164167
}
165168

169+
public void SetupSslIfNeeded(ITracer tracer, HttpClientHandler httpClientHandler, GitProcess gitProcess)
170+
{
171+
if (!string.IsNullOrEmpty(this.GitSsl.SslCertificate))
172+
{
173+
string certificatePassword = null;
174+
if (this.GitSsl.SslCertPasswordProtected)
175+
{
176+
certificatePassword = this.GitSsl.GetCertificatePassword(tracer, gitProcess);
177+
178+
if (string.IsNullOrEmpty(certificatePassword))
179+
{
180+
tracer.RelatedWarning(
181+
new EventMetadata
182+
{
183+
{ "SslCertificate", this.GitSsl.SslCertificate }
184+
},
185+
"Git config indicates, that certificate is password protected, but retrieved password was null or empty!");
186+
}
187+
}
188+
189+
X509Certificate2 cert = this.GitSsl.LoadCertificate(tracer, certificatePassword, this.GitSsl.SslVerify);
190+
if (cert != null)
191+
{
192+
if (!this.GitSsl.SslVerify)
193+
{
194+
httpClientHandler.ServerCertificateCustomValidationCallback =
195+
(httpRequestMessage, c, cetChain, policyErrors) =>
196+
{
197+
return true;
198+
};
199+
}
200+
201+
httpClientHandler.ClientCertificateOptions = ClientCertificateOption.Manual;
202+
httpClientHandler.ClientCertificates.Add(cert);
203+
}
204+
}
205+
}
206+
166207
private bool TryAnonymousQuery(ITracer tracer, Enlistment enlistment, out bool isAnonymous)
167208
{
168209
bool querySucceeded;

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

Lines changed: 119 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,119 @@
1+
using System;
2+
using System.Collections.Generic;
3+
using System.IO;
4+
using System.Linq;
5+
using System.Security.Cryptography;
6+
using System.Security.Cryptography.X509Certificates;
7+
using GVFS.Common.Tracing;
8+
9+
namespace GVFS.Common.Git
10+
{
11+
public class GitSsl : IDisposable
12+
{
13+
public readonly string SslCertificate;
14+
public readonly bool SslCertPasswordProtected;
15+
public readonly bool SslVerify;
16+
17+
private readonly Lazy<X509Store> store = new Lazy<X509Store>(() =>
18+
{
19+
X509Store s = new X509Store();
20+
s.Open(OpenFlags.OpenExistingOnly | OpenFlags.ReadOnly);
21+
return s;
22+
});
23+
24+
public GitSsl()
25+
{
26+
this.SslCertificate = null;
27+
this.SslCertPasswordProtected = false;
28+
this.SslVerify = true;
29+
}
30+
31+
public GitSsl(IDictionary<string, GitConfigSetting> configSettings) : this()
32+
{
33+
if (configSettings != null)
34+
{
35+
if (configSettings.TryGetValue(GitConfigSetting.HttpSslCert, out GitConfigSetting sslCerts))
36+
{
37+
this.SslCertificate = sslCerts.Values.Single();
38+
}
39+
40+
if (configSettings.TryGetValue(GitConfigSetting.HttpSslCertPasswordProtected, out GitConfigSetting isSslCertPasswordProtected))
41+
{
42+
this.SslCertPasswordProtected = isSslCertPasswordProtected.Values.Select(bool.Parse).Single();
43+
}
44+
45+
if (configSettings.TryGetValue(GitConfigSetting.HttpSslVerify, out GitConfigSetting sslVerify))
46+
{
47+
this.SslVerify = sslVerify.Values.Select(bool.Parse).Single();
48+
}
49+
}
50+
}
51+
52+
public string LoadCertificatePassword(ITracer tracer, GitProcess git)
53+
{
54+
if (git.TryGetCertificatePassword(tracer, this.SslCertificate, out string password, out string error))
55+
{
56+
return password;
57+
}
58+
59+
return null;
60+
}
61+
62+
public X509Certificate2 LoadCertificate(ITracer tracer, string certificatePassword, bool onlyLoadValidCertificateFromStore)
63+
{
64+
EventMetadata metadata = new EventMetadata
65+
{
66+
{ "certId", this.SslCertificate },
67+
{ "isPasswordSpecified", string.IsNullOrEmpty(certificatePassword) },
68+
{ "shouldVerify", onlyLoadValidCertificateFromStore }
69+
};
70+
71+
if (File.Exists(this.SslCertificate))
72+
{
73+
try
74+
{
75+
X509Certificate2 cert = new X509Certificate2(this.SslCertificate, certificatePassword);
76+
if (onlyLoadValidCertificateFromStore && cert != null && !cert.Verify())
77+
{
78+
tracer.RelatedWarning(metadata, "Certficate was found, but is invalid.");
79+
return null;
80+
}
81+
82+
return cert;
83+
}
84+
catch (CryptographicException cryptEx)
85+
{
86+
metadata.Add("Exception", cryptEx);
87+
tracer.RelatedError(metadata, "Error, while loading certificate from disk");
88+
return null;
89+
}
90+
}
91+
92+
try
93+
{
94+
X509Certificate2Collection findResults = this.store.Value.Certificates.Find(X509FindType.FindBySubjectName, this.SslCertificate, onlyLoadValidCertificateFromStore);
95+
if (findResults?.Count > 0)
96+
{
97+
return findResults[0];
98+
}
99+
}
100+
catch (CryptographicException cryptEx)
101+
{
102+
metadata.Add("Exception", cryptEx);
103+
tracer.RelatedError(metadata, "Error, while searching for certificate in store");
104+
return null;
105+
}
106+
107+
tracer.RelatedError("Certificate {0} not found", this.SslCertificate);
108+
return null;
109+
}
110+
111+
public void Dispose()
112+
{
113+
if (this.store.IsValueCreated)
114+
{
115+
this.store.Value.Dispose();
116+
}
117+
}
118+
}
119+
}

‎GVFS/GVFS.Common/Git/GitSslSettings.cs‎

Lines changed: 0 additions & 40 deletions
This file was deleted.

‎GVFS/GVFS.Common/Http/HttpRequestor.cs‎

Lines changed: 2 additions & 109 deletions
Original file line numberDiff line numberDiff line change
@@ -8,9 +8,6 @@
88
using System.Net;
99
using System.Net.Http;
1010
using System.Net.Http.Headers;
11-
using System.Security;
12-
using System.Security.Cryptography;
13-
using System.Security.Cryptography.X509Certificates;
1411
using System.Text;
1512
using System.Threading;
1613
using System.Threading.Tasks;
@@ -26,13 +23,6 @@ public abstract class HttpRequestor : IDisposable
2623

2724
private readonly GitAuthentication authentication;
2825

29-
private readonly Lazy<X509Store> store = new Lazy<X509Store>(() =>
30-
{
31-
X509Store s = new X509Store();
32-
s.Open(OpenFlags.OpenExistingOnly | OpenFlags.ReadOnly);
33-
return s;
34-
});
35-
3626
private HttpClient client;
3727

3828
static HttpRequestor()
@@ -50,42 +40,9 @@ protected HttpRequestor(ITracer tracer, RetryConfig retryConfig, Enlistment enli
5040

5141
this.Tracer = tracer;
5242

53-
var httpClientHandler = new HttpClientHandler() { UseDefaultCredentials = true };
54-
55-
if (!this.authentication.GitSslSettings.SslVerify)
56-
{
57-
httpClientHandler.ServerCertificateCustomValidationCallback =
58-
(httpRequestMessage, cert, cetChain, policyErrors) =>
59-
{
60-
return true;
61-
};
62-
}
63-
64-
if (!string.IsNullOrEmpty(this.authentication.GitSslSettings.SslCertificate))
65-
{
66-
string certificatePassword = null;
67-
if (this.authentication.GitSslSettings.SslCertPasswordProtected)
68-
{
69-
certificatePassword = this.LoadCertificatePassword(this.authentication.GitSslSettings.SslCertificate, enlistment.CreateGitProcess());
70-
71-
if (string.IsNullOrEmpty(certificatePassword))
72-
{
73-
this.Tracer.RelatedWarning(
74-
new EventMetadata
75-
{
76-
{ "SslCertificate", this.authentication.GitSslSettings.SslCertificate }
77-
},
78-
"Git config indicates, that certificate is password protected, but retrieved password was null or empty!");
79-
}
80-
}
43+
HttpClientHandler httpClientHandler = new HttpClientHandler() { UseDefaultCredentials = true };
8144

82-
var cert = this.LoadCertificate(this.authentication.GitSslSettings.SslCertificate, certificatePassword, this.authentication.GitSslSettings.SslVerify);
83-
if (cert != null)
84-
{
85-
httpClientHandler.ClientCertificateOptions = ClientCertificateOption.Manual;
86-
httpClientHandler.ClientCertificates.Add(cert);
87-
}
88-
}
45+
this.authentication.SetupSslIfNeeded(this.Tracer, httpClientHandler, enlistment.CreateGitProcess());
8946

9047
this.client = new HttpClient(httpClientHandler)
9148
{
@@ -111,11 +68,6 @@ public void Dispose()
11168
this.client.Dispose();
11269
this.client = null;
11370
}
114-
115-
if (this.store.IsValueCreated)
116-
{
117-
this.store.Value.Close();
118-
}
11971
}
12072

12173
protected GitEndPointResponseData SendRequest(
@@ -321,64 +273,5 @@ private static string GetSingleHeaderOrEmpty(HttpHeaders headers, string headerN
321273

322274
return string.Empty;
323275
}
324-
325-
private string LoadCertificatePassword(string certId, GitProcess git)
326-
{
327-
if (git.TryGetCertificatePassword(this.Tracer, certId, out var password, out var error))
328-
{
329-
return password;
330-
}
331-
332-
return null;
333-
}
334-
335-
private X509Certificate2 LoadCertificate(string certId, string certificatePassword, bool onlyLoadValidCertificateFromStore)
336-
{
337-
EventMetadata metadata = new EventMetadata
338-
{
339-
{ "certId", certId },
340-
{ "isPasswordSpecified", string.IsNullOrEmpty(certificatePassword) },
341-
{ "shouldVerify", onlyLoadValidCertificateFromStore }
342-
};
343-
344-
if (File.Exists(certId))
345-
{
346-
try
347-
{
348-
var cert = new X509Certificate2(certId, certificatePassword);
349-
if (onlyLoadValidCertificateFromStore && cert != null && !cert.Verify())
350-
{
351-
this.Tracer.RelatedWarning(metadata, "Certficate was found, but is invalid.");
352-
return null;
353-
}
354-
355-
return cert;
356-
}
357-
catch (CryptographicException cryptEx)
358-
{
359-
metadata.Add("Exception", cryptEx);
360-
this.Tracer.RelatedError(metadata, "Error, while loading certificate from disk");
361-
return null;
362-
}
363-
}
364-
365-
try
366-
{
367-
var findResults = this.store.Value.Certificates.Find(X509FindType.FindBySubjectName, certId, onlyLoadValidCertificateFromStore);
368-
if (findResults?.Count > 0)
369-
{
370-
return findResults[0];
371-
}
372-
}
373-
catch (CryptographicException cryptEx)
374-
{
375-
metadata.Add("Exception", cryptEx);
376-
this.Tracer.RelatedError(metadata, "Error, while searching for certificate in store");
377-
return null;
378-
}
379-
380-
this.Tracer.RelatedError("Certificate {0} not found", certId);
381-
return null;
382-
}
383276
}
384277
}

0 commit comments

Comments
 (0)