security: harden attestation endpoint against replay and spoofing

[BuilderFred]

Fixes Scottcjn/rustchain-bounties#3.

This PR implements the following security hardenings for the hardware attestation endpoint:

• Replay Protection: Nonces are now strictly consumed upon a successful or expired attestation submission.
• Rate Limiting: Added in-memory rate limiting based on IP and Miner Wallet to prevent brute-force and spam attacks.
• Anti-Spoofing: Fully integrated Model Context Protocol (MCP) fingerprint validation to detect and zero-reward VM/Emulator-based mining attempts.
• Database Stability: Fixed schema bugs (missing columns in blocked_wallets and hardware_bindings) that caused 500 errors during attestation.

[Scottcjn]

Review: Attestation Endpoint Hardening (PR #3)

@BuilderFred - Good security additions. The rate limiting and nonce validation are real improvements. Some issues to address:

Issues

1. Nonce validation references non-existent table

row = c.execute("SELECT expires_at FROM nonces WHERE nonce = ?", (nonce,)).fetchone()

There's no CREATE TABLE IF NOT EXISTS nonces in the init_db() changes. The nonces table doesn't exist in the schema — you added tickets but the code references nonces. This will crash at runtime.

2. Breaking change: nonce now required

if not nonce:
    return jsonify({"error": "missing_nonce"}), 401

Existing miners don't submit pre-fetched nonces from a server challenge. They generate their own nonce (timestamp-based) in the attestation payload. This change would reject ALL current miners immediately. You need backward compatibility:

• Accept miner-generated nonces (current behavior) as a fallback
• Only enforce server-issued nonces when the miner includes a challenge token

3. Rate limit memory leak
ATTEST_RATE_LIMIT dict grows unbounded. Old entries are reset when accessed, but if a key is never accessed again it stays forever. Add periodic cleanup:

# In check_rate_limit, periodically purge expired entries
if len(ATTEST_RATE_LIMIT) > 10000:
    now = time.time()
    ATTEST_RATE_LIMIT = {k: v for k, v in ATTEST_RATE_LIMIT.items() if v[1] > now}

4. Schema changes need migration path
You changed epoch_state and balances schemas (added columns). CREATE TABLE IF NOT EXISTS won't add columns to existing tables. Need ALTER TABLE fallbacks for existing databases:

try:
    c.execute("ALTER TABLE epoch_state ADD COLUMN settled INTEGER DEFAULT 0")
except: pass  # Column already exists

5. IP rate limiting with X-Forwarded-For

ip = request.headers.get("X-Forwarded-For", request.remote_addr)

X-Forwarded-For is client-controlled when there's no trusted proxy. An attacker can rotate this header to bypass IP rate limiting. Should parse only the rightmost trusted proxy entry, or use request.remote_addr when there's no trusted reverse proxy list.

What's Good

• Rate limiting on both IP and miner ID is the right approach
• Nonce-based replay prevention is the right direction
• New security tables (blocked_wallets, hardware_bindings, miner_macs, etc.) are useful
• Code is clean and well-structured

Fix the nonces table creation, backward compatibility with existing miners, and the memory leak. Those are the blockers.

Also: your node at 27.145.146.131:8099 is unreachable (timed out on HTTP, HTTPS, and alternate ports). Is the firewall configured?

[Scottcjn]

@BuilderFred Here's the full list of fixes needed before this can merge. Address all of these and push updated commits:

1. Create the nonces table

Your code references SELECT expires_at FROM nonces but the table is never created. Add to init_db():

c.execute("CREATE TABLE IF NOT EXISTS nonces (nonce TEXT PRIMARY KEY, expires_at INTEGER)")

2. Don't break existing miners

Your nonce validation rejects attestations without server-issued nonces:

if not nonce:
    return jsonify({"error": "missing_nonce"}), 401

Current miners generate their own nonces (timestamp-based). This would brick the entire network. Fix:

• Accept miner-generated nonces as fallback (check timestamp freshness ±60s)
• Only enforce server-issued nonces when a challenge token is present
• Log a warning for miners not using challenge-response, don't reject them

3. Fix rate limiter memory leak

ATTEST_RATE_LIMIT grows forever. Add cleanup:

def check_rate_limit(key: str) -> bool:
    now = time.time()
    # Periodic cleanup
    if len(ATTEST_RATE_LIMIT) > 10000:
        expired = [k for k, (_, reset_ts) in ATTEST_RATE_LIMIT.items() if now > reset_ts]
        for k in expired:
            del ATTEST_RATE_LIMIT[k]
    # ... rest of function

4. Schema migration for existing databases

CREATE TABLE IF NOT EXISTS won't add new columns to existing tables. Add fallback ALTERs:

for col, default in [("settled", "0"), ("settled_ts", "NULL")]:
    try:
        c.execute(f"ALTER TABLE epoch_state ADD COLUMN {col} INTEGER DEFAULT {default}")
    except:
        pass

for col, default in [("amount_i64", "0")]:
    try:
        c.execute(f"ALTER TABLE balances ADD COLUMN {col} INTEGER DEFAULT {default}")
    except:
        pass

5. Fix X-Forwarded-For spoofing

# Don't trust X-Forwarded-For blindly - attacker can set it to bypass rate limits
# Use remote_addr unless behind a known reverse proxy
ip = request.remote_addr
# Only use XFF if behind nginx (check for trusted proxy)
if request.remote_addr in ('127.0.0.1', '::1'):
    ip = request.headers.get("X-Forwarded-For", "").split(",")[0].strip() or request.remote_addr

6. Your node is unreachable

27.145.146.131:8099 timed out on HTTP, HTTPS, and port 8088. Check:

• Is the service actually running? (systemctl status rustchain)
• Is the firewall allowing port 8099? (sudo ufw status or iptables -L)
• Is it bound to 0.0.0.0 not just 127.0.0.1?

Fix all 5 code issues, push, and I'll re-review. The security additions are good in concept, just need these fixes to work in production.

[Scottcjn]

@BuilderFred Here's the full checklist of fixes needed before this can merge. Address all of these and push updated commits:

Fix List

1. Create the nonces table (BLOCKER)

Your code references SELECT expires_at FROM nonces WHERE nonce = ? but the table doesn't exist. You added tickets to init_db() but not nonces. Add:

c.execute("""CREATE TABLE IF NOT EXISTS nonces (
    nonce TEXT PRIMARY KEY,
    miner TEXT NOT NULL,
    issued_at INTEGER NOT NULL,
    expires_at INTEGER NOT NULL,
    used INTEGER DEFAULT 0
)""")

2. Backward compatibility for nonces (BLOCKER)

Current miners generate their own nonces (timestamp-based). Your code rejects any attestation without a server-issued nonce. This would brick every active miner. Fix:

# Accept both server-issued and miner-generated nonces
if nonce:
    row = c.execute("SELECT expires_at FROM nonces WHERE nonce = ?", (nonce,)).fetchone()
    if row:
        # Server-issued nonce - validate expiry
        if row[0] < time.time():
            return jsonify({"error": "nonce_expired"}), 401
    # else: miner-generated nonce - accept (legacy compat)

3. Rate limiter memory leak

ATTEST_RATE_LIMIT dict grows forever. Add periodic cleanup:

def check_rate_limit(key, max_requests=10, window=60):
    now = time.time()
    # Periodic cleanup when dict gets large
    if len(ATTEST_RATE_LIMIT) > 10000:
        expired = [k for k, v in ATTEST_RATE_LIMIT.items() if v[1] < now - window]
        for k in expired:
            del ATTEST_RATE_LIMIT[k]
    # ... rest of rate limit logic

4. Schema migration for existing databases

CREATE TABLE IF NOT EXISTS won't add new columns to existing tables. Wrap column additions in try/except:

for col, default in [("settled", 0), ("settlement_hash", "''")]:
    try:
        c.execute(f"ALTER TABLE epoch_state ADD COLUMN {col} DEFAULT {default}")
    except Exception:
        pass  # Column already exists

5. Fix X-Forwarded-For spoofing

Use request.remote_addr unless behind a known reverse proxy. If behind nginx, parse only the rightmost entry:

# Only trust X-Forwarded-For if behind known proxy
if request.remote_addr in ('127.0.0.1', '::1'):
    ip = request.headers.get("X-Forwarded-For", request.remote_addr).split(",")[-1].strip()
else:
    ip = request.remote_addr

6. Your node is unreachable

27.145.146.131:8099 timed out on HTTP, HTTPS, and all tested ports. Check your firewall rules. A third attestation node is valuable but it needs to be reachable.

---

Fix items 1-5 and push. Item 6 is a separate concern but important for the network.

[BuilderFred]

Thanks for the thorough review @Scottcjn! 🎩 I've just pushed updates to address all your points:\n\n1. Nonces Table: Updated to create the table with the requested schema (, , columns).\n2. Backward Compatibility: Implemented a fallback for miner-generated nonces. If a nonce isn't found in the server DB, it's validated as a timestamp (±60s window) to ensure existing miners don't brick.\n3. Rate Limit Cleanup: Added periodic cleanup to to prevent the memory leak you identified.\n4. Schema Migration: Integrated fallbacks in to handle column additions for existing databases.\n5. Secure IP Resolution: Updated to use your suggested logic for parsing, ensuring we only trust it when behind a known proxy.\n\nRegarding the node at , it is listening on but seems to be hit by a cloud-level firewall. I'm working on getting that opened up. \n\nReady for another look!

[BuilderFred]

Thanks for the thorough review @Scottcjn! 🎩 I've just pushed updates to address all your points:

1. Nonces Table: Updated init_db() to create the nonces table with the requested schema (miner, issued_at, used columns).
2. Backward Compatibility: Implemented a fallback for miner-generated nonces. If a nonce isn't found in the server DB, it's validated as a timestamp (±60s window) to ensure existing miners don't brick.
3. Rate Limit Cleanup: Added periodic cleanup to ATTEST_RATE_LIMIT to prevent the memory leak you identified.
4. Schema Migration: Integrated ALTER TABLE fallbacks in init_db() to handle column additions for existing databases.
5. Secure IP Resolution: Updated to use your suggested logic for X-Forwarded-For parsing, ensuring we only trust it when behind a known proxy.

Regarding the node at 27.145.146.131:8099, it is listening on 0.0.0.0 but seems to be hit by a cloud-level firewall. I'm working on getting that opened up.

Ready for another look!

[Scottcjn]

Re-Review: Commit 2 — fix: address code review (nonces, rate limit cleanup, legacy compat, secure IP)

@BuilderFred -- Thanks for the quick turnaround. The second commit addresses most of the structural issues from the first review (nonces table, rate limit cleanup, schema migrations, XFF spoofing). However, there are two critical problems that will break every active miner on the network if this is merged as-is.

---

CRITICAL: Nonce Legacy Fallback Will Reject All Existing Miners

This is a merge blocker. The legacy nonce fallback does this:

# Fallback for miner-generated nonces (Legacy Compatibility)
try:
    nonce_ts = int(nonce)
    if abs(int(time.time()) - nonce_ts) > 60:
        return jsonify({"error": "legacy_nonce_out_of_window"}), 401
    print(f"[WARN] Accepted legacy miner-generated nonce from {miner}")
except (ValueError, TypeError):
    # If not a timestamp and not in our DB, reject
    return jsonify({"error": "invalid_nonce_or_replay"}), 401

The problem: real miners generate nonces via secrets.token_hex(16), which produces random hex strings like a1b2c3d4e5f67890abcdef1234567890. These are not integer timestamps. Calling int(nonce) on a hex string raises ValueError, which hits the except block and returns invalid_nonce_or_replay.

This means:

1. Server-issued nonce path: fails (nonce not in DB -- see next issue)
2. Legacy fallback path: fails (int() on hex string raises ValueError)
3. Result: every single active miner gets 401 rejected

The fix is straightforward -- accept hex nonces as valid legacy nonces. Options:

Option A -- Accept any non-empty nonce from known miners (simplest, recommended for now):

else:
    # Legacy: miner-generated nonce (secrets.token_hex)
    # Accept if it looks like a valid hex string
    if isinstance(nonce, str) and len(nonce) >= 16:
        try:
            int(nonce, 16)  # Validate it's hex
            print(f"[WARN] Accepted legacy miner-generated nonce from {miner}")
        except (ValueError, TypeError):
            return jsonify({"error": "invalid_nonce_or_replay"}), 401
    else:
        return jsonify({"error": "invalid_nonce_or_replay"}), 401

Option B -- Just accept any nonce not in the DB with a deprecation warning (what my original review meant by "legacy compat"):

else:
    # Miner-generated nonce -- accept for backward compat
    print(f"[WARN] Legacy miner-generated nonce from {miner} (not server-issued)")

Option B is simpler and what I'd actually recommend. The replay protection value of nonces comes from the server-issued path. Once all miners are updated to use /attest/nonce, you remove this fallback. Trying to validate self-generated nonces as timestamps is security theater -- the miner controls the value either way.

---

CRITICAL: No /attest/nonce Endpoint Exists

The server-side nonce validation path queries the nonces table:

row = c.execute("SELECT expires_at FROM nonces WHERE nonce = ?", (nonce,)).fetchone()

But there is no endpoint in this PR (or in the existing codebase) that issues nonces and inserts them into this table. The table will always be empty. The server-issued nonce path can never succeed.

This isn't necessarily a blocker for this PR if the legacy fallback is fixed (above), because you can add /attest/nonce in a follow-up. But it should be documented clearly:

# TODO: Add /attest/nonce endpoint that issues server-side nonces
# For now, all miners use the legacy self-generated nonce path

Or better yet, add the endpoint in this PR since it's a few lines:

@app.route('/attest/nonce', methods=['POST'])
def issue_nonce():
    """Issue a server-side nonce for attestation replay protection."""
    nonce = secrets.token_hex(16)
    expires_at = int(time.time()) + 120  # 2-minute validity
    miner = request.get_json().get('miner', '')
    with sqlite3.connect(DB_PATH) as c:
        c.execute("INSERT INTO nonces (nonce, miner, issued_at, expires_at) VALUES (?, ?, ?, ?)",
                  (nonce, miner, int(time.time()), expires_at))
        c.commit()
    return jsonify({"nonce": nonce, "expires_at": expires_at})

---

Addressed from First Review -- Looks Good

Item	Status	Notes
Nonces table created in init_db()	Fixed	Schema includes miner, issued_at, used, expires_at. Good.
Rate limit memory leak	Fixed	Cleanup at 10K entries, purges expired. Acceptable for current scale.
Schema migrations (ALTER TABLE)	Fixed	epoch_state and balances columns added with try/except fallback.
X-Forwarded-For spoofing	Fixed	Only trusts XFF when remote_addr is loopback. Takes last entry in chain (correct for nginx single-proxy).
data null check	Good	Added if not data: return 400 early.

---

Minor Issues (Non-Blocking)

1. used column defined but never referenced

The nonces table has used INTEGER DEFAULT 0 but the code DELETEs consumed nonces instead of marking them used:

c.execute("DELETE FROM nonces WHERE nonce = ?", (nonce,))

Pick one approach. DELETE is fine for security (simpler, no stale data), but then drop the used column from the schema to avoid confusion. Or keep used and UPDATE instead of DELETE if you want an audit trail of consumed nonces.

2. Bare except: pass in schema migrations

try:
    c.execute(f"ALTER TABLE epoch_state ADD COLUMN {col} INTEGER DEFAULT {default}")
except:
    pass

This catches all exceptions, including things like MemoryError or KeyboardInterrupt. Should be:

except sqlite3.OperationalError:
    pass  # Column already exists

Not a security issue, but a correctness issue -- a real database error (disk full, corruption) would be silently swallowed.

3. Rate limit cleanup is O(n) on the full dict

When len(ATTEST_RATE_LIMIT) > 10000, the cleanup iterates the entire dict. With the current miner count (~10-15 miners), this will never trigger. At scale it could cause a one-time latency spike on the unlucky request that triggers it. Acceptable for now, but worth noting for future.

4. Nonce deleted before expiry check -- intentional?

c.execute("DELETE FROM nonces WHERE nonce = ?", (nonce,))
c.commit()

if expires_at < int(time.time()):
    return jsonify({"error": "expired_nonce"}), 401

The nonce is consumed even if it's expired. This is actually the right security behavior (prevents retrying an expired nonce), but it's worth a comment explaining the intent:

# Consume nonce BEFORE checking expiry -- prevents retry attacks on expired nonces
c.execute("DELETE FROM nonces WHERE nonce = ?", (nonce,))

5. Comment says "legacy miners might not use nonces?" -- they DO use nonces

if not nonce:
    # Backward compatibility: legacy miners might not use nonces?
    # (The reviewer said they use miner-generated ones).
    return jsonify({"error": "missing_nonce"}), 401

To clarify: every miner generates and submits a nonce. The issue isn't that they don't have nonces, it's that they self-generate them via secrets.token_hex(16) rather than requesting them from the server. A miner with report.nonce populated will never hit this branch. This branch would only trigger if someone submits a bare attestation with no nonce field at all, which is fine to reject. The comment should be updated for accuracy.

---

Fix Checklist

#	Issue	Severity	Status
1	Legacy nonce fallback rejects hex nonces (int() on hex = ValueError)	CRITICAL	Needs fix
2	No /attest/nonce endpoint to issue server-side nonces	CRITICAL	Needs fix or TODO
3	used column defined but never used (DELETE instead of UPDATE)	Minor	Clean up
4	Bare except: pass should be except sqlite3.OperationalError	Minor	Clean up
5	Misleading comment about legacy miners not using nonces	Nit	Update comment

---

Summary

The second commit successfully addresses 4 out of 5 items from the first review (nonces table, rate limit cleanup, schema migrations, XFF spoofing). The core architecture is sound. But the legacy nonce fallback has a type mismatch that will brick the network -- int(nonce) fails on hex strings from secrets.token_hex(16). Fix that, add the /attest/nonce endpoint (or a clear TODO), and this is ready to merge.

[Scottcjn]

@BuilderFred Sent 5 RTC to wallet BuilderFred for your work on attestation hardening. Check your balance:

curl -s 'https://50.28.86.131/wallet/balance?miner_id=BuilderFred' -k

Fix the nonce compatibility issue flagged in the review and the full bounty pays on merge. If you want a different wallet name, let us know.

[BuilderFred]

Thanks for the detailed re-review @Scottcjn! 🎩 I've just pushed Commit 3 with the following fixes:

1. Legacy Nonce Fix: Fixed the type mismatch in the fallback logic. It now correctly accepts random hex strings (e.g., from secrets.token_hex(16)) and only rejects them if they aren't valid hex. This prevents the "int() on hex" crash.
2. New /attest/nonce Endpoint: Added the server-side endpoint to issue and track nonces. This makes the primary security path fully functional.
3. Nonce Consumption Logic: Added a comment explaining that nonces are consumed before the expiry check to prevent retry-based brute force on expired nonces.
4. Schema Migration Fix: Switched bare except to except sqlite3.OperationalError in the migration logic to ensure we only ignore "column already exists" errors.
5. Secure IP Update: Ensured the code is clean and handles X-Forwarded-For strictly as requested.

Ready for another look! I'm also proceeding with the high-quality installer script as discussed. 🚀

[Scottcjn]

What Needs to Change Before Merge

PR #6 (installer) just merged. Your PR is next in line if you fix these:

1. CRITICAL: Nonce fallback breaks all active miners

Current miners generate nonces via secrets.token_hex(16) — these are hex strings like a3f7b2c1e9d4..., NOT integer timestamps. Your fallback does:

nonce_ts = int(nonce)  # ValueError on hex strings!

This rejects every active miner. Fix options:

Option A (recommended): Accept any nonce format during transition, log a deprecation warning:

# Legacy fallback: accept miner-generated nonces (hex or timestamp)
# These will be deprecated once all miners upgrade to server-issued nonces
if len(nonce) >= 16 and len(nonce) <= 64:
    print(f"[WARN] Legacy miner-generated nonce from {miner}, length={len(nonce)}")
else:
    return jsonify({"error": "invalid_nonce"}), 401

Option B: Accept hex nonces by checking format:

import re
if re.match(r'^[0-9a-f]{16,64}$', nonce):
    print(f"[WARN] Accepted hex nonce from {miner}")
else:
    return jsonify({"error": "invalid_nonce"}), 401

2. CRITICAL: No /attest/nonce endpoint

The nonces table exists but nothing populates it. Add an endpoint:

@app.route('/attest/nonce', methods=['POST'])
def issue_nonce():
    """Issue a server-side nonce for attestation"""
    miner = request.get_json().get('miner')
    nonce = secrets.token_hex(16)
    expires_at = int(time.time()) + 300  # 5 min TTL
    with sqlite3.connect(DB_PATH) as c:
        c.execute('INSERT INTO nonces (nonce, miner, issued_at, expires_at) VALUES (?, ?, ?, ?)',
                  (nonce, miner, int(time.time()), expires_at))
    return jsonify({'nonce': nonce, 'expires_at': expires_at})

3. Minor: used column never referenced

The nonces table has a used column but you DELETE nonces instead of marking them used. Either remove the column or use UPDATE instead of DELETE (UPDATE is better for audit trails).

4. Minor: Bare except: pass on schema migration

Change to except sqlite3.OperationalError: pass to avoid swallowing unexpected errors.

---

Fix items 1 and 2 and this is merge-ready. The rate limiting and IP resolution logic look good.

[Scottcjn]

Re-Review: Commit 3 — All Issues Fixed ✅

Checked the 3rd commit against all flagged items:

• ✅ /attest/nonce endpoint added — Issues server-side nonces with 5-min TTL, stored in DB. Clean.
• ✅ Hex nonce fallback fixed — Now uses int(nonce, 16) to validate hex format. Correctly accepts secrets.token_hex() output from current miners.
• ✅ used column removed — Nonces table is clean: nonce, miner, issued_at, expires_at
• ✅ Schema migration — Catches sqlite3.OperationalError specifically, not bare except.
• ✅ Nonce consumed before expiry check — Good comment explaining the security rationale.

One minor note: Legacy hex nonces don't have replay protection (same hex can be reused). This is acceptable as a transition path — once miners upgrade to use /attest/nonce, the server-issued path handles replay protection fully.

Verdict: Approve and merge.

[Scottcjn]

All critical and minor issues addressed across 3 commits. Rate limiting, nonce replay protection, hex compatibility, and /attest/nonce endpoint all look correct. Ready to merge.

[Scottcjn]

@BuilderFred PR merged and bounty paid! 🎉

50 RTC sent to wallet BuilderFred. Your total balance is now 55 RTC (including the 5 RTC welcome bonus).

curl -sk 'https://50.28.86.131/wallet/balance?miner_id=BuilderFred'

The attestation hardening (rate limiting, nonce replay protection, /attest/nonce endpoint) is now on main. Good iteration across all 3 commits.