From ad60240162e5d4a4156d459c45ec1980be5bce4c Mon Sep 17 00:00:00 2001 From: David Shulman Date: Sun, 10 Feb 2019 20:37:47 -0800 Subject: [PATCH] Fix test failures in CertificateValidationClientServer_EndToEnd_Ok (dotnet/corefx#35225) This System.Net.Security Functional test was always failing on my machine. But not failing in CI. The reason is that the CI machines don't have the TestRoot certificate installed but my machine does. So `Capability.IsTrustedRootCertificateInstalled()` is always false on the CI machines but true on my machine. Due to this test almost never running that codepath, the test got bad due to some prior refactoring and the failure wasn't noticed until now. The test was making an incorrect assumption about the contents and sizes of certificate collections. I fixed the test so that it validates what it should have been doing all along. The validation is about checking that the received/built trusted chain is valid and that the end-entity certificate is on that chain and is trusted by the root on that chain. Commit migrated from https://github.com/dotnet/corefx/commit/224fa8e1e58b10b7fe1a5d3cfdedf2a1afbfe2e1 --- .../FunctionalTests/CertificateChainValidation.cs | 41 ---------------------- .../CertificateValidationClientServer.cs | 27 ++++++++++++-- .../System.Net.Security.Tests.csproj | 1 - 3 files changed, 24 insertions(+), 45 deletions(-) delete mode 100644 src/libraries/System.Net.Security/tests/FunctionalTests/CertificateChainValidation.cs diff --git a/src/libraries/System.Net.Security/tests/FunctionalTests/CertificateChainValidation.cs b/src/libraries/System.Net.Security/tests/FunctionalTests/CertificateChainValidation.cs deleted file mode 100644 index d18a702..0000000 --- a/src/libraries/System.Net.Security/tests/FunctionalTests/CertificateChainValidation.cs +++ /dev/null @@ -1,41 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. -// See the LICENSE file in the project root for more information. - -using System.Collections.Generic; -using System.Diagnostics; -using System.Net.Sockets; -using System.Net.Test.Common; -using System.Security.Cryptography.X509Certificates; -using System.Threading.Tasks; - -using Xunit; -using Xunit.Abstractions; - -namespace System.Net.Security.Tests -{ - public static class CertificateChainValidation - { - public static void Validate(X509Certificate2Collection expected, X509Chain actual) - { - ITestOutputHelper log = TestLogging.GetInstance(); - log.WriteLine("CertificateChainValidation()"); - - var expectedIndexed = new Dictionary(); - var actualIndexed = new Dictionary(); - - Assert.Equal(expected.Count, actual.ChainElements.Count); - - for (int i = 0; i < expected.Count; i++) - { - expectedIndexed.Add(expected[i].Thumbprint, expected[i]); - actualIndexed.Add(actual.ChainElements[i].Certificate.Thumbprint, actual.ChainElements[i].Certificate); - } - - foreach (string thumbprint in expectedIndexed.Keys) - { - Assert.Equal(expectedIndexed[thumbprint], actualIndexed[thumbprint]); - } - } - } -} diff --git a/src/libraries/System.Net.Security/tests/FunctionalTests/CertificateValidationClientServer.cs b/src/libraries/System.Net.Security/tests/FunctionalTests/CertificateValidationClientServer.cs index e140430..3f9764b 100644 --- a/src/libraries/System.Net.Security/tests/FunctionalTests/CertificateValidationClientServer.cs +++ b/src/libraries/System.Net.Security/tests/FunctionalTests/CertificateValidationClientServer.cs @@ -8,6 +8,7 @@ using System.Security.Cryptography.X509Certificates; using System.Threading.Tasks; using Xunit; +using Xunit.Abstractions; namespace System.Net.Security.Tests { @@ -15,14 +16,17 @@ namespace System.Net.Security.Tests public class CertificateValidationClientServer : IDisposable { + private readonly ITestOutputHelper _output; private readonly X509Certificate2 _clientCertificate; private readonly X509Certificate2Collection _clientCertificateCollection; private readonly X509Certificate2 _serverCertificate; private readonly X509Certificate2Collection _serverCertificateCollection; private bool _clientCertificateRemovedByFilter; - public CertificateValidationClientServer() + public CertificateValidationClientServer(ITestOutputHelper output) { + _output = output; + _serverCertificateCollection = Configuration.Certificates.GetServerCertificateCollection(); _serverCertificate = Configuration.Certificates.GetServerCertificate(); @@ -161,7 +165,7 @@ namespace System.Net.Security.Tests else { // Validate only if we're able to build a trusted chain. - CertificateChainValidation.Validate(_clientCertificateCollection, chain); + ValidateCertificateAndChain(_clientCertificate, chain); } Assert.Equal(expectedSslPolicyErrors, sslPolicyErrors); @@ -184,7 +188,7 @@ namespace System.Net.Security.Tests else { // Validate only if we're able to build a trusted chain. - CertificateChainValidation.Validate(_serverCertificateCollection, chain); + ValidateCertificateAndChain(_serverCertificate, chain); } Assert.Equal(expectedSslPolicyErrors, sslPolicyErrors); @@ -192,5 +196,22 @@ namespace System.Net.Security.Tests return true; } + + private void ValidateCertificateAndChain(X509Certificate2 cert, X509Chain trustedChain) + { + _output.WriteLine("ValidateCertificateAndChain()"); + + // Verify that the certificate is in the trustedChain. + _output.WriteLine($"cert: subject={cert.Subject}, issuer={cert.Issuer}, thumbprint={cert.Thumbprint}"); + Assert.Equal(cert.Thumbprint, trustedChain.ChainElements[0].Certificate.Thumbprint); + + // Verify that the root certificate in the chain is the one that issued the received certificate. + foreach (X509ChainElement element in trustedChain.ChainElements) + { + _output.WriteLine( + $"chain cert: subject={element.Certificate.Subject}, issuer={element.Certificate.Issuer}, thumbprint={element.Certificate.Thumbprint}"); + } + Assert.Equal(cert.Issuer, trustedChain.ChainElements[1].Certificate.Subject); + } } } diff --git a/src/libraries/System.Net.Security/tests/FunctionalTests/System.Net.Security.Tests.csproj b/src/libraries/System.Net.Security/tests/FunctionalTests/System.Net.Security.Tests.csproj index 3dd8b3c..a5fdd07 100644 --- a/src/libraries/System.Net.Security/tests/FunctionalTests/System.Net.Security.Tests.csproj +++ b/src/libraries/System.Net.Security/tests/FunctionalTests/System.Net.Security.Tests.csproj @@ -11,7 +11,6 @@ - -- 2.7.4