Bug 1013064 (part 2) - Store sensitive FxA credentials in the login manager. r=ckarlof
authorMark Hammond <mhammond@skippinet.com.au>
Sat, 14 Jun 2014 14:33:20 +1000
changeset 214982 11f3a97d1d2c1a3ddbecd37cb7ed2d9a91d380bf
parent 214981 e5dfe9801f76b1694bd83036cdf69856779a8f5a
child 214983 e2374762f521eb97ad90be7105c0a77b84aeaadd
push id3857
push userraliiev@mozilla.com
push dateTue, 02 Sep 2014 16:39:23 +0000
treeherdermozilla-beta@5638b907b505 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersckarlof
bugs1013064, 100644
milestone33.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1013064 (part 2) - Store sensitive FxA credentials in the login manager. r=ckarlof 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
services/fxaccounts/FxAccountsCommon.js
services/fxaccounts/moz.build
services/fxaccounts/tests/xpcshell/test_loginmgr_storage.js
services/fxaccounts/tests/xpcshell/xpcshell.ini
services/sync/modules/util.js
--- a/services/fxaccounts/FxAccounts.jsm
+++ b/services/fxaccounts/FxAccounts.jsm
@@ -298,17 +298,22 @@ function FxAccountsInternal() {
   // able to abort all work on the first sign-in process.  The currentTimer and
   // currentAccountState are used for this purpose.
   // (XXX - should the timer be directly on the currentAccountState?)
   this.currentTimer = null;
   this.currentAccountState = new AccountState(this);
 
   // 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,
   });
 }
 
 /**
  * The internal API's prototype.
  */
@@ -896,16 +901,181 @@ JSONStorage.prototype = {
       .then(CommonUtils.writeJSON.bind(null, contents, this.path));
   },
 
   get: function() {
     return CommonUtils.readJSON(this.path);
   }
 };
 
+/**
+ * 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();
 
   // XXX Bug 947061 - We need a strategy for resuming email verification after
   // browser restart
   a.loadAndPoll();
 
--- a/services/fxaccounts/FxAccountsCommon.js
+++ b/services/fxaccounts/FxAccountsCommon.js
@@ -173,10 +173,23 @@ SERVER_ERRNO_TO_ERROR[ERRNO_INVALID_AUTH
 SERVER_ERRNO_TO_ERROR[ERRNO_ENDPOINT_NO_LONGER_SUPPORTED]   = ERROR_ENDPOINT_NO_LONGER_SUPPORTED;
 SERVER_ERRNO_TO_ERROR[ERRNO_INCORRECT_LOGIN_METHOD]         = ERROR_INCORRECT_LOGIN_METHOD;
 SERVER_ERRNO_TO_ERROR[ERRNO_INCORRECT_KEY_RETRIEVAL_METHOD] = ERROR_INCORRECT_KEY_RETRIEVAL_METHOD;
 SERVER_ERRNO_TO_ERROR[ERRNO_INCORRECT_API_VERSION]          = ERROR_INCORRECT_API_VERSION;
 SERVER_ERRNO_TO_ERROR[ERRNO_INCORRECT_EMAIL_CASE]           = ERROR_INCORRECT_EMAIL_CASE;
 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);
--- a/services/fxaccounts/moz.build
+++ b/services/fxaccounts/moz.build
@@ -5,16 +5,19 @@
 # file, You can obtain one at http://mozilla.org/MPL/2.0/.
 
 PARALLEL_DIRS += ['interfaces']
 
 TEST_DIRS += ['tests']
 
 EXTRA_JS_MODULES += [
   'Credentials.jsm',
-  'FxAccounts.jsm',
   'FxAccountsClient.jsm',
   'FxAccountsCommon.js'
 ]
 
+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']
new file mode 100644
--- /dev/null
+++ b/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)
+});
--- a/services/fxaccounts/tests/xpcshell/xpcshell.ini
+++ b/services/fxaccounts/tests/xpcshell/xpcshell.ini
@@ -1,10 +1,12 @@
 [DEFAULT]
 head = head.js ../../../common/tests/unit/head_helpers.js ../../../common/tests/unit/head_http.js
 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
--- a/services/sync/modules/util.js
+++ b/services/sync/modules/util.js
@@ -14,16 +14,23 @@ Cu.import("resource://services-common/as
 Cu.import("resource://services-crypto/utils.js");
 Cu.import("resource://services-sync/constants.js");
 Cu.import("resource://gre/modules/Preferences.jsm");
 Cu.import("resource://gre/modules/Services.jsm", this);
 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
  */
 
 this.Utils = {
   // Alias in functions from CommonUtils. These previously were defined here.
   // In the ideal world, references to these would be removed.
   nextTick: CommonUtils.nextTick,
@@ -589,18 +596,21 @@ this.Utils = {
    * reset when we drop sync credentials, etc.
    */
   getSyncCredentialsHosts: function() {
     // This is somewhat expensive and the result static, so we cache the result.
     if (this._syncCredentialsHosts) {
       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",
                           "identity.fxaccounts.remote.signup.uri",
                           "identity.fxaccounts.remote.signin.uri",
                           "identity.fxaccounts.settings.uri"]) {
       let prefVal;
       try {