fix: sbctl-batch-sign: Better handling of batch file signing by blacklisting known improper PE/COFF executables #129
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Using `read` in a while loop makes the script for foolproof because the script will not run on empty STDOUT. Filenames with spaces (who??) are also handled correctly now instead of being treated as different files in the previous implementation. Signed-off-by: Eric Naim <[email protected]>
Previously, the script was just grabbing all files recognized by sbctl and mass signing them. This was used to check if the script was working correctly (i.e. signing files with sbctl) but wasn't changed to filter for unsigned files instead after checking it worked correctly. I left it untouched because it didn't seem like a problem at the time, but issues are arising now due to this poorly written script. Signed-off-by: Eric Naim <[email protected]>
Filter absolute file paths using awk instead. Alternatively, `find` has -name to filter for filenames so there was never any reason to use grep here too. Signed-off-by: Eric Naim <[email protected]>
We expect users who use this script to enroll sbctl created keys alongside Microsoft's. With that in mind, there's no need for the script to go sign ESP files from Microsoft/Windows. This should better avoid crashes from sbctl due to unproper PE/COFF executables from Microsoft. Signed-off-by: Eric Naim <[email protected]>
1Naim
force-pushed
the
fix/sbctl-win-sign
branch
from
eb4baca to
bc8e15d
Compare
Signed-off-by: Eric Naim <[email protected]>
Technically these files can live anywhere in the boot directory, not just in Microsoft's own folder. Better safeguard against these files by placing additional checks. Signed-off-by: Eric Naim <[email protected]>
1Naim
force-pushed
the
fix/sbctl-win-sign
branch
from
bc8e15d to
d8d9f92
Compare
ptr1337
approved these changes
Feb 6, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Unfortunately, the prior version of the script caused massive issues for upstream[1][2] due to being poorly written and without any safeguards (mostly due to a lot of assumptions). The script is now hopefully better implemented due better protect against these issues by:
sbctlorfind, and hypothetical filenames with spaces.[1] Foxboron/sbctl#414
[2] Foxboron/sbctl#376