Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
martincostello opened this issue Sep 26, 2024 · 9 comments
Closed

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

martincostello opened this issue Sep 26, 2024 · 9 comments
Assignees
Labels
investigate Priority:1 Work that is critical for the release, but we could probably ship without

Comments

@martincostello
Copy link
Member

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 (App-vNext/Polly#2178, App-vNext/Polly#2205, App-vNext/Polly#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 (App-vNext/Polly#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 App-vNext/Polly@f7dc35f, 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
Copy link
Collaborator

dtivel commented Sep 30, 2024

@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

@martincostello
Copy link
Member Author

martincostello commented Oct 1, 2024

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

@dtivel dtivel added Priority:1 Work that is critical for the release, but we could probably ship without investigate labels Oct 1, 2024
@jnm2
Copy link

jnm2 commented Nov 1, 2024

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

--azure-key-vault-tenant-id "$(SignTenantId)" `
--azure-key-vault-client-id "$(SignClientId)" `
--azure-key-vault-client-secret '$(SignClientSecret)' `

@Perksey
Copy link
Member

Perksey commented Nov 5, 2024

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

@clairernovotny
Copy link
Member

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
Copy link
Member

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
Copy link
Member Author

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

martincostello added a commit to martincostello/Polly that referenced this issue Nov 13, 2024
@martincostello
Copy link
Member Author

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: App-vNext/Polly#2383.

Thanks everyone.

@ChrisSfanos
Copy link
Member

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
investigate Priority:1 Work that is critical for the release, but we could probably ship without
Projects
None yet
Development

No branches or pull requests

6 participants