Remove insecure AES/CBC/PKCS5Padding encryption

[franco-zalamena-iterable]

Summary

• Remove insecure AES/CBC/PKCS5Padding legacy encryption path
• Migrate encrypted data to use AES/GCM/NoPadding exclusively
• Maintain backward compatibility with one-time migration from CBC to GCM

Test plan

• Verify new data is encrypted with GCM
• Verify legacy CBC data is decrypted and re-encrypted with GCM
• Run existing encryption unit tests

🤖 Generated with Claude Code

[franco-zalamena-iterable]

PR Analysis

Problem: The SDK uses AES/CBC/PKCS5Padding for encryption on older API levels, which is susceptible to Padding Oracle Attacks. The CBC+PKCS padding combination allows byte-by-byte decryption via side-channel responses if error behavior is distinguishable.

Ideal fix plan:

• Stop encrypting new data with AES/CBC entirely — use AES/GCM/NoPadding exclusively for all new writes
• Keep CBC decryption path as read-only for backward compatibility so existing encrypted data can be migrated
• On decrypt of legacy CBC data, let the normal save/re-encrypt cycle re-persist it under GCM
• Remove CBC from the KeyStore key spec so new keys are not authorized for CBC at all
• Mark all CBC-related code as deprecated with clear documentation explaining it is migration-only
• Remove legacy API-level branching if the SDK's current minSdk already requires GCM support
• Update tests to reflect that encryption is always GCM, and add a test for the CBC-to-GCM migration path

What the PR did:

• Renamed constants (TRANSFORMATION_MODERN -> TRANSFORMATION_GCM, TRANSFORMATION_LEGACY -> TRANSFORMATION_LEGACY_CBC) with @Deprecated annotations
• Removed encryptLegacy — all new encryption goes through encryptGcm (formerly encryptModern)
• Kept decryptLegacyCbc as a deprecated, read-only migration path with a log warning
• Removed CBC and PKCS7 from the KeyStore KeyGenParameterSpec (.setBlockModes / .setEncryptionPaddings now only list GCM/None)
• Removed all Build.VERSION.SDK_INT < KITKAT branching and the legacy-device fail-fast check
• Removed generateIV helper (GCM IV is now generated by Cipher.init)
• Hardcoded the combined[0] flag to 1 (always GCM) for new encryptions
• Removed legacy-focused tests (cross-API-level, legacy encrypt/decrypt, manipulated version flag) and replaced with a single testAlwaysUsesGcmEncryption test

Assessment:

• Root cause identified: yes — CBC with PKCS padding is the vulnerability; the fix eliminates it from the write path
• Fix correctness: correct — new data is always GCM, CBC is retained read-only for migration, KeyStore no longer authorizes CBC for new keys
• Missed:
  • No dedicated migration round-trip test. There is no test that encrypts data with the old CBC path, then verifies the new code can decrypt it and re-encrypt under GCM. The existing testAlwaysUsesGcmEncryption only tests the happy path. A test that fabricates a CBC-encrypted blob (flag=0, CBC IV, CBC ciphertext) and asserts successful decryption would give confidence the migration path actually works.
  • Existing keys already authorized for CBC are not rotated. The KeyGenParameterSpec change only affects newly generated keys. Devices that already have a key in the AndroidKeyStore with CBC authorization will keep it. This is acceptable for a migration strategy (the key still works for GCM), but worth noting — a key rotation strategy could fully close the door.
  • No explicit minSdk guard. The PR removes all API-level checks, implicitly assuming GCM is always available. This is fine if minSdk >= 19, but there is no compile-time or runtime assertion enforcing that. If the SDK ever lowers its minSdk, CBC would silently break.
• Wrong assessment: none — the approach is sound
• Tests: needed but partially missing — the legacy-to-GCM migration path is the most critical scenario and has no dedicated test coverage