electron-userland / electron-builder

A complete solution to package and build a ready for distribution Electron app with “auto update” support out of the box
https://www.electron.build
MIT License
13.66k stars 1.74k forks source link

New Azure Trusted Signing not adding a timestamp #8626

Closed 255kb closed 5 days ago

255kb commented 6 days ago

I just finished updating my app to use Azure Trusted Signing. Everything went well and the exe was signed. However, it triggered a Trojan detection in Windows Defender which I suspected to be a false positive. I looked at the exe properties and noticed that the timestamp was missing:

image

Looking at the Azure signing code it seems two params are missing in the Invoke-TrustedSigning command: TimestampRfc3161 and TimestampDigest. They are used by the Trusted Signing Action.

I guess it should adding a timestamp using the default timestamp URL, or maybe I'm missing something?

I solved the issue by adding additional params to the azureSignOptions object:

azureSignOptions: {
      endpoint: 'https://eus.codesigning.azure.net',
      certificateProfileName: 'cert-name',
      codeSigningAccountName: 'account-name',

      // new properties
      TimestampRfc3161: 'http://timestamp.acs.microsoft.com',
      TimestampDigest: 'SHA256'
}
mmaietta commented 6 days ago

Great callout and thanks for reporting the issue. I can add those as default entries to the configuration. The Trusted Signing Action indeed uses them, but none of the params in that action are actually depicted as "required" other than CodeSigningAccountName. That was the main reason I left azureSignOptions to have a [Key: string]: string object so that the user could apply custom TrustedSigning vars.

Makes sense that Timestamp is provided with default values. I'll get a PR set up for it today 🙂

mmaietta commented 6 days ago

@255kb did you perchance run into this issue during your signing+publishing? https://github.com/electron-userland/electron-builder/issues/8612 Trying to fix as many azure-related issues reported in the next release as possible

255kb commented 6 days ago

Great callout and thanks for reporting the issue. I can add those as default entries to the configuration. The Trusted Signing Action indeed uses them, but none of the params in that action are actually depicted as "required" other than CodeSigningAccountName. That was the main reason I left azureSignOptions to have a [Key: string]: string object so that the user could apply custom TrustedSigning vars.

Makes sense that Timestamp is provided with default values. I'll get a PR set up for it today 🙂

It's true they are not marked as required. Which is strange because who wants code signing without timestamping considering the Azure Trusted Signing certificates are short lived. Thank you for your awesome work @mmaietta! It's really appreciated.

@255kb did you perchance run into this issue during your signing+publishing? #8612 Trying to fix as many azure-related issues reported in the next release as possible

I don't think so. I don't have spaces in binary names.

mmaietta commented 11 hours ago

@255kb do you happen to have a production azure-trusted-signed exe you could send me? I'd like to see if I can implement signature verification for the auto-update flow, or if azure trusted signing even can support that flow.

255kb commented 4 hours ago

@mmaietta you will find one here: https://github.com/mockoon/mockoon/releases/download/v9.0.0/mockoon.setup.9.0.0.exe However, I never implemented auto update, so I'm not sure if it will affect your testing or not.