contrib: add ELF OS ABI check to symbol-check.py #26953
Merged
+26
−0
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ReviewsSee the guideline for information on the review process.
If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update. |
|
GUIX hashes x86: |
fanquake
force-pushed
the
symbol_check_elf_abi
branch
from
332310e
to
2d80eda
Compare
|
Need to follow up here in regards to powerpc64 vs le. le is currently 3.10.0: ELF 64-bit LSB pie executable, 64-bit PowerPC or cisco 7500, OpenPOWER ELF V2 ABI, version 1 (GNU/Linux), dynamically linked, interpreter /lib64/ld64.so.2, for GNU/Linux 3.10.0, with debug_info, not strippedas opposed to non-le: ELF 64-bit MSB pie executable, 64-bit PowerPC or cisco 7500, Power ELF V1 ABI, version 1 (GNU/Linux), dynamically linked, interpreter /lib64/ld64.so.1, for GNU/Linux 3.2.0, with debug_info, not stripped |
fanquake
force-pushed
the
symbol_check_elf_abi
branch
2 times, most recently
from
e37246e
to
407bad3
Compare
laanwj
reviewed
Apr 27, 2023
| def check_ELF_ABI(binary) -> bool: | ||
| expected_abi = ELF_ABIS[binary.header.machine_type][binary.abstract.header.endianness] | ||
| note = binary.get(lief.ELF.NOTE_TYPES.ABI_TAG) | ||
| return note.details.version == expected_abi |
There was a problem hiding this comment.
lets check note.details.abi == lief.ELF.NOTE_ABIS.LINUX as well
|
Concept and code review ACK 407bad3 , left a nit |
fanquake
force-pushed
the
symbol_check_elf_abi
branch
from
407bad3
to
65ba8a7
Compare
|
Code review ACK 65ba8a7 |
TheCharlatan
suggested changes
May 2, 2023
| lief.ENDIANNESS.LITTLE: [3,7,0], | ||
| }, | ||
| lief.ELF.ARCH.PPC64: { | ||
| lief.ENDIANNESS.LITTLE: [3,10,0], |
There was a problem hiding this comment.
I tested all these versions manually first by reading them with readelf --notes bitcoin-cli and then running this script against the 24.0.1 release binaries. All the others matched, but the power pc little endian version gave me 3.2.0. Against which binaries did you check these?
There was a problem hiding this comment.
This is against master (binaries produced with a Guix build). The symbol/security scripts are (only) expected to work with the version of the source they are shipped with. In this case, we are using a newer version of glibc to produce the binaries in master, hence the difference. Think I also alluded to this in a comment above.
There was a problem hiding this comment.
Ah right, all the better that we catch these now. I found a recent guix build and it checks out there.
Guix builds
|
|
Guix Build: 80e0201c2f4c0c2639bfa4f53d7a0ca982e42e19e7abcbd013675e8323394bd8 guix-build-65ba8a79a291/output/aarch64-linux-gnu/SHA256SUMS.part
c19b6dd1413d1796d59cee5243b8b004460e36ff0f86f277febbdcc43e5c5356 guix-build-65ba8a79a291/output/aarch64-linux-gnu/bitcoin-65ba8a79a291-aarch64-linux-gnu-debug.tar.gz
9a6f9a798d1d686828e1c658735bc230e4d141b6abfb75e6e819058566707535 guix-build-65ba8a79a291/output/aarch64-linux-gnu/bitcoin-65ba8a79a291-aarch64-linux-gnu.tar.gz
20b76fd565b64c010988a79934788d54661052f695a46f4b9b6ea6ed114f99ee guix-build-65ba8a79a291/output/arm-linux-gnueabihf/SHA256SUMS.part
ff48663d82fb16ab3a0cc8e8c488b5c0b37c1f2c9549b49d887bc8947bbee9a1 guix-build-65ba8a79a291/output/arm-linux-gnueabihf/bitcoin-65ba8a79a291-arm-linux-gnueabihf-debug.tar.gz
6f11731fae29e7da16beb2dd04507affb89371699c7c8d3d3a4b2ae6e8108014 guix-build-65ba8a79a291/output/arm-linux-gnueabihf/bitcoin-65ba8a79a291-arm-linux-gnueabihf.tar.gz
cbbacbb66bbfdf0790d1f666fe65327658df39f9a4e6333d679c708792ec46dc guix-build-65ba8a79a291/output/arm64-apple-darwin/SHA256SUMS.part
581aecf32136215efac1382aed334de9799f738e1f846f71c4632b4e51ce812e guix-build-65ba8a79a291/output/arm64-apple-darwin/bitcoin-65ba8a79a291-arm64-apple-darwin-unsigned.dmg
6ffcabee1de3c94fd2ece7654b0d11b8f0c1f8bae107784de7125dfd1178d2ae guix-build-65ba8a79a291/output/arm64-apple-darwin/bitcoin-65ba8a79a291-arm64-apple-darwin-unsigned.tar.gz
dc4f50ae658d16aab1137ab58a6b27bc65de8475a3c6274a21981750777e8851 guix-build-65ba8a79a291/output/arm64-apple-darwin/bitcoin-65ba8a79a291-arm64-apple-darwin.tar.gz
3d4f97fe7f80745f3a29c22f7577fc219c7dbcd6e1f9e2be2bdf864cfb6ffb08 guix-build-65ba8a79a291/output/dist-archive/bitcoin-65ba8a79a291.tar.gz
54be372ebc78013a6814521d7d3d7af99ff0db767be9451d435e99caa5b65b6a guix-build-65ba8a79a291/output/powerpc64-linux-gnu/SHA256SUMS.part
41103101dfe04fc4ea60876448f8759ff2c858618363f8f4037be0e02da838a5 guix-build-65ba8a79a291/output/powerpc64-linux-gnu/bitcoin-65ba8a79a291-powerpc64-linux-gnu-debug.tar.gz
5df94212d59d849f62c19433796de380a5140693ec3c49678fe2c36ce3791511 guix-build-65ba8a79a291/output/powerpc64-linux-gnu/bitcoin-65ba8a79a291-powerpc64-linux-gnu.tar.gz
d0a3870d3412775cd62566a4890d201db96f0f77489f3b3757ec1a6d0fa4685e guix-build-65ba8a79a291/output/powerpc64le-linux-gnu/SHA256SUMS.part
3d2c7766b677bc348b63fe736721fd6eb1bece77d0a0293880adb9ae94846c84 guix-build-65ba8a79a291/output/powerpc64le-linux-gnu/bitcoin-65ba8a79a291-powerpc64le-linux-gnu-debug.tar.gz
de75217ba3482e9ee7078a61a24396d3bf13a9b44858c3c4b057a5ed01ebefb7 guix-build-65ba8a79a291/output/powerpc64le-linux-gnu/bitcoin-65ba8a79a291-powerpc64le-linux-gnu.tar.gz
16be15a04a0e0dea88de597dbe6a3405f2bdaad1a1be19596b81e9b6885c13a9 guix-build-65ba8a79a291/output/riscv64-linux-gnu/SHA256SUMS.part
22826a8cb15cf4e446589304f75531a0f7e7c140bb2b9e048658a0cfb64d2cf8 guix-build-65ba8a79a291/output/riscv64-linux-gnu/bitcoin-65ba8a79a291-riscv64-linux-gnu-debug.tar.gz
dcbb57f1852691d1f05eeb9a202f98d6476ef04b9cb5049c78099cb47c67279e guix-build-65ba8a79a291/output/riscv64-linux-gnu/bitcoin-65ba8a79a291-riscv64-linux-gnu.tar.gz
da7fb28e8767d7cbf383fb8a2da22a8162f83c5d48ae424d73785cfb91659e66 guix-build-65ba8a79a291/output/x86_64-apple-darwin/SHA256SUMS.part
70ca0d8ef414c6d2f67fa6cdcc1e80a659061ea861b18ab34ff7921745790e83 guix-build-65ba8a79a291/output/x86_64-apple-darwin/bitcoin-65ba8a79a291-x86_64-apple-darwin-unsigned.dmg
2e13bff7f50b62fc831bcacbb8dd9cd2f5858dd4a90e728668971b5ad41a93c5 guix-build-65ba8a79a291/output/x86_64-apple-darwin/bitcoin-65ba8a79a291-x86_64-apple-darwin-unsigned.tar.gz
5133a7efe6b40eba5b1d1c44bb5ce0570f489a70521e09622d47a98c61b79d49 guix-build-65ba8a79a291/output/x86_64-apple-darwin/bitcoin-65ba8a79a291-x86_64-apple-darwin.tar.gz
55df50bd3dd148299204a4e25d524fba4e815a11bf3adba7a016979c2e7fb924 guix-build-65ba8a79a291/output/x86_64-linux-gnu/SHA256SUMS.part
226c88785333cc917ed9a0a27a5c33a6efd803f282ad8cdb3a3b120dd18c1c86 guix-build-65ba8a79a291/output/x86_64-linux-gnu/bitcoin-65ba8a79a291-x86_64-linux-gnu-debug.tar.gz
e73ec7a683cff540fa35d5c3a1237ff0b8400103adde5278fac72076b2246016 guix-build-65ba8a79a291/output/x86_64-linux-gnu/bitcoin-65ba8a79a291-x86_64-linux-gnu.tar.gz
415f1cebc7fc19f15a93b36fbca7e75bf28d5ceb54f38db63cdcea57e097245d guix-build-65ba8a79a291/output/x86_64-w64-mingw32/SHA256SUMS.part
5b5192cbb488eaef2cba10ac0bd1cc215a27dc665bd6348be57648f2f2afeab7 guix-build-65ba8a79a291/output/x86_64-w64-mingw32/bitcoin-65ba8a79a291-win64-debug.zip
b1afb1c4850a6597b6d38392c0d86e98ebaaddf840335987a8ddef54e3d242e5 guix-build-65ba8a79a291/output/x86_64-w64-mingw32/bitcoin-65ba8a79a291-win64-setup-unsigned.exe
d2511915d8c29309a7291dadf45f00cf4320509605dc43b6a3198ad9019dc24e guix-build-65ba8a79a291/output/x86_64-w64-mingw32/bitcoin-65ba8a79a291-win64-unsigned.tar.gz
9e2711b3a0933584a8a8019c03eb0981c35300f17565315cb0d74a18f9544373 guix-build-65ba8a79a291/output/x86_64-w64-mingw32/bitcoin-65ba8a79a291-win64.zip |
sidhujag
pushed a commit
to syscoin/syscoin
that referenced
this pull request
May 4, 2023
65ba8a7 contrib: add ELF ABI check to symbol-check.py (fanquake) Pull request description: Check that the operating system ABI version embedded into the release binaries, is the version we expect it to be. ACKs for top commit: laanwj: Code review ACK 65ba8a7 TheCharlatan: ACK 65ba8a7 Tree-SHA512: 798d7c3b05183becf113a2ea13d889e18f1cec01d3cc279e64dbddede4d57f87444978f3f52c44bc5fdf0ba93d77c7c0be37aa815f93f348c35da45dc3d30ac2
fanquake
added a commit
to fanquake/bitcoin
that referenced
this pull request
May 17, 2023
We no-longer run any security/syymbol checks in the CI, and doubt we will in future (if we do, it'll be via Guix, where this var would be redundant in any case). The CI environment doesn't (exactly) match the release build environment (and is semi-regularly changing), and the binaries produced in the CI don't match how we build release binaries, so there is no point trying to run these checks, especially as we add more involved tests, i.e bitcoin#26953.
fanquake
added a commit
that referenced
this pull request
May 22, 2023
6a93658 ci: remove RUN_SECURITY_TESTS (fanquake) Pull request description: We no-longer run any security/symbol checks in the CI, and doubt we will in future (if we do, it'll be via Guix, where this var would be redundant in any case). The CI environment doesn't (exactly) match the release build environment (and is semi-regularly changing), and the binaries produced in the CI don't match how we build release binaries, so there is no point trying to run these checks, especially as we add more involved tests, i.e #26953. ACKs for top commit: josibake: code review ACK 6a93658 TheCharlatan: ACK 6a93658 Tree-SHA512: c0eec61a4b873bac487ba9321b50116a215b4796bd7d416d98ffcd09969dbf635c2cb5aeb225c89d1e6462838fa2a48565048ebe730f48d76d3db46b64855a91
sidhujag
pushed a commit
to syscoin/syscoin
that referenced
this pull request
May 23, 2023
6a93658 ci: remove RUN_SECURITY_TESTS (fanquake) Pull request description: We no-longer run any security/symbol checks in the CI, and doubt we will in future (if we do, it'll be via Guix, where this var would be redundant in any case). The CI environment doesn't (exactly) match the release build environment (and is semi-regularly changing), and the binaries produced in the CI don't match how we build release binaries, so there is no point trying to run these checks, especially as we add more involved tests, i.e bitcoin#26953. ACKs for top commit: josibake: code review ACK bitcoin@6a93658 TheCharlatan: ACK 6a93658 Tree-SHA512: c0eec61a4b873bac487ba9321b50116a215b4796bd7d416d98ffcd09969dbf635c2cb5aeb225c89d1e6462838fa2a48565048ebe730f48d76d3db46b64855a91
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Check that the operating system ABI version embedded into the release binaries, is the version we expect it to be.