Skip to content

fix: require admin key on all BFT consensus POST endpoints #6638

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
BossChaos wants to merge 1 commit into from
Closed

fix: require admin key on all BFT consensus POST endpoints #6638

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 26 additions & 3 deletions node/rustchain_bft_consensus.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1023,6 +1023,7 @@ def get_status(self) -> Dict:

def create_bft_routes(app, bft: BFTConsensus):
"""Add BFT consensus routes to Flask app"""
import os
from flask import request, jsonify

def _json_object():
Expand All @@ -1034,6 +1035,16 @@ def _json_object():
def _missing_fields(data: Dict, required: Iterable[str]) -> List[str]:
return [field for field in required if field not in data]

def _require_admin():
"""Require RC_ADMIN_KEY for administrative BFT endpoints."""
admin_key = os.environ.get("RC_ADMIN_KEY", "")
if not admin_key:
return jsonify({'error': 'RC_ADMIN_KEY not configured -- BFT admin endpoints disabled'}), 503
provided = request.headers.get("X-Admin-Key", "")
Copy link
Copy Markdown
Contributor

@luminet1 luminet1 May 30, 2026

Choose a reason for hiding this comment

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

This makes /bft/message and /bft/view_change require X-Admin-Key, but the local peer broadcasters at _broadcast_message() and _broadcast_view_change() still call requests.post(...) without that header. With RC_ADMIN_KEY configured, normal peer traffic will be rejected before receive_message() / handle_view_change() runs, so the fix trades public injection for a consensus liveness regression. Please update the broadcast path or use a separate peer-auth mechanism, and add a regression test covering authenticated inter-node BFT traffic.

if not hmac.compare_digest(provided, admin_key):
return jsonify({'error': 'Unauthorized -- admin key required'}), 401
return None

def _internal_error_response(message: str, status_code: int):
return jsonify({'error': message}), status_code

Expand All @@ -1044,7 +1055,11 @@ def bft_status():

@app.route('/bft/message', methods=['POST'])
def bft_receive_message():
"""Receive consensus message from peer"""
"""Receive consensus message from peer (internal P2P only -- public Internet)."""
# SECURITY: Require admin key -- prevents unauthorized BFT message injection
admin_err = _require_admin()
if admin_err:
return admin_err
try:
msg_data, error = _json_object()
if error:
Expand All @@ -1067,7 +1082,11 @@ def bft_receive_message():

@app.route('/bft/view_change', methods=['POST'])
def bft_view_change():
"""Receive view change message"""
"""Receive view change message."""
# SECURITY: Require admin key -- prevents unauthorized BFT view changes
admin_err = _require_admin()
if admin_err:
return admin_err
try:
msg_data, error = _json_object()
if error:
Expand All @@ -1090,7 +1109,11 @@ def bft_view_change():

@app.route('/bft/propose', methods=['POST'])
def bft_propose():
"""Manually trigger epoch proposal (admin)"""
"""Manually trigger epoch proposal (admin only)."""
# SECURITY: Require admin key -- unauthorized epoch proposals can disrupt consensus
admin_err = _require_admin()
if admin_err:
return admin_err
try:
data = request.get_json(silent=True)
if not isinstance(data, dict):
Expand Down
Loading