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

Cannot sign clickonce manifest if the certificate for the CA that issued the signing cert is not installed locally #761

Open
jackmtpt opened this issue Sep 12, 2024 · 0 comments
Labels
area-clickonce Related to ClickOnce signing Priority:2 Work that is important, but not critical for the release

Comments

@jackmtpt
Copy link
Contributor

jackmtpt commented Sep 12, 2024

Describe the bug
If you try and sign a clickonce manifest using a certificate B stored in Azure Key Vault, issued by a CA certificate A, certificate A must be installed on the local machine otherwise signing will fail. You'll get a stack trace like this:

fail: Sign.Core.ISigner[0]
      Error occurred during a cryptographic operation.
      System.ApplicationException: Error occurred during a cryptographic operation.
       ---> System.Security.Cryptography.CryptographicException: Error occurred during a cryptographic operation.
         at System.Deployment.Internal.CodeSigning.SignedCmiManifest2.InsertPublisherIdentity(XmlDocument manifestDom, X509Certificate2 signerCert) in /_/src/Sign.Core/Native/mansign2.cs:line 421
         at System.Deployment.Internal.CodeSigning.SignedCmiManifest2.Sign(CmiManifestSigner2 signer, String timeStampUrl, Boolean disallowMansignTimestampFallback) in /_/src/Sign.Core/Native/mansign2.cs:line 351
         at Sign.Core.ManifestSigner.Sign(FileInfo file, X509Certificate2 certificate, RSA rsaPrivateKey, SignOptions options) in /_/src/Sign.Core/DataFormatSigners/ManifestSigner.cs:line 42

Looking at the code in mansign2.cs, it's trying to locate the public key of the certificate (A) that issued the certificate (B) that's being used to sign the manifest. This lookup is done using the local cert store, even if certificate A is stored in Azure Key Vault.

This scenario can easily happen if you have an company internal CA that issues a code signing certificate for internal applications which is then stored in a key vault. If you try and run sign from e.g. an Azure DevOps hosted agent that doesn't have your company CA cert(s) installed on it, then the signing will fail.

Repro steps

# Generate certificate A
New-SelfSignedCertificate -KeyAlgorithm RSA -KeyLength 2048 -Subject "CN=TEST temporary CA" -CertStoreLocation Cert:\CurrentUser\my

# Generate certificate B. Set the signer thumbprint to the one of the CA cert generated above.
New-SelfSignedCertificate -KeyAlgorithm RSA -KeyLength 2048 -Subject "CN=TEST temporary code signing cert" -CertStoreLocation Cert:\CurrentUser\my -signer Cert:\CurrentUser\my\0E07DD3CF22224ED029C62151AEE55CAB5FD73E3 -KeyExportPolicy Exportable

Export certificate B to a .pfx file and upload it to an AKV. Delete certificate A from the Personal and Intermediate Certification Authority stores. You must ensure that when viewing cert B via the certificates mmc snapin that you get the message windows does not have enough information to verify this certificate, indicating that it does not know the public key of B's issuer.

Now run sign.exe pointing it at a clickonce .application manifest, using certificate B from your AKV. You should get the above exception.

Expected behavior
Honestly this is kind of the only possible behaviour - the clickonce manifest format requires the issuerKeyHash property to be set, which is the SKID (public key hash) of certificate A. It's therefore impossible to generate a valid manifest without it. sign should explicitly check for this though and report a better error than Error occurred during a cryptographic operation.

Actual behavior
The above exception, with an unhelpful error message that requires you to read the code and perform the same experiment that I did in order to write this report.

Additional context

> sign --version
0.9.1-beta.24406.1+6584f5d081d8a06660d58d1a777b2352ff376a68
@dtivel dtivel added the Priority:2 Work that is important, but not critical for the release label Oct 1, 2024
@dtivel dtivel added the area-clickonce Related to ClickOnce signing label Nov 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-clickonce Related to ClickOnce signing Priority:2 Work that is important, but not critical for the release
Projects
None yet
Development

No branches or pull requests

2 participants