Bug 1013064 (part 2) - Store sensitive FxA credentials in the login manager. r=ckarlof,dveditz

From 4a92f9ee1ba35989f82a24bba18806f8616a5be8 Mon Sep 17 00:00:00 2001
---
 services/fxaccounts/FxAccounts.jsm                 | 170 ++++++++++++++++++
 services/fxaccounts/FxAccountsCommon.js            |  13 ++
 services/fxaccounts/moz.build                      |   5 +-
 .../tests/xpcshell/test_loginmgr_storage.js        | 196 +++++++++++++++++++++
 services/fxaccounts/tests/xpcshell/xpcshell.ini    |   2 +
 services/sync/modules/util.js                      |  12 +-
 6 files changed, 396 insertions(+), 2 deletions(-)
 create mode 100644 services/fxaccounts/tests/xpcshell/test_loginmgr_storage.js

services/fxaccounts/FxAccounts.jsm

@@ -303,7 +303,12 @@ function FxAccountsInternal() {
 
   // We don't reference |profileDir| in the top-level module scope
   // as we may be imported before we know where it is.
+  // We only want the fancy new LoginManagerStorage on desktop.
+#if defined(MOZ_B2G)
   this.signedInUserStorage = new JSONStorage({
+#else
+  this.signedInUserStorage = new LoginManagerStorage({
+#endif
     filename: DEFAULT_STORAGE_FILENAME,
     baseDir: OS.Constants.Path.profileDir,
   });
@@ -901,6 +906,171 @@ JSONStorage.prototype = {
   }
 };
 
+/**
+ * LoginManagerStorage constructor that creates instances that may set/get
+ * from a combination of a clear-text JSON file and stored securely in
+ * the nsILoginManager.
+ *
+ * @param options {
+ *                  filename: of the plain-text file to write to
+ *                  baseDir: directory where the file resides
+ *                }
+ * @return instance
+ */
+
+function LoginManagerStorage(options) {
+  // we reuse the JSONStorage for writing the plain-text stuff.
+  this.jsonStorage = new JSONStorage(options);
+}
+
+LoginManagerStorage.prototype = {
+  // The fields in the credentials JSON object that are stored in plain-text
+  // in the profile directory.  All other fields are stored in the login manager,
+  // and thus are only available when the master-password is unlocked.
+
+  // a hook point for testing.
+  get _isLoggedIn() {
+    return Services.logins.isLoggedIn;
+  },
+
+  // Clear any data from the login manager.  Returns true if the login manager
+  // was unlocked (even if no existing logins existed) or false if it was
+  // locked (meaning we don't even know if it existed or not.)
+  _clearLoginMgrData: Task.async(function* () {
+    try { // Services.logins might be third-party and broken...
+      yield Services.logins.initializationPromise;
+      if (!this._isLoggedIn) {
+        return false;
+      }
+      let logins = Services.logins.findLogins({}, FXA_PWDMGR_HOST, null, FXA_PWDMGR_REALM);
+      for (let login of logins) {
+        Services.logins.removeLogin(login);
+      }
+      return true;
+    } catch (ex) {
+      log.error("Failed to clear login data: ${}", ex);
+      return false;
+    }
+  }),
+
+  set: Task.async(function* (contents) {
+    if (!contents) {
+      // User is signing out - write the null to the json file.
+      yield this.jsonStorage.set(contents);
+
+      // And nuke it from the login manager.
+      let cleared = yield this._clearLoginMgrData();
+      if (!cleared) {
+        // just log a message - we verify that the email address matches when
+        // we reload it, so having a stale entry doesn't really hurt.
+        log.info("not removing credentials from login manager - not logged in");
+      }
+      return;
+    }
+
+    // We are saving actual data.
+    // Split the data into 2 chunks - one to go to the plain-text, and the
+    // other to write to the login manager.
+    let toWriteJSON = {version: contents.version};
+    let accountDataJSON = toWriteJSON.accountData = {};
+    let toWriteLoginMgr = {version: contents.version};
+    let accountDataLoginMgr = toWriteLoginMgr.accountData = {};
+    for (let [name, value] of Iterator(contents.accountData)) {
+      if (FXA_PWDMGR_PLAINTEXT_FIELDS.indexOf(name) >= 0) {
+        accountDataJSON[name] = value;
+      } else {
+        accountDataLoginMgr[name] = value;
+      }
+    }
+    yield this.jsonStorage.set(toWriteJSON);
+
+    try { // Services.logins might be third-party and broken...
+      // and the stuff into the login manager.
+      yield Services.logins.initializationPromise;
+      // If MP is locked we silently fail - the user may need to re-auth
+      // next startup.
+      if (!this._isLoggedIn) {
+        log.info("not saving credentials to login manager - not logged in");
+        return;
+      }
+      // write the rest of the data to the login manager.
+      let loginInfo = new Components.Constructor(
+         "@mozilla.org/login-manager/loginInfo;1", Ci.nsILoginInfo, "init");
+      let login = new loginInfo(FXA_PWDMGR_HOST,
+                                null, // aFormSubmitURL,
+                                FXA_PWDMGR_REALM, // aHttpRealm,
+                                contents.accountData.email, // aUsername
+                                JSON.stringify(toWriteLoginMgr), // aPassword
+                                "", // aUsernameField
+                                "");// aPasswordField
+
+      let existingLogins = Services.logins.findLogins({}, FXA_PWDMGR_HOST, null,
+                                                      FXA_PWDMGR_REALM);
+      if (existingLogins.length) {
+        Services.logins.modifyLogin(existingLogins[0], login);
+      } else {
+        Services.logins.addLogin(login);
+      }
+    } catch (ex) {
+      log.error("Failed to save data to the login manager: ${}", ex);
+    }
+  }),
+
+  get: Task.async(function* () {
+    // we need to suck some data from the .json file in the profile dir and
+    // some other from the login manager.
+    let data = yield this.jsonStorage.get();
+    if (!data) {
+      // no user logged in, nuke the storage data incase we couldn't remove
+      // it previously and then we are done.
+      yield this._clearLoginMgrData();
+      return null;
+    }
+
+    // if we have encryption keys it must have been saved before we
+    // used the login manager, so re-save it.
+    if (data.accountData.kA || data.accountData.kB || data.keyFetchToken) {
+      log.info("account data needs migration to the login manager.");
+      yield this.set(data);
+    }
+
+    try { // Services.logins might be third-party and broken...
+      // read the data from the login manager and merge it for return.
+      yield Services.logins.initializationPromise;
+
+      if (!this._isLoggedIn) {
+        log.info("returning partial account data as the login manager is locked.");
+        return data;
+      }
+
+      let logins = Services.logins.findLogins({}, FXA_PWDMGR_HOST, null, FXA_PWDMGR_REALM);
+      if (logins.length == 0) {
+        // This could happen if the MP was locked when we wrote the data.
+        log.info("Can't find the rest of the credentials in the login manager");
+        return data;
+      }
+      let login = logins[0];
+      if (login.username == data.accountData.email) {
+        let lmData = JSON.parse(login.password);
+        if (lmData.version == data.version) {
+          // Merge the login manager data
+          copyObjectProperties(lmData.accountData, data.accountData);
+        } else {
+          log.info("version field in the login manager doesn't match - ignoring it");
+          yield this._clearLoginMgrData();
+        }
+      } else {
+        log.info("username in the login manager doesn't match - ignoring it");
+        yield this._clearLoginMgrData();
+      }
+    } catch (ex) {
+      log.error("Failed to get data from the login manager: ${}", ex);
+    }
+    return data;
+  }),
+
+}
+
 // A getter for the instance to export
 XPCOMUtils.defineLazyGetter(this, "fxAccounts", function() {
   let a = new FxAccounts();

services/fxaccounts/FxAccountsCommon.js

@@ -178,5 +178,18 @@ SERVER_ERRNO_TO_ERROR[ERRNO_INCORRECT_EMAIL_CASE]           = ERROR_INCORRECT_EM
 SERVER_ERRNO_TO_ERROR[ERRNO_SERVICE_TEMP_UNAVAILABLE]       = ERROR_SERVICE_TEMP_UNAVAILABLE;
 SERVER_ERRNO_TO_ERROR[ERRNO_UNKNOWN_ERROR]                  = ERROR_UNKNOWN;
 
+// FxAccounts has the ability to "split" the credentials between a plain-text
+// JSON file in the profile dir and in the login manager.
+// These constants relate to that.
+
+// The fields we save in the plaintext JSON.
+// See bug 1013064 comments 23-25 for why the sessionToken is "safe"
+this.FXA_PWDMGR_PLAINTEXT_FIELDS = ["email", "verified", "authAt",
+                                    "sessionToken", "uid"];
+// The pseudo-host we use in the login manager
+this.FXA_PWDMGR_HOST = "chrome://FirefoxAccounts";
+// The realm we use in the login manager.
+this.FXA_PWDMGR_REALM = "Firefox Accounts credentials";
+
 // Allow this file to be imported via Components.utils.import().
 this.EXPORTED_SYMBOLS = Object.keys(this);

services/fxaccounts/moz.build

@@ -10,12 +10,15 @@ TEST_DIRS += ['tests']
 
 EXTRA_JS_MODULES += [
   'Credentials.jsm',
-  'FxAccounts.jsm',
   'FxAccountsClient.jsm',
   'FxAccountsCommon.js',
   'FxAccountsOAuthClient.jsm',
 ]
 
+EXTRA_PP_JS_MODULES += [
+  'FxAccounts.jsm',
+]
+
 # For now, we will only be using the FxA manager in B2G.
 if CONFIG['MOZ_B2G']:
   EXTRA_JS_MODULES += ['FxAccountsManager.jsm']

services/fxaccounts/tests/xpcshell/test_loginmgr_storage.js

@@ -0,0 +1,196 @@
+/* Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/publicdomain/zero/1.0/ */
+
+"use strict";
+
+// Tests for FxAccounts, storage and the master password.
+
+// Stop us hitting the real auth server.
+Services.prefs.setCharPref("identity.fxaccounts.auth.uri", "http://localhost");
+
+Cu.import("resource://gre/modules/Services.jsm");
+Cu.import("resource://gre/modules/FxAccounts.jsm");
+Cu.import("resource://gre/modules/FxAccountsClient.jsm");
+Cu.import("resource://gre/modules/FxAccountsCommon.js");
+Cu.import("resource://gre/modules/osfile.jsm");
+Cu.import("resource://services-common/utils.js");
+Cu.import("resource://gre/modules/FxAccountsCommon.js");
+
+initTestLogging("Trace");
+// See verbose logging from FxAccounts.jsm
+Services.prefs.setCharPref("identity.fxaccounts.loglevel", "DEBUG");
+
+function run_test() {
+  run_next_test();
+}
+
+function getLoginMgrData() {
+  let logins = Services.logins.findLogins({}, FXA_PWDMGR_HOST, null, FXA_PWDMGR_REALM);
+  if (logins.length == 0) {
+    return null;
+  }
+  Assert.equal(logins.length, 1, "only 1 login available");
+  return logins[0];
+}
+
+add_task(function test_simple() {
+  let fxa = new FxAccounts({});
+
+  let creds = {
+    email: "test@example.com",
+    sessionToken: "sessionToken",
+    kA: "the kA value",
+    kB: "the kB value",
+    verified: true
+  };
+  yield fxa.setSignedInUser(creds);
+
+  // This should have stored stuff in both the .json file in the profile
+  // dir, and the login dir.
+  let path = OS.Path.join(OS.Constants.Path.profileDir, "signedInUser.json");
+  let data = yield CommonUtils.readJSON(path);
+
+  Assert.strictEqual(data.accountData.email, creds.email, "correct email in the clear text");
+  Assert.strictEqual(data.accountData.sessionToken, creds.sessionToken, "correct sessionToken in the clear text");
+  Assert.strictEqual(data.accountData.verified, creds.verified, "correct verified flag");
+
+  Assert.ok(!("kA" in data.accountData), "kA not stored in clear text");
+  Assert.ok(!("kB" in data.accountData), "kB not stored in clear text");
+
+  let login = getLoginMgrData();
+  Assert.strictEqual(login.username, creds.email, "email matches");
+  let loginData = JSON.parse(login.password);
+  Assert.strictEqual(loginData.version, data.version, "same version flag in both places");
+  Assert.strictEqual(loginData.accountData.kA, creds.kA, "correct kA in the login mgr");
+  Assert.strictEqual(loginData.accountData.kB, creds.kB, "correct kB in the login mgr");
+
+  Assert.ok(!("email" in loginData), "email not stored in the login mgr json");
+  Assert.ok(!("sessionToken" in loginData), "sessionToken not stored in the login mgr json");
+  Assert.ok(!("verified" in loginData), "verified not stored in the login mgr json");
+
+  yield fxa.signOut(/* localOnly = */ true);
+  Assert.strictEqual(getLoginMgrData(), null, "login mgr data deleted on logout");
+});
+
+add_task(function test_MPLocked() {
+  let fxa = new FxAccounts({});
+
+  let creds = {
+    email: "test@example.com",
+    sessionToken: "sessionToken",
+    kA: "the kA value",
+    kB: "the kB value",
+    verified: true
+  };
+
+  // tell the storage that the MP is locked.
+  fxa.internal.signedInUserStorage.__defineGetter__("_isLoggedIn", function() false);
+  yield fxa.setSignedInUser(creds);
+
+  // This should have stored stuff in the .json, and the login manager stuff
+  // will not exist.
+  let path = OS.Path.join(OS.Constants.Path.profileDir, "signedInUser.json");
+  let data = yield CommonUtils.readJSON(path);
+
+  Assert.strictEqual(data.accountData.email, creds.email, "correct email in the clear text");
+  Assert.strictEqual(data.accountData.sessionToken, creds.sessionToken, "correct sessionToken in the clear text");
+  Assert.strictEqual(data.accountData.verified, creds.verified, "correct verified flag");
+
+  Assert.ok(!("kA" in data.accountData), "kA not stored in clear text");
+  Assert.ok(!("kB" in data.accountData), "kB not stored in clear text");
+
+  Assert.strictEqual(getLoginMgrData(), null, "login mgr data doesn't exist");
+  yield fxa.signOut(/* localOnly = */ true)
+});
+
+add_task(function test_consistentWithMPEdgeCases() {
+  let fxa = new FxAccounts({});
+
+  let creds1 = {
+    email: "test@example.com",
+    sessionToken: "sessionToken",
+    kA: "the kA value",
+    kB: "the kB value",
+    verified: true
+  };
+
+  let creds2 = {
+    email: "test2@example.com",
+    sessionToken: "sessionToken2",
+    kA: "the kA value2",
+    kB: "the kB value2",
+    verified: false,
+  };
+
+  // Log a user in while MP is unlocked.
+  yield fxa.setSignedInUser(creds1);
+
+  // tell the storage that the MP is locked - this will prevent logout from
+  // being able to clear the data.
+  fxa.internal.signedInUserStorage.__defineGetter__("_isLoggedIn", function() false);
+
+  // now set the second credentials.
+  yield fxa.setSignedInUser(creds2);
+
+  // We should still have creds1 data in the login manager.
+  let login = getLoginMgrData();
+  Assert.strictEqual(login.username, creds1.email);
+  // and that we do have the first kA in the login manager.
+  Assert.strictEqual(JSON.parse(login.password).accountData.kA, creds1.kA,
+                     "stale data still in login mgr");
+
+  // Make a new FxA instance (otherwise the values in memory will be used.)
+  // Because we haven't overridden _isLoggedIn for this new instance it will
+  // treat the MP as unlocked.
+  let fxa = new FxAccounts({});
+
+  let accountData = yield fxa.getSignedInUser();
+  Assert.strictEqual(accountData.email, creds2.email);
+  // we should have no kA at all.
+  Assert.strictEqual(accountData.kA, undefined, "stale kA wasn't used");
+  yield fxa.signOut(/* localOnly = */ true)
+});
+
+add_task(function test_migration() {
+  // manually write out the full creds data to the JSON - this will look like
+  // old data that needs migration.
+  let creds = {
+    email: "test@example.com",
+    sessionToken: "sessionToken",
+    kA: "the kA value",
+    kB: "the kB value",
+    verified: true
+  };
+  let toWrite = {
+    version: 1,
+    accountData: creds,
+  };
+
+  let path = OS.Path.join(OS.Constants.Path.profileDir, "signedInUser.json");
+  let data = yield CommonUtils.writeJSON(toWrite, path);
+
+  // Create an FxA object - and tell it to load the data.
+  let fxa = new FxAccounts({});
+  data = yield fxa.getSignedInUser();
+
+  Assert.deepEqual(data, creds, "we should have everything available");
+
+  // now sniff the data on disk - it should have been magically migrated.
+  data = yield CommonUtils.readJSON(path);
+
+  Assert.strictEqual(data.accountData.email, creds.email, "correct email in the clear text");
+  Assert.strictEqual(data.accountData.sessionToken, creds.sessionToken, "correct sessionToken in the clear text");
+  Assert.strictEqual(data.accountData.verified, creds.verified, "correct verified flag");
+
+  Assert.ok(!("kA" in data.accountData), "kA not stored in clear text");
+  Assert.ok(!("kB" in data.accountData), "kB not stored in clear text");
+
+  // and it should magically be in the login manager.
+  let login = getLoginMgrData();
+  Assert.strictEqual(login.username, creds.email);
+  // and that we do have the first kA in the login manager.
+  Assert.strictEqual(JSON.parse(login.password).accountData.kA, creds.kA,
+                     "kA was migrated");
+
+  yield fxa.signOut(/* localOnly = */ true)
+});

services/fxaccounts/tests/xpcshell/xpcshell.ini

@@ -5,6 +5,8 @@ tail =
 [test_accounts.js]
 [test_client.js]
 [test_credentials.js]
+[test_loginmgr_storage.js]
+skip-if = appname == 'b2g' # login manager storage only used on desktop.
 [test_manager.js]
 run-if = appname == 'b2g'
 reason = FxAccountsManager is only available for B2G for now

services/sync/modules/util.js

@@ -19,6 +19,13 @@ Cu.import("resource://gre/modules/XPCOMUtils.jsm", this);
 Cu.import("resource://gre/modules/osfile.jsm", this);
 Cu.import("resource://gre/modules/Task.jsm", this);
 
+// FxAccountsCommon.js doesn't use a "namespace", so create one here.
+XPCOMUtils.defineLazyGetter(this, "FxAccountsCommon", function() {
+  let FxAccountsCommon = {};
+  Cu.import("resource://gre/modules/FxAccountsCommon.js", FxAccountsCommon);
+  return FxAccountsCommon;
+});
+
 /*
  * Utility functions
  */
@@ -594,8 +601,11 @@ this.Utils = {
       return this._syncCredentialsHosts;
     }
     let result = new Set();
-    // the legacy sync host.
+    // the legacy sync host
     result.add(PWDMGR_HOST);
+    // the FxA host
+    result.add(FxAccountsCommon.FXA_PWDMGR_HOST);
+    //
     // The FxA hosts - these almost certainly all have the same hostname, but
     // better safe than sorry...
     for (let prefName of ["identity.fxaccounts.remote.force_auth.uri",