Skip to content

Conversation

@achow101
Copy link
Member

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_allocate from native_cctools and codesign_allocate distributed by Apple with the Xcode command line tools package. The result is that the vmsize of the __LINKEDIT section of the binary differed between the two which made the signature produced invalid for our gitian built binary.

codesign_allocate is used by the codesign tool to allocate space in the binary for the code signature before signing. During the gitian build where the signature is attached, the detached-sig-apply.sh script also uses codesign_allocate to 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 distributed codesign_allocate will compute a vmsize that is rounded to 0x2000, while the codesign_allocate from native_cctools (that we use in gitian) rounds to 0x1000. So this vmsize field in the __LINKEDIT section differs.

To work around this problem, we use the fact that codesign_allocate doesn't reduce the size of vmsize, only increases it. We can thus pre-allocate space for the code signature before signing even happens, and when signing happens, the execution of codesign_allocate will not change the vmsize, it only effects the real size of the binary. This ensures that vmsize is 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 codesign tool 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.

@achow101
Copy link
Member Author

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.

Copy link
Member

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?

Copy link
Member

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.

Copy link
Member Author

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.

Copy link
Member

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

@laanwj
Copy link
Member

laanwj commented Dec 12, 2020

Concept ACK, thanks for investigating this weird problem.

@achow101 achow101 force-pushed the mac-codesign-fixed-alloc branch from ba8a527 to 73d1a54 Compare December 12, 2020 17:33
@achow101
Copy link
Member Author

MacOS Gitian build of 73d1a541147e72b07ce0e6c3a23ca6e94f74c3ce

bf682a0e960d153a6c862ad4af4f7fce584df3139ee74ecfd6c864ea91bf5c63  bitcoin-73d1a541147e-osx-unsigned.dmg
836a13543e82062074400b7ef82d4e62ffc16a4f030c3e89e500e6164dca58ab  bitcoin-73d1a541147e-osx-unsigned.tar.gz
87ba11ddf0fe08c531afd50ba2e638b1d74a857fcbf3f9a2a1dac1c2c515881e  bitcoin-73d1a541147e-osx64.tar.gz
e6ee88e761dc0894daf6b094768453f9baea5a26209ecb64b44000cc6030dbdd  src/bitcoin-73d1a541147e.tar.gz

@jonasschnelli
Copy link
Contributor

Concept ACK
Thanks @achow101 and @sipa for investigating this cumbersome issue.

Will do a gitian build (+ manual code signatures) asap.

@achow101 achow101 force-pushed the mac-codesign-fixed-alloc branch from 73d1a54 to 3ca08c2 Compare December 12, 2020 19:18
@DrahtBot
Copy link
Contributor

DrahtBot commented Dec 12, 2020

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Conflicts

No conflicts as of last run.

@achow101
Copy link
Member Author

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 --pagesize 0. As we can kind of predict what the signature size will be, it isn't necessary to make the signature as small as possible. Additionally, I believe MacOS is doing something with that list of hashes, perhaps an integrity check of each page as it is loaded into memory, so changing that may not be a good idea.

@achow101 achow101 force-pushed the mac-codesign-fixed-alloc branch from 3ca08c2 to ef0ea90 Compare December 12, 2020 19:25
@achow101
Copy link
Member Author

MacOS Gitian build for ef0ea90

e0f51c79af8598f8b99ed2341a476d97b846d2d556d375c3d93a163c77b6118a  bitcoin-ef0ea90d3e81-osx-unsigned.dmg
017b20d59ef2c2594606dd0c3c1de11ab36c56b5cc34c2add1c78fc8db698b56  bitcoin-ef0ea90d3e81-osx-unsigned.tar.gz
98560f5bef1938303a1f55448ba087f76ec7f35205ae64fec0efd129188fd8e1  bitcoin-ef0ea90d3e81-osx64.tar.gz
8bd279c7439e19d783e70f2cf425c40b3306b97b9b2410297c2cb3e48f3bef33  src/bitcoin-ef0ea90d3e81.tar.gz

@jonatack
Copy link
Member

Approach ACK. Nice work.

@achow101 achow101 changed the title Mac codesign fixed alloc build: Fix macOS code signing by pre-allocating space for the code signature during gitian build Dec 13, 2020
@decryp2kanon
Copy link
Contributor

Concept ACK 💖

@sipa
Copy link
Member

