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

Automate Code Signing of Windows Binary #1763

Merged

Conversation

phargogh
Copy link
Member

This PR restores automated codesigning as a part of our GHA-based build process. Configuration (minus secrets) has been provided for a queue implemented as a GCS-backed cloud function, and a worker running as a debian:bookworm systemd service. Our github actions configuration and secrets have been updated to use the new codesigning.

I would also like to add that there are several enhancements that I think we should look into long term, including:

  • Using identity federation to manage access to the function instead of the primitive token-based approach used
  • Triggering the enqueue operation based on GCS events instead of an HTTP request

I have removed the old windows codesigning stuff that is no longer relevant. I think make codesign_windows should still work on a windows computer that has a yubikey (and corresponding minidriver) available, but we aren't using it for our build automation any longer. I left the recipe in the Makefile in case we need to sign locally.

This has been tested and is working on my fork (example signed dev build binary)

Fixes #1580

Checklist

  • Updated HISTORY.rst and link to any relevant issue (if these changes are user-facing)
  • Updated the user's guide (if needed)
  • Tested the Workbench UI (if relevant)

@phargogh phargogh requested a review from emlys February 1, 2025 02:42
Copy link
Member

@dcdenu4 dcdenu4 left a comment

Choose a reason for hiding this comment

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

Thanks @phargogh for getting this workflow set up so quickly. It looks good to me, just a few grammatical suggestions.

I had a few general questions for my understanding though:

  • The signed windows binary will end up on GCS, but the github artifact will be unsigned?
  • Does this play nicely with the automated release workflows, thinking about the above?

phargogh and others added 3 commits February 3, 2025 09:47
@phargogh
Copy link
Member Author

phargogh commented Feb 3, 2025

@dcdenu4 great point about using the signed binary in the release! I just updated this in 782dbd1 so that the binary is fetched from GCS at release time. Because we have a human review step, it's pretty likely that the binary will be signed before the release part 2 automation is run, but it isn't a guarantee if we are super quick on the draw for merging the release PR or if there are a lot of binaries in the queue to sign.

FYI @emlys

@phargogh phargogh requested a review from dcdenu4 February 4, 2025 01:12
Copy link
Member

@dcdenu4 dcdenu4 left a comment

Choose a reason for hiding this comment

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

Thanks @phargogh , looks good to me!

@phargogh
Copy link
Member Author

phargogh commented Feb 4, 2025

OK! @dcdenu4 I'm re-requesting your review because there have been a few nontrivial changes in response to our discussion during the team meeting.

@emlys The only changes since your approval have been bugfix-related, so pretty minor :)

The main changes since our team discussion are now:

  • Each binary now has a .exe.signature file sitting alongside it on GCS in the same directory as the binary. This .signature file is a plain-text file with info about the certificate.
  • The cloud function now checks the existence of .exe.signature to see whether a file has already been signed.
  • When a file is pulled down to the worker, we first check to see if the binary is already signed. If it is, we re-upload the .exe.signature file but don't re-upload the .exe file.
  • Slack notifications now go to the #sw-invest channel.

@phargogh phargogh requested a review from dcdenu4 February 4, 2025 23:46
Copy link
Member

@dcdenu4 dcdenu4 left a comment

Choose a reason for hiding this comment

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

Thanks @phargogh ! This looks good to me, just a conflict with History :)

@phargogh phargogh requested a review from dcdenu4 February 5, 2025 17:47
@dcdenu4 dcdenu4 merged commit f1c0bb6 into natcap:main Feb 5, 2025
29 checks passed
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.

Automate code-signing as part of our release process
3 participants