path: root/data/patches/0002-Bug-1013064-part-2-Store-sensitive-FxA-credentials-i.patch

From 4a92f9ee1ba35989f82a24bba18806f8616a5be8 Mon Sep 17 00:00:00 2001
From: Mark Hammond <mhammond@skippinet.com.au>
Date: Sat, 14 Jun 2014 14:33:20 +1000
Subject: Bug 1013064 (part 2) - Store sensitive FxA credentials in the login
 manager. r=ckarlof,dveditz

---
 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

diff --git a/services/fxaccounts/FxAccounts.jsm b/services/fxaccounts/FxAccounts.jsm
index cc521b8..e9328dd 100644
--- 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();
 
diff --git a/services/fxaccounts/FxAccountsCommon.js b/services/fxaccounts/FxAccountsCommon.js
index 2bef093..476e528 100644
--- a/services/fxaccounts/FxAccountsCommon.js
+++ b/services/fxaccounts/FxAccountsCommon.js
@@ -170,10 +170,23 @@ SERVER_ERRNO_TO_ERROR[ERRNO_INVALID_AUTH_NONCE]             = ERROR_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);
diff --git a/services/fxaccounts/moz.build b/services/fxaccounts/moz.build
index f959714..1bb8c09 100644
--- 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']
diff --git a/services/fxaccounts/tests/xpcshell/test_loginmgr_storage.js b/services/fxaccounts/tests/xpcshell/test_loginmgr_storage.js
new file mode 100644
index 0000000..297b256
--- /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)
+});
diff --git a/services/fxaccounts/tests/xpcshell/xpcshell.ini b/services/fxaccounts/tests/xpcshell/xpcshell.ini
index 9d9bffe..4448d76 100644
--- 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
diff --git a/services/sync/modules/util.js b/services/sync/modules/util.js
index 194e0b2..4691428 100644
--- a/services/sync/modules/util.js
+++ b/services/sync/modules/util.js
@@ -14,16 +14,23 @@ Cu.import("resource://services-common/async.js", this);
 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 {
-- 
1.8.3.msysgit.0