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

Add GitHub workflow to check that binaries are reproducible. #94

Merged
merged 3 commits into from
May 13, 2020

Conversation

gendx
Copy link
Collaborator

@gendx gendx commented Apr 14, 2020

Fixes #70.

This pull request is the last step for reproducible builds.

  • A new reproduce_hashes.sh tool generates reference SHA-256 sums for the OpenSK binaries. These hashes must then be checked in git in the reference_binaries.sha256sum file.
  • A new GitHub workflow then compares the cryptographic hashes obtained on continuous integration with those checked into git by the developer. If the hashes don't match, then it means that the builds were not reproducible (although the converse is not necessarily true, this should catch most causes of non-reproducibility).
  • Anyone can compare the SHA-256 sums obtained on their machine with those published in git and computed by continuous integration.

  • Tests pass
  • Appropriate changes to README are included in PR

Currently blocked on:

@gendx
Copy link
Collaborator Author

gendx commented Apr 14, 2020

For now, this fails for two reasons.

@gendx
Copy link
Collaborator Author

gendx commented Apr 14, 2020

This is currently blocked on ./deploy.py --board=$board --opensk --no-tockos --programmer=none, itself blocked on #79 aka tock/tockloader#58.

@gendx gendx added the blocked Depends on another pull-request or bugfix label Apr 14, 2020
@gendx
Copy link
Collaborator Author

gendx commented Apr 20, 2020

