raise warning if insecure sha-1 certificate fingerprint is passed to …

…mssign and reposign commands (#5932)
NuGet · Aug 6, 2024 · 198ea76 · 198ea76
1 parent a06319d
commit 198ea76
Show file tree

Hide file tree

Showing 9 changed files with 278 additions and 67 deletions.
diff --git a/src/NuGet.Clients/NuGet.MSSigning.Extensions/MSSignAbstract.cs b/src/NuGet.Clients/NuGet.MSSigning.Extensions/MSSignAbstract.cs
@@ -88,7 +88,29 @@ protected CngKey GetPrivateKey(X509Certificate2 cert)
 
         protected X509Certificate2 GetCertificate(X509Certificate2Collection certCollection)
         {
-            var matchingCertCollection = certCollection.Find(X509FindType.FindByThumbprint, CertificateFingerprint, validOnly: false);
+            X509Certificate2Collection matchingCertCollection = [];
+
+            if (CertificateUtility.TryDeduceHashAlgorithm(CertificateFingerprint, out Common.HashAlgorithmName hashAlgorithmName))
+            {
+                if (hashAlgorithmName == Common.HashAlgorithmName.SHA1)
+                {
+                    matchingCertCollection = certCollection.Find(X509FindType.FindByThumbprint, CertificateFingerprint, validOnly: false);
+                }
+                else if (hashAlgorithmName != Common.HashAlgorithmName.Unknown)
+                {
+                    foreach (var cert in certCollection)
+                    {
+                        string actualFingerprint = CertificateUtility.GetHashString(cert, hashAlgorithmName);
+
+                        if (string.Equals(actualFingerprint, CertificateFingerprint, StringComparison.InvariantCultureIgnoreCase))
+                        {
+                            matchingCertCollection.Add(cert);
+                            break;
+                        }
+                    }
+
+                }
+            }
 
             if (matchingCertCollection == null || matchingCertCollection.Count == 0)
             {
@@ -132,7 +154,7 @@ protected void EnsureOutputDirectory()
             }
         }
 
-        protected void ValidateCertificateInputs()
+        protected void ValidateCertificateInputs(ILogger logger)
         {
             if (string.IsNullOrEmpty(CertificateFile))
             {
@@ -157,9 +179,20 @@ protected void ValidateCertificateInputs()
             if (string.IsNullOrEmpty(CertificateFingerprint))
             {
                 throw new ArgumentException(string.Format(CultureInfo.CurrentCulture,
-                        NuGetMSSignCommand.MSSignCommandInvalidArgumentException,
+                        NuGetMSSignCommand.MSSignCommandInvalidCertificateFingerprint,
                         nameof(CertificateFingerprint)));
             }
+            else
+            {
+                if (!CertificateUtility.TryDeduceHashAlgorithm(CertificateFingerprint, out Common.HashAlgorithmName hashAlgorithmName))
+                {
+                    throw new ArgumentException(NuGetMSSignCommand.MSSignCommandInvalidCertificateFingerprint);
+                }
+                else if (hashAlgorithmName == Common.HashAlgorithmName.SHA1)
+                {
+                    logger.Log(LogMessage.CreateWarning(NuGetLogCode.NU3043, NuGetMSSignCommand.MSSignCommandInvalidCertificateFingerprint));
+                }
+            }
         }
 
         protected Common.HashAlgorithmName ValidateAndParseHashAlgorithm(string value, string name, SigningSpecifications spec)

diff --git a/src/NuGet.Clients/NuGet.MSSigning.Extensions/MSSignCommand.cs b/src/NuGet.Clients/NuGet.MSSigning.Extensions/MSSignCommand.cs
@@ -40,7 +40,7 @@ public AuthorSignPackageRequest GetAuthorSignRequest()
         {
             ValidatePackagePath();
             WarnIfNoTimestamper(Console);
-            ValidateCertificateInputs();
+            ValidateCertificateInputs(Console);
             EnsureOutputDirectory();
 
             var signingSpec = SigningSpecifications.V1;

diff --git a/src/NuGet.Clients/NuGet.MSSigning.Extensions/NuGetMSSignCommand.Designer.cs b/src/NuGet.Clients/NuGet.MSSigning.Extensions/NuGetMSSignCommand.Designer.cs
diff --git a/src/NuGet.Clients/NuGet.MSSigning.Extensions/NuGetMSSignCommand.resx b/src/NuGet.Clients/NuGet.MSSigning.Extensions/NuGetMSSignCommand.resx
@@ -121,7 +121,7 @@
     <value>File path to the p7b file to be used while signing the package.</value>
   </data>
   <data name="MSSignCommandCertificateFingerprintDescription" xml:space="preserve">
-    <value>SHA-1 fingerprint of the certificate used to search a p7b file for the certificate.</value>
+    <value>SHA-256, SHA-384 or SHA-512 fingerprint of the certificate used to search a p7b file for the certificate.</value>
   </data>
   <data name="MSSignCommandCSPNameDescription" xml:space="preserve">
     <value>Name of the cryptographic service provider which contains the private key container.</value>
@@ -133,7 +133,7 @@
     <value>Hash algorithm to be used while generating the package manifest file. Defaults to SHA256.</value>
   </data>
   <data name="MSSignCommandInvalidArgumentException" xml:space="preserve">
-    <value>Invalid value provided for '{0}'. For a list of accepted values, please visit https://docs.nuget.org/docs/reference/command-line-reference</value>
+    <value>Invalid value provided for '{0}'. For a list of accepted values, please visit https://learn.microsoft.com/nuget/reference/cli-reference/cli-ref-sign</value>
     <comment>{0} - argument name</comment>
   </data>
   <data name="MSSignCommandInvalidCngKeyException" xml:space="preserve">
@@ -202,4 +202,7 @@ nuget reposign MyPackage.nupkg -Timestamper http://timestamper.test -Certificate
   <data name="RepoSignCommandV3ServiceIndexUrlDescription" xml:space="preserve">
     <value>Repository V3 service index URL.</value>
   </data>
+  <data name="MSSignCommandInvalidCertificateFingerprint" xml:space="preserve">
+    <value>Invalid value for 'CertificateFingerprint' option. The value must be a SHA-256, SHA-384, or SHA-512 certificate fingerprint (in hexadecimal).</value>
+  </data>
 </root>
diff --git a/src/NuGet.Clients/NuGet.MSSigning.Extensions/RepoSignCommand.cs b/src/NuGet.Clients/NuGet.MSSigning.Extensions/RepoSignCommand.cs
@@ -53,7 +53,7 @@ public RepositorySignPackageRequest GetRepositorySignRequest()
         {
             ValidatePackagePath();
             WarnIfNoTimestamper(Console);
-            ValidateCertificateInputs();
+            ValidateCertificateInputs(Console);
             EnsureOutputDirectory();
             ValidatePackageOwners();
 

diff --git a/...uGet.Clients.FuncTests/NuGet.MSSigning.Extensions.FuncTest/Commands/MSSignCommandTests.cs b/...uGet.Clients.FuncTests/NuGet.MSSigning.Extensions.FuncTest/Commands/MSSignCommandTests.cs
@@ -24,12 +24,15 @@ namespace NuGet.MSSigning.Extensions.FuncTest.Commands
     public class MSSignCommandTests
     {
         private readonly string _noTimestamperWarningCode = NuGetLogCode.NU3002.ToString();
+        private readonly string _invalidCertificateFingerprintCode = NuGetLogCode.NU3043.ToString();
+        private const string Sha1Hash = "89967D1DD995010B6C66AE24FF8E66885E6E03A8";
+        private const string Sha256Hash = "a591a6d40bf420404a011733cfb7b190d62c65bf0bcda32b55b046cbb7f506fb";
 
-        private TrustedTestCert<TestCertificate> _trustedTestCertWithPrivateKey;
-        private TrustedTestCert<TestCertificate> _trustedTestCertWithoutPrivateKey;
+        private readonly TrustedTestCert<TestCertificate> _trustedTestCertWithPrivateKey;
+        private readonly TrustedTestCert<TestCertificate> _trustedTestCertWithoutPrivateKey;
 
         private MSSignCommandTestFixture _testFixture;
-        private string _nugetExePath;
+        private readonly string _nugetExePath;
 
         public MSSignCommandTests(MSSignCommandTestFixture fixture)
         {
@@ -120,8 +123,10 @@ public void GetAuthorSignRequest_InvalidKeyContainer()
             }
         }
 
-        [CIOnlyFact]
-        public void GetAuthorSignRequest_InvalidFingerprint()
+        [CIOnlyTheory]
+        [InlineData(Sha1Hash)]
+        [InlineData(Sha256Hash)]
+        public void GetAuthorSignRequest_InvalidFingerprint(string certificateFingerPrint)
         {
             var mockConsole = new Mock<IConsole>();
             var timestampUri = "http://timestamp.test/url";
@@ -137,7 +142,7 @@ public void GetAuthorSignRequest_InvalidFingerprint()
                     CertificateFile = test.CertificatePath,
                     CSPName = test.CertificateCSPName,
                     KeyContainer = test.CertificateKeyContainer,
-                    CertificateFingerprint = "invalid-fingerprint",
+                    CertificateFingerprint = certificateFingerPrint,
                 };
                 signCommand.Arguments.Add(Path.Combine(dir, "package.nupkg"));
 
@@ -281,5 +286,48 @@ public async Task MSSignCommand_ResignPackageWithOverwriteSuccessAsync()
                 result.AllOutput.Should().Contain(_noTimestamperWarningCode);
             }
         }
+
+        [CIOnlyFact]
+        public async Task MSSignCommand_SignPackageWithSHA1CertificateFingerprint_Raises_WarningAsync()
+        {
+            var result = await ExecuteMSSignCommandAsync(Common.HashAlgorithmName.SHA1);
+
+            result.Success.Should().BeTrue();
+            result.AllOutput.Should().Contain(_invalidCertificateFingerprintCode);
+        }
+
+        [CIOnlyTheory]
+        [InlineData(Common.HashAlgorithmName.SHA256)]
+        [InlineData(Common.HashAlgorithmName.SHA384)]
+        [InlineData(Common.HashAlgorithmName.SHA512)]
+        public async Task MSSignCommand_SignPackageWithSecureCertificateFingerprint_SucceedsAsync(Common.HashAlgorithmName hashAlgorithmName)
+        {
+            var result = await ExecuteMSSignCommandAsync(hashAlgorithmName);
+
+            result.Success.Should().BeTrue();
+            result.AllOutput.Should().NotContain(_invalidCertificateFingerprintCode);
+        }
+
+        private async Task<CommandRunnerResult> ExecuteMSSignCommandAsync(Common.HashAlgorithmName hashAlgorithmName)
+        {
+            var timestampService = await _testFixture.GetDefaultTrustedTimestampServiceAsync();
+            var package = new SimpleTestPackageContext();
+
+            // Arrange
+            using var test = new MSSignCommandTestContext(_trustedTestCertWithPrivateKey.TrustedCert);
+            var unsignedPackageFile = await package.CreateAsFileAsync(test.Directory, Guid.NewGuid().ToString());
+            string certificateFingerprint = hashAlgorithmName == Common.HashAlgorithmName.SHA1
+                ? test.Cert.Thumbprint
+                : SignatureTestUtility.GetFingerprint(test.Cert, hashAlgorithmName);
+            var command = $"mssign {unsignedPackageFile} -Timestamper {timestampService.Url} -CertificateFile {test.CertificatePath} -CSPName \"{test.CertificateCSPName}\" -KeyContainer \"{test.CertificateKeyContainer}\" -CertificateFingerprint {certificateFingerprint}";
+
+            var result = CommandRunner.Run(
+                _nugetExePath,
+                test.Directory,
+                command);
+
+            return result;
+        }
+
     }
 }
