Skip to content

Commit

Permalink
introduce EncryptorDecryptorTrait for Logins component
Browse files Browse the repository at this point in the history
This prepares the Logins component for the desktop and simplifies its
API.

BREAKING CHANGE:
This commit introduces breaking changes to the Logins component:

During initialization, it receives an additional argument, a
EncryptorDecryptorTrait implementation. In addition, several LoginsStore
API methods have been changed to not require an encryption key argument
anymore, and return Logins objects instead of EncryptedLogins.

Additionally, a new API method has been added to the LoginsStore,
`has_logins_by_base_domain(&self, base_domain: &str)`, which can be used
to check for the existence of a login for a given base domain.

**EncryptorDecryptor**

With the introduction of the EncryptorDecryptor trait, encryption
becomes transparent. That means, the LoginStore API receives some
breaking changes as outlined above.  A ManagedEncryptorDecryptor will
provide an EncryptorDecryptor implementation which uses the currently
used crypto methods, given a KeyManager implementation. This eases
adaption for mobile.  Furthermore, we provide a StaticKeyManager
implementation, which can be used in tests and in cases where the key is
- you name it - static.  Constructors Now an implementation of the above
property must be passed to the constructors. To do this, the signatures
are extended as follows:

```
pub fn new(path: impl AsRef<Path>, encdec: Arc<dyn EncryptorDecryptor>) -> ApiResult<Self>
pub fn new_from_db(db: LoginDb, encdec: Arc<dyn EncryptorDecryptor>) -> Self
pub fn new_in_memory(encdec: Arc<dyn EncryptorDecryptor>) -> ApiResult<Self>
```

**LoginStore API Methods**
This allows the LoginStore API to be simplified as follows, making
encryption transparent by eliminating the need to pass the key and
allowing the methods to return decrypted login objects.

```
pub fn list(&self) -> ApiResult<Vec<Login>>
pub fn get(&self, id: &str) -> ApiResult<Option<Login>>
pub fn get_by_base_domain(&self, base_domain: &str) -> ApiResult<Vec<Login>>
pub fn find_login_to_update(&self, entry: LoginEntry) -> ApiResult<Option<Login>>
pub fn update(&self, id: &str, entry: LoginEntry) -> ApiResult<Login>
pub fn add(&self, entry: LoginEntry) -> ApiResult<Login>
pub fn add_or_update(&self, entry: LoginEntry) -> ApiResult<Login>
```

We will stop Uniffi-exposing the crypto primitives encrypt, decrypt,
encrypt_struct and decrypt_struct. Also EncryptedLogin will not be
exposed anymore.  Checking for the Existence of Logins for a given Base
Domain In order to check for the existence of stored logins for a given
base domain, we provide an additional store method,
has_logins_by_base_domain(&self, base_domain: &str), which does not
utilize the EncryptorDecryptor.

Another by-change is in the `check_canary` function: here we do not
throw anymore if a wrong key is used but return false.
  • Loading branch information
jo committed Dec 17, 2024
1 parent 7896001 commit ac69d88
Show file tree
Hide file tree
Showing 22 changed files with 660 additions and 488 deletions.
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
website/build
target
credentials.json
logins.jwk
*-engine.json
*.db
.*.swp
Expand Down
4 changes: 3 additions & 1 deletion components/logins/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,9 @@ To effectively work on the Logins component, you will need to be familiar with:

### Implementation Overview

