Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

EncryptorDecryptor Trait for Logins Component #6469

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open
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
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
63 changes: 63 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,68 @@
# v136.0 (In progress)

### Logins
The Logins component has been rewritten to use a newly introduced `EncryptorDecryptor` trait.

#### BREAKING CHANGE
The LoginsStore constructor and several API methods have been changed:

The signatures of the constructors 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>
```

The methods do not require an encryption key argument anymore, and return `Login` objects instead of `EncryptedLogin`:
```
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>
```

New LoginsStore methods:
```
// Checking whether the database contains logins (does not utilize the `EncryptorDecryptor`):
is_empty(&self) -> ApiResult<bool>
// Checking for the Existence of Logins for a given base domain (also does not utilize the `EncryptorDecryptor`):
has_logins_by_base_domain(&self, base_domain: &str) -> ApiResult<bool>
```

The crypto primitives `encrypt`, `decrypt`, `encrypt_struct` and `decrypt_struct` are not exposed anymore via UniFFI, as well as `EncryptedLogin` will not be exposed anymore. In addition we also do not expose the structs `RecordFields`, `LoginFields` and `SecureLoginFields` anymore.


##### SyncEngine
The logins sync engine has been adapted for above EncryptorDecryptor trait and therefore does not support a `set_local_encryption_key` method anymore.

##### Flattened Login Struct
The flattened Login struct now does not expose internal structuring to the consumer:
```
Login {
// record fields
string id;
i64 times_used;
i64 time_created;
i64 time_last_used;
i64 time_password_changed;

// login fields
string origin;
string? http_realm;
string? form_action_origin;
string username_field;
string password_field;

// secure login fields
string password;
string username;
}
```


### `rc_crypto`
- New low level bindings for dealing with primary password.

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,58 @@ class DatabaseLoginsStorage(dbPath: String) : AutoCloseable {
}

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

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

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

@Throws(LoginsApiException::class)
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,49 +32,40 @@ 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()

store.add(
LoginEntry(
fields = LoginFields(
origin = "https://www.example.com",
httpRealm = "Something",
usernameField = "users_name",
passwordField = "users_password",
formActionOrigin = null,
),
secFields = SecureLoginFields(
username = "Foobar2000",
password = "hunter2",
),
origin = "https://www.example.com",
httpRealm = "Something",
usernameField = "users_name",
passwordField = "users_password",
formActionOrigin = null,
username = "Foobar2000",
password = "hunter2",
),
encryptionKey,
)

store.add(
LoginEntry(
fields = LoginFields(
origin = "https://www.example.org",
httpRealm = "",
formActionOrigin = "https://www.example.org/login",
usernameField = "users_name",
passwordField = "users_password",
),
secFields = SecureLoginFields(
password = "MyVeryCoolPassword",
username = "Foobar2000",
),
origin = "https://www.example.org",
httpRealm = "",
formActionOrigin = "https://www.example.org/login",
usernameField = "users_name",
passwordField = "users_password",
password = "MyVeryCoolPassword",
username = "Foobar2000",
),
encryptionKey,
)

return store
Expand All @@ -94,41 +85,32 @@ class DatabaseLoginsStorageTest {

val login = store.add(
LoginEntry(
fields = LoginFields(
origin = "https://www.example.com",
httpRealm = "Something",
usernameField = "users_name",
passwordField = "users_password",
formActionOrigin = null,
),
secFields = SecureLoginFields(
username = "Foobar2000",
password = "hunter2",
),
origin = "https://www.example.com",
httpRealm = "Something",
usernameField = "users_name",
passwordField = "users_password",
formActionOrigin = null,
username = "Foobar2000",
password = "hunter2",
),
encryptionKey,
)

assertEquals(LoginsStoreMetrics.writeQueryCount.testGetValue(), 1)
assertNull(LoginsStoreMetrics.writeQueryErrorCount["invalid_record"].testGetValue())

// N.B. this is invalid due to `formActionOrigin` being an invalid url.
val invalid = LoginEntry(
fields = LoginFields(
origin = "https://test.example.com",
formActionOrigin = "not a url",
httpRealm = "",
usernameField = "users_name",
passwordField = "users_password",
),
secFields = SecureLoginFields(
username = "Foobar2000",
password = "hunter2",
),
origin = "https://test.example.com",
formActionOrigin = "not a url",
httpRealm = "",
usernameField = "users_name",
passwordField = "users_password",
username = "Foobar2000",
password = "hunter2",
)

try {
store.add(invalid, encryptionKey)
store.add(invalid)
fail("Should have thrown")
} catch (e: LoginsApiException.InvalidRecord) {
// All good.
Expand All @@ -140,8 +122,8 @@ class DatabaseLoginsStorageTest {
assertNull(LoginsStoreMetrics.readQueryCount.testGetValue())
assertNull(LoginsStoreMetrics.readQueryErrorCount["storage_error"].testGetValue())

val record = store.get(login.record.id)!!
assertEquals(record.fields.origin, "https://www.example.com")
val record = store.get(login.id)!!
assertEquals(record.origin, "https://www.example.com")

assertEquals(LoginsStoreMetrics.readQueryCount.testGetValue(), 1)
assertNull(LoginsStoreMetrics.readQueryErrorCount["storage_error"].testGetValue())
Expand All @@ -155,13 +137,13 @@ class DatabaseLoginsStorageTest {
val login = store.list()[0]
// Wait 100ms so that touch is certain to change timeLastUsed.
Thread.sleep(100)
store.touch(login.record.id)
store.touch(login.id)

val updatedLogin = store.get(login.record.id)
val updatedLogin = store.get(login.id)

assertNotNull(updatedLogin)
assertEquals(login.record.timesUsed + 1, updatedLogin!!.record.timesUsed)
assert(updatedLogin.record.timeLastUsed > login.record.timeLastUsed)
assertEquals(login.timesUsed + 1, updatedLogin!!.timesUsed)
assert(updatedLogin.timeLastUsed > login.timeLastUsed)

assertThrows(LoginsApiException.NoSuchRecord::class.java) { store.touch("abcdabcdabcd") }

Expand All @@ -173,11 +155,11 @@ class DatabaseLoginsStorageTest {
val store = getTestStore()
val login = store.list()[0]

assertNotNull(store.get(login.record.id))
assertTrue(store.delete(login.record.id))
assertNull(store.get(login.record.id))
assertFalse(store.delete(login.record.id))
assertNull(store.get(login.record.id))
assertNotNull(store.get(login.id))
assertTrue(store.delete(login.id))
assertNull(store.get(login.id))
assertFalse(store.delete(login.id))
assertNull(store.get(login.id))

finishAndClose(store)
}
Expand All @@ -191,8 +173,8 @@ class DatabaseLoginsStorageTest {
test.wipeLocal()
assertEquals(0, test.list().size)

assertNull(test.get(logins[0].record.id))
assertNull(test.get(logins[1].record.id))
assertNull(test.get(logins[0].id))
assertNull(test.get(logins[1].id))

finishAndClose(test)
}
Expand Down
Loading