Skip to content

Add a WPA2/CCMP station-mode client (APFPV)#335

Open
GingerFluffyCat wants to merge 7 commits into
OpenIPC:masterfrom
GingerFluffyCat:apfpv-station-mode
Open

Add a WPA2/CCMP station-mode client (APFPV)#335
GingerFluffyCat wants to merge 7 commits into
OpenIPC:masterfrom
GingerFluffyCat:apfpv-station-mode

Conversation

@GingerFluffyCat

Copy link
Copy Markdown
Contributor

Summary

devourer currently has no way to be a WPA2 client — it injects/receives in
monitor mode, and the AP-mode test harnesses under tests/ play the AP role
against a real station. This adds the missing piece: a full station-mode
client that scans, authenticates, associates, runs the WPA2 4-way handshake,
gets an IP over DHCP, and streams data — usable as a real Wi-Fi uplink for an
RTL8812AU-family dongle, not just a monitor/injection tool.

New library surface (mirrors the existing devourer/chip-family target
pattern): two new CMake targets, apfpv (driver-independent: WPA2
supplicant, 802.11 frame builders, CCMP crypto, beacon/scan parsing) and
apfpv_station (the driver-dependent orchestrator + RX pipeline).

What's included

  • src/apfpv/Wpa2Supplicant (4-way handshake, client side),
    Dot11Frames (auth/assoc/probe request builders), Wpa2Crypto +
    crypto/{AesCcm,AesCmac,Sha1Hmac} (own CCMP/PBKDF2/PRF implementation, no
    external crypto dependency), ScanProbe (beacon parsing for AP
    discovery), StationMode (the arming sequence: MACID → MSR=STATION →
    BSSID → RCR filter, ported from the kernel's hw_var_set_opmode STATION
    path), StationTxDesc (the unicast TX descriptor — BMC=0 enables the
    ACK handshake, the fix that made auth/assoc frames actually get ACKed).
  • ApfpvStation — the top-level orchestrator: connect() runs the
    gated scan→arm→auth→assoc→4-way→DHCP→stream chain; a driver-level
    supervisor thread auto-reconnects on link loss (deauth or an RX/beacon
    timeout), re-arming on the known channel rather than a full rescan.
    Also exposes a general-IP bridge (arbitrary IPv4 over the CCMP link, not
    just an RTP/video assumption) and a minimal in-order TCP stack over it.
  • RxDeframe — the station RX pipeline: CCMP decrypt, A-MPDU RX
    reorder (kernel recv_indicatepkt_reorder equivalent — required once the
    AP aggregates; without it, out-of-order subframes are dropped), BlockAck
    request handling, and RTP-sequence reordering for the video use case this
    was built for.
  • ApfpvDhcp / LqFeedback — a DHCP client and a link-quality
    telemetry feed.
  • Driver hooks needed for the above (RtlAdapter): fillH2CCmd/
    sendH2CPacket (firmware mailbox), setSecCamKey/enableHwSec (security
    CAM programming for the PTK/GTK), sendStationFrameSync (concurrent
    IN/OUT unicast TX with an ACK-drain check, validated against the
    worst-case USB/IP transport), reset_device.
  • Two hardware-validated fixes (RadioManagementModule/
    PhydmWatchdog): SetStationRxFilter — the kernel-matched RCR/RXFLTMAP
    configuration station mode needs (the prior permissive filter dropped
    management frames the auth/assoc handshake needs); and a close-range
    DIG-saturation fix — once linked, track the receiver's gain floor/ceiling
    off the live beacon RSSI instead of the always-monitor-mode bounds, or a
    strong/close signal storms the false-alarm counter and the front-end goes
    deaf (no RX at all) even though the link should be trivially strong.

Caveats / what's not claimed

  • Single client only (no multi-station bookkeeping beyond what's needed for
    the one active connection).
  • ApfpvStation.cpp also contains AP-mode (SoftAP) scaffolding —
    beaconing, RSSI tracking, and a WPA2 4-way authenticator (the AP side) —
    wired onto this repo's own StartBeacon/StopBeacon. It's explicitly
    marked work-in-progress in its own comments: association completion and
    the data plane (ARP/ICMP/DHCP responding to an associated client) aren't
    there yet, unlike this repo's own tests/ap_wpa2.cpp/ap_responder.cpp
    harnesses, which do have a working data plane. Happy to split it out of
    this PR if you'd rather review the station-mode work on its own — say the
    word and I'll drop those commits/functions.
  • Bench-validated on RTL8812AU (Jaguar1) against a real OpenIPC AP; not
    tested against Jaguar2/3/Kestrel chips.

