Skip to content

fix: verify installer downloads with checksums#4676

Open
JuanERombado wants to merge 2 commits into
Scottcjn:mainfrom
JuanERombado:codex/install-checksum-verification
Open

fix: verify installer downloads with checksums#4676
JuanERombado wants to merge 2 commits into
Scottcjn:mainfrom
JuanERombado:codex/install-checksum-verification

Conversation

@JuanERombado
Copy link
Copy Markdown

@JuanERombado JuanERombado commented May 12, 2026

Summary

Fixes #2033 by hardening the one-line installer download path:

  • removes curl --insecure / wget --no-check-certificate TLS bypasses
  • downloads the existing miners/checksums.sha256 manifest before installing miner files
  • verifies rustchain_linux_miner.py and fingerprint_checks.py with SHA-256 before writing config or starting the miner
  • updates the manifest hashes so the current Linux/macOS miner entries match the checked-in files
  • switches stale installer URLs to the current Linux miner paths and adds RUSTCHAIN_REF / RUSTCHAIN_BASE_URL overrides for release/testing flows

Proof

bash -n install.sh
bash install.sh --wallet test-wallet --dry-run
HOME=/tmp/rustchain-install-home RUSTCHAIN_BASE_URL=http://127.0.0.1:8765 ./install.sh --wallet codex-test --silent

The local served-tree install verified both downloads:

✓ Verified: linux/rustchain_linux_miner.py
✓ Verified: linux/fingerprint_checks.py
Downloaded to: /tmp/rustchain-install-home/.rustchain/

Bounty

This targets the maintainer-confirmed 5 RTC hardening task in #2033. Payout wallet can be provided when requested by the maintainer.

BCOS

Security-sensitive supply-chain hardening; please apply BCOS-L2 if required by the repo process.

@github-actions github-actions Bot added the BCOS-L1 Beacon Certified Open Source tier BCOS-L1 (required for non-doc PRs) label May 12, 2026
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 12, 2026

Welcome to RustChain! Thanks for your first pull request.

Before we review, please make sure:

  • Your PR has a BCOS-L1 or BCOS-L2 label
  • New code files include an SPDX license header
  • You've tested your changes against the live node

Bounty tiers: Micro (1-10 RTC) | Standard (20-50) | Major (75-100) | Critical (100-150)

A maintainer will review your PR soon. Thanks for contributing!

@github-actions github-actions Bot added the size/M PR: 51-200 lines label May 12, 2026
@xordanx xordanx mentioned this pull request May 12, 2026
Open
strongkeep-debug
strongkeep-debug suggested changes May 12, 2026
View reviewed changes
Copy link
Copy Markdown

@strongkeep-debug strongkeep-debug left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Requesting changes because the checksum mechanism works on the Linux path, but the patch leaves the still-supported macOS path selecting Linux artifacts. install.sh accepts Darwin as a supported OS, yet MINER_PATH and FINGERPRINT_PATH are fixed before OS detection to linux/rustchain_linux_miner.py and linux/fingerprint_checks.py. The new manifest contains a macOS checksum, but the installer never selects that macOS entry.

Check Result
bash -n install.sh Passed.
Linux dry-run with uname -s=Linux, uname -m=x86_64 Printed the new Linux miner, fingerprint, and checksum URLs as expected.
macOS dry-run with uname -s=Darwin, uname -m=arm64 Still printed the Linux miner and Linux fingerprint URLs.
miners/checksums.sha256 verification against checked-in files All three manifest entries match the current files.
Local served-tree install with RUSTCHAIN_BASE_URL=http://127.0.0.1:8765 Downloaded and verified the Linux miner and fingerprint files, then completed install setup.
git diff --check origin/main...HEAD -- install.sh miners/checksums.sha256 Passed.

The fix can stay small: choose the miner and fingerprint manifest paths after OS detection, or explicitly remove macOS from the supported path in this installer change. As written, a macOS user gets a successful checksum verification over the wrong platform files, which makes the hardening look complete while changing the installer behavior underneath them.

saim256
saim256 suggested changes May 12, 2026
View reviewed changes
Copy link
Copy Markdown

@saim256 saim256 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Requesting changes on current head 631a2d0a8962d658244c0a906f44b15e1c225270.

Blocking finding:

  • The checksum hardening pins MINER_PATH="linux/rustchain_linux_miner.py" and FINGERPRINT_PATH="linux/fingerprint_checks.py" before OS detection, but the installer still accepts Darwin as a supported OS. Dry-run/download output later uses those precomputed URLs and manifest paths, so a macOS install would verify and install the Linux miner/fingerprint files rather than the macOS miner already listed in miners/checksums.sha256. That makes checksum verification succeed for the wrong platform artifact.

