Skip to content

Commit

Permalink
http: respect http.sslCAInfo setting(s) in Git
Browse files Browse the repository at this point in the history
Replicate the behavior of `http.sslCAInfo` (internally, cURL's `cainfo`/
OpenSSL's `cafile` arguments). Implemented by manually comparing the server
certificate with the config-specified CA file.
  • Loading branch information
vdye committed Aug 15, 2021
1 parent 03bf5d1 commit 8a7b46e
Show file tree
Hide file tree
Showing 7 changed files with 253 additions and 11 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ public class HttpClientFactoryTests
[Fact]
public void HttpClientFactory_GetClient_SetsDefaultHeaders()
{
var factory = new HttpClientFactory(Mock.Of<ITrace>(), Mock.Of<ISettings>(), new TestStandardStreams());
var factory = new HttpClientFactory(Mock.Of<IFileSystem>(), Mock.Of<ITrace>(), Mock.Of<ISettings>(), new TestStandardStreams());

HttpClient client = factory.CreateClient();

Expand All @@ -27,7 +27,7 @@ public void HttpClientFactory_GetClient_SetsDefaultHeaders()
[Fact]
public void HttpClientFactory_GetClient_MultipleCalls_ReturnsNewInstance()
{
var factory = new HttpClientFactory(Mock.Of<ITrace>(), Mock.Of<ISettings>(), new TestStandardStreams());
var factory = new HttpClientFactory(Mock.Of<IFileSystem>(), Mock.Of<ITrace>(), Mock.Of<ISettings>(), new TestStandardStreams());

HttpClient client1 = factory.CreateClient();
HttpClient client2 = factory.CreateClient();
Expand All @@ -47,7 +47,7 @@ public void HttpClientFactory_TryCreateProxy_NoProxy_ReturnsFalseOutNull()
RemoteUri = repoRemoteUri,
RepositoryPath = repoPath
};
var httpFactory = new HttpClientFactory(Mock.Of<ITrace>(), settings, Mock.Of<IStandardStreams>());
var httpFactory = new HttpClientFactory(Mock.Of<IFileSystem>(), Mock.Of<ITrace>(), settings, Mock.Of<IStandardStreams>());

bool result = httpFactory.TryCreateProxy(out IWebProxy proxy);

Expand All @@ -71,7 +71,7 @@ public void HttpClientFactory_TryCreateProxy_ProxyNoCredentials_ReturnsTrueOutPr
RepositoryPath = repoPath,
ProxyConfiguration = proxyConfig
};
var httpFactory = new HttpClientFactory(Mock.Of<ITrace>(), settings, Mock.Of<IStandardStreams>());
var httpFactory = new HttpClientFactory(Mock.Of<IFileSystem>(), Mock.Of<ITrace>(), settings, Mock.Of<IStandardStreams>());

bool result = httpFactory.TryCreateProxy(out IWebProxy proxy);

Expand Down Expand Up @@ -104,7 +104,7 @@ public void HttpClientFactory_TryCreateProxy_ProxyWithBypass_ReturnsTrueOutProxy
RepositoryPath = repoPath,
ProxyConfiguration = proxyConfig
};
var httpFactory = new HttpClientFactory(Mock.Of<ITrace>(), settings, Mock.Of<IStandardStreams>());
var httpFactory = new HttpClientFactory(Mock.Of<IFileSystem>(), Mock.Of<ITrace>(), settings, Mock.Of<IStandardStreams>());

bool result = httpFactory.TryCreateProxy(out IWebProxy proxy);

Expand Down Expand Up @@ -140,7 +140,7 @@ public void HttpClientFactory_TryCreateProxy_ProxyWithCredentials_ReturnsTrueOut
RepositoryPath = repoPath,
ProxyConfiguration = proxyConfig
};
var httpFactory = new HttpClientFactory(Mock.Of<ITrace>(), settings, Mock.Of<IStandardStreams>());
var httpFactory = new HttpClientFactory(Mock.Of<IFileSystem>(), Mock.Of<ITrace>(), settings, Mock.Of<IStandardStreams>());

bool result = httpFactory.TryCreateProxy(out IWebProxy proxy);