sipa commented Dec 13, 2020

utACK 6753b74195d80a75b3bceef459d767906bea8e7d

@achow101 achow101 force-pushed the mac-codesign-fixed-alloc branch 4 times, most recently from 737bf4e to 7f9d623 Compare December 13, 2020 23:30
@achow101
Copy link
Member Author

Apparently doing codesign_allocate before the actual code signing causes codesign to fail. This is because the tool detects that there is supposed to be a signature there. It then attempts to parse the signature and verify it. However because it is just 0's, it fails to do this and errors. To work around this issue, after doing codesign_allocate, we can apply a bogus signature. Since we pass the -f/--force option to codesign already, this bogus signature will be overwritten with the correct signature during actual signing. But because a validly formatted signature is present, codesign does not error when it encounters the bogus signature. For the bogus signature, I am just using the rc3 signature. This bogus signature is only attached to the binary that the codesigner signs, not the unsigned binary that we upload.

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 vmsize than the final result`, indicating that this trick will work.

Gitian build results:

c587da1413ef8f942b4fb90bd077c93b883cfdcaf86c1965ba63ba83eabd0601  bitcoin-7f9d623e0210-osx-unsigned.dmg
3fcfd6cf91018a0afeefec43bc226eb61eed5d2cd41aa7880be3700abf8555cd  bitcoin-7f9d623e0210-osx-unsigned.tar.gz
2b01414c875ac85e79dbb5df4eff863db29e99be8a0eec43fb1e9940298b4d3f  bitcoin-7f9d623e0210-osx64.tar.gz
a017e7c291f81dd16b24368dc513303b65deb71d5d144f3726facee03605eb08  src/bitcoin-7f9d623e0210.tar.gz

Here is the signature tarball if anyone wants to test the attaching: signature-osx.tar.gz

Signed binary result:

28c5fe82d1a93d096059e7a35a57286c28f57b21b7688dc523df39d211c17ae1  bitcoin-osx-signed.dmg

@jonasschnelli
Copy link
Contributor

Tested ACK 7f9d623e0210ff539455b0d8dd70b31f14231b8b
built this in gitian an got:

c587da1413ef8f942b4fb90bd077c93b883cfdcaf86c1965ba63ba83eabd0601  bitcoin-7f9d623e0210-osx-unsigned.dmg
3fcfd6cf91018a0afeefec43bc226eb61eed5d2cd41aa7880be3700abf8555cd  bitcoin-7f9d623e0210-osx-unsigned.tar.gz
2b01414c875ac85e79dbb5df4eff863db29e99be8a0eec43fb1e9940298b4d3f  bitcoin-7f9d623e0210-osx64.tar.gz
a017e7c291f81dd16b24368dc513303b65deb71d5d144f3726facee03605eb08  src/bitcoin-7f9d623e0210.tar.gz

signed it offline (signature is here).
Did a gitian osx signer and got.

c1ae8014f7a404212b0c43e4e53e7ce888845af72f1797a169c09445b7da9d92  bitcoin-osx-signed.dmg

Successfully verified the signature.

@jonasschnelli
Copy link
Contributor

I think this solution is less fragile than #20644.

@sipa
Copy link
Member

sipa commented Dec 14, 2020

@jonasschnelli Maybe, and I'm happy with either. But in the end both solutions are attempts to appease/mimick codesigns behavior, and changes in it could break either solution.

@DrahtBot
Copy link
Contributor

Gitian builds

File commit ade38b6
(master)
commit 0c9b41f025f54d8b37c6f459bafa8eb030b5ef6a
(master and this pull)
*-aarch64-linux-gnu-debug.tar.gz c3a964aaa2ba9589... 03e669743580ee7d...
*-aarch64-linux-gnu.tar.gz 00b5f52578d5f27a... 9e53cc68eb2b29c4...
*-arm-linux-gnueabihf-debug.tar.gz 772d924c796545b6... 42f660969d8d0817...
*-arm-linux-gnueabihf.tar.gz 7a02bb412e8732e7... a816024c97b741d1...
*-osx-unsigned.dmg 4ac83142579bad75... 3f50568365f93486...
*-osx64.tar.gz df9a5699ec87e5fa... fd97f30aa6dda450...
*-riscv64-linux-gnu-debug.tar.gz dc05951ab640be51... 79a201d39c6e6bac...
*-riscv64-linux-gnu.tar.gz a77409c55f1f91ef... b6a954c2f10bf270...
*-win64-debug.zip 734b1193bb298f3e... b19ab6eb45681e51...
*-win64-setup-unsigned.exe a5ed4f4745a499d6... d61e158e8af66c23...
*-win64.zip 8e06e2f0b0327249... 14908ba07ca9cd10...
*-x86_64-linux-gnu-debug.tar.gz 29a0600a42fe1633... 9fc1ff2924d3cf79...
*-x86_64-linux-gnu.tar.gz 908c33411d596431... 4d7e085d8d044d10...
*.tar.gz 14fa3e6ed91f532a... 3ac641b02fec1e97...
bitcoin-core-linux-22-res.yml 8a4ffcdd65fb2377... 9771c70b6f3d4908...
bitcoin-core-osx-22-res.yml 8e505c86e522825b... 8ef5b40b95f19a43...
bitcoin-core-win-22-res.yml 7af634a65b8fb989... 304d98f495cd42b0...
linux-build.log 0c9ff44af9c73585... 2fb62f35dce2ebd5...
osx-build.log 589e548ec0f5edec... aa745f2ca4c8ed2f...
win-build.log 16ab7ba60e13c962... 55d665f1437af947...
bitcoin-core-linux-22-res.yml.diff 0b67347549a0562a...
bitcoin-core-osx-22-res.yml.diff 2b5b96236abaea32...
bitcoin-core-win-22-res.yml.diff 933a23c0289317e7...
linux-build.log.diff 13e89eb768faea62...
osx-build.log.diff 9ef17fb344188b56...
win-build.log.diff a39972f9cdb8fb2a...

@jonasschnelli
Copy link
Contributor

jonasschnelli commented Dec 14, 2020

@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.

@Sjors
Copy link
Member

Sjors commented Dec 14, 2020

MacOS Gitian build of 7f9d623e0210ff539455b0d8dd70b31f14231b8b:

494ccf46f66a57bf93bafd5308cadb1a56ee0fe152bb7f378ce9eea2b63b7354  bitcoin-7f9d623e0210-osx-unsigned.dmg
2110b65d5223980121602ed4cb0e50558278877148ee59d445a56bd2542014d7  bitcoin-7f9d623e0210-osx-unsigned.tar.gz
3cfedace3c8d0553450db723aed2f5c3d0a5870b3f5b95e735314232a3fea32e  bitcoin-7f9d623e0210-osx64.tar.gz
a017e7c291f81dd16b24368dc513303b65deb71d5d144f3726facee03605eb08  src/bitcoin-7f9d623e0210.tar.gz

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

@fanquake
Copy link
Member

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.
@achow101 achow101 force-pushed the mac-codesign-fixed-alloc branch from 7f9d623 to 159d203 Compare December 16, 2020 23:19
@Sjors
Copy link
Member

Sjors commented Dec 17, 2020

MacOS Gitian build of 159d203 (after nuking cache):

f3a02590e3223fa9a02989530b427e148e886302bbcf6a2368955c06aca42434  bitcoin-159d203017b6-osx-unsigned.dmg
33a91c9555828e454ce4127a64685fa4020a688de645e7b9303d2644d1ce2d67  bitcoin-159d203017b6-osx-unsigned.tar.gz
99a41ee9468b474d24c1f7cc0273a0d07efabd232cf8fa9b6f788a8df8518a88  bitcoin-159d203017b6-osx64.tar.gz
2c31464f5b2c4a41b846b9e161f1e6cfbcf094714233a04dcda5fa5e2328434b  src/bitcoin-159d203017b6.tar.gz

@sipa
Copy link
Member

sipa commented Dec 17, 2020

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.

laanwj added a commit that referenced this pull request Dec 17, 2020
…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
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Dec 17, 2020
…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
@laanwj
Copy link
Member

laanwj commented Dec 18, 2020

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.

@achow101
Copy link
Member Author

Closing for now. Will reopen if needed.

@Emzy
Copy link
Contributor

Emzy commented Dec 23, 2020

I was able to run even the wrongly signed binary v0.21.0rc3 from the .dmg after setting an extended attribute via:

sudo xattr -rd com.apple.quarantine /Applications/Bitcoin-Qt.app

@bitcoin bitcoin locked as resolved and limited conversation to collaborators Feb 15, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.