diff --git a/scripts/webconfig_mock_server.py b/scripts/webconfig_mock_server.py index f286ab72..06623278 100644 --- a/scripts/webconfig_mock_server.py +++ b/scripts/webconfig_mock_server.py @@ -108,6 +108,8 @@ class State: self.setup_mode = args.setup self.active_slots = args.active_slots self.cfg = default_config(args.setup) + # latched at AP start, like WebConfigServer::_initial_setup + self.initial_setup = args.setup and self.cfg["wifi"]["ssid"] == "" self.start = time.time() self.session = None # cookie token when logged in (LAN mode) self.batch = {"state": "idle"} @@ -181,6 +183,14 @@ def apply_set(cfg, key, val): if key in LEN_LIMITS and len(val) > LEN_LIMITS[key]: return False, "Error: %s too long (max %d chars)" % (key, LEN_LIMITS[key]) + if key == "password": + # Stored outside cfg: it must never appear in the /api/config GET. The + # firmware overwrites the CLI's "password now: " echo, so the + # reply carries no secret either. + global ADMIN_PASSWORD + ADMIN_PASSWORD = val + return True, "OK" + if key == "radio": try: f, bw, sf, cr = val.split(",") @@ -393,6 +403,16 @@ class Handler(BaseHTTPRequestHandler): reboot = bool(body.get("reboot", False)) setmap = body.get("set", {}) or {} + # `password` maps to the top-level CLI command rather than a setter. It + # is accepted in both modes (LAN already required a login), but first + # onboarding cannot finish without it. + if "password" in setmap: + pwd = str(setmap["password"]) + if not 0 < len(pwd) <= 15 or "\r" in pwd or "\n" in pwd: + return self._json(400, {"error": "admin password must be 1-15 characters with no line breaks"}) + elif ST.setup_mode and ST.initial_setup and (reboot or "wifi.ssid" in setmap): + return self._json(400, {"error": "admin password required for initial setup"}) + with ST.lock: if ST.batch.get("state") != "idle" and ST.batch.get("reqid") == reqid: return self._json(202, { diff --git a/src/helpers/WebConfigKeys.h b/src/helpers/WebConfigKeys.h index 7579cc12..fbfedfba 100644 --- a/src/helpers/WebConfigKeys.h +++ b/src/helpers/WebConfigKeys.h @@ -14,7 +14,8 @@ // Keys mapping to CLI `set ` handlers. Everything not listed here // is rejected, so a crafted request can't reach arbitrary commands (`erase`, -// `password`, ...) through the batch. +// etc.) through the batch. The portal's admin-password field is classified +// separately, see wcIsAdminPasswordKey below. static const char* const WC_ALLOWED_SET_KEYS[] = { // NodePrefs (radio / node) "name", "lat", "lon", "radio", "tx", "af", "rxdelay", "txdelay", @@ -53,6 +54,24 @@ static inline bool wcIsAllowedSetKey(const char* key) { return false; } +// The admin password maps to the top-level `password` command, not a setter, so +// it is classified apart from the `set` allowlist. It is the only key that gets +// this treatment, which is what keeps the allowlist the sole route to `set` and +// leaves no general path from a batch to arbitrary top-level CLI commands. +static inline bool wcIsAdminPasswordKey(const char* key) { + return strcmp(key, "password") == 0; +} + +static inline bool wcIsValidAdminPassword(const char* value) { + if (value == NULL) return false; + const size_t len = strlen(value); + if (len == 0 || len > 15) return false; // NodePrefs::password[16], including NUL + for (size_t i = 0; i < len; i++) { + if (value[i] == '\r' || value[i] == '\n') return false; // reject, never silently strip + } + return true; +} + // Keys carrying a secret whose stored value is masked with the placeholder in // the UI; a POST echoing the placeholder for one of these is dropped (unchanged). static inline bool wcIsSecretKey(const char* key) { diff --git a/src/helpers/esp32/WebConfigServer.cpp b/src/helpers/esp32/WebConfigServer.cpp index 8093c969..0fb64c13 100644 --- a/src/helpers/esp32/WebConfigServer.cpp +++ b/src/helpers/esp32/WebConfigServer.cpp @@ -127,6 +127,7 @@ bool WebConfigServer::startSetupMode(char reply[]) { _dns->start(53, "*", ip); // captive portal: every name resolves to us _mode = MODE_SETUP; + _initial_setup = (_obs->wifi_ssid[0] == 0); createServer(); _was_setup_ap = true; _last_activity = millis(); @@ -229,6 +230,7 @@ void WebConfigServer::finalizeTeardown() { } _was_setup_ap = false; } + _initial_setup = false; _stopping = false; _stop_warn_at = 0; _stop_warned = false; @@ -314,6 +316,10 @@ void WebConfigServer::drainBatch(uint32_t now) { WCLock lock(_mux); _cb->execCommand(e.cmd, e.reply); if (e.reply[0] == 0) strcpy(e.reply, "OK"); + // The upstream `password` command echoes the new password back in its + // reply, and replies are served to the client over the open setup AP. + // Overwrite it: the command cannot fail, so there is nothing to report. + if (wcIsAdminPasswordKey(e.key)) strcpy(e.reply, "OK"); // Success convention across every allowlisted setter is an "OK" prefix // (the UI relies on the same test); anything else is a rejection. _batch_all_ok = WebConfigBatch::nextAllOk(_batch_all_ok, @@ -680,11 +686,25 @@ void WebConfigServer::handleConfigPost(AsyncWebServerRequest* req) { return; } + // First onboarding is not complete until the known factory password has + // been replaced. Enforce this server-side so the Advanced editor or a crafted + // request cannot save WiFi and strand the node with the default password. + if (_mode == MODE_SETUP && _initial_setup && !set.containsKey("password") && + (reboot_after || set.containsKey("wifi.ssid"))) { + req->send(400, "application/json", "{\"error\":\"admin password required for initial setup\"}"); + return; + } + int count = 0; for (JsonPair kv : set) { const char* key = kv.key().c_str(); const char* val = kv.value().as(); - if (!val || !isAllowedSetKey(key)) { + // The admin password is the one key outside the `set` allowlist. It is safe + // in both modes: MODE_OFF was refused above, MODE_LAN required a login to + // get here, and MODE_SETUP has physical proximity. Rotating it is why the + // portal exists — restricting it to the AP would force a bridge outage. + const bool admin_pwd = wcIsAdminPasswordKey(key); + if (!val || (!isAllowedSetKey(key) && !admin_pwd)) { // Build with ArduinoJson so an attacker-supplied key containing quotes or // backslashes is escaped rather than breaking out of the JSON string. char safe_key[33]; @@ -698,6 +718,11 @@ void WebConfigServer::handleConfigPost(AsyncWebServerRequest* req) { req->send(400, "application/json", out); return; } + if (admin_pwd && !wcIsValidAdminPassword(val)) { + req->send(400, "application/json", + "{\"error\":\"admin password must be 1-15 characters with no line breaks\"}"); + return; + } if (isSecretKey(key) && strcmp(val, SECRET_SENTINEL) == 0) continue; // unchanged if (count >= MAX_BATCH) { req->send(400, "application/json", "{\"error\":\"too many changes\"}"); @@ -706,9 +731,11 @@ void WebConfigServer::handleConfigPost(AsyncWebServerRequest* req) { BatchEntry& e = _batch[count]; strncpy(e.key, key, sizeof(e.key) - 1); e.key[sizeof(e.key) - 1] = 0; - // Build "set ", stripping CR/LF so a value can't smuggle in - // a second command. - int pos = snprintf(e.cmd, sizeof(e.cmd), "set %s ", key); + // Build the allowlisted CLI command, stripping CR/LF from the value so it + // can't smuggle in a second command. The admin password reuses the existing + // top-level `password` command, so it persists exactly as the CLI does. + int pos = admin_pwd ? snprintf(e.cmd, sizeof(e.cmd), "password ") + : snprintf(e.cmd, sizeof(e.cmd), "set %s ", key); for (const char* p = val; *p && pos < (int)sizeof(e.cmd) - 1; p++) { if (*p == '\r' || *p == '\n') continue; e.cmd[pos++] = *p; diff --git a/src/helpers/esp32/WebConfigServer.h b/src/helpers/esp32/WebConfigServer.h index 2772057b..e2d9e08a 100644 --- a/src/helpers/esp32/WebConfigServer.h +++ b/src/helpers/esp32/WebConfigServer.h @@ -110,7 +110,7 @@ private: } } struct BatchEntry { - char key[24]; // allowlisted `set` key (echoed back to the UI) + char key[24]; // allowlisted config key (echoed back to the UI) char cmd[160]; // full CLI command (may contain secrets - never echoed) char reply[160]; }; @@ -129,6 +129,7 @@ private: Mode _mode = MODE_OFF; bool _stopping = false; bool _was_setup_ap = false; + bool _initial_setup = false; char _ap_ssid[33] = {0}; // Currently attached session, also used by the display's setup-info poll. diff --git a/test/README.md b/test/README.md index 15a19a54..2e2b81e2 100644 --- a/test/README.md +++ b/test/README.md @@ -25,7 +25,7 @@ does not reflect the GoogleTest count — run the built binary directly |-------|-------------------|--------| | `test_mqtt_presets` | `src/helpers/MQTTPresets.h` | preset lookup; table integrity (unique names, non-empty URLs, JWT-audience invariant, names fit the slot buffer); `mqttPresetNeedsSlotCredentials`; slot-count constants | | `test_observer_validation` | `src/helpers/MQTTObserverValidation.h` | IATA (exactly 3 alphanumerics), owner key (64 hex), NTP hostname, and the buffer-fit check behind the #17 length validation — including boundaries and nulls | -| `test_webconfig_keys` | `src/helpers/WebConfigKeys.h` | POST-key allowlist, secret detection, slot-index bounds, and the short-key out-of-bounds guard (attacker-supplied keys) | +| `test_webconfig_keys` | `src/helpers/WebConfigKeys.h` | POST-key allowlist, secret detection, admin-password classification/validation, slot-index bounds, and the short-key out-of-bounds guard (attacker-supplied keys) | | `test_topic_template` | `src/helpers/MQTTTopicTemplate.h` | `{iata}/{device}/{token}/{type}` expansion, overflow/NUL-termination, and a buffer-size fuzz | | `test_mqtt_topic_router` | `src/helpers/MQTTTopicRouter.h` | complete preset/custom topic-routing contract; MeshRank packets-only behavior; required identifiers; invalid inputs/slots; exact buffer boundaries | | `test_mqtt_connection_policy` | `src/helpers/MQTTConnectionPolicy.h` | reconnect guard/backoff/stagger and breaker transitions; stable reset; JWT lifetime/renewal policy; exact timing boundaries and 32-bit `millis()` rollover | diff --git a/test/test_webconfig_keys/test_webconfig_keys.cpp b/test/test_webconfig_keys/test_webconfig_keys.cpp index b2ef4fe4..f55f15c9 100644 --- a/test/test_webconfig_keys/test_webconfig_keys.cpp +++ b/test/test_webconfig_keys/test_webconfig_keys.cpp @@ -34,6 +34,23 @@ TEST(WebConfigKeys, RejectsDangerousOrUnknownKeys) { EXPECT_FALSE(wcIsAllowedSetKey("")); } +TEST(WebConfigKeys, AdminPasswordIsNotAnAllowlistedSetKey) { + EXPECT_TRUE(wcIsAdminPasswordKey("password")); + EXPECT_FALSE(wcIsAdminPasswordKey("admin.password")); + EXPECT_FALSE(wcIsAdminPasswordKey("")); + EXPECT_FALSE(wcIsAllowedSetKey("password")); // never reachable as `set password` +} + +TEST(WebConfigKeys, AdminPasswordFitsNodePrefsAndRejectsLineBreaks) { + EXPECT_FALSE(wcIsValidAdminPassword(NULL)); + EXPECT_FALSE(wcIsValidAdminPassword("")); + EXPECT_TRUE(wcIsValidAdminPassword("new-password")); + EXPECT_TRUE(wcIsValidAdminPassword("123456789012345")); + EXPECT_FALSE(wcIsValidAdminPassword("1234567890123456")); + EXPECT_FALSE(wcIsValidAdminPassword("line\nbreak")); + EXPECT_FALSE(wcIsValidAdminPassword("line\rbreak")); +} + TEST(WebConfigKeys, SlotIndexBoundsMatchMaxSlots) { EXPECT_FALSE(wcIsAllowedSetKey("mqtt0.preset")); // slot 0 invalid EXPECT_TRUE(wcIsAllowedSetKey("mqtt6.preset")); // last valid slot diff --git a/webui/index.html b/webui/index.html index bc09bee0..dad8daa2 100644 --- a/webui/index.html +++ b/webui/index.html @@ -175,6 +175,15 @@ canvas{width:100%;height:56px;display:block}
Need settings that aren't listed? Choose Keep current settings, finish setup, then fine-tune in the Advanced editor.
+
+

