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

Commit c153e2d

Browse files
authored
[Package Signing] Improved CertificateVerificationResult type (#326)
1 parent 8d2b271 commit c153e2d

6 files changed

Lines changed: 332 additions & 55 deletions

File tree

src/Validation.PackageSigning.ValidateCertificate/CertificateVerificationResult.cs

Lines changed: 147 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -13,27 +13,167 @@ namespace Validation.PackageSigning.ValidateCertificate
1313
/// </summary>
1414
public class CertificateVerificationResult
1515
{
16+
/// <summary>
17+
/// Create a new verification result.
18+
/// </summary>
19+
/// <param name="status">The determined status of the verified certificate.</param>
20+
/// <param name="statusFlags">The flags explaining the certificate's status.</param>
21+
/// <param name="statusUpdateTime">The time the revocation info was published.</param>
22+
/// <param name="revocationTime">The date the certificate was revoked, if applicable.</param>
23+
public CertificateVerificationResult(
24+
EndCertificateStatus status,
25+
X509ChainStatusFlags statusFlags,
26+
DateTime? statusUpdateTime = null,
27+
DateTime? revocationTime = null)
28+
{
29+
if (status != EndCertificateStatus.Revoked && revocationTime.HasValue)
30+
{
31+
throw new ArgumentException(
32+
$"End certificate revoked at {revocationTime} but status isn't {nameof(EndCertificateStatus.Revoked)}",
33+
nameof(status));
34+
}
35+
36+
switch (status)
37+
{
38+
case EndCertificateStatus.Good:
39+
if (statusFlags != X509ChainStatusFlags.NoError)
40+
{
41+
throw new ArgumentException(
42+
$"Invalid flags '{statusFlags}' for status '{status}'",
43+
nameof(statusFlags));
44+
}
45+
46+
break;
47+
48+
case EndCertificateStatus.Invalid:
49+
if (statusFlags == X509ChainStatusFlags.NoError)
50+
{
51+
throw new ArgumentException(
52+
$"Invalid flags '{statusFlags}' for status '{status}'",
53+
nameof(statusFlags));
54+
}
55+
56+
break;
57+
58+
case EndCertificateStatus.Revoked:
59+
if ((statusFlags & X509ChainStatusFlags.Revoked) == 0)
60+
{
61+
throw new ArgumentException(
62+
$"Invalid flags '{statusFlags}' for status '{status}'",
63+
nameof(statusFlags));
64+
}
65+
break;
66+
67+
case EndCertificateStatus.Unknown:
68+
if ((statusFlags & (X509ChainStatusFlags.RevocationStatusUnknown | X509ChainStatusFlags.OfflineRevocation)) == 0)
69+
{
70+
throw new ArgumentException(
71+
$"Invalid flags '{statusFlags}' for status '{status}'",
72+
nameof(statusFlags));
73+
}
74+
75+
break;
76+
77+
default:
78+
throw new ArgumentException($"Unknown status '{status}'", nameof(status));
79+
}
80+
81+
Status = status;
82+
StatusFlags = statusFlags;
83+
StatusUpdateTime = statusUpdateTime;
84+
RevocationTime = revocationTime;
85+
}
86+
1687
/// <summary>
1788
/// The status of the end <see cref="X509Certificate2"/>.
1889
/// </summary>
19-
public EndCertificateStatus Status { get; set; }
90+
public EndCertificateStatus Status { get; }
2091

2192
/// <summary>
22-
/// The flattened flags for the <see cref="X509Certificate2"/> and its entire chain.
93+
/// The flattened flags for the <see cref="X509Certificate2"/>'s entire chain.
2394
/// </summary>
24-
public X509ChainStatusFlags StatusFlags { get; set; }
95+
public X509ChainStatusFlags StatusFlags { get; }
2596

2697
/// <summary>
2798
/// 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>.
99+
/// Certificate Authority. This value may be <c>null</c> if the <see cref="Status"/> is
100+
/// <see cref="EndCertificateStatus.Unknown"/> or if the status could not be determined.
30101
/// </summary>
31-
public DateTime? StatusUpdateTime { get; set; }
102+
public DateTime? StatusUpdateTime { get; }
32103

33104
/// <summary>
34105
/// The time at which the end <see cref="X509Certificate2"/> was revoked. If <see cref="Status"/>
35106
/// is not <see cref="CertificateStatus.Revoked"/>, this will have a value of <c>null</c>.
36107
/// </summary>
37-
public DateTime? RevocationTime { get; set; }
108+
public DateTime? RevocationTime { get; }
109+
110+
/// <summary>
111+
/// Convert a verification to a human readable string.
112+
/// </summary>
113+
/// <returns>A human readable string that summarizes the verification result.</returns>
114+
public override string ToString()
115+
{
116+
switch (Status)
117+
{
118+
case EndCertificateStatus.Good:
119+
return $"Good (StatusUpdateTime = {StatusUpdateTime})";
120+
121+
case EndCertificateStatus.Invalid:
122+
return $"Invalid (Flags = {StatusFlags}, StatusUpdateTime = {StatusUpdateTime})";
123+
124+
case EndCertificateStatus.Revoked:
125+
return $"Revoked (Flags = {StatusFlags}, RevocationTime = {RevocationTime}, StatusUpdateTime = {StatusUpdateTime})";
126+
127+
case EndCertificateStatus.Unknown:
128+
return $"Unknown (Flags = {StatusFlags}, StatusUpdateTime = {StatusUpdateTime})";
129+
130+
default:
131+
throw new InvalidOperationException($"Unknown status {Status}");
132+
}
133+
}
134+
135+
/// <summary>
136+
/// Helper used to create valid <see cref="CertificateVerificationResult"/>s.
137+
/// </summary>
138+
public class Builder
139+
{
140+
private EndCertificateStatus _status;
141+
private X509ChainStatusFlags _statusFlags;
142+
private DateTime? _statusUpdateTime;
143+
private DateTime? _revocationTime;
144+
145+
public Builder WithStatus(EndCertificateStatus value)
146+
{
147+
_status = value;
148+
return this;
149+
}
150+
151+
public Builder WithStatusFlags(X509ChainStatusFlags value)
152+
{
153+
_statusFlags = value;
154+
return this;
155+
}
156+
157+
public Builder WithStatusUpdateTime(DateTime? value)
158+
{
159+
_statusUpdateTime = value;
160+
return this;
161+
}
162+
163+
public Builder WithRevocationTime(DateTime? value)
164+
{
165+
_revocationTime = value;
166+
return this;
167+
}
168+
169+
public CertificateVerificationResult Build()
170+
{
171+
return new CertificateVerificationResult(
172+
_status,
173+
_statusFlags,
174+
_statusUpdateTime,
175+
_revocationTime);
176+
}
177+
}
38178
}
39179
}

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

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

