Fix WiFi password corruption when a stored value cannot be decrypted

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<JsonObject>();
+        String ssid = entry.isNull() ? String() : entry["ssid"].as<String>();
+        String pwd = entry.isNull() ? String() : entry["pwd"].as<String>();
+
+        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<JsonObject>();
+            if (other.isNull() || other["ssid"].as<String>() != ssid) continue;
+            if (other["pwd"].as<String>().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<JsonArray>()) {
         for (JsonObject wifiEntry : setting["wifi"].as<JsonArray>()) {
             if (wifiEntry["secure"].as<bool>()) {
-                wifiEntry["pwd"] = wifiPwdDecrypt(wifiEntry["pwd"].as<String>());
+                String decoded;
+                if (wifiPwdDecryptStored(wifiEntry["pwd"].as<String>(), 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<String>().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<String>() == ssidValue) {
+    for (size_t i = 0; i < wifiList.size();) {
+        JsonObject wifiEntry = wifiList[i].as<JsonObject>();
+        if (wifiEntry.isNull() || wifiEntry["ssid"].as<String>() != 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<String>() == 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<JsonArray>();
-    if (!wifiList.isNull() && wifiList.size() == 0) {
-        JsonObject wifiObj = wifiList.add<JsonObject>();
-        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<JsonArray>());
 
     // Encrypt wifi passwords before writing to SD
     {

src/settings.h

@@ -23,12 +23,7 @@ config.conf JSON structure
       "ALCOLOR":63488,
       "even":13029,
       "odd": 12485,
-      "wifi":[
-         {
-            "ssid":"myNetSSID",
-            "pwd":"myNetPwd",
-         },
-      ]
+      "wifi": []
    }
 ]
 

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;
 }

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);