Expand Down Expand Up @@ -176,7 +176,7 @@ public void HttpClientFactory_TryCreateProxy_ProxyWithNonEmptyUserAndEmptyPass_R
RepositoryPath = repoPath,
ProxyConfiguration = proxyConfig
};
var httpFactory = new HttpClientFactory(Mock.Of<ITrace>(), settings, Mock.Of<IStandardStreams>());
var httpFactory = new HttpClientFactory(Mock.Of<IFileSystem>(), Mock.Of<ITrace>(), settings, Mock.Of<IStandardStreams>());

bool result = httpFactory.TryCreateProxy(out IWebProxy proxy);

Expand Down Expand Up @@ -212,7 +212,7 @@ public void HttpClientFactory_TryCreateProxy_ProxyWithEmptyUserAndNonEmptyPass_R
RepositoryPath = repoPath,
ProxyConfiguration = proxyConfig
};
var httpFactory = new HttpClientFactory(Mock.Of<ITrace>(), settings, Mock.Of<IStandardStreams>());
var httpFactory = new HttpClientFactory(Mock.Of<IFileSystem>(), Mock.Of<ITrace>(), settings, Mock.Of<IStandardStreams>());

bool result = httpFactory.TryCreateProxy(out IWebProxy proxy);

Expand Down Expand Up @@ -247,7 +247,7 @@ public void HttpClientFactory_TryCreateProxy_ProxyEmptyUserAndEmptyPass_ReturnsT
RepositoryPath = repoPath,
ProxyConfiguration = proxyConfig
};
var httpFactory = new HttpClientFactory(Mock.Of<ITrace>(), settings, Mock.Of<IStandardStreams>());
var httpFactory = new HttpClientFactory(Mock.Of<IFileSystem>(), Mock.Of<ITrace>(), settings, Mock.Of<IStandardStreams>());

bool result = httpFactory.TryCreateProxy(out IWebProxy proxy);

Expand All @@ -259,6 +259,32 @@ public void HttpClientFactory_TryCreateProxy_ProxyEmptyUserAndEmptyPass_ReturnsT
AssertDefaultCredentials(proxy.Credentials);
}

[Theory]
[InlineData(null, null, false, false)]
[InlineData("ca-bundle.crt", null, false, true)]
[InlineData("ca-bundle.crt", TlsBackend.Other, false, true)]
[InlineData("ca-bundle.crt", TlsBackend.Schannel, false, false)]
[InlineData("ca-bundle.crt", TlsBackend.Schannel, true, true)]
public void HttpClientFactory_GetClient_ChecksCertBundleOnlyIfEnabled(string customCertBundle,
TlsBackend? tlsBackend, bool useCustomCertBundleWithSchannel, bool expectBundleChecked)
{
var fileSystemMock = new Mock<IFileSystem>();
fileSystemMock.Setup(fs => fs.FileExists(It.IsAny<string>())).Returns(true);

var settings = new TestSettings()
{
CustomCertificateBundlePath = customCertBundle,
TlsBackend = tlsBackend,
UseCustomCertificateBundleWithSchannel = useCustomCertBundleWithSchannel
};

var factory = new HttpClientFactory(fileSystemMock.Object, Mock.Of<ITrace>(), settings, new TestStandardStreams());

HttpClient client = factory.CreateClient();

fileSystemMock.Verify(fs => fs.FileExists(It.IsAny<string>()), expectBundleChecked ? Times.Once : Times.Never);
}

private static void AssertDefaultCredentials(ICredentials credentials)
{
var netCred = (NetworkCredential) credentials;
Expand Down
69 changes: 69 additions & 0 deletions src/shared/Microsoft.Git.CredentialManager.Tests/SettingsTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1254,5 +1254,74 @@ public void Settings_GetSettingValues_IgnoresSectionAndPropertyCase_ScopeIsCaseS
Assert.NotNull(actualValues);
Assert.Equal(expectedValues, actualValues);
}

