-
-
Notifications
You must be signed in to change notification settings - Fork 9.8k
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
keg: add codesigning #9102
keg: add codesigning #9102
Conversation
@@ -31,4 +33,37 @@ def change_install_name(old, new, file) | |||
EOS | |||
raise | |||
end | |||
|
|||
def apply_ad_hoc_signature(file) | |||
return if MacOS.version < :big_sur |
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.
Do we want to also limit this to ARM for now?
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.
I am waiting for feedback from @mistydemeo, who is checking whether Intel Big Sur codesigns by default or not.
If we want to limit to ARM, I don't know how to do in terms of Ruby calls. Can you suggest something?
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.
Xcode 12.2, even on ARM Big Sur, does not codesign for Intel:
% clang -arch x86_64 a.c
% file a.out
a.out: Mach-O 64-bit executable x86_64
% codesign -v a.out
a.out: code object is not signed at all
In architecture: x86_64
So I suppose you're right, we should limit to Intel. Help is welcome. Looking at os/mac/mach.rb
it looks like I should be able to simply call file.arch
? But maybe it's not handled, as the code does not appear to include Arm variants in the list…
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.
@fxcoudert Try Hardware::CPU.arm?
, it’s in hardware.rb
.
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.
Relaying some feedback received via DM from the security team of a large-ish tech company (paraphrased, with my own comments added in italics):
-
Shelling out to
install_name_tool
is preferred as it will handle the architectures and everything fine -
Re-signing already-signed artifacts may cause issues down the line if revocation is necessary
- This refers to e. g. malware which is certificate-signed and the certificate is later revoked by Apple. We can fix this by re-signing only if the original signature is linker-signed
-
If you don't
--force
, linker signed signatures will be silently replaced and existing ones will not (withcodesign
returning an error), which might be a better paradigm- I fully agree
-
Patching keg-installed artifacts is probably breaking the existing signatures anyway and will require a re-sign
- True, that’s one of the reasons why this PR exists
That's not really touched by this PR, but Homebrew chose to use a native Mach-O library to do that.
Signed binaries are currently incompatible with our relocation system anyway: we manipulate them and therefore invalidate the signature.
However, the binaries produced will then be invalid, because the signature won't match the content anymore. So we can't do that, because:
Yes. |
Fair points @fxcoudert. |
@MikeMcQuaid I've limited to ARM, as requested. I've been testing this (and earlier versions) for a week now, building many formulas with it (that are broken without) |
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.
Been using this patch actively since last night and my manual tests have shown this works as expected, both on Rosetta and native Apple Silicon.
$ codesign -vv /opt/dunnobrew/opt/nettle/bin/nettle-hash
/opt/dunnobrew/opt/nettle/bin/nettle-hash: code object is not signed at all
In architecture: x86_64
$ codesign -vv /opt/homebrew/opt/nettle/bin/nettle-hash
/opt/homebrew/opt/nettle/bin/nettle-hash: valid on disk
/opt/homebrew/opt/nettle/bin/nettle-hash: satisfies its Designated Requirement
@MikeMcQuaid @mistydemeo Would love to have one of you look at it for good measure. |
I tried to propose an implementation of the discussion in #9082
by building on @mistydemeo's #9041
codesign
fails the first timeTesting shows that this allows to build and install formulas on ARM, including the ones that trigger the codesign bug (like
gettext
andnettle
).Possible improvements: