fix(firmware): OTA upload fails closed when no PSK in NVS (RuView#596 audit)#623
Merged
Conversation
… audit)
ota_check_auth() previously returned true when s_ota_psk[0] == '\0'
("permissive for dev"). A freshly-flashed node — or any node where
nobody had provisioned an OTA PSK yet — accepted attacker-controlled
firmware over plain HTTP on port 8032 from any host on the WiFi. No
Secure Boot V2, no signed-image verification, no transport encryption.
Single LAN call could brick or backdoor a node.
This was flagged in the deep security review of PR #596 but was a
PRE-EXISTING bug in main, not new code from that PR — so it stood as
a critical-severity production issue until this commit.
Fix:
- ota_check_auth() now returns false when no PSK is provisioned, with
ESP_LOGW("OTA rejected: no PSK in NVS …") at the call site so the
operator can diagnose the rejection from serial logs
- ota_update_init() ESP_LOGW message updated to surface the new posture
at boot ("upload endpoint will REJECT all requests until provisioned")
- Doc comment on ota_check_auth() rewritten to make the contract
explicit and reference the audit
The OTA HTTP server itself still starts even when no PSK is set. That
lets the operator run `provision.py --ota-psk <hex>` over USB-CDC to
write the NVS key without reflashing the firmware. The upload endpoint
just refuses every request in the meantime.
Breaking change for any deployment that depended on the unauthenticated
OTA path working out of the box. Documented in CHANGELOG under
[Unreleased] / Security so it's visible at the next release cut.
Fix-marker RuView#596-ota-fail-closed (scripts/fix-markers.json)
requires the new behaviour and forbids the old "permissive for dev"
fallback strings, so a future revert fails CI.
Co-Authored-By: claude-flow <ruv@ruv.net>
This was referenced May 18, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Pre-existing critical-severity bug, not from PR #596
firmware/esp32-csi-node/main/ota_update.c:44-49(current main) had:So a freshly-flashed node — or any node where nobody had provisioned an OTA PSK yet — accepted attacker-controlled firmware over plain HTTP on port 8032 from any host on the WiFi. No Secure Boot V2, no signed-image verification, no transport encryption. Single LAN call could brick or backdoor a node.
I flagged this in the deep security review of PR #596 but it was pre-existing code in
main, not introduced by #596 — so it stayed live as a critical production issue until this commit.⚠ Breaking change
After this PR, any deployment that relied on the unauthenticated OTA path working out of the box will need to provision a PSK before subsequent OTA pushes succeed. The OTA HTTP server itself still starts when no PSK is set, so operators can:
to write the NVS key over USB-CDC without re-flashing. Only the
POST /otaupload endpoint refuses requests until that's done.Boot-time
ESP_LOGWmakes the new posture visible on serial:CHANGELOG entry under
[Unreleased]/Securityis loud about the breaking-change part.Fix shape
static bool ota_check_auth(httpd_req_t *req) { if (s_ota_psk[0] == '\0') { - /* No PSK provisioned — auth disabled (permissive for dev). */ - return true; + /* No PSK provisioned — fail closed (RuView#596 audit). */ + ESP_LOGW(TAG, "OTA rejected: no PSK in NVS (run provision.py --ota-psk <hex>)"); + return false; } ... }Test plan
cargo check -p wifi-densepose-sensing-server --no-default-features4.4s — unrelated, just sanity)python scripts/check_fix_markers.py— 20/20 markers pass including newRuView#596-ota-fail-closedwhich forbids the oldauth disabled (permissive for dev)stringcurl -X POST http://<node>:8032/ota -H 'Authorization: Bearer wrong' --data-binary @dummy.binreturns 403provision.py --ota-psk <hex> --force-partial, then same curl with correct PSK returns 200Regression guard
Fix-marker
RuView#596-ota-fail-closedrequiresfail-closed, see RuView#596 auditandOTA rejected: no PSK in NVSsubstrings inota_update.c, and forbids the oldauth disabled (permissive for dev)andNo PSK provisioned — auth disabledstrings. Any revert would fail the fix-marker CI workflow.🤖 Generated with claude-flow