[Package Signing] Add more Validate Certificate integration tests (#338)

Author: loic-sharma

src/Validation.PackageSigning.ValidateCertificate/CertificateVerificationResult.cs

@@ -26,10 +26,12 @@ public CertificateVerificationResult(
             DateTime? statusUpdateTime = null,
             DateTime? revocationTime = null)
         {
-            if (status != EndCertificateStatus.Revoked && revocationTime.HasValue)
+            if (revocationTime.HasValue &&
+                status != EndCertificateStatus.Revoked &&
+                status != EndCertificateStatus.Invalid)
             {
                 throw new ArgumentException(
-                    $"End certificate revoked at {revocationTime} but status isn't {nameof(EndCertificateStatus.Revoked)}",
+                    $"End certificate revoked at {revocationTime} but status is {status}",
                     nameof(status));
             }
 
@@ -119,7 +121,7 @@ public override string ToString()
                     return $"Good (StatusUpdateTime = {StatusUpdateTime})";
 
                 case EndCertificateStatus.Invalid:
-                    return $"Invalid (Flags = {StatusFlags}, StatusUpdateTime = {StatusUpdateTime})";
+                    return $"Invalid (Flags = {StatusFlags}, RevocationTime = {RevocationTime}, StatusUpdateTime = {StatusUpdateTime})";
 
                 case EndCertificateStatus.Revoked:
                     return $"Revoked (Flags = {StatusFlags}, RevocationTime = {RevocationTime}, StatusUpdateTime = {StatusUpdateTime})";

src/Validation.PackageSigning.ValidateCertificate/SignatureDeciderFactory.cs

@@ -68,14 +68,17 @@ public SignatureDecider MakeDeciderForInvalidatedCertificate(EndCertificate cert
                 return RejectAllSignaturesDecider;
             }
 
-            // NotTimeValid and HasWeakSignature fail packages only at ingestion.
-            else if (ResultHasOnlyFlags(result, X509ChainStatusFlags.NotTimeValid | X509ChainStatusFlags.HasWeakSignature))
+            // NotTimeValid and HasWeakSignature fail packages only at ingestion. It is assumed that a chain with HasWeakSignature will
+            // ALWAYS have NotSignatureValid.
+            else if (result.StatusFlags == X509ChainStatusFlags.NotTimeValid ||
+                     result.StatusFlags == (X509ChainStatusFlags.HasWeakSignature | X509ChainStatusFlags.NotSignatureValid) ||
+                     result.StatusFlags == (X509ChainStatusFlags.NotTimeValid | X509ChainStatusFlags.HasWeakSignature | X509ChainStatusFlags.NotSignatureValid))
             {
                 return RejectSignaturesAtIngestionDecider;
             }
 
-            // NotTimeNested does not affect signatures and should be ignored.
-            else if (ResultHasOnlyFlags(result, X509ChainStatusFlags.NotTimeNested))
+            // NotTimeNested does not affect signatures and should be ignored if is the only status.
+            else if (result.StatusFlags == X509ChainStatusFlags.NotTimeNested)
             {
                 return NoActionDecider;
             }
@@ -145,15 +148,5 @@ private SignatureDecision RejectSignaturesAtIngestionOtherwiseWarnDecider(Packag
                     ? SignatureDecision.Reject
                     : SignatureDecision.Warn;
         }
-
-        private bool ResultHasOnlyFlags(CertificateVerificationResult result, X509ChainStatusFlags flags)
-        {
-            if (result.StatusFlags == X509ChainStatusFlags.NoError)
-            {
-                return flags == X509ChainStatusFlags.NoError;
-            }
-
-            return (result.StatusFlags & flags) == result.StatusFlags;
-        }
     }
 }

tests/Validation.PackageSigning.Core.Tests/Support/CertificateIntegrationTestFixture.cs

@@ -2,16 +2,19 @@
 // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
 
 using System;
+using System.Collections.Generic;
 using System.Security.Cryptography.X509Certificates;
 using System.Security.Principal;
 using System.Threading.Tasks;
 using Org.BouncyCastle.Asn1;
 using Org.BouncyCastle.Asn1.X509;
 using Org.BouncyCastle.Crypto.Parameters;
 using Org.BouncyCastle.Security;
+using Org.BouncyCastle.X509;
 using Test.Utility.Signing;
 using Xunit;
 using GeneralName = Org.BouncyCastle.Asn1.X509.GeneralName;
