From cbfbb1759d83ae04e7d8f46b43d1a079c8060145 Mon Sep 17 00:00:00 2001 From: theidkamp Date: Wed, 2 Sep 2026 14:18:29 +0200 Subject: [PATCH] FXCM-2281: Integrate EncryptorDecryptor into the autofill database Move autofill's credit-card encryption onto the shared db-crypto crate and let AutofillDb own the encryptor, as logins' LoginDb does. The consumer supplies it when building the store, so no key is passed into individual calls or down through the sync layers. --- CHANGELOG.md | 6 ++ Cargo.lock | 1 + components/autofill/Cargo.toml | 1 + components/autofill/src/autofill.udl | 7 +- components/autofill/src/db/credit_cards.rs | 38 ++++----- components/autofill/src/db/mod.rs | 24 ++++-- components/autofill/src/db/schema.rs | 3 +- components/autofill/src/db/store.rs | 38 ++++----- components/autofill/src/encryption.rs | 83 ++++++++++++++----- components/autofill/src/error.rs | 18 ++-- components/autofill/src/lib.rs | 1 + components/autofill/src/sync/address/mod.rs | 7 +- components/autofill/src/sync/bridge.rs | 9 +- .../autofill/src/sync/credit_card/incoming.rs | 71 ++++++++-------- .../autofill/src/sync/credit_card/mod.rs | 36 ++++---- .../autofill/src/sync/credit_card/outgoing.rs | 45 +++++----- components/autofill/src/sync/engine.rs | 34 +++----- 17 files changed, 245 insertions(+), 177 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ffce80ff4d3..23d8b3af5fd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,12 @@ [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 diff --git a/Cargo.lock b/Cargo.lock index 625905a7e70..1845cbe5235 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -208,6 +208,7 @@ name = "autofill" version = "0.1.0" dependencies = [ "anyhow", + "db-crypto", "error-support", "interrupt-support", "jwcrypto", diff --git a/components/autofill/Cargo.toml b/components/autofill/Cargo.toml index ed0f3199966..6d52686307b 100644 --- a/components/autofill/Cargo.toml +++ b/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" diff --git a/components/autofill/src/autofill.udl b/components/autofill/src/autofill.udl index 7fc40bace05..ab5471fba64 100644 --- a/components/autofill/src/autofill.udl +++ b/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,7 +260,7 @@ 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 /// diff --git a/components/autofill/src/db/credit_cards.rs b/components/autofill/src/db/credit_cards.rs index 3499b646621..ab3902bd5a4 100644 --- a/components/autofill/src/db/credit_cards.rs +++ b/components/autofill/src/db/credit_cards.rs @@ -9,10 +9,11 @@ use crate::db::{ Metadata, }, schema::{CREDIT_CARD_COMMON_COLS, CREDIT_CARD_COMMON_VALS}, + AutofillDb, }; +use crate::encryption::decrypt_str; use crate::error::*; -use jwcrypto::EncryptorDecryptor; use rusqlite::{Connection, Transaction}; use sync_guid::Guid; use types::Timestamp; @@ -226,16 +227,15 @@ 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 { + 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| decrypt_str(db.encdec.as_ref(), &credit_card.cc_number_enc).is_err()) .map(|credit_card| credit_card.guid) .collect::>(); @@ -290,7 +290,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 +588,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 +610,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 +620,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 +657,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,19 +683,16 @@ 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, @@ -702,7 +700,7 @@ pub(crate) mod tests { }, )?; - 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 +713,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)?; diff --git a/components/autofill/src/db/mod.rs b/components/autofill/src/db/mod.rs index cfb5e35500f..5ba5ac88f26 100644 --- a/components/autofill/src/db/mod.rs +++ b/components/autofill/src/db/mod.rs @@ -9,6 +9,7 @@ pub mod passports; pub mod schema; pub mod store; +use crate::encryption::EncryptorDecryptor; use crate::error::*; use error_support::error; @@ -24,21 +25,22 @@ use url::Url; pub struct AutofillDb { pub writer: Connection, + pub encdec: Arc, interrupt_handle: Arc, } impl AutofillDb { - pub fn new(db_path: impl AsRef) -> Result { + pub fn new(db_path: impl AsRef, encdec: Arc) -> Result { let db_path = normalize_path(db_path)?; - Self::new_named(db_path) + Self::new_named(db_path, encdec) } - pub fn new_memory(db_path: &str) -> Result { + pub fn new_memory(db_path: &str, encdec: Arc) -> Result { let name = PathBuf::from(format!("file:{}?mode=memory&cache=shared", db_path)); - Self::new_named(name) + Self::new_named(name, encdec) } - fn new_named(db_path: PathBuf) -> Result { + fn new_named(db_path: PathBuf, encdec: Arc) -> Result { // We always create the read-write connection for an initial open so // we can create the schema and/or do version upgrades. let flags = OpenFlags::SQLITE_OPEN_NO_MUTEX @@ -55,6 +57,7 @@ impl AutofillDb { Ok(Self { interrupt_handle: Arc::new(SqlInterruptHandle::new(&conn)), writer: conn, + encdec, }) } @@ -152,10 +155,19 @@ pub mod test { // A helper for our tests to get their own memory Api. static ATOMIC_COUNTER: AtomicUsize = AtomicUsize::new(0); + pub fn test_encdec() -> Arc { + nss_as::ensure_initialized(); + Arc::new(crate::encryption::random_key_encryptor().expect("should get a key")) + } + pub fn new_mem_db() -> AutofillDb { + new_mem_db_with_encdec(test_encdec()) + } + + pub fn new_mem_db_with_encdec(encdec: Arc) -> AutofillDb { error_support::init_for_tests(); let counter = ATOMIC_COUNTER.fetch_add(1, Ordering::Relaxed); - AutofillDb::new_memory(&format!("test_autofill-api-{}", counter)) + AutofillDb::new_memory(&format!("test_autofill-api-{}", counter), encdec) .expect("should get an API") } } diff --git a/components/autofill/src/db/schema.rs b/components/autofill/src/db/schema.rs index cf2e0f2d9ce..85a32c7fa9c 100644 --- a/components/autofill/src/db/schema.rs +++ b/components/autofill/src/db/schema.rs @@ -306,7 +306,8 @@ mod tests { fn test_wal_size_is_bounded() { // A memory database has no -wal file, so open a real one. let db_file = MigratedDatabaseFile::new(AutofillConnectionInitializer, ""); - let db = AutofillDb::new(&db_file.path).expect("should open the database"); + let db = AutofillDb::new(&db_file.path, crate::db::test::test_encdec()) + .expect("should open the database"); let journal_mode: String = db .query_row("PRAGMA journal_mode", [], |row| row.get(0)) diff --git a/components/autofill/src/db/store.rs b/components/autofill/src/db/store.rs index 4bb7d1cd257..efb46c7874d 100644 --- a/components/autofill/src/db/store.rs +++ b/components/autofill/src/db/store.rs @@ -11,6 +11,7 @@ use crate::db::models::passport::{Passport, UpdatablePassportFields}; use crate::db::{ addresses, credit_cards, credit_cards::CreditCardsDeletionMetrics, passports, AutofillDb, }; +use crate::encryption::EncryptorDecryptor; use crate::error::*; use error_support::handle_error; use parking_lot::{MappedMutexGuard, Mutex, MutexGuard}; @@ -58,9 +59,9 @@ pub struct Store { impl Store { #[handle_error(Error)] - pub fn new(db_path: impl AsRef) -> ApiResult { + pub fn new(db_path: impl AsRef, encdec: Arc) -> ApiResult { Ok(Self { - db: Mutex::new(Some(AutofillDb::new(db_path)?)), + db: Mutex::new(Some(AutofillDb::new(db_path, encdec)?)), }) } @@ -74,9 +75,12 @@ impl Store { /// Creates a store backed by an in-memory database that shares its memory API (required for autofill sync tests). #[handle_error(Error)] - pub fn new_shared_memory(db_name: &str) -> ApiResult { + pub fn new_shared_memory( + db_name: &str, + encdec: Arc, + ) -> ApiResult { Ok(Self { - db: Mutex::new(Some(AutofillDb::new_memory(db_name)?)), + db: Mutex::new(Some(AutofillDb::new_memory(db_name, encdec)?)), }) } @@ -312,14 +316,10 @@ impl Store { #[handle_error(Error)] pub fn scrub_undecryptable_credit_card_data_for_remote_replacement( self: Arc, - local_encryption_key: String, ) -> ApiResult { let db = self.lock_db()?; let deletion_stats = - credit_cards::scrub_undecryptable_credit_card_data_for_remote_replacement( - &db.writer, - local_encryption_key, - )?; + credit_cards::scrub_undecryptable_credit_card_data_for_remote_replacement(&db)?; // Here we reset the local sync data so that the credit card engine syncs as if // it were the first sync. This will potentially allow a previous sync of the @@ -393,8 +393,8 @@ pub(crate) fn delete_meta(conn: &Connection, key: &str) -> Result<()> { #[cfg(test)] mod tests { use super::*; - use crate::db::test::new_mem_db; - use crate::encryption::EncryptorDecryptor; + use crate::db::test::{new_mem_db, test_encdec}; + use crate::encryption::encrypt_str; use nss_as::ensure_initialized; #[test] @@ -434,7 +434,7 @@ mod tests { #[test] fn test_sync_manager_registration() { - let store = Arc::new(Store::new_shared_memory("sync-mgr-test").unwrap()); + let store = Arc::new(Store::new_shared_memory("sync-mgr-test", test_encdec()).unwrap()); assert_eq!(Arc::strong_count(&store), 1); assert_eq!(Arc::weak_count(&store), 0); Arc::clone(&store).register_with_sync_manager(); @@ -457,7 +457,7 @@ mod tests { #[test] fn test_shutdown_closes_the_store() { - let store = Store::new_shared_memory("shutdown-test").expect("create store"); + let store = Store::new_shared_memory("shutdown-test", test_encdec()).expect("create store"); // Operations succeed before shutdown. assert_eq!(store.count_all_passports().expect("count"), 0); @@ -474,15 +474,15 @@ mod tests { #[test] fn test_scrub_undecryptable_credit_card_data_for_remote_replacement() { ensure_initialized(); - let store = Arc::new(Store::new_shared_memory("sync-mgr-test").expect("create store")); - let key = EncryptorDecryptor::create_key().expect("create key"); - let encdec = EncryptorDecryptor::new(&key).expect("create EncryptorDecryptor"); + let store = + Arc::new(Store::new_shared_memory("scrub-test", test_encdec()).expect("create store")); + // The guard has to go out of scope before we touch the store again. + let encdec = store.lock_db().expect("db").encdec.clone(); store .add_credit_card(UpdatableCreditCardFields { cc_name: "john deer".to_string(), - cc_number_enc: encdec - .encrypt("567812345678123456781") + cc_number_enc: encrypt_str(encdec.as_ref(), "567812345678123456781") .expect("encrypt cc number"), cc_number_last_4: "6781".to_string(), cc_exp_month: 10, @@ -492,7 +492,7 @@ mod tests { .expect("add credit card to database"); store - .scrub_undecryptable_credit_card_data_for_remote_replacement(key) + .scrub_undecryptable_credit_card_data_for_remote_replacement() .expect("scrub credit card record"); } } diff --git a/components/autofill/src/encryption.rs b/components/autofill/src/encryption.rs index b9fa9b1cfac..4d5ae8a6319 100644 --- a/components/autofill/src/encryption.rs +++ b/components/autofill/src/encryption.rs @@ -10,9 +10,10 @@ // * We use regular sqlite, but want to ensure the credit-card numbers are // encrypted in the DB - so we store the number encrypted, and the key // is managed by the app. -// * The credit-card API always just accepts and returns the encrypted string, -// so we also expose encryption and decryption public functions that take -// the key and text. The core storage API never knows the unencrypted number. +// * The app hands us an `EncryptorDecryptor` when it builds the store, and the +// db holds on to it. Everything that reads or writes an encrypted column +// takes it from the db. The core storage API never knows the unencrypted +// number. // // This makes life tricky for Sync - sync has its own encryption and its own // management of sync keys. The entire records are encrypted on the server - @@ -23,14 +24,39 @@ // * When transforming a record from Sync into a DB record, we need to *encrypt* // the field. // -// So Sync needs to know the key etc, and that needs to get passed down -// multiple layers, from the app saying "sync now" all the way down to the -// low level sync code. -// To make life a little easier, we do that via a struct. +// The sync code takes the encryptor from the store it already holds. use crate::error::*; use error_support::handle_error; -pub use jwcrypto::EncryptorDecryptor; +use std::sync::Arc; + +pub use db_crypto::{EncryptorDecryptor, KeyManager, ManagedEncryptorDecryptor, StaticKeyManager}; + +// TODO(FXCM-2282): only `encrypt_string` and `decrypt_string` still build an +// encryptor from a key. When those go, so does this. +pub(crate) fn static_key_encryptor(key: &str) -> Result { + // Validate eagerly so an invalid key isn't treated as undecryptable card data. + jwcrypto::EncryptorDecryptor::new(key)?; + + Ok(ManagedEncryptorDecryptor::new(Arc::new( + StaticKeyManager::new(key.to_string()), + ))) +} + +#[cfg(test)] +pub(crate) fn random_key_encryptor() -> Result { + static_key_encryptor(&db_crypto::create_key()?) +} + +pub(crate) fn encrypt_str(encdec: &dyn EncryptorDecryptor, cleartext: &str) -> Result { + let ciphertext = encdec.encrypt(cleartext.as_bytes().to_vec())?; + String::from_utf8(ciphertext).map_err(|e| Error::CryptoNotUtf8(format!("encrypting: {e}"))) +} + +pub(crate) fn decrypt_str(encdec: &dyn EncryptorDecryptor, ciphertext: &str) -> Result { + let cleartext = encdec.decrypt(ciphertext.as_bytes().to_vec())?; + String::from_utf8(cleartext).map_err(|e| Error::CryptoNotUtf8(format!("decrypting: {e}"))) +} // public functions we expose over the FFI (which is why they take `String` // rather than the `&str` you'd otherwise expect) @@ -38,19 +64,19 @@ pub use jwcrypto::EncryptorDecryptor; pub fn encrypt_string(key: String, cleartext: String) -> ApiResult { // It would be nice to have more detailed error messages, but that would require the consumer // to pass them in. Let's not change the API yet. - Ok(EncryptorDecryptor::new(&key)?.encrypt(&cleartext)?) + encrypt_str(&static_key_encryptor(&key)?, &cleartext) } #[handle_error(Error)] pub fn decrypt_string(key: String, ciphertext: String) -> ApiResult { // It would be nice to have more detailed error messages, but that would require the consumer // to pass them in. Let's not change the API yet. - Ok(EncryptorDecryptor::new(&key)?.decrypt(&ciphertext)?) + decrypt_str(&static_key_encryptor(&key)?, &ciphertext) } #[handle_error(Error)] pub fn create_autofill_key() -> ApiResult { - Ok(EncryptorDecryptor::create_key()?) + Ok(db_crypto::create_key()?) } #[cfg(test)] @@ -61,28 +87,41 @@ mod test { #[test] fn test_encrypt() { ensure_initialized(); - let ed = EncryptorDecryptor::new(&create_autofill_key().unwrap()).unwrap(); + let ed = static_key_encryptor(&create_autofill_key().unwrap()).unwrap(); let cleartext = "secret"; - let ciphertext = ed.encrypt(cleartext).unwrap(); - assert_eq!(ed.decrypt(&ciphertext).unwrap(), cleartext); - let ed2 = EncryptorDecryptor::new(&create_autofill_key().unwrap()).unwrap(); + let ciphertext = encrypt_str(&ed, cleartext).unwrap(); + assert_eq!(decrypt_str(&ed, &ciphertext).unwrap(), cleartext); + let ed2 = static_key_encryptor(&create_autofill_key().unwrap()).unwrap(); assert!(matches!( - ed2.decrypt(&ciphertext).map_err(Error::from), - Err(Error::CryptoError(_)) + decrypt_str(&ed2, &ciphertext), + Err(Error::EncryptionError( + db_crypto::DbCryptoApiError::DecryptionFailed { .. } + )) )); } #[test] fn test_decryption_errors() { + // The shared crate maps all jwcrypto decryption failures to DecryptionFailed. ensure_initialized(); - let ed = EncryptorDecryptor::new(&create_autofill_key().unwrap()).unwrap(); + let ed = static_key_encryptor(&create_autofill_key().unwrap()).unwrap(); assert!(matches!( - ed.decrypt("invalid-ciphertext").map_err(Error::from), - Err(Error::CryptoError(_)), + decrypt_str(&ed, "invalid-ciphertext"), + Err(Error::EncryptionError( + db_crypto::DbCryptoApiError::DecryptionFailed { .. } + )), )); assert!(matches!( - ed.decrypt("").unwrap_err(), - jwcrypto::JwCryptoError::EmptyCyphertext, + decrypt_str(&ed, ""), + Err(Error::EncryptionError( + db_crypto::DbCryptoApiError::DecryptionFailed { .. } + )), )); } + + #[test] + fn test_an_invalid_key_is_rejected_up_front() { + ensure_initialized(); + assert!(static_key_encryptor("not-a-key").is_err()); + } } diff --git a/components/autofill/src/error.rs b/components/autofill/src/error.rs index 5eab83daba6..d38f5d8a512 100644 --- a/components/autofill/src/error.rs +++ b/components/autofill/src/error.rs @@ -71,8 +71,11 @@ pub enum Error { #[error("Crypto Error: {0}")] CryptoError(#[from] jwcrypto::JwCryptoError), - #[error("Missing local encryption key")] - MissingEncryptionKey, + #[error("Encryption Error: {0}")] + EncryptionError(#[from] db_crypto::DbCryptoApiError), + + #[error("Crypto data is not valid UTF-8: {0}")] + CryptoNotUtf8(String), #[error("No record with guid exists: {0}")] NoSuchRecord(String), @@ -131,10 +134,15 @@ impl GetErrorHandling for Error { }) .report_error("autofill-crypto-error"), - Self::MissingEncryptionKey => ErrorHandling::convert(AutofillApiError::CryptoError { - reason: "Missing encryption key".to_string(), + Self::EncryptionError(e) => ErrorHandling::convert(AutofillApiError::CryptoError { + reason: e.to_string(), + }) + .report_error("autofill-encryption-error"), + + Self::CryptoNotUtf8(reason) => ErrorHandling::convert(AutofillApiError::CryptoError { + reason: reason.clone(), }) - .report_error("autofill-missing-encryption-key"), + .report_error("autofill-crypto-not-utf8"), Self::NoSuchRecord(guid) => { ErrorHandling::convert(AutofillApiError::NoSuchRecord { guid: guid.clone() }) diff --git a/components/autofill/src/lib.rs b/components/autofill/src/lib.rs index 5eae09c76e6..c4fb3dff5cd 100644 --- a/components/autofill/src/lib.rs +++ b/components/autofill/src/lib.rs @@ -22,6 +22,7 @@ use crate::db::models::passport::*; use crate::db::store::Store; use crate::encryption::{create_autofill_key, decrypt_string, encrypt_string}; pub use crate::sync::AddressesBridgedEngine; +use db_crypto::EncryptorDecryptor; pub use error::{ApiResult, AutofillApiError, Error, Result}; uniffi::include_scaffolding!("autofill"); diff --git a/components/autofill/src/sync/address/mod.rs b/components/autofill/src/sync/address/mod.rs index 61ef0e6d701..566a4f062e2 100644 --- a/components/autofill/src/sync/address/mod.rs +++ b/components/autofill/src/sync/address/mod.rs @@ -13,6 +13,7 @@ use super::{ UnknownFields, }; use crate::db::models::address::InternalAddress; +use crate::encryption::EncryptorDecryptor; use crate::error::*; use crate::sync_merge_field_check; use incoming::IncomingAddressesImpl; @@ -41,9 +42,8 @@ pub(super) struct AddressesEngineStorageImpl {} impl SyncEngineStorageImpl for AddressesEngineStorageImpl { fn get_incoming_impl( &self, - enc_key: &Option, + _encdec: &Arc, ) -> Result>> { - assert!(enc_key.is_none()); Ok(Box::new(IncomingAddressesImpl {})) } @@ -57,9 +57,8 @@ impl SyncEngineStorageImpl for AddressesEngineStorageImpl { fn get_outgoing_impl( &self, - enc_key: &Option, + _encdec: &Arc, ) -> Result>> { - assert!(enc_key.is_none()); Ok(Box::new(OutgoingAddressesImpl {})) } } diff --git a/components/autofill/src/sync/bridge.rs b/components/autofill/src/sync/bridge.rs index 58715aa696a..631ffb39000 100644 --- a/components/autofill/src/sync/bridge.rs +++ b/components/autofill/src/sync/bridge.rs @@ -30,7 +30,9 @@ mod tests { fn test_sync_meta() { error_support::init_for_tests(); - let store = Arc::new(Store::new_shared_memory("addresses-bridge").unwrap()); + let store = Arc::new( + Store::new_shared_memory("addresses-bridge", crate::db::test::test_encdec()).unwrap(), + ); let bridge = store.addresses_bridged_engine(); bridge.sync_started().unwrap(); @@ -67,7 +69,10 @@ mod tests { fn test_sync_via_bridge() { error_support::init_for_tests(); - let store = Arc::new(Store::new_shared_memory("addresses-bridge-roundtrip").unwrap()); + let store = Arc::new( + Store::new_shared_memory("addresses-bridge-roundtrip", crate::db::test::test_encdec()) + .unwrap(), + ); // A local-only address: nothing on the server knows about it yet, so it // should be uploaded. diff --git a/components/autofill/src/sync/credit_card/incoming.rs b/components/autofill/src/sync/credit_card/incoming.rs index f73feb3298c..76f38adad73 100644 --- a/components/autofill/src/sync/credit_card/incoming.rs +++ b/components/autofill/src/sync/credit_card/incoming.rs @@ -7,7 +7,7 @@ use super::CreditCardPayload; use crate::db::credit_cards::{add_internal_credit_card, update_internal_credit_card}; use crate::db::models::credit_card::InternalCreditCard; use crate::db::schema::CREDIT_CARD_COMMON_COLS; -use crate::encryption::EncryptorDecryptor; +use crate::encryption::{decrypt_str, encrypt_str, EncryptorDecryptor}; use crate::error::*; use crate::sync::common::*; use crate::sync::{ @@ -17,6 +17,7 @@ use crate::sync::{ use interrupt_support::Interruptee; use rusqlite::{named_params, Transaction}; use sql_support::ConnExt; +use std::sync::Arc; use sync_guid::Guid as SyncGuid; // Takes a raw payload, as stored in our database, and returns an InternalCreditCard @@ -25,9 +26,9 @@ use sync_guid::Guid as SyncGuid; fn raw_payload_to_incoming( id: SyncGuid, raw: String, - encdec: &EncryptorDecryptor, + encdec: &dyn EncryptorDecryptor, ) -> Result> { - let payload = encdec.decrypt(&raw)?; + let payload = decrypt_str(encdec, &raw)?; // Turn it into a BSO let bso = IncomingBso { envelope: IncomingEnvelope { @@ -58,7 +59,7 @@ fn raw_payload_to_incoming( } pub(super) struct IncomingCreditCardsImpl { - pub(super) encdec: EncryptorDecryptor, + pub(super) encdec: Arc, } impl ProcessIncomingRecordImpl for IncomingCreditCardsImpl { @@ -76,7 +77,7 @@ impl ProcessIncomingRecordImpl for IncomingCreditCardsImpl { .into_iter() .map(|bso| { // consider turning this into malformed? - let encrypted = self.encdec.encrypt(&bso.payload)?; + let encrypted = encrypt_str(self.encdec.as_ref(), &bso.payload)?; Ok((bso.envelope.id, encrypted, bso.envelope.modified)) }) .collect::>()?; @@ -121,7 +122,7 @@ impl ProcessIncomingRecordImpl for IncomingCreditCardsImpl { // the 'guid' and 's_payload' rows must be non-null. let guid: SyncGuid = row.get("guid")?; let incoming = - raw_payload_to_incoming(guid.clone(), row.get("s_payload")?, &self.encdec)?; + raw_payload_to_incoming(guid.clone(), row.get("s_payload")?, self.encdec.as_ref())?; Ok(IncomingState { incoming, local: match row.get_unwrap::<_, Option>("l_guid") { @@ -155,7 +156,8 @@ impl ProcessIncomingRecordImpl for IncomingCreditCardsImpl { match row.get::<_, Option>("m_payload")? { Some(m_payload) => { // a tombstone in the mirror can be treated as though it's missing. - raw_payload_to_incoming(guid, m_payload, &self.encdec)?.content() + raw_payload_to_incoming(guid, m_payload, self.encdec.as_ref())? + .content() } None => None, } @@ -208,9 +210,9 @@ impl ProcessIncomingRecordImpl for IncomingCreditCardsImpl { Ok(Self::Record::from_row(row)?) })?; - let incoming_cc_number = self.encdec.decrypt(&incoming.cc_number_enc)?; + let incoming_cc_number = decrypt_str(self.encdec.as_ref(), &incoming.cc_number_enc)?; for record in records { - if self.encdec.decrypt(&record.cc_number_enc)? == incoming_cc_number { + if decrypt_str(self.encdec.as_ref(), &record.cc_number_enc)? == incoming_cc_number { return Ok(Some(record)); } } @@ -264,6 +266,7 @@ mod tests { use super::super::super::test::new_syncable_mem_db; use super::*; use crate::db::credit_cards::get_credit_card; + use crate::encryption::random_key_encryptor; use crate::sync::common::tests::*; use error_support::{info, trace}; @@ -332,7 +335,7 @@ mod tests { .clone() } - fn test_record(guid_prefix: char, encdec: &EncryptorDecryptor) -> InternalCreditCard { + fn test_record(guid_prefix: char, encdec: &dyn EncryptorDecryptor) -> InternalCreditCard { let json = test_json_record(guid_prefix); let payload = serde_json::from_value(json).unwrap(); InternalCreditCard::from_payload(payload, encdec).expect("should be valid") @@ -385,7 +388,7 @@ mod tests { for tc in test_cases { info!("starting new testcase"); let tx = db.transaction().unwrap(); - let encdec = EncryptorDecryptor::new_with_random_key().unwrap(); + let encdec: Arc = Arc::new(random_key_encryptor().unwrap()); // Add required items to the mirrors. let mirror_sql = "INSERT OR REPLACE INTO credit_cards_mirror (guid, payload) @@ -395,7 +398,7 @@ mod tests { mirror_sql, rusqlite::named_params! { ":guid": payload["id"].as_str().unwrap(), - ":payload": encdec.encrypt(&payload.to_string())?, + ":payload": encrypt_str(encdec.as_ref(), &payload.to_string())?, }, ) .expect("should insert mirror record"); @@ -414,7 +417,7 @@ mod tests { |row| -> Result> { let guid: SyncGuid = row.get_unwrap("guid"); let enc_payload: String = row.get_unwrap("payload"); - raw_payload_to_incoming(guid, enc_payload, &ri.encdec) + raw_payload_to_incoming(guid, enc_payload, ri.encdec.as_ref()) }, )?; @@ -441,10 +444,10 @@ mod tests { let mut db = new_syncable_mem_db(); let tx = db.transaction()?; let ri = IncomingCreditCardsImpl { - encdec: EncryptorDecryptor::new_with_random_key().unwrap(), + encdec: Arc::new(random_key_encryptor().unwrap()), }; - ri.insert_local_record(&tx, test_record('C', &ri.encdec))?; + ri.insert_local_record(&tx, test_record('C', ri.encdec.as_ref()))?; ri.change_record_guid( &tx, @@ -463,12 +466,12 @@ mod tests { let mut db = new_syncable_mem_db(); let tx = db.transaction().expect("should get tx"); let ci = IncomingCreditCardsImpl { - encdec: EncryptorDecryptor::new_with_random_key().unwrap(), + encdec: Arc::new(random_key_encryptor().unwrap()), }; - let record = test_record('C', &ci.encdec); + let record = test_record('C', ci.encdec.as_ref()); let bso = record .clone() - .into_test_incoming_bso(&ci.encdec, Default::default()); + .into_test_incoming_bso(ci.encdec.as_ref(), Default::default()); do_test_incoming_same(&ci, &tx, record, bso); } @@ -478,9 +481,9 @@ mod tests { let mut db = new_syncable_mem_db(); let tx = db.transaction().expect("should get tx"); let ci = IncomingCreditCardsImpl { - encdec: EncryptorDecryptor::new_with_random_key().unwrap(), + encdec: Arc::new(random_key_encryptor().unwrap()), }; - do_test_incoming_tombstone(&ci, &tx, test_record('C', &ci.encdec)); + do_test_incoming_tombstone(&ci, &tx, test_record('C', ci.encdec.as_ref())); } #[test] @@ -489,12 +492,12 @@ mod tests { let mut db = new_syncable_mem_db(); let tx = db.transaction().expect("should get tx"); let ci = IncomingCreditCardsImpl { - encdec: EncryptorDecryptor::new_with_random_key().unwrap(), + encdec: Arc::new(random_key_encryptor().unwrap()), }; - let mut scrubbed_record = test_record('A', &ci.encdec); + let mut scrubbed_record = test_record('A', ci.encdec.as_ref()); let bso = scrubbed_record .clone() - .into_test_incoming_bso(&ci.encdec, Default::default()); + .into_test_incoming_bso(ci.encdec.as_ref(), Default::default()); scrubbed_record.cc_number_enc = "".to_string(); do_test_scrubbed_local_data(&ci, &tx, scrubbed_record, bso); } @@ -505,12 +508,12 @@ mod tests { let mut db = new_syncable_mem_db(); let tx = db.transaction().expect("should get tx"); let ci = IncomingCreditCardsImpl { - encdec: EncryptorDecryptor::new_with_random_key().unwrap(), + encdec: Arc::new(random_key_encryptor().unwrap()), }; - let record = test_record('C', &ci.encdec); + let record = test_record('C', ci.encdec.as_ref()); let bso = record .clone() - .into_test_incoming_bso(&ci.encdec, Default::default()); + .into_test_incoming_bso(ci.encdec.as_ref(), Default::default()); do_test_staged_to_mirror(&ci, &tx, record, bso, "credit_cards_mirror"); } @@ -519,15 +522,16 @@ mod tests { ensure_initialized(); let mut db = new_syncable_mem_db(); let tx = db.transaction().expect("should get tx"); - let encdec = EncryptorDecryptor::new_with_random_key().unwrap(); - let ci = IncomingCreditCardsImpl { encdec }; - let local_record = test_record('C', &ci.encdec); + let ci = IncomingCreditCardsImpl { + encdec: Arc::new(random_key_encryptor().unwrap()), + }; + let local_record = test_record('C', ci.encdec.as_ref()); let local_guid = local_record.guid.clone(); ci.insert_local_record(&tx, local_record.clone()).unwrap(); // Now the same record incoming - it should find the one we just added // above as a dupe. - let mut incoming_record = test_record('C', &ci.encdec); + let mut incoming_record = test_record('C', ci.encdec.as_ref()); // sanity check that the encrypted numbers are different even though // the decrypted numbers are identical. assert_ne!(local_record.cc_number_enc, incoming_record.cc_number_enc); @@ -555,9 +559,10 @@ mod tests { ensure_initialized(); let mut db = new_syncable_mem_db(); let tx = db.transaction().expect("should get tx"); - let encdec = EncryptorDecryptor::new_with_random_key().unwrap(); - let ci = IncomingCreditCardsImpl { encdec }; - let local_record = test_record('C', &ci.encdec); + let ci = IncomingCreditCardsImpl { + encdec: Arc::new(random_key_encryptor().unwrap()), + }; + let local_record = test_record('C', ci.encdec.as_ref()); let local_guid = local_record.guid.clone(); ci.insert_local_record(&tx, local_record.clone()).unwrap(); diff --git a/components/autofill/src/sync/credit_card/mod.rs b/components/autofill/src/sync/credit_card/mod.rs index 213f8bf158c..27c96a63d09 100644 --- a/components/autofill/src/sync/credit_card/mod.rs +++ b/components/autofill/src/sync/credit_card/mod.rs @@ -12,7 +12,9 @@ use super::{ UnknownFields, }; use crate::db::models::credit_card::InternalCreditCard; -use crate::encryption::EncryptorDecryptor; +#[cfg(test)] +use crate::encryption::static_key_encryptor; +use crate::encryption::{decrypt_str, encrypt_str, EncryptorDecryptor}; use crate::error::*; use crate::sync_merge_field_check; use incoming::IncomingCreditCardsImpl; @@ -40,14 +42,11 @@ pub(super) struct CreditCardsEngineStorageImpl {} impl SyncEngineStorageImpl for CreditCardsEngineStorageImpl { fn get_incoming_impl( &self, - enc_key: &Option, + encdec: &Arc, ) -> Result>> { - let enc_key = match enc_key { - None => return Err(Error::MissingEncryptionKey), - Some(enc_key) => enc_key, - }; - let encdec = EncryptorDecryptor::new(enc_key)?; - Ok(Box::new(IncomingCreditCardsImpl { encdec })) + Ok(Box::new(IncomingCreditCardsImpl { + encdec: Arc::clone(encdec), + })) } fn reset_storage(&self, tx: &Transaction<'_>) -> Result<()> { @@ -60,14 +59,11 @@ impl SyncEngineStorageImpl for CreditCardsEngineStorageImpl fn get_outgoing_impl( &self, - enc_key: &Option, + encdec: &Arc, ) -> Result>> { - let enc_key = match enc_key { - None => return Err(Error::MissingEncryptionKey), - Some(enc_key) => enc_key, - }; - let encdec = EncryptorDecryptor::new(enc_key)?; - Ok(Box::new(OutgoingCreditCardsImpl { encdec })) + Ok(Box::new(OutgoingCreditCardsImpl { + encdec: Arc::clone(encdec), + })) } } @@ -113,7 +109,7 @@ pub(super) struct PayloadEntry { } impl InternalCreditCard { - fn from_payload(p: CreditCardPayload, encdec: &EncryptorDecryptor) -> Result { + fn from_payload(p: CreditCardPayload, encdec: &dyn EncryptorDecryptor) -> Result { if p.entry.version != 3 { // when new versions are introduced we will start accepting and // converting old ones - but 3 is the lowest we support. @@ -123,7 +119,7 @@ impl InternalCreditCard { ))); } // need to encrypt the cleartext in the sync record. - let cc_number_enc = encdec.encrypt(&p.entry.cc_number)?; + let cc_number_enc = encrypt_str(encdec, &p.entry.cc_number)?; let cc_number_last_4 = get_last_4(&p.entry.cc_number); Ok(InternalCreditCard { @@ -144,8 +140,8 @@ impl InternalCreditCard { }) } - pub(crate) fn into_payload(self, encdec: &EncryptorDecryptor) -> Result { - let cc_number = encdec.decrypt(&self.cc_number_enc)?; + pub(crate) fn into_payload(self, encdec: &dyn EncryptorDecryptor) -> Result { + let cc_number = decrypt_str(encdec, &self.cc_number_enc)?; Ok(CreditCardPayload { id: self.guid, entry: PayloadEntry { @@ -269,7 +265,7 @@ fn test_to_from_payload() { cc_type: "foo".to_string(), ..Default::default() }; - let encdec = EncryptorDecryptor::new(&key).unwrap(); + let encdec = static_key_encryptor(&key).unwrap(); let payload: CreditCardPayload = cc.clone().into_payload(&encdec).unwrap(); assert_eq!(payload.id, cc.guid); diff --git a/components/autofill/src/sync/credit_card/outgoing.rs b/components/autofill/src/sync/credit_card/outgoing.rs index cb315a729ba..9bda3a4f63c 100644 --- a/components/autofill/src/sync/credit_card/outgoing.rs +++ b/components/autofill/src/sync/credit_card/outgoing.rs @@ -5,11 +5,12 @@ use crate::db::models::credit_card::InternalCreditCard; use crate::db::schema::CREDIT_CARD_COMMON_COLS; -use crate::encryption::EncryptorDecryptor; +use crate::encryption::{decrypt_str, encrypt_str, EncryptorDecryptor}; use crate::error::*; use crate::sync::common::*; use crate::sync::{credit_card::CreditCardPayload, OutgoingBso, ProcessOutgoingRecordImpl}; use rusqlite::{Row, Transaction}; +use std::sync::Arc; use sync_guid::Guid as SyncGuid; const DATA_TABLE_NAME: &str = "credit_cards_data"; @@ -17,7 +18,7 @@ const MIRROR_TABLE_NAME: &str = "credit_cards_mirror"; const STAGING_TABLE_NAME: &str = "credit_cards_sync_outgoing_staging"; pub(super) struct OutgoingCreditCardsImpl { - pub(super) encdec: EncryptorDecryptor, + pub(super) encdec: Arc, } impl ProcessOutgoingRecordImpl for OutgoingCreditCardsImpl { @@ -47,12 +48,13 @@ impl ProcessOutgoingRecordImpl for OutgoingCreditCardsImpl { common_cols = CREDIT_CARD_COMMON_COLS, ); let record_from_data_row: &dyn Fn(&Row<'_>) -> Result<(OutgoingBso, i64)> = &|row| { - let mut record = InternalCreditCard::from_row(row)?.into_payload(&self.encdec)?; + let mut record = + InternalCreditCard::from_row(row)?.into_payload(self.encdec.as_ref())?; // If the server had unknown fields we fetch it and add it to the record if let Some(enc_s) = row.get::<_, Option>("payload")? { // The full payload in the credit cards mirror is encrypted let mirror_payload: CreditCardPayload = - serde_json::from_str(&self.encdec.decrypt(&enc_s)?)?; + serde_json::from_str(&decrypt_str(self.encdec.as_ref(), &enc_s)?)?; record.entry.unknown_fields = mirror_payload.entry.unknown_fields; }; @@ -74,7 +76,7 @@ impl ProcessOutgoingRecordImpl for OutgoingCreditCardsImpl { .into_iter() .map(|(bso, change_counter)| { // Turn the record into an encrypted repr to save in the mirror. - let encrypted = self.encdec.encrypt(&bso.payload)?; + let encrypted = encrypt_str(self.encdec.as_ref(), &bso.payload)?; Ok((bso.envelope.id, encrypted, change_counter)) }) .collect::>()?; @@ -110,6 +112,7 @@ impl ProcessOutgoingRecordImpl for OutgoingCreditCardsImpl { mod tests { use super::*; use crate::db::credit_cards::{add_internal_credit_card, tests::test_insert_mirror_record}; + use crate::encryption::random_key_encryptor; use crate::sync::{common::tests::*, test::new_syncable_mem_db, UnknownFields}; use serde_json::{json, Map, Value}; use types::Timestamp; @@ -164,7 +167,7 @@ mod tests { .clone() } - fn test_record(guid_prefix: char, encdec: &EncryptorDecryptor) -> InternalCreditCard { + fn test_record(guid_prefix: char, encdec: &dyn EncryptorDecryptor) -> InternalCreditCard { let json = test_json_record(guid_prefix); let payload = serde_json::from_value(json).unwrap(); InternalCreditCard::from_payload(payload, encdec).expect("should be valid") @@ -175,9 +178,9 @@ mod tests { let mut db = new_syncable_mem_db(); let tx = db.transaction().expect("should get tx"); let co = OutgoingCreditCardsImpl { - encdec: EncryptorDecryptor::new_with_random_key().unwrap(), + encdec: Arc::new(random_key_encryptor().unwrap()), }; - let test_record = test_record('C', &co.encdec); + let test_record = test_record('C', co.encdec.as_ref()); // create date record assert!(add_internal_credit_card(&tx, &test_record).is_ok()); @@ -196,9 +199,9 @@ mod tests { let mut db = new_syncable_mem_db(); let tx = db.transaction().expect("should get tx"); let co = OutgoingCreditCardsImpl { - encdec: EncryptorDecryptor::new_with_random_key().unwrap(), + encdec: Arc::new(random_key_encryptor().unwrap()), }; - let test_record = test_record('C', &co.encdec); + let test_record = test_record('C', co.encdec.as_ref()); // create tombstone record assert!(tx @@ -231,19 +234,19 @@ mod tests { let mut db = new_syncable_mem_db(); let tx = db.transaction().expect("should get tx"); let co = OutgoingCreditCardsImpl { - encdec: EncryptorDecryptor::new_with_random_key().unwrap(), + encdec: Arc::new(random_key_encryptor().unwrap()), }; // create synced record with non-zero sync_change_counter - let mut test_record = test_record('C', &co.encdec); + let mut test_record = test_record('C', co.encdec.as_ref()); let initial_change_counter_val = 2; test_record.metadata.sync_change_counter = initial_change_counter_val; assert!(add_internal_credit_card(&tx, &test_record).is_ok()); let guid = test_record.guid.clone(); //test_insert_mirror_record doesn't encrypt the mirror payload, but in reality we do // so we encrypt here so our fetch_outgoing_records doesn't break - let mut bso = test_record.into_test_incoming_bso(&co.encdec, Default::default()); - bso.payload = co.encdec.encrypt(&bso.payload).unwrap(); + let mut bso = test_record.into_test_incoming_bso(co.encdec.as_ref(), Default::default()); + bso.payload = encrypt_str(co.encdec.as_ref(), &bso.payload).unwrap(); test_insert_mirror_record(&tx, bso); exists_with_counter_value_in_table(&tx, DATA_TABLE_NAME, &guid, initial_change_counter_val); @@ -262,16 +265,16 @@ mod tests { let mut db = new_syncable_mem_db(); let tx = db.transaction().expect("should get tx"); let co = OutgoingCreditCardsImpl { - encdec: EncryptorDecryptor::new_with_random_key().unwrap(), + encdec: Arc::new(random_key_encryptor().unwrap()), }; // create synced record with no changes (sync_change_counter = 0) - let test_record = test_record('C', &co.encdec); + let test_record = test_record('C', co.encdec.as_ref()); let guid = test_record.guid.clone(); assert!(add_internal_credit_card(&tx, &test_record).is_ok()); test_insert_mirror_record( &tx, - test_record.into_test_incoming_bso(&co.encdec, Default::default()), + test_record.into_test_incoming_bso(co.encdec.as_ref(), Default::default()), ); do_test_outgoing_synced_with_no_change( @@ -288,11 +291,11 @@ mod tests { let mut db = new_syncable_mem_db(); let tx = db.transaction().expect("should get tx"); let co = OutgoingCreditCardsImpl { - encdec: EncryptorDecryptor::new_with_random_key().unwrap(), + encdec: Arc::new(random_key_encryptor().unwrap()), }; // create synced record with non-zero sync_change_counter - let mut test_record = test_record('D', &co.encdec); + let mut test_record = test_record('D', co.encdec.as_ref()); let initial_change_counter_val = 2; test_record.metadata.sync_change_counter = initial_change_counter_val; assert!(add_internal_credit_card(&tx, &test_record).is_ok()); @@ -304,8 +307,8 @@ mod tests { // so we encrypt here so our fetch_outgoing_records doesn't break let mut bso = test_record .clone() - .into_test_incoming_bso(&co.encdec, unknown_fields); - bso.payload = co.encdec.encrypt(&bso.payload).unwrap(); + .into_test_incoming_bso(co.encdec.as_ref(), unknown_fields); + bso.payload = encrypt_str(co.encdec.as_ref(), &bso.payload).unwrap(); test_insert_mirror_record(&tx, bso); exists_with_counter_value_in_table( &tx, diff --git a/components/autofill/src/sync/engine.rs b/components/autofill/src/sync/engine.rs index 204d0e7eedd..47cfa645165 100644 --- a/components/autofill/src/sync/engine.rs +++ b/components/autofill/src/sync/engine.rs @@ -3,6 +3,7 @@ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ use super::{plan_incoming, ProcessIncomingRecordImpl, ProcessOutgoingRecordImpl, SyncRecord}; +use crate::encryption::EncryptorDecryptor; use crate::error::*; use crate::Store; use error_support::warn; @@ -33,12 +34,12 @@ pub const COLLECTION_SYNCID_META_KEY: &str = "sync_id"; pub trait SyncEngineStorageImpl: Send + Sync { fn get_incoming_impl( &self, - enc_key: &Option, + encdec: &Arc, ) -> Result>>; fn reset_storage(&self, conn: &Transaction<'_>) -> Result<()>; fn get_outgoing_impl( &self, - enc_key: &Option, + encdec: &Arc, ) -> Result>>; } @@ -47,7 +48,6 @@ pub struct ConfigSyncEngine { pub(crate) config: EngineConfig, pub(crate) store: Arc, pub(crate) storage_impl: Box>, - local_enc_key: Option, } impl ConfigSyncEngine { @@ -60,7 +60,6 @@ impl ConfigSyncEngine { config, store, storage_impl, - local_enc_key: None, } } fn put_meta(&self, conn: &Connection, tail: &str, value: &dyn ToSql) -> Result<()> { @@ -102,11 +101,6 @@ impl SyncEngine for ConfigSyncEngine { self.config.collection.clone() } - fn set_local_encryption_key(&mut self, key: &str) -> anyhow::Result<()> { - self.local_enc_key = Some(key.to_string()); - Ok(()) - } - fn sync_started(&self) -> anyhow::Result<()> { let db = self.store.lock_db()?; let signal = db.begin_interrupt_scope()?; @@ -128,7 +122,7 @@ impl SyncEngine for ConfigSyncEngine { incoming_telemetry.applied(inbound.len() as u32); telem.incoming(incoming_telemetry); let tx = db.writer.unchecked_transaction()?; - let incoming_impl = self.storage_impl.get_incoming_impl(&self.local_enc_key)?; + let incoming_impl = self.storage_impl.get_incoming_impl(&db.encdec)?; incoming_impl.stage_incoming(&tx, inbound, &signal)?; tx.commit()?; @@ -143,8 +137,8 @@ impl SyncEngine for ConfigSyncEngine { let db = self.store.lock_db()?; let signal = db.begin_interrupt_scope()?; let tx = db.writer.unchecked_transaction()?; - let incoming_impl = self.storage_impl.get_incoming_impl(&self.local_enc_key)?; - let outgoing_impl = self.storage_impl.get_outgoing_impl(&self.local_enc_key)?; + let incoming_impl = self.storage_impl.get_incoming_impl(&db.encdec)?; + let outgoing_impl = self.storage_impl.get_outgoing_impl(&db.encdec)?; // Get "states" for each record... for state in incoming_impl.fetch_incoming_states(&tx)? { @@ -178,7 +172,7 @@ impl SyncEngine for ConfigSyncEngine { let db = self.store.lock_db()?; self.put_meta(&db.writer, LAST_SYNC_META_KEY, &new_timestamp.as_millis())?; let tx = db.writer.unchecked_transaction()?; - let outgoing_impl = self.storage_impl.get_outgoing_impl(&self.local_enc_key)?; + let outgoing_impl = self.storage_impl.get_outgoing_impl(&db.encdec)?; outgoing_impl.finish_synced_items(&tx, ids)?; tx.commit()?; Ok(()) @@ -268,7 +262,7 @@ mod tests { }; use crate::db::models::credit_card::InternalCreditCard; use crate::db::schema::create_empty_sync_temp_tables; - use crate::encryption::EncryptorDecryptor; + use crate::encryption::{encrypt_str, random_key_encryptor, EncryptorDecryptor}; use crate::sync::{IncomingBso, UnknownFields}; use nss_as::ensure_initialized; use sql_support::ConnExt; @@ -276,7 +270,7 @@ mod tests { impl InternalCreditCard { pub fn into_test_incoming_bso( self, - encdec: &EncryptorDecryptor, + encdec: &dyn EncryptorDecryptor, unknown_fields: UnknownFields, ) -> IncomingBso { let mut payload = self.into_payload(encdec).expect("is json"); @@ -303,11 +297,7 @@ mod tests { #[test] fn test_credit_card_engine_apply_timestamp() -> Result<()> { ensure_initialized(); - let mut credit_card_engine = create_engine(); - let test_key = crate::encryption::create_autofill_key().unwrap(); - credit_card_engine - .set_local_encryption_key(&test_key) - .unwrap(); + let credit_card_engine = create_engine(); { let db = credit_card_engine.store.lock_db()?; create_empty_sync_temp_tables(&db.writer)?; @@ -367,12 +357,12 @@ mod tests { fn test_engine_sync_reset() -> Result<()> { ensure_initialized(); let engine = create_engine(); - let encdec = EncryptorDecryptor::new_with_random_key().unwrap(); + let encdec = random_key_encryptor().unwrap(); let cc = InternalCreditCard { guid: Guid::random(), cc_name: "Ms Jane Doe".to_string(), - cc_number_enc: encdec.encrypt("12341232412341234")?, + cc_number_enc: encrypt_str(&encdec, "12341232412341234")?, cc_number_last_4: "1234".to_string(), cc_exp_month: 12, cc_exp_year: 2021,