Testing

Built cleanly as a standalone CMake target (apfpv_station) against this
branch; also integrated and building end-to-end in a downstream Android app
(PixelPilot) alongside the existing WFB-ng monitor-mode path.

Confirmed by research: Wpa2Supplicant/Dot11Frames/Wpa2Crypto/crypto/* have
zero coupling to the driver layer (pure byte-buffer logic behind a SendFn
callback) -- ported byte-for-byte with zero code changes, only new build
wiring (apfpv CMake target) + the already-proven android/log.h host-build
compat shim reused from WiFiDriver.

ApfpvDhcp, LqFeedback, Wpa2Authenticator: same story, built clean.

ApfpvStation.cpp/h and RxDeframe.cpp/h are copied onto the tree but NOT yet
wired into the build -- these need real porting work against upstream's
restructured driver API (IRtlDevice/RtlJaguarDevice, new H2C/station
convenience wrappers that don't exist upstream yet) before they'll compile.
Next commit.
…nto RtlAdapter

Zero logic changes -- these only ever used rtw_read<T>/rtw_write<T> (same
template signatures upstream) and hardcoded/named H2C mailbox register
offsets (REG_HMETFR/REG_HMEBOX_0..3/REG_HMEBOX_EXT_0..3, confirmed present
in hal/hal_com_reg.h). reset_device() needed one small new addition:
upstream's transport layer had no USB-reset concept at all (no station
client flow existed there to need reconnect recovery) -- added a
virtual int reset() to IRtlTransport (default no-op, e.g. for a future PCIe
transport) with a real libusb_reset_device() override in UsbTransport.

Verified: devourer library builds clean (Jaguar1-only config).
Zero logic changes -- confirmed hw_var_rcr_config() (the actual register
write) and every RCR_*/REG_RXFLTMAP* constant this function needs already
exist upstream, used the same way in its own _InitWMACSetting_8812A(). This
is the kernel-exact station RCR fix: RCR_AAP (promiscuous) bypasses the HW
auto-Block-Ack engine for A-MPDU aggregates entirely (ADDBA/DELBA/single-
frame-fallback ~25Mbps ceiling); APM+CBSSID_DATA routes through the real
station RX path so the chip auto-BAs aggregates instead.

Verified: devourer library builds clean.
…not yet wired)

Mechanical port of the accessors + backing state (_lastRfCh, _scanMode) from
the pre-rebuild WiFiDriver. Honest caveat, flagged in the code: the actual
BEHAVIOR (reading RF 0x18 back into _lastRfCh after a tune, skipping IQK
when _scanMode is set) lives inside set_channel_bwmode and is NOT yet
wired up -- these always report "not wedged"/have no IQK-suppression
effect until that integration lands. Tracked as follow-up work.

Verified: builds clean.
- RF-verify-read (BB 0x834 + RF 0x18 readback -> _lastRfCh) added to
  PHY_HandleSwChnlAndSetBW8812, Android-gated, same placement/behavior as
  the pre-rebuild WiFiDriver -- confirmed phy_query_bb_reg/phy_query_rf_reg
  exist upstream with matching signatures (this code path is skipped on the
  host/non-Android build, so verified by symbol lookup rather than compile
  here; will compile-check for real on the next Android/Gradle build).
- _scanMode now actually suppresses the IQK trigger (fast unsettled retunes
  across a scan sweep wedge the RF synth if IQK runs mid-sweep).

rf_wedged()/setScanMode() are no longer inert -- both declared AND wired.