+using BCCertificate = Org.BouncyCastle.X509.X509Certificate;
 
 namespace Validation.PackageSigning.Core.Tests.Support
 {
@@ -22,6 +25,7 @@ namespace Validation.PackageSigning.Core.Tests.Support
     public class CertificateIntegrationTestFixture : IDisposable
     {
         private readonly Lazy<Task<SigningTestServer>> _testServer;
+        private readonly Lazy<Task<CertificateAuthority>> _rootCertificateAuthority;
         private readonly Lazy<Task<CertificateAuthority>> _certificateAuthority;
         private readonly Lazy<Task<TimestampService>> _timestampService;
         private readonly Lazy<Task<Uri>> _timestampServiceUrl;
@@ -37,6 +41,7 @@ public CertificateIntegrationTestFixture()
                 "This test must be executing with administrator privileges since it installs a trusted root.");
 
             _testServer = new Lazy<Task<SigningTestServer>>(SigningTestServer.CreateAsync);
+            _rootCertificateAuthority = new Lazy<Task<CertificateAuthority>>(CreateDefaultTrustedRootCertificateAuthorityAsync);
             _certificateAuthority = new Lazy<Task<CertificateAuthority>>(CreateDefaultTrustedCertificateAuthorityAsync);
             _timestampService = new Lazy<Task<TimestampService>>(CreateDefaultTrustedTimestampServiceAsync);
             _timestampServiceUrl = new Lazy<Task<Uri>>(CreateDefaultTrustedTimestampServiceUrlAsync);
@@ -50,6 +55,10 @@ public CertificateIntegrationTestFixture()
         public Task<X509Certificate2> GetSigningCertificateAsync() => _signingCertificate.Value;
         public Task<string> GetSigningCertificateThumbprintAsync() => _signingCertificateThumbprint.Value;
 
+        protected Task<CertificateAuthority> GetRootCertificateAuthority() => _rootCertificateAuthority.Value;
+        protected Task<CertificateAuthority> GetCertificateAuthority() => _certificateAuthority.Value;
+        protected DisposableList GetResponders() => _responders;
+
         public void Dispose()
         {
             _trustedRoot?.Dispose();
@@ -61,11 +70,10 @@ public void Dispose()
             }
         }
 
-        private async Task<CertificateAuthority> CreateDefaultTrustedCertificateAuthorityAsync()
+        private async Task<CertificateAuthority> CreateDefaultTrustedRootCertificateAuthorityAsync()
         {
             var testServer = await GetTestServerAsync();
             var rootCa = CertificateAuthority.Create(testServer.Url);
-            var intermediateCa = rootCa.CreateIntermediateCertificateAuthority();
             var rootCertificate = new X509Certificate2(rootCa.Certificate.GetEncoded());
 
             _trustedRoot = new TrustedTestCert<X509Certificate2>(
@@ -74,6 +82,15 @@ private async Task<CertificateAuthority> CreateDefaultTrustedCertificateAuthorit
                 StoreName.Root,
                 StoreLocation.LocalMachine);
 
+            return rootCa;
+        }
+
+        private async Task<CertificateAuthority> CreateDefaultTrustedCertificateAuthorityAsync()
+        {
+            var testServer = await GetTestServerAsync();
+            var rootCa = await GetRootCertificateAuthority();
+            var intermediateCa = rootCa.CreateIntermediateCertificateAuthority();
+
             _responders.AddRange(testServer.RegisterResponders(intermediateCa));
 
             return intermediateCa;
@@ -103,30 +120,32 @@ private async Task<X509Certificate2> CreateDefaultTrustedSigningCertificateAsync
         }
 
         public X509Certificate2 CreateSigningCertificate(CertificateAuthority ca)
+        {
+            void CustomizeAsSigningCertificate(X509V3CertificateGenerator generator)
+            {
+                generator.AddSigningEku();
+                generator.AddAuthorityInfoAccess(ca, addOcsp: true, addCAIssuers: true);
+            }
+
+            return IssueCertificate(ca, "Signing", CustomizeAsSigningCertificate).certificate;
+        }
+
+        protected (BCCertificate publicCertificate, X509Certificate2 certificate) IssueCertificate(
+            CertificateAuthority ca,
+            string name,
+            Action<X509V3CertificateGenerator> customizeCertificate)
         {
             var keyPair = SigningTestUtility.GenerateKeyPair(publicKeyLength: 2048);
             var publicCertificate = ca.IssueCertificate(
                 keyPair.Public,
-                new X509Name($"C=US,ST=WA,L=Redmond,O=NuGet,CN=NuGet Test Signing Certificate ({Guid.NewGuid()})"),
-                generator =>
-                {
-                    SigningTestUtility.CertificateModificationGeneratorForCodeSigningEkuCert(generator);
-
-                    generator.AddExtension(
-                        X509Extensions.AuthorityInfoAccess,
-                        critical: false,
-                        extensionValue: new DerSequence(
-                            new AccessDescription(AccessDescription.IdADOcsp,
-                                new GeneralName(GeneralName.UniformResourceIdentifier, ca.OcspResponderUri.OriginalString)),
-                            new AccessDescription(AccessDescription.IdADCAIssuers,
-                                new GeneralName(GeneralName.UniformResourceIdentifier, ca.CertificateUri.OriginalString))));
-                },
+                new X509Name($"C=US,ST=WA,L=Redmond,O=NuGet,CN=NuGet Test ${name} Certificate ({Guid.NewGuid()})"),
+                customizeCertificate,
                 notBefore: DateTime.UtcNow.AddSeconds(-10));
 
             var certificate = new X509Certificate2(publicCertificate.GetEncoded());
             certificate.PrivateKey = DotNetUtilities.ToRSA(keyPair.Private as RsaPrivateCrtKeyParameters);
 
-            return certificate;
+            return (publicCertificate, certificate);
         }
 
         private async Task<string> GetDefaultTrustedSigningCertificateThumbprintAsync()

