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

feat(bundler): add support for EV codesigning certs via azuresigntool #8718

Closed
wants to merge 15 commits into from

Conversation

tr3ysmith
Copy link
Contributor

This feature adds support for EV code signing via azuresigntool.exe. The original method using using signtool.exe still works. But if the environment variables are set properly before running tauri build, the Tauri bundler will attempt to use AzureSignTool.

To use AzureSignTool, the user must have an EV certificate hosted in Azure Key Vault.

@tr3ysmith tr3ysmith requested a review from a team as a code owner January 31, 2024 16:38
@tr3ysmith
Copy link
Contributor Author

due to the complexity of the azuresigntool package, I removed it from the binary and it'll need to currently be preinstalled on the host machine. With GitHub runners, this should be possible by running dotnet tool install --global AzureSignTool before executing the Tauri build command

@tr3ysmith tr3ysmith changed the title feat: add support for EV codesigning certs via azuresigntool feat(bundler): add support for EV codesigning certs via azuresigntool Feb 6, 2024
@tr3ysmith
Copy link
Contributor Author

@lucasfernog should we bump the rust min version on the main crate too? I had to bump it to 1.72 on the CLI crate because of the dependency for jsonschema -> aHash (aHash requires 1.72)

tkaitchuck/aHash#196

Copy link
Member

@amrbashir amrbashir left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure if we should support azuresigntool tbh since it is not an official tool (correct me if I am wrong), should we instead provide a generic option to specify a signing command, similar to this https://github.com/crabnebula-dev/cargo-packager/blob/ec1a3a12296275341fd59c083d98202de316805f/crates/packager/src/config/mod.rs#L1206-L1214?

tooling/cli/Cargo.toml Show resolved Hide resolved
tooling/bundler/src/bundle/windows/sign.rs Outdated Show resolved Hide resolved
tooling/bundler/src/bundle/windows/msi/wix.rs Show resolved Hide resolved
tooling/bundler/src/bundle/windows/sign.rs Show resolved Hide resolved
@tr3ysmith
Copy link
Contributor Author

@amrbashir do I need to do anything to fix the clippy workflow failure?

@amrbashir
Copy link
Member

if you want, that would be appreciated, otherwise we can fix it in another PR

@amrbashir
Copy link
Member

cc @lucasfernog about #8718 (review)

@lucasfernog
Copy link
Member

I am not sure if we should support azuresigntool tbh since it is not an official tool (correct me if I am wrong), should we instead provide a generic option to specify a command to signing command, similar to this crabnebula-dev/cargo-packager@ec1a3a1/crates/packager/src/config/mod.rs#L1206-L1214?

It gets complicated when you think about how complicated it is to sign a Windows app.. we need a better solution, and this might be one (I still didn't run through it yet, hopefully when I get back from vacation).

@lucasfernog
Copy link
Member

@lucasfernog should we bump the rust min version on the main crate too? I had to bump it to 1.72 on the CLI crate because of the dependency for jsonschema -> aHash (aHash requires 1.72)

tkaitchuck/aHash#196

Yeah we should.. we need to do a better job on the msrv for the CLI

@tr3ysmith
Copy link
Contributor Author

@lucasfernog should we bump the rust min version on the main crate too? I had to bump it to 1.72 on the CLI crate because of the dependency for jsonschema -> aHash (aHash requires 1.72)
tkaitchuck/aHash#196

Yeah we should.. we need to do a better job on the msrv for the CLI

@lucasfernog @amrbashir should we then bump both the CLI and Core, or just CLI?

@tr3ysmith
Copy link
Contributor Author

if you want, that would be appreciated, otherwise we can fix it in another PR

I think we should do it in another PR, since it looks to be completely unrelated to this change

@lucasfernog
Copy link
Member

that's ok we can do that separately

@tr3ysmith
Copy link
Contributor Author

@lucasfernog MSRV for cli is updated

@WalrusSoup
Copy link

WalrusSoup commented Feb 16, 2024

I am not sure if we should support azuresigntool tbh since it is not an official tool (correct me if I am wrong), should we instead provide a generic option to specify a signing command, similar to this https://github.com/crabnebula-dev/cargo-packager/blob/ec1a3a12296275341fd59c083d98202de316805f/crates/packager/src/config/mod.rs#L1206-L1214?

+1. We just got our new digicert token and it would be nice to be able to just call our own signing script. Otherwise, I could see this just growing and growing to accommodate the dozens of signing approaches.

@amrbashir
Copy link
Member

I agree, @tr3ysmith would a generic command work for you as well?

github-merge-queue bot pushed a commit to firezone/firezone that referenced this pull request Mar 6, 2024
Refs #3230 

It looks like we need to sign the internal exe before it gets bundled
too. We can use `beforeBundleCommand` to do so.

Soon, Tauri should have native support for this exact scenario:
tauri-apps/tauri#8718
@tr3ysmith
Copy link
Contributor Author

@amrbashir sorry ya'll. I've had some crazy life stuff pop up. Yes a generic command would work for me as long as I can pass all the required arguments through via the tauri-action. I can work on that here in the next few days.

@amrbashir
Copy link
Member

thanks @tr3ysmith hope you're doing great, let me know if I can help with anything

@amrbashir
Copy link
Member

amrbashir commented May 24, 2024

Hi @tr3ysmith I have opened #9865 to implement the custom signing command, gonna close this PR now in favor of that one. Thanks for your contributions

@amrbashir amrbashir closed this May 24, 2024
@tr3ysmith
Copy link
Contributor Author

@amrbashir no problem, is this something that'll get merge into 1.x at all?

@amrbashir
Copy link
Member

there is a PR to backport it in #9902

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

Successfully merging this pull request may close these issues.

4 participants