mirror of
https://github.com/bitcoin/bitcoin.git
synced 2026-09-11 21:20:39 +02:00
Merge bitcoin/bitcoin#35980: contrib: reject divergent verify-commits history
465bca734econtrib: reject divergent verify-commits history (Lőrinc)b3d1dca338contrib: fail on verify-commits ancestry errors (Lőrinc) Pull request description: **Problem:** `verify-commits.py` checks a Git commit's history for trusted signatures and tree hashes back to configured roots. The documented workflow runs this check after fetching a commit and before checkout, proceeding only when the script succeeds. A commit that is an ancestor of a configured root is intentionally accepted without checking earlier history. The script also takes this success path after Git errors or for divergent commits, even though neither establishes that relationship. **Fix:** Require Git to prove the ancestor relationship before taking this success path. **Reproducers:** Each commit can be validated manually. <details><summary>Manual reproducer: Git error</summary> Run this on `master` and at this PR's head: ```bash contrib/verify-commits/verify-commits.py 0000000000000000000000000000000000000000 && echo ❌ || echo ✅ ``` `master` exits successfully without verifying the missing commit, while the PR head rejects the Git error. </details> <details><summary>Manual reproducer: divergent history</summary> On `master` and at this PR's head, create an unreferenced sibling of the trusted root and run the verifier: ```bash root=$(head -n1 contrib/verify-commits/trusted-git-root) divergent_commit=$(git commit-tree "$root^{tree}" -p "$root^" -m 'divergent commit') contrib/verify-commits/verify-commits.py "$divergent_commit" && echo ❌ || echo ✅ ``` `master` exits successfully without verifying the sibling commit, while the PR head rejects divergent history. </details> This issue was also found and disclosed responsibly by the Red Team 🟥. ACKs for top commit: 151henry151: tACK465bca734ejeanpablojp: tACK465bca734eachow101: ACK465bca734esedited: ACK465bca734emaflcko: review ACK465bca734e🥜 Tree-SHA512: 72b8cd9902d881e59a1d99fda8e5d511806826fa27c05a2c21a7d2eb62b2a5a0b1b6bdc67e8d19d57f9171278f4858fd019eb7890b960df0475ba4713683f0ac
This commit is contained in:
@@ -14,6 +14,23 @@ import time
|
||||
|
||||
GIT = os.getenv('GIT', 'git')
|
||||
|
||||
def is_ancestor(older, newer, root_name):
|
||||
"""Return whether older is an ancestor of newer, rejecting Git errors."""
|
||||
result = subprocess.run([GIT, "merge-base", "--is-ancestor", older, newer])
|
||||
if result.returncode not in (0, 1):
|
||||
print(f'Failed to determine ancestry between "{older}" and "{newer}" for the {root_name} (git merge-base exited with {result.returncode}).', file=sys.stderr)
|
||||
sys.exit(1)
|
||||
return result.returncode == 0
|
||||
|
||||
def predates(commit, root, root_name):
|
||||
"""Return whether commit is provably older than root, rejecting divergent history."""
|
||||
if is_ancestor(root, commit, root_name):
|
||||
return False
|
||||
elif is_ancestor(commit, root, root_name):
|
||||
return True
|
||||
print(f'"{commit}" diverges from the {root_name} "{root}", refusing to verify.', file=sys.stderr)
|
||||
sys.exit(1)
|
||||
|
||||
def tree_sha512sum(commit='HEAD'):
|
||||
"""Calculate the Tree-sha512 for the commit.
|
||||
|
||||
@@ -107,27 +124,23 @@ def main():
|
||||
logging.debug("verify-commits: [in-progress] processing commit {}".format(current_commit[:8]))
|
||||
|
||||
if current_commit == verified_root:
|
||||
# Ensure the trusted root identifies an existing commit.
|
||||
is_ancestor(verified_root, current_commit, "trusted Git root")
|
||||
print('There is a valid path from "{}" to {} where all commits are signed!'.format(initial_commit, verified_root))
|
||||
sys.exit(0)
|
||||
else:
|
||||
# Make sure this commit isn't older than trusted roots
|
||||
check_root_older_res = subprocess.run([GIT, "merge-base", "--is-ancestor", verified_root, current_commit])
|
||||
if check_root_older_res.returncode != 0:
|
||||
print(f"\"{current_commit}\" predates the trusted root, stopping!")
|
||||
sys.exit(0)
|
||||
elif predates(current_commit, verified_root, "trusted Git root"):
|
||||
print(f"\"{current_commit}\" predates the trusted root, stopping!")
|
||||
sys.exit(0)
|
||||
|
||||
if verify_tree:
|
||||
if current_commit == verified_sha512_root:
|
||||
print("All Tree-SHA512s matched up to {}".format(verified_sha512_root), file=sys.stderr)
|
||||
verify_tree = False
|
||||
no_sha1 = False
|
||||
else:
|
||||
# Skip the tree check if we are older than the trusted root
|
||||
check_root_older_res = subprocess.run([GIT, "merge-base", "--is-ancestor", verified_sha512_root, current_commit])
|
||||
if check_root_older_res.returncode != 0:
|
||||
print(f"\"{current_commit}\" predates the trusted SHA512 root, disabling tree verification.")
|
||||
verify_tree = False
|
||||
no_sha1 = False
|
||||
elif predates(current_commit, verified_sha512_root, "trusted Tree-SHA512 root"):
|
||||
print(f"\"{current_commit}\" predates the trusted SHA512 root, disabling tree verification.")
|
||||
verify_tree = False
|
||||
no_sha1 = False
|
||||
|
||||
|
||||
os.environ['BITCOIN_VERIFY_COMMITS_ALLOW_SHA1'] = "0" if no_sha1 else "1"
|
||||
|
||||
Reference in New Issue
Block a user