-
Notifications
You must be signed in to change notification settings - Fork 38.8k
build: Fix macOS code signing by pre-allocating space for the code signature during gitian build #20638
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
Conversation
|
Needs backport to all branches that we plan on making further releases from. We also need to verify that after doing the code signing that the behavior is what we expect, so @jonasschnelli will need to make a code signature for this PR. |
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.
an aside but 500k is crazy large for a cryptographic signature, i only know some of the quantum-hard signature algorithms produce such large signatures, is it known what algorithm they're using?
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 assume it's just because without --pagesize 0, it splits the binary into pages, and signs every page separately. If a page is 4 kB by default (I don't know what the default is), it's not too surprising to get a huge result.
With --pagesize 0 the signature is just 20 kB. I believe it has still that size because "resources" that are embedded in the binary are still signed separately.
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 believe it's RSA. The reason the signature is so big is because it's actually multiple signatures. By default, the binary is signed in page sized chunks with each piece getting its own signature. Or at least I think that's what the code is doing.
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.
thanks!
yes 20kB isn't that surprising to me for RSA+metadata, and if it signs every page separately that definitely explains things
|
Concept ACK, thanks for investigating this weird problem. |
ba8a527 to
73d1a54
Compare
|
MacOS Gitian build of 73d1a541147e72b07ce0e6c3a23ca6e94f74c3ce |
73d1a54 to
3ca08c2
Compare
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ConflictsNo conflicts as of last run. |
|
I've mostly worked out why the code signature is so big. The signature is primarily composed of hashes of 4096 byte pages of the unsigned binary. Then there is ~8kb of signature, and another ~10kb of overhead that I'm not quite sure. But given this, we can get a more accurate estimate of the actual size of the code signature. With that in mind, I've dropped the commit that does |
3ca08c2 to
ef0ea90
Compare
|
MacOS Gitian build for ef0ea90 |
|
Approach ACK. Nice work. |
ef0ea90 to
6753b74
Compare
|
Concept ACK 💖 |
|
utACK 6753b74195d80a75b3bceef459d767906bea8e7d |
737bf4e to
7f9d623
Compare
|
Apparently doing I was able to test the entire process using a self-signed certificate. The initial gitian build works, I was able to sign create the detached signature, the Gitian signer build works, and the resulting binary validates. I also did a sign of the unmodified unsigned binary. This had a different Gitian build results: Here is the signature tarball if anyone wants to test the attaching: signature-osx.tar.gz Signed binary result: |
|
Tested ACK 7f9d623e0210ff539455b0d8dd70b31f14231b8b c587da1413ef8f942b4fb90bd077c93b883cfdcaf86c1965ba63ba83eabd0601 bitcoin-7f9d623e0210-osx-unsigned.dmg
3fcfd6cf91018a0afeefec43bc226eb61eed5d2cd41aa7880be3700abf8555cd bitcoin-7f9d623e0210-osx-unsigned.tar.gz
2b01414c875ac85e79dbb5df4eff863db29e99be8a0eec43fb1e9940298b4d3f bitcoin-7f9d623e0210-osx64.tar.gz
a017e7c291f81dd16b24368dc513303b65deb71d5d144f3726facee03605eb08 src/bitcoin-7f9d623e0210.tar.gzsigned it offline (signature is here). c1ae8014f7a404212b0c43e4e53e7ce888845af72f1797a169c09445b7da9d92 bitcoin-osx-signed.dmgSuccessfully verified the signature. |
|
I think this solution is less fragile than #20644. |
|
@jonasschnelli Maybe, and I'm happy with either. But in the end both solutions are attempts to appease/mimick |
|
@sipa: agree. Both solutions are workarounds. I'd like to file a bug at apple and see how they react which might lead to an longterm solution. |
|
MacOS Gitian build of 7f9d623e0210ff539455b0d8dd70b31f14231b8b: Which is different, except for the source... sorry to spoil the party. Maybe I did something wrong? I'm not sure how to apply @jonasschnelli's signature (if that's useful for this PR). Update 18:39 CET: I nuked cache and tried again, but it's the same result I built v0.21.0rc3 and the macOS checksums do seem to match the other contributors: bitcoin-core/gitian.sigs#1388 |
|
Concept ACK |
The tool codesign_allocate compiled from native_cctools (based on Apple's published source code) and the codesign_allocate shipped by Apple in its Xcode command line tools behave differently when it comes to rounding to a page size for a field named "vmsize". The open source one rounds to 0x1000 while the binary rounds to 0x2000. This can result in a situation where a valid signature is made, but when we attach it o the unsigned binary during Gitian builds, a different binary comes out that fails the signature validation. To work around this, we rely on the behavior that codesign_allocate does not reduce "vmsize", only increases it. At the end of the Gitian build step when we have created the unsigned binary, we use codesign_allocate to allocate a large amount of space at the end of the file for the code signature. This will set "vmsize" to something fairly high. Then during signing, Apple's codesign tool will call codesign_allocate and reallocate space for the signature. However this only effects the real size of the binary, not the "vmsize". So our larger "vmsize" remains, using the rounding of the native_cctools version of codesign_allocate. Thus we can ensure that "vmsize" remains the same throughout the signing process and then the subsequent attaching.
7f9d623 to
159d203
Compare
|
MacOS Gitian build of 159d203 (after nuking cache): |
|
utACK 159d203. I reviewed the code and the approach matches our understanding of what went wrong. It's a pretty hacky solution and who knows what requirements the Apple codesigner will have in the future. But in the short term, based on reports of it being tested by others, it appears to work. |
…le's a4118c6 Add patch to make codesign_allocate compatible with Apple's (Pieter Wuille) Pull request description: This is an alternative to #20638. The problem is that Apple's codesign(_allocate) apparently rounds the "vmsize" attribute on the __LINKEDIT section to a multiple of 0x2000 on x86_64 rather than 0x1000 (as their published source code does). This divergence means that the binary signed by codesign is slightly different from the one recreated by our reattach-sig-to-gitian-output process, and the signature being invalid. This fixes it by patching our codesign_allocate source code to also use 0x2000. In tests, this appears to result in matching binaries. ACKs for top commit: jonasschnelli: Tested ACK a4118c6 - removed the osx cache, built commit a4118c6 for osx in gitian (dependency where built, patch was applied), signed on my signing mac (detach-sig-create), ran gitian osx signer with the produces signature and the a4118c6 build (detach-sig-apply), signature then was successful verified on my Mac (codesign -v /Volumes/Bitcoin-Core/Bitcoin-Qt.app) MarcoFalke: Concept ACK a4118c6 Tree-SHA512: 07b8cdf8216249ddfe4bd38b39f2b48b2e190d4002b84d8981e62197bbbc9f25ac5c137bcc32057b23fbf38cbb2889ef95101ce008edfbf608cd170b88b3acbc
…ith Apple's a4118c6 Add patch to make codesign_allocate compatible with Apple's (Pieter Wuille) Pull request description: This is an alternative to bitcoin#20638. The problem is that Apple's codesign(_allocate) apparently rounds the "vmsize" attribute on the __LINKEDIT section to a multiple of 0x2000 on x86_64 rather than 0x1000 (as their published source code does). This divergence means that the binary signed by codesign is slightly different from the one recreated by our reattach-sig-to-gitian-output process, and the signature being invalid. This fixes it by patching our codesign_allocate source code to also use 0x2000. In tests, this appears to result in matching binaries. ACKs for top commit: jonasschnelli: Tested ACK a4118c6 - removed the osx cache, built commit a4118c6 for osx in gitian (dependency where built, patch was applied), signed on my signing mac (detach-sig-create), ran gitian osx signer with the produces signature and the a4118c6 build (detach-sig-apply), signature then was successful verified on my Mac (codesign -v /Volumes/Bitcoin-Core/Bitcoin-Qt.app) MarcoFalke: Concept ACK a4118c6 Tree-SHA512: 07b8cdf8216249ddfe4bd38b39f2b48b2e190d4002b84d8981e62197bbbc9f25ac5c137bcc32057b23fbf38cbb2889ef95101ce008edfbf608cd170b88b3acbc
|
On IRC it was decided decided in favor of #20644 because it looks like it more closely mimics the change that Apple made in the binary-only upstream. There was some worry that it might have by-effects for other tools used in production of the macos binary, so if it turns out that rc4 has other issues we could revert it and still go for this (slightly harder to grasp, but more targeted) solution for rc5. |
|
Closing for now. Will reopen if needed. |
|
I was able to run even the wrongly signed binary v0.21.0rc3 from the .dmg after setting an extended attribute via: |
With 0.21.0rc3, we found that the code signature for the MacOS dmg was invalid. Further investigation revealed that this was due to a difference in the behavior of
codesign_allocatefrom native_cctools andcodesign_allocatedistributed by Apple with the Xcode command line tools package. The result is that thevmsizeof the__LINKEDITsection of the binary differed between the two which made the signature produced invalid for our gitian built binary.codesign_allocateis used by thecodesigntool to allocate space in the binary for the code signature before signing. During the gitian build where the signature is attached, thedetached-sig-apply.shscript also usescodesign_allocateto allocate space in the binary before the code signature is attached. Thus we need both versions to have the same behavior. But what we observe is that the Apple distributedcodesign_allocatewill compute avmsizethat is rounded to 0x2000, while thecodesign_allocatefrom native_cctools (that we use in gitian) rounds to 0x1000. So thisvmsizefield in the__LINKEDITsection differs.To work around this problem, we use the fact that
codesign_allocatedoesn't reduce the size ofvmsize, only increases it. We can thus pre-allocate space for the code signature before signing even happens, and when signing happens, the execution ofcodesign_allocatewill not change thevmsize, it only effects the real size of the binary. This ensures thatvmsizeis set before signing occurs so that the only thing that we really need to do is actually just attaching the signature to the binary. This PR uses a fixed pre-allocation value of 500000 because the largest MacOS code signature since 0.12.1 that I could find was 407424 bytes.Additionally, the
codesigntool has an option to change the "page size". It appears that the tool signs the executable split into pages. But if we do--pagesize 0, it will sign the binary as a single large page. This should reduce the size of the code signature and also make it more consistent. This makes it much more likely that the signature will be smaller than the pre-allocation of 500000 bytes.