diff --git a/src/fileSystem/ftp.js b/src/fileSystem/ftp.js index 8ba1ea720..bb045ec63 100644 --- a/src/fileSystem/ftp.js +++ b/src/fileSystem/ftp.js @@ -1,3 +1,4 @@ +import secureCredentials from "lib/secureCredentials"; import settings from "lib/settings"; import mimeType from "mime-types"; import { decode, encode } from "utils/encodings"; @@ -359,10 +360,12 @@ Ftp.fromUrl = (url) => { const { username, password, hostname, pathname, port, query } = Url.decodeUrl(url); const { security, mode } = query; + // Secrets are kept in the encrypted store, not in the saved URL (#2561). + const stored = secureCredentials.get(url) || {}; const ftp = new FtpClient( hostname, username, - password, + password || stored.password, port || 21, security, mode, diff --git a/src/fileSystem/sftp.js b/src/fileSystem/sftp.js index 7c4b72980..3d0a5301f 100644 --- a/src/fileSystem/sftp.js +++ b/src/fileSystem/sftp.js @@ -1,3 +1,4 @@ +import secureCredentials from "lib/secureCredentials"; import settings from "lib/settings"; import mimeType from "mime-types"; import { decode, encode } from "utils/encodings"; @@ -592,10 +593,13 @@ Sftp.fromUrl = (url) => { Url.decodeUrl(url); const { keyFile, passPhrase } = query; + // Secrets are kept in the encrypted store, not in the saved URL (#2561). + const stored = secureCredentials.get(url) || {}; + const sftp = new SftpClient(hostname, port || 22, username, { - password, + password: password || stored.password, keyFile, - passPhrase, + passPhrase: passPhrase || stored.passPhrase, }); sftp.setPath(pathname); diff --git a/src/lib/remoteStorage.js b/src/lib/remoteStorage.js index 5185de0ce..a8d9f8935 100644 --- a/src/lib/remoteStorage.js +++ b/src/lib/remoteStorage.js @@ -3,6 +3,7 @@ import Ftp from "fileSystem/ftp"; import Sftp from "fileSystem/sftp"; import loader from "dialogs/loader"; import multiPrompt from "dialogs/multiPrompt"; +import secureCredentials from "lib/secureCredentials"; import URLParse from "url-parse"; import helpers from "utils/helpers"; import Url from "utils/Url"; @@ -54,8 +55,12 @@ export default { }, }); + // Keep the password in the encrypted store instead of the saved + // URL, which lives in plaintext localStorage (#2561). + await secureCredentials.set(url, { password }); + const res = { - url, + url: secureCredentials.stripPassword(url), alias, name: alias, type: "ftp", @@ -232,10 +237,14 @@ export default { }); loader.destroy(); await helpers.showInterstitialIfReady(); + + // Keep secrets in the encrypted store instead of the saved URL (#2561). + await secureCredentials.set(url, { password, passPhrase }); + return { alias, name: alias, - url, + url: secureCredentials.stripPassword(url), type: "sftp", home, }; @@ -369,6 +378,11 @@ export default { edit({ name, storageType, url }) { let { username, password, hostname, port, query } = URLParse(url, true); + // Passwords are no longer kept in the saved URL (#2561), so pull the + // stored secret back in to prefill the edit form. + const stored = secureCredentials.get(url) || {}; + if (!password && stored.password) password = stored.password; + if (username) { username = decodeURIComponent(username); } diff --git a/src/lib/secureCredentials.js b/src/lib/secureCredentials.js new file mode 100644 index 000000000..3c8322d32 --- /dev/null +++ b/src/lib/secureCredentials.js @@ -0,0 +1,171 @@ +/** + * Encrypted storage for remote-server secrets (FTP/SFTP passwords and key + * passphrases). + * + * The saved-server list itself stays in `localStorage.storageList` so plugins + * that read it keep working; only the secrets are moved out into the native + * encrypted store, keyed by connection identity. Secrets are put back into the + * URL at connect time. See #2561. + */ + +const SECURE_KEY = "remoteCredentials"; + +/** @type {Record} */ +let cache = {}; + +/** + * Connection identity used as the lookup key: protocol, user, host and port. + * Deliberately excludes the path so every folder under a server shares one entry. + * @param {string} url + * @returns {string|null} + */ +function keyFor(url) { + if (!url) return null; + const m = /^([a-z0-9+.-]+:)\/\/([^@/]*@)?([^/:?#]+)(:(\d+))?/i.exec(url); + if (!m) return null; + const protocol = m[1].toLowerCase(); + const userinfo = (m[2] || "").replace(/@$/, ""); + const username = decodeURIComponent(userinfo.split(":")[0] || ""); + const host = m[3].toLowerCase(); + const port = m[5] || ""; + return `${protocol}//${username}@${host}${port ? ":" + port : ""}`; +} + +/** + * Remove `user:password@` credentials from a URL, keeping the username. + * Used so URLs saved before the migration still prefix-match today's URLs. + * @param {string} url + * @returns {string} + */ +function stripPassword(url) { + if (!url) return url; + return url.replace( + /^([a-z0-9+.-]+:\/\/)([^@/]*?):([^@/]*)@/i, + (_, scheme, user) => `${scheme}${user}@`, + ); +} + +/** Promisified bridge helpers — resolve to null instead of throwing. */ +function secureGet(key) { + return new Promise((resolve) => { + try { + window.system.secureGet(key, resolve, () => resolve(null)); + } catch (_) { + resolve(null); + } + }); +} + +function secureSet(key, value) { + return new Promise((resolve, reject) => { + try { + window.system.secureSet(key, value, resolve, reject); + } catch (error) { + reject(error); + } + }); +} + +/** + * Load secrets into memory, and migrate any credentials still embedded in + * `localStorage.storageList` from older versions. + * Must be awaited during startup, before anything connects to a remote server. + */ +async function hydrate() { + try { + const stored = await secureGet(SECURE_KEY); + cache = stored ? JSON.parse(stored) || {} : {}; + } catch (error) { + cache = {}; + window.log?.("error", `secureCredentials: hydrate failed - ${error}`); + } + + await migrateLegacy(); +} + +/** + * One-time move of inline credentials out of `localStorage.storageList`. + * The plaintext copy is only rewritten once the encrypted write is confirmed on + * disk, so an interrupted migration can't lose a saved server. + */ +async function migrateLegacy() { + let list; + try { + list = JSON.parse(localStorage.storageList || "[]"); + } catch (_) { + return; + } + if (!Array.isArray(list) || !list.length) return; + + let changed = false; + const pending = { ...cache }; + + for (const entry of list) { + const url = entry?.url; + if (!url || !/^[a-z0-9+.-]+:\/\/[^@/]*:[^@/]*@/i.test(url)) continue; + + const key = keyFor(url); + if (!key) continue; + + const password = decodeURIComponent( + /^[a-z0-9+.-]+:\/\/[^@/]*?:([^@/]*)@/i.exec(url)?.[1] || "", + ); + if (!password) continue; + + pending[key] = { ...(pending[key] || {}), password }; + entry.url = stripPassword(url); + changed = true; + } + + if (!changed) return; + + try { + await secureSet(SECURE_KEY, JSON.stringify(pending)); + cache = pending; + localStorage.storageList = JSON.stringify(list); + } catch (error) { + // Keep the legacy copy and retry on the next launch rather than lose it. + window.log?.("error", `secureCredentials: migration failed - ${error}`); + } +} + +/** + * Secrets for a connection, or null. Synchronous by design so the existing + * synchronous `fromUrl` paths keep working. + * @param {string} url + */ +function get(url) { + const key = keyFor(url); + return (key && cache[key]) || null; +} + +/** + * Persist secrets for a connection. Empty values remove the entry. + * @param {string} url + * @param {{password?: string, passPhrase?: string}} secrets + */ +async function set(url, secrets) { + const key = keyFor(url); + if (!key) return; + + const clean = {}; + if (secrets?.password) clean.password = secrets.password; + if (secrets?.passPhrase) clean.passPhrase = secrets.passPhrase; + + const next = { ...cache }; + if (Object.keys(clean).length) next[key] = clean; + else delete next[key]; + + await secureSet(SECURE_KEY, JSON.stringify(next)); + cache = next; +} + +/** + * Drop stored secrets for a connection (used when a server is removed). + * @param {string} url + */ +async function remove(url) { + await set(url, {}); +} + +export default { hydrate, get, set, remove, stripPassword, keyFor }; diff --git a/src/main.js b/src/main.js index c29eee871..79d4ca79a 100644 --- a/src/main.js +++ b/src/main.js @@ -52,6 +52,7 @@ import notificationManager from "lib/notificationManager"; import openFolder, { addedFolder } from "lib/openFolder"; import { registerPrettierFormatter } from "lib/registerPrettierFormatter"; import restoreFiles from "lib/restoreFiles"; +import secureCredentials from "lib/secureCredentials"; import settings from "lib/settings"; import startAd, { BANNER_SUPPRESSION_REASON, @@ -101,6 +102,10 @@ document.addEventListener("menubutton", menuButtonHandler); async function onDeviceReady() { await initEncodings(); // important to load encodings before anything else + // Load remote-server secrets from the encrypted native store, migrating any + // credentials still embedded in localStorage. Must run before anything + // connects to a saved FTP/SFTP server. See issue #2561. + await secureCredentials.hydrate(); const isFreePackage = /(free)$/.test(BuildInfo.packageName); const oldResolveURL = window.resolveLocalFileSystemURL; diff --git a/src/plugins/system/android/com/foxdebug/system/SecureStore.java b/src/plugins/system/android/com/foxdebug/system/SecureStore.java new file mode 100644 index 000000000..df8e15cca --- /dev/null +++ b/src/plugins/system/android/com/foxdebug/system/SecureStore.java @@ -0,0 +1,85 @@ +package com.foxdebug.system; + +import android.content.Context; +import android.content.SharedPreferences; +import androidx.security.crypto.EncryptedSharedPreferences; +import androidx.security.crypto.MasterKeys; +import java.io.IOException; +import java.security.GeneralSecurityException; + +/** + * Encrypted key/value store for secrets that must not sit in cleartext on disk + * (saved FTP/SFTP credentials — see #2561). Backed by AndroidX Security-Crypto + * (AES256-GCM values, AES256-SIV keys), the same mechanism the auth plugin uses + * for the account token. + * + * If the encrypted store can't be opened (keystore/crypto failure), reads and + * writes fail rather than falling back to plaintext. A plaintext fallback would + * both re-introduce cleartext credentials and become unreadable once encryption + * recovers — EncryptedSharedPreferences encrypts lookup keys, so a literal key + * written in fallback mode can't be found again. Failing instead lets the caller + * keep its source copy and retry on the next launch. + */ +public class SecureStore { + + private static final String PREF_NAME = "acode_secure_store"; + + private final Context context; + private SharedPreferences prefs; + + public SecureStore(Context context) { + this.context = context.getApplicationContext(); + } + + /** The encrypted preferences, or null if encryption is currently unavailable. */ + private SharedPreferences prefs() { + if (prefs != null) return prefs; + try { + String masterKeyAlias = MasterKeys.getOrCreate(MasterKeys.AES256_GCM_SPEC); + prefs = EncryptedSharedPreferences.create( + PREF_NAME, + masterKeyAlias, + context, + EncryptedSharedPreferences.PrefKeyEncryptionScheme.AES256_SIV, + EncryptedSharedPreferences.PrefValueEncryptionScheme.AES256_GCM + ); + } catch (GeneralSecurityException | IOException e) { + prefs = null; + } + return prefs; + } + + /** + * Store a value durably. Passing null removes the key. + * Uses commit() (not apply()) so the write is on disk before returning — the + * JS migration deletes the legacy plaintext copy only after this reports + * success. + * @return true if the write reached disk; false if encryption is unavailable. + */ + public boolean set(String key, String value) { + if (value == null) { + return remove(key); + } + SharedPreferences p = prefs(); + if (p == null) return false; + return p.edit().putString(key, value).commit(); + } + + /** Return the stored value, or null if absent or encryption is unavailable. */ + public String get(String key) { + SharedPreferences p = prefs(); + if (p == null) return null; + return p.getString(key, null); + } + + public boolean remove(String key) { + SharedPreferences p = prefs(); + if (p == null) return false; + return p.edit().remove(key).commit(); + } + + public boolean contains(String key) { + SharedPreferences p = prefs(); + return p != null && p.contains(key); + } +} diff --git a/src/plugins/system/android/com/foxdebug/system/System.java b/src/plugins/system/android/com/foxdebug/system/System.java index c97ef0c77..bfe44b772 100644 --- a/src/plugins/system/android/com/foxdebug/system/System.java +++ b/src/plugins/system/android/com/foxdebug/system/System.java @@ -86,6 +86,7 @@ public class System extends CordovaPlugin { private CordovaWebView webView; private String fileProviderAuthority; private RewardPassManager rewardPassManager; + private SecureStore secureStore; public void initialize(CordovaInterface cordova, CordovaWebView webView) { super.initialize(cordova, webView); @@ -93,6 +94,7 @@ public void initialize(CordovaInterface cordova, CordovaWebView webView) { this.activity = cordova.getActivity(); this.webView = webView; this.rewardPassManager = new RewardPassManager(this.context); + this.secureStore = new SecureStore(this.context); this.activity.runOnUiThread( new Runnable() { @Override @@ -217,6 +219,31 @@ public void run() { case "getFilesDir": callbackContext.success(getFilesDir()); return true; + case "secure-set": + // arg1 = key, arg2 = value (null clears the key). Report failure if the + // durable write did not reach disk, so the JS migration keeps its + // fallback copy rather than deleting it. See #2561. + if (secureStore.set(arg1, args.isNull(1) ? null : arg2)) { + callbackContext.success(); + } else { + callbackContext.error("secure write failed"); + } + return true; + case "secure-get": + { + String storedValue = secureStore.get(arg1); + // success(String) with null would throw; send an explicit empty result + if (storedValue == null) { + callbackContext.success((String) null); + } else { + callbackContext.success(storedValue); + } + } + return true; + case "secure-remove": + secureStore.remove(arg1); + callbackContext.success(); + return true; case "getRewardStatus": callbackContext.success(rewardPassManager.getRewardStatus()); return true; diff --git a/src/plugins/system/plugin.xml b/src/plugins/system/plugin.xml index ed80bf0cb..9d08c9b73 100644 --- a/src/plugins/system/plugin.xml +++ b/src/plugins/system/plugin.xml @@ -35,12 +35,14 @@ - - - - + + + + + + diff --git a/src/plugins/system/www/plugin.js b/src/plugins/system/www/plugin.js index 8bebd47ef..f1e30c96d 100644 --- a/src/plugins/system/www/plugin.js +++ b/src/plugins/system/www/plugin.js @@ -272,5 +272,44 @@ module.exports = { [text1, text2] ); }); + }, + /** + * Store a secret encrypted at rest (AES256-GCM, hardware-backed key). + * Use for credentials that must not sit in cleartext in the WebView's + * localStorage. Passing an empty/undefined value clears the key. + * @param {string} key + * @param {string} value + * @returns {Promise} + */ + secureSet: function (key, value) { + return new Promise((resolve, reject) => { + cordova.exec(resolve, reject, 'System', 'secure-set', [key, value == null ? null : String(value)]); + }); + }, + /** + * Read a secret previously stored with secureSet. + * @param {string} key + * @returns {Promise} the value, or null if absent + */ + secureGet: function (key) { + return new Promise((resolve, reject) => { + cordova.exec( + function (result) { resolve(result == null || result === '' ? null : result); }, + reject, + 'System', + 'secure-get', + [key] + ); + }); + }, + /** + * Remove a stored secret. + * @param {string} key + * @returns {Promise} + */ + secureRemove: function (key) { + return new Promise((resolve, reject) => { + cordova.exec(resolve, reject, 'System', 'secure-remove', [key]); + }); } }; diff --git a/src/utils/helpers.js b/src/utils/helpers.js index d2b97f532..8f899a234 100644 --- a/src/utils/helpers.js +++ b/src/utils/helpers.js @@ -4,6 +4,7 @@ import alert from "dialogs/alert"; import escapeStringRegexp from "escape-string-regexp"; import adRewards from "lib/adRewards"; import config from "lib/config"; +import secureCredentials from "lib/secureCredentials"; import { interstitialAd, requestBannerForPage } from "lib/startAd"; import { isBinaryFile } from "./binaryExtensions"; import { isPlayStoreInstall } from "./installSource"; @@ -249,16 +250,21 @@ export default { if (!Array.isArray(storageList)) return url; const storageListLen = storageList.length; + // Compare with the password stripped from both sides: URLs saved before + // the credentials migration still contain `user:pass@` (#2561). + const bareUrl = secureCredentials.stripPassword(url); + for (let i = 0; i < storageListLen; ++i) { const uuid = storageList[i]; let storageUrl = Url.parse(uuid.uri || uuid.url || "").url; if (!storageUrl) continue; + storageUrl = secureCredentials.stripPassword(storageUrl); if (storageUrl.endsWith("/")) { storageUrl = storageUrl.slice(0, -1); } const regex = new RegExp("^" + escapeStringRegexp(storageUrl)); - if (regex.test(url)) { - url = url.replace(regex, uuid.name); + if (regex.test(bareUrl)) { + url = bareUrl.replace(regex, uuid.name); break; } }