From 64dd9f36ff50366969a718ab6fd2de26bca661c2 Mon Sep 17 00:00:00 2001 From: wishel Date: Wed, 19 Jul 2017 10:27:04 -0700 Subject: [PATCH 1/2] Fix for a problem with validating certificates in which the intermediate certificate, not the root, was used as an anchor --- csharp/agent/Certificates/TrustChainValidator.cs | 4 +++- csharp/agent/Certificates/X509ChainEngine.cs | 3 ++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/csharp/agent/Certificates/TrustChainValidator.cs b/csharp/agent/Certificates/TrustChainValidator.cs index d969efbc3..4769c57f1 100644 --- a/csharp/agent/Certificates/TrustChainValidator.cs +++ b/csharp/agent/Certificates/TrustChainValidator.cs @@ -291,10 +291,12 @@ public bool IsTrustedCertificate(X509Certificate2 certificate, X509Certificate2C return false; } + // X509ChainEngine, which is used for CRL validation, requires Windows 2012 to run due to required changes in the CERT_CHAIN_ENGINE_CONFIG structure. + // For more information on version numbers, see https://msdn.microsoft.com/en-us/library/windows/desktop/ms724832.aspx private static bool IsNewerThanWin2008R2() { return Environment.OSVersion.Version.Major > 6 || - (Environment.OSVersion.Version.Major >= 6 && Environment.OSVersion.Version.Minor > 2); + (Environment.OSVersion.Version.Major >= 6 && Environment.OSVersion.Version.Minor >= 2); } private bool ChainElementHasProblems(X509ChainElement chainElement) diff --git a/csharp/agent/Certificates/X509ChainEngine.cs b/csharp/agent/Certificates/X509ChainEngine.cs index 5d840e591..4cb28285d 100644 --- a/csharp/agent/Certificates/X509ChainEngine.cs +++ b/csharp/agent/Certificates/X509ChainEngine.cs @@ -88,7 +88,8 @@ public unsafe X509ChainEngine(IEnumerable trustedRoots) cbSize = (uint)sizeof(NativeMethods.CERT_CHAIN_ENGINE_CONFIG), dwFlags = NativeMethods.CERT_CHAIN_ENABLE_CACHE_AUTO_UPDATE | NativeMethods.CERT_CHAIN_ENABLE_SHARE_STORE, // Need to retrieve underlying IntPtr. The struct cannot contain a Safehandle field as we need to be able to use sizeof() - hExclusiveRoot = this.safeCertStoreHandle.DangerousGetHandle() + hExclusiveRoot = this.safeCertStoreHandle.DangerousGetHandle(), + dwExclusiveFlags = NativeMethods.CERT_CHAIN_EXCLUSIVE_ENABLE_CA_FLAG }; if (!NativeMethods.CertCreateCertificateChainEngine(new IntPtr(&certChainEngineConfig), out this.safeChainEngineHandle)) From 53b29699478ff1d34a407a223844a9ab8fd9ec65 Mon Sep 17 00:00:00 2001 From: wishel Date: Mon, 31 Jul 2017 14:48:10 -0700 Subject: [PATCH 2/2] Updated unit test to validate trust for intermediate anchor --- csharp/unittests/agent/TrustChainTests.cs | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/csharp/unittests/agent/TrustChainTests.cs b/csharp/unittests/agent/TrustChainTests.cs index c01759325..cf45c8ff3 100644 --- a/csharp/unittests/agent/TrustChainTests.cs +++ b/csharp/unittests/agent/TrustChainTests.cs @@ -44,9 +44,22 @@ public TrustChainTests() [Fact] public void TestValidTrustChain() + { + this.ValidateCertChainWithAnchor(m_trustedAnchors); + } + + + [Fact] + public void TestValidTrustedChainIntermediateAnchor() + { + var trustedIntermediate = m_resolver.GetCertificatesForDomain("inter1.xyz"); + this.ValidateCertChainWithAnchor(trustedIntermediate); + } + + private void ValidateCertChainWithAnchor(X509Certificate2Collection trustedIntermediate) { Assert.True(!m_endCerts.IsNullOrEmpty()); - Assert.True(!m_trustedAnchors.IsNullOrEmpty()); + Assert.True(!trustedIntermediate.IsNullOrEmpty()); // // Ok, verify certs.. @@ -55,7 +68,7 @@ public void TestValidTrustChain() { X509Certificate2Collection issuers = m_validator.ResolveIntermediateIssuers(cert); Assert.True(!issuers.IsNullOrEmpty() && issuers.Count == 3); - Assert.True(m_validator.IsTrustedCertificate(cert, m_trustedAnchors)); + Assert.True(m_validator.IsTrustedCertificate(cert, trustedIntermediate)); } }