Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enabled optional time validation in ChainTrustVaildator #97

Merged
merged 8 commits into from
Aug 5, 2024
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 30 additions & 22 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,12 +1,20 @@
# Changelog

## [v1.2.3-pre7](https://github.com/microsoft/CoseSignTool/tree/v1.2.3-pre7) (2024-06-14)
## [v1.2.4-pre1](https://github.com/microsoft/CoseSignTool/tree/v1.2.4-pre1) (2024-07-15)

[Full Changelog](https://github.com/microsoft/CoseSignTool/compare/v1.2.4...v1.2.4-pre1)

**Merged pull requests:**

[Full Changelog](https://github.com/microsoft/CoseSignTool/compare/v1.2.4...v1.2.3-pre7)
- User/lemccomb/fileread [\#94](https://github.com/microsoft/CoseSignTool/pull/94) ([lemccomb](https://github.com/lemccomb))

## [v1.2.4](https://github.com/microsoft/CoseSignTool/tree/v1.2.4) (2024-06-14)

[Full Changelog](https://github.com/microsoft/CoseSignTool/compare/v1.2.3-pre6...v1.2.4)
[Full Changelog](https://github.com/microsoft/CoseSignTool/compare/v1.2.3-pre7...v1.2.4)

## [v1.2.3-pre7](https://github.com/microsoft/CoseSignTool/tree/v1.2.3-pre7) (2024-06-14)

[Full Changelog](https://github.com/microsoft/CoseSignTool/compare/v1.2.3-pre6...v1.2.3-pre7)

**Merged pull requests:**

Expand Down Expand Up @@ -170,19 +178,19 @@

## [v1.1.4-pre1](https://github.com/microsoft/CoseSignTool/tree/v1.1.4-pre1) (2024-01-31)

[Full Changelog](https://github.com/microsoft/CoseSignTool/compare/v1.1.3-pre1...v1.1.4-pre1)
[Full Changelog](https://github.com/microsoft/CoseSignTool/compare/v1.1.4...v1.1.4-pre1)

**Merged pull requests:**

- write validation output to standard out [\#74](https://github.com/microsoft/CoseSignTool/pull/74) ([elantiguamsft](https://github.com/elantiguamsft))

## [v1.1.3-pre1](https://github.com/microsoft/CoseSignTool/tree/v1.1.3-pre1) (2024-01-26)
## [v1.1.4](https://github.com/microsoft/CoseSignTool/tree/v1.1.4) (2024-01-26)

[Full Changelog](https://github.com/microsoft/CoseSignTool/compare/v1.1.4...v1.1.3-pre1)
[Full Changelog](https://github.com/microsoft/CoseSignTool/compare/v1.1.3-pre1...v1.1.4)

## [v1.1.4](https://github.com/microsoft/CoseSignTool/tree/v1.1.4) (2024-01-26)
## [v1.1.3-pre1](https://github.com/microsoft/CoseSignTool/tree/v1.1.3-pre1) (2024-01-26)

[Full Changelog](https://github.com/microsoft/CoseSignTool/compare/v1.1.3...v1.1.4)
[Full Changelog](https://github.com/microsoft/CoseSignTool/compare/v1.1.3...v1.1.3-pre1)

**Merged pull requests:**

Expand All @@ -194,19 +202,19 @@

## [v1.1.2-pre1](https://github.com/microsoft/CoseSignTool/tree/v1.1.2-pre1) (2024-01-24)

[Full Changelog](https://github.com/microsoft/CoseSignTool/compare/v1.1.1-pre2...v1.1.2-pre1)
[Full Changelog](https://github.com/microsoft/CoseSignTool/compare/v1.1.2...v1.1.2-pre1)

**Merged pull requests:**

- Updating snk for internal package compatibility [\#72](https://github.com/microsoft/CoseSignTool/pull/72) ([elantiguamsft](https://github.com/elantiguamsft))

## [v1.1.1-pre2](https://github.com/microsoft/CoseSignTool/tree/v1.1.1-pre2) (2024-01-18)
## [v1.1.2](https://github.com/microsoft/CoseSignTool/tree/v1.1.2) (2024-01-18)

[Full Changelog](https://github.com/microsoft/CoseSignTool/compare/v1.1.2...v1.1.1-pre2)
[Full Changelog](https://github.com/microsoft/CoseSignTool/compare/v1.1.1-pre2...v1.1.2)

## [v1.1.2](https://github.com/microsoft/CoseSignTool/tree/v1.1.2) (2024-01-18)
## [v1.1.1-pre2](https://github.com/microsoft/CoseSignTool/tree/v1.1.1-pre2) (2024-01-18)

[Full Changelog](https://github.com/microsoft/CoseSignTool/compare/v1.1.1-pre1...v1.1.2)
[Full Changelog](https://github.com/microsoft/CoseSignTool/compare/v1.1.1-pre1...v1.1.1-pre2)

**Merged pull requests:**

Expand Down Expand Up @@ -283,7 +291,7 @@

## [v1.1.0-pre1](https://github.com/microsoft/CoseSignTool/tree/v1.1.0-pre1) (2023-11-03)

[Full Changelog](https://github.com/microsoft/CoseSignTool/compare/v1.1.0...v1.1.0-pre1)
[Full Changelog](https://github.com/microsoft/CoseSignTool/compare/v0.3.1-pre.10...v1.1.0-pre1)

**Merged pull requests:**

Expand All @@ -293,13 +301,13 @@
- DetachedSignatureFactory accepts pre-hashed content as payload [\#53](https://github.com/microsoft/CoseSignTool/pull/53) ([elantiguamsft](https://github.com/elantiguamsft))
- Add password support for certificate files [\#52](https://github.com/microsoft/CoseSignTool/pull/52) ([lemccomb](https://github.com/lemccomb))

## [v1.1.0](https://github.com/microsoft/CoseSignTool/tree/v1.1.0) (2023-10-10)
## [v0.3.1-pre.10](https://github.com/microsoft/CoseSignTool/tree/v0.3.1-pre.10) (2023-10-10)

[Full Changelog](https://github.com/microsoft/CoseSignTool/compare/v0.3.1-pre.10...v1.1.0)
[Full Changelog](https://github.com/microsoft/CoseSignTool/compare/v1.1.0...v0.3.1-pre.10)

## [v0.3.1-pre.10](https://github.com/microsoft/CoseSignTool/tree/v0.3.1-pre.10) (2023-10-10)
## [v1.1.0](https://github.com/microsoft/CoseSignTool/tree/v1.1.0) (2023-10-10)

[Full Changelog](https://github.com/microsoft/CoseSignTool/compare/v0.3.1-pre.9...v0.3.1-pre.10)
[Full Changelog](https://github.com/microsoft/CoseSignTool/compare/v0.3.2...v1.1.0)

**Merged pull requests:**

Expand All @@ -309,13 +317,13 @@
- Port changes from ADO repo to GitHub repo [\#46](https://github.com/microsoft/CoseSignTool/pull/46) ([lemccomb](https://github.com/lemccomb))
- Re-enable CodeQL [\#45](https://github.com/microsoft/CoseSignTool/pull/45) ([lemccomb](https://github.com/lemccomb))

## [v0.3.1-pre.9](https://github.com/microsoft/CoseSignTool/tree/v0.3.1-pre.9) (2023-09-28)
## [v0.3.2](https://github.com/microsoft/CoseSignTool/tree/v0.3.2) (2023-09-28)

[Full Changelog](https://github.com/microsoft/CoseSignTool/compare/v0.3.2...v0.3.1-pre.9)
[Full Changelog](https://github.com/microsoft/CoseSignTool/compare/v0.3.1-pre.9...v0.3.2)

## [v0.3.2](https://github.com/microsoft/CoseSignTool/tree/v0.3.2) (2023-09-28)
## [v0.3.1-pre.9](https://github.com/microsoft/CoseSignTool/tree/v0.3.1-pre.9) (2023-09-28)

[Full Changelog](https://github.com/microsoft/CoseSignTool/compare/v0.3.1-pre.8...v0.3.2)
[Full Changelog](https://github.com/microsoft/CoseSignTool/compare/v0.3.1-pre.8...v0.3.1-pre.9)

**Merged pull requests:**

Expand Down
2 changes: 1 addition & 1 deletion CoseHandler/ValidationResult.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright (c) Microsoft Corporation.
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT License.

namespace CoseX509;
Expand Down
2 changes: 1 addition & 1 deletion CoseSign1.Abstractions/CoseSign1MessageValidator.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright (c) Microsoft Corporation.
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT License.

namespace CoseSign1.Abstractions;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright (c) Microsoft Corporation.
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT License.

namespace CoseSign1.Certificates.Tests;
Expand Down
27 changes: 26 additions & 1 deletion CoseSign1.Certificates.Tests/X509ChainTrustValidatorTests.cs
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
// Copyright (c) Microsoft Corporation.
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT License.

namespace CoseSign1.Certificates.Tests;

using System.Threading;

/// <summary>
/// Test class for <see cref="X509ChainTrustValidator"/>
/// </summary>
Expand Down Expand Up @@ -341,6 +343,29 @@ public void X509TrustValidatorSelfSigned()
results[0].Includes?.Count.Should().Be(1);
X509ChainStatus? status = results[0].Includes?.Cast<X509ChainStatus>().FirstOrDefault();
status.Value.Status.Should().Be(X509ChainStatusFlags.UntrustedRoot);

// Validate with AllowUntrusted AND AllowOutdated and an expired certificate
var cert2 = TestCertificateUtils.CreateCertificate("X509TrustValidatorSelfSigned-Expired", null, false, null, TimeSpan.FromSeconds(1));
Thread.Sleep(1000);
X509Certificate2CoseSigningKeyProvider keyProvider2 = new(null, cert2);
var message2 = factory.CreateCoseSign1Message(DefaultTestArray, keyProvider2, embedPayload: true);
Validator = new(X509RevocationMode.NoCheck, false, allowUntrusted: true, allowOutdated: true);
Validator.TryValidate(message2, out List<CoseSign1ValidationResult> results2).Should().BeTrue();
results2.Count.Should().Be(1);
results2[0].PassedValidation.Should().BeTrue();
results2[0].ResultMessage.Should().Contain("AllowOutdated and AllowUntrusted");
results2[0].Includes.Should().BeNull();

// Validate with AllowOutdated OFF and an expired certificate
Validator = new(X509RevocationMode.NoCheck, false, allowUntrusted: true);
Validator.TryValidate(message2, out results2).Should().BeFalse();
results2.Count.Should().Be(1);
results2[0].PassedValidation.Should().BeFalse();
results2[0].Includes.Should().NotBeNull();
results2[0].Includes?.Count.Should().Be(2);
status = results2[0].Includes?.Cast<X509ChainStatus>()
.FirstOrDefault(s => s.Status == X509ChainStatusFlags.NotTimeValid);
status.Should().NotBeNull();
}
lemccomb marked this conversation as resolved.
Show resolved Hide resolved

// Check outputs for miscellaneous error cases.
Expand Down
2 changes: 1 addition & 1 deletion CoseSign1.Certificates/CertificateCoseHeaderLabels.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright (c) Microsoft Corporation.
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT License.

namespace CoseSign1.Certificates;
Expand Down
75 changes: 46 additions & 29 deletions CoseSign1.Certificates/Local/Validators/X509ChainTrustValidator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,6 @@

namespace CoseSign1.Certificates.Local.Validators;

using System.Threading;

/// <summary>
/// Validation chain element for verifying a <see cref="CoseSign1Message"/> is signed by a trusted <see cref="X509Certifiate2"/>
/// </summary>
Expand All @@ -15,11 +13,15 @@ namespace CoseSign1.Certificates.Local.Validators;
/// <param name="chainBuilder">The <see cref="ICertificateChainBuilder"/> used to build a chain.</param>
/// <param name="allowUnprotected">True if the UnprotectedHeaders is allowed, False otherwise.</param>
/// <param name="allowUntrusted">True to allow untrusted certificates.</param>
/// <param name="allowOutdated">True to allow signatures with expired certificates to pass validation unless the expired certificate has a lifetime EKU.</param>
public class X509ChainTrustValidator(
ICertificateChainBuilder chainBuilder,
bool allowUnprotected = false,
bool allowUntrusted = false) : X509Certificate2MessageValidator(allowUnprotected)
bool allowUntrusted = false,
bool allowOutdated = false) : X509Certificate2MessageValidator(allowUnprotected)
{
private const string LifetimeEkuOidValue = "1.3.6.1.4.1.311.10.3.13";

#region Public Properties
/// <summary>
/// True to allow untrusted certificates to pass validation. This does not apply to self-signed certificates, which trust themselves.
Expand All @@ -45,7 +47,6 @@ public class X509ChainTrustValidator(
/// Assumes that user-supplied roots are trusted. True by default.
/// </summary>
public bool TrustUserRoots { get; set; } = true;

#endregion
#region Constructors

Expand All @@ -59,11 +60,13 @@ public class X509ChainTrustValidator(
public X509ChainTrustValidator(
X509RevocationMode revocationMode = X509RevocationMode.Online,
bool allowUnprotected = false,
bool allowUntrusted = false) :
bool allowUntrusted = false,
bool allowOutdated = false):
this(
new X509ChainBuilder() { ChainPolicy = new X509ChainPolicy() { RevocationMode = revocationMode } },
allowUnprotected,
allowUntrusted)
allowUntrusted,
allowOutdated)
{
}

Expand All @@ -79,11 +82,13 @@ public X509ChainTrustValidator(
List<X509Certificate2>? roots,
X509RevocationMode revocationMode = X509RevocationMode.Online,
bool allowUnprotected = false,
bool allowUntrusted = false) :
bool allowUntrusted = false,
bool allowOutdated = false) :
this(
new X509ChainBuilder() { ChainPolicy = new X509ChainPolicy() { RevocationMode = revocationMode } },
allowUnprotected,
allowUntrusted)
allowUntrusted,
allowOutdated)
{
Roots = roots;
}
Expand Down Expand Up @@ -129,32 +134,44 @@ protected override CoseSign1ValidationResult ValidateCertificate(
}
}

// Chain build failed, but if the only failure is Untrusted Root we may still pass.
if (ChainBuilder.ChainStatus.All(st => (st.Status &~ (X509ChainStatusFlags.UntrustedRoot | X509ChainStatusFlags.NoError)) == 0)) // use &~ to mask out UntrustedRoot and NoError
// If we're here, chain build failed.
// This is the result of building the certificate chain.
CoseSign1ValidationResult baseResult = new (GetType(), false,
$"[{string.Join("][", ChainBuilder.ChainStatus.Select(cs => cs.StatusInformation).ToArray())}]",
ChainBuilder.ChainStatus.Cast<object>().ToList());

// Ignore failures from untrusted roots or expired certificates if the user tells us to.
X509ChainStatusFlags flagsToIgnore = X509ChainStatusFlags.NoError;
flagsToIgnore |= AllowUntrusted ? X509ChainStatusFlags.UntrustedRoot : 0;

// If we have a valid user-supplied root, consider it trusted. (Not supported by .NET Standard 2.0 so we have to do it ourselves.)
string chainRootThumb = ChainBuilder.ChainElements.FirstOrDefault(element => element.Subject.Equals(element.Issuer))?.Thumbprint ?? string.Empty;
bool trustUserRoot = hasRoots && TrustUserRoots && !string.IsNullOrEmpty(chainRootThumb) && Roots!.Any(r => r.Thumbprint == chainRootThumb);
flagsToIgnore |= trustUserRoot ? X509ChainStatusFlags.UntrustedRoot : 0;

// If allowOutdated is set and none of the outdated certificates in the chain have a lifetime EKU, ignore NotTimeValid.
List<X509Certificate2>? outdatedCerts = ChainBuilder.ChainElements?.Where(c => c.NotAfter < DateTime.Now).ToList();
bool expired = outdatedCerts?.Any(c => c.Extensions.OfType<X509Extension>().Any(e => e.Oid?.Value == LifetimeEkuOidValue)) ?? false;
lemccomb marked this conversation as resolved.
Show resolved Hide resolved
flagsToIgnore |= allowOutdated && !expired ? X509ChainStatusFlags.NotTimeValid : 0;

// If we only have the allowed chain status messages, return success.
if (ChainBuilder.ChainStatus.All(st => (st.Status &~ flagsToIgnore) == 0)) // use &~ to mask out UntrustedRoot and NoError
{
// We can't specify an alternative root in .netstandard 2.0, so our work-around is to consider any user-supplied roots trusted.
// This logic should be replaced once the library updates to a .NET Standard version that supports assigning arbitrary root trust like .NET 7 does.
string chainRootThumb = ChainBuilder.ChainElements.FirstOrDefault(element => element.Subject.Equals(element.Issuer))?.Thumbprint ?? string.Empty;
if (hasRoots
&& TrustUserRoots
&& !string.IsNullOrEmpty(chainRootThumb)
&& Roots.Any(r => r.Thumbprint == chainRootThumb))
{
// The root of the chain is one of the user-supplied roots, so return success.
return new CoseSign1ValidationResult(GetType(), true, "Certificate was Trusted.");
}
bool resultUntrusted = ChainBuilder.ChainStatus.Any(s => s.Status.HasFlag(X509ChainStatusFlags.UntrustedRoot));
bool untrustedAndAllowed = resultUntrusted && AllowUntrusted && !trustUserRoot;
bool outdatedAndAllowed = ChainBuilder.ChainStatus.Any(s => s.Status.HasFlag(X509ChainStatusFlags.NotTimeValid)) && allowOutdated;

if (AllowUntrusted)
{
// The root was untrusted but the user allowed it.
return new CoseSign1ValidationResult(GetType(), true, "Certificate was allowed because AllowUntrusted was specified.");
}
string message =
outdatedAndAllowed && untrustedAndAllowed ? "Certificate was allowed because AllowOutdated and AllowUntrusted were both specified." :
outdatedAndAllowed ? "Certificate was allowed because AllowOutdated was specified." :
resultUntrusted && trustUserRoot ? "Certificate was Trusted." :
"Certificate was allowed because AllowUntrusted was specified." ;

return new CoseSign1ValidationResult(GetType(), true, message);
}

// Validation failed.
return new CoseSign1ValidationResult(GetType(), false,
$"[{string.Join("][", ChainBuilder.ChainStatus.Select(cs => cs.StatusInformation).ToArray())}]",
ChainBuilder.ChainStatus.Cast<object>().ToList());
return baseResult;
}
#endregion
}
3 changes: 2 additions & 1 deletion CoseSign1.Certificates/Usings.cs
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,11 @@
global using System.Security.Cryptography;
global using System.Security.Cryptography.Cose;
global using System.Security.Cryptography.X509Certificates;
global using System.Threading;
global using CoseSign1.Abstractions;
global using CoseSign1.Abstractions.Exceptions;
global using CoseSign1.Abstractions.Interfaces;
global using CoseSign1.Certificates.Exceptions;
global using CoseSign1.Certificates.Extensions;
global using CoseSign1.Certificates.Interfaces;
global using CoseSign1.Certificates.Local;
global using CoseSign1.Certificates.Local;
14 changes: 7 additions & 7 deletions CoseSign1.Tests.Common/TestCertificateUtils.cs
Original file line number Diff line number Diff line change
Expand Up @@ -18,19 +18,17 @@ public static class TestCertificateUtils
/// <param name="issuingCa">(Optional) The issuing CA if present to sign this certificate, self-signed otherwise.</param>
/// <param name="useEcc">(Optional) True for ECC certificates, false (default) for RSA certificates.</param>
/// <param name="keySize">(Optional) The optional key size for the cert being created.</param>
/// <param name="lifetime">(Optional) How long the certificate should be valid for after it is created. Default value is one year.</param>
/// <returns>An <see cref="X509Certificate2"/> object for use in testing.</returns>
/// <exception cref="ArgumentOutOfRangeException"></exception>
public static X509Certificate2 CreateCertificate(
[CallerMemberName] string subjectName = "none",
X509Certificate2? issuingCa = null,
bool useEcc = false,
int? keySize = null)
int? keySize = null,
TimeSpan? lifetime = null)
{
using AsymmetricAlgorithm? algo = useEcc ? ECDsa.Create() : RSA.Create();
if (algo == null)
{
throw new ArgumentOutOfRangeException(nameof(algo), "algo was null after creation");
}
using AsymmetricAlgorithm algo = useEcc ? ECDsa.Create() : RSA.Create();
algo.KeySize = keySize ?? (useEcc ? 256 : 2048);

CertificateRequest request = useEcc
Expand Down Expand Up @@ -105,7 +103,9 @@ public static X509Certificate2 CreateCertificate(
{
notbefore = new DateTimeOffset(issuingCa.NotBefore);
}
DateTimeOffset notafter = DateTimeOffset.UtcNow.AddDays(365);
DateTimeOffset notafter =
lifetime is not null ? DateTimeOffset.UtcNow.Add(lifetime.Value) :
DateTimeOffset.UtcNow.AddDays(365);
if (issuingCa != null && notafter > issuingCa.NotAfter)
{
notafter = new DateTimeOffset(issuingCa.NotAfter);
Expand Down
Loading