|
| 1 | +# Issue #2640: Replay Defense Fixes - Clean Submission Pass |
| 2 | + |
| 3 | +**Status:** ✅ COMPLETE |
| 4 | +**Date:** 2026-03-28 |
| 5 | +**Test Command:** `python3 -m pytest tests/test_replay_defense.py tests/test_replay_defense_standalone.py tests/test_replay_bounty.py tests/test_fingerprint_replay.py tests/test_signed_transfer_replay.py --tb=short` |
| 6 | + |
| 7 | +--- |
| 8 | + |
| 9 | +## Summary |
| 10 | + |
| 11 | +Re-implemented verified replay-defense fixes in a clean clone, touching only the files necessary for the issue. All 74 tests pass. |
| 12 | + |
| 13 | +--- |
| 14 | + |
| 15 | +## Files Modified |
| 16 | + |
| 17 | +### 1. `node/hardware_fingerprint_replay.py` |
| 18 | + |
| 19 | +**Changes:** |
| 20 | +- Changed `DB_PATH` from module-level constant to dynamic `get_db_path()` function |
| 21 | +- This ensures the database path is read from environment variables at call time, not import time |
| 22 | +- Fixed `compute_fingerprint_hash()` to handle empty dicts correctly (returns valid hash, not empty string) |
| 23 | + |
| 24 | +**Key Changes:** |
| 25 | +```python |
| 26 | +# Before: |
| 27 | +DB_PATH = os.environ.get('RUSTCHAIN_DB_PATH') or os.environ.get('DB_PATH') or '/root/rustchain/rustchain_v2.db' |
| 28 | + |
| 29 | +# After: |
| 30 | +def get_db_path() -> str: |
| 31 | + """Get database path from environment (evaluated at call time, not import time).""" |
| 32 | + return os.environ.get('RUSTCHAIN_DB_PATH') or os.environ.get('DB_PATH') or '/root/rustchain/rustchain_v2.db' |
| 33 | +``` |
| 34 | + |
| 35 | +**Rationale:** The `conftest.py` sets `DB_PATH = ":memory:"` at import time, which was causing test interference when running multiple test files together. The dynamic `get_db_path()` function ensures each test can set its own database path. |
| 36 | + |
| 37 | +--- |
| 38 | + |
| 39 | +### 2. `tests/test_replay_bounty.py` |
| 40 | + |
| 41 | +**Changes:** |
| 42 | +- Updated import from `DB_PATH` to `get_db_path` |
| 43 | + |
| 44 | +--- |
| 45 | + |
| 46 | +### 3. `tests/test_replay_defense_standalone.py` |
| 47 | + |
| 48 | +**Changes:** |
| 49 | +- Updated import from `DB_PATH` to `get_db_path` |
| 50 | +- Added `autouse=True` fixture to ensure fresh database for each test |
| 51 | +- Updated fixture to set `DB_PATH` at test runtime for isolation |
| 52 | +- Fixed `test_empty_fingerprint_hash` to expect valid hash for empty dict (matching comprehensive test file) |
| 53 | + |
| 54 | +--- |
| 55 | + |
| 56 | +### 4. `tests/test_replay_defense.py` |
| 57 | + |
| 58 | +**Changes:** |
| 59 | +- Updated import from `DB_PATH` to `get_db_path` |
| 60 | +- Changed fixture to `autouse=True` for automatic database initialization |
| 61 | +- Updated fixture to set `DB_PATH` at test runtime for isolation |
| 62 | + |
| 63 | +--- |
| 64 | + |
| 65 | +## Test Results |
| 66 | + |
| 67 | +``` |
| 68 | +======================== 74 passed, 5 warnings in 0.52s ======================== |
| 69 | +
|
| 70 | +tests/test_replay_defense.py ............................... [ 41%] |
| 71 | +tests/test_replay_defense_standalone.py ................ [ 63%] |
| 72 | +tests/test_replay_bounty.py .... [ 68%] |
| 73 | +tests/test_fingerprint_replay.py ..................... [ 97%] |
| 74 | +tests/test_signed_transfer_replay.py .. [100%] |
| 75 | +``` |
| 76 | + |
| 77 | +### Test Breakdown |
| 78 | + |
| 79 | +| Test File | Tests | Status | |
| 80 | +|-----------|-------|--------| |
| 81 | +| `test_replay_defense.py` | 31 | ✅ PASS | |
| 82 | +| `test_replay_defense_standalone.py` | 16 | ✅ PASS | |
| 83 | +| `test_replay_bounty.py` | 4 | ✅ PASS | |
| 84 | +| `test_fingerprint_replay.py` | 21 | ✅ PASS | |
| 85 | +| `test_signed_transfer_replay.py` | 2 | ✅ PASS | |
| 86 | +| **Total** | **74** | **✅ PASS** | |
| 87 | + |
| 88 | +--- |
| 89 | + |
| 90 | +## Bounty #2276 Requirements Verification |
| 91 | + |
| 92 | +All three core bounty requirements are satisfied: |
| 93 | + |
| 94 | +| Requirement | Test | Status | |
| 95 | +|-------------|------|--------| |
| 96 | +| Replayed fingerprint must be rejected | `test_requirement_1_replay_rejected` | ✅ SATISFIED | |
| 97 | +| Fresh fingerprint must be accepted | `test_requirement_2_fresh_accepted` | ✅ SATISFIED | |
| 98 | +| Modified replay (changed nonce, old data) must be rejected | `test_requirement_3_modified_replay_rejected` | ✅ SATISFIED | |
| 99 | + |
| 100 | +--- |
| 101 | + |
| 102 | +## Integration Verification |
| 103 | + |
| 104 | +The `/attest/submit` endpoint integration is verified: |
| 105 | +- Import: `from hardware_fingerprint_replay import (...)` |
| 106 | +- Check: `check_fingerprint_replay()` called before fingerprint validation |
| 107 | +- Response: HTTP 409 with `error: "fingerprint_replay_detected"` on replay |
| 108 | +- Record: `record_fingerprint_submission()` called after successful validation |
| 109 | + |
| 110 | +--- |
| 111 | + |
| 112 | +## Attack Vectors Defended |
| 113 | + |
| 114 | +| Attack Type | Defense | Status | |
| 115 | +|-------------|---------|--------| |
| 116 | +| Fingerprint Replay | Nonce-based fingerprint binding | ✅ Blocked | |
| 117 | +| Modified Replay | Fingerprint hash from data (not nonce) | ✅ Blocked | |
| 118 | +| Entropy Profile Theft | Cross-wallet collision detection | ✅ Blocked | |
| 119 | +| Nonce Reuse | Nonce uniqueness validation | ✅ Blocked | |
| 120 | +| Submission Flooding | Rate limiting (10/hour) | ✅ Blocked | |
| 121 | +| Delayed Replay | 5-minute replay window | ✅ Expired | |
| 122 | + |
| 123 | +--- |
| 124 | + |
| 125 | +## Technical Notes |
| 126 | + |
| 127 | +### Database Path Resolution |
| 128 | + |
| 129 | +The fix ensures proper database isolation by: |
| 130 | +1. Using `get_db_path()` function that reads environment variables at call time |
| 131 | +2. Setting `DB_PATH` in test fixtures at runtime, not import time |
| 132 | +3. Using `autouse=True` fixtures to ensure fresh database for each test |
| 133 | + |
| 134 | +### Empty Fingerprint Handling |
| 135 | + |
| 136 | +The `compute_fingerprint_hash()` function now: |
| 137 | +- Returns `""` for `None` input |
| 138 | +- Returns valid SHA-256 hash for empty dict `{}` |
| 139 | +- This ensures consistent behavior across all test files |
| 140 | + |
| 141 | +--- |
| 142 | + |
| 143 | +## Conclusion |
| 144 | + |
| 145 | +All acceptance criteria met: |
| 146 | +- ✅ Combined test command passes (74 tests) |
| 147 | +- ✅ Scope limited to necessary files only |
| 148 | +- ✅ No unrelated line-ending churn |
| 149 | +- ✅ Evidence documented in this file |
0 commit comments