FXCM-2280: Implement Migration for Encrypted Autofill Storage

CHANGELOG.md

@@ -2,11 +2,18 @@
 
 [Full Changelog](In progress)
 
+## ⚠️ Breaking Changes ⚠️
+
+### Autofill
+
+- **BREAKING**: `Store::new()` now takes an `EncryptorDecryptor`, which the store hands to the database and which is used for every encrypted column, and `scrub_undecryptable_credit_card_data_for_remote_replacement()` no longer takes an encryption key. Credit-card encryption moved to the shared `db-crypto` crate. `encrypt_string()` and `decrypt_string()` are unchanged.
+
 ## ✨ What's Changed ✨
 
 ### Autofill
 
 - `update_address()` now sets `time_last_modified` to the time of the update, matching `update_credit_card()` and `update_passport()`.
+- Credit-card numbers are now stored as a versioned JSON blob rather than the bare number, so a second encrypted field can be added later without rewriting every row again. `run_maintenance()` rewrites existing rows once, in place, without touching sync metadata. The schema version is bumped to 6, so opening the database with an older build now fails rather than reading a blob as a card number.
 
 ### Ads-Client
 

components/autofill/Cargo.toml

@@ -12,6 +12,7 @@ exclude = ["/android", "/ios"]
 anyhow = "1.0"
 error-support = { path = "../support/error" }
 interrupt-support = { path = "../support/interrupt" }
+db-crypto = { path = "../support/db-crypto" }
 jwcrypto = { path = "../support/jwcrypto" }
 lazy_static = "1.4"
 parking_lot = ">=0.11,<=0.12"

components/autofill/src/autofill.udl

@@ -164,9 +164,12 @@ interface AutofillApiError {
     UnexpectedAutofillApiError(string reason);
 };
 
