-
Notifications
You must be signed in to change notification settings - Fork 52
Add warning on Windows if files aren't authenticode signed #1130
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
Adds Authenticode signature verification for Windows to improve security by warning when configuration files, resource manifests, or executable files are not properly signed. The implementation uses Windows API calls to verify digital signatures and logs warnings for unsigned files.
- Introduces a new security module with Authenticode signature verification functionality
- Integrates security checks into resource discovery and file processing workflows
- Adds localized warning messages for various signature validation failures
Reviewed Changes
Copilot reviewed 10 out of 13 changed files in this pull request and generated 4 comments.
Show a summary per file
File | Description |
---|---|
dsc_lib/src/security/mod.rs | Main security module entry point with file checking coordination |
dsc_lib/src/security/authenticode.rs | Windows-specific Authenticode signature verification implementation |
dsc_lib/src/lib.rs | Exports the new security module |
dsc_lib/src/dscresources/command_resource.rs | Adds security checks for resource executables |
dsc_lib/src/dscerror.rs | Defines new error types for Authenticode and which command failures |
dsc_lib/src/discovery/mod.rs | Integrates security checks into resource discovery process |
dsc_lib/locales/en-us.toml | Adds localized error messages for Authenticode validation |
dsc_lib/Cargo.toml | Adds Windows API dependencies for signature verification |
dsc/tests/dsc_security.tests.ps1 | Test coverage for security warning functionality |
dsc/src/util.rs | Adds security checks for configuration files |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
63235d5
to
7b9d745
Compare
a14f97d
to
ccad5d3
Compare
ccad5d3
to
a1fa23e
Compare
Please elaborate on why you are verifying the file |
Can we have an issue or rfc on this PR? I have concerns about the current design and how cross platform differences. |
This is linked to existing issues. Cross platform is not part of this PR as we are targeting Windows first. |
Thinking about this, I think it would be better to change this a bit so that whether a manifest or the target exe is signed should be part of the DscResource struct and shows up under |
Since |
PR Summary
If a configuration file, resources manifest, or the exe used by the resource manifest is not authenticode signed on Windows, you will get a warning message.
Also, when listing extensions and resources, there is a new
Trust
column/property indicating the trust level. On Linux/macOS, it's currently alwaysunknown
Future work will make it configurable if the warning is an error. Catalog signed files are currently out-of-scope.
The Win32 calls replicate the example from the docs https://learn.microsoft.com/en-us/windows/win32/seccrypto/example-c-program--verifying-the-signature-of-a-pe-file
Because of the new warning message, many existing tests were breaking expecting no messages so added setting
DSC_TRACE_LEVEL='error'
to many tests and removing it at end. Any explicit setting of trace level will override this.PR Context
Fix #210
Fix #327