Skip to content
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

Notification for mismatching builds #1203

Closed
laanwj opened this issue Apr 23, 2024 · 8 comments · Fixed by #1207
Closed

Notification for mismatching builds #1203

laanwj opened this issue Apr 23, 2024 · 8 comments · Fixed by #1207
Labels
enhancement New feature or request

Comments

@laanwj
Copy link
Member

laanwj commented Apr 23, 2024

In #1201 it turns out that @0xB10C had been building mismatching builds for a while, without it being noticed before.

It would be useful to have some kind of notification when this happens. A mismatching build always points to a problem, either (in the good case) a local configuration issue, but it can also be a bug in the build system or worst-case even a backdoor attempt. So it needs to be investigated.

See discussion here: #1201 (comment)

@laanwj laanwj added the enhancement New feature or request label Apr 23, 2024
@achow101
Copy link
Member

I think we've previously discussed that having CI to look for mismatches in PRs was bad because we want to know about and investigate mismatches. But it also seems like in the PR is when we want to know about that, so maybe we can have a CI task that is ignorable and just serves as an indicator that there is a mismatch?

I don't think we'd necessarily want to have a CI task on the main branch that fails when anything mismatches because I there are classes of mismatch which we should keep, e.g. actual nondeterminism in the build process.

Also, our guix stuff is a bit less resilient to mismatches than gitian was. If someone didn't build a system, we would get a mismatch, whereas I think this didn't always happen with gitian? Sometimes this is intentional if the builder doesn't have the MacOS SDK for example, but other times it does indicate that something went wrong with the build.

@laanwj
Copy link
Member Author

laanwj commented Apr 24, 2024

I think we've previously discussed that having CI to look for mismatches in PRs was bad because we want to know about and investigate mismatches. But it also seems like in the PR is when we want to know about that, so maybe we can have a CI task that is ignorable and just serves as an indicator that there is a mismatch?

Agree that in the PR is when you want to know. A daily check on master might already be too late.

But yes, blocking the merge on mismatching hashes would be bad, imo. The CI can't tell which set of hashes is correct (if there's such a thing), it's not necessarily the one already in the repository.

Having a signaling-only CI run, if that's possible, sounds like a good compromise

If someone didn't build a system, we would get a mismatch, whereas I think this didn't always happen with gitian?

Right. Gitian had separate attestations per group of platforms (-windows, -linux, -osx, plus codesigned variants). If someone would sign for only linux, this would be fine.

