From df3e8e09c6a5242fe4c3d6f91aa404d41f589443 Mon Sep 17 00:00:00 2001 From: Ahmad Vazirna Date: Fri, 3 Apr 2026 16:33:51 +0200 Subject: [PATCH 1/8] Decrypt saved forms when keystore encrypted --- .../org/commcare/activities/FormEntryActivity.java | 2 +- app/src/org/commcare/tasks/FormLoaderTask.java | 12 ++++++++++-- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/app/src/org/commcare/activities/FormEntryActivity.java b/app/src/org/commcare/activities/FormEntryActivity.java index 3fb8ebf243..fe546060d3 100644 --- a/app/src/org/commcare/activities/FormEntryActivity.java +++ b/app/src/org/commcare/activities/FormEntryActivity.java @@ -1068,7 +1068,7 @@ private void loadForm() { return; } - mFormLoaderTask = new FormLoaderTask(symetricKey, instanceIsReadOnly, + mFormLoaderTask = new FormLoaderTask(symetricKey, isKeyFromKeystore, instanceIsReadOnly, formEntryRestoreSession.isRecording(), FormEntryInstanceState.mFormRecordPath, this, savedFormSession) { @Override protected void deliverResult(FormEntryActivity receiver, FECWrapper wrapperResult) { diff --git a/app/src/org/commcare/tasks/FormLoaderTask.java b/app/src/org/commcare/tasks/FormLoaderTask.java index dab3f3397b..5671639bc4 100644 --- a/app/src/org/commcare/tasks/FormLoaderTask.java +++ b/app/src/org/commcare/tasks/FormLoaderTask.java @@ -19,6 +19,7 @@ import org.commcare.models.database.InterruptedFormState; import org.commcare.models.encryption.EncryptionIO; import org.commcare.preferences.DeveloperPreferences; +import org.commcare.services.CommCareKeyManager; import org.commcare.tasks.templates.CommCareTask; import org.commcare.util.LogTypes; import org.commcare.utils.FileUtil; @@ -67,6 +68,7 @@ public abstract class FormLoaderTask extends CommCareTask extends CommCareTask Date: Thu, 2 Apr 2026 17:08:30 +0200 Subject: [PATCH 2/8] Use Keystore key for new HybridFileBackedSqlStorage records Replace per-record derived AES key with CommCareKeyManager.generateLegacyKeyOrEmpty() so new large records use the Android Keystore key when available. Co-Authored-By: Claude Opus 4.6 --- .../commcare/models/database/HybridFileBackedSqlStorage.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/app/src/org/commcare/models/database/HybridFileBackedSqlStorage.java b/app/src/org/commcare/models/database/HybridFileBackedSqlStorage.java index 6ae654a5c7..6313adaeb2 100644 --- a/app/src/org/commcare/models/database/HybridFileBackedSqlStorage.java +++ b/app/src/org/commcare/models/database/HybridFileBackedSqlStorage.java @@ -8,6 +8,7 @@ import org.apache.commons.io.FilenameUtils; import org.commcare.CommCareApplication; import org.commcare.google.services.analytics.CCPerfMonitoring; +import org.commcare.services.CommCareKeyManager; import org.commcare.interfaces.AppFilePathBuilder; import org.commcare.models.encryption.EncryptionIO; import org.commcare.modern.database.DatabaseHelper; @@ -325,7 +326,7 @@ protected boolean blobFitsInDb(ByteArrayOutputStream blobStream) { } protected byte[] generateKeyAndAdd(ContentValues contentValues) { - byte[] key = CommCareApplication.instance().createNewSymmetricKey().getEncoded(); + byte[] key = CommCareKeyManager.generateLegacyKeyOrEmpty(); contentValues.put(DatabaseHelper.AES_COL, key); return key; } From 32b359aa9762f2c0183538c96e7bcef006052f99 Mon Sep 17 00:00:00 2001 From: Ahmad Vazirna Date: Thu, 2 Apr 2026 17:13:43 +0200 Subject: [PATCH 3/8] Branch HybridFileBackedSqlStorage read/write for Keystore encryption Add usesKeystoreEncryption() helper and update getOutputFileStream() to use Keystore encryption when AES key is empty. Co-Authored-By: Claude Opus 4.6 --- .../models/database/HybridFileBackedSqlStorage.java | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/app/src/org/commcare/models/database/HybridFileBackedSqlStorage.java b/app/src/org/commcare/models/database/HybridFileBackedSqlStorage.java index 6313adaeb2..b61a248e9f 100644 --- a/app/src/org/commcare/models/database/HybridFileBackedSqlStorage.java +++ b/app/src/org/commcare/models/database/HybridFileBackedSqlStorage.java @@ -331,6 +331,10 @@ protected byte[] generateKeyAndAdd(ContentValues contentValues) { return key; } + protected static boolean usesKeystoreEncryption(byte[] aesKeyBytes) { + return aesKeyBytes == null || aesKeyBytes.length == 0; + } + private void writeStreamToFile(ByteArrayOutputStream bos, String filename, byte[] key) throws IOException { Trace trace = CCPerfMonitoring.INSTANCE.startTracing(CCPerfMonitoring.TRACE_FILE_ENCRYPTION_TIME); @@ -358,8 +362,13 @@ private void writeStreamToFile(ByteArrayOutputStream bos, String filename, protected DataOutputStream getOutputFileStream(String filename, byte[] aesKeyBytes) throws IOException { - SecretKeySpec aesKey = new SecretKeySpec(aesKeyBytes, "AES"); - return new DataOutputStream(EncryptionIO.createFileOutputStream(filename, aesKey)); + if (usesKeystoreEncryption(aesKeyBytes)) { + return new DataOutputStream(EncryptionIO.createFileOutputStreamWithKeystore( + filename, CommCareKeyManager.retrieveSessionKeyAndTransformation())); + } else { + SecretKeySpec aesKey = new SecretKeySpec(aesKeyBytes, "AES"); + return new DataOutputStream(EncryptionIO.createFileOutputStream(filename, aesKey)); + } } @Override From 3491e9a6d34175ff6e9b08f635f431b38fa98fbe Mon Sep 17 00:00:00 2001 From: Ahmad Vazirna Date: Thu, 2 Apr 2026 17:16:12 +0200 Subject: [PATCH 4/8] Branch getInputStreamFromFile() for Keystore decryption Co-Authored-By: Claude Opus 4.6 --- .../models/database/HybridFileBackedSqlStorage.java | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/app/src/org/commcare/models/database/HybridFileBackedSqlStorage.java b/app/src/org/commcare/models/database/HybridFileBackedSqlStorage.java index b61a248e9f..cf46f7d27e 100644 --- a/app/src/org/commcare/models/database/HybridFileBackedSqlStorage.java +++ b/app/src/org/commcare/models/database/HybridFileBackedSqlStorage.java @@ -166,8 +166,13 @@ private IDatabase getDbOrThrow() { } protected InputStream getInputStreamFromFile(String filename, byte[] aesKeyBytes) throws FileNotFoundException { - SecretKeySpec aesKey = new SecretKeySpec(aesKeyBytes, "AES"); - return EncryptionIO.getFileInputStream(filename, aesKey, null, false); + if (usesKeystoreEncryption(aesKeyBytes)) { + return EncryptionIO.getFileInputStreamWithKeystore( + filename, CommCareKeyManager.retrieveSessionKeyAndTransformation()); + } else { + SecretKeySpec aesKey = new SecretKeySpec(aesKeyBytes, "AES"); + return EncryptionIO.getFileInputStream(filename, aesKey, null, false); + } } @Override From 32a5d490138a9c50338f932eee3519a4521b5253 Mon Sep 17 00:00:00 2001 From: Ahmad Vazirna Date: Thu, 2 Apr 2026 22:00:46 +0200 Subject: [PATCH 5/8] Pass Keystore encryption flag to file encryption tracing Co-Authored-By: Claude Opus 4.6 --- .../models/database/HybridFileBackedSqlStorage.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/app/src/org/commcare/models/database/HybridFileBackedSqlStorage.java b/app/src/org/commcare/models/database/HybridFileBackedSqlStorage.java index cf46f7d27e..b58599e9f3 100644 --- a/app/src/org/commcare/models/database/HybridFileBackedSqlStorage.java +++ b/app/src/org/commcare/models/database/HybridFileBackedSqlStorage.java @@ -341,12 +341,12 @@ protected static boolean usesKeystoreEncryption(byte[] aesKeyBytes) { } private void writeStreamToFile(ByteArrayOutputStream bos, String filename, - byte[] key) throws IOException { + byte[] aesKeyBytes) throws IOException { Trace trace = CCPerfMonitoring.INSTANCE.startTracing(CCPerfMonitoring.TRACE_FILE_ENCRYPTION_TIME); DataOutputStream fileOutputStream = null; try { - fileOutputStream = getOutputFileStream(filename, key); + fileOutputStream = getOutputFileStream(filename, aesKeyBytes); bos.writeTo(fileOutputStream); } finally { if (fileOutputStream != null) { @@ -360,7 +360,7 @@ private void writeStreamToFile(ByteArrayOutputStream bos, String filename, trace, bos.size(), FilenameUtils.getExtension(filename), - false + usesKeystoreEncryption(aesKeyBytes) ); } } From 6e0bf82ff300685e9a7cf2796275cb5c3baa0f63 Mon Sep 17 00:00:00 2001 From: Ahmad Vazirna Date: Thu, 2 Apr 2026 22:04:33 +0200 Subject: [PATCH 6/8] Add unit tests for HybridFileBackedSqlStorage Keystore encryption Test write, read, and filesystem-to-database migration with the mock Keystore provider registered to exercise the Keystore path. Co-Authored-By: Claude Opus 4.6 --- ...ybridFileBackedSqlStorageKeystoreTest.java | 127 ++++++++++++++++++ 1 file changed, 127 insertions(+) create mode 100644 app/unit-tests/src/org/commcare/models/database/HybridFileBackedSqlStorageKeystoreTest.java diff --git a/app/unit-tests/src/org/commcare/models/database/HybridFileBackedSqlStorageKeystoreTest.java b/app/unit-tests/src/org/commcare/models/database/HybridFileBackedSqlStorageKeystoreTest.java new file mode 100644 index 0000000000..cf35353e0f --- /dev/null +++ b/app/unit-tests/src/org/commcare/models/database/HybridFileBackedSqlStorageKeystoreTest.java @@ -0,0 +1,127 @@ +package org.commcare.models.database; + +import androidx.test.ext.junit.runners.AndroidJUnit4; + +import org.commcare.CommCareApplication; +import org.commcare.CommCareTestApplication; +import org.commcare.utils.MockAndroidKeyStoreProvider; +import org.javarosa.core.model.instance.FormInstance; +import org.junit.After; +import org.junit.Assert; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.robolectric.annotation.Config; + +import java.io.File; + +/** + * Tests for HybridFileBackedSqlStorage using Android Keystore encryption. + * Registers a mock Keystore provider so that generateLegacyKeyOrEmpty() + * returns an empty key, triggering the Keystore encryption path. + */ +@Config(application = CommCareTestApplication.class) +@RunWith(AndroidJUnit4.class) +public class HybridFileBackedSqlStorageKeystoreTest { + + @Before + public void setup() { + MockAndroidKeyStoreProvider.registerProvider(); + UnencryptedHybridFileBackedSqlStorageMock.alwaysPutInFilesystem(); + HybridFileBackedSqlStorageMock.alwaysPutInFilesystem(); + + StoreFixturesOnFilesystemTests.installAppWithFixtureData(this.getClass(), "odk_level_ipm_restore.xml"); + } + + @After + public void tearDown() { + MockAndroidKeyStoreProvider.deregisterProvider(); + } + + @Test + public void testKeystoreEncryptedWriteAndRead() { + HybridFileBackedSqlStorage userFixtureStorage = + CommCareApplication.instance().getFileBackedUserStorage( + HybridFileBackedSqlStorage.FIXTURE_STORAGE_TABLE_NAME, FormInstance.class); + + FormInstance form = userFixtureStorage.getRecordForValues( + new String[]{FormInstance.META_ID}, + new String[]{"commtrack:programs"}); + + String updatedName = "keystore_encrypted_fixture"; + form.setName(updatedName); + userFixtureStorage.update(form.getID(), form); + + FormInstance readBack = userFixtureStorage.getRecordForValues( + new String[]{FormInstance.META_ID}, + new String[]{"commtrack:programs"}); + Assert.assertEquals("Fixture should be readable after Keystore-encrypted write", + updatedName, readBack.getName()); + } + + @Test + public void testKeystoreEncryptedNewWrite() { + HybridFileBackedSqlStorage userFixtureStorage = + CommCareApplication.instance().getFileBackedUserStorage( + HybridFileBackedSqlStorage.FIXTURE_STORAGE_TABLE_NAME, FormInstance.class); + + FormInstance form = userFixtureStorage.getRecordForValues( + new String[]{FormInstance.META_ID}, + new String[]{"commtrack:programs"}); + form.setID(-1); + form.initialize(null, "keystore-new-fixture"); + userFixtureStorage.write(form); + + FormInstance readBack = userFixtureStorage.getRecordForValues( + new String[]{FormInstance.META_ID}, + new String[]{"keystore-new-fixture"}); + Assert.assertNotNull("New Keystore-encrypted fixture should be readable", readBack); + } + + @Test + public void testMoveKeystoreEncryptedFixtureFromFsToDbAndBack() { + HybridFileBackedSqlStorageMock.alwaysPutInDatabase(); + + HybridFileBackedSqlStorage userFixtureStorage = + CommCareApplication.instance().getFileBackedUserStorage( + HybridFileBackedSqlStorage.FIXTURE_STORAGE_TABLE_NAME, FormInstance.class); + FormInstance form = userFixtureStorage.getRecordForValues( + new String[]{FormInstance.META_ID}, + new String[]{"commtrack:programs"}); + + File dbDir = userFixtureStorage.getDbDirForTesting(); + int fileCountBefore = dbDir.listFiles().length; + + // move fixture from filesystem to database + String newName = "keystore_fixture_in_db"; + form.setName(newName); + userFixtureStorage.update(form.getID(), form); + + // ensure the data can still be read + form = userFixtureStorage.getRecordForValues( + new String[]{FormInstance.META_ID}, + new String[]{"commtrack:programs"}); + Assert.assertEquals(newName, form.getName()); + + // ensure the old file was removed + HybridFileBackedSqlHelpers.removeOrphanedFiles( + CommCareApplication.instance().getUserDbHandle()); + int fileCountAfter = dbDir.listFiles().length; + Assert.assertEquals(fileCountBefore - 1, fileCountAfter); + + // move fixture back into filesystem (now with Keystore encryption) + HybridFileBackedSqlStorageMock.alwaysPutInFilesystem(); + newName = "keystore_fixture_back_in_fs"; + form.setName(newName); + userFixtureStorage.update(form.getID(), form); + + // ensure the data can still be read + form = userFixtureStorage.getRecordForValues( + new String[]{FormInstance.META_ID}, + new String[]{"commtrack:programs"}); + Assert.assertEquals(newName, form.getName()); + + fileCountAfter = dbDir.listFiles().length; + Assert.assertEquals(fileCountBefore, fileCountAfter); + } +} \ No newline at end of file From df184754ea5ada4d7dcba3904a63cbb4c6a816a6 Mon Sep 17 00:00:00 2001 From: Ahmad Vazirna Date: Thu, 2 Apr 2026 23:38:52 +0200 Subject: [PATCH 7/8] Add test override for CommCareKeyManager session key --- .../commcare/services/CommCareKeyManager.kt | 25 ++++++++++++++++++- ...ybridFileBackedSqlStorageKeystoreTest.java | 5 ++++ 2 files changed, 29 insertions(+), 1 deletion(-) diff --git a/app/src/org/commcare/services/CommCareKeyManager.kt b/app/src/org/commcare/services/CommCareKeyManager.kt index 1ec93be8e9..208a8e8631 100644 --- a/app/src/org/commcare/services/CommCareKeyManager.kt +++ b/app/src/org/commcare/services/CommCareKeyManager.kt @@ -1,5 +1,6 @@ package org.commcare.services +import androidx.annotation.VisibleForTesting import org.commcare.CommCareApplication import org.commcare.android.security.AesKeyStoreHandler import org.commcare.android.security.AndroidKeyStore @@ -17,7 +18,8 @@ class CommCareKeyManager { } @JvmStatic - fun retrieveSessionKeyAndTransformation(): EncryptionKeyAndTransform = sessionKeyAndTransformation + fun retrieveSessionKeyAndTransformation(): EncryptionKeyAndTransform = + testKeyAndTransformation ?: sessionKeyAndTransformation /** * An empty array indicates that the Android Keystore is supported and the key should be retrieved @@ -30,5 +32,26 @@ class CommCareKeyManager { } else { CommCareApplication.instance().createNewSymmetricKey().encoded } + + /** + * For testing purposes only + */ + @VisibleForTesting + private var testKeyAndTransformation: EncryptionKeyAndTransform? = null + + /** + * Set a test override for the session key. When set, retrieveSessionKeyAndTransformation() + * returns this value instead of going through AesKeyStoreHandler. + */ + @JvmStatic + fun setTestKeyAndTransformation(encryptionKeyAndTransform: EncryptionKeyAndTransform?) { + testKeyAndTransformation = encryptionKeyAndTransform + } + + @JvmStatic + fun clearTestKeyAndTransformation() { + testKeyAndTransformation = null + } + } } diff --git a/app/unit-tests/src/org/commcare/models/database/HybridFileBackedSqlStorageKeystoreTest.java b/app/unit-tests/src/org/commcare/models/database/HybridFileBackedSqlStorageKeystoreTest.java index cf35353e0f..d14b04ebf6 100644 --- a/app/unit-tests/src/org/commcare/models/database/HybridFileBackedSqlStorageKeystoreTest.java +++ b/app/unit-tests/src/org/commcare/models/database/HybridFileBackedSqlStorageKeystoreTest.java @@ -4,7 +4,9 @@ import org.commcare.CommCareApplication; import org.commcare.CommCareTestApplication; +import org.commcare.services.CommCareKeyManager; import org.commcare.utils.MockAndroidKeyStoreProvider; +import org.commcare.utils.MockEncryptionKeyProvider; import org.javarosa.core.model.instance.FormInstance; import org.junit.After; import org.junit.Assert; @@ -27,6 +29,8 @@ public class HybridFileBackedSqlStorageKeystoreTest { @Before public void setup() { MockAndroidKeyStoreProvider.registerProvider(); + MockEncryptionKeyProvider provider = new MockEncryptionKeyProvider(CommCareApplication.instance()); + CommCareKeyManager.setTestKeyAndTransformation(provider.getKeyForEncryption()); UnencryptedHybridFileBackedSqlStorageMock.alwaysPutInFilesystem(); HybridFileBackedSqlStorageMock.alwaysPutInFilesystem(); @@ -36,6 +40,7 @@ public void setup() { @After public void tearDown() { MockAndroidKeyStoreProvider.deregisterProvider(); + CommCareKeyManager.clearTestKeyAndTransformation(); } @Test From 75246b43bd50745819ac622d54fdf798501829f1 Mon Sep 17 00:00:00 2001 From: Ahmad Vazirna Date: Fri, 3 Apr 2026 16:50:48 +0200 Subject: [PATCH 8/8] Lint --- .../models/encryption/EncryptionIO.java | 1 - .../org/commcare/utils/FormUploadUtil.java | 23 ------------------- 2 files changed, 24 deletions(-) diff --git a/app/src/org/commcare/models/encryption/EncryptionIO.java b/app/src/org/commcare/models/encryption/EncryptionIO.java index 86156c7ca0..65689e9b22 100644 --- a/app/src/org/commcare/models/encryption/EncryptionIO.java +++ b/app/src/org/commcare/models/encryption/EncryptionIO.java @@ -29,7 +29,6 @@ import javax.crypto.CipherOutputStream; import javax.crypto.NoSuchPaddingException; import javax.crypto.spec.IvParameterSpec; -import javax.crypto.spec.SecretKeySpec; /** * Methods for dealing with encrypted input/output. diff --git a/app/src/org/commcare/utils/FormUploadUtil.java b/app/src/org/commcare/utils/FormUploadUtil.java index 5016a4037a..b821193387 100644 --- a/app/src/org/commcare/utils/FormUploadUtil.java +++ b/app/src/org/commcare/utils/FormUploadUtil.java @@ -69,29 +69,6 @@ public class FormUploadUtil { ".m4v", ".mpg", ".mpeg", ".qcp", ".ogg", ".pdf", ".html", ".rtf", ".txt", ".docx", ".xlsx", ".msg"}; - public static Cipher getDecryptCipher(Key key) { - return getDecryptCipher(key, null, null); - } - - public static Cipher getDecryptCipher(Key key, String transformation, byte[] iv) { - Cipher cipher; - try { - cipher = Cipher.getInstance(Objects.requireNonNullElse(transformation, "AES")); - if (iv != null) { - cipher.init(Cipher.DECRYPT_MODE, key, new IvParameterSpec(iv)); - } else { - cipher.init(Cipher.DECRYPT_MODE, key); - } - - return cipher; - //TODO: Something smart here; - } catch (NoSuchAlgorithmException | - NoSuchPaddingException | InvalidKeyException | InvalidAlgorithmParameterException e) { - Logger.exception("Failed to initialize decryption cipher ", e); - } - return null; - } - /** * Send unencrypted data to the server without user facing progress * reporting.