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,