diff --git a/...et.Clients.FuncTests/NuGet.MSSigning.Extensions.FuncTest/Commands/ReposignCommandTests.cs b/...et.Clients.FuncTests/NuGet.MSSigning.Extensions.FuncTest/Commands/ReposignCommandTests.cs
@@ -24,12 +24,15 @@ namespace NuGet.MSSigning.Extensions.FuncTest.Commands
     public class ReposignCommandTests
     {
         private readonly string _noTimestamperWarningCode = NuGetLogCode.NU3002.ToString();
+        private readonly string _invalidCertificateFingerprintCode = NuGetLogCode.NU3043.ToString();
+        private const string Sha1Hash = "89967D1DD995010B6C66AE24FF8E66885E6E03A8";
+        private const string Sha256Hash = "a591a6d40bf420404a011733cfb7b190d62c65bf0bcda32b55b046cbb7f506fb";
 
-        private TrustedTestCert<TestCertificate> _trustedTestCertWithPrivateKey;
-        private TrustedTestCert<TestCertificate> _trustedTestCertWithoutPrivateKey;
+        private readonly TrustedTestCert<TestCertificate> _trustedTestCertWithPrivateKey;
+        private readonly TrustedTestCert<TestCertificate> _trustedTestCertWithoutPrivateKey;
 
         private MSSignCommandTestFixture _testFixture;
-        private string _nugetExePath;
+        private readonly string _nugetExePath;
 
         public ReposignCommandTests(MSSignCommandTestFixture fixture)
         {
@@ -126,8 +129,10 @@ public void GetRepositorySignRequest_InvalidKeyContainer()
             }
         }
 