This could only work because the generation and signing of the final SHA256SUMS file was manually done by the release maintainer. Guix's automatic process is more strict. Even if it'd have separate attestations per platform (which was considered, but IIRC we didn't go for it because if one is using, say, a Yubikey one had to press to sign a zillion times*), separate PGP signatures over different fragments likely can't be joined into a single file for release. Which would make checking more burdensome for the end user.

* Another thing considered was separate detached PGP signatures per file. But even worse in this regard, the sheer number of signatures needed.

Sometimes this is intentional if the builder doesn't have the MacOS SDK for example, but other times it does indicate that something went wrong with the build.

Agree. i had a bug in my scripting where i would still attest when the build errored out halfway and thus only part of the platforms was built. It would have been nice to catch this sooner.

Might have two seperate (signalling only) checks:

  • Not all platforms are built. This means the attestation can't be used in the release.
  • Mismatching hashes (more serious).

@0xB10C
Copy link
Contributor

0xB10C commented Apr 24, 2024

I noticed that for 23.0rc1 and 23.0rc2 there are multiple distinct hashes for bitcoin-23.0rc2-osx-unsigned.dmg, bitcoin-23.0rc2-osx-unsigned.tar.gz, and bitcoin-23.0rc2-osx-signed.dmg. I'm not sure if this is known and was discussed at the time.

edit: see bitcoin/bitcoin#24549, not a problem as this were RCs

23.0rc2 fanquake bitcoin-23.0rc2-osx-unsigned.dmg already in noncodesigned.SHA256SUMS
23.0rc2 fanquake bitcoin-23.0rc2-osx-unsigned.tar.gz already in noncodesigned.SHA256SUMS
23.0rc2 fanquake bitcoin-23.0rc2-osx-signed.dmg already in all.SHA256SUMS
23.0rc2 fanquake bitcoin-23.0rc2-osx-unsigned.dmg already in all.SHA256SUMS
23.0rc2 fanquake bitcoin-23.0rc2-osx-unsigned.tar.gz already in all.SHA256SUMS
23.0rc2 jnewbery bitcoin-23.0rc2-osx-unsigned.dmg already in noncodesigned.SHA256SUMS
23.0rc2 jnewbery bitcoin-23.0rc2-osx-unsigned.tar.gz already in noncodesigned.SHA256SUMS
23.0rc2 jnewbery bitcoin-23.0rc2-osx-signed.dmg already in all.SHA256SUMS
23.0rc2 jnewbery bitcoin-23.0rc2-osx-unsigned.dmg already in all.SHA256SUMS
23.0rc2 jnewbery bitcoin-23.0rc2-osx-unsigned.tar.gz already in all.SHA256SUMS
23.0rc2 0xb10c bitcoin-23.0rc2-osx-unsigned.dmg already in noncodesigned.SHA256SUMS
23.0rc2 0xb10c bitcoin-23.0rc2-osx-unsigned.tar.gz already in noncodesigned.SHA256SUMS
23.0rc2 hebasto bitcoin-23.0rc2-osx-unsigned.dmg already in noncodesigned.SHA256SUMS
23.0rc2 hebasto bitcoin-23.0rc2-osx-unsigned.tar.gz already in noncodesigned.SHA256SUMS
23.0rc2 hebasto bitcoin-23.0rc2-osx-signed.dmg already in all.SHA256SUMS
23.0rc2 hebasto bitcoin-23.0rc2-osx-unsigned.dmg already in all.SHA256SUMS
23.0rc2 hebasto bitcoin-23.0rc2-osx-unsigned.tar.gz already in all.SHA256SUMS
23.0rc2 darosior bitcoin-23.0rc2-osx-unsigned.dmg already in noncodesigned.SHA256SUMS
23.0rc2 darosior bitcoin-23.0rc2-osx-unsigned.tar.gz already in noncodesigned.SHA256SUMS
23.0rc2 darosior bitcoin-23.0rc2-osx-signed.dmg already in all.SHA256SUMS
23.0rc2 darosior bitcoin-23.0rc2-osx-unsigned.dmg already in all.SHA256SUMS
23.0rc2 darosior bitcoin-23.0rc2-osx-unsigned.tar.gz already in all.SHA256SUMS
23.0rc2 sipsorcery bitcoin-23.0rc2-osx-unsigned.dmg already in noncodesigned.SHA256SUMS
23.0rc2 sipsorcery bitcoin-23.0rc2-osx-unsigned.tar.gz already in noncodesigned.SHA256SUMS
23.0rc2 sipsorcery bitcoin-23.0rc2-osx-signed.dmg already in all.SHA256SUMS
23.0rc2 sipsorcery bitcoin-23.0rc2-osx-unsigned.dmg already in all.SHA256SUMS
23.0rc2 sipsorcery bitcoin-23.0rc2-osx-unsigned.tar.gz already in all.SHA256SUMS
23.0rc2 dongcarl bitcoin-23.0rc2-osx-unsigned.dmg already in noncodesigned.SHA256SUMS
23.0rc2 dongcarl bitcoin-23.0rc2-osx-unsigned.tar.gz already in noncodesigned.SHA256SUMS
23.0rc2 benthecarman bitcoin-23.0rc2-osx-unsigned.dmg already in noncodesigned.SHA256SUMS
23.0rc2 benthecarman bitcoin-23.0rc2-osx-unsigned.tar.gz already in noncodesigned.SHA256SUMS
23.0rc2 will8clark bitcoin-23.0rc2-osx-unsigned.dmg already in noncodesigned.SHA256SUMS
23.0rc2 will8clark bitcoin-23.0rc2-osx-unsigned.tar.gz already in noncodesigned.SHA256SUMS
23.0rc2 will8clark bitcoin-23.0rc2-osx-signed.dmg already in all.SHA256SUMS
23.0rc2 will8clark bitcoin-23.0rc2-osx-unsigned.dmg already in all.SHA256SUMS
23.0rc2 will8clark bitcoin-23.0rc2-osx-unsigned.tar.gz already in all.SHA256SUMS
23.0rc2 jackielove4u bitcoin-23.0rc2-osx-unsigned.dmg already in noncodesigned.SHA256SUMS
23.0rc2 jackielove4u bitcoin-23.0rc2-osx-unsigned.tar.gz already in noncodesigned.SHA256SUMS
23.0rc2 Sjors bitcoin-23.0rc2-osx-unsigned.dmg already in noncodesigned.SHA256SUMS
23.0rc2 Sjors bitcoin-23.0rc2-osx-unsigned.tar.gz already in noncodesigned.SHA256SUMS
23.0rc2 Sjors bitcoin-23.0rc2-osx-signed.dmg already in all.SHA256SUMS
23.0rc2 Sjors bitcoin-23.0rc2-osx-unsigned.dmg already in all.SHA256SUMS
23.0rc2 Sjors bitcoin-23.0rc2-osx-unsigned.tar.gz already in all.SHA256SUMS
23.0rc2 Emzy bitcoin-23.0rc2-osx-unsigned.dmg already in noncodesigned.SHA256SUMS
23.0rc2 Emzy bitcoin-23.0rc2-osx-unsigned.tar.gz already in noncodesigned.SHA256SUMS
23.0rc2 glozow bitcoin-23.0rc2-osx-unsigned.dmg already in noncodesigned.SHA256SUMS
23.0rc2 glozow bitcoin-23.0rc2-osx-unsigned.tar.gz already in noncodesigned.SHA256SUMS
23.0rc2 glozow bitcoin-23.0rc2-osx-signed.dmg already in all.SHA256SUMS
23.0rc2 glozow bitcoin-23.0rc2-osx-unsigned.dmg already in all.SHA256SUMS
23.0rc2 glozow bitcoin-23.0rc2-osx-unsigned.tar.gz already in all.SHA256SUMS
23.0rc2 guggero bitcoin-23.0rc2-osx-unsigned.dmg already in noncodesigned.SHA256SUMS
23.0rc2 guggero bitcoin-23.0rc2-osx-unsigned.tar.gz already in noncodesigned.SHA256SUMS
23.0rc2 guggero bitcoin-23.0rc2-osx-signed.dmg already in all.SHA256SUMS
23.0rc2 guggero bitcoin-23.0rc2-osx-unsigned.dmg already in all.SHA256SUMS
23.0rc2 guggero bitcoin-23.0rc2-osx-unsigned.tar.gz already in all.SHA256SUMS
23.0rc2 achow101 bitcoin-23.0rc2-osx-unsigned.dmg already in noncodesigned.SHA256SUMS
23.0rc2 achow101 bitcoin-23.0rc2-osx-unsigned.tar.gz already in noncodesigned.SHA256SUMS
23.0rc2 achow101 bitcoin-23.0rc2-osx-signed.dmg already in all.SHA256SUMS
23.0rc2 achow101 bitcoin-23.0rc2-osx-unsigned.dmg already in all.SHA256SUMS
23.0rc2 achow101 bitcoin-23.0rc2-osx-unsigned.tar.gz already in all.SHA256SUMS
23.0rc2 laanwj bitcoin-23.0rc2-osx-unsigned.dmg already in noncodesigned.SHA256SUMS
23.0rc2 laanwj bitcoin-23.0rc2-osx-unsigned.tar.gz already in noncodesigned.SHA256SUMS
23.0rc2 laanwj bitcoin-23.0rc2-osx-signed.dmg already in all.SHA256SUMS
23.0rc2 laanwj bitcoin-23.0rc2-osx-unsigned.dmg already in all.SHA256SUMS
23.0rc2 laanwj bitcoin-23.0rc2-osx-unsigned.tar.gz already in all.SHA256SUMS
23.0rc1 fanquake bitcoin-23.0rc1-osx-unsigned.dmg already in noncodesigned.SHA256SUMS
23.0rc1 fanquake bitcoin-23.0rc1-osx-unsigned.tar.gz already in noncodesigned.SHA256SUMS
23.0rc1 hebasto bitcoin-23.0rc1-osx-unsigned.dmg already in noncodesigned.SHA256SUMS
23.0rc1 hebasto bitcoin-23.0rc1-osx-unsigned.tar.gz already in noncodesigned.SHA256SUMS
23.0rc1 sipsorcery bitcoin-23.0rc1-osx-unsigned.dmg already in noncodesigned.SHA256SUMS
23.0rc1 sipsorcery bitcoin-23.0rc1-osx-unsigned.tar.gz already in noncodesigned.SHA256SUMS
23.0rc1 benthecarman bitcoin-23.0rc1-osx-unsigned.dmg already in noncodesigned.SHA256SUMS
23.0rc1 benthecarman bitcoin-23.0rc1-osx-unsigned.tar.gz already in noncodesigned.SHA256SUMS
23.0rc1 will8clark bitcoin-23.0rc1-osx-unsigned.dmg already in noncodesigned.SHA256SUMS
23.0rc1 will8clark bitcoin-23.0rc1-osx-unsigned.tar.gz already in noncodesigned.SHA256SUMS
23.0rc1 jackielove4u bitcoin-23.0rc1-osx-unsigned.dmg already in noncodesigned.SHA256SUMS
23.0rc1 jackielove4u bitcoin-23.0rc1-osx-unsigned.tar.gz already in noncodesigned.SHA256SUMS
23.0rc1 Emzy bitcoin-23.0rc1-osx-unsigned.dmg already in noncodesigned.SHA256SUMS
23.0rc1 Emzy bitcoin-23.0rc1-osx-unsigned.tar.gz already in noncodesigned.SHA256SUMS
23.0rc1 guggero bitcoin-23.0rc1-osx-unsigned.dmg already in noncodesigned.SHA256SUMS
23.0rc1 guggero bitcoin-23.0rc1-osx-unsigned.tar.gz already in noncodesigned.SHA256SUMS
23.0rc1 achow101 bitcoin-23.0rc1-osx-unsigned.dmg already in noncodesigned.SHA256SUMS
23.0rc1 achow101 bitcoin-23.0rc1-osx-unsigned.tar.gz already in noncodesigned.SHA256SUMS
23.0rc1 laanwj bitcoin-23.0rc1-osx-unsigned.dmg already in noncodesigned.SHA256SUMS
23.0rc1 laanwj bitcoin-23.0rc1-osx-unsigned.tar.gz already in noncodesigned.SHA256SUMS
23.0rc1 jamesob bitcoin-23.0rc1-osx-unsigned.dmg already in noncodesigned.SHA256SUMS
23.0rc1 jamesob bitcoin-23.0rc1-osx-unsigned.tar.gz already in noncodesigned.SHA256SUMS

@TheCharlatan
Copy link
Contributor

Maybe a script could be added that maintainers use to check, merge, and push the attestations? Maintainers can then decide on the course of action if there is a mismatch, or missing file. At least that is what I am doing for the gitian.sigs repo I still maintain.

@laanwj
Copy link
Member Author

laanwj commented Apr 24, 2024

Maybe a script could be added that maintainers use to check, merge, and push the attestations? Maintainers can then decide on the course of action if there is a mismatch, or missing file. At least that is what I am doing for the gitian.sigs repo I still maintain.

FWIW, this was my goal for https://github.com/bitcoin-core/bitcoin-maintainer-tools/blob/main/guix-verify.py, to check and display the build results in a table to have a better overview.

In any case, i think such a thing is seperate from PR-time signalling.

@0xB10C
Copy link
Contributor

0xB10C commented Apr 24, 2024

Maybe a script could be added that maintainers use to check, merge, and push the attestations? Maintainers can then decide on the course of action if there is a mismatch, or missing file. At least that is what I am doing for the gitian.sigs repo I still maintain.

FWIW, this was my goal for https://github.com/bitcoin-core/bitcoin-maintainer-tools/blob/main/guix-verify.py, to check and display the build results in a table to have a better overview.

In any case, i think such a thing is seperate from PR-time signalling.

Cool, wasn't aware of this. This particular script might be separate from PR-time signaling, but the idea still works?

For example, have the CI generate and comment a table/image/heatmap that shows the derivation from mean hash value for each touched release and noncodesigned/all SHASUMS combination.

For example, without mismatches is could look like this:

image

A single mismatch like this (there's actually a mismatch there)

image

Missing artifacts/hosts like this

image

And non-determinism across the board like this:

image

edit: having the CI leave a table as comment seems significantly easier - no need to worry about image hosting and similar

@0xB10C
Copy link
Contributor

0xB10C commented Apr 24, 2024

having a text-based table is probably easier, so could be a CI job that comments on a PR for each changed release/release candidate. Before merging, it can be quickly visually verified that there are no mismatches. Example:

22.0rc3 all.SHA256SUMS
                                   A B C D E F G H I J K L M
aarch64-linux-gnu-debug.tar.gz---- █ █ █ █ █ █ █ █ █ █ █ █ █
aarch64-linux-gnu.tar.gz---------- █ █ █ █ █ █ █ █ █ █ █ █ █
arm-linux-gnueabihf-debug.tar.gz-- █ █ █ █ █ █ █ █ █ █ █ █ █
arm-linux-gnueabihf.tar.gz-------- █ █ █ █ █ █ █ █ █ █ █ █ █
codesignatures-22.0rc3.tar.gz----- █ █ █ █ █ █ █ █ █ █ █ █ █
osx-signed.dmg-------------------- ░ ░ █ ░ ░ ░ ░ ░ ░ ░ ░ ░ ░
osx-unsigned.dmg------------------ █ █ █ █ █ █ █ █ █ █ █ █ █
osx-unsigned.tar.gz--------------- █ █ █ █ █ █ █ █ █ █ █ █ █
osx64.tar.gz---------------------- █ █ █ █ █ █ █ █ █ █ █ █ █
powerpc64-linux-gnu-debug.tar.gz-- █ █ █ █ █ █ █ █ █ █ █ █ █
powerpc64-linux-gnu.tar.gz-------- █ █ █ █ █ █ █ █ █ █ █ █ █
powerpc64le-linux-gnu-debug.tar.gz █ █ █ █ █ █ █ █ █ █ █ █ █
powerpc64le-linux-gnu.tar.gz------ █ █ █ █ █ █ █ █ █ █ █ █ █
riscv64-linux-gnu-debug.tar.gz---- █ █ █ █ █ █ █ █ █ █ █ █ █
riscv64-linux-gnu.tar.gz---------- █ █ █ █ █ █ █ █ █ █ █ █ █
win-unsigned.tar.gz--------------- █ █ █ █ █ █ █ █ █ █ █ █ █
win64-debug.zip------------------- █ █ █ █ █ █ █ █ █ █ █ █ █
win64-setup-unsigned.exe---------- █ █ █ █ █ █ █ █ █ █ █ █ █
win64-setup.exe------------------- █ █ █ █ █ █ █ █ █ █ █ █ █
win64.zip------------------------- █ █ █ █ █ █ █ █ █ █ █ █ █
x86_64-linux-gnu-debug.tar.gz----- █ █ █ █ █ █ █ █ █ █ █ █ █
x86_64-linux-gnu.tar.gz----------- █ █ █ █ █ █ █ █ █ █ █ █ █
bitcoin-22.0rc3.tar.gz------------ █ █ █ █ █ █ █ █ █ █ █ █ █
char to username mapping
  • A duncandean
  • B fanquake
  • C jonatack
  • D 0xb10c
  • E hebasto
  • F darosior
  • G sipsorcery
  • H dongcarl
  • I benthecarman
  • J Emzy
  • K guggero
  • L achow101
  • M laanwj

If this looks useful, I'd be happy to PR it.

@0xB10C
Copy link
Contributor

0xB10C commented Apr 26, 2024

Proposed a solution in #1207

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants