Skip to content

fix: use constant-time Sophia admin checks#4216

Closed
bolasse1234 wants to merge 1 commit intoScottcjn:mainfrom
bolasse1234:codex/constant-time-sophia-admin
Closed

fix: use constant-time Sophia admin checks#4216
bolasse1234 wants to merge 1 commit intoScottcjn:mainfrom
bolasse1234:codex/constant-time-sophia-admin

Conversation

@bolasse1234
Copy link
Copy Markdown
Contributor

Summary

Fixes #3228 and #3229.

Two Sophia admin paths still compared configured admin keys with plain string equality:

  • node/sophia_attestation_inspector.py for /sophia/inspect and /sophia/batch
  • node/sophia_governor_review_service.py for review-service admin auth

This switches those checks to hmac.compare_digest() and adds regression tests that spy on the comparison path so the tests cover the timing-safe behavior, not just the HTTP status.

Severity: Medium timing-side-channel hardening. These are public, already-reported issues; I am claiming clean implementation credit, not first-finder credit.

Payment: please use my GitHub-login miner_id for any payout.

Tests

  • python -m pytest node\tests\test_sophia_attestation_admin_auth.py node\tests\test_sophia_governor_review_service.py -q => 15 passed
  • python -m py_compile node\sophia_attestation_inspector.py node\sophia_governor_review_service.py node\tests\test_sophia_attestation_admin_auth.py node\tests\test_sophia_governor_review_service.py
  • git diff --check

@github-actions github-actions Bot added BCOS-L1 Beacon Certified Open Source tier BCOS-L1 (required for non-doc PRs) node Node server related tests Test suite changes labels May 10, 2026
@github-actions
Copy link
Copy Markdown
Contributor

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 10, 2026
@bolasse1234
Copy link
Copy Markdown
Contributor Author

Closing this because #4213 and #4214 already cover the same two Sophia admin comparison fixes with clean focused PRs. I missed those when I searched before opening this one; no need to make maintainers review a duplicate.

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

Labels

BCOS-L1 Beacon Certified Open Source tier BCOS-L1 (required for non-doc PRs) node Node server related size/M PR: 51-200 lines tests Test suite changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] sophia_attestation_inspector.py: _is_admin() uses == for key comparison — timing attack on inspection endpoints

1 participant