Admin password

+

Replaces the factory password. Used to log in to this page later and to administer the node remotely over the mesh.

+
+ +
Maximum 15 characters.
+
+
+
@@ -239,6 +248,15 @@ canvas{width:100%;height:56px;display:block}
+
+

Admin password

+
Signs in here and authenticates remote admin commands over the mesh. Leave blank to keep the current password. A change takes effect immediately — this session stays signed in, but the next login needs the new password. During first-time setup a new password is required before the node can reboot into normal operation.
+
+ +
Maximum 15 characters.
+
+
+

LoRa radio reboot to apply

Frequency, bandwidth, SF and CR must match your mesh exactly — a wrong value takes this node off the air until fixed over serial.
@@ -472,7 +490,7 @@ function boot(){ // Show only as many server slots as the board can keep active at once // (active_slots: 2 without PSRAM, 5 with). Fall back to the runtime array // size for older firmware that doesn't report it. - st.mode=s.mode;st.authed=s.auth;st.nslots=s.active_slots||s.runtime_slots||6; + st.mode=s.mode;st.authed=s.auth;st.needsSetup=!!s.needs_setup;st.nslots=s.active_slots||s.runtime_slots||6; $("#h-name").textContent=s.name||"MeshCore"; $("#h-sub").textContent=s.role+" · "+s.fw+" · "+s.board; var b=$("#h-badge");b.classList.remove("hide"); @@ -522,6 +540,7 @@ function cfgVal(k){ // map a `set` key to its current value string, from st.cfg case"loop.detect":return r.loop_detect; case"advert.interval":return String(r.advert_interval); case"flood.advert.interval":return String(r.flood_advert_interval); + case"password":return""; case"wifi.ssid":return w.ssid;case"wifi.pwd":return w.pwd;case"wifi.powersave":return w.powersave; case"mqtt.origin":return q.origin;case"mqtt.iata":return q.iata; case"mqtt.status":return q.status?"on":"off";case"mqtt.packets":return q.packets?"on":"off"; @@ -556,6 +575,7 @@ function loadConfig(){ st.orig[k]=v;setEl(el,v);el.classList.remove("dirty"); var lab=el.closest(".f");var ch=lab&&lab.querySelector(".chip");if(ch)ch.remove(); }); + $$("[data-cfm]").forEach(function(el){el.value=""}); // confirm twins have no stored value $$("[data-rg]").forEach(function(el){el.value=String(c.radio[el.dataset.rg]);el.classList.remove("dirty")}); st.orig.radio=radioCombo();afHint(); for(var i=1;i<=st.nslots;i++){refreshSlotFields($("#app-slots"),i);refreshSlotFields($("#wz-slots"),i)} @@ -582,6 +602,8 @@ document.addEventListener("input",function(ev){ // keep duplicate fields (wizard vs app share keys) in sync $$('[data-k="'+el.dataset.k+'"]').forEach(function(o){if(o!==el)setEl(o,elVal(el))}); markDirty(el.dataset.k,elVal(el),el); + }else if(el.dataset.cfm){ // confirm-only twin, mirrored but never submitted + $$('[data-cfm="'+el.dataset.cfm+'"]').forEach(function(o){if(o!==el)o.value=el.value}); }else if(el.dataset.rg){ var v=radioCombo(); $$("[data-rg]").forEach(function(o){o.classList.toggle("dirty",v!==st.orig.radio)}); @@ -703,7 +725,13 @@ function saveChanges(extra,cb){ // device only arms the reboot if every command succeeded (see the backend), // so a partially-rejected batch stays put for the operator to correct. function appSave(){ + // Validate before the wifi split: a password-only change (the common LAN + // case) never reaches the reboot path but still must match its confirm field. var wifiChanged=("wifi.ssid" in st.dirty)||("wifi.pwd" in st.dirty); + var pwdErr=adminPwdError(wifiChanged); + // The save bar is global, so the offending field may be on another tab — + // show it, or the toast names a problem the user cannot see. + if(pwdErr){$('#tabs button[data-t="radio"]').click();toast(pwdErr);return} if(!wifiChanged){saveChanges();return} saveChanges({reboot:true},function(r,acked){ if(!r){ @@ -784,6 +812,9 @@ function loadConfigSoft(){ // re-sync accepted values without clobbering chips/r if((k in st.dirty)&&st.dirty[k]!==stored){setEl(el,st.dirty[k]);el.classList.add("dirty")} else{delete st.dirty[k];setEl(el,stored);el.classList.remove("dirty")} }); + // Clear the confirm twins alongside their (write-only, so always-cleared) + // password field; a value left behind here fails the next save as a mismatch. + if(!("password" in st.dirty))$$("[data-cfm]").forEach(function(el){el.value=""}); var storedRadio=[c.radio.freq,c.radio.bw,c.radio.sf,c.radio.cr].join(","); st.orig.radio=storedRadio; var keepRadio=("radio" in st.dirty)&&!comboEq(st.dirty.radio,storedRadio); @@ -1020,6 +1051,17 @@ function popOut(){ // presence rather than truthiness stops a cleared field from silently reverting // to the original in the review/validation while the batch still submits "". function eff(k){return (k in st.dirty)?st.dirty[k]:(st.orig[k]||"")} +// Admin-password gate shared by both save paths: "" when OK, else the reason. +// The confirm field is UI-only (never submitted), so it is read from the DOM. +// `finishing` marks a save that completes onboarding (reboots or sets WiFi) — +// only those must carry a new password, matching the server's 400. A first-boot +// save of some unrelated setting is allowed through, as the firmware allows it. +function adminPwdError(finishing){ + var p=eff("password"),c=$('[data-cfm="password"]').value; + if(finishing&&st.needsSetup&&!p)return"Set an admin password"; + if((p||c)&&p!==c)return"Admin passwords do not match"; + return""; +} function buildReview(){ var rows=[],d=st.dirty; function row(l,v){rows.push('
'+l+""+esc(v)+"
")} @@ -1033,6 +1075,7 @@ function buildReview(){ row("Radio",(rp?rp.t+" — ":"")+rc.split(",").join(" / ")+(("radio" in d)?"":" (unchanged)")); if(("tx" in d)&&d.tx!=="")row("TX power",d.tx+" dBm"); row("Repeat",eff("repeat")==="off"?"off (listen-only)":"on"); + if(st.needsSetup||eff("password"))row("Admin password",eff("password")?"set":"(not set)"); row("Observer name",eff("mqtt.origin")||"(not set)"); row("IATA",(eff("mqtt.iata")||"(not set)").toUpperCase()); var own=eff("mqtt.owner"); @@ -1043,7 +1086,7 @@ function buildReview(){ var p=eff("mqtt"+i+".preset"); if(p&&p!=="none")row("Slot "+i,p); } - var extra=Object.keys(d).filter(function(k){return!/^(wifi\.|name$|radio$|tx$|repeat$|mqtt\.origin|mqtt\.iata|mqtt\.owner|mqtt\.email|mqtt[1-9]\.)/.test(k)}).length; + var extra=Object.keys(d).filter(function(k){return!/^(wifi\.|password$|name$|radio$|tx$|repeat$|mqtt\.origin|mqtt\.iata|mqtt\.owner|mqtt\.email|mqtt[1-9]\.)/.test(k)}).length; if(extra)row("Other changes",extra+" setting(s)"); $("#wz-review").innerHTML=rows.join(""); } @@ -1052,6 +1095,8 @@ function wizardSave(){ // even if the node originally had one (the batch would submit an empty SSID). if(!eff("wifi.ssid")){$("#wz-err").textContent="Please set a WiFi network (step 1).";return} if($("#wz-rp").value===""){$("#wz-err").textContent="Please choose a radio preset (step 2) — nodes ship with non-US defaults.";return} + var pwdErr=adminPwdError(true); // the wizard always saves WiFi and reboots + if(pwdErr){$("#wz-err").textContent=pwdErr+" (step 2).";return} // IATA is optional, but if given it must be exactly 3 chars (goes into topics). var iata=eff("mqtt.iata"); if(iata&&iata.length!==3){$("#wz-err").textContent="IATA region code must be exactly 3 characters (e.g. DEN), or left blank.";return} @@ -1077,7 +1122,8 @@ function wizardSave(){ return; } showReboot("The node is joining “"+eff("wifi.ssid") - +"”. Reconnect your phone or laptop to your normal WiFi. Find the node's IP on your router, or run “start webconfig” over serial to manage it again.", + +"”. Reconnect your phone or laptop to your normal WiFi. Find the node's IP on your router, or run “start webconfig” over serial to manage it again" + +(eff("password")?" — logging in there needs the admin password you just set.":"."), false,"Settings saved ✓ — rebooting in a moment"); st.dirty={}; });