search

dotnet / sign

Code Signing CLI tool supporting Authenticode, NuGet, VSIX, and ClickOnce
MIT License
483 stars 91 forks source link

Unable to Authenticode sign using version 0.9.1-beta.24406.1 #764

Open martincostello opened 1 month ago

martincostello commented 1 month ago

Describe the bug

In Polly we Authenticode sign our assemblies and NuGet packages using a code signing certificate provided by the .NET Foundation.

Trying to release a new version today, signing failed with errors trying to download the key.

Since our last release, v8.4.1, we had ingested three dependabot updates for sign (https://github.com/App-vNext/Polly/pull/2178, https://github.com/App-vNext/Polly/pull/2205, https://github.com/App-vNext/Polly/pull/2260). We only sign our builds that are intended to be published to NuGet.org, so the issue didn't come to light until now - we also can't validate signing in dependabot PRs anyway, as secrets are not available to them.

Reverting those changes (https://github.com/App-vNext/Polly/pull/2305) and going back to the version we used for our 8.4.1 release resolved the issue, with the signing succeeding to publish 8.4.2 (logs).

Something in the last three public releases appears to have broken something.

This may be related to #753, but I assume the change their has been released, so if so either than change broke this use case, or it's a different problem.

I have no direct access to the certificate or the Azure resources it's stored in, so I can't check any settings related to it myself.

/cc @joelhulen

Repro steps

We were broken as of https://github.com/App-vNext/Polly/commit/f7dc35fb365407fd7579316b4b220595281e2e5c, but without you having access to our secrets I'm not sure how useful that is...

Expected behavior

Assemblies and NuGet packages are successfully signed.

Actual behavior

Signing fails with the following errors:

fail: Sign.Core.INuGetSignTool[0]
      Cannot download the key
      System.InvalidOperationException: Cannot download the key
         at Azure.Security.KeyVault.Keys.Cryptography.RSAKeyVault.get_KeySize()
         at System.Security.Cryptography.Pkcs.CmsSignature.RSACmsSignature.SignCore(ReadOnlySpan`1 dataHash, HashAlgorithmName hashAlgorithmName, X509Certificate2 certificate, AsymmetricAlgorithm key, Boolean silent, RSASignaturePadding signaturePadding, Byte[]& signatureValue)
         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)
         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)
         at System.Security.Cryptography.Pkcs.CmsSigner.Sign(ReadOnlyMemory`1 data, String contentTypeOid, Boolean silent, X509Certificate2Collection& chainCerts)
         at System.Security.Cryptography.Pkcs.SignedCms.ComputeSignature(CmsSigner signer, Boolean silent)
         at Sign.Core.NuGetSignatureProvider.CreatePrimarySignature(AuthorSignPackageRequest request, SignatureContent signatureContent, ILogger logger) in /_/src/Sign.Core/Tools/NuGet/NuGetSignatureProvider.cs:line 103
         at Sign.Core.NuGetSignatureProvider.CreateAuthorSignatureAsync(AuthorSignPackageRequest request, SignatureContent signatureContent, ILogger logger, CancellationToken token) in /_/src/Sign.Core/Tools/NuGet/NuGetSignatureProvider.cs:line 67
         at NuGet.Packaging.Signing.SigningUtility.SignAsync(SigningOptions options, SignPackageRequest signRequest, CancellationToken token)
         at Sign.Core.NuGetPackageSigner.SignAsync(String packagePath, String outputPath, Uri timestampUrl, SignatureType signatureType, HashAlgorithmName signatureHashAlgorithm, HashAlgorithmName timestampHashAlgorithm, X509Certificate2 signingCertificate, RSA rsa, Boolean overwrite, CancellationToken cancellationToken) in /_/src/Sign.Core/Tools/NuGet/NuGetPackageSigner.cs:line 96

Additional context

The referenced commit where signing fails uses sign 0.9.1-beta.24406.1 and .NET SDK 8.0.402.

dtivel commented 1 month ago

@martincostello, can you try the latest version of Sign CLI just uploaded to NuGet.org? https://www.nuget.org/packages/sign/0.9.1-beta.24469.1

BTW, this is unrelated to the above issue, but your CI output includes the warning:

The client secret options are obsolete and should no longer be specified.

These options are deprecated:

Going forward, these values should be passed using environment variables:

martincostello commented 1 month ago

@dtivel Still failing with 0.9.1-beta.24469.1 (logs):

fail: Sign.Core.INuGetSignTool[0]
      Cannot download the key
      System.InvalidOperationException: Cannot download the key
         at Azure.Security.KeyVault.Keys.Cryptography.RSAKeyVault.get_KeySize()
         at System.Security.Cryptography.Pkcs.CmsSignature.RSACmsSignature.SignCore(ReadOnlySpan`1 dataHash, HashAlgorithmName hashAlgorithmName, X509Certificate2 certificate, AsymmetricAlgorithm key, Boolean silent, RSASignaturePadding signaturePadding, Byte[]& signatureValue)
         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)
         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)
         at System.Security.Cryptography.Pkcs.CmsSigner.Sign(ReadOnlyMemory`1 data, String contentTypeOid, Boolean silent, X509Certificate2Collection& chainCerts)
         at System.Security.Cryptography.Pkcs.SignedCms.ComputeSignature(CmsSigner signer, Boolean silent)
         at Sign.Core.NuGetSignatureProvider.CreatePrimarySignature(AuthorSignPackageRequest request, SignatureContent signatureContent, ILogger logger) in /_/src/Sign.Core/Tools/NuGet/NuGetSignatureProvider.cs:line 103
         at Sign.Core.NuGetSignatureProvider.CreateAuthorSignatureAsync(AuthorSignPackageRequest request, SignatureContent signatureContent, ILogger logger, CancellationToken token) in /_/src/Sign.Core/Tools/NuGet/NuGetSignatureProvider.cs:line 67
         at NuGet.Packaging.Signing.SigningUtility.SignAsync(SigningOptions options, SignPackageRequest signRequest, CancellationToken token)
         at Sign.Core.NuGetPackageSigner.SignAsync(String packagePath, String outputPath, Uri timestampUrl, SignatureType signatureType, HashAlgorithmName signatureHashAlgorithm, HashAlgorithmName timestampHashAlgorithm, X509Certificate2 signingCertificate, RSA rsa, Boolean overwrite, CancellationToken cancellationToken) in /_/src/Sign.Core/Tools/NuGet/NuGetPackageSigner.cs:line 96

Looking at our validation step, it seems like the .dll files are being signed, but signing the .nupkg files is failing (logs):

D:\a\Polly\Polly\extracted\Polly.8.4.3-pr.2322.3981.nupkg\lib\net462\Polly.dll in NuGet package D:\a\Polly\Polly\Polly.8.4.3-pr.2322.3981.nupkg has a valid signature.
D:\a\Polly\Polly\extracted\Polly.8.4.3-pr.2322.3981.nupkg\lib\net472\Polly.dll in NuGet package D:\a\Polly\Polly\Polly.8.4.3-pr.2322.3981.nupkg has a valid signature.
D:\a\Polly\Polly\extracted\Polly.8.4.3-pr.2322.3981.nupkg\lib\net6.0\Polly.dll in NuGet package D:\a\Polly\Polly\Polly.8.4.3-pr.2322.3981.nupkg has a valid signature.
D:\a\Polly\Polly\extracted\Polly.8.4.3-pr.2322.3981.nupkg\lib\netstandard2.0\Polly.dll in NuGet package D:\a\Polly\Polly\Polly.8.4.3-pr.2322.3981.nupkg has a valid signature.
All 4 DLLs in NuGet package D:\a\Polly\Polly\Polly.8.4.3-pr.2322.3981.nupkg have valid signatures.

Verifying Polly.8.4.3-pr.2322.3981

error: NU3004: The package is not signed.

Package signature validation failed.
jnm2 commented 1 week ago

BTW, this is unrelated to the above issue, but your CI output includes the warning:

The client secret options are obsolete and should no longer be specified.

These options are deprecated:

  • --azure-key-vault-tenant-id / -kvt
  • --azure-key-vault-client-id / -kvi
  • --azure-key-vault-client-secret / -kvs

Going forward, these values should be passed using environment variables:

  • AZURE_TENANT_ID
  • AZURE_CLIENT_ID
  • AZURE_CLIENT_SECRET

The readme still links to a sample AzDO yaml file which uses the obsolete options (https://github.com/dotnet/sign#sample-workflows):

https://github.com/dotnet/sign/blob/8b9d88d4dc369b2d518bb2cd79d31f2a51e482c1/docs/azdo-build-and-sign.yml#L87-L89

Perksey commented 1 week ago

Just hit this with Silk.NET as well. Will continue using the old version.

clairernovotny commented 1 day ago

Hi,

Looks like the issue in both cases here are the access policies for the identity accessing key vault.

The required permissions are:

Key: Sign, Get Certificate: Get

With HSM keys, it won't return the private key material, but will still return metadata about the key, such as size.

@ChrisSfanos Are you able to update the access policies for the key vaults used by DNF projects to ensure the permissions are set accordingly? This may be a change from the way they were originally set up.

ChrisSfanos commented 16 hours ago

Thanks Claire - The App access policy did only have Sign for Key permissions, so I've added Get to it (this should support the 'old' way - when we migrate to using managed identities, we will need to do work again to provide an access policy)

If this works, I'll make the same change to Silk.NET for @Perksey

martincostello commented 16 hours ago

Thanks @ChrisSfanos - I'll re-test our signing pipeline with the new version tomorrow and get back to you.