identity.rs icon indicating copy to clipboard operation
identity.rs copied to clipboard

[Task] Improve `Stronghold` constructor and `did_create` impl

Open PhilippGackstatter opened this issue 2 years ago • 0 comments

Description

After #787 we should make some quality of life and security improvements to our Stronghold wrapper. Some of these will be breaking changes.

  • [ ] Increase PBKDF2_HMAC_SHA512 iterations to 120,000.
    • Note that this breaks using older stronghold snapshot files, so it can be considered a more severe breaking change than changing an API.
  • [ ] Implement transaction-like behaviour in did_create. Investigate if this is necessary for other functions as well. This should perhaps also be documented and/or recommended behaviour.
    • For instance, Someone re-trying to create an identity from an existing keypair might fail every time, because the DID was added to the index, but the client syncing later failed.
  • [ ] Add a create flag for Stronghold that, if true, will create a snapshot file if it doesn't exist, or, if false, throw an error if the file doesn't exist.

This last point warrants re-thinking the Stronghold constructor.

The broad options are (copied from @cycraig):

  1. Stronghold::new and Stronghold::new_with_options with a StrongholdOptions (would have to be a builder to avoid breaking changes).
    Stronghold::new_with_options("./snapshot.hodl", "password", StrongholdOptions::new().dropsave(false)).await?;
    
  2. Introducing a StrongholdBuilder, e.g.,
    Stronghold::builder()
      .path("./snapshot.hodl")
      .password("password")
      .dropsave(false)
      .create(false)
      .build()
      .await?;
    
  3. Placing everything in a non-exhaustive StrongholdOptions struct, embracing the TypeScript approach:
    Stronghold::new(StrongholdOptions {
      path: "./snapshot.hold".into(),
      password: "some-password".into(),
      create: false,
      ..Default::default()
    }),await?;
    

I prefer option 1, because the other options leave the possibility of a user not providing a password and path which are required, requiring us to introduce an error variant (à la MissingRequiredField). This can of course be further discussed.

Motivation

Improve security and provide users a way to better express their expectations (create flag).

Resources

Change checklist

Add an x to the boxes that are relevant to your changes, and delete any items that are not.

  • [ ] The feature or fix is implemented in Rust and across all bindings whereas possible.
  • [ ] The feature or fix has sufficient testing coverage
  • [ ] All tests and examples build and run locally as expected
  • [ ] Every piece of code has been document according to the documentation guidelines.
  • [ ] If conceptual documentation (mdbook) and examples highlighting the feature exist, they are properly updated.
  • [ ] If the feature is not currently documented, a documentation task Issue has been opened to address this.

PhilippGackstatter avatar May 05 '22 10:05 PhilippGackstatter