Closed Bug 1634061 Opened 4 years ago Closed 4 years ago

Figure out persistence functions in the FxA Rust Bridge

Categories

(Firefox :: Sync, task)

task

Tracking

()

RESOLVED WONTFIX

People

(Reporter: vladikoff, Unassigned)

References

()

Details

In the FxA Rust + the component we have bits such as:

  /**
   * Restore a previous instance of `RustFxABridge` from a
   * serialized state (obtained with `toJSON(...)`).
   * @returns {Promise<RustFxABridge>}
   */
  static fromJSON(json) {
    log.trace("fromJSON");
  }
  /**
   * Serialize the state of a `RustFxABridge` instance. It can be restored
   * later with `fromJSON(...)`. It is the responsibility of the caller to
   * persist that serialized state regularly (after operations that mutate
   * `RustFxABridge`) in a **secure** location.
   * @returns {Promise<string>} The JSON representation of the state.
   */
  async toJSON() {
    log.trace("toJSON");
  }

We need to figure out where to persist and restore state from in the bridge, there are a few existing places in Firefox such as signedInUser.json, but there can also be new solutions we can use

I'm setting this as a blocker for 1632897 because I cannot find another meta bug to attach it at this time. I think that can probably be moved to block a different milestone.

Surfacing some comments that came up over in https://bugzilla.mozilla.org/show_bug.cgi?id=1633867#c10

State storage on Desktop is currently split so that the encryption keys are stored in the password manager, while the rest of the state is stored in a plaintext JSON file on disk. The reason for this is so that we don't interfere with the "master password" feature, which is supposed to protect against local access to the user's saved passwords. The invariant we want to maintain here is: for someone with access to the user's local Firefox profile, it should be at least as hard for them to get the user's saved passwords out of sync, as it is for them to get at the saved passwords locally.

and for completeness, it is split because we want enough account data to be not secured so that certain functionality works as expected even when the master password remains locked.

We met today to discuss this, but haven't yet settled on a concrete plan. I wanted to jot down some thoughts coming out of that meeting while they're still fresh in my head. Here's a grab-bag of such.

Do we need to worry about sharing or coordinating state between the existing JS code and the new Rust code? For example, if I'm signed in to the browser already and I flip the useRustComponent pref to true, is that a one-off migration of my account state so that it's now entirely managed by the rust component? Do we intend to support flipping the useRustComponent pref back to false and have that work correctly?

It's clear from the meeting that we need to solve the "the sync key must be protected by master password if master password is enabled" problem somehow. I believe there were three broach approaches suggested. (There are probably others as well! I just wanted to capture the ones we discussed).

1) Protect all the FxA state with master password, e.g. by storing it all in the password manager.

For users who have both master-password and FxA, all FxA-related functionality in the browser would be in a "locked" state until the user enters their master password. For example the accounts toolbar menu would probably start out as some sort of "warning" icon until the user clicks on it to unlock.

This would conceptually be the nicest approach from the point of view of the existing fxa-client component, but it seems like it has a lot of UX implications.

2) Duplicate some data into an unprotected cache, similar to signedInUser.json.

It would only be possible to use the fxa-client code after unlocking master password and de-serializing the stored state, but we might be able to keep e.g. the profile information in an unprotected store and use it to power basic read-only FxA functionality in the browser.

This would probably be a UX improvement over (1), but it would involve some amount of additional one-off code in Desktop. We might have to parse the JSON returned by toJSON() in order to extract the state, breaking the abstraction boundary.

3) Add secure-storage functionality to fxa-client itself.

We could have the rust-component somehow manage an extra layer of security in storing the sync keys for itself. Similar to the logins component, imagine the the application could (optionally!) provide an encryption key with which to protect the stored passwords, like this strawman API suggests:

impl FirefoxAccount {

  /// Configure an encryption key to be used to protect sensitive data in local storage.
  /// When a secure storage key is set, then you need to call `unlock_secure_storage()`
  /// after deserializing a `FirefoxAccount`, otherwise certain secure operations will fail.

  pub fn set_secure_storage_key(&mut self, key: [u8]) -> Result<()> { ... }

  /// If a secure storage key is configured, then the returned JSON blob
  /// will have some of its fields encrypted using that key. You need to
  /// call `unlock_secure_storage` after deserializing in order to access
  /// these fields.

  pub fn to_json(&self) -> Result<String> { ... }

  /// Enable access to high-security data fields, such as scoped keys.
  /// Note that there is no corresponding `lock` operation; just discard the unlocked
  /// object and deserialize it afresh from stored state.

  unlock_secure_storage(&mut self, key: [u8]) -> Result<()> { ... }

  /// Remove an encryption key previously configured by `set_secure_storage_key`.

  pub fn clear_secure_storage_key(&mut self, key: [u8]) -> Result<()> { ... }
}

Or if we don't like the idea of doing our own encrypting of things, we could have something like:

impl FirefoxAccount {

  /// In addition to the usual `to_json` method, we have a new one that splits out the
  /// keys into a separate string, to be stored separately. It's up to the app to manage
  /// the separate storage of the two JSON blobs.

  pub fn to_json(&self) -> Result<String> { ... }

  pub fn to_json_with_detached_keys(&self) -> Result<(String, String)> { ... }

  /// You can deserialize by providing just the unsecured data, and the resulting
  /// object won't have access to any keys.

  pub fn from_json(data: &str) -> Result<Self> { ... }

  /// And can later provide the keys if/when you decide you need them.

  pub fn set_keys_from_json(&mut self, data: &str) -> Result<()> { ... }
}

This seems like it would be the most amount of work, but it would make the "secure storage" functionality available for other platforms. (Whether we'd ever actually want to use it on other platforms, is a separate question).

Thanks Ryan,
And while I don't think this was in doubt, I'm firmly in favor of (3). To summarize why:

  • If a platform offers storage enclaves with different and increasing levels of security for the information stored inside them, I think our FxA component itself should support storing our most sensitive information in the most secure area.

  • Storing all our information in the most secure area prohibits reasonable UX (ie, I don't think (1) will ever fly)

Thus I see no reasonable alternative to (3).

Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.