sign icon indicating copy to clipboard operation
sign copied to clipboard
verified publisher icon dotnet

Unable to Authenticode sign using version 0.9.1-beta.24406.1

Open martincostello opened this issue 1 year ago • 2 comments

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.

martincostello avatar Sep 26 '24 17:09 martincostello

@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:

  • --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

dtivel avatar Sep 30 '24 19:09 dtivel

@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.

martincostello avatar Oct 01 '24 08:10 martincostello

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

jnm2 avatar Nov 01 '24 19:11 jnm2

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

Perksey avatar Nov 05 '24 17:11 Perksey

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.

clairernovotny avatar Nov 11 '24 21:11 clairernovotny

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

ChrisSfanos avatar Nov 12 '24 16:11 ChrisSfanos

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

martincostello avatar Nov 12 '24 16:11 martincostello

Looks like the permissions change in Azure was the fix - I've tested with the latest sign version here, and everything now seems to be working as expected: https://github.com/App-vNext/Polly/pull/2383.

Thanks everyone.

martincostello avatar Nov 13 '24 22:11 martincostello

Thanks Martin - I've made the same change for Silk.NET too

ChrisSfanos avatar Nov 13 '24 23:11 ChrisSfanos