106106
public static IEnumerable<object[]> MessageIsConsumedIfValidationEndsGracefullyData()
107107
{
108-
yield return new[] { new CertificateVerificationResult() { Status = EndCertificateStatus.Good } };
108+
yield return new[]
109+
{
110+
new CertificateVerificationResult(
111+
status: EndCertificateStatus.Good,
112+
statusFlags: X509ChainStatusFlags.NoError)
113+
};
109114

110-
yield return new[] { new CertificateVerificationResult() { Status = EndCertificateStatus.Invalid } };
115+
yield return new[]
116+
{
117+
new CertificateVerificationResult(
118+
status: EndCertificateStatus.Invalid,
119+
statusFlags: X509ChainStatusFlags.ExplicitDistrust)
120+
};
111121

112-
yield return new[] { new CertificateVerificationResult() { Status = EndCertificateStatus.Revoked, RevocationTime = DateTime.UtcNow } };
122+
yield return new[]
123+
{
124+
new CertificateVerificationResult(
125+
status: EndCertificateStatus.Revoked,
126+
statusFlags: X509ChainStatusFlags.Revoked,
127+
revocationTime: DateTime.UtcNow)
128+
};
113129
}
114130

115131
[Theory]
@@ -176,13 +192,17 @@ private async Task<bool> HandleUnknownResultAsync(int validationFailuresStart)
176192
}
177193
};
178194

195+
var certificateVerificationResult = new CertificateVerificationResult(
196+
status: EndCertificateStatus.Unknown,
197+
statusFlags: X509ChainStatusFlags.RevocationStatusUnknown);
198+
179199
_certificateValidationService
180200
.Setup(s => s.FindCertificateValidationAsync(It.IsAny<CertificateValidationMessage>()))
181201
.ReturnsAsync(certificateValidation);
182202