There is currently a discrepancy between Linux and OSX builds of Tock, despite the --remap-path-prefix parameter. For example, the tock_cells library is built with a different metadata parameter on both platforms (whereas I've observed it to be the same on the workflow's Linux machine and on my own Linux machine).

  • On Linux (https://github.com/google/OpenSK/pull/94/checks?check_run_id=602439620): rustc --crate-name tock_cells libraries/tock-cells/src/lib.rs --error-format=json --json=diagnostic-rendered-ansi,artifacts --crate-type lib --emit=dep-info,metadata,link -C opt-level=z -C panic=abort -C debuginfo=2 -C metadata=92487154152022d3 -C extra-filename=-92487154152022d3 --out-dir /home/runner/work/OpenSK/OpenSK/third_party/tock/target/thumbv7em-none-eabi/release/deps --target thumbv7em-none-eabi -L dependency=/home/runner/work/OpenSK/OpenSK/third_party/tock/target/thumbv7em-none-eabi/release/deps -L dependency=/home/runner/work/OpenSK/OpenSK/third_party/tock/target/release/deps -C link-arg=-Tlayout.ld -C linker=rust-lld -C linker-flavor=ld.lld -C relocation-model=dynamic-no-pic -C link-arg=-zmax-page-size=512 -C link-arg=-icf=all --remap-path-prefix=/home/runner/work/OpenSK/OpenSK/third_party/tock/= -D warnings
  • On OSX (https://github.com/google/OpenSK/pull/94/checks?check_run_id=602439631): rustc --crate-name tock_cells libraries/tock-cells/src/lib.rs --error-format=json --json=diagnostic-rendered-ansi,artifacts --crate-type lib --emit=dep-info,metadata,link -C opt-level=z -C panic=abort -C debuginfo=2 -C metadata=9fc982d890c0358d -C extra-filename=-9fc982d890c0358d --out-dir /Users/runner/runners/2.169.0/work/OpenSK/OpenSK/third_party/tock/target/thumbv7em-none-eabi/release/deps --target thumbv7em-none-eabi -L dependency=/Users/runner/runners/2.169.0/work/OpenSK/OpenSK/third_party/tock/target/thumbv7em-none-eabi/release/deps -L dependency=/Users/runner/runners/2.169.0/work/OpenSK/OpenSK/third_party/tock/target/release/deps -C link-arg=-Tlayout.ld -C linker=rust-lld -C linker-flavor=ld.lld -C relocation-model=dynamic-no-pic -C link-arg=-zmax-page-size=512 -C link-arg=-icf=all --remap-path-prefix=/Users/runner/runners/2.169.0/work/OpenSK/OpenSK/third_party/tock/= -D warnings

I'll report it upstream to Tock and Rust.

@gendx
Copy link
Collaborator Author

gendx commented Apr 24, 2020

It seems that Rust builds won't be reproducible across hosts for the time being (rust-lang/cargo#8140). Until then, setting up two reference files for the SHA-256 hashes.

@gendx gendx force-pushed the reproducible-check branch from a0b67c3 to 0bc7b62 Compare April 24, 2020 14:45
@jmichelp
Copy link
Collaborator

Should we do like in the tock-on-titan project and symlink the rust-toolchain from the kernel at the app level so that we use the same version?

@gendx
Copy link
Collaborator Author

gendx commented Apr 24, 2020

Should we do like in the tock-on-titan project and symlink the rust-toolchain from the kernel at the app level so that we use the same version?

Definitely! This would also reduce the time wasted by the workflows to install various versions of the compiler, and therefore reduce the likelihood of being cancelled by the GitHub runner.

@gendx gendx force-pushed the reproducible-check branch from 4f6830b to caad28a Compare May 12, 2020 14:46
@gendx gendx force-pushed the reproducible-check branch from 1764326 to 20f65f9 Compare May 12, 2020 15:41
@gendx gendx removed the blocked Depends on another pull-request or bugfix label May 12, 2020
@gendx gendx requested a review from jmichelp May 12, 2020 15:42
@gendx
Copy link
Collaborator Author

gendx commented May 12, 2020

With elf2tab 0.5.0 published on crates.io, this pull request is now unblocked.

@jmichelp
Copy link
Collaborator

What's going to be the process to keep these hashes up-to-date?
They're going to change on pretty much every merged PR and waiting for the workflow to fail in order to grab the correct hashes for both platforms and then update the PR with it is not the most friendly workflow IMHO.

@gendx gendx force-pushed the reproducible-check branch from e22b345 to b10460b Compare May 13, 2020 10:38
@gendx
Copy link
Collaborator Author

gendx commented May 13, 2020

What's going to be the process to keep these hashes up-to-date?
They're going to change on pretty much every merged PR and waiting for the workflow to fail in order to grab the correct hashes for both platforms and then update the PR with it is not the most friendly workflow IMHO.

One should be able to run the ./reproduce_hashes.sh script locally to update the hashes on the platform they use. Unfortunately, one still needs to check the workflow for other platforms until rust-lang/cargo#8140 is fixed.

Maybe we can leave this workflow as non required until then? And for example only update hashes after all the reviews have been resolved on a pull request.

@gendx gendx force-pushed the reproducible-check branch 2 times, most recently from 74999b2 to 82c1eeb Compare May 13, 2020 10:52
@gendx gendx force-pushed the reproducible-check branch 2 times, most recently from 5320c7a to 15bd2c7 Compare May 13, 2020 11:21
@gendx gendx force-pushed the reproducible-check branch from 15bd2c7 to 0604be6 Compare May 13, 2020 11:25
@jmichelp
Copy link
Collaborator

Ok, let's merge it like this and revisit later when we need to run the workflow.
Maybe we need to move to a stable vs. dev branches where reproducibility needs to be run only on stable branch. The other option I have in mind is to modify the workflow: instead of checking hashes, it can publish them.

@gendx
Copy link
Collaborator Author

gendx commented May 13, 2020

The other option I have in mind is to modify the workflow: instead of checking hashes, it can publish them.

This would be a good idea too (although wouldn't catch regressions in reproducibility). The current workflow already uploads the reproduced binaries, so it wouldn't be hard to upload the expected hashes in there, or to print them in the workflow's console output.

@gendx gendx merged commit af79d92 into google:master May 13, 2020
@gendx gendx deleted the reproducible-check branch May 13, 2020 15:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Reproducible builds
3 participants