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

…nuget.exe sign command (#5924)

src/NuGet.Clients/NuGet.CommandLine/Commands/SignCommand.cs

@@ -4,7 +4,6 @@
 using System;
 using System.Globalization;
 using System.IO;
-using System.Linq;
 using System.Security.Cryptography.X509Certificates;
 using System.Threading.Tasks;
 using NuGet.Commands;
@@ -79,7 +78,7 @@ public SignArgs GetSignArgs()
         {
             ValidatePackagePath();
             WarnIfNoTimestamper(Console);
-            ValidateCertificateInputs();
+            ValidateCertificateInputs(Console);
             ValidateOutputDirectory();
 
             var signingSpec = SigningSpecifications.V1;
@@ -165,7 +164,7 @@ private void ValidateOutputDirectory()
             }
         }
 
-        private void ValidateCertificateInputs()
+        private void ValidateCertificateInputs(ILogger logger)
         {
             if (string.IsNullOrEmpty(CertificatePath) &&
                 string.IsNullOrEmpty(CertificateFingerprint) &&
@@ -180,14 +179,25 @@ private void ValidateCertificateInputs()
                  !string.IsNullOrEmpty(CertificateStoreLocation) ||
                  !string.IsNullOrEmpty(CertificateStoreName)))
             {
-                // Thow if the user provided a path and any one of the other options
+                // Throw if the user provided a path and any one of the other options
                 throw new ArgumentException(NuGetCommand.SignCommandMultipleCertificateException);
             }
             else if (!string.IsNullOrEmpty(CertificateFingerprint) && !string.IsNullOrEmpty(CertificateSubjectName))
             {
-                // Thow if the user provided a fingerprint and a subject
+                // Throw if the user provided a fingerprint and a subject
                 throw new ArgumentException(NuGetCommand.SignCommandMultipleCertificateException);
             }
+            else if (CertificateFingerprint != null)
+            {
+                if (!CertificateUtility.TryDeduceHashAlgorithm(CertificateFingerprint, out HashAlgorithmName hashAlgorithmName))
+                {
+                    throw new ArgumentException(NuGetCommand.SignCommandInvalidCertificateFingerprint);
+                }
+                else if (hashAlgorithmName == HashAlgorithmName.SHA1)
+                {
+                    logger.Log(LogMessage.CreateWarning(NuGetLogCode.NU3043, NuGetCommand.SignCommandInvalidCertificateFingerprint));
+                }
+            }
         }
     }
 }

src/NuGet.Clients/NuGet.CommandLine/NuGetCommand.resx

@@ -678,7 +678,7 @@ This option can be used to specify the password for the certificate. If no passw
     <value>File path to the certificate to be used while signing the package.</value>
   </data>
   <data name="SignCommandCertificateFingerprintDescription" xml:space="preserve">
-    <value>SHA-1 fingerprint of the certificate used to search a local certificate store for the certificate.
+    <value>SHA-256, SHA-384 or SHA-512 fingerprint of the certificate used to search a local certificate store for the certificate.
 The certificate store can be specified by -CertificateStoreName and -CertificateStoreLocation options.</value>
   </data>
   <data name="SignCommandCertificateSubjectNameDescription" xml:space="preserve">
@@ -937,4 +937,7 @@ nuget client-certs List</value>
   <data name="SourcesCommandAllowInsecureConnectionsDescription" xml:space="preserve">
     <value>Allows HTTP connections for adding or updating packages. Note: This method is not secure. For secure options, see https://aka.ms/nuget-https-everywhere for more information.</value>
   </data>
+  <data name="SignCommandInvalidCertificateFingerprint" 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>

test/NuGet.Clients.FuncTests/NuGet.CommandLine.FuncTest/Commands/SignCommandTests.cs

@@ -13,6 +13,7 @@
 using Test.Utility.Signing;
 using Xunit;
 using Xunit.Abstractions;
+using HashAlgorithmName = NuGet.Common.HashAlgorithmName;
 
 namespace NuGet.CommandLine.FuncTest.Commands
 {
@@ -29,6 +30,7 @@ public class SignCommandTests
         private readonly string _noCertFoundErrorCode = NuGetLogCode.NU3001.ToString();
         private readonly string _noTimestamperWarningCode = NuGetLogCode.NU3002.ToString();
         private readonly string _timestampUnsupportedDigestAlgorithmCode = NuGetLogCode.NU3024.ToString();
+        private readonly string _insecureCertificateFingerprintCode = NuGetLogCode.NU3043.ToString();
 
         private SignCommandTestFixture _testFixture;
         private readonly ITestOutputHelper _testOutputHelper;
@@ -717,5 +719,61 @@ public async Task SignCommand_SignPackageWithUnsuportedTimestampHashAlgorithm_Sh
                 }
             }
         }
