xmr-btc-swap
xmr-btc-swap copied to clipboard
monero wallet-rpc: check binaries sha256sum
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:
- like we have a constant for the URL to fetch binaries for each OS & arch, have a constant for their checksums
- pick the sums from https://www.getmonero.org/downloads/hashes.txt after validating that the signed message is valid (+ confirm looking at gitian sigs)
- after
reqwest
downloads the contents and before the extraction, pipe those bytes through sha256sum and check
Wdyt?
thx!
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.
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 maybe look into separating the download and decompress stages?
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.
Yeah. And since the hashes come from the same domain, we would also have to verify the pgp signature.
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).