From e3e55f4b0ef3333907f2174c63bf9ed06b815957 Mon Sep 17 00:00:00 2001 From: FERMIN Date: Thu, 12 Nov 2026 01:28:40 +0100 Subject: [PATCH] Fix WiFi password corruption when a stored value cannot be decrypted wifiPwdDecrypt() returns the input unchanged when it cannot decrypt it (not base64, not whole AES blocks, or bad PKCS7 padding). applySettingsFromRoot() and getWifiFromNVS() treated that return value as a plaintext password, so saveConfigs()/saveWifiIntoNVS() encrypted the ciphertext again: every boot nested another layer and the real password was lost. That happens when config.conf or NVS was written by a build with a different WIFI_ENC_KEY, which is normal when an SD card moves between devices or the key changes between releases. wifiPwdDecryptStored() now reports whether the value was readable. When it was not, the caller keeps a good password already loaded from config.conf, or stores the SSID without one, instead of re-encrypting garbage. Values this build can read are handled exactly as before; the cipher, key, IV and stored format are unchanged. Also stop seeding config.conf with the myNetSSID/myNetPwd placeholder, which showed up as a saved network and came back after every reset, and collapse duplicate SSIDs when NVS is merged with config.conf. --- src/settings.cpp | 106 +++++++++++++++++++++++++++++++++++--------- src/settings.h | 7 +-- src/wifi_crypto.cpp | 66 ++++++++++++++++++++++++--- src/wifi_crypto.h | 8 ++++ 4 files changed, 153 insertions(+), 34 deletions(-) diff --git a/src/settings.cpp b/src/settings.cpp index bb452dd9..6ce06aea 100644 --- a/src/settings.cpp +++ b/src/settings.cpp @@ -62,6 +62,35 @@ JsonArray ensureWifiListInternal() { return wifiList; } +// NVS and config.conf are merged on every boot, so the same SSID can end up in +// the list twice (a copy from another device, a trailing-space twin, or the +// old "myNetSSID" placeholder). Keep one entry per SSID, preferring the copy +// that still has a password. The legacy placeholder pair is dropped entirely. +void dedupeWifiList(JsonArray wifiList) { + if (wifiList.isNull()) return; + + for (size_t i = wifiList.size(); i-- > 0;) { + JsonObject entry = wifiList[i].as(); + String ssid = entry.isNull() ? String() : entry["ssid"].as(); + String pwd = entry.isNull() ? String() : entry["pwd"].as(); + + if (ssid.isEmpty() || (ssid == "myNetSSID" && (pwd.isEmpty() || pwd == "myNetPwd"))) { + wifiList.remove(i); + wifiListDirty = true; + continue; + } + + for (size_t j = 0; j < i; j++) { + JsonObject other = wifiList[j].as(); + if (other.isNull() || other["ssid"].as() != ssid) continue; + if (other["pwd"].as().isEmpty() && !pwd.isEmpty()) other["pwd"] = pwd; + wifiList.remove(i); + wifiListDirty = true; + break; + } + } +} + JsonObject ensureKeyBindingObjectInternal() { JsonObject setting = ensureSettingsRoot(); if (setting.isNull()) return JsonObject(); @@ -282,7 +311,18 @@ int applySettingsFromRoot(JsonObject setting) { if (setting["wifi"].is()) { for (JsonObject wifiEntry : setting["wifi"].as()) { if (wifiEntry["secure"].as()) { - wifiEntry["pwd"] = wifiPwdDecrypt(wifiEntry["pwd"].as()); + String decoded; + if (wifiPwdDecryptStored(wifiEntry["pwd"].as(), decoded)) { + wifiEntry["pwd"] = decoded; + } else { + // Written by a build with a different WIFI_ENC_KEY (for + // example config.conf carried over from another device). + // Keep the SSID but drop the unreadable password so it is + // not re-encrypted as if it were the plaintext, and force + // a re-save so the stale ciphertext disappears. + wifiEntry["pwd"] = ""; + ++count; + } wifiEntry.remove("secure"); } else if (!wifiEntry["ssid"].as().isEmpty()) { ++count; // plain-text entry — trigger re-save with encryption @@ -427,11 +467,20 @@ bool setWifiCredential(const String &ssidValue, const String &passwordValue, boo if (wifiList.isNull()) return false; JsonObject target; - for (JsonObject wifiEntry : wifiList) { - if (wifiEntry["ssid"].as() == ssidValue) { + for (size_t i = 0; i < wifiList.size();) { + JsonObject wifiEntry = wifiList[i].as(); + if (wifiEntry.isNull() || wifiEntry["ssid"].as() != ssidValue) { + i++; + continue; + } + if (target.isNull()) { target = wifiEntry; - break; + i++; + continue; } + // Collapse duplicates that survived an NVS/config.conf merge. + wifiList.remove(i); + wifiListDirty = true; } if (target.isNull()) { target = wifiList.createNestedObject(); } if (target.isNull()) { @@ -451,15 +500,18 @@ bool removeWifiCredential(const String &ssidValue) { JsonArray wifiList = ensureWifiListInternal(); if (wifiList.isNull()) return false; - for (size_t i = 0; i < wifiList.size(); i++) { + bool removed = false; + for (size_t i = wifiList.size(); i-- > 0;) { if (wifiList[i]["ssid"].as() == ssidValue) { wifiList.remove(i); - wifiListDirty = true; - saveConfigs(); - return true; + removed = true; } } - return false; + if (removed) { + wifiListDirty = true; + saveConfigs(); + } + return removed; } bool clearWifiCredentials() { @@ -1132,10 +1184,10 @@ bool saveSessionToken(const String &token) { } bool saveWifiIntoNVS() { - if (!wifiListDirty) return true; - JsonArray wifiList = ensureWifiListInternal(); if (wifiList.isNull()) return false; + dedupeWifiList(wifiList); + if (!wifiListDirty) return true; lnvs::Handle nvsHandle("l_wifi", true); if (!nvsHandle) return false; @@ -1274,11 +1326,28 @@ bool getWifiFromNVS(bool mergeExisting) { uint8_t isSecure = 0; nvs_get_u8(handle.raw(), ("b_" + suffix).c_str(), &isSecure); - if (isSecure) pwdValue = wifiPwdDecrypt(pwdValue); + if (isSecure) { + String decoded; + if (wifiPwdDecryptStored(pwdValue, decoded)) { + pwdValue = decoded; + } else { + // NVS was written by a build with a different WIFI_ENC_KEY. + // Keep a good password already loaded from config.conf; + // otherwise store the SSID without one so it can be entered + // again instead of re-encrypting the ciphertext. + String existing; + if (getWifiCredential(ssid, existing) && !existing.isEmpty()) { + launcherConsolePrintf("SSID: %s (password kept from config.conf)\n", ssid.c_str()); + continue; + } + pwdValue = ""; + } + } setWifiCredential(ssid, pwdValue, false); launcherConsolePrintf("SSID: %s\n", ssid.c_str()); } + dedupeWifiList(wifiList); return true; } @@ -1353,15 +1422,10 @@ void saveConfigs() { populateSettingsFromGlobals(setting); - // Ensure wifi array has at least a placeholder entry - JsonArray wifiList = setting["wifi"].as(); - if (!wifiList.isNull() && wifiList.size() == 0) { - JsonObject wifiObj = wifiList.add(); - if (!wifiObj.isNull()) { - wifiObj["ssid"] = ssid.length() == 0 ? "myNetSSID" : ssid; - wifiObj["pwd"] = pwd.length() == 0 ? "myNetPwd" : pwd; - } - } + // An empty list is valid: WiFi is optional, and seeding it with a fake + // "myNetSSID" entry made that placeholder look like a saved network and + // kept coming back after every reset. Just collapse duplicates here. + dedupeWifiList(setting["wifi"].as()); // Encrypt wifi passwords before writing to SD { diff --git a/src/settings.h b/src/settings.h index 4e3ffd9c..bb95c5eb 100644 --- a/src/settings.h +++ b/src/settings.h @@ -23,12 +23,7 @@ config.conf JSON structure "ALCOLOR":63488, "even":13029, "odd": 12485, - "wifi":[ - { - "ssid":"myNetSSID", - "pwd":"myNetPwd", - }, - ] + "wifi": [] } ] diff --git a/src/wifi_crypto.cpp b/src/wifi_crypto.cpp index 56fad195..0ba3e355 100644 --- a/src/wifi_crypto.cpp +++ b/src/wifi_crypto.cpp @@ -99,17 +99,20 @@ String wifiPwdEncrypt(const String &plain) { return result; } -String wifiPwdDecrypt(const String &cipher) { - if (cipher.isEmpty()) return cipher; +// Decrypt one layer. Returns false when `cipher` is not a clean ciphertext for +// this build's key (not base64, not whole AES blocks, or bad PKCS7 padding). +static bool decryptOnce(const String &cipher, String &plain) { + plain = ""; + if (cipher.isEmpty()) return true; size_t maxBin = (cipher.length() / 4) * 3 + 4; uint8_t *buf = (uint8_t *)malloc(maxBin); - if (!buf) return cipher; + if (!buf) return false; size_t binLen = b64Decode(cipher, buf, maxBin); if (binLen == 0 || binLen % 16 != 0) { free(buf); - return cipher; + return false; } uint8_t key[16]; @@ -128,9 +131,58 @@ String wifiPwdDecrypt(const String &cipher) { uint8_t padByte = buf[binLen - 1]; if (padByte == 0 || padByte > 16 || padByte > binLen) { free(buf); - return cipher; + return false; } - String result((char *)buf, binLen - padByte); + plain = String((char *)buf, binLen - padByte); free(buf); - return result; + return true; +} + +// Shape test for "this is one of our ciphertexts", independent of whether the +// key matches. Used to tell a real password apart from a foreign ciphertext +// that failed to decrypt. +static bool looksLikeCipher(const String &value) { + const size_t len = value.length(); + if (len < 24 || len % 4 != 0) return false; + + size_t pad = 0; + for (size_t i = 0; i < len; i++) { + const char c = value[i]; + if (c == '=') { + if (i < len - 2) return false; + pad++; + } else if (b64Val(c) < 0) { + return false; + } + } + if (pad > 2) return false; + return ((len / 4) * 3 - pad) % 16 == 0; +} + +String wifiPwdDecrypt(const String &cipher) { + String plain; + if (!decryptOnce(cipher, plain)) return cipher; + return plain; +} + +bool wifiPwdDecryptStored(const String &cipher, String &plain) { + plain = ""; + if (cipher.isEmpty()) return true; + + String value = cipher; + // A value written by this build decrypts in one step. Older builds could + // wrap a password more than once: when they could not decrypt a value + // (different WIFI_ENC_KEY) they re-encrypted the ciphertext, so unwrap + // until the value no longer looks like one of our ciphertexts. + for (int depth = 0; depth < 3; depth++) { + String inner; + if (!decryptOnce(value, inner)) { + if (looksLikeCipher(value)) return false; + plain = value; + return true; + } + value = inner; + } + plain = value; + return true; } diff --git a/src/wifi_crypto.h b/src/wifi_crypto.h index bc2a1399..f336f2f0 100644 --- a/src/wifi_crypto.h +++ b/src/wifi_crypto.h @@ -8,3 +8,11 @@ String wifiPwdEncrypt(const String &plain); // Decrypt a base64+AES-128-CBC password produced by wifiPwdEncrypt. // Returns the ciphertext unchanged if decoding/decryption fails. String wifiPwdDecrypt(const String &cipher); + +// Decrypt a stored password and report whether it was actually readable with +// this build's key. Returns false when `cipher` is a ciphertext from another +// build (config.conf shared across devices, or NVS written with a different +// WIFI_ENC_KEY); in that case the caller must not re-encrypt it as if it were +// the plaintext password. Also unwraps values that older builds encrypted more +// than once after a failed decrypt. +bool wifiPwdDecryptStored(const String &cipher, String &plain);