+
+        [CIOnlyFact]
+        public async Task SignCommand_SignPackageWithInsecureCertificateFingerprint_RaisesWarningAsync()
+        {
+            await ExecuteSignPackageTestWithCertificateFingerprintAsync(HashAlgorithmName.SHA1, expectInsecureFingerprintWarning: true);
+        }
+
+        [CIOnlyTheory]
+        [InlineData(HashAlgorithmName.SHA256)]
+        [InlineData(HashAlgorithmName.SHA384)]
+        [InlineData(HashAlgorithmName.SHA512)]
+        public async Task SignCommand_SignPackageWithSecureCertificateFingerprint_SucceedsAsync(HashAlgorithmName hashAlgorithmName)
+        {
+            await ExecuteSignPackageTestWithCertificateFingerprintAsync(hashAlgorithmName, expectInsecureFingerprintWarning: false);
+        }
+
+        private async Task ExecuteSignPackageTestWithCertificateFingerprintAsync(
+            HashAlgorithmName hashAlgorithmName,
+            bool expectInsecureFingerprintWarning)
+        {
+            // Arrange
+            var testServer = await _testFixture.GetSigningTestServerAsync();
+            var certificateAuthority = await _testFixture.GetDefaultTrustedCertificateAuthorityAsync();
+            var options = new TimestampServiceOptions() { SignatureHashAlgorithm = new Oid(Oids.Sha256) };
+            var timestampService = TimestampService.Create(certificateAuthority, options);
+
+            using (testServer.RegisterResponder(timestampService))
+            using (var directory = TestDirectory.Create())
+            {
+                var packageContext = new SimpleTestPackageContext();
+                var packageFile = await packageContext.CreateAsFileAsync(directory, fileName: Guid.NewGuid().ToString());
+                var originalFile = File.ReadAllBytes(packageFile.FullName);
+
+                using var certificate = _testFixture.UntrustedSelfIssuedCertificateInCertificateStore;
+
+                string certFingerprint = expectInsecureFingerprintWarning ? certificate.Thumbprint :
+                    SignatureTestUtility.GetFingerprint(certificate, hashAlgorithmName);
+
+                var result = CommandRunner.Run(
+                    _nugetExePath,
+                    directory,
+                    $"sign {packageFile.FullName} -CertificateFingerprint {certFingerprint} -Timestamper {timestampService.Url}",
+                testOutputHelper: _testOutputHelper);
+
+                // Assert
+                Assert.True(result.Success, result.AllOutput);
+                if (expectInsecureFingerprintWarning)
+                {
+                    Assert.True(result.AllOutput.Contains(_insecureCertificateFingerprintCode), result.AllOutput);
+                }
+                else
+                {
+                    Assert.False(result.AllOutput.Contains(_insecureCertificateFingerprintCode), result.AllOutput);
+                }
+            }
+        }
     }
 }

test/NuGet.Clients.Tests/NuGet.CommandLine.Test/NuGetSignCommandTest.cs

@@ -3,6 +3,7 @@
 
 using System;
 using System.Security.Cryptography.X509Certificates;
+using System.Text;
 using Moq;
 using NuGet.Commands;
 using NuGet.Common;
@@ -18,6 +19,7 @@ public class NuGetSignCommandTest
         private const string _multipleCertificateException = "Multiple options were used to specify a certificate. For a list of accepted ways to provide a certificate, please visit https://docs.nuget.org/docs/reference/command-line-reference";
         private const string _noCertificateException = "No certificate was provided. For a list of accepted ways to provide a certificate, please visit https://docs.nuget.org/docs/reference/command-line-reference";
         private const string _missingArgumentException = "No value provided for '{0}', which is needed when using the '{1}' option. For a list of accepted values, please visit https://docs.nuget.org/docs/reference/command-line-reference";
+        private const string _sha1Hash = "89967D1DD995010B6C66AE24FF8E66885E6E03A8";
 
         [Fact]
         public void SignCommandArgParsing_NoPackagePath()
@@ -82,7 +84,7 @@ public void SignCommandArgParsing_ValidCertificateStoreName(string storeName)
             // Arrange
             var packagePath = @"\\path\package.nupkg";
             var timestamper = "https://timestamper.test";
-            var certificateFingerprint = new Guid().ToString();
+            var certificateFingerprint = _sha1Hash;
             var parsable = Enum.TryParse(storeName, ignoreCase: true, result: out StoreName parsedStoreName);
             var mockConsole = new Mock<IConsole>();
             var signCommand = new SignCommand
