-
-
Notifications
You must be signed in to change notification settings - Fork 243
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
Panic signing a particular executable file #616
Comments
Thanks for the reproduce case. The assertion is triggered when writing the Using
We see that the All the Mach-O binaries that I've seen so far don't leave a gap between segments in the file! Yet here we are with a Mach-O binary that has 16kb of 0s between the segments. The zero padding of segment / section data is using done within the declared segment boundaries, not by leaving a gap between the in-file segment offsets. Why the linker did this, I'm not sure. Probably a linker script or some linker argument forcing the padding. Anyway, leaving a gap between segments is valid. So we should catch this and write out the padding bytes to preserve the original file. Did you want to have a go at the fix? If not, I'll code it up. |
Actually, I'll just code up a patch. |
Before, Mach-O segments that had padding between segments crashed due to an assertion. Having padding between Mach-O segments is allowed - albeit not observed until a bug was filed. So this commit teaches the segment writing code to pad the space between segments with zeroes so this case is handled. Should fix #616.
The code signing process appears to succeed:
But the binary still gets sigkilled when I run it. Relevant logs from Console.app:
Signing with Apple's codesign does work (via If it's relevant, the binary was created by Clang 11 on Linux via osxcross. |
That's what I get for authoring the change on Linux and not testing :) The code signature contains content digests over chunks/windows of the Mach-O. These chunks stop prematurely at segment boundaries (presumably so the loader can easily map / verify entire segments). The bug here is likely that our digesting code isn't digesting the empty space between the segments. We'll need to teach it to do so. (Although I need to verify this by comparing signatures with Apple's tooling.) |
Looks like my theory about not including space between segments in the digests is correct. That's the good news. The bad news is the API around computing the segment windows to digest operates against a Maybe this is the opportunity I was looking for to migrate to the |
Before, Mach-O segments that had padding between segments crashed due to an assertion. Having padding between Mach-O segments is allowed - albeit not observed until a bug was filed. So this commit teaches the segment writing code to pad the space between segments with zeroes so this case is handled. Part of #616.
Something else wonky is going on here. There's a field in the embedded code signature that effectively states the total number of bytes being encapsulated by paged content digests. My belief is that this is the number of on-file bytes between the beginning of the file and the code signature. Essentially, the file-level offset of the code signature. Every Mach-O I've seen so far is this way. However, the binary you provided is Moreover, your binary is reporting If I run Anyway, I think I'm close to a fix here. I just wanted to report the wonkiness with the sample binary. |
Interesting - I can see a couple of possibilities there:
|
Before, Mach-O segments that had padding between segments crashed due to an assertion. Having padding between Mach-O segments is allowed - albeit not observed until recently. So this commit teaches the segment writing code to preserve the space between segments when writing out a Mach-O. We still fail to handle these gaps when computing code signatures. This will be addressed in a separate commit. Part of #588 and #616.
It seems like this particular binary causes apple-codesign to break for some reason:
Here's the offending binary in a tarball: cc1plus.tar.gz
The text was updated successfully, but these errors were encountered: