Fixing Command Line False Positives

CHANGELOG.md

@@ -1,5 +1,13 @@
 # Changelog
 
+## [v1.1.0-pre4](https://github.com/microsoft/CoseSignTool/tree/v1.1.0-pre4) (2023-11-15)
+
+[Full Changelog](https://github.com/microsoft/CoseSignTool/compare/v1.1.0-pre3...v1.1.0-pre4)
+
+**Merged pull requests:**
+
+- update moq dependency to 4.18.4 [\#61](https://github.com/microsoft/CoseSignTool/pull/61) ([JeromySt](https://github.com/JeromySt))
+
 ## [v1.1.0-pre3](https://github.com/microsoft/CoseSignTool/tree/v1.1.0-pre3) (2023-11-15)
 
 [Full Changelog](https://github.com/microsoft/CoseSignTool/compare/v1.1.0-pre2...v1.1.0-pre3)
@@ -35,7 +43,7 @@
 
 ## [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.2...v0.3.1-pre.10)
+[Full Changelog](https://github.com/microsoft/CoseSignTool/compare/v0.3.1-pre.9...v0.3.1-pre.10)
 
 **Merged pull requests:**
 
@@ -45,13 +53,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.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.9...v0.3.2)
+[Full Changelog](https://github.com/microsoft/CoseSignTool/compare/v0.3.2...v0.3.1-pre.9)
 
-## [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.1-pre.8...v0.3.1-pre.9)
+[Full Changelog](https://github.com/microsoft/CoseSignTool/compare/v0.3.1-pre.8...v0.3.2)
 
 **Merged pull requests:**
 

CoseHandler.Tests/CoseSignValidateTests.cs

@@ -263,19 +263,32 @@ public void SelfSigned()
     {
         ReadOnlyMemory<byte> signedBytes = CoseHandler.Sign(Payload1Bytes, SelfSignedCert);
         signedBytes.ToArray().Should().NotBeNull();
-        var result = CoseHandler.Validate(signedBytes.ToArray(), Payload1Bytes, null, RevMode);
+        var result = CoseHandler.Validate(signedBytes.ToArray(), Payload1Bytes, new List<X509Certificate2> { SelfSignedCert }, RevMode);
         result.Success.Should().Be(true);
     }
 
     /// <summary>
-    /// Validate that signing with an untrusted cert causes validation to return ValidationResultTypes.ValidUntrusted
+    /// Validate that signing with an untrusted cert causes validation to fail
     /// </summary>
     [TestMethod]
     public void Untrusted()
     {
         ReadOnlyMemory<byte> signedBytes = CoseHandler.Sign(Payload1Bytes, new X509Certificate2(PrivateKeyCertFileChained));
         signedBytes.ToArray().Should().NotBeNull();
         CoseHandler.Validate(signedBytes.ToArray(), Payload1Bytes, null, RevMode)
+            .Success.Should().Be(false);
+    }
+
+    /// <summary>
+    /// Validate that signing with an untrusted cert causes validation to return ValidationResultTypes.ValidUntrusted
+    /// </summary>
+    [TestMethod]
+    public void UntrustedAllowed()
+    {
+        ReadOnlyMemory<byte> signedBytes = CoseHandler.Sign(Payload1Bytes, new X509Certificate2(PrivateKeyCertFileChained));
+        signedBytes.ToArray().Should().NotBeNull();
+        X509ChainTrustValidator chainValidator = new(revocationMode: RevMode, allowUntrusted: true);
+        CoseHandler.Validate(signedBytes.ToArray(), chainValidator, Payload1Bytes)
             .Success.Should().Be(true);
     }
     #endregion

CoseHandler/CoseHandler.cs

@@ -561,7 +561,8 @@ internal static ValidationResult ValidateInternal(
         // Validate trust of the signing certificate for the message if a CoseSign1MessageValidator was passed.
         if (!validator.TryValidate(msg, out List<CoseSign1ValidationResult> certValidationResults))
         {
-            return new ValidationResult(false, null, certValidationResults);
+            errorCodes.Add(ValidationFailureCode.CertificateChainInvalid);
+            return new ValidationResult(false, errorCodes, certValidationResults);
         }
 
         // Get the signing certificate
@@ -699,7 +700,7 @@ internal static CoseSign1MessageValidator GetValidator(
                 roots,
                 revocationMode,
                 allowUnprotected: true,
-                allowUntrusted: true);
+                allowUntrusted: false);
 
         // If validating CommonName, we'll do that first, and set it to call for chain trust validation when it finishes.
         if (!string.IsNullOrWhiteSpace(requiredCommonName))

CoseHandler/CoseValidationError.cs

@@ -46,5 +46,6 @@ public CoseValidationError(ValidationFailureCode errorCode)
         { ValidationFailureCode.PayloadUnreadable, "The payload content could not be read."},
         { ValidationFailureCode.RedundantPayload, "The embedded signature was not validated because external payload was also specified."},
         { ValidationFailureCode.CoseHeadersInvalid, "The COSE headers in the signature could not be read." },
+        { ValidationFailureCode.Unknown, "An unknown error was thrown." }
     };
 }

CoseHandler/ValidationResult.cs

@@ -88,7 +88,7 @@ public readonly string ToString(bool verbose = false)
         if (!verbose)
         {
             // Print just the header and the top level error messages.
-            return $"{header}{errorBlock}";
+            return $"{header}{errorBlock}{footer}";
         }
 
         // We're in Verbose mode, so get all the Includes from the internal validators.

CoseSignTool.tests/ValidateCommandTests.cs

@@ -1,9 +1,66 @@
 // Copyright (c) Microsoft Corporation.
 // Licensed under the MIT License.
 
-namespace CoseSignTool.tests;
+namespace CoseSignUnitTests;
 
-internal class ValidateCommandTests
+using System;
+using Microsoft.VisualStudio.TestTools.UnitTesting;
+
+using CST = CoseSignTool.CoseSignTool;
+
+[TestClass]
+public class ValidateCommandTests
 {
-    // Placeholder for tests for the ValidateCommand class
+    // Certificates
+    private static readonly X509Certificate2 SelfSignedCert = TestCertificateUtils.CreateCertificate(nameof(CoseHandlerSignValidateTests) + " self signed");    // A self-signed cert
+    private static readonly X509Certificate2Collection CertChain1 = TestCertificateUtils.CreateTestChain(nameof(CoseHandlerSignValidateTests) + " set 1");      // Two complete cert chains
+    private static readonly X509Certificate2Collection CertChain2 = TestCertificateUtils.CreateTestChain(nameof(CoseHandlerSignValidateTests) + " set 2");
+    private static readonly X509Certificate2 Root1Priv = CertChain1[0];                                                                                         // Roots from the chains       
+    private static readonly X509Certificate2 Root2Priv = CertChain2[0];
+    private static readonly X509Certificate2 Int1Priv = CertChain1[1];
+    private static readonly X509Certificate2 Leaf1Priv = CertChain1[^1];                                                                                        // Leaf node certs
+    private static readonly X509Certificate2 Leaf2Priv = CertChain2[^1];
+
+    // File paths to export them to
+    private static readonly string PrivateKeyCertFileSelfSigned = Path.GetTempFileName() + "_SelfSigned.pfx";
+    private static readonly string PublicKeyCertFileSelfSigned = Path.GetTempFileName() + "_SelfSigned.cer";
+    private static readonly string PrivateKeyRootCertFile = Path.GetTempFileName() + ".pfx";
+    private static readonly string PublicKeyIntermediateCertFile = Path.GetTempFileName() + ".cer";
+    private static readonly string PublicKeyRootCertFile = Path.GetTempFileName() + ".cer";
+    private static readonly string PrivateKeyCertFileChained = Path.GetTempFileName() + ".pfx";
+    private static readonly string PrivateKeyCertFileChainedWithPassword = Path.GetTempFileName() + ".pfx";
+    private static readonly string CertPassword = Guid.NewGuid().ToString();
+
+
+    private static string PayloadFile;
+    private static readonly byte[] Payload1Bytes = Encoding.ASCII.GetBytes("Payload1!");
+
+    public ValidateCommandTests()
+    {
+        // make payload file
+        PayloadFile = Path.GetTempFileName();
+        File.WriteAllBytes(PayloadFile, Payload1Bytes);
+
+        // export generated certs to files
+        File.WriteAllBytes(PrivateKeyCertFileSelfSigned, SelfSignedCert.Export(X509ContentType.Pkcs12));
+        File.WriteAllBytes(PublicKeyCertFileSelfSigned, SelfSignedCert.Export(X509ContentType.Cert));
+        File.WriteAllBytes(PrivateKeyRootCertFile, Root1Priv.Export(X509ContentType.Pkcs12));
+        File.WriteAllBytes(PublicKeyRootCertFile, Root1Priv.Export(X509ContentType.Cert));
+        File.WriteAllBytes(PublicKeyIntermediateCertFile, Int1Priv.Export(X509ContentType.Cert));
+        File.WriteAllBytes(PrivateKeyCertFileChained, Leaf1Priv.Export(X509ContentType.Pkcs12));
+        File.WriteAllBytes(PrivateKeyCertFileChainedWithPassword, Leaf1Priv.Export(X509ContentType.Pkcs12, CertPassword));
+    }
+
+    // Validates that signatures made from untrusted chains are rejected
+    [TestMethod]
+    public void ValidateTrustTest()
+    {
+        // sign detached
+        string[] args1 = { "sign", @"/p", PayloadFile, @"/pfx", PrivateKeyCertFileChained };
+        CST.Main(args1).Should().Be((int)ExitCode.Success, "Detach sign failed.");
+
+        // validate without the root cert installed or passed in
+        string[] args2 = { "validate", @"/payload", PayloadFile, @"/sf", PayloadFile + ".cose" };
+        CST.Main(args2).Should().Be((int)ExitCode.CertificateChainValidationFailure);
+    }
 }

CoseSignTool/ValidateCommand.cs

@@ -133,7 +133,7 @@ public override ExitCode Run()
             Console.Error.WriteLine(result.ToString());
 
             return result.Success ? ExitCode.Success
-                : result.Errors?.Count > 0 ? ErrorMap[result.Errors.FirstOrDefault().ErrorCode]
+                : result.Errors?.Count > 0 || result.Errors is not null ? ErrorMap[result.Errors.FirstOrDefault().ErrorCode]
                 : ExitCode.UnknownError;
         }
         catch (ArgumentException ex)