Add patch to make codesign_allocate compatible with Apple's

sipa commented at 7:37 pm on December 13, 2020: member

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.

sipa marked this as a draft on Dec 13, 2020

sipa force-pushed on Dec 13, 2020

DrahtBot added the label Build system on Dec 13, 2020

Add patch to make codesign_allocate compatible with Apple's a4118c6e20

sipa force-pushed on Dec 13, 2020

achow101 commented at 1:33 am on December 14, 2020: member

Tried this with a self signed certificate. ~~Did not work and vmsize was still a multiple of 0x1000~~

Apparently my gitian is doing something incorrectly. Running the apply locally works as expected and the correct binary is produced.

jonasschnelli commented at 7:44 am on December 14, 2020: contributor

Tested ACK a4118c6e200e02e7560f8bc213697aa2909d95b1 - removed the osx cache, built commit a4118c6e200e02e7560f8bc213697aa2909d95b1 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 a4118c6e200e02e7560f8bc213697aa2909d95b1 build (detach-sig-apply), signature then was successful verified on my Mac (codesign -v /Volumes/Bitcoin-Core/Bitcoin-Qt.app)

MarcoFalke added the label Needs gitian build on Dec 14, 2020

DrahtBot commented at 5:43 am on December 15, 2020: member

Gitian builds

File	commit eec9366f7dd60e522ab339d69a0052a57598829a(master)	commit 31bfd2e433dd526464096e8f38ba1f49411dcd0b(master and this pull)
bitcoin-core-linux-22-res.yml	`caad1af8b3475755...`	`00851859c56a74ae...`
bitcoin-core-osx-22-res.yml	`3bddb6b4015d0e9d...`	`f816f03f12ff6a21...`
bitcoin-core-win-22-res.yml	`c59950d0031b62e3...`	`e3796b3f030f5dbc...`
*-aarch64-linux-gnu-debug.tar.gz	`6a2e5aa01a536671...`	`ca976b32630f326f...`
*-aarch64-linux-gnu.tar.gz	`948f89703e4cb382...`	`4556bd5a4324ed1b...`
*-arm-linux-gnueabihf-debug.tar.gz	`14f17381f0384ae0...`	`0f29b6c666bf6fac...`
*-arm-linux-gnueabihf.tar.gz	`4efabff95bcb5e83...`	`b877c11e1f78598e...`
*-osx-unsigned.dmg	`d21116cc35358ed2...`	`3f9df3878506a86e...`
*-osx64.tar.gz	`c0bd9ab7e7cfc9c5...`	`9e90928a2e943937...`
*-riscv64-linux-gnu-debug.tar.gz	`99118103d12d1481...`	`038a9260144205e3...`
*-riscv64-linux-gnu.tar.gz	`70529a59e9c10989...`	`c3698393ece83975...`
*-win64-debug.zip	`8d3ae190278bc27e...`	`639af3228a260cf1...`
*-win64-setup-unsigned.exe	`2726732d5dc84e6c...`	`d0d8d78a4b3069ab...`
*-win64.zip	`76f84b3b35661702...`	`5503e485f3f0a8d3...`
*-x86_64-linux-gnu-debug.tar.gz	`79cdcff7e7ca8e17...`	`6b292d5f6873a285...`
*-x86_64-linux-gnu.tar.gz	`c72263ec5ee2aee3...`	`1a308d34c3570743...`
*.tar.gz	`dc2391bb1f1e716e...`	`f610cd75b84033b5...`
linux-build.log	`2b46d6d6dec7a861...`	`2801729fe970a2ea...`
osx-build.log	`e7cea61e8c3348d1...`	`623a5e4796e31d80...`
win-build.log	`aefa4d9075aa6f7c...`	`6b91a0eb8e3d489b...`
bitcoin-core-linux-22-res.yml.diff		`a802e5dbf1144b84...`
bitcoin-core-osx-22-res.yml.diff		`29440e9d277d80fa...`
bitcoin-core-win-22-res.yml.diff		`1fcbd5bae2f329d4...`
linux-build.log.diff		`bece3b1301c590aa...`
osx-build.log.diff		`45e8069606d0a956...`
win-build.log.diff		`06816c34dda0ba0d...`

DrahtBot removed the label Needs gitian build on Dec 15, 2020

fanquake commented at 1:22 pm on December 16, 2020: member

While this patch looks simple, it feels like black magic (at least without the PR description), so I currently prefer #20638. Also given this is patching libstuff, the change ends up in all of the tools, rather than being targeted to codesign* in some way.

MarcoFalke marked this as ready for review on Dec 17, 2020

MarcoFalke commented at 7:47 pm on December 17, 2020: member

Concept ACK a4118c6e200e02e7560f8bc213697aa2909d95b1

sipa commented at 7:50 pm on December 17, 2020: member

Updated PR description. @fanquake It’s easy to trace which functions in cctools call get_segalign_from_flag (the only function that accesses the modified field): it’s codesign_allocate, lipo, segedit, and bitcode_strip. I believe we only use the former.

jonasschnelli added the label Needs backport (0.21) on Dec 17, 2020

jonasschnelli added the label Needs backport (0.20) on Dec 17, 2020

laanwj merged this on Dec 17, 2020

laanwj closed this on Dec 17, 2020

MarcoFalke referenced this in commit 35a10e4ebc on Dec 17, 2020

sidhujag referenced this in commit 646c6a8bad on Dec 17, 2020

MarcoFalke added the label Needs backport (0.19) on Dec 18, 2020

fanquake removed the label Needs backport (0.21) on Dec 21, 2020

fanquake commented at 3:52 am on December 21, 2020: member

Being backported to 0.21 in #20669.

MarcoFalke referenced this in commit 2f510296a7 on Dec 21, 2020

MarcoFalke referenced this in commit 21a13a7a5e on Dec 21, 2020

fanquake referenced this in commit 85a3ca33f6 on Dec 22, 2020

fanquake referenced this in commit bd943d1959 on Dec 22, 2020

fanquake removed the label Needs backport (0.19) on Dec 22, 2020

fanquake removed the label Needs backport (0.20) on Dec 22, 2020

fanquake commented at 5:42 am on December 22, 2020: member

Being backported to 0.19 in #20739 & 0.20 in #20738.

MarcoFalke referenced this in commit cfccb939fa on Dec 22, 2020

fanquake referenced this in commit b847e63a47 on Dec 22, 2020

fanquake referenced this in commit 5d5cf12738 on Dec 22, 2020

MarcoFalke referenced this in commit b40254b232 on Jan 5, 2021

sidhujag referenced this in commit 4a1cf95e52 on Jan 5, 2021

DrahtBot locked this on Feb 15, 2022

Add patch to make codesign_allocate compatible with Apple’s #20644

Gitian builds