nuget sign should validate certificate input has a private key

[vcsjones]

NuGet Product Used

dotnet.exe

Product Version

9.0.100-preview.2.24157.14

Worked before?

No response

Impact

It bothers me. A fix would be nice

Repro Steps & Context

When using dotnet nuget sign the signing operation does not validate that the X509Certificate2 instance contains a private key when it is used for signing purposes.

When used with a public X.509 certificate, not a PKCS#12/PFX, it produces an exception.

I would recommend checking X509Certificate2.HasPrivateKey prior to performing a signing operation, and if false, print an appropriate error.

Currently, this produces an exception that is difficult to diagnose. The easiest way to reproduce this is to pass a public certificate to --certificate-path.

I don't know the best place to insert that check, but maybe somewhere around here: https://github.com/NuGet/NuGet.Client/blob/7ad6fcc9c56c960975c37b2416c7eae1d53ba3fd/src/NuGet.Core/NuGet.Commands/SignCommand/SignCommandRunner.cs#L47

Without this check, you'll receive an error that looks something like this:

X.509 certificate chain validation will use the default trust store selected by .NET for code signing.
X.509 certificate chain validation will use the default trust store selected by .NET for timestamping.

Signing package(s) with certificate:
  Subject Name: CN=_REDACTED_, SERIALNUMBER=_REDACTED_, OID.2.5.4.15=Private Organization, O=_REDACTED_, OID.1.3.6.1.4.1.311.60.2.1.3=_REDACTED_, L=_REDACTED_, S=_REDACTED_, C=_REDACTED_
  SHA1 hash: _REDACTED_
  SHA256 hash: _REDACTED_
  Issued by: CN=Entrust Extended Validation Code Signing CA - EVCS2, O="Entrust, Inc.", C=US
  Valid from: 8/14/2023 7:34:11 PM to 8/14/2026 7:34:10 PM
Timestamping package(s) with:
http://timestamp.entrust.net/rfc3161ts2
error: Unknown error (0xc100000d)
trace: System.Security.Cryptography.CryptographicException: Unknown error (0xc100000d)
trace:    at System.Security.Cryptography.RSABCrypt.TrySignHash(ReadOnlySpan`1 hash, Span`1 destination, HashAlgorithmName hashAlgorithm, RSASignaturePadding padding, Int32& bytesWritten)
trace:    at System.Security.Cryptography.Pkcs.CmsSignature.RSACmsSignature.SignCore(ReadOnlySpan`1 dataHash, HashAlgorithmName hashAlgorithmName, X509Certificate2 certificate, AsymmetricAlgorithm key, Boolean silent, RSASignaturePadding signaturePadding, Byte[]& signatureValue)
trace:    at System.Security.Cryptography.Pkcs.CmsSignature.RSAPkcs1CmsSignature.Sign(ReadOnlySpan`1 dataHash, HashAlgorithmName hashAlgorithmName, X509Certificate2 certificate, AsymmetricAlgorithm key, Boolean silent, String& signatureAlgorithm, Byte[]& signatureValue, Byte[]& signatureParameters)
trace:    at System.Security.Cryptography.Pkcs.CmsSignature.Sign(ReadOnlySpan`1 dataHash, HashAlgorithmName hashAlgorithmName, X509Certificate2 certificate, AsymmetricAlgorithm key, Boolean silent, RSASignaturePadding rsaSignaturePadding, String& oid, ReadOnlyMemory`1& signatureValue, ReadOnlyMemory`1& signatureParameters)
trace:    at System.Security.Cryptography.Pkcs.CmsSigner.Sign(ReadOnlyMemory`1 data, String contentTypeOid, Boolean silent, X509Certificate2Collection& chainCerts)
trace:    at System.Security.Cryptography.Pkcs.SignedCms.ComputeSignature(CmsSigner signer, Boolean silent)
trace:    at NuGet.Packaging.Signing.X509SignatureProvider.CreatePrimarySignature(CmsSigner cmsSigner, SignPackageRequest request, Byte[] signingData)
trace:    at NuGet.Packaging.Signing.X509SignatureProvider.CreatePrimarySignatureAsync(SignPackageRequest request, SignatureContent signatureContent, ILogger logger, CancellationToken token)
trace:    at NuGet.Packaging.Signing.SigningUtility.SignAsync(SigningOptions options, SignPackageRequest signRequest, CancellationToken token)
trace:    at NuGet.Commands.SignCommandRunner.ExecuteCommandAsync(IEnumerable`1 packagesToSign, SignPackageRequest signPackageRequest, String timestamper, ILogger logger, String outputDirectory, Boolean overwrite, CancellationToken token)

as noted over at dotnet/runtime#100414.

.NET will try an make the exception have a better error message, but it would be nice if NuGet handled this more gracefully instead of producing a stack trace.

Verbose Logs

No response

[bartonjs]

I was confused as to how a public key was getting that far. Apparently it's intentionally falling back to the public key just so it can throw from within the Sign routine; probably so that it looked a lot closer to the exception from .NET Framework (where SignedCms was implemented in terms of the cert handle and calling into Win32). We might also want to change SignedCms to throw a better exception here instead of waking up the public key object... but that's another vanity change, not something we'd backport.