Validation performed in this Windows environment:

  • Confirmed static flow in install.sh: paths set at lines 31-34, Darwin accepted at lines 88-91, dry-run prints those fixed URLs at lines 223-226, and verification uses the same fixed manifest paths at lines 249-250.
  • Verified the updated manifest hashes match the checked-in Linux miner, Linux fingerprint helper, and macOS miner using Get-FileHash -Algorithm SHA256.
  • git diff --check origin/main...HEAD -- install.sh miners/checksums.sha256 -> passed
  • python tools\bcos_spdx_check.py --base-ref origin/main -> BCOS SPDX check: OK
  • Could not run bash -n install.sh or shell dry-runs locally because bash is not installed on this Windows runner.

A small fix would be to choose the miner/fingerprint manifest paths after OS detection, or to explicitly narrow this installer to Linux if macOS is no longer supported by this path.

jaxint
jaxint approved these changes May 12, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

@jaxint jaxint left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Thanks for contributing. Approved.

jaxint
jaxint approved these changes May 12, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

@jaxint jaxint left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Thanks for contributing. Approved.

jaxint
jaxint approved these changes May 12, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

@jaxint jaxint left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Thanks for contributing. Approved.

jaxint
jaxint approved these changes May 12, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

@jaxint jaxint left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Thanks for contributing. Approved.

jaxint
jaxint approved these changes May 12, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

@jaxint jaxint left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Thanks for contributing. Approved.

jaxint
jaxint approved these changes May 12, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

@jaxint jaxint left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code review: LGTM! Thanks for contributing to RustChain. Approved.

@JuanERombado
Copy link
Copy Markdown
Author

JuanERombado commented May 12, 2026

Bounty/fixer payout info:

RTC wallet/miner ID: RTCd84b6e2d917d0272ecaae49f2f0dfe2f5474d585

Requesting bounty/fixer consideration for resolving the installer download integrity issue in this PR. The patch removes insecure download flags, verifies downloaded installer payloads against the checksum manifest, and includes dry-run/local installer validation.

@JuanERombado JuanERombado requested a review from Scottcjn as a code owner May 12, 2026 19:04
@JuanERombado
Copy link
Copy Markdown
Author

JuanERombado commented May 12, 2026

Addressed the macOS artifact-selection blocker in commit c9f2724. The installer now chooses miner/fingerprint manifest paths after OS detection: Linux uses miners/linux/* and macOS uses miners/macos/*. I added a macOS fingerprint helper path plus checksum entry so macOS verification no longer succeeds against Linux URLs. Validation: bash -n install.sh, Linux dry-run URLs, simulated Darwin/arm64 dry-run URLs, checksum manifest verification, py_compile on the new macOS fingerprint helper, BCOS SPDX check, and git diff --check.

@github-actions github-actions Bot added size/XL PR: 500+ lines and removed size/M PR: 51-200 lines labels May 12, 2026
dazer1234
dazer1234 reviewed May 12, 2026
View reviewed changes
Copy link
Copy Markdown

@dazer1234 dazer1234 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code review for RustChain bounty #73.

Summary: The installer now downloads the miner, fingerprint helper, and checksum manifest from the same ref and verifies the downloaded scripts before installing. The OS-specific paths also line up with the existing linux/macos miner directories.

Findings:

  • Low: this protects against transport/corruption mismatches, but the checksum manifest is fetched from the same Git ref as the payloads, so it does not protect against a compromised ref. That is probably acceptable for this installer, but the README/help text should avoid implying this is a supply-chain signature.

Verdict: Good improvement with one documentation caveat.

Reviewed with OpenAI Codex assistance.

jaxint
jaxint approved these changes May 12, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

@jaxint jaxint left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Thanks for contributing. Approved.

jaxint
jaxint approved these changes May 12, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

@jaxint jaxint left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Thanks for contributing. Approved.

jaxint
jaxint approved these changes May 12, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

@jaxint jaxint left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Thanks for contributing. Approved.

jaxint
jaxint approved these changes May 12, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

@jaxint jaxint left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Thanks for contributing. Approved.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Scottcjn Scottcjn Awaiting requested review from Scottcjn Scottcjn is a code owner

4 more reviewers

@dazer1234 dazer1234 dazer1234 left review comments

@strongkeep-debug strongkeep-debug strongkeep-debug requested changes

@saim256 saim256 saim256 requested changes

@jaxint jaxint jaxint approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

BCOS-L1 Beacon Certified Open Source tier BCOS-L1 (required for non-doc PRs) size/XL PR: 500+ lines

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Security: install.sh downloads and executes remote code without verification

5 participants

@JuanERombado @dazer1234 @jaxint @strongkeep-debug @saim256