Verified: devourer library builds clean.
Kept the proven default behavior exactly (concurrent IN/OUT, ACK-driven
TXPKT_EMPTY drain check, opt-in clear_halt for the WSL/vhci wedge case,
_txFastPath skipping the drain diagnostic once streaming). Dropped the
legacy DEVOURER_TX_USE_PAUSE fallback: it paused/resumed a per-adapter
async-RX worker that doesn't exist in this form anymore (RX-loop ownership
moved to whoever calls bulk_read_async_loop, i.e. RtlJaguarDevice::
StartRxLoop/StopRxLoop) -- that branch was already documented as legacy/
non-default in the pre-rebuild code, so it's dropped rather than force-fit
onto a different architecture, not silently lost functionality.

Adapted bulk_out_eps[0]/libusb_clear_halt(_dev_handle,...) to this class's
own first_bulk_out_ep()/bulk_clear_halt() equivalents.

Verified: devourer library builds clean.
…tream base

- PhydmWatchdog: restore the close-range DIG-saturation fix (RSSI-tracked
  IGI floor/ceiling once linked) that upstream's monitor-only DigTick had
  no equivalent for.
- StationTxDesc: port the unicast TX descriptor (BMC=0 ACK-handshake fix).
- ApfpvStation: replace every StartMonitorAsyncRx/StopAsyncRx call site
  with upstream's blocking StartRxLoop/StopRxLoop run on a dedicated
  thread (RtlJaguarDevice::Init/StartRxLoop no longer return until
  stopped, unlike the old async URB-pool contract).
- AP mode: wire onto upstream's IRtlDevice::StartBeacon/StopBeacon (real
  HW TBTT beacon + ACK-engine arming) instead of the old manual register
  pokes + software beacon-poll loop; fix apSend() calling send_packet
  synchronously from the RX callback thread (returns BUSY per
  docs/ap-mode.md) by queuing to a separate TX-pump thread.
- RadioManagementModule: port prime_offset_40mhz (40 MHz primary-channel
  offset) and add no-op pauseAsyncRx/resumeAsyncRx to RtlAdapter (RX
  ownership moved out of the adapter, so there's no in-adapter pump left
  to pause).

apfpv_station now builds clean against upstream's rebuilt base.

@josephnef josephnef left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Request changes — WPA2/CCMP station mode (APFPV)

Substantial, genuinely useful work, and the crypto core is mostly correct — but this can't merge as-is. CI is red (see below), there are two KRACK-class handshake defects, and the PR broadly violates this repo's standing conventions (the library reads no environment; two-plane logging; generation-agnostic core). Inline comments mark the specific spots; the must-fix summary is below.

CI is red 🔴

build (windows-latest, Release, cl) fails — the rest of the matrix is green. Root cause is a real MSVC portability break in ApfpvStation.cpp (min/max macro collision), flagged inline. It must build on MSVC — Windows is first-class here.

Blockers

  1. EAPOL-Key MIC is never verified — M3, group-rekey, and PTK-rekey all act on unauthenticated frames (Wpa2Supplicant.cpp). This is the authentication step of the 4-way; skipping it is a KRACK-class defect.
  2. EAPOL replay gate uses < not <= — an equal-counter replayed group-rekey passes and resets the GTK PN window (KRACK group-key reinstallation).
  3. MSVC build broken (CI red) — std::min/std::max vs the <windows.h> min/max macros.
  4. Android NDK build brokenRadioManagementModule.cpp uses __android_log_print / __system_property_get / PROP_VALUE_MAX under #if __ANDROID__ without including <android/log.h> / <sys/system_properties.h> (host builds preprocess the block away, hiding it — ironically the PR's own target platform).

Major

  1. The library reads the environment pervasively — 40+ getenv/Android-prop sites in ApfpvStation.cpp, plus RxDeframe.cpp, AesCcm.cpp, StationMode.cpp, RadioManagementModule.cpp. CLAUDE.md: "The library reads no environment." These belong in DeviceConfig (env mapped in examples/common/env_config.cpp); the one-shot *_OLD / FORCE_SW_AES A/B knobs should just be deleted now the fixes are validated.
  2. Direct logging bypasses the two-plane contractfprintf(stderr,...) and __android_log_print(...) throughout library sources, including one that prints GTK bytes to stderr. Route through the repo Logger.
  3. Layering — generation-agnostic core (RxDeframe, ApfpvStation) reaches into jaguar1/ and hardcodes 8812AU registers; nothing in the API says "8812 only." apfpv_station isn't gated on DEVOURER_JAGUAR1 (builds as a static archive today so CI doesn't catch it, but any consumer linking it with JAGUAR1=OFF gets undefined refs).
  4. Negotiated-cipher assoc-request truncates the HT/VHT/ExtCap tail (Dot11Frames.cpp) — latent (only the non-default cipher path), but a real frame-corruption bug.
  5. No tests — hand-rolled AES/CCM/PBKDF2/PRF/RFC-3394 with zero KATs, in a repo with a strong ctest culture. Highest-leverage thing to add; the code comments themselves record several already-fixed crypto bugs.
  6. WIP AP-mode/authenticator scaffolding ships in the same PR (admittedly untested). Please take up your own offer to split it out — it's a second unverified 4-way implementation and enlarges the review/security surface.

Not regressed (good)

The jaguar1 monitor-mode path is correctly fenced off: the DIG fix and SetStationRxFilter gate on station-only state, so the primary use case is safe.

Minor items (fixed 2048-byte CCM scratch caps decrypt; non-constant-time MIC memcmp; no key zeroization; #if 0 dead code; dated archaeology comments; chooseCipher can pick unsupported suites) can ride the same pass.

Happy to re-review once the blockers + CI are addressed.

Comment thread src/ApfpvStation.cpp
try {
rtl->StopRxLoop(); safeJoinThread(_rxThread); // no RX needed for a TX-only beacon
rtl->InitWrite(legalApfpvChannel(channel, 20)); // tune + bring up (no RX loop)
rtl->SetTxPowerIndexOverride(std::max(0, std::min(63, txIndex)));

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Blocker — this is the red CI (MSVC). build (windows-latest, Release, cl) fails here:

ApfpvStation.cpp(139,43): error C2589: '(': illegal token on right side of '::'
ApfpvStation.cpp(1675,32): error C2589: ...

bcrypt/ws2_32 pull in <windows.h>, which defines function-like min(/max( macros, so std::min(63, txIndex) (and line 1675) won't compile. Fix: #define NOMINMAX before any Windows header (or wrap as (std::min)(...)). <algorithm> is already included — the macro is the problem.

}
return false;
case State::WaitMsg3:
if (hasMic && isPairwise) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Blocker (KRACK-class) — the M3 MIC is never verified. This case installs the GTK and goes Done without ever computing _c.eapol_mic(_kck, ...) over the frame (MIC field zeroed) and comparing it to the received MIC. Same omission on the group-rekey and PTK-rekey paths below. MIC verification is the step that proves the AP holds the PMK — without it the STA cannot reject a forged/rogue M3. Verify the MIC before deriving/installing keys or sending M4; reject on mismatch. (GTK unwrap's RFC-3394 check and TK-decrypt failure are not substitutes.)

// the handshake is up — rejects replayed EAPOL-Key frames (KRACK-class). Only enforced past
// Done so a reconnect's fresh (possibly lower) M1 counter is never rejected; begin() resets.
uint64_t rc = 0; for (int i = 0; i < 8; ++i) rc = (rc << 8) | body[9 + i];
if (_state == State::Done && rc < _replay) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Blocker (KRACK-class) — off-by-one in the replay gate. rc < _replay accepts an equal counter. A replayed post-Done group-rekey carries rc == _replay, passes here, and installGtkFromKeyData() reinstalls the GTK and resets its RX PN window to 0 (line ~170) — group-key reinstallation. Combined with the missing MIC check above, an attacker can replay a captured rekey frame to reset the broadcast replay window. Post-handshake rekeys must require a strictly greater counter and a valid MIC.

_gtkKeyId = newId;
std::memcpy(_gtk.data(), &kp[i+8], 16);
_rxPnGtk[newId & 3] = 0; winClear(_rxPnGtkWin[newId & 3]); // fresh key: reset RX replay window
fprintf(stderr, "[wpa] GTK installed keyid=%d (%02x%02x..)\n", _gtkKeyId, _gtk[0], _gtk[1]);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Key material must never hit logs — this prints the first two GTK bytes to stderr. Drop it (and route any diagnostics through the repo Logger, not fprintf).

if(ctlen<8) return false;
// A/B knob: DEVOURER_FORCE_SW_AES=1 forces the software-AES fallback (the pre-fix path)
// so the AES-NI win can be measured back-to-back on the same link.
static const bool kForceSw = std::getenv("DEVOURER_FORCE_SW_AES") != nullptr;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

The library must not read the environment (DEVOURER_FORCE_SW_AES). This is a bench A/B knob — either delete it now the AES-NI path is validated, or lift it to DeviceConfig. Same rule applies to the getenv sites in RxDeframe.cpp, StationMode.cpp, RadioManagementModule.cpp, and the 40+ in ApfpvStation.cpp.

while(i<ptlen){ A[14]=ctr>>8;A[15]=ctr&0xff; uint8_t Sx[16]; aes_enc(a,A,Sx);
size_t k=ptlen-i<16?ptlen-i:16; for(size_t j=0;j<k;j++) out[i+j]=ct[i+j]^Sx[j]; i+=k; ctr++; }
uint8_t mic[8],calc[16]; (void)calc;
uint8_t tmp[2048]; if(ptlen>sizeof(tmp)) return false;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

The fixed tmp[2048] scratch silently fails decrypt for any plaintext > 2048 bytes (large/A-MSDU MPDUs). The scratch is also pointless — it holds a re-encryption nobody reads; compute the CBC-MAC without emitting output. (Also: the MIC compare at line 109/125 is memcmp, not constant-time — minor, but easy to fix here.)

Comment thread src/RxDeframe.cpp
// Feed live RX RSSI to DIG so it uses phydm *connected-mode* boundaries
// (IGI floor tracks RSSI ~0x37 @ -45 dBm) instead of monitor coverage
// bounds (capped 0x2a → over-gained → FA storm → AP rate-caps us).
PhydmWatchdog::SetLinkRssi(toDbm(pkt.RxAtrib.rssi[0]));

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Layering: this is generation-agnostic src/ core reaching into jaguar1/PhydmWatchdog (include at line 7). Together with the 8812AU-hardcoded registers in ApfpvStation/StationMode, this station code is effectively jaguar1-only despite living in the neutral tree — scope it jaguar1-side and gate the build on DEVOURER_JAGUAR1.

// eats 8 bytes of real payload off every frame -> corrupt NALUs -> black
// screen). Default off (the proven SW-CCMP path). DEVOURER_RCR_APP or
// DEVOURER_HW_DECRYPT opts in.
bool rcrApp = std::getenv("DEVOURER_RCR_APP") != nullptr

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Two problems here: (1) library reads env (DEVOURER_RCR_APP/DEVOURER_HW_DECRYPT/…) — move to DeviceConfig; (2) NDK build blocker — the __ANDROID__ branch just below calls __system_property_get/PROP_VALUE_MAX (and __android_log_print at line 616) but this file includes neither <sys/system_properties.h> nor <android/log.h>. Host builds preprocess the block away, so it compiles here but breaks on the actual Android target.

Comment thread src/apfpv/Dot11Frames.cpp
// it is identical, so only rebuild when different.
if (pairwiseCipher == 0x000FAC04 && groupCipher == 0x000FAC04) return f;
// remove last RSN IE (0x30 ... ) — find last 0x30 tag
for (size_t i = f.size(); i-- > 0; ) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

The negotiated-RSN overload strips the RSN IE by scanning backwards for byte 0x30 and f.resize(i) — but the base builder appends RSN before the WMM/HT/VHT/ExtCap tail, so this truncates the entire tail (the AP then sees an assoc-req with no HT/VHT caps — the exact 'never sends M1' failure your own comment describes). It also mis-triggers on an rsnCaps low byte of 0x30. Track the RSN offset when building and splice in place, or walk IEs properly.

Comment thread CMakeLists.txt
# ApfpvStation/RxDeframe DO need the driver layer (RtlAdapter/RtlJaguarDevice/
# RadioManagementModule) -- separate target so `apfpv` itself stays
# driver-independent (it's also used as-is on hosts with no libusb at all).
add_library(apfpv_station

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

apfpv_station links RadioManagementModule/PhydmWatchdog/RtlJaguarDevice (jaguar1) but is built unconditionally. It's a static archive so CI's JAGUAR1=OFF cells don't fail today, but any executable linking it with jaguar1 off gets undefined refs. Gate this target on DEVOURER_JAGUAR1 like DEVOURER_8814 does.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants