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 sign ClickOnce manifests since commit 6584f5d (KeyVault with HSM storage) #753

Closed
pjobin-semex opened this issue Aug 8, 2024 · 6 comments

Comments

@pjobin-semex
Copy link

We use a GlobalSign EV CodeSign Cert stored in a premium Key Vault (HSM storage).

Since commit 6584f5d the process crashes while signing a ClickOnce manifest:

System.ApplicationException: Cannot download the key
---> System.InvalidOperationException: Cannot download the key
    at Azure.Security.KeyVault.Keys.Cryptography.RSAKeyVault.ExportParameters(Boolean includePrivateParameters)
    at System.Deployment.Internal.CodeSigning.SignedCmiManifest2.ReplacePublicKeyToken(XmlDocument manifestDom, AsymmetricAlgorithm snKey) in C:\Dev\_lab\sign\src\Sign.Core\Native\mansign2.cs:line 513
    at System.Deployment.Internal.CodeSigning.SignedCmiManifest2.Sign(CmiManifestSigner2 signer, String timeStampUrl, Boolean disallowMansignTimestampFallback) in C:\Dev\_lab\sign\src\Sign.Core\Native\mansign2.cs:line 342
    at Sign.Core.ManifestSigner.Sign(FileInfo file, X509Certificate2 certificate, RSA rsaPrivateKey, SignOptions options) in C:\Dev\_lab\sign\src\Sign.Core\DataFormatSigners\ManifestSigner.cs:line 42
    --- End of inner exception stack trace ---
    at Sign.Core.ManifestSigner.Sign(FileInfo file, X509Certificate2 certificate, RSA rsaPrivateKey, SignOptions options) in C:\Dev\_lab\sign\src\Sign.Core\DataFormatSigners\ManifestSigner.cs:line 49
    at Sign.Core.ClickOnceSigner.SignCoreAsync(String args, FileInfo file, RSA rsaPrivateKey, X509Certificate2 certificate, SignOptions options) in C:\Dev\_lab\sign\src\Sign.Core\DataFormatSigners\ClickOnceSigner.cs:line 220
    at Sign.Core.RetryingSigner.SignAsync(String args, FileInfo file, RSA rsaPrivateKey, X509Certificate2 publicCertificate, SignOptions options) in C:\Dev\_lab\sign\src\Sign.Core\DataFormatSigners\RetryingSigner.cs:line 40
    at Sign.Core.ClickOnceSigner.<>c__DisplayClass9_2.<<SignAsync>b__0>d.MoveNext() in C:\Dev\_lab\sign\src\Sign.Core\DataFormatSigners\ClickOnceSigner.cs:line 134
--- End of stack trace from previous location ---
    at System.Threading.Tasks.Parallel.<>c__53`1.<<ForEachAsync>b__53_0>d.MoveNext()
--- End of stack trace from previous location ---
    at Sign.Core.ClickOnceSigner.SignAsync(IEnumerable`1 files, SignOptions options) in C:\Dev\_lab\sign\src\Sign.Core\DataFormatSigners\ClickOnceSigner.cs:line 82
    at Sign.Core.AggregatingSigner.SignAsync(IEnumerable`1 files, SignOptions options) in C:\Dev\_lab\sign\src\Sign.Core\DataFormatSigners\AggregatingSigner.cs:line 204
    at Sign.Core.Signer.<>c__DisplayClass3_0.<<SignAsync>b__0>d.MoveNext() in C:\Dev\_lab\sign\src\Sign.Core\Signer.cs:line 155
--- End of stack trace from previous location ---
    at System.Threading.Tasks.Parallel.<>c__53`1.<<ForEachAsync>b__53_0>d.MoveNext()
--- End of stack trace from previous location ---
    at Sign.Core.Signer.SignAsync(IReadOnlyList`1 inputFiles, String outputFile, FileInfo fileList, DirectoryInfo baseDirectory, String applicationName, String publisherName, String description, Uri descriptionUrl, Uri timestampUrl, Int32 maxConcurrency, HashAlgorithmName fileHashAlgorithm, HashAlgorithmName timestampHashAlgorithm) in C:\Dev\_lab\sign\src\Sign.Core\Signer.cs:line 84

Everything works properly when we use the previous version at commit 74cd61f.

Additional context

sign --version

0.9.1-beta.24406.1+6584f5d081d8a06660d58d1a777b2352ff376a68
dotnet --info

.NET SDK:
 Version:           8.0.400-preview.0.24324.5
 Commit:            736a2616db
 Workload version:  8.0.400-manifests.88eef10b
 MSBuild version:   17.11.0+1192b22fd

Runtime Environment:
 OS Name:     Windows
 OS Version:  10.0.22631
 OS Platform: Windows
 RID:         win-x64
 Base Path:   C:\Program Files\dotnet\sdk\8.0.400-preview.0.24324.5\

.NET workloads installed:
Configured to use loose manifests when installing new manifests.
 [aspire]
   Installation Source: VS 17.11.35118.90
   Manifest Version:    8.0.0/8.0.100
   Manifest Path:       C:\Program Files\dotnet\sdk-manifests\8.0.100\microsoft.net.sdk.aspire\8.0.0\WorkloadManifest.json
   Install Type:        FileBased


Host:
  Version:      8.0.5
  Architecture: x64
  Commit:       087e15321b

.NET SDKs installed:
  3.1.426 [C:\Program Files\dotnet\sdk]
  8.0.400-preview.0.24324.5 [C:\Program Files\dotnet\sdk]

.NET runtimes installed:
  Microsoft.AspNetCore.All 2.1.30 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All]
  Microsoft.AspNetCore.App 2.1.30 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 3.1.32 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 6.0.30 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 7.0.19 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 8.0.5 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.NETCore.App 2.1.30 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 3.1.32 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 6.0.30 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 7.0.19 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 8.0.5 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.WindowsDesktop.App 3.1.32 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
  Microsoft.WindowsDesktop.App 6.0.30 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
  Microsoft.WindowsDesktop.App 7.0.19 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
  Microsoft.WindowsDesktop.App 8.0.5 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]

Other architectures found:
  x86   [C:\Program Files (x86)\dotnet]
    registered at [HKLM\SOFTWARE\dotnet\Setup\InstalledVersions\x86\InstallLocation]

Environment variables:
  Not set

global.json file:
  Not found

Learn more:
  https://aka.ms/dotnet/info

Download .NET:
  https://aka.ms/dotnet/download
@dlemstra
Copy link
Contributor

dlemstra commented Aug 9, 2024

Can you confirm if PR #755 solves your issue? I am unable to test this properly myself because I no longer have a valid certificate in my Azure key vault.

@pjobin-semex
Copy link
Author

Hello @dlemstra! Thanks for your quick reply. Sadly, we have the same issue with PR #755.

@pjobin-semex
Copy link
Author

Hello again @dlemstra.

I performed more debugging today and I think I’ve found the issue.

It looks like the Azure.Security.KeyVault.Keys package requires the Key Permissions > Key Management Operations > Get permission to be able to retrieve the RSA key.

Without this permission, when KeyVaultService.GetRsaAsync calls internally CryptographyClient.InitializeAsync, it triggers a 403 RequestFailedException, preventing the key from being loaded and causing the System.InvalidOperationException: Cannot download the key exception downstream.

I added the permission and tried again with version 0.9.1-beta.24406.1+6584f5d081d8a06660d58d1a777b2352ff376a68 and it worked!

If that makes sense, you may want to update the minimum permissions in best-practices.

Thanks!

@dlemstra
Copy link
Contributor

Thanks for helping us with this and coming back with a detailed explanation

@dtivel
Copy link
Collaborator

dtivel commented Aug 15, 2024

@pjobin-semex, thanks for opening this issue and your detailed investigation. I updated documentation on necessary permissions.

@dtivel dtivel closed this as completed Aug 15, 2024
@MH-ZShearer
Copy link

I wanted to add onto this that this only works if your key vault is using Access Policies. If your key vault is using RBAC, or Access control (IAM), following this will result in all accounts configured with roles to be locked out.

The short of it is that we have a custom role for a custom automated application used for code signing that has the following three permissions. We had to add keys/read to work with the sign tool even though the AzureSignTool did not require it for some reason.

{
    "id": "/subscriptions/***SECRET***/providers/Microsoft.Authorization/roleDefinitions/***SECRET***",
    "properties": {
        "roleName": "Code Signing Operator",
        "description": "Enables getting Certificates and signing with Keys",
        "assignableScopes": [
            "/subscriptions/***SECRET***/resourceGroups/ResourceGroup"
        ],
        "permissions": [
            {
                "actions": [],
                "notActions": [],
                "dataActions": [
                    "Microsoft.KeyVault/vaults/keys/read",
                    "Microsoft.KeyVault/vaults/keys/sign/action",
                    "Microsoft.KeyVault/vaults/certificates/read"
                ],
                "notDataActions": []
            }
        ]
    }
}

I don't know what the minimum built-in roles are to meet these requirements, but I believe Key Vault Certificate User and Key Vault Crypto User together will accomplish the necessary actions.

Admittedly, I'm generally very lost when it comes to this code signing business and I was very confused as to why I was able to test the sign tool locally but it failed in our GitHub Action. Turned out sign, or the Azure libraries, was using my local Kerberos token/Azure session, even if I was signed out of the Azure CLI.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants