Skip to content

Encrypt HybridFileBackedSqlStorage with Android Keystore#3646

Draft
avazirna wants to merge 8 commits intoencrypt-form-submissions-with-keystorefrom
encrypt-hybrid-storage-with-keystore
Draft

Encrypt HybridFileBackedSqlStorage with Android Keystore#3646
avazirna wants to merge 8 commits intoencrypt-form-submissions-with-keystorefrom
encrypt-hybrid-storage-with-keystore

Conversation

@avazirna
Copy link
Copy Markdown
Contributor

@avazirna avazirna commented Apr 3, 2026

Product Description

No user-facing changes. This is an internal security enhancement that extends Android Keystore encryption to the HybridFileBackedSqlStorage layer, which stores fixtures and other file-backed data.

Technical Summary

Extends the Keystore encryption work from encrypt-form-submissions-with-keystore to cover HybridFileBackedSqlStorage. New records are encrypted using the Android Keystore key (via CommCareKeyManager), while existing records encrypted with legacy per-record AES keys continue to be readable.

Key changes:

  • HybridFileBackedSqlStorage: branch read/write/output-stream paths for Keystore vs legacy encryption based on whether the stored AES key is empty
  • CommCareKeyManager: add @VisibleForTesting test override for session key to enable unit testing without a real Keystore
  • EncryptionIO: remove unused SecretKeySpec import
  • FormUploadUtil: remove dead getDecryptCipher methods (cipher creation now centralized in EncryptionIO)
  • Add HybridFileBackedSqlStorageKeystoreTest with round-trip write/read, new record write, and filesystem-to-DB migration tests
  • Pass Keystore encryption flag to performance tracing

Feature Flag

Gated by AndroidKeyStore.isKeystoreAvailable() — only activates on devices with hardware-backed Keystore support.

Safety Assurance

Safety story

  • Backward compatible: existing records with non-empty AES keys continue to use the legacy decryption path
  • The empty-key sentinel (aesKeyBytes == null || aesKeyBytes.length == 0) cleanly distinguishes Keystore-encrypted records from legacy ones without schema changes
  • Tested locally with Robolectric unit tests using a mock Keystore provider

Automated test coverage

  • HybridFileBackedSqlStorageKeystoreTest: 3 tests covering Keystore-encrypted write+read, new record creation, and filesystem↔DB migration round-trips

QA Plan

  • Install on a device with Keystore support, log in, and verify fixtures load correctly
  • Verify that previously saved data (legacy encrypted) is still readable after the update
  • Monitor TRACE_FILE_ENCRYPTION_TIME traces for Keystore vs legacy performance

Labels and Review

  • Do we need to enhance the manual QA test coverage ? If yes, the "QA Note" label is set correctly
  • Does the PR introduce any major changes worth communicating ? If yes, the "Release Note" label is set and a "Release Note" is specified in PR description.
  • Risk label is set correctly
  • The set of people pinged as reviewers is appropriate for the level of risk of the change

avazirna and others added 8 commits April 3, 2026 16:33
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 <noreply@anthropic.com>
Add usesKeystoreEncryption() helper and update getOutputFileStream()
to use Keystore encryption when AES key is empty.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
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 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant