diff --git a/README.md b/README.md index 416f215..177464b 100644 --- a/README.md +++ b/README.md @@ -134,7 +134,6 @@ UI design notes and the visual design system live in [docs/DESIGN.md](docs/DESIG ## Roadmap -- Encrypt stored WireGuard private keys (SQLCipher) - Certificate pinning for provider APIs - More providers (NordVPN connector exists but is disabled; Surfshark) - Play Store release diff --git a/SECURITY.md b/SECURITY.md index 67c581b..37edfa1 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -14,9 +14,9 @@ UniVPN is designed for a home TV on a trusted network. Know these before relying |------|------------| | **Switching** | No kill switch. Traffic can briefly leave unprotected while one tunnel goes down and the next comes up. | | **Device-wide tunnel** | Android allows one VPN at a time. The foreground app's route applies to all traffic on the device, including background apps. | -| **Key storage** | WireGuard configs, including private keys, are stored unencrypted in the app's private Room database. Provider passwords are stored separately using `EncryptedSharedPreferences`. A rooted device or a debuggable build exposes both. | +| **Key storage** | WireGuard configs, including private keys, are encrypted with AES-256-GCM before they are written to the app's private database; the key lives in the Android Keystore and can't be exported. Provider passwords are stored with `EncryptedSharedPreferences`. Code running as UniVPN (a debuggable build, or a rooted device) can still use the Keystore key while the device is unlocked. | | **Web import server** | While the **Profiles** screen is open, UniVPN serves plain HTTP on port 8080 with no authentication. Anyone on the same local network can upload a WireGuard config or submit provider credentials at that time, and credentials typed into it cross the LAN unencrypted. Close the Profiles screen when you're done importing. | | **Provider APIs** | Provider API calls trust the device's CA store without certificate pinning (on Android 6 that includes user-installed CAs). The PIA key-registration call is the exception: it trusts only PIA's own CA and checks the server hostname. | | **Logging** | The app logs tunnel and provider activity to logcat, including server addresses, in all build types. Anyone with ADB access to the device can read it. | -Fixes for the key storage and pinning items are on the roadmap. +Certificate pinning for provider APIs is on the roadmap. diff --git a/univpn/src/androidTest/kotlin/com/univpn/app/data/ConfigEncryptionTest.kt b/univpn/src/androidTest/kotlin/com/univpn/app/data/ConfigEncryptionTest.kt new file mode 100644 index 0000000..8817024 --- /dev/null +++ b/univpn/src/androidTest/kotlin/com/univpn/app/data/ConfigEncryptionTest.kt @@ -0,0 +1,113 @@ +package com.univpn.app.data + +import androidx.room.Room +import androidx.test.core.app.ApplicationProvider +import androidx.test.ext.junit.runners.AndroidJUnit4 +import com.univpn.app.data.db.AppDatabase +import com.univpn.app.data.db.ConfigCipher +import com.univpn.app.data.model.TunnelType +import com.univpn.app.data.model.VpnProfile +import kotlinx.coroutines.flow.first +import kotlinx.coroutines.runBlocking +import org.junit.After +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertNotEquals +import org.junit.Assert.assertTrue +import org.junit.Before +import org.junit.Test +import org.junit.runner.RunWith + +/** WireGuard configs (with private keys) are encrypted at rest (#13). */ +@RunWith(AndroidJUnit4::class) +class ConfigEncryptionTest { + + private lateinit var db: AppDatabase + private val config = "[Interface]\nPrivateKey = yAnz5TF+lXXJte14tji3zlMNq+hd2rYUIgJBgB3fBmk=\n\n[Peer]\nEndpoint = 192.0.2.1:51820\n" + + @Before + fun setUp() { + db = Room.inMemoryDatabaseBuilder(ApplicationProvider.getApplicationContext(), AppDatabase::class.java) + .allowMainThreadQueries() + .build() + } + + @After + fun tearDown() { db.close() } + + private fun storedConfig(id: String): String = + db.openHelper.readableDatabase.query("SELECT configContent FROM vpn_profiles WHERE id = ?", arrayOf(id)).use { + it.moveToFirst(); it.getString(0) + } + + @Test + fun cipher_roundTrips_andUsesFreshIvs() { + val a = ConfigCipher.encrypt(config) + val b = ConfigCipher.encrypt(config) + assertTrue(ConfigCipher.isEncrypted(a)) + assertNotEquals(a, b) + assertEquals(config, ConfigCipher.decrypt(a)) + } + + @Test + fun dao_storesCiphertext_andReturnsPlainConfig() = runBlocking { + db.vpnProfileDao().insert(VpnProfile("p1", "UK", TunnelType.WIREGUARD, config)) + + val stored = storedConfig("p1") + assertTrue(ConfigCipher.isEncrypted(stored)) + assertFalse(stored.contains("PrivateKey")) + assertEquals(config, db.vpnProfileDao().getById("p1")?.configContent) + assertEquals(config, db.vpnProfileDao().getAll().first().single().configContent) + + db.vpnProfileDao().update(VpnProfile("p1", "UK", TunnelType.WIREGUARD, config + "# edited\n")) + assertTrue(ConfigCipher.isEncrypted(storedConfig("p1"))) + assertEquals(config + "# edited\n", db.vpnProfileDao().getById("p1")?.configContent) + } + + @Test + fun legacyPlaintextRow_isStillReadable() = runBlocking { + db.openHelper.writableDatabase.execSQL( + "INSERT INTO vpn_profiles (id, name, type, configContent) VALUES (?, ?, ?, ?)", + arrayOf("old", "Old", "WIREGUARD", config) + ) + assertEquals(config, db.vpnProfileDao().getById("old")?.configContent) + } + + @Test + fun legacyEncryption_leavesNoPlaintextKeyInTheFile() = runBlocking { + val ctx = ApplicationProvider.getApplicationContext() + val name = "legacy-encryption-test.db" + ctx.deleteDatabase(name) + val file = ctx.getDatabasePath(name) + try { + // A pre-0.9.2 database in WAL mode with plaintext configs, some rows rewritten. + android.database.sqlite.SQLiteDatabase.openOrCreateDatabase(file, null).use { raw -> + raw.enableWriteAheadLogging() + raw.execSQL("CREATE TABLE vpn_profiles (id TEXT NOT NULL PRIMARY KEY, name TEXT NOT NULL, type TEXT NOT NULL, configContent TEXT NOT NULL)") + for (i in 1..20) raw.execSQL("INSERT INTO vpn_profiles VALUES ('p$i', 'P$i', 'WIREGUARD', ?)", arrayOf(config)) + raw.execSQL("UPDATE vpn_profiles SET configContent = configContent || '# edited\n'") + } + + assertEquals(20, com.univpn.app.data.db.LegacyConfigEncryption.run(file)) + assertEquals(0, com.univpn.app.data.db.LegacyConfigEncryption.run(file)) + + val bytes = listOf("", "-wal", "-journal").map { java.io.File(file.path + it) } + .filter { it.exists() }.flatMap { it.readBytes().toList() }.toByteArray() + assertFalse("plaintext key left on disk", String(bytes, Charsets.ISO_8859_1).contains("PrivateKey")) + + android.database.sqlite.SQLiteDatabase.openDatabase(file.path, null, 0).use { raw -> + raw.rawQuery("SELECT configContent FROM vpn_profiles WHERE id = 'p7'", null).use { + it.moveToFirst() + assertEquals(config + "# edited\n", ConfigCipher.decrypt(it.getString(0))) + } + } + } finally { + ctx.deleteDatabase(name) + } + } + + @Test + fun corruptCiphertext_decryptsToEmpty() { + assertEquals("", ConfigCipher.decrypt("enc:v1:AAAAAAAAAAAAAAAAAAAAAAAAAAAA")) + } +} diff --git a/univpn/src/main/kotlin/com/univpn/app/data/db/AppDatabase.kt b/univpn/src/main/kotlin/com/univpn/app/data/db/AppDatabase.kt index 78a8cfa..367b427 100644 --- a/univpn/src/main/kotlin/com/univpn/app/data/db/AppDatabase.kt +++ b/univpn/src/main/kotlin/com/univpn/app/data/db/AppDatabase.kt @@ -1,6 +1,7 @@ package com.univpn.app.data.db import android.content.Context +import android.util.Log import androidx.room.Database import androidx.room.Room import androidx.room.RoomDatabase @@ -69,14 +70,17 @@ abstract class AppDatabase : RoomDatabase() { internal val MIGRATIONS = arrayOf(MIGRATION_3_4, MIGRATION_4_5) fun get(context: Context): AppDatabase = instance ?: synchronized(this) { - instance ?: Room.databaseBuilder( - context.applicationContext, - AppDatabase::class.java, - "univpn.db" - ) - .addMigrations(*MIGRATIONS) - .build() - .also { instance = it } + instance ?: run { + val app = context.applicationContext + runCatching { LegacyConfigEncryption.run(app.getDatabasePath(NAME)) } + .onFailure { Log.e("AppDatabase", "Encrypting stored configs failed", it) } + Room.databaseBuilder(app, AppDatabase::class.java, NAME) + .addMigrations(*MIGRATIONS) + .build() + .also { instance = it } + } } + + private const val NAME = "univpn.db" } } diff --git a/univpn/src/main/kotlin/com/univpn/app/data/db/ConfigCipher.kt b/univpn/src/main/kotlin/com/univpn/app/data/db/ConfigCipher.kt new file mode 100644 index 0000000..d3f7cc1 --- /dev/null +++ b/univpn/src/main/kotlin/com/univpn/app/data/db/ConfigCipher.kt @@ -0,0 +1,67 @@ +package com.univpn.app.data.db + +import android.security.keystore.KeyGenParameterSpec +import android.security.keystore.KeyProperties +import android.util.Base64 +import android.util.Log +import java.security.KeyStore +import javax.crypto.Cipher +import javax.crypto.KeyGenerator +import javax.crypto.SecretKey +import javax.crypto.spec.GCMParameterSpec + +/** + * Encrypts WireGuard configs (which contain private keys) before they reach the database. + * The AES-256-GCM key lives in the Android Keystore and can't be read out of it. + * + * Stored format: "enc:v1:" + Base64(12-byte IV + ciphertext). Text without the prefix is a + * config saved before encryption existed and is returned as is. + */ +object ConfigCipher { + private const val TAG = "ConfigCipher" + private const val KEYSTORE = "AndroidKeyStore" + private const val KEY_ALIAS = "univpn_config_key" + private const val PREFIX = "enc:v1:" + private const val IV_BYTES = 12 + private const val TAG_BITS = 128 + + fun isEncrypted(stored: String) = stored.startsWith(PREFIX) + + fun encrypt(plain: String): String { + if (isEncrypted(plain)) return plain + val cipher = Cipher.getInstance("AES/GCM/NoPadding") + cipher.init(Cipher.ENCRYPT_MODE, key()) + val out = cipher.iv + cipher.doFinal(plain.toByteArray(Charsets.UTF_8)) + return PREFIX + Base64.encodeToString(out, Base64.NO_WRAP) + } + + /** Returns the plain config, or "" if it can't be decrypted (e.g. the Keystore key is gone). */ + fun decrypt(stored: String): String { + if (!isEncrypted(stored)) return stored + return try { + val data = Base64.decode(stored.substring(PREFIX.length), Base64.NO_WRAP) + val cipher = Cipher.getInstance("AES/GCM/NoPadding") + cipher.init(Cipher.DECRYPT_MODE, key(), GCMParameterSpec(TAG_BITS, data, 0, IV_BYTES)) + String(cipher.doFinal(data, IV_BYTES, data.size - IV_BYTES), Charsets.UTF_8) + } catch (e: Exception) { + Log.e(TAG, "Could not decrypt a stored config: ${e.javaClass.simpleName}") + "" + } + } + + @Synchronized + private fun key(): SecretKey { + val ks = KeyStore.getInstance(KEYSTORE).apply { load(null) } + (ks.getEntry(KEY_ALIAS, null) as? KeyStore.SecretKeyEntry)?.let { return it.secretKey } + return KeyGenerator.getInstance(KeyProperties.KEY_ALGORITHM_AES, KEYSTORE).run { + init( + KeyGenParameterSpec.Builder(KEY_ALIAS, KeyProperties.PURPOSE_ENCRYPT or KeyProperties.PURPOSE_DECRYPT) + .setBlockModes(KeyProperties.BLOCK_MODE_GCM) + .setEncryptionPaddings(KeyProperties.ENCRYPTION_PADDING_NONE) + .setKeySize(256) + .build() + ) + generateKey() + } + } +} diff --git a/univpn/src/main/kotlin/com/univpn/app/data/db/LegacyConfigEncryption.kt b/univpn/src/main/kotlin/com/univpn/app/data/db/LegacyConfigEncryption.kt new file mode 100644 index 0000000..15631be --- /dev/null +++ b/univpn/src/main/kotlin/com/univpn/app/data/db/LegacyConfigEncryption.kt @@ -0,0 +1,49 @@ +package com.univpn.app.data.db + +import android.database.sqlite.SQLiteDatabase +import android.util.Log +import java.io.File + +/** + * Encrypts configs that versions before 0.9.2 stored in plain text. Runs on the database file + * before Room opens it: with no other connections, the old rows can be overwritten + * (secure_delete), and the file rewritten (VACUUM), so no plaintext private key is left in + * freed pages or the WAL. + */ +object LegacyConfigEncryption { + private const val TAG = "LegacyConfigEncryption" + + /** Returns the number of configs encrypted. */ + fun run(dbFile: File): Int { + if (!dbFile.exists()) return 0 + // Opening without WAL checkpoints any existing WAL into the main file and removes it. + return SQLiteDatabase.openDatabase(dbFile.path, null, SQLiteDatabase.OPEN_READWRITE).use { db -> + val hasTable = db.rawQuery( + "SELECT 1 FROM sqlite_master WHERE type = 'table' AND name = 'vpn_profiles'", null + ).use { it.moveToFirst() } + if (!hasTable) return@use 0 + + val legacy = db.rawQuery( + "SELECT id, configContent FROM vpn_profiles WHERE configContent NOT LIKE 'enc:v1:%'", null + ).use { c -> buildList { while (c.moveToNext()) add(c.getString(0) to c.getString(1)) } } + if (legacy.isEmpty()) return@use 0 + + db.rawQuery("PRAGMA secure_delete = ON", null).use { it.moveToFirst() } + db.beginTransaction() + try { + for ((id, config) in legacy) { + db.execSQL( + "UPDATE vpn_profiles SET configContent = ? WHERE id = ?", + arrayOf(ConfigCipher.encrypt(config), id) + ) + } + db.setTransactionSuccessful() + } finally { + db.endTransaction() + } + db.execSQL("VACUUM") + Log.i(TAG, "Encrypted ${legacy.size} stored configs") + legacy.size + } + } +} diff --git a/univpn/src/main/kotlin/com/univpn/app/data/db/VpnProfileDao.kt b/univpn/src/main/kotlin/com/univpn/app/data/db/VpnProfileDao.kt index b1dcced..6db0015 100644 --- a/univpn/src/main/kotlin/com/univpn/app/data/db/VpnProfileDao.kt +++ b/univpn/src/main/kotlin/com/univpn/app/data/db/VpnProfileDao.kt @@ -3,22 +3,38 @@ package com.univpn.app.data.db import androidx.room.* import com.univpn.app.data.model.VpnProfile import kotlinx.coroutines.flow.Flow +import kotlinx.coroutines.flow.map +/** + * Profiles are encrypted on the way in and decrypted on the way out (see [ConfigCipher]), so + * callers only ever see plain configs and the database only ever holds encrypted ones. + */ @Dao -interface VpnProfileDao { +abstract class VpnProfileDao { @Query("SELECT * FROM vpn_profiles ORDER BY name") - fun getAll(): Flow> + protected abstract fun getAllStored(): Flow> @Query("SELECT * FROM vpn_profiles WHERE id = :id") - suspend fun getById(id: String): VpnProfile? + protected abstract suspend fun getByIdStored(id: String): VpnProfile? @Insert(onConflict = OnConflictStrategy.REPLACE) - suspend fun insert(profile: VpnProfile) + protected abstract suspend fun insertStored(profile: VpnProfile) - /** Returns the number of rows updated: 0 if the profile no longer exists. */ @Update - suspend fun update(profile: VpnProfile): Int + protected abstract suspend fun updateStored(profile: VpnProfile): Int + + fun getAll(): Flow> = getAllStored().map { list -> list.map(::open) } + + suspend fun getById(id: String): VpnProfile? = getByIdStored(id)?.let(::open) + + suspend fun insert(profile: VpnProfile) = insertStored(seal(profile)) + + /** Returns the number of rows updated: 0 if the profile no longer exists. */ + suspend fun update(profile: VpnProfile): Int = updateStored(seal(profile)) @Delete - suspend fun delete(profile: VpnProfile) + abstract suspend fun delete(profile: VpnProfile) + + private fun seal(p: VpnProfile) = p.copy(configContent = ConfigCipher.encrypt(p.configContent)) + private fun open(p: VpnProfile) = p.copy(configContent = ConfigCipher.decrypt(p.configContent)) }