Skip to content
This repository was archived by the owner on Jul 30, 2024. It is now read-only.

Commit 549de3f

Browse files
authored
Added logic on how to update signatures after certificate status changes (#324)
1 parent b1cd817 commit 549de3f

12 files changed

Lines changed: 677 additions & 180 deletions
Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
<?xml version="1.0" encoding="utf-8"?>
22
<configuration>
33
<startup>
4-
<supportedRuntime version="v4.0" sku=".NETFramework,Version=v4.6"/>
4+
<supportedRuntime version="v4.0" sku=".NETFramework,Version=v4.6.1"/>
55
</startup>
66
</configuration>

src/Validation.PackageSigning.ValidateCertificate/CertificateValidationService.cs

Lines changed: 92 additions & 104 deletions
Large diffs are not rendered by default.

src/Validation.PackageSigning.ValidateCertificate/CertificateVerificationResult.cs

Lines changed: 12 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
33

44
using System;
5+
using System.Security.Cryptography.X509Certificates;
56
using NuGet.Services.Validation;
67

78
namespace Validation.PackageSigning.ValidateCertificate
@@ -13,39 +14,26 @@ namespace Validation.PackageSigning.ValidateCertificate
1314
public class CertificateVerificationResult
1415
{
1516
/// <summary>
16-
/// Create a new non-revoked certificate verification result.
17+
/// The status of the end <see cref="X509Certificate2"/>.
1718
/// </summary>
18-
/// <param name="status">The status of the <see cref="X509Certificate2"/></param>
19-
/// <param name="revocationTime">The time of </param>
20-
public CertificateVerificationResult(EndCertificateStatus status)
21-
{
22-
if (status == EndCertificateStatus.Revoked)
23-
{
24-
throw new ArgumentException("Provide a revocation date for a revoked certificate result.", nameof(status));
25-
}
26-
27-
Status = status;
28-
}
19+
public EndCertificateStatus Status { get; set; }
2920

3021
/// <summary>
31-
/// Create a new revoked certificate verification result.
22+
/// The flattened flags for the <see cref="X509Certificate2"/> and its entire chain.
3223
/// </summary>
33-
/// <param name="revocationTime">The start of the certificate's invalidity period.</param>
34-
public CertificateVerificationResult(DateTime revocationTime)
35-
{
36-
Status = EndCertificateStatus.Revoked;
37-
RevocationTime = revocationTime;
38-
}
24+
public X509ChainStatusFlags StatusFlags { get; set; }
3925

4026
/// <summary>
41-
/// The status of the <see cref="X509Certificate2"/>.
27+
/// The time that the end <see cref="X509Certificate2"/>'s status was last updated, according to the
28+
/// Certificate Authority. If <see cref="Status"/> is <see cref="EndCertificateStatus.Revoked"/>
29+
/// or <see cref="EndCertificateStatus.Unknown"/>, this will have a value of <c>null</c>.
4230
/// </summary>
43-
public EndCertificateStatus Status { get; }
31+
public DateTime? StatusUpdateTime { get; set; }
4432

4533
/// <summary>
46-
/// The time at which the <see cref="X509Certificate2"/> was revoked. Null unless
47-
/// <see cref="Status"/> is <see cref="CertificateStatus.Revoked"/>.
34+
/// The time at which the end <see cref="X509Certificate2"/> was revoked. If <see cref="Status"/>
35+
/// is not <see cref="CertificateStatus.Revoked"/>, this will have a value of <c>null</c>.
4836
/// </summary>
49-
public DateTime? RevocationTime { get; }
37+
public DateTime? RevocationTime { get; set; }
5038
}
5139
}

