diff --git a/components/logins/src/db.rs b/components/logins/src/db.rs index 0b29fe215c..2a82f710ce 100644 --- a/components/logins/src/db.rs +++ b/components/logins/src/db.rs @@ -265,16 +265,12 @@ impl LoginDb { // with a blank username. // // Returns an Err if the new login is not valid and could not be fixed up - pub fn find_login_to_update( - &self, - look: LoginEntry, - encdec: &dyn EncryptorDecryptor, - ) -> Result> { + pub fn find_login_to_update(&self, look: LoginEntry) -> Result> { let look = look.fixup()?; let logins = self .get_by_entry_target(&look)? .into_iter() - .map(|enc_login| enc_login.decrypt(encdec)) + .map(|enc_login| enc_login.decrypt(self.encdec.as_ref())) .collect::>>()?; Ok(logins // First, try to match the username @@ -313,22 +309,14 @@ impl LoginDb { /// /// Encrypts and stores passwords, automatically filtering out duplicates. /// Used by `add_many_with_meta()` to populate the breach database during import. - pub fn record_potentially_vulnerable_passwords( - &self, - passwords: Vec, - encdec: &dyn EncryptorDecryptor, - ) -> Result<()> { + pub fn record_potentially_vulnerable_passwords(&self, passwords: Vec) -> Result<()> { let tx = self.unchecked_transaction()?; - self.insert_potentially_vulnerable_passwords(passwords, encdec)?; + self.insert_potentially_vulnerable_passwords(passwords)?; tx.commit()?; Ok(()) } - fn insert_potentially_vulnerable_passwords( - &self, - passwords: Vec, - encdec: &dyn EncryptorDecryptor, - ) -> Result<()> { + fn insert_potentially_vulnerable_passwords(&self, passwords: Vec) -> Result<()> { let encrypted_existing_potentially_vulnerable_passwords: Vec = self .db .query_rows_and_then_cached("SELECT encryptedPassword FROM breachesL", [], |row| { @@ -338,8 +326,10 @@ impl LoginDb { encrypted_existing_potentially_vulnerable_passwords .iter() .map(|ciphertext| { - let decrypted_bytes = - encdec.decrypt(ciphertext.as_bytes().into()).map_err(|e| { + let decrypted_bytes = self + .encdec + .decrypt(ciphertext.as_bytes().into()) + .map_err(|e| { Error::DecryptionFailed(format!( "Failed to decrypt password from breachesL: {}", e @@ -367,7 +357,8 @@ impl LoginDb { .collect(); for password in difference { - let encrypted_password_bytes = encdec + let encrypted_password_bytes = self + .encdec .encrypt(password.as_bytes().into()) .map_err(|e| Error::EncryptionFailed(format!("{e} (encrypting password)")))?; let encrypted_password = @@ -395,11 +386,7 @@ impl LoginDb { /// Performance: O(M + N) where M = breached passwords, N = logins to check /// - Single check: Use `is_potentially_vulnerable_password()` (simpler) /// - Multiple checks: Use this method (faster) - pub fn are_potentially_vulnerable_passwords( - &self, - guids: &[&str], - encdec: &dyn EncryptorDecryptor, - ) -> Result> { + pub fn are_potentially_vulnerable_passwords(&self, guids: &[&str]) -> Result> { if guids.is_empty() { return Ok(Vec::new()); } @@ -413,9 +400,15 @@ impl LoginDb { let mut breached_passwords = std::collections::HashSet::new(); for ciphertext in &all_encrypted_passwords { - let decrypted_bytes = encdec.decrypt(ciphertext.as_bytes().into()).map_err(|e| { - Error::DecryptionFailed(format!("Failed to decrypt password from breachesL: {}", e)) - })?; + let decrypted_bytes = + self.encdec + .decrypt(ciphertext.as_bytes().into()) + .map_err(|e| { + Error::DecryptionFailed(format!( + "Failed to decrypt password from breachesL: {}", + e + )) + })?; let decrypted_password = std::str::from_utf8(&decrypted_bytes).map_err(|e| { Error::DecryptionFailed(format!( @@ -431,7 +424,7 @@ impl LoginDb { let mut vulnerable_guids = Vec::new(); for guid in guids { if let Some(login) = self.get_by_id(guid)? { - let decrypted_login = login.decrypt(encdec)?; + let decrypted_login = login.decrypt(self.encdec.as_ref())?; if breached_passwords.contains(&decrypted_login.password) { vulnerable_guids.push(guid.to_string()); } @@ -441,13 +434,9 @@ impl LoginDb { Ok(vulnerable_guids) } - pub fn is_potentially_vulnerable_password( - &self, - guid: &str, - encdec: &dyn EncryptorDecryptor, - ) -> Result { + pub fn is_potentially_vulnerable_password(&self, guid: &str) -> Result { // Delegate to batch method for code reuse - let vulnerable = self.are_potentially_vulnerable_passwords(&[guid], encdec)?; + let vulnerable = self.are_potentially_vulnerable_passwords(&[guid])?; Ok(!vulnerable.is_empty()) } @@ -591,11 +580,7 @@ impl LoginDb { } /// Adds multiple logins within a single transaction and returns the successfully saved logins. - pub fn add_many( - &self, - entries: Vec, - encdec: &dyn EncryptorDecryptor, - ) -> Result>> { + pub fn add_many(&self, entries: Vec) -> Result>> { let now_ms = util::system_time_ms_i64(SystemTime::now()); let entries_with_meta = entries @@ -616,7 +601,7 @@ impl LoginDb { }) .collect(); - self.add_many_with_meta(entries_with_meta, encdec) + self.add_many_with_meta(entries_with_meta) } /// Adds multiple logins **including metadata** within a single transaction and returns the successfully saved logins. @@ -626,19 +611,18 @@ impl LoginDb { pub fn add_many_with_meta( &self, entries_with_meta: Vec, - encdec: &dyn EncryptorDecryptor, ) -> Result>> { let tx = self.unchecked_transaction()?; let mut results = vec![]; for entry_with_meta in entries_with_meta { let guid = Guid::from_string(entry_with_meta.meta.id.clone()); - match self.fixup_and_check_for_dupes(&guid, entry_with_meta.entry, encdec) { + match self.fixup_and_check_for_dupes(&guid, entry_with_meta.entry) { Ok(new_entry) => { let sec_fields = SecureLoginFields { username: new_entry.username, password: new_entry.password, } - .encrypt(encdec, &entry_with_meta.meta.id)?; + .encrypt(self.encdec.as_ref(), &entry_with_meta.meta.id)?; let encrypted_login = EncryptedLogin { meta: entry_with_meta.meta, fields: LoginFields { @@ -665,11 +649,7 @@ impl LoginDb { Ok(results) } - pub fn add( - &self, - entry: LoginEntry, - encdec: &dyn EncryptorDecryptor, - ) -> Result { + pub fn add(&self, entry: LoginEntry) -> Result { let guid = Guid::random(); let now_ms = util::system_time_ms_i64(SystemTime::now()); @@ -685,27 +665,18 @@ impl LoginDb { }, }; - self.add_with_meta(entry_with_meta, encdec) + self.add_with_meta(entry_with_meta) } /// Adds a login **including metadata**. /// Normally, you will use `add` instead, and AS Logins will take care of the metadata (setting timestamps, generating an ID) itself. /// However, in some cases, this method is necessary, for example when migrating data from another store that already contains the metadata. - pub fn add_with_meta( - &self, - entry_with_meta: LoginEntryWithMeta, - encdec: &dyn EncryptorDecryptor, - ) -> Result { - let mut results = self.add_many_with_meta(vec![entry_with_meta], encdec)?; + pub fn add_with_meta(&self, entry_with_meta: LoginEntryWithMeta) -> Result { + let mut results = self.add_many_with_meta(vec![entry_with_meta])?; results.pop().expect("there should be a single result") } - pub fn update( - &self, - sguid: &str, - entry: LoginEntry, - encdec: &dyn EncryptorDecryptor, - ) -> Result { + pub fn update(&self, sguid: &str, entry: LoginEntry) -> Result { let guid = Guid::new(sguid); let now_ms = util::system_time_ms_i64(SystemTime::now()); let tx = self.unchecked_transaction()?; @@ -717,7 +688,7 @@ impl LoginDb { // just log an error and continue. This avoids a crash on android-components // (mozilla-mobile/android-components#11251). - if self.check_for_dupes(&guid, &entry, encdec).is_err() { + if self.check_for_dupes(&guid, &entry).is_err() { // Try to detect if sync is enabled by checking if there are any mirror logins let has_mirror_row: bool = self .db @@ -735,7 +706,7 @@ impl LoginDb { // We must read the existing record so we can correctly manage timePasswordChanged. let existing = match self.get_by_id(sguid)? { - Some(e) => e.decrypt(encdec)?, + Some(e) => e.decrypt(self.encdec.as_ref())?, None => return Err(Error::NoSuchRecord(sguid.to_owned())), }; let time_password_changed = if existing.password == entry.password { @@ -749,7 +720,7 @@ impl LoginDb { username: entry.username, password: entry.password, } - .encrypt(encdec, &existing.id)?; + .encrypt(self.encdec.as_ref(), &existing.id)?; let result = EncryptedLogin { meta: LoginMeta { id: existing.id, @@ -774,60 +745,36 @@ impl LoginDb { Ok(result) } - pub fn add_or_update( - &self, - entry: LoginEntry, - encdec: &dyn EncryptorDecryptor, - ) -> Result { + pub fn add_or_update(&self, entry: LoginEntry) -> Result { // Make sure to fixup the entry first, in case that changes the username let entry = entry.fixup()?; - match self.find_login_to_update(entry.clone(), encdec)? { - Some(login) => self.update(&login.id, entry, encdec), - None => self.add(entry, encdec), + match self.find_login_to_update(entry.clone())? { + Some(login) => self.update(&login.id, entry), + None => self.add(entry), } } - pub fn fixup_and_check_for_dupes( - &self, - guid: &Guid, - entry: LoginEntry, - encdec: &dyn EncryptorDecryptor, - ) -> Result { + pub fn fixup_and_check_for_dupes(&self, guid: &Guid, entry: LoginEntry) -> Result { let entry = entry.fixup()?; - self.check_for_dupes(guid, &entry, encdec)?; + self.check_for_dupes(guid, &entry)?; Ok(entry) } - pub fn check_for_dupes( - &self, - guid: &Guid, - entry: &LoginEntry, - encdec: &dyn EncryptorDecryptor, - ) -> Result<()> { - if self.dupe_exists(guid, entry, encdec)? { + pub fn check_for_dupes(&self, guid: &Guid, entry: &LoginEntry) -> Result<()> { + if self.dupe_exists(guid, entry)? { return Err(InvalidLogin::DuplicateLogin.into()); } Ok(()) } - pub fn dupe_exists( - &self, - guid: &Guid, - entry: &LoginEntry, - encdec: &dyn EncryptorDecryptor, - ) -> Result { - Ok(self.find_dupe(guid, entry, encdec)?.is_some()) + pub fn dupe_exists(&self, guid: &Guid, entry: &LoginEntry) -> Result { + Ok(self.find_dupe(guid, entry)?.is_some()) } - pub fn find_dupe( - &self, - guid: &Guid, - entry: &LoginEntry, - encdec: &dyn EncryptorDecryptor, - ) -> Result> { + pub fn find_dupe(&self, guid: &Guid, entry: &LoginEntry) -> Result> { for possible in self.get_by_entry_target(entry)? { if possible.guid() != *guid { - let pos_sec_fields = possible.decrypt_fields(encdec)?; + let pos_sec_fields = possible.decrypt_fields(self.encdec.as_ref())?; if pos_sec_fields.username == entry.username { return Ok(Some(possible.guid())); } @@ -983,13 +930,12 @@ impl LoginDb { pub fn delete_undecryptable_records_for_remote_replacement( &self, - encdec: &dyn EncryptorDecryptor, ) -> Result { // Retrieve a list of guids for logins that cannot be decrypted let corrupted_logins = self .get_all()? .into_iter() - .filter(|login| login.clone().decrypt(encdec).is_err()) + .filter(|login| login.clone().decrypt(self.encdec.as_ref()).is_err()) .collect::>(); let ids = corrupted_logins .iter() @@ -1339,27 +1285,18 @@ mod tests { }; let db = LoginDb::open_in_memory(); - db.add(login.clone(), &*TEST_ENCDEC) + db.add(login.clone()) .expect("should be able to add first login"); // We will reject new logins with the same username value... let exp_err = "Invalid login: Login already exists"; - assert_eq!( - db.add(login.clone(), &*TEST_ENCDEC) - .unwrap_err() - .to_string(), - exp_err - ); + assert_eq!(db.add(login.clone()).unwrap_err().to_string(), exp_err); // Add one with an empty username - not a dupe. login.username = "".to_string(); - db.add(login.clone(), &*TEST_ENCDEC) - .expect("empty login isn't a dupe"); + db.add(login.clone()).expect("empty login isn't a dupe"); - assert_eq!( - db.add(login, &*TEST_ENCDEC).unwrap_err().to_string(), - exp_err - ); + assert_eq!(db.add(login).unwrap_err().to_string(), exp_err); // one with a username, 1 without. assert_eq!(db.get_all().unwrap().len(), 2); @@ -1387,7 +1324,7 @@ mod tests { let db = LoginDb::open_in_memory(); let added = db - .add_many(vec![login_a.clone(), login_b.clone()], &*TEST_ENCDEC) + .add_many(vec![login_a.clone(), login_b.clone()]) .expect("should be able to add logins"); let [added_a, added_b] = added.as_slice() else { @@ -1442,11 +1379,8 @@ mod tests { }; let db = LoginDb::open_in_memory(); - db.add_many( - vec![login_a.clone(), login_b.clone(), login_umlaut.clone()], - &*TEST_ENCDEC, - ) - .expect("should be able to add logins"); + db.add_many(vec![login_a.clone(), login_b.clone(), login_umlaut.clone()]) + .expect("should be able to add logins"); assert_eq!(db.count_by_origin(origin_a).unwrap(), 1); assert_eq!(db.count_by_origin(origin_umlaut).unwrap(), 1); @@ -1486,11 +1420,8 @@ mod tests { }; let db = LoginDb::open_in_memory(); - db.add_many( - vec![login_a.clone(), login_b.clone(), login_umlaut.clone()], - &*TEST_ENCDEC, - ) - .expect("should be able to add logins"); + db.add_many(vec![login_a.clone(), login_b.clone(), login_umlaut.clone()]) + .expect("should be able to add logins"); assert_eq!(db.count_by_form_action_origin(origin_a).unwrap(), 1); assert_eq!(db.count_by_form_action_origin(origin_umlaut).unwrap(), 1); @@ -1510,7 +1441,7 @@ mod tests { }; let db = LoginDb::open_in_memory(); - db.add(login, &*TEST_ENCDEC) + db.add(login) .expect("should be able to add login with invalid form_action_origin"); assert_eq!(db.count_by_form_action_origin("email").unwrap(), 1); } @@ -1538,7 +1469,7 @@ mod tests { let db = LoginDb::open_in_memory(); let added = db - .add_many(vec![login_a.clone(), login_b.clone()], &*TEST_ENCDEC) + .add_many(vec![login_a.clone(), login_b.clone()]) .expect("should be able to add logins"); let [added_a, added_b] = added.as_slice() else { @@ -1585,7 +1516,7 @@ mod tests { meta: meta.clone(), }; - db.add_with_meta(entry_with_meta, &*TEST_ENCDEC) + db.add_with_meta(entry_with_meta) .expect("should be able to add login with record"); let fetched = db @@ -1609,10 +1540,11 @@ mod tests { assert_eq!(count, 0); // Record some passwords - db.record_potentially_vulnerable_passwords( - vec!["password1".into(), "password2".into(), "password3".into()], - &*TEST_ENCDEC, - ) + db.record_potentially_vulnerable_passwords(vec![ + "password1".into(), + "password2".into(), + "password3".into(), + ]) .unwrap(); // Verify they were inserted @@ -1623,11 +1555,8 @@ mod tests { assert_eq!(count, 3); // Try to insert duplicates - should be filtered out - db.record_potentially_vulnerable_passwords( - vec!["password1".into(), "password4".into()], - &*TEST_ENCDEC, - ) - .unwrap(); + db.record_potentially_vulnerable_passwords(vec!["password1".into(), "password4".into()]) + .unwrap(); // Only password4 should have been added let count: i64 = db @@ -1637,11 +1566,8 @@ mod tests { assert_eq!(count, 4); // Try to insert only duplicates - should be a no-op - db.record_potentially_vulnerable_passwords( - vec!["password1".into(), "password2".into()], - &*TEST_ENCDEC, - ) - .unwrap(); + db.record_potentially_vulnerable_passwords(vec!["password1".into(), "password2".into()]) + .unwrap(); let count: i64 = db .db @@ -1678,7 +1604,7 @@ mod tests { meta: meta.clone(), }; - db.add_with_meta(entry_with_meta, &*TEST_ENCDEC) + db.add_with_meta(entry_with_meta) .expect("should be able to add login with record"); db.delete(&guid).expect("should be able to delete login"); @@ -1688,7 +1614,7 @@ mod tests { meta: meta.clone(), }; - db.add_with_meta(entry_with_meta2, &*TEST_ENCDEC) + db.add_with_meta(entry_with_meta2) .expect("should be able to re-add login with record"); let fetched = db @@ -1704,18 +1630,15 @@ mod tests { ensure_initialized(); let db = LoginDb::open_in_memory(); let added = db - .add( - LoginEntry { - form_action_origin: Some("http://😍.com".into()), - origin: "http://😍.com".into(), - http_realm: None, - username_field: "😍".into(), - password_field: "😍".into(), - username: "😍".into(), - password: "😍".into(), - }, - &*TEST_ENCDEC, - ) + .add(LoginEntry { + form_action_origin: Some("http://😍.com".into()), + origin: "http://😍.com".into(), + http_realm: None, + username_field: "😍".into(), + password_field: "😍".into(), + username: "😍".into(), + password: "😍".into(), + }) .unwrap(); let fetched = db .get_by_id(&added.meta.id) @@ -1729,7 +1652,7 @@ mod tests { ); assert_eq!(fetched.fields.username_field, "😍"); assert_eq!(fetched.fields.password_field, "😍"); - let sec_fields = fetched.decrypt_fields(&*TEST_ENCDEC).unwrap(); + let sec_fields = fetched.decrypt_fields(db.encdec.as_ref()).unwrap(); assert_eq!(sec_fields.username, "😍"); assert_eq!(sec_fields.password, "😍"); } @@ -1739,17 +1662,14 @@ mod tests { ensure_initialized(); let db = LoginDb::open_in_memory(); let added = db - .add( - LoginEntry { - form_action_origin: None, - origin: "http://😍.com".into(), - http_realm: Some("😍😍".into()), - username: "😍".into(), - password: "😍".into(), - ..Default::default() - }, - &*TEST_ENCDEC, - ) + .add(LoginEntry { + form_action_origin: None, + origin: "http://😍.com".into(), + http_realm: Some("😍😍".into()), + username: "😍".into(), + password: "😍".into(), + ..Default::default() + }) .unwrap(); let fetched = db .get_by_id(&added.meta.id) @@ -1781,15 +1701,12 @@ mod tests { ) { let db = LoginDb::open_in_memory(); for h in good.iter().chain(bad.iter()) { - db.add( - LoginEntry { - origin: (*h).into(), - http_realm: Some((*h).into()), - password: "test".into(), - ..Default::default() - }, - &*TEST_ENCDEC, - ) + db.add(LoginEntry { + origin: (*h).into(), + http_realm: Some((*h).into()), + password: "test".into(), + ..Default::default() + }) .unwrap(); } for query in good_queries { @@ -1883,7 +1800,7 @@ mod tests { password: "test_password".into(), ..Default::default() }; - let login = db.add(to_add, &*TEST_ENCDEC).unwrap(); + let login = db.add(to_add).unwrap(); let login2 = db.get_by_id(&login.meta.id).unwrap().unwrap(); assert_eq!(login.fields.origin, login2.fields.origin); @@ -1896,16 +1813,13 @@ mod tests { ensure_initialized(); let db = LoginDb::open_in_memory(); let login = db - .add( - LoginEntry { - origin: "https://www.example.com".into(), - http_realm: Some("https://www.example.com".into()), - username: "user1".into(), - password: "password1".into(), - ..Default::default() - }, - &*TEST_ENCDEC, - ) + .add(LoginEntry { + origin: "https://www.example.com".into(), + http_realm: Some("https://www.example.com".into()), + username: "user1".into(), + password: "password1".into(), + ..Default::default() + }) .unwrap(); db.update( &login.meta.id, @@ -1916,7 +1830,6 @@ mod tests { password: "password2".into(), ..Default::default() // TODO: check and fix if needed }, - &*TEST_ENCDEC, ) .unwrap(); @@ -1927,7 +1840,7 @@ mod tests { login2.fields.http_realm, Some("https://www.example2.com".into()) ); - let sec_fields = login2.decrypt_fields(&*TEST_ENCDEC).unwrap(); + let sec_fields = login2.decrypt_fields(db.encdec.as_ref()).unwrap(); assert_eq!(sec_fields.username, "user2"); assert_eq!(sec_fields.password, "password2"); } @@ -1937,16 +1850,13 @@ mod tests { ensure_initialized(); let db = LoginDb::open_in_memory(); let login = db - .add( - LoginEntry { - origin: "https://www.example.com".into(), - http_realm: Some("https://www.example.com".into()), - username: "user1".into(), - password: "password1".into(), - ..Default::default() - }, - &*TEST_ENCDEC, - ) + .add(LoginEntry { + origin: "https://www.example.com".into(), + http_realm: Some("https://www.example.com".into()), + username: "user1".into(), + password: "password1".into(), + ..Default::default() + }) .unwrap(); // Simulate touch happening at another "time" thread::sleep(time::Duration::from_millis(50)); @@ -1961,16 +1871,13 @@ mod tests { ensure_initialized(); let db = LoginDb::open_in_memory(); let login = db - .add( - LoginEntry { - origin: "https://www.example.com".into(), - http_realm: Some("https://www.example.com".into()), - username: "user1".into(), - password: "password1".into(), - ..Default::default() - }, - &*TEST_ENCDEC, - ) + .add(LoginEntry { + origin: "https://www.example.com".into(), + http_realm: Some("https://www.example.com".into()), + username: "user1".into(), + password: "password1".into(), + ..Default::default() + }) .unwrap(); // initial state assert!(login.meta.time_last_breach_alert_dismissed.is_none()); @@ -1986,16 +1893,13 @@ mod tests { ensure_initialized(); let db = LoginDb::open_in_memory(); let login = db - .add( - LoginEntry { - origin: "https://www.example.com".into(), - http_realm: Some("https://www.example.com".into()), - username: "user1".into(), - password: "password1".into(), - ..Default::default() - }, - &*TEST_ENCDEC, - ) + .add(LoginEntry { + origin: "https://www.example.com".into(), + http_realm: Some("https://www.example.com".into()), + username: "user1".into(), + password: "password1".into(), + ..Default::default() + }) .unwrap(); let dismiss_time = login.meta.time_password_changed + 1000; @@ -2006,7 +1910,7 @@ mod tests { .get_by_id(&login.meta.id) .unwrap() .unwrap() - .decrypt(&*TEST_ENCDEC) + .decrypt(db.encdec.as_ref()) .unwrap(); assert_eq!( retrieved.time_last_breach_alert_dismissed, @@ -2019,16 +1923,13 @@ mod tests { ensure_initialized(); let db = LoginDb::open_in_memory(); let login = db - .add( - LoginEntry { - origin: "https://www.example.com".into(), - http_realm: Some("https://www.example.com".into()), - username: "test_user".into(), - password: "test_password".into(), - ..Default::default() - }, - &*TEST_ENCDEC, - ) + .add(LoginEntry { + origin: "https://www.example.com".into(), + http_realm: Some("https://www.example.com".into()), + username: "test_user".into(), + password: "test_password".into(), + ..Default::default() + }) .unwrap(); assert!(db.delete(login.guid_str()).unwrap()); @@ -2052,29 +1953,23 @@ mod tests { let db = LoginDb::open_in_memory(); let login_a = db - .add( - LoginEntry { - origin: "https://a.example.com".into(), - http_realm: Some("https://www.example.com".into()), - username: "test_user".into(), - password: "test_password".into(), - ..Default::default() - }, - &*TEST_ENCDEC, - ) + .add(LoginEntry { + origin: "https://a.example.com".into(), + http_realm: Some("https://www.example.com".into()), + username: "test_user".into(), + password: "test_password".into(), + ..Default::default() + }) .unwrap(); let login_b = db - .add( - LoginEntry { - origin: "https://b.example.com".into(), - http_realm: Some("https://www.example.com".into()), - username: "test_user".into(), - password: "test_password".into(), - ..Default::default() - }, - &*TEST_ENCDEC, - ) + .add(LoginEntry { + origin: "https://b.example.com".into(), + http_realm: Some("https://www.example.com".into()), + username: "test_user".into(), + password: "test_password".into(), + ..Default::default() + }) .unwrap(); let result = db @@ -2092,16 +1987,13 @@ mod tests { let db = LoginDb::open_in_memory(); let login = db - .add( - LoginEntry { - origin: "https://a.example.com".into(), - http_realm: Some("https://www.example.com".into()), - username: "test_user".into(), - password: "test_password".into(), - ..Default::default() - }, - &*TEST_ENCDEC, - ) + .add(LoginEntry { + origin: "https://a.example.com".into(), + http_realm: Some("https://www.example.com".into()), + username: "test_user".into(), + password: "test_password".into(), + ..Default::default() + }) .unwrap(); let result = db.delete_many(vec![login.guid_str()]).unwrap(); @@ -2126,16 +2018,13 @@ mod tests { ensure_initialized(); let db = LoginDb::open_in_memory(); let login = db - .add( - LoginEntry { - origin: "https://www.example.com".into(), - http_realm: Some("https://www.example.com".into()), - username: "test_user".into(), - password: "test_password".into(), - ..Default::default() - }, - &*TEST_ENCDEC, - ) + .add(LoginEntry { + origin: "https://www.example.com".into(), + http_realm: Some("https://www.example.com".into()), + username: "test_user".into(), + password: "test_password".into(), + ..Default::default() + }) .unwrap(); let result = db @@ -2165,9 +2054,9 @@ mod tests { } fn make_saved_login(db: &LoginDb, username: &str, password: &str) -> Login { - db.add(make_entry(username, password), &*TEST_ENCDEC) + db.add(make_entry(username, password)) .unwrap() - .decrypt(&*TEST_ENCDEC) + .decrypt(db.encdec.as_ref()) .unwrap() } @@ -2178,8 +2067,7 @@ mod tests { let login = make_saved_login(&db, "user", "pass"); assert_eq!( Some(login), - db.find_login_to_update(make_entry("user", "pass"), &*TEST_ENCDEC) - .unwrap(), + db.find_login_to_update(make_entry("user", "pass")).unwrap(), ); } @@ -2190,33 +2078,26 @@ mod tests { // Non-match because the username is different make_saved_login(&db, "other-user", "pass"); // Non-match because the http_realm is different - db.add( - LoginEntry { - origin: "https://www.example.com".into(), - http_realm: Some("the other website".into()), - username: "user".into(), - password: "pass".into(), - ..Default::default() - }, - &*TEST_ENCDEC, - ) + db.add(LoginEntry { + origin: "https://www.example.com".into(), + http_realm: Some("the other website".into()), + username: "user".into(), + password: "pass".into(), + ..Default::default() + }) .unwrap(); // Non-match because it uses form_action_origin instead of http_realm - db.add( - LoginEntry { - origin: "https://www.example.com".into(), - form_action_origin: Some("https://www.example.com/".into()), - username: "user".into(), - password: "pass".into(), - ..Default::default() - }, - &*TEST_ENCDEC, - ) + db.add(LoginEntry { + origin: "https://www.example.com".into(), + form_action_origin: Some("https://www.example.com/".into()), + username: "user".into(), + password: "pass".into(), + ..Default::default() + }) .unwrap(); assert_eq!( None, - db.find_login_to_update(make_entry("user", "pass"), &*TEST_ENCDEC) - .unwrap(), + db.find_login_to_update(make_entry("user", "pass")).unwrap(), ); } @@ -2227,8 +2108,7 @@ mod tests { let login = make_saved_login(&db, "", "pass"); assert_eq!( Some(login), - db.find_login_to_update(make_entry("user", "pass"), &*TEST_ENCDEC) - .unwrap(), + db.find_login_to_update(make_entry("user", "pass")).unwrap(), ); } @@ -2240,8 +2120,7 @@ mod tests { let username_match = make_saved_login(&db, "user", "pass"); assert_eq!( Some(username_match), - db.find_login_to_update(make_entry("user", "pass"), &*TEST_ENCDEC) - .unwrap(), + db.find_login_to_update(make_entry("user", "pass")).unwrap(), ); } @@ -2250,14 +2129,11 @@ mod tests { ensure_initialized(); let db = LoginDb::open_in_memory(); assert!(db - .find_login_to_update( - LoginEntry { - http_realm: None, - form_action_origin: None, - ..LoginEntry::default() - }, - &*TEST_ENCDEC - ) + .find_login_to_update(LoginEntry { + http_realm: None, + form_action_origin: None, + ..LoginEntry::default() + }) .is_err()); } @@ -2274,11 +2150,11 @@ mod tests { let mut entry = login.entry(); entry.password = "pass2".to_string(); - db.update(&login.id, entry, &*TEST_ENCDEC).unwrap(); + db.update(&login.id, entry).unwrap(); let mut entry = login.entry(); entry.password = "pass3".to_string(); - db.add_or_update(entry, &*TEST_ENCDEC).unwrap(); + db.add_or_update(entry).unwrap(); } #[test] @@ -2288,64 +2164,49 @@ mod tests { // Create two logins with the same password let login1 = db - .add( - LoginEntry { - origin: "https://site1.com".into(), - http_realm: Some("realm".into()), - username: "user1".into(), - password: "shared_password".into(), - ..Default::default() - }, - &*TEST_ENCDEC, - ) + .add(LoginEntry { + origin: "https://site1.com".into(), + http_realm: Some("realm".into()), + username: "user1".into(), + password: "shared_password".into(), + ..Default::default() + }) .unwrap(); let login2 = db - .add( - LoginEntry { - origin: "https://site2.com".into(), - http_realm: Some("realm".into()), - username: "user2".into(), - password: "shared_password".into(), - ..Default::default() - }, - &*TEST_ENCDEC, - ) + .add(LoginEntry { + origin: "https://site2.com".into(), + http_realm: Some("realm".into()), + username: "user2".into(), + password: "shared_password".into(), + ..Default::default() + }) .unwrap(); // Initially, neither login is vulnerable assert!(!db - .is_potentially_vulnerable_password(&login1.meta.id, &*TEST_ENCDEC) + .is_potentially_vulnerable_password(&login1.meta.id) .unwrap()); assert!(!db - .is_potentially_vulnerable_password(&login2.meta.id, &*TEST_ENCDEC) + .is_potentially_vulnerable_password(&login2.meta.id) .unwrap()); // And checking both logins should return empty (none are vulnerable yet) let vulnerable = db - .are_potentially_vulnerable_passwords( - &[&login1.meta.id, &login2.meta.id], - &*TEST_ENCDEC, - ) + .are_potentially_vulnerable_passwords(&[&login1.meta.id, &login2.meta.id]) .unwrap(); assert_eq!(vulnerable.len(), 0); // Record "shared_password" as a vulnerable password - db.record_potentially_vulnerable_passwords( - vec!["shared_password".into()], - &*TEST_ENCDEC, - ) - .unwrap(); + db.record_potentially_vulnerable_passwords(vec!["shared_password".into()]) + .unwrap(); // login2 should be recognized as vulnerable (same password as breached login1) assert!(db - .is_potentially_vulnerable_password(&login2.meta.id, &*TEST_ENCDEC) + .is_potentially_vulnerable_password(&login2.meta.id) .unwrap()); // Batch check: both logins should be vulnerable (they share the same password) let vulnerable = db - .are_potentially_vulnerable_passwords( - &[&login1.meta.id, &login2.meta.id], - &*TEST_ENCDEC, - ) + .are_potentially_vulnerable_passwords(&[&login1.meta.id, &login2.meta.id]) .unwrap(); assert_eq!(vulnerable.len(), 2); assert!(vulnerable.contains(&login1.meta.id)); @@ -2361,12 +2222,11 @@ mod tests { password: "different_password".into(), ..Default::default() }, - &*TEST_ENCDEC, ) .unwrap(); assert!(!db - .is_potentially_vulnerable_password(&login2.meta.id, &*TEST_ENCDEC) + .is_potentially_vulnerable_password(&login2.meta.id) .unwrap()); } @@ -2376,19 +2236,16 @@ mod tests { let db = LoginDb::open_in_memory(); let login = db - .add( - LoginEntry { - origin: "https://example.com".into(), - http_realm: Some("realm".into()), - username: "user".into(), - password: "password123".into(), - ..Default::default() - }, - &*TEST_ENCDEC, - ) + .add(LoginEntry { + origin: "https://example.com".into(), + http_realm: Some("realm".into()), + username: "user".into(), + password: "password123".into(), + ..Default::default() + }) .unwrap(); - db.record_potentially_vulnerable_passwords(vec!["password123".into()], &*TEST_ENCDEC) + db.record_potentially_vulnerable_passwords(vec!["password123".into()]) .unwrap(); // Verify that breachesL has an entry @@ -2399,7 +2256,7 @@ mod tests { assert_eq!(count, 1); // And verify via the API that this login is vulnerable let vulnerable = db - .are_potentially_vulnerable_passwords(&[&login.meta.id], &*TEST_ENCDEC) + .are_potentially_vulnerable_passwords(&[&login.meta.id]) .unwrap(); assert_eq!(vulnerable.len(), 1); assert_eq!(vulnerable[0], login.meta.id); @@ -2415,7 +2272,7 @@ mod tests { assert_eq!(count, 0); // And verify via the API that no logins are vulnerable anymore let vulnerable = db - .are_potentially_vulnerable_passwords(&[&login.meta.id], &*TEST_ENCDEC) + .are_potentially_vulnerable_passwords(&[&login.meta.id]) .unwrap(); assert_eq!(vulnerable.len(), 0); } @@ -2426,45 +2283,36 @@ mod tests { let db = LoginDb::open_in_memory(); let login1 = db - .add( - LoginEntry { - origin: "https://site1.com".into(), - http_realm: Some("realm".into()), - username: "user".into(), - password: "password_A".into(), - ..Default::default() - }, - &*TEST_ENCDEC, - ) + .add(LoginEntry { + origin: "https://site1.com".into(), + http_realm: Some("realm".into()), + username: "user".into(), + password: "password_A".into(), + ..Default::default() + }) .unwrap(); let login2 = db - .add( - LoginEntry { - origin: "https://site2.com".into(), - http_realm: Some("realm".into()), - username: "user".into(), - password: "password_B".into(), - ..Default::default() - }, - &*TEST_ENCDEC, - ) + .add(LoginEntry { + origin: "https://site2.com".into(), + http_realm: Some("realm".into()), + username: "user".into(), + password: "password_B".into(), + ..Default::default() + }) .unwrap(); - db.record_potentially_vulnerable_passwords(vec!["password_A".into()], &*TEST_ENCDEC) + db.record_potentially_vulnerable_passwords(vec!["password_A".into()]) .unwrap(); // login2 has a different password → not vulnerable assert!(!db - .is_potentially_vulnerable_password(&login2.meta.id, &*TEST_ENCDEC) + .is_potentially_vulnerable_password(&login2.meta.id) .unwrap()); // Batch check: login1 should be vulnerable (its password is in breachesL) // login2 has a different password, so it's not vulnerable let vulnerable = db - .are_potentially_vulnerable_passwords( - &[&login1.meta.id, &login2.meta.id], - &*TEST_ENCDEC, - ) + .are_potentially_vulnerable_passwords(&[&login1.meta.id, &login2.meta.id]) .unwrap(); assert_eq!(vulnerable.len(), 1); assert!(vulnerable.contains(&login1.meta.id)); diff --git a/components/logins/src/store.rs b/components/logins/src/store.rs index 60ce1a3563..31ff089c8c 100644 --- a/components/logins/src/store.rs +++ b/components/logins/src/store.rs @@ -170,7 +170,7 @@ impl LoginStore { #[handle_error(Error)] pub fn find_login_to_update(&self, entry: LoginEntry) -> ApiResult> { let db = self.lock_db()?; - db.find_login_to_update(entry, db.encdec.as_ref()) + db.find_login_to_update(entry) } #[handle_error(Error)] @@ -183,19 +183,19 @@ impl LoginStore { // Note: Vec<&str> is not supported with UDL, so we receive Vec and convert let db = self.lock_db()?; let ids: Vec<&str> = ids.iter().map(|id| &**id).collect(); - db.are_potentially_vulnerable_passwords(&ids, db.encdec.as_ref()) + db.are_potentially_vulnerable_passwords(&ids) } #[handle_error(Error)] pub fn is_potentially_vulnerable_password(&self, id: &str) -> ApiResult { let db = self.lock_db()?; - db.is_potentially_vulnerable_password(id, db.encdec.as_ref()) + db.is_potentially_vulnerable_password(id) } #[handle_error(Error)] pub fn record_potentially_vulnerable_passwords(&self, passwords: Vec) -> ApiResult<()> { let db = self.lock_db()?; - db.record_potentially_vulnerable_passwords(passwords, db.encdec.as_ref()) + db.record_potentially_vulnerable_passwords(passwords) } #[handle_error(Error)] @@ -239,8 +239,7 @@ impl LoginStore { let engine = LoginsSyncEngine::new(Arc::clone(&self))?; let db = self.lock_db()?; - let deletion_stats = - db.delete_undecryptable_records_for_remote_replacement(db.encdec.as_ref())?; + let deletion_stats = db.delete_undecryptable_records_for_remote_replacement()?; engine.set_last_sync(&db, ServerTimestamp(0))?; Ok(deletion_stats) } @@ -264,24 +263,25 @@ impl LoginStore { #[handle_error(Error)] pub fn update(&self, id: &str, entry: LoginEntry) -> ApiResult { let db = self.lock_db()?; - db.update(id, entry, db.encdec.as_ref()) + db.update(id, entry) .and_then(|enc_login| enc_login.decrypt(db.encdec.as_ref())) } #[handle_error(Error)] pub fn add(&self, entry: LoginEntry) -> ApiResult { let db = self.lock_db()?; - db.add(entry, db.encdec.as_ref()) + db.add(entry) .and_then(|enc_login| enc_login.decrypt(db.encdec.as_ref())) } #[handle_error(Error)] pub fn add_many(&self, entries: Vec) -> ApiResult> { let db = self.lock_db()?; - db.add_many(entries, db.encdec.as_ref()).map(|enc_logins| { + let encdec = db.encdec.as_ref(); + db.add_many(entries).map(|enc_logins| { enc_logins .into_iter() - .map(|enc_login| map_bulk_result_entry(enc_login, db.encdec.as_ref())) + .map(|enc_login| map_bulk_result_entry(enc_login, encdec)) .collect() }) } @@ -292,7 +292,7 @@ impl LoginStore { #[handle_error(Error)] pub fn add_with_meta(&self, entry_with_meta: LoginEntryWithMeta) -> ApiResult { let db = self.lock_db()?; - db.add_with_meta(entry_with_meta, db.encdec.as_ref()) + db.add_with_meta(entry_with_meta) .and_then(|enc_login| enc_login.decrypt(db.encdec.as_ref())) } @@ -302,19 +302,19 @@ impl LoginStore { entries_with_meta: Vec, ) -> ApiResult> { let db = self.lock_db()?; - db.add_many_with_meta(entries_with_meta, db.encdec.as_ref()) - .map(|enc_logins| { - enc_logins - .into_iter() - .map(|enc_login| map_bulk_result_entry(enc_login, db.encdec.as_ref())) - .collect() - }) + let encdec = db.encdec.as_ref(); + db.add_many_with_meta(entries_with_meta).map(|enc_logins| { + enc_logins + .into_iter() + .map(|enc_login| map_bulk_result_entry(enc_login, encdec)) + .collect() + }) } #[handle_error(Error)] pub fn add_or_update(&self, entry: LoginEntry) -> ApiResult { let db = self.lock_db()?; - db.add_or_update(entry, db.encdec.as_ref()) + db.add_or_update(entry) .and_then(|enc_login| enc_login.decrypt(db.encdec.as_ref())) } @@ -335,7 +335,7 @@ impl LoginStore { let options = options.unwrap_or_default(); run_maintenance(&conn)?; if options.delete_undecryptable_records_for_remote_replacement { - conn.delete_undecryptable_records_for_remote_replacement(conn.encdec.as_ref())?; + conn.delete_undecryptable_records_for_remote_replacement()?; } Ok(()) } @@ -384,7 +384,6 @@ impl Default for RunMaintenanceOptions { #[cfg(test)] mod tests { use super::*; - use crate::encryption::test_utils::TEST_ENCDEC; use crate::util; use nss_as::ensure_initialized; use std::cmp::Reverse; @@ -550,18 +549,15 @@ mod tests { ensure_initialized(); // If the database has data, then wipe_local() returns > 0 rows deleted let db = LoginDb::open_in_memory(); - db.add_or_update( - LoginEntry { - origin: "https://www.example.com".into(), - form_action_origin: Some("https://www.example.com".into()), - username_field: "user_input".into(), - password_field: "pass_input".into(), - username: "coolperson21".into(), - password: "p4ssw0rd".into(), - ..Default::default() - }, - &TEST_ENCDEC.clone(), - ) + db.add_or_update(LoginEntry { + origin: "https://www.example.com".into(), + form_action_origin: Some("https://www.example.com".into()), + username_field: "user_input".into(), + password_field: "pass_input".into(), + username: "coolperson21".into(), + password: "p4ssw0rd".into(), + ..Default::default() + }) .unwrap(); assert!(db.wipe_local().unwrap() > 0); diff --git a/testing/sync-test/src/logins.rs b/testing/sync-test/src/logins.rs index fab4718857..de478811d6 100644 --- a/testing/sync-test/src/logins.rs +++ b/testing/sync-test/src/logins.rs @@ -379,24 +379,27 @@ fn test_delete_undecryptable_records_for_remote_replacement( ))); log::info!("Add another login to client0"); - - let login1 = c0 - .logins_store - .lock_db() - .expect("db lock retrieved") - .add( - LoginEntry { - origin: "http://www.example3.com".into(), - form_action_origin: Some("http://login.example3.com".into()), - username_field: "uname".into(), - password_field: "pword".into(), - username: "cool_username".into(), - password: "hunter2".into(), - ..Default::default() - }, - &*new_encdec, - ) + // Store the login using the new/wrong encdec + // + // Note: this code has not actually been tested since the sync-tests were already failing when + // BDK made changes in #7373 + let mut db = c0.logins_store.lock_db().expect("db lock retrieved"); + let old_encdec = std::mem::replace(&mut db.encdec, new_encdec.clone()); + + let login1 = db + .add(LoginEntry { + origin: "http://www.example3.com".into(), + form_action_origin: Some("http://login.example3.com".into()), + username_field: "uname".into(), + password_field: "pword".into(), + username: "cool_username".into(), + password: "hunter2".into(), + ..Default::default() + }) .expect("add login1"); + db.encdec = old_encdec; + drop(db); + let l1id = login1.guid(); // Check that the corrupted login exists on first device