+[External = "db_crypto"]
+typedef trait_with_foreign EncryptorDecryptor;
+
 interface Store {
     [Throws=AutofillApiError]
-    constructor(string dbpath);
+    constructor(string dbpath, EncryptorDecryptor encdec);
 
     [Throws=AutofillApiError]
     CreditCard add_credit_card(UpdatableCreditCardFields cc);
@@ -257,10 +260,13 @@ interface Store {
     /// NB: This function was created to unblock iOS credit card users who are unable to sync records and should not be used
     /// outside of this use case.
     [Throws=AutofillApiError, Self=ByArc]
-    CreditCardsDeletionMetrics scrub_undecryptable_credit_card_data_for_remote_replacement(string local_encryption_key);
+    CreditCardsDeletionMetrics scrub_undecryptable_credit_card_data_for_remote_replacement();
 
     /// Run maintenance on the DB
     ///
+    /// Also completes the one-off rewrite of credit-card rows that predate the
+    /// versioned secure-fields encoding. Cheap once that has finished.
+    ///
     /// This is intended to be run during idle time and will take steps / to clean up / shrink the
     /// database.
     [Throws=AutofillApiError]

components/autofill/src/db/credit_cards.rs

@@ -5,14 +5,14 @@
 
 use crate::db::{
     models::{
-        credit_card::{InternalCreditCard, UpdatableCreditCardFields},
+        credit_card::{InternalCreditCard, SecureCreditCardFields, UpdatableCreditCardFields},
         Metadata,
     },
     schema::{CREDIT_CARD_COMMON_COLS, CREDIT_CARD_COMMON_VALS},
+    AutofillDb,
 };
 use crate::error::*;
 
-use jwcrypto::EncryptorDecryptor;
 use rusqlite::{Connection, Transaction};
 use sync_guid::Guid;
 use types::Timestamp;
@@ -226,16 +226,22 @@ pub fn scrub_encrypted_credit_card_data(conn: &Connection) -> Result<()> {
 }
 
 pub fn scrub_undecryptable_credit_card_data_for_remote_replacement(
-    conn: &Connection,
-    local_encryption_key: String,
+    db: &AutofillDb,
 ) -> Result<CreditCardsDeletionMetrics> {
+    let conn = &db.writer;
     let tx = conn.unchecked_transaction()?;
     let mut scrubbed_records = 0;
-    let encdec = EncryptorDecryptor::new(local_encryption_key.as_str()).unwrap();
 
     let undecryptable_record_ids = get_all_credit_cards(conn)?
         .into_iter()
-        .filter(|credit_card| encdec.decrypt(&credit_card.cc_number_enc).is_err())
+        .filter(|credit_card| {
+            SecureCreditCardFields::decrypt(
+                &credit_card.cc_number_enc,
+                db.encdec.as_ref(),
+                credit_card.guid.as_str(),
+            )
+            .is_err()
+        })
         .map(|credit_card| credit_card.guid)
         .collect::<Vec<_>>();
 
@@ -290,7 +296,7 @@ pub fn touch(conn: &Connection, guid: &Guid) -> Result<()> {
 pub(crate) mod tests {
     use super::*;
     use crate::db::test::new_mem_db;
-    use crate::encryption::EncryptorDecryptor;
+    use crate::encryption::{encrypt_str, random_key_encryptor};
     use nss_as::ensure_initialized;
     use sync15::bso::IncomingBso;
 
@@ -588,13 +594,13 @@ pub(crate) mod tests {
     fn test_credit_card_delete() -> Result<()> {
         ensure_initialized();
         let db = new_mem_db();
-        let encdec = EncryptorDecryptor::new_with_random_key().unwrap();
+        let encdec = db.encdec.clone();
 
         let saved_credit_card = add_credit_card(
             &db,
             UpdatableCreditCardFields {
                 cc_name: "john deer".to_string(),
-                cc_number_enc: encdec.encrypt("1234567812345678")?,
+                cc_number_enc: encrypt_str(encdec.as_ref(), "1234567812345678")?,
                 cc_number_last_4: "5678".to_string(),
                 cc_exp_month: 10,
                 cc_exp_year: 2025,
@@ -610,7 +616,7 @@ pub(crate) mod tests {
             &db,
             UpdatableCreditCardFields {
                 cc_name: "john doe".to_string(),
-                cc_number_enc: encdec.encrypt("1234123412341234")?,
+                cc_number_enc: encrypt_str(encdec.as_ref(), "1234123412341234")?,
                 cc_number_last_4: "1234".to_string(),
                 cc_exp_month: 5,
                 cc_exp_year: 2024,
@@ -620,7 +626,8 @@ pub(crate) mod tests {
 
         // create a mirror record to check that a tombstone record is created upon deletion
         let cc2_guid = saved_credit_card2.guid.clone();
-        let payload = saved_credit_card2.into_test_incoming_bso(&encdec, Default::default());
+        let payload =
+            saved_credit_card2.into_test_incoming_bso(encdec.as_ref(), Default::default());
 
         test_insert_mirror_record(&db, payload);
 
@@ -656,14 +663,14 @@ pub(crate) mod tests {
     fn test_scrub_encrypted_credit_card_data() -> Result<()> {
         ensure_initialized();
         let db = new_mem_db();
-        let encdec = EncryptorDecryptor::new_with_random_key().unwrap();
+        let encdec = db.encdec.clone();
         let mut saved_credit_cards = Vec::with_capacity(10);
         for _ in 0..5 {
             saved_credit_cards.push(add_credit_card(
                 &db,
                 UpdatableCreditCardFields {
                     cc_name: "john deer".to_string(),
-                    cc_number_enc: encdec.encrypt("1234567812345678")?,
+                    cc_number_enc: encrypt_str(encdec.as_ref(), "1234567812345678")?,
                     cc_number_last_4: "5678".to_string(),
                     cc_exp_month: 10,
                     cc_exp_year: 2025,
@@ -682,27 +689,24 @@ pub(crate) mod tests {
     }
 
     #[test]
-    fn test_scrub_undecryptable_credit_card_date_for_remote_replacement() -> Result<()> {
+    fn test_scrub_undecryptable_credit_card_data_for_remote_replacement() -> Result<()> {
         ensure_initialized();
         let db = new_mem_db();
-        let old_key = EncryptorDecryptor::create_key()?;
-        let old_encdec = EncryptorDecryptor::new(&old_key)?;
-        let key = EncryptorDecryptor::create_key()?;
-        let encdec = EncryptorDecryptor::new(&key)?;
+        let foreign_encdec = random_key_encryptor()?;
 
         let undecryptable_credit_card = add_credit_card(
             &db,
             UpdatableCreditCardFields {
                 cc_name: "jane doe".to_string(),
-                cc_number_enc: old_encdec.encrypt("2345678923456789")?,
+                cc_number_enc: encrypt_str(&foreign_encdec, "2345678923456789")?,
                 cc_number_last_4: "6789".to_string(),
                 cc_exp_month: 9,
                 cc_exp_year: 2027,
                 cc_type: "visa".to_string(),
             },
         )?;
 
-        let encrypted_cc_number = encdec.encrypt("567812345678123456781")?;
+        let encrypted_cc_number = encrypt_str(db.encdec.as_ref(), "567812345678123456781")?;
         let credit_card = add_credit_card(
             &db,
             UpdatableCreditCardFields {
@@ -715,7 +719,7 @@ pub(crate) mod tests {
             },
         )?;
 
-        let metrics = scrub_undecryptable_credit_card_data_for_remote_replacement(&db.writer, key)?;
+        let metrics = scrub_undecryptable_credit_card_data_for_remote_replacement(&db)?;
         assert_eq!(metrics.total_scrubbed_records, 1);
 
         let credit_cards = get_all_credit_cards(&db)?;