xmr-btc-swap icon indicating copy to clipboard operation
xmr-btc-swap copied to clipboard

monero wallet-rpc: check binaries sha256sum

Open cirocosta opened this issue 3 years ago • 6 comments

After downloading the binaries (in case we don't see them in the filesystem already)

https://github.com/comit-network/xmr-btc-swap/blob/42125049413ea2d1fbf389ad9ff0642ea02d6aba/swap/src/monero/wallet_rpc.rs#L84

there are no checks for integrity being performed, but I believe it'd be best if we did - sure, downloading over HTTPS would indeed guarantee that we're not getting anything that's not coming from the servers we trust (as long as we trust the root CAs), but ideally we should make sure they are also what we expect them to be: the exact contents are reproducibly built by the community (e.g., see https://github.com/monero-project/gitian.sigs/blob/master/v0.17.2.0-linux/selsta/monero-linux-0.17-build.assert), not just what's said to be true by the getmonero.org servers.

My recommendation:

  1. like we have a constant for the URL to fetch binaries for each OS & arch, have a constant for their checksums
  2. pick the sums from https://www.getmonero.org/downloads/hashes.txt after validating that the signed message is valid (+ confirm looking at gitian sigs)
  3. after reqwest downloads the contents and before the extraction, pipe those bytes through sha256sum and check

Wdyt?

thx!

cirocosta avatar Aug 22 '21 13:08 cirocosta

Yes sounds like a good idea!

Would definitely accept a PR that implements this.

As for the actual implementation, doing as much as possible in pure Rust would be preferable, i.e. we don't want to rely on the use having sha256sum installed as a binary, instead we should do the actual checksum check ourselves.

thomaseizinger avatar Aug 23 '21 04:08 thomaseizinger

Seems difficult to accomplish this with the current "decompress as the file is downloaded" behavior. https://github.com/comit-network/xmr-btc-swap/blob/c3b474d7db598f3afb6a5ade8264ac0a1f5d0ece/swap/src/monero/wallet_rpc.rs#L234-L237

ikmckenz avatar Feb 21 '24 20:02 ikmckenz

@ikmckenz maybe look into separating the download and decompress stages?

delta1 avatar Feb 21 '24 20:02 delta1

That's fine with me, but we might lose this "decompress as download is still happening" trick. Not that it's super important I guess, this download is a one time setup related thing.

ikmckenz avatar Feb 21 '24 23:02 ikmckenz

Yeah. And since the hashes come from the same domain, we would also have to verify the pgp signature.

delta1 avatar Feb 22 '24 04:02 delta1

Since https://www.getmonero.org/downloads/hashes.txt only has the most recent releases hashes and we are rarely on the most recent copy of the cli, I propose just hardcoding the hashes for the version we use in the code and checking against that. Still offers protection of hash check, without needing to add even more complexity for pgp verification (can be done by developers on updating).

ikmckenz avatar Mar 09 '24 20:03 ikmckenz