[Theory]
[InlineData(null, null, null)]
[InlineData(null, "ca-config.crt", "ca-config.crt")]
[InlineData("ca-envar.crt", "ca-config.crt", "ca-envar.crt")]
public void Settings_CustomCertificateBundlePath_ReturnsExpectedValue(string sslCaInfoEnvar, string sslCaInfoConfig, string expectedValue)
{
const string envarName = Constants.EnvironmentVariables.GitSslCaInfo;
const string section = Constants.GitConfiguration.Http.SectionName;
const string sslCaInfo = Constants.GitConfiguration.Http.SslCaInfo;

var envars = new TestEnvironment();
if (sslCaInfoEnvar != null)
{
envars.Variables[envarName] = sslCaInfoEnvar;
}

var git = new TestGit();
if (sslCaInfoConfig != null)
{
git.Configuration.Local[$"{section}.{sslCaInfo}"] = new[] {sslCaInfoConfig};
}

var settings = new Settings(envars, git);

string actualValue = settings.CustomCertificateBundlePath;

if (expectedValue is null)
{
Assert.Null(actualValue);
}
else
{
Assert.NotNull(actualValue);
Assert.Equal(expectedValue, actualValue);
}
}

[Theory]
[InlineData(null, null)]
[InlineData("schannel", TlsBackend.Schannel)]
[InlineData("openssl", TlsBackend.Other)]
public void Settings_TlsBackend_ReturnsExpectedValue(string sslBackendConfig, TlsBackend? expectedValue)
{
const string section = Constants.GitConfiguration.Http.SectionName;
const string sslBackend = Constants.GitConfiguration.Http.SslBackend;

var envars = new TestEnvironment();

var git = new TestGit();
if (sslBackendConfig != null)
{
git.Configuration.Local[$"{section}.{sslBackend}"] = new[] {sslBackendConfig};
}

var settings = new Settings(envars, git);

TlsBackend? actualValue = settings.TlsBackend;

if (expectedValue is null)
{
Assert.Null(actualValue);
}
else
{
Assert.NotNull(actualValue);
Assert.Equal(expectedValue, actualValue);
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ public CommandContext(string appPath)
throw new PlatformNotSupportedException();
}

HttpClientFactory = new HttpClientFactory(Trace, Settings, Streams);
HttpClientFactory = new HttpClientFactory(FileSystem, Trace, Settings, Streams);

// Set the parent window handle/ID
SystemPrompts.ParentWindowId = Settings.ParentWindowId;
Expand Down
4 changes: 4 additions & 0 deletions src/shared/Microsoft.Git.CredentialManager/Constants.cs
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ public static class EnvironmentVariables
public const string CurlHttpsProxy = "HTTPS_PROXY";
public const string GcmHttpProxy = "GCM_HTTP_PROXY";
public const string GitSslNoVerify = "GIT_SSL_NO_VERIFY";
public const string GitSslCaInfo = "GIT_SSL_CAINFO";
public const string GcmInteractive = "GCM_INTERACTIVE";
public const string GcmParentWindow = "GCM_MODAL_PARENTHWND";
public const string MsAuthFlow = "GCM_MSAUTH_FLOW";
Expand Down Expand Up @@ -97,7 +98,10 @@ public static class Http
{
public const string SectionName = "http";
public const string Proxy = "proxy";
public const string SchannelUseSslCaInfo = "schannelUseSSLCAInfo";
public const string SslBackend = "sslBackend";
public const string SslVerify = "sslVerify";
public const string SslCaInfo = "sslCAInfo";
}

public static class Remote
Expand Down
94 changes: 93 additions & 1 deletion src/shared/Microsoft.Git.CredentialManager/HttpClientFactory.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,13 @@
// Licensed under the MIT license.
using System;
using System.Collections.Generic;
using System.IO;
using System.Linq;
using System.Net;
using System.Net.Http;
using System.Net.Http.Headers;
using System.Net.Security;
using System.Security.Cryptography.X509Certificates;

namespace Microsoft.Git.CredentialManager
{
Expand Down Expand Up @@ -34,16 +37,19 @@ public interface IHttpClientFactory

public class HttpClientFactory : IHttpClientFactory
{
private readonly IFileSystem _fileSystem;
private readonly ITrace _trace;
private readonly ISettings _settings;
private readonly IStandardStreams _streams;

public HttpClientFactory(ITrace trace, ISettings settings, IStandardStreams streams)
public HttpClientFactory(IFileSystem fileSystem, ITrace trace, ISettings settings, IStandardStreams streams)
{
EnsureArgument.NotNull(fileSystem, nameof(fileSystem));
EnsureArgument.NotNull(trace, nameof(trace));
EnsureArgument.NotNull(settings, nameof(settings));
EnsureArgument.NotNull(streams, nameof(streams));

_fileSystem = fileSystem;
_trace = trace;
_settings = settings;
_streams = streams;
Expand All @@ -70,6 +76,7 @@ public HttpClient CreateClient()
handler = new HttpClientHandler();
}

// IsCertificateVerificationEnabled takes precedence over custom TLS cert verification
if (!_settings.IsCertificateVerificationEnabled)
{
_trace.WriteLine("TLS certificate verification has been disabled.");
Expand All @@ -84,6 +91,91 @@ public HttpClient CreateClient()
handler.ServerCertificateCustomValidationCallback = (req, cert, chain, errors) => true;
#endif
}
// If schannel is the TLS backend, custom certificate usage must be explicitly enabled
else if (!string.IsNullOrWhiteSpace(_settings.CustomCertificateBundlePath) &&
((_settings.TlsBackend != TlsBackend.Schannel) || _settings.UseCustomCertificateBundleWithSchannel))
{
string certBundlePath = _settings.CustomCertificateBundlePath;
_trace.WriteLine($"Custom certificate verification has been enabled with certificate bundle at {certBundlePath}");

// Throw exception if cert bundle file not found
if (!_fileSystem.FileExists(certBundlePath))
{
throw new FileNotFoundException($"Custom certificate bundle not found at path: {certBundlePath}", certBundlePath);
}

Func<X509Certificate2, X509Chain, SslPolicyErrors, bool> validationCallback = (cert, chain, errors) =>
{
// Fail immediately if there are non-chain issues with the remote cert
if ((errors & ~SslPolicyErrors.RemoteCertificateChainErrors) != 0)
{
return false;
}

// Import the custom certs
X509Certificate2Collection certBundle = new X509Certificate2Collection();
certBundle.Import(certBundlePath);

// Add the certs to the chain
chain.ChainPolicy.ExtraStore.AddRange(certBundle);

bool isValid = chain.Build(cert);

// Rebuild the chain
if (chain.Build(cert))
{
return true;
}
else if (chain.ChainStatus.Any(status => status.Status != X509ChainStatusFlags.UntrustedRoot))
{
// Fail for any errors other than untrusted root
return false;
}
else if (chain.ChainStatus.Any(status => status.Status == X509ChainStatusFlags.UntrustedRoot))
{
// Manually verify root is contained within the certBundle
X509Certificate2 rootCert = chain.ChainElements[chain.ChainElements.Count - 1].Certificate;
var matchingCerts = certBundle.Find(X509FindType.FindByThumbprint, rootCert.Thumbprint, true);
if (matchingCerts.Count > 0)
{
// Check the content of the first matching cert found (do
// not try others if mismatched - mirrors OpenSSL:
// https://www.openssl.org/docs/man1.1.1/man3/SSL_CTX_set_default_verify_paths.html#WARNINGS)
return rootCert.RawData.SequenceEqual(matchingCerts[0].RawData);
}
else
{
// Untrusted root not found in custom cert bundle
return false;
}
}
else
{
// Errors not due to ChainStatus errors - fail
return false;
}
};

// Set the custom server certificate validation callback.
// NOTE: this is executed after the default platform server certificate validation is performed
#if NETFRAMEWORK
ServicePointManager.ServerCertificateValidationCallback = (_, cert, chain, errors) =>
{
// Fail immediately if the cert or chain isn't present
if (cert is null || chain is null)
{
return false;
}

using (X509Certificate2 cert2 = new X509Certificate2(cert))
{
return validationCallback(cert2, chain, errors);
}
};
#elif NETSTANDARD
handler.ServerCertificateCustomValidationCallback = (_, cert, chain, errors) => validationCallback(cert, chain, errors);
#endif
}

var client = new HttpClient(handler);

Expand Down
Loading

0 comments on commit 8a7b46e

Please sign in to comment.