src/Validation.PackageSigning.ValidateCertificate/ITelemetryService.cs

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,19 @@ namespace Validation.PackageSigning.ValidateCertificate
77
{
88
public interface ITelemetryService
99
{
10+
/// <summary>
11+
/// The event that tracks when a signature may be invalid and should be manually inspected.
12+
/// Unlike <see cref="TrackPackageSignatureShouldBeInvalidatedEvent(PackageSignature)"/>, this
13+
/// package MAY be found to still be valid!
14+
/// </summary>
15+
/// <param name="signature">The signature that should be invalidated.</param>
16+
/// <returns>A task that returns when the event has been recorded.</returns>
17+
void TrackPackageSignatureMayBeInvalidatedEvent(PackageSignature signature);
18+
1019
/// <summary>
1120
/// The event that tracks when a signature should be manually invalidated.
21+
/// Unlike <see cref="TrackPackageSignatureMayBeInvalidatedEvent(PackageSignature)"/>, this
22+
/// package SHOULD be invalidated.
1223
/// </summary>
1324
/// <param name="signature">The signature that should be invalidated.</param>
1425
/// <returns>A task that returns when the event has been recorded.</returns>
Lines changed: 149 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,149 @@
1+
// Copyright (c) .NET Foundation. All rights reserved.
2+
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
3+
4+
using System;
5+
using System.Linq;
6+
using System.Security.Cryptography.X509Certificates;
7+
using NuGet.Services.Validation;
8+
9+
namespace Validation.PackageSigning.ValidateCertificate
10+
{
11+
/// <summary>
12+
/// Deciders are created from a given <see cref="CertificateVerificationResult"/> to decide
13+
/// how each of the certificate's dependent <see cref="PackageSignature"/>s should be affected.
14+
/// </summary>
15+
/// <param name="signature">A signature that depends on the <see cref="EndCertificate"/> that was verified.</param>
16+
/// <returns>How the signature should be handled.</returns>
17+
public delegate SignatureDecision SignatureDecider(PackageSignature signature);
18+
19+
/// <summary>
20+
/// Creates functions that decide how <see cref="PackageSignature"/>s should be affected by
21+
/// <see cref="EndCertificate.Status"/> changes.
22+
/// </summary>
23+
public class SignatureDeciderFactory
24+
{
25+
/// <summary>
26+
/// Create a function to decide how signatures should be affected by a revoked certificate.
27+
/// </summary>
28+
/// <param name="certificate">The certificate that was revoked.</param>
29+
/// <param name="result">The verification result that describes when the certificate was revoked.</param>
30+
/// <returns>The function that describes how dependent signatures should be affected by the certificate status change.</returns>
31+
public SignatureDecider MakeDeciderForRevokedCertificate(EndCertificate certificate, CertificateVerificationResult result)
32+
{
33+
switch (certificate.Use)
34+
{
35+
case EndCertificateUse.Timestamping:
36+
return RejectSignaturesAtIngestionOtherwiseWarnDecider;
37+
38+
case EndCertificateUse.CodeSigning:
39+
return MakeDeciderForRevokedCodeSigningCertificate(result.RevocationTime);
40+
41+
default:
42+
throw new InvalidOperationException($"Revoked certificate has unknown use: {certificate.Use}");
43+
}
44+
}
45+
46+
/// <summary>
47+
/// Create a function to decide how signatures should be affected by an invalidated certificate.
48+
/// </summary>
49+
/// <param name="certificate">The certificate that was invalidated.</param>
50+
/// <param name="result">The verification result that describes why the certificate was invalidated.</param>
51+
/// <returns>The function that describes how dependent signatures should be affected by the certificate status change.</returns>
52+
public SignatureDecider MakeDeciderForInvalidatedCertificate(EndCertificate certificate, CertificateVerificationResult result)
53+
{
54+
if (result.Status != EndCertificateStatus.Invalid)
55+
{
56+
throw new ArgumentException($"Result must have a status of {nameof(EndCertificateStatus.Invalid)}", nameof(result));
57+
}
58+
59+
if (result.StatusFlags == X509ChainStatusFlags.NoError ||
60+
(result.StatusFlags & (X509ChainStatusFlags.OfflineRevocation | X509ChainStatusFlags.RevocationStatusUnknown)) != 0)
61+
{
62+
throw new ArgumentException($"Invalid flags on invalid verification result: {result.StatusFlags}!", nameof(result));
63+
}
64+
65+
// If a certificate used for the primary signature is revoked, all dependent signatures should be invalidated.
66+
// Note that in this case the revoked certificate is a parent of the end certificate - NOT the end certificate.
67+
if (certificate.Use == EndCertificateUse.CodeSigning && (result.StatusFlags & X509ChainStatusFlags.Revoked) != 0)
68+
{
69+
return RejectAllSignaturesDecider;
70+
}
71+
72+
// NotTimeValid and HasWeakSignature fail packages only at ingestion.
73+
else if (ResultHasOnlyFlags(result, X509ChainStatusFlags.NotTimeValid | X509ChainStatusFlags.HasWeakSignature))
74+
{
75+
return RejectSignaturesAtIngestionDecider;
76+
}
77+
78+
// NotTimeNested does not affect signatures and should be ignored.
79+
else if (ResultHasOnlyFlags(result, X509ChainStatusFlags.NotTimeNested))
80+
{
81+
return NoActionDecider;
82+
}
83+
84+
// In all other cases, reject signatures at ingestion and warn on all other signatures.
85+
else
86+
{
87+
return RejectSignaturesAtIngestionOtherwiseWarnDecider;
88+
}
89+
}
90+
91+
private SignatureDecider MakeDeciderForRevokedCodeSigningCertificate(DateTime? revocationTime)
92+
{
93+
// If the time the certificate was revoked is unknown, assume the worst and reject all dependent signatures.
94+
if (!revocationTime.HasValue)
95+
{
96+
return RejectAllSignaturesDecider;
97+
}
98+
99+
return (PackageSignature signature) =>
100+
{
101+
// The revoked code signing certificate invalidates signatures with no valid timestamps.
102+
// TODO: This should skip trusted timestamps with invalid/revoked certs. This requires:
103+
// * Getting trusted timestamps' certificates and their statuses.
104+
if (!signature.TrustedTimestamps.Any())
105+
{
106+
return SignatureDecision.Reject;
107+
}
108+
109+
// The revoked code signing certificate invalidates signatures with at least one trusted
110+
// timestamp before the revocation date.
111+
if (signature.TrustedTimestamps.Any(t => revocationTime.Value <= t.Value))
112+
{
113+
return SignatureDecision.Reject;
114+
}
115+
116+
// This signature lives to see another day.
117+
return SignatureDecision.Ignore;
118+
};
119+
}
120+
121+
private SignatureDecision NoActionDecider(PackageSignature signature) => SignatureDecision.Ignore;
122+
123+
private SignatureDecision RejectAllSignaturesDecider(PackageSignature signature) => SignatureDecision.Reject;
124+
125+
private SignatureDecision RejectSignaturesAtIngestionDecider(PackageSignature signature)
126+
{
127+
return (signature.Status == PackageSignatureStatus.Unknown)
128+
? SignatureDecision.Reject
129+
: SignatureDecision.Ignore;
130+
}
131+
132+
private SignatureDecision RejectSignaturesAtIngestionOtherwiseWarnDecider(PackageSignature signature)
133+
{
134+
return (signature.Status == PackageSignatureStatus.Unknown)
135+
? SignatureDecision.Reject
136+
: SignatureDecision.Warn;
137+
}
138+
139+
private bool ResultHasOnlyFlags(CertificateVerificationResult result, X509ChainStatusFlags flags)
140+
{
141+
if (result.StatusFlags == X509ChainStatusFlags.NoError)
142+
{
143+
return flags == X509ChainStatusFlags.NoError;
144+
}
145+
146+
return (result.StatusFlags & flags) == result.StatusFlags;
147+
}
148+
}
149+
}
Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
// Copyright (c) .NET Foundation. All rights reserved.
2+
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
3+
4+
using NuGet.Services.Validation;
5+
6+
namespace Validation.PackageSigning.ValidateCertificate
7+
{
8+
/// <summary>
9+
/// The ways an <see cref="EndCertificate"/>'s dependent <see cref="PackageSignature"/> may be affected by
10+
/// a change in the certificate's status.
11+
/// </summary>
12+
public enum SignatureDecision
13+
{
14+
/// <summary>
15+
/// The <see cref="PackageSignature"/> is unaffected by the status change of the <see cref="EndCertificate"/>.
16+
/// </summary>
17+
Ignore,
18+
19+
/// <summary>
20+
/// The <see cref="PackageSignature"/> may be affected by the status change of the <see cref="EndCertificate"/>.
21+
/// The NuGet client may no longer accept this signature, however, the package does not necessarily need to be
22+
/// deleted from the server. This decision will only happen for packages whose status is currently
23+
/// <see cref="PackageSignatureStatus.Valid"/>.
24+
/// </summary>
25+
Warn,
26+
27+
/// <summary>
28+
/// The <see cref="PackageSignature"/> is affected by the status change of the <see cref="EndCertificate"/>.
29+
/// The package should be deleted by an administrator.
30+
/// </summary>
31+
Reject,
32+
}
33+
}

src/Validation.PackageSigning.ValidateCertificate/TelemetryService.cs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,12 @@ namespace Validation.PackageSigning.ValidateCertificate
88
{
99
public class TelemetryService : ITelemetryService
1010
{
11+
public void TrackPackageSignatureMayBeInvalidatedEvent(PackageSignature signature)
12+
{
13+
// TODO
14+
throw new NotImplementedException();
15+
}
16+
1117
public void TrackPackageSignatureShouldBeInvalidatedEvent(PackageSignature signature)
1218
{
1319
// TODO

src/Validation.PackageSigning.ValidateCertificate/Validation.PackageSigning.ValidateCertificate.csproj

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
<OutputType>Exe</OutputType>
99
<RootNamespace>Validation.PackageSigning.ValidateCertificate</RootNamespace>
1010
<AssemblyName>Validation.PackageSigning.ValidateCertificate</AssemblyName>
11-
<TargetFrameworkVersion>v4.6</TargetFrameworkVersion>
11+
<TargetFrameworkVersion>v4.6.1</TargetFrameworkVersion>
1212
<FileAlignment>512</FileAlignment>
1313
<AutoGenerateBindingRedirects>true</AutoGenerateBindingRedirects>
1414
<TargetFrameworkProfile />
@@ -42,6 +42,8 @@
4242
<Reference Include="System.Xml" />
4343
</ItemGroup>
4444
<ItemGroup>
45+
<Compile Include="SignatureDeciderFactory.cs" />
46+
<Compile Include="SignatureDecision.cs" />
4547
<Compile Include="TelemetryService.cs" />
4648
<Compile Include="CertificateValidationMessageHandler.cs" />
4749
<Compile Include="CertificateValidationService.cs" />

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -105,11 +105,11 @@ public async Task RetriesIfSavingCertificateValidationEntityFails()
105105

106106
public static IEnumerable<object[]> MessageIsConsumedIfValidationEndsGracefullyData()
107107
{
108-
yield return new[] { new CertificateVerificationResult(EndCertificateStatus.Good) };
108+
yield return new[] { new CertificateVerificationResult() { Status = EndCertificateStatus.Good } };
109109

110-
yield return new[] { new CertificateVerificationResult(EndCertificateStatus.Invalid) };
110+
yield return new[] { new CertificateVerificationResult() { Status = EndCertificateStatus.Invalid } };
111111

112-
yield return new[] { new CertificateVerificationResult(revocationTime: DateTime.UtcNow) };
112+
yield return new[] { new CertificateVerificationResult() { Status = EndCertificateStatus.Revoked, RevocationTime = DateTime.UtcNow } };
113113
}
114114

115115
[Theory]
@@ -182,7 +182,7 @@ private async Task<bool> HandleUnknownResultAsync(int validationFailuresStart)
182182

183183
_certificateValidationService
184184
.Setup(s => s.VerifyAsync(It.IsAny<X509Certificate2>()))
185-
.ReturnsAsync(new CertificateVerificationResult(EndCertificateStatus.Unknown));
185+
.ReturnsAsync(new CertificateVerificationResult() { Status = EndCertificateStatus.Unknown });
186186

187187
_certificateValidationService
188188
.Setup(

0 commit comments

Comments
 (0)