build: Fix macOS code signing by pre-allocating space for the code signature during gitian build #20638

pull achow101 wants to merge 2 commits into bitcoin:master from achow101:mac-codesign-fixed-alloc changing 2 files +36 −0
  1. achow101 commented at 4:57 pm on December 12, 2020: 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.

  2. achow101 commented at 4:59 pm on December 12, 2020: member

    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.

  3. MarcoFalke added the label Needs backport (0.19) on Dec 12, 2020
  4. MarcoFalke added the label Needs backport (0.20) on Dec 12, 2020
  5. MarcoFalke added the label Needs backport (0.21) on Dec 12, 2020
  6. MarcoFalke added the label Build system on Dec 12, 2020
  7. MarcoFalke added the label Needs gitian build on Dec 12, 2020
  8. MarcoFalke added the label Needs Guix build on Dec 12, 2020
  9. MarcoFalke removed the label Needs Guix build on Dec 12, 2020
  10. in contrib/gitian-descriptors/gitian-osx.yml:150 in ba8a52725f outdated
    142+    #
    143+    # Thus we can workaround this by calculating an overestimate of what the signature will be and then using codesign_allocate to preallocate
    144+    # space for the code signature. Because the same codesign_allocate is used in this step, the rounding problem will not be present.
    145+    # Since the space allocated for the signature should be way larger than actually needed, the signing step will not reduce "vmsize" so we
    146+    # won't have any problems there. We use a fixed overestimate because that's easier. Looking at previous releases, the largest MacOS
    147+    # code signature since 0.12.1 was ~408000 bytes, so using 500000 should be sufficiently large.
    


    laanwj commented at 5:10 pm on December 12, 2020:
    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?

    sipa commented at 5:31 pm on December 12, 2020:

    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.


    achow101 commented at 6:00 pm on December 12, 2020:
    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.

    laanwj commented at 6:06 pm on December 12, 2020:
    thanks! yes 20kB isn’t that surprising to me for RSA+metadata, and if it signs every page separately that definitely explains things
  11. laanwj commented at 5:22 pm on December 12, 2020: member
    Concept ACK, thanks for investigating this weird problem.
  12. achow101 force-pushed on Dec 12, 2020
  13. achow101 commented at 6:02 pm on December 12, 2020: member

    MacOS Gitian build of 73d1a541147e72b07ce0e6c3a23ca6e94f74c3ce

    0bf682a0e960d153a6c862ad4af4f7fce584df3139ee74ecfd6c864ea91bf5c63  bitcoin-73d1a541147e-osx-unsigned.dmg
    1836a13543e82062074400b7ef82d4e62ffc16a4f030c3e89e500e6164dca58ab  bitcoin-73d1a541147e-osx-unsigned.tar.gz
    287ba11ddf0fe08c531afd50ba2e638b1d74a857fcbf3f9a2a1dac1c2c515881e  bitcoin-73d1a541147e-osx64.tar.gz
    3e6ee88e761dc0894daf6b094768453f9baea5a26209ecb64b44000cc6030dbdd  src/bitcoin-73d1a541147e.tar.gz
    
  14. jonasschnelli commented at 6:53 pm on December 12, 2020: contributor

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

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

  15. achow101 force-pushed on Dec 12, 2020
  16. DrahtBot commented at 7:18 pm on December 12, 2020: member

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

    Conflicts

    No conflicts as of last run.

  17. in contrib/gitian-descriptors/gitian-osx.yml:148 in 3ca08c2d19 outdated
    143+    # Fortunately codesign_allocate will not reduce the "vmsize" attribute, and when the signature is appended, the difference in actual size and
    144+    # "vmsize" will not be present in the final binary.
    145+    #
    146+    # Thus we can workaround this by calculating an overestimate of what the signature will be and then using codesign_allocate to preallocate
    147+    # space for the code signature. Because the same codesign_allocate is used in this step, the rounding problem will not be present.
    148+    # Since the space allocated for the signature should be way larger than actually needed, the signing step will not reduce "vmsize" so we
    


    sipa commented at 7:20 pm on December 12, 2020:
    This line seems outdated now.

    achow101 commented at 7:21 pm on December 12, 2020:
    Which part?

    sipa commented at 7:22 pm on December 12, 2020:
    It’s not way larger overestimate anymore.

    achow101 commented at 7:25 pm on December 12, 2020:
    Fixed
  18. achow101 commented at 7:21 pm on December 12, 2020: member

    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.

  19. achow101 force-pushed on Dec 12, 2020
  20. achow101 commented at 7:32 pm on December 12, 2020: member

    MacOS Gitian build for ef0ea90

    0e0f51c79af8598f8b99ed2341a476d97b846d2d556d375c3d93a163c77b6118a  bitcoin-ef0ea90d3e81-osx-unsigned.dmg
    1017b20d59ef2c2594606dd0c3c1de11ab36c56b5cc34c2add1c78fc8db698b56  bitcoin-ef0ea90d3e81-osx-unsigned.tar.gz
    298560f5bef1938303a1f55448ba087f76ec7f35205ae64fec0efd129188fd8e1  bitcoin-ef0ea90d3e81-osx64.tar.gz
    38bd279c7439e19d783e70f2cf425c40b3306b97b9b2410297c2cb3e48f3bef33  src/bitcoin-ef0ea90d3e81.tar.gz
    
  21. jonatack commented at 7:41 pm on December 12, 2020: member
    Approach ACK. Nice work.
  22. achow101 renamed this:
    Mac codesign fixed alloc
    build: Fix macOS code signing by pre-allocating space for the code signature during gitian build
    on Dec 13, 2020
  23. in contrib/gitian-descriptors/gitian-osx.yml:157 in ef0ea90d3e outdated
    152+    # of the unsigned binary. There are a few other hashes of something. Then the signature itself, which is ~9kb. Then ~10kb of additional overhead.
    153+    # We can thus produce an overestimate as follows: (((unsigned_size / 4096) + 1) * 32) + 50000. This gives us an expected number of SHA256 hashes
    154+    # and how large they will be. By adding 50k, we can include the signature, overhead, and still have room if Apple decides to cram more stuff
    155+    # into the code signature.
    156+    BINARY_RESULT="dist/Bitcoin-Qt.app/Contents/MacOS/Bitcoin-Qt"
    157+    UNSIGNED_SIZE=`stat -c %s ${BINARY_RESULT}`
    


    MarcoFalke commented at 6:22 am on December 13, 2020:
     0In gitian-osx.yml line 122:
     1
     2  UNSIGNED_SIZE=`stat -c %s ${BINARY_RESULT}`
     3
     4                ^---------------------------^ SC2006: Use $(...) notation instead of legacy backticked `...`.
     5
     6Did you mean: 
     7
     8  UNSIGNED_SIZE=$(stat -c %s ${BINARY_RESULT})
     9
    10In gitian-osx.yml line 123:
    11
    12  SIG_SIZE_EST=$(((((${UNSIGNED_SIZE} / 4096) + 1) * 32) + 50000))
    13
    14                     ^--------------^ SC2004: $/${} is unnecessary on arithmetic variables.
    15
    16For more information:
    17
    18  https://www.shellcheck.net/wiki/SC2004 -- $/${} is unnecessary on arithmeti...
    19
    20  https://www.shellcheck.net/wiki/SC2006 -- Use $(...) notation instead of le...
    21
    22^---- failure generated from test/lint/lint-shell.sh
    

    achow101 commented at 6:28 am on December 13, 2020:
    Fixed.
  24. MarcoFalke removed the label Needs gitian build on Dec 13, 2020
  25. achow101 force-pushed on Dec 13, 2020
  26. MarcoFalke added the label Needs gitian build on Dec 13, 2020
  27. decryp2kanon commented at 3:57 pm on December 13, 2020: contributor
    Concept ACK 💖
  28. sipa commented at 7:00 pm on December 13, 2020: member
    utACK 6753b74195d80a75b3bceef459d767906bea8e7d
  29. achow101 force-pushed on Dec 13, 2020
  30. achow101 force-pushed on Dec 13, 2020
  31. achow101 force-pushed on Dec 13, 2020
  32. achow101 force-pushed on Dec 13, 2020
  33. achow101 force-pushed on Dec 13, 2020
  34. achow101 commented at 0:16 am on December 14, 2020: member

    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:

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

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

    Signed binary result:

    028c5fe82d1a93d096059e7a35a57286c28f57b21b7688dc523df39d211c17ae1  bitcoin-osx-signed.dmg
    
  35. jonasschnelli commented at 8:11 am on December 14, 2020: contributor

    Tested ACK 7f9d623e0210ff539455b0d8dd70b31f14231b8b built this in gitian an got:

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

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

    0c1ae8014f7a404212b0c43e4e53e7ce888845af72f1797a169c09445b7da9d92  bitcoin-osx-signed.dmg
    

    Successfully verified the signature.

  36. jonasschnelli commented at 8:12 am on December 14, 2020: contributor
    I think this solution is less fragile than #20644.
  37. sipa commented at 8:16 am on December 14, 2020: member
    @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.
  38. DrahtBot commented at 8:43 am on December 14, 2020: member

    Gitian builds

    File commit ade38b6ee8f91e441507c0ee7ffe6ca020748991(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...
  39. DrahtBot removed the label Needs gitian build on Dec 14, 2020
  40. jonasschnelli commented at 8:44 am on December 14, 2020: contributor
    @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.
  41. Sjors commented at 3:10 pm on December 14, 2020: member

    MacOS Gitian build of 7f9d623e0210ff539455b0d8dd70b31f14231b8b:

    0494ccf46f66a57bf93bafd5308cadb1a56ee0fe152bb7f378ce9eea2b63b7354  bitcoin-7f9d623e0210-osx-unsigned.dmg
    12110b65d5223980121602ed4cb0e50558278877148ee59d445a56bd2542014d7  bitcoin-7f9d623e0210-osx-unsigned.tar.gz
    23cfedace3c8d0553450db723aed2f5c3d0a5870b3f5b95e735314232a3fea32e  bitcoin-7f9d623e0210-osx64.tar.gz
    3a017e7c291f81dd16b24368dc513303b65deb71d5d144f3726facee03605eb08  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: https://github.com/bitcoin-core/gitian.sigs/pull/1388

  42. fanquake commented at 1:17 pm on December 16, 2020: member
    Concept ACK
  43. DrahtBot added the label Needs rebase on Dec 16, 2020
  44. Work around codesign_allocate differences
    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.
    b7016ea241
  45. gitian: Apply a bogus signature to the binary that the codesigner signs 159d203017
  46. achow101 force-pushed on Dec 16, 2020
  47. DrahtBot removed the label Needs rebase on Dec 17, 2020
  48. Sjors commented at 1:12 pm on December 17, 2020: member

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

    0f3a02590e3223fa9a02989530b427e148e886302bbcf6a2368955c06aca42434  bitcoin-159d203017b6-osx-unsigned.dmg
    133a91c9555828e454ce4127a64685fa4020a688de645e7b9303d2644d1ce2d67  bitcoin-159d203017b6-osx-unsigned.tar.gz
    299a41ee9468b474d24c1f7cc0273a0d07efabd232cf8fa9b6f788a8df8518a88  bitcoin-159d203017b6-osx64.tar.gz
    32c31464f5b2c4a41b846b9e161f1e6cfbcf094714233a04dcda5fa5e2328434b  src/bitcoin-159d203017b6.tar.gz
    
  49. sipa commented at 7:41 pm on December 17, 2020: member

    utACK 159d203017b66bc7ae158bf2d3318264332cd5ae. 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.

  50. laanwj referenced this in commit 816314ef0f on Dec 17, 2020
  51. sidhujag referenced this in commit 646c6a8bad on Dec 17, 2020
  52. laanwj commented at 12:00 pm on December 18, 2020: member
    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.
  53. achow101 commented at 10:24 pm on December 21, 2020: member
    Closing for now. Will reopen if needed.
  54. achow101 closed this on Dec 21, 2020

  55. fanquake removed the label Needs backport (0.19) on Dec 22, 2020
  56. fanquake removed the label Needs backport (0.20) on Dec 22, 2020
  57. fanquake removed the label Needs backport (0.21) on Dec 22, 2020
  58. Emzy commented at 1:33 pm on December 23, 2020: contributor

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

    0sudo xattr -rd com.apple.quarantine /Applications/Bitcoin-Qt.app
    
  59. DrahtBot locked this on Feb 15, 2022

github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2024-09-27 22:12 UTC

This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me