Logins implements encrypted storage for login records on top of NSS. The storage schema is based on the one
Logins implements encrypted storage for login records on top of a consumer
implemented EncryptorDecryptor, or via ManagedEncryptorDecryptor, using NSS
based crypto algorithms (AES256-GCM). The storage schema is based on the one
originally used in [Firefox for
iOS](https://github.com/mozilla-mobile/firefox-ios/blob/faa6a2839abf4da2c54ff1b3291174b50b31ab2c/Storage/SQL/SQLiteLogins.swift),
but with the following notable differences:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,12 @@ import org.mozilla.appservices.logins.GleanMetrics.LoginsStore as LoginsStoreMet
* LoginStore.
*/

class DatabaseLoginsStorage(dbPath: String) : AutoCloseable {
class DatabaseLoginsStorage(dbPath: String, keyManager: KeyManager) : AutoCloseable {
private var store: LoginStore

init {
this.store = LoginStore(dbPath)
val encdec = createManagedEncdec(keyManager)
this.store = LoginStore(dbPath, encdec)
}

@Throws(LoginsApiException::class)
Expand All @@ -46,7 +47,7 @@ class DatabaseLoginsStorage(dbPath: String) : AutoCloseable {
}

@Throws(LoginsApiException::class)
fun get(id: String): EncryptedLogin? {
fun get(id: String): Login? {
return readQueryCounters.measure {
store.get(id)
}
Expand All @@ -60,44 +61,44 @@ class DatabaseLoginsStorage(dbPath: String) : AutoCloseable {
}

@Throws(LoginsApiException::class)
fun list(): List<EncryptedLogin> {
fun list(): List<Login> {
return readQueryCounters.measure {
store.list()
}
}

@Throws(LoginsApiException::class)
fun getByBaseDomain(baseDomain: String): List<EncryptedLogin> {
fun getByBaseDomain(baseDomain: String): List<Login> {
return readQueryCounters.measure {
store.getByBaseDomain(baseDomain)
}
}

@Throws(LoginsApiException::class)
fun findLoginToUpdate(look: LoginEntry, encryptionKey: String): Login? {
fun findLoginToUpdate(look: LoginEntry): Login? {
return readQueryCounters.measure {
store.findLoginToUpdate(look, encryptionKey)
store.findLoginToUpdate(look)
}
}

@Throws(LoginsApiException::class)
fun add(entry: LoginEntry, encryptionKey: String): EncryptedLogin {
fun add(entry: LoginEntry): Login {
return writeQueryCounters.measure {
store.add(entry, encryptionKey)
store.add(entry)
}
}

@Throws(LoginsApiException::class)
fun update(id: String, entry: LoginEntry, encryptionKey: String): EncryptedLogin {
fun update(id: String, entry: LoginEntry): Login {
return writeQueryCounters.measure {
store.update(id, entry, encryptionKey)
store.update(id, entry)
}
}

@Throws(LoginsApiException::class)
fun addOrUpdate(entry: LoginEntry, encryptionKey: String): EncryptedLogin {
fun addOrUpdate(entry: LoginEntry): Login {
return writeQueryCounters.measure {
store.addOrUpdate(entry, encryptionKey)
store.addOrUpdate(entry)
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,14 +32,15 @@ class DatabaseLoginsStorageTest {
@get:Rule
val gleanRule = GleanTestRule(ApplicationProvider.getApplicationContext())

protected val encryptionKey = createKey()

fun createTestStore(): DatabaseLoginsStorage {
Megazord.init()
val dbPath = dbFolder.newFile()
return DatabaseLoginsStorage(dbPath = dbPath.absolutePath)
val keyManager = createStaticKeyManager(key = encryptionKey)
return DatabaseLoginsStorage(dbPath = dbPath.absolutePath, keyManager = keyManager)
}

protected val encryptionKey = createKey()

protected fun getTestStore(): DatabaseLoginsStorage {
val store = createTestStore()

Expand All @@ -57,7 +58,6 @@ class DatabaseLoginsStorageTest {
password = "hunter2",
),
),
encryptionKey,
)

store.add(
Expand All @@ -74,7 +74,6 @@ class DatabaseLoginsStorageTest {
username = "Foobar2000",
),
),
encryptionKey,
)

return store
Expand Down Expand Up @@ -106,7 +105,6 @@ class DatabaseLoginsStorageTest {
password = "hunter2",
),
),
encryptionKey,
)

assertEquals(LoginsStoreMetrics.writeQueryCount.testGetValue(), 1)
Expand All @@ -128,7 +126,7 @@ class DatabaseLoginsStorageTest {
)

try {
store.add(invalid, encryptionKey)
store.add(invalid)
fail("Should have thrown")
} catch (e: LoginsApiException.InvalidRecord) {
// All good.
Expand Down
18 changes: 9 additions & 9 deletions components/logins/ios/Logins/LoginsStorage.swift
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@ open class LoginsStorage {
private var store: LoginStore
private let queue = DispatchQueue(label: "com.mozilla.logins-storage")

public init(databasePath: String) throws {
store = try LoginStore(path: databasePath)
public init(databasePath: String, keyManager: KeyManager) throws {
store = try LoginStore(path: databasePath, encdec: createManagedEncdec(keyManager: keyManager))
}

open func wipeLocal() throws {
Expand Down Expand Up @@ -47,36 +47,36 @@ open class LoginsStorage {
/// then this throws `LoginStoreError.DuplicateGuid` if there is a collision
///
/// Returns the `id` of the newly inserted record.
open func add(login: LoginEntry, encryptionKey: String) throws -> EncryptedLogin {
open func add(login: LoginEntry) throws -> Login {
return try queue.sync {
try self.store.add(login: login, encryptionKey: encryptionKey)
try self.store.add(login: login)
}
}

/// Update `login` in the database. If `login.id` does not refer to a known
/// login, then this throws `LoginStoreError.NoSuchRecord`.
open func update(id: String, login: LoginEntry, encryptionKey: String) throws -> EncryptedLogin {
open func update(id: String, login: LoginEntry) throws -> Login {
return try queue.sync {
try self.store.update(id: id, login: login, encryptionKey: encryptionKey)
try self.store.update(id: id, login: login)
}
}

/// Get the record with the given id. Returns nil if there is no such record.
open func get(id: String) throws -> EncryptedLogin? {
open func get(id: String) throws -> Login? {
return try queue.sync {
try self.store.get(id: id)
}
}

/// Get the entire list of records.
open func list() throws -> [EncryptedLogin] {
open func list() throws -> [Login] {
return try queue.sync {
try self.store.list()
}
}

/// Get the list of records for some base domain.
open func getByBaseDomain(baseDomain: String) throws -> [EncryptedLogin] {
open func getByBaseDomain(baseDomain: String) throws -> [Login] {
return try queue.sync {
try self.store.getByBaseDomain(baseDomain: baseDomain)
}
Expand Down
Loading

0 comments on commit ac69d88

Please sign in to comment.