Page MenuHomePhabricator

Add instructions for verifying download integrity against release signer keys
ClosedPublic

Authored by jasonbcox on Mon, Dec 30, 20:17.

Details

Summary

The current process for verifying Bitcoin ABC binaries is unclear.
An example of how users experience this can be seen here: https://old.reddit.com/r/btc/comments/egj10s/where_can_i_find_bitcoin_abc_pgp_keys/
However, that example only scratches the surface as there are multiple issues here:

  1. The keys are not easy to find/download.
  2. Having some copy of the key fingerprints has no gaurantees against tampering of the fingerprints out of the box.
  3. While verifying the binary hashes match the signature file(s) is easy, not all users verify the integrity of the signature files themselves.

This patch is a good first step to help tackling the above issues. It provides a mechanism for users to easily identify
tampering in any part of the download process, given that they are not downloading a fresh copy of the release keys.
If the later cannot be assumed, the user is provided with the necessary tools to do so for future downloads.

Test Plan

Version 0.20.11+:

# test a similar set of commands on locally-generated source package
ninja package_source
VERSION="0.20.11"
KEYS_FILE="bitcoin-abc-${VERSION}/contrib/gitian-signing/keys.txt"
# tweaked the line below since no release currently contains keys.txt
tar -zxOf <build-dir>/bitcoin-abc-0.20.11.tar.gz "${KEYS_FILE}" | while read FINGERPRINT _; do gpg --recv-keys "${FINGERPRINT}"; done

Earlier versions:

  1. Download any number of binaries from https://download.bitcoinabc.org/0.20.9/<platform>
  2. Download signature files from https://download.bitcoinabc.org/0.20.9/
  3. Follow the second set of instructions for earlier versions.

Diff Detail

Repository
rABC Bitcoin ABC
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

jasonbcox created this revision.Mon, Dec 30, 20:17
jasonbcox added inline comments.Mon, Dec 30, 20:18
doc/README.md
19 ↗(On Diff #15090)

Reviewer note: Doing gpg --recv-keys for *every* release is important, because fetching the keys will also fetch any revoke certificates, if they exist.

jasonbcox edited the test plan for this revision. (Show Details)Mon, Dec 30, 20:20
Fabien requested changes to this revision.Thu, Jan 2, 11:26
Fabien added a subscriber: Fabien.

Adding the keys.txt file at the root of download.bitcoinabc.org will cause additional maintenance, as someone has to remember to re-upload the file each time it gets updated.
This seems an unnecessary duplication to me as the file is already in the source tree.
Furthermore doing so can cause verification of the previous releases to fail if the file is updated.

I'll suggest to add the keys.txt into the source package and extract the file from the release source archive instead:

wget -q https://download.bitcoinabc.org/${VERSION}/src/bitcoin-abc-${VERSION}.tar.gz
tar -xvzf bitcoin-abc-${VERSION}.tar.gz contrib/gitian-signing/keys.txt
[...]

And for the previous releases:

wget -q https://github.com/Bitcoin-ABC/bitcoin-abc/blob/v${VERSION}/contrib/gitian-signing/keys.txt
[...]
This revision now requires changes to proceed.Thu, Jan 2, 11:26
jasonbcox updated this revision to Diff 15409.Mon, Jan 13, 17:52
  • Use versioned keys.txt file from bitcoinabc.org or github.com depending on availability by version.
  • Fixed a bug where the script would break on multiple signature files.
jasonbcox edited the test plan for this revision. (Show Details)Mon, Jan 13, 17:58
Fabien requested changes to this revision.Tue, Jan 14, 11:10
Fabien added inline comments.
doc/README.md
35 ↗(On Diff #15409)

You can have the common part in a common section, something like:

  • Get the keys for v >= 0.20.11
  • Get the keys for v < 0.20.11
  • Check the binaries (common to all versions)
35 ↗(On Diff #15409)

This command does not work for me. It looks like the pipe to ls is not working, so it ends up verifying all the files even if they don't exist, which causes the failure.

This revision now requires changes to proceed.Tue, Jan 14, 11:10
jasonbcox updated this revision to Diff 15631.Fri, Jan 17, 23:30

Fixes according to feedback:

  • Merge common lines into a single step
  • Fix file verification step when the working directory isn't clean
Fabien accepted this revision.Mon, Jan 20, 12:39
Fabien added inline comments.
doc/README.md
41 ↗(On Diff #15631)

Nit: Extra space after the dot.

This revision is now accepted and ready to land.Mon, Jan 20, 12:39