-
Notifications
You must be signed in to change notification settings - Fork 3.4k
Validate authenticode signature using the certificate Subject #12474
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
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR updates authenticode signature verification to match on the certificate’s Subject rather than its thumbprint, preventing failures when certificates are renewed.
- Removed
signature
fields from toolset JSONs. - Extended
Install-Binary
andTest-FileSignature
to useExpectedSubject
and compare certificate .Subject. - Updated all scripts to pass the new
ExpectedSubject
(and introducedGet-MicrosoftPublisher
for Microsoft-signed binaries).
Reviewed Changes
Copilot reviewed 25 out of 25 changed files in this pull request and generated 1 comment.
Show a summary per file
File | Description |
---|---|
images/windows/toolsets/toolset-*.json | Removed manifest signature entries for all toolsets |
images/windows/scripts/helpers/VisualStudioHelpers.ps1 | Switched Test-FileSignature call to use ExpectedSubject |
images/windows/scripts/helpers/InstallHelpers.ps1 | Renamed parameter/docs to ExpectedSubject and logic update |
images/windows/scripts/helpers/ImageHelpers.psm1 | Exported new Get-MicrosoftPublisher helper |
All images/windows/scripts/build/*.ps1 scripts |
Replaced -ExpectedSignature arguments with -ExpectedSubject |
Comments suppressed due to low confidence (4)
images/windows/scripts/helpers/InstallHelpers.ps1:37
- Consider updating the module's README or an external docs file to reflect the new
ExpectedSubject
parameter and deprecateExpectedSignature
.
.EXAMPLE
images/windows/scripts/helpers/InstallHelpers.ps1:83
- Add unit or integration tests for
Test-FileSignature
covering both valid and invalid subject scenarios to ensure new subject-based validation is exercised.
Test-FileSignature -Path $filePath -ExpectedSubject $ExpectedSubject
images/windows/scripts/helpers/ImageHelpers.psm1:22
- [nitpick] The name
Get-MicrosoftPublisher
could be more descriptive asGet-MicrosoftPublisherSubject
to clarify that it returns a certificate subject string.
'Get-MicrosoftPublisher'
images/windows/scripts/helpers/VisualStudioHelpers.ps1:53
- [nitpick] For readability and to avoid repeated calls, consider assigning
$(Get-MicrosoftPublisher)
to a local variable before passing it toTest-FileSignature
.
Test-FileSignature -Path $bootstrapperFilePath -ExpectedSubject $(Get-MicrosoftPublisher)
@@ -30,7 +30,7 @@ $mysqlVersionUrl = "https://cdn.mysql.com/Downloads/MySQL-${mysqlVersionMajorMin | |||
|
|||
Install-Binary ` | |||
-Url $mysqlVersionUrl ` | |||
-ExpectedSignature (Get-ToolsetContent).mysql.signature | |||
-ExpectedSubject 'CN="Oracle America, Inc.", O="Oracle America, Inc.", L=Redwood City, S=California, C=US, SERIALNUMBER=2101822, OID.2.5.4.15=Private Organization, OID.1.3.6.1.4.1.311.60.2.1.2=Delaware, OID.1.3.6.1.4.1.311.60.2.1.3=US' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] This issuer subject string is lengthy and duplicated inline; consider centralizing common subjects (e.g., Oracle, Google) in helper functions or config to reduce repetition.
Copilot uses AI. Check for mistakes.
Description
This PR switches verification of authenticode signature to use the certificate subject instead of the thumbprint. This prevents occasional verification failures caused by certficate renewal.
Related issue: https://github.com/github/hosted-runners-images/issues/111
Check list