-        [CIOnlyFact]
-        public void GetRepositorySignRequest_InvalidFingerprint()
+        [CIOnlyTheory]
+        [InlineData(Sha1Hash)]
+        [InlineData(Sha256Hash)]
+        public void GetRepositorySignRequest_InvalidFingerprint(string certificateFingerPrint)
         {
             var mockConsole = new Mock<IConsole>();
             var v3serviceIndex = "https://v3serviceindex.test/api/index.json";
@@ -144,7 +149,7 @@ public void GetRepositorySignRequest_InvalidFingerprint()
                     CertificateFile = test.CertificatePath,
                     CSPName = test.CertificateCSPName,
                     KeyContainer = test.CertificateKeyContainer,
-                    CertificateFingerprint = "invalid-fingerprint",
+                    CertificateFingerprint = certificateFingerPrint,
                     V3ServiceIndexUrl = v3serviceIndex,
                 };
                 reposignCommand.Arguments.Add(Path.Combine(dir, "package.nupkg"));
@@ -362,5 +367,47 @@ public async Task ReposignCommand_Countersign_RepositoryCountersignedPackage_Fai
                 result.Errors.Should().Contain("NU3032: The package already contains a repository countersignature. Please remove the existing signature before adding a new repository countersignature.");
             }
         }
