WIP Nss persistence import key encryptor decryptor trait #4
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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.
The current Login structure is nested:
```
Login {
RecordFields record;
LoginFields fields;
SecureLoginFields sec_fields;
}
```
and thus exposes internal data structuring to the consumer. Since we make the encryption transparent for the consumer (Android, iOS, desktop) here, such a separation no longer makes sense here and above can be simplified to
```
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;
}
```
The advantage of eliminating this separation lies, on the one hand, in the simplification of the API and the resulting easier use of the component, and on the other hand, in the easier changeability of the internal data structure. If, for example, we decide later to encrypt additional fields, such a change is possible without having to adapt the consumers.
jo
force-pushed
the
nss-persistence-import-key-encryptor-decryptor-trait
branch
from
6f8b8b1 to
a56b06a
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Add EncryptorDecryptor Trait for Logins Component mozilla#6469