- Gate capture on SD_IsReady() after successful SD_Init; avoid false 'insert SD' when SD.cardType() lies after WiFi on shared SPI with LCD - SD_Init: LCD CS high, SD.end() on empty card; HandshakeCapture save path retries SD_Init before discard - FAT filename sanitize, discard deadlocks, WPA3 scan labels, docs (README, PRODUCTION, IMPROVEMENTS), firmware version 1.0.1 Made-with: Cursor
252 lines
9.8 KiB
Markdown
252 lines
9.8 KiB
Markdown
# Full-scale fix plan — handshake-capture-c6
|
||
|
||
This plan addresses the issues from the firmware review. Work in **phases** so each step compiles, flashes, and can be verified before the next.
|
||
|
||
## Implementation status (in firmware)
|
||
|
||
| Phase | Status |
|
||
|-------|--------|
|
||
| 1 String/memset | Done — `clearNetworkList()` replaces `memset(networks)` |
|
||
| 2 State machine | Done — `deauth_cycle_count` + `MAX_OBSERVE_CYCLES`; `stopCapture()` after last cycle |
|
||
| 3 EAPOL UI | Done — `latest_eapol_count[MAX_SLOTS]`, status `E a/b/c` |
|
||
| 4 Stagger deauth | Done — `DEAUTH_GAP_AFTER_CLIENT_MS` / `DEAUTH_GAP_AFTER_SLOT_MS` |
|
||
| 5 Pending save | Done — clear pending + slot only after successful SD write |
|
||
| 6 STA-only | Done — `#define USE_SOFTAP 1` default; set `0` to try STA-only |
|
||
| 7 Hidden SSID | Done — `scanNetworks(false,true)`, `<hidden>_XXXXXX` names |
|
||
| 8 Enterprise | Not done (optional) |
|
||
| 9 PMKID TLV | Not done (optional) |
|
||
| 10 Cleanup | Done — removed `getBestChannel`, `WiFiScanner_Init`; `setSleep` in init |
|
||
| 11 UI lock | Not done (optional) |
|
||
|
||
---
|
||
|
||
## Phase 0 — Preconditions (no code)
|
||
|
||
| Item | Action |
|
||
|------|--------|
|
||
| Backup | Tag or branch `before-plan-fixes` |
|
||
| Test harness | One known WPA2 AP + one client; SD inserted; serial at 115200 |
|
||
| Success criteria | After each phase: compile `esp32:esp32:esp32c6`, flash, run 2 full channel cycles without crash |
|
||
|
||
---
|
||
|
||
## Phase 1 — Critical: `String` + `memset` (must fix first)
|
||
|
||
**Problem:** `scanNetworksSortedByRSSI()` does `memset(networks, 0, sizeof(networks))` while `WifiNetwork` contains `String` — undefined behavior / heap corruption risk.
|
||
|
||
**Approach (pick one):**
|
||
|
||
### Option A — Minimal change (recommended)
|
||
- Replace `memset(networks, 0, sizeof(networks))` with an explicit clear loop:
|
||
```cpp
|
||
for (int i = 0; i < MAX_NETWORKS; i++) {
|
||
networks[i].ssid = "";
|
||
networks[i].encryption = "";
|
||
networks[i].bssid[0] = 0; // or memset only the POD tail
|
||
networks[i].ch = 0;
|
||
networks[i].rssi = 0;
|
||
networks[i].handshake_captured = false;
|
||
}
|
||
```
|
||
- Do **not** `memset` the whole struct.
|
||
|
||
### Option B — Structural (later refactor)
|
||
- Change `WifiNetwork` to fixed buffers: `char ssid[33]`, `char encryption[24]`, then `memset` or zero-init is safe.
|
||
- Touches: `HandshakeCapture.h`, `HandshakeCapture.cpp`, `WiFi_Scanner.cpp`, `handshake-capture-c6.ino` (any `.c_str()` / `String` compare).
|
||
|
||
**Files:** `HandshakeCapture.cpp` (required), optionally full Option B across project.
|
||
|
||
**Acceptance:** Scan → list → capture → rescan 10× with no heap weirdness / reboot.
|
||
|
||
---
|
||
|
||
## Phase 2 — State machine clarity (`phase_retries` / channel window)
|
||
|
||
**Problem:** `phase_retries` increments on observe→capture transition; after 3rd capture timeout the inner loop stops cycling but stays in `PHASE_CAPTURING` until `.ino` `CHANNEL_TIMEOUT_MS`. Confusing and hard to tune.
|
||
|
||
**Approach:**
|
||
1. Rename for truth: e.g. `deauth_burst_count` or `observe_cycles_completed` (document what increments where).
|
||
2. **Either:**
|
||
- **2a.** After the last allowed cycle (no more re-observe), explicitly call `stopCapture()` from `handshakeCaptureLoop()` so channel session ends on inner logic (and align `CHANNEL_TIMEOUT_MS` as a hard ceiling only), **or**
|
||
- **2b.** Keep current “linger in CAPTURING” but document in code + README that outer timeout is intentional listen-only tail.
|
||
|
||
**Files:** `HandshakeCapture.cpp`, `handshake-capture-c6.ino` (timeout values if 2a).
|
||
|
||
**Acceptance:** Serial log shows exactly N observe→deauth→capture cycles, then predictable stop or handoff to scan.
|
||
|
||
---
|
||
|
||
## Phase 3 — `latest_eapol_count` / UI honesty
|
||
|
||
**Problem:** One global counter; UI “EAPOL N” is ambiguous with 3 slots.
|
||
|
||
**Approach:**
|
||
1. Replace `volatile int latest_eapol_count` with `volatile uint8_t latest_eapol_count[MAX_SLOTS]` (or keep max across slots for a single “best” display).
|
||
2. In callback, set `latest_eapol_count[i] = slots[i].eapol_count` when slot `i` gets EAPOL.
|
||
3. In `updateCaptureVisuals()` / status line: show e.g. `EAPOL 1/1/2` or `max=2` + `ch X` — pick one rule and stick to it.
|
||
|
||
**Files:** `HandshakeCapture.h`, `HandshakeCapture.cpp`, `handshake-capture-c6.ino`.
|
||
|
||
**Acceptance:** With 2 active targets, status reflects both or clearly states “max”.
|
||
|
||
---
|
||
|
||
## Phase 4 — Staggered deauth (capture yield)
|
||
|
||
**Problem:** Back-to-back TX blinds RX around reconnect.
|
||
|
||
**Approach:**
|
||
1. In `sendDeauthBurstAll()` / `sendDeauthToClient()`:
|
||
- After each **client’s** burst (or each N frames), `delay(50–100)` or `vTaskDelay` ms **or** yield-only if you prefer minimal sleep.
|
||
- Round-robin: 1 frame per client × 5 rounds with 20 ms between rounds (tune).
|
||
2. Add `#define` constants at top of `HandshakeCapture.cpp` for easy tuning.
|
||
|
||
**Files:** `HandshakeCapture.cpp`.
|
||
|
||
**Acceptance:** Same AP/client test; subjective + optional Wireshark on second radio showing less contiguous TX.
|
||
|
||
---
|
||
|
||
## Phase 5 — `pending_save_slots` hardening
|
||
|
||
**Problem:** Rare race: callback sets pending again while main clears and writes SD.
|
||
|
||
**Approach:**
|
||
1. Don’t set `pending_save_slots[i] = -1` until **after** successful `file.close()` **or** use a two-phase flag: `save_requested[i]` vs `save_in_progress[i]`.
|
||
2. Simpler variant: only clear pending after write succeeds; on failure leave slot active and re-queue.
|
||
|
||
**Files:** `HandshakeCapture.cpp` (`handshakeCaptureProcessPending`).
|
||
|
||
**Acceptance:** Stress test: force slow SD (if possible) or inject double-EAPOL path — no lost save flag.
|
||
|
||
---
|
||
|
||
## Phase 6 — WiFi mode: STA-only trial
|
||
|
||
**Problem:** `WIFI_AP_STA` with unused SoftAP wastes airtime/power.
|
||
|
||
**Approach:**
|
||
1. Add `#define USE_SOFTAP 0` (default 0).
|
||
2. In `handshakeCaptureInit()`, `WiFi.mode(USE_SOFTAP ? WIFI_AP_STA : WIFI_STA)`.
|
||
3. **Test matrix:** promiscuous on, `esp_wifi_80211_tx` deauth still accepted on your core. If TX fails, document and keep `WIFI_AP_STA`.
|
||
|
||
**Files:** `HandshakeCapture.cpp`, short note in `README.md`.
|
||
|
||
**Acceptance:** Capture + deauth still work; no regression on C6.
|
||
|
||
---
|
||
|
||
## Phase 7 — Hidden SSIDs
|
||
|
||
**Problem:** Empty SSID skipped in scan.
|
||
|
||
**Approach:**
|
||
1. `WiFi.scanNetworks(false, true)` — include hidden where supported.
|
||
2. If `ssid.isEmpty()`, set `networks[i].ssid = "<hidden>"` (or `HIDDEN_xx` + last BSSID octets for uniqueness in filenames).
|
||
3. Filename sanitizer: strip/replace `/` and odd chars for FAT.
|
||
|
||
**Files:** `HandshakeCapture.cpp`, `handshakeCaptureProcessPending` (filename).
|
||
|
||
**Acceptance:** Hidden WPA2 AP appears in list and can be captured if BSSID/channel match.
|
||
|
||
---
|
||
|
||
## Phase 8 — Optional coverage: WPA2 Enterprise / WPA3
|
||
|
||
**Problem:** `isCaptureable()` excludes enterprise; WPA3 is different handshake.
|
||
|
||
**Approach:**
|
||
1. **Enterprise:** Add `enc == "WPA2 Enterprise"` to `isCaptureable()` if goal is EAP capture (PCAP still useful; cracking differs).
|
||
2. **WPA3:** Separate project slice — detect SAE, different EAPOL handling; only if you need it.
|
||
|
||
**Files:** `HandshakeCapture.cpp` (minimal: enterprise only first).
|
||
|
||
**Acceptance:** Enterprise networks show as targets; no crash (capture may still be hard depending on network).
|
||
|
||
---
|
||
|
||
## Phase 9 — PMKID parser robustness (optional)
|
||
|
||
**Problem:** Fixed WPA2 RSN offsets; odd drivers may differ.
|
||
|
||
**Approach:**
|
||
1. Walk Key Data as TLV (length-checked) instead of fixed offset where possible.
|
||
2. Keep existing fast path; add fallback scan for `00 0f ac 04` PMKID KDE.
|
||
|
||
**Files:** `HandshakeCapture.cpp` (`eapolHasPmkid`).
|
||
|
||
**Acceptance:** Still detects PMKID on standard APs; no regressions on bounds.
|
||
|
||
---
|
||
|
||
## Phase 10 — Cleanup + init
|
||
|
||
**Problem:** Dead `getBestChannel()` vs `getNextChannelSweep()`, unused `WiFiScanner_Init()`.
|
||
|
||
**Approach:**
|
||
1. Remove `getBestChannel` from header if unused, or use it inside sweep for “first channel” bias.
|
||
2. Call `WiFiScanner_Init()` from `setup()` **or** delete and merge `WiFi.setSleep(false)` into `handshakeCaptureInit()`.
|
||
|
||
**Files:** `HandshakeCapture.h`, `HandshakeCapture.cpp`, `WiFi_Scanner.cpp`, `handshake-capture-c6.ino`.
|
||
|
||
---
|
||
|
||
## Phase 11 — UI concurrency (optional polish)
|
||
|
||
**Problem:** `updateCaptureVisuals()` reads slots without lock.
|
||
|
||
**Approach:**
|
||
1. Snapshot `slots[i].active`, `network_index`, `eapol_count`, `client_count` under `portENTER_CRITICAL` into a small struct array, then paint LVGL outside lock.
|
||
2. Or accept rare flicker and document as cosmetic-only.
|
||
|
||
**Files:** `handshake-capture-c6.ino` (or move snapshot helper to `HandshakeCapture.cpp`).
|
||
|
||
---
|
||
|
||
## Recommended order (dependency graph)
|
||
|
||
```
|
||
Phase 1 (String/memset) ──┬──► Phase 2 (state machine)
|
||
├──► Phase 3 (EAPOL UI)
|
||
├──► Phase 4 (deauth stagger)
|
||
├──► Phase 5 (pending save)
|
||
├──► Phase 6 (STA-only test)
|
||
└──► Phase 7 (hidden SSID)
|
||
|
||
Phase 8–11 independent after Phase 1, in any order you care about.
|
||
```
|
||
|
||
---
|
||
|
||
## Effort summary
|
||
|
||
| Phase | Effort | Risk |
|
||
|-------|--------|------|
|
||
| 1 | Small | Low |
|
||
| 2 | Small–medium | Medium (behavior change if 2a) |
|
||
| 3 | Small | Low |
|
||
| 4 | Small | Low |
|
||
| 5 | Small | Low |
|
||
| 6 | Tiny | Medium (HW/core dependent) |
|
||
| 7 | Small | Low |
|
||
| 8 | Small | Medium (scope creep) |
|
||
| 9 | Medium | Medium |
|
||
| 10 | Tiny | Low |
|
||
| 11 | Small | Low |
|
||
|
||
---
|
||
|
||
## Single “definition of done” for the whole plan
|
||
|
||
- [ ] No `memset` over structs containing `String`.
|
||
- [ ] Cycle count / stop behavior documented and matches serial logs.
|
||
- [ ] UI EAPOL display unambiguous for multi-slot.
|
||
- [ ] Deauth stagger tuned and `#define`’d.
|
||
- [ ] Pending-save race handled or explicitly accepted.
|
||
- [ ] STA-only tested; default documented.
|
||
- [ ] Hidden SSIDs handled or explicitly out of scope.
|
||
- [ ] Dead code removed or wired.
|
||
- [ ] Clean build + flash on ESP32-C6.
|
||
|
||
When you’re ready, say **“implement phase N”** and we’ll execute that slice only.
|