tests/Validation.PackageSigning.Core.Tests/Support/X509V3CertificateGeneratorExtensions.cs

@@ -0,0 +1,53 @@
+// Copyright (c) .NET Foundation. All rights reserved.
+// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
+
+using System.Collections.Generic;
+using Org.BouncyCastle.Asn1;
+using Org.BouncyCastle.Asn1.X509;
+using Test.Utility.Signing;
+
+namespace Org.BouncyCastle.X509
+{
+    public static class X509V3CertificateGeneratorExtensions
+    {
+        public static void MakeExpired(this X509V3CertificateGenerator generator)
+        {
+            SigningTestUtility.CertificateModificationGeneratorExpiredCert(generator);
+        }
+
+        public static void AddSigningEku(this X509V3CertificateGenerator generator)
+        {
+            SigningTestUtility.CertificateModificationGeneratorForCodeSigningEkuCert(generator);
+        }
+
+        public static void AddAuthorityInfoAccess(
+            this X509V3CertificateGenerator generator,
+            CertificateAuthority ca,
+            bool addOcsp = false,
+            bool addCAIssuers = false)
+        {
+            var vector = new List<Asn1Encodable>();
+
+            if (addOcsp)
+            {
+                vector.Add(
+                    new AccessDescription(
+                        AccessDescription.IdADOcsp,
+                        new GeneralName(GeneralName.UniformResourceIdentifier, ca.OcspResponderUri.OriginalString)));
+            }
+
+            if (addCAIssuers)
+            {
+                vector.Add(
+                    new AccessDescription(
+                        AccessDescription.IdADCAIssuers,
+                        new GeneralName(GeneralName.UniformResourceIdentifier, ca.CertificateUri.OriginalString)));
+            }
+
+            generator.AddExtension(
+                X509Extensions.AuthorityInfoAccess,
+                critical: false,
+                extensionValue: new DerSequence(vector.ToArray()));
+        }
+    }
+}

tests/Validation.PackageSigning.Core.Tests/Validation.PackageSigning.Core.Tests.csproj

@@ -44,6 +44,7 @@
     <Compile Include="Support\TestDbAsyncEnumerable.cs" />
     <Compile Include="Support\TestDbAsyncEnumerator.cs" />
     <Compile Include="Support\TestDbAsyncQueryProvider.cs" />
+    <Compile Include="Support\X509V3CertificateGeneratorExtensions.cs" />
     <Compile Include="Support\XunitLogger.cs" />
     <Compile Include="Support\XunitLoggerFactoryExtensions.cs" />
     <Compile Include="Support\XunitLoggerProvider.cs" />
@@ -62,6 +63,9 @@
     <PackageReference Include="Portable.BouncyCastle">
       <Version>1.8.1.3</Version>
     </PackageReference>
+    <PackageReference Include="System.ValueTuple">
+      <Version>4.4.0</Version>
+    </PackageReference>
     <PackageReference Include="Test.Utility">
       <Version>4.7.0-preview1-4886</Version>
     </PackageReference>

tests/Validation.PackageSigning.ValidateCertificate.Tests/CertificateVerificationResultFacts.cs

@@ -72,7 +72,6 @@ public void CanCreateInvalidResultWithStatusUpdateTime()
 
         [Theory]
         [InlineData(EndCertificateStatus.Good, X509ChainStatusFlags.NoError)]
-        [InlineData(EndCertificateStatus.Invalid, X509ChainStatusFlags.ExplicitDistrust)]
         [InlineData(EndCertificateStatus.Unknown, X509ChainStatusFlags.OfflineRevocation)]
         public void CannotCreateNonRevokedResultWithRevocationDate(EndCertificateStatus status, X509ChainStatusFlags flags)
         {
@@ -83,7 +82,7 @@ public void CannotCreateNonRevokedResultWithRevocationDate(EndCertificateStatus
                             .WithRevocationTime(new DateTime(2000, 1, 2))
                             .Build());
 
-            Assert.StartsWith("End certificate revoked at 1/2/2000 12:00:00 AM but status isn't Revoked", exception.Message);
+            Assert.StartsWith($"End certificate revoked at 1/2/2000 12:00:00 AM but status is {status}", exception.Message);
         }
 
         [Fact]