+
+        [CIOnlyFact]
+        public async Task RepoSignCommand_SignPackageWithSHA1CertificateFingerprint_Raises_WarningAsync()
+        {
+            var result = await ExecuteRepoSignCommandAsync(Common.HashAlgorithmName.SHA1);
+
+            result.Success.Should().BeTrue();
+            result.AllOutput.Should().Contain(_invalidCertificateFingerprintCode);
+        }
+
+        [CIOnlyTheory]
+        [InlineData(Common.HashAlgorithmName.SHA256)]
+        [InlineData(Common.HashAlgorithmName.SHA384)]
+        [InlineData(Common.HashAlgorithmName.SHA512)]
+        public async Task RepoSignCommand_SignPackageWithSecureCertificateFingerprint_SucceedsAsync(Common.HashAlgorithmName hashAlgorithmName)
+        {
+            var result = await ExecuteRepoSignCommandAsync(hashAlgorithmName);
+
+            result.Success.Should().BeTrue();
+            result.AllOutput.Should().NotContain(_invalidCertificateFingerprintCode);
+        }
+
+        private async Task<CommandRunnerResult> ExecuteRepoSignCommandAsync(Common.HashAlgorithmName hashAlgorithmName)
+        {
+            var timestampService = await _testFixture.GetDefaultTrustedTimestampServiceAsync();
+            var package = new SimpleTestPackageContext();
+
+            // Arrange
+            using var test = new MSSignCommandTestContext(_trustedTestCertWithPrivateKey.TrustedCert);
+            var unsignedPackageFile = await package.CreateAsFileAsync(test.Directory, Guid.NewGuid().ToString());
+            string certificateFingerprint = hashAlgorithmName == Common.HashAlgorithmName.SHA1
+                ? test.Cert.Thumbprint
+                : SignatureTestUtility.GetFingerprint(test.Cert, hashAlgorithmName);
+            var command = $"reposign {unsignedPackageFile} -Timestamper {timestampService.Url} -CertificateFile {test.CertificatePath} -CSPName \"{test.CertificateCSPName}\" -KeyContainer \"{test.CertificateKeyContainer}\" -CertificateFingerprint {certificateFingerprint} -V3ServiceIndexUrl https://v3serviceIndex.test/api/index.json";
+
+            var result = CommandRunner.Run(
+                _nugetExePath,
+                test.Directory,
+                command);
+
+            return result;
+        }
     }
 }