Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,12 @@
and `LoginStore.wipe_local_except_fxa()`, a variant of `wipe_local()` that preserves the FxA session-credentials login
([#7467](https://github.com/mozilla/application-services/pull/7467)) ([Bug 2053557](https://bugzilla.mozilla.org/show_bug.cgi?id=2053557))

## 🔧 What's Fixed 🔧

### Logins

- `LoginStore.shutdown()` now releases its reference to the consumer-provided `EncryptorDecryptor` so foreign (e.g. JS) callback handles are unregistered during shutdown instead of lingering. ([#PRNUM](https://github.com/mozilla/application-services/pull/7476))

# v153.0 (_2026-06-15_)

## ⚠️ Breaking Changes ⚠️
Expand Down
8 changes: 6 additions & 2 deletions components/logins/src/db.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
/// server.
/// - After we sync, we move all records from loginsL to loginsM, overwriting any previous data.
/// loginsL will be an empty table after this. See mark_as_synchronized() for the details.
use crate::encryption::EncryptorDecryptor;
use crate::encryption::{EncryptorDecryptor, NoopEncryptorDecryptor};
use crate::error::*;
use crate::login::*;
use crate::schema;
Expand Down Expand Up @@ -1079,7 +1079,11 @@ impl LoginDb {
Ok(row_count)
}

pub fn shutdown(self) -> Result<()> {
pub fn shutdown(mut self) -> Result<()> {
// Drop our reference to the (possibly foreign/JS-backed) encryptor before
// we tear the rest down, so its callback handle is released during
// shutdown instead of lingering.
self.encdec = Arc::new(NoopEncryptorDecryptor);
self.db.close().map_err(|(_, e)| Error::SqlError(e))
}
}
Expand Down
38 changes: 38 additions & 0 deletions components/logins/src/encryption.rs
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,29 @@ impl<T: EncryptorDecryptor> EncryptorDecryptor for Arc<T> {
}
}

/// A placeholder `EncryptorDecryptor` used after the store has been shut down.
///
/// On shutdown we swap the real encryptor (which, for foreign consumers like
/// Desktop, is a JS-backed callback interface) out for this one. That drops our
/// reference to the foreign callback so its handle is unregistered during
/// shutdown rather than lingering. Any call that still reaches it (e.g. via a
/// stray clone) fails cleanly instead of calling into torn-down foreign code.
pub struct NoopEncryptorDecryptor;

impl EncryptorDecryptor for NoopEncryptorDecryptor {
fn encrypt(&self, _clearbytes: Vec<u8>) -> ApiResult<Vec<u8>> {
Err(LoginsApiError::UnexpectedLoginsApiError {
reason: "encrypt called on a shut-down store".to_string(),
})
}

fn decrypt(&self, _cipherbytes: Vec<u8>) -> ApiResult<Vec<u8>> {
Err(LoginsApiError::UnexpectedLoginsApiError {
reason: "decrypt called on a shut-down store".to_string(),
})
}
}

/// The ManagedEncryptorDecryptor makes use of the NSS provided cryptographic algorithms. The
/// ManagedEncryptorDecryptor uses a KeyManager for encryption key retrieval.
pub struct ManagedEncryptorDecryptor {
Expand Down Expand Up @@ -372,6 +395,21 @@ mod tests {
assert_eq!(key.as_bytes(), key_manager.get_key().unwrap());
}

#[test]
fn test_noop_encdec_errors() {
// The placeholder we swap in on shutdown must never encrypt/decrypt; it
// should fail cleanly instead.
let encdec = NoopEncryptorDecryptor;
assert!(matches!(
encdec.encrypt("secret".as_bytes().into()).err().unwrap(),
LoginsApiError::UnexpectedLoginsApiError { .. }
));
assert!(matches!(
encdec.decrypt("secret".as_bytes().into()).err().unwrap(),
LoginsApiError::UnexpectedLoginsApiError { .. }
));
}

#[test]
fn test_managed_encdec_with_invalid_key() {
ensure_initialized();
Expand Down