@@ -107,7 +109,7 @@ public void SignCommandArgParsing_InvalidCertificateStoreName()
             // Arrange
             var packagePath = @"\\path\package.nupkg";
             var timestamper = "https://timestamper.test";
-            var certificateFingerprint = new Guid().ToString();
+            var certificateFingerprint = _sha1Hash;
             var storeName = "random_store";
             var mockConsole = new Mock<IConsole>();
             var signCommand = new SignCommand
@@ -135,7 +137,7 @@ public void SignCommandArgParsing_ValidCertificateStoreLocation(string storeLoca
             // Arrange
             var packagePath = @"\\path\package.nupkg";
             var timestamper = "https://timestamper.test";
-            var certificateFingerprint = new Guid().ToString();
+            var certificateFingerprint = _sha1Hash;
             var parsable = Enum.TryParse(storeLocation, ignoreCase: true, result: out StoreLocation parsedStoreLocation);
             var mockConsole = new Mock<IConsole>();
             var signCommand = new SignCommand
@@ -161,7 +163,7 @@ public void SignCommandArgParsing_InvalidCertificateStoreLocation()
             // Arrange
             var packagePath = @"\\path\package.nupkg";
             var timestamper = "https://timestamper.test";
-            var certificateFingerprint = new Guid().ToString();
+            var certificateFingerprint = _sha1Hash;
             var storeLocation = "random_location";
             var mockConsole = new Mock<IConsole>();
             var signCommand = new SignCommand
@@ -299,7 +301,7 @@ public void SignCommandArgParsing_ValidArgs_CertFingerprintAsync()
             // Arrange
             var packagePath = @"\\path\package.nupkg";
             var timestamper = "https://timestamper.test";
-            var certificateFingerprint = new Guid().ToString();
+            var certificateFingerprint = _sha1Hash;
             var hashAlgorithm = "sha256";
             Enum.TryParse(hashAlgorithm, ignoreCase: true, result: out HashAlgorithmName parsedHashAlgorithm);
             var timestampHashAlgorithm = "sha512";
@@ -459,5 +461,62 @@ public void SignCommand_Failure_InvalidArguments(string cmd)
         {
             Util.TestCommandInvalidArguments(cmd);
         }
+
+        [Fact]
+        public void SignCommandArgParsing_LogsAWarningForInsecureCertificateFingerprint()
+        {
+            //Arrange
+            var logMessages = new StringBuilder();
+            var signCommand = ArrangeSignCommand(_sha1Hash, logMessages);
+
+            // Act
+            _ = signCommand.GetSignArgs();
+
+            //Assert
+            Assert.Equal(expected: NuGetCommand.SignCommandInvalidCertificateFingerprint, actual: logMessages.ToString().Trim());
+        }
+
+        [Theory]
+        [InlineData("89967D1DD995010B6C66AE24FF8E66885E6E03")] // 39 characters long not SHA-1
+        [InlineData("invalid-certificate-fingerprint")]
+        public void SignCommandArgParsing_ThrowsAnExceptionWarningForInsecureCertificateFingerprint(string certificateFingerprint)
+        {
+            var signCommand = ArrangeSignCommand(certificateFingerprint);
+
+            // Act & Assert
+            var ex = Assert.Throws<ArgumentException>(() => signCommand.GetSignArgs());
+            Assert.True(ex.Message.Contains(NuGetCommand.SignCommandInvalidCertificateFingerprint));
+        }
+
+        private static SignCommand ArrangeSignCommand(string certificateFingerprint, StringBuilder logMessages = null)
+        {
+            var packagePath = @"\\path\package.nupkg";
+            var timestamper = "https://timestamper.test";
+            var hashAlgorithm = "sha256";
+            var timestampHashAlgorithm = "sha512";
+            var outputDir = @".\test\output\path";
+            var mockConsole = new Mock<IConsole>();
+            mockConsole.Setup(c => c.Verbosity).Returns(Verbosity.Detailed);
+            mockConsole.Setup(c => c.Log(It.IsAny<ILogMessage>())).Callback<ILogMessage>((message) =>
+            {
+                logMessages?.AppendLine($"{message.Message}");
+            });
+
+            var signCommand = new SignCommand
+            {
+                Console = mockConsole.Object,
+                Timestamper = timestamper,
+                CertificateFingerprint = certificateFingerprint,
+                HashAlgorithm = hashAlgorithm,
+                TimestampHashAlgorithm = timestampHashAlgorithm,
+                OutputDirectory = outputDir,
+                NonInteractive = true,
+                Overwrite = true,
+            };
+
+            signCommand.Arguments.Add(packagePath);
+
+            return signCommand;
+        }
     }
 }