183203
_certificateValidationService
184204
.Setup(s => s.VerifyAsync(It.IsAny<X509Certificate2>()))
185-
.ReturnsAsync(new CertificateVerificationResult() { Status = EndCertificateStatus.Unknown });
205+
.ReturnsAsync(certificateVerificationResult);
186206

187207
_certificateValidationService
188208
.Setup(

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

Lines changed: 16 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,9 @@ public class TheTrySaveResultAsyncMethod : FactsBase
8181
public async Task GoodResultUpdatesCertificateValidation()
8282
{
8383
// Arrange
84-
var verificationResult = new CertificateVerificationResult() { Status = EndCertificateStatus.Good };
84+
var verificationResult = new CertificateVerificationResult(
85+
status: EndCertificateStatus.Good,
86+
statusFlags: X509ChainStatusFlags.NoError);
8587

8688
// Act & Assert
8789
var result = await _target.TrySaveResultAsync(_certificateValidation1, verificationResult);
@@ -98,11 +100,9 @@ public async Task InvalidResultInvalidatesDependentSignatures()
98100
{
99101
// Arrange - Invalidate a certificate that is depended on by "signature1"'s certificate.
100102
// This should result in "signature1" being invalidated.
101-
var verificationResult = new CertificateVerificationResult()
102-
{
103-
Status = EndCertificateStatus.Invalid,
104-
StatusFlags = X509ChainStatusFlags.ExplicitDistrust
105-
};
103+
var verificationResult = new CertificateVerificationResult(
104+
status: EndCertificateStatus.Invalid,
105+
statusFlags: X509ChainStatusFlags.ExplicitDistrust);
106106

107107
var signingState = new PackageSigningState { SigningStatus = PackageSigningStatus.Valid };
108108
var signature1 = new PackageSignature { Key = 123, Status = PackageSignatureStatus.Valid };
@@ -157,7 +157,10 @@ public async Task RevokedResultInvalidatesDependentSignatures()
157157
// is a signature that doesn't depend on the certificate.
158158
var revocationTime = DateTime.UtcNow;
159159

160-
var verificationResult = new CertificateVerificationResult() { Status = EndCertificateStatus.Revoked, RevocationTime = revocationTime };
160+
var verificationResult = new CertificateVerificationResult(
161+
status: EndCertificateStatus.Revoked,
162+
statusFlags: X509ChainStatusFlags.Revoked,
163+
revocationTime: revocationTime);
161164

162165
var signingState = new PackageSigningState { SigningStatus = PackageSigningStatus.Valid };
163166
var signature1 = new PackageSignature { Key = 12, Status = PackageSignatureStatus.Valid };
@@ -220,7 +223,9 @@ public async Task RevokedResultInvalidatesDependentSignatures()
220223
public async Task UnknownResultUpdatesCertificateValidation()
221224
{
222225
// Arrange - Create a signature whose certificate and trusted timestamp depends on "_certificateValidation1".
223-
var verificationResult = new CertificateVerificationResult() { Status = EndCertificateStatus.Unknown };
226+
var verificationResult = new CertificateVerificationResult(
227+
status: EndCertificateStatus.Unknown,
228+
statusFlags: X509ChainStatusFlags.RevocationStatusUnknown);
224229

225230
var signature = new PackageSignature { Status = PackageSignatureStatus.Valid };
226231
var timestamp = new TrustedTimestamp { Value = DateTime.UtcNow };
@@ -248,7 +253,9 @@ public async Task UnknownResultUpdatesCertificateValidation()
248253
public async Task UnknownResultAlertsIfReachesMaxFailureThreshold()
249254
{
250255
// Arrange - Create a signature whose certificate and trusted timestamp depends on "_certificateValidation1".
251-
var verificationResult = new CertificateVerificationResult() { Status = EndCertificateStatus.Unknown };
256+
var verificationResult = new CertificateVerificationResult(
257+
status: EndCertificateStatus.Unknown,
258+
statusFlags: X509ChainStatusFlags.RevocationStatusUnknown);
252259

253260
var signature = new PackageSignature { Status = PackageSignatureStatus.Valid };
254261
var timestamp = new TrustedTimestamp { Value = DateTime.UtcNow };

0 commit comments

Comments
 (0)