Bug 1368560 part 1 - Remove un-used legacy crypto methods. r=markh
authorEdouard Oger <eoger@fastmail.com>
Mon, 29 May 2017 17:27:13 -0400
changeset 361925 732d5e2ec5d77c829d22f40669b8927b7a257d25
parent 361924 e8e908b731ad73478a68c39f87427341409ac5fe
child 361926 d71442ad6f31e3b5053e64adf5ed5765cb4bc025
push id31952
push usercbook@mozilla.com
push dateFri, 02 Jun 2017 12:17:25 +0000
treeherdermozilla-central@194c009d6295 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmarkh
bugs1368560
milestone55.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 1368560 part 1 - Remove un-used legacy crypto methods. r=markh MozReview-Commit-ID: JbzweOMFLpR
services/crypto/modules/WeaveCrypto.js
services/crypto/modules/utils.js
services/crypto/tests/unit/test_crypto_deriveKey.js
services/crypto/tests/unit/xpcshell.ini
services/sync/modules-testing/fakeservices.js
services/sync/modules/util.js
services/sync/tests/unit/test_utils_deriveKey.js
services/sync/tests/unit/test_utils_passphrase.js
services/sync/tests/unit/xpcshell.ini
--- a/services/crypto/modules/WeaveCrypto.js
+++ b/services/crypto/modules/WeaveCrypto.js
@@ -225,38 +225,9 @@ WeaveCrypto.prototype = {
     },
 
     makeUint8Array(input, isEncoded) {
         if (isEncoded) {
             input = atob(input);
         }
         return this.byteCompressInts(input);
     },
-
-    /**
-     * Returns the expanded data string for the derived key.
-     */
-    deriveKeyFromPassphrase(passphrase, saltStr, keyLength = 32) {
-        this.log("deriveKeyFromPassphrase() called.");
-        let keyData = this.makeUint8Array(passphrase, false);
-        let salt = this.makeUint8Array(saltStr, true);
-        let importAlgo = { name: KEY_DERIVATION_ALGO };
-        let deriveAlgo = {
-            name: KEY_DERIVATION_ALGO,
-            salt,
-            iterations: KEY_DERIVATION_ITERATIONS,
-            hash: { name: KEY_DERIVATION_HASHING_ALGO },
-        };
-        let derivedKeyType = {
-            name: DERIVED_KEY_ALGO,
-            length: keyLength * 8,
-        };
-        return Async.promiseSpinningly(
-            crypto.subtle.importKey("raw", keyData, importAlgo, false, ["deriveKey"])
-            .then(key => crypto.subtle.deriveKey(deriveAlgo, key, derivedKeyType, true, []))
-            .then(derivedKey => crypto.subtle.exportKey("raw", derivedKey))
-            .then(keyBytes => {
-                keyBytes = new Uint8Array(keyBytes);
-                return this.expandData(keyBytes);
-            })
-        );
-    },
 };
--- a/services/crypto/modules/utils.js
+++ b/services/crypto/modules/utils.js
@@ -248,29 +248,16 @@ this.CryptoUtils = {
     for (let i = 0; i < l - 1;) {
       ret += T[i++];
     }
     ret += T[l - 1].substr(0, r);
 
     return ret;
   },
 
-  deriveKeyFromPassphrase: function deriveKeyFromPassphrase(passphrase,
-                                                            salt,
-                                                            keyLength,
-                                                            forceJS) {
-    if (Svc.Crypto.deriveKeyFromPassphrase && !forceJS) {
-      return Svc.Crypto.deriveKeyFromPassphrase(passphrase, salt, keyLength);
-    }
-    // Fall back to JS implementation.
-    // 4096 is hardcoded in WeaveCrypto, so do so here.
-    return CryptoUtils.pbkdf2Generate(passphrase, atob(salt), 4096,
-                                      keyLength);
-  },
-
   /**
    * Compute the HTTP MAC SHA-1 for an HTTP request.
    *
    * @param  identifier
    *         (string) MAC Key Identifier.
    * @param  key
    *         (string) MAC Key.
    * @param  method
@@ -557,24 +544,15 @@ XPCOMUtils.defineLazyGetter(CryptoUtils,
 
 var Svc = {};
 
 XPCOMUtils.defineLazyServiceGetter(Svc,
                                    "KeyFactory",
                                    "@mozilla.org/security/keyobjectfactory;1",
                                    "nsIKeyObjectFactory");
 
-Svc.__defineGetter__("Crypto", function() {
-  let ns = {};
-  Cu.import("resource://services-crypto/WeaveCrypto.js", ns);
-
-  let wc = new ns.WeaveCrypto();
-  delete Svc.Crypto;
-  return Svc.Crypto = wc;
-});
-
 Observers.add("xpcom-shutdown", function unloadServices() {
   Observers.remove("xpcom-shutdown", unloadServices);
 
   for (let k in Svc) {
     delete Svc[k];
   }
 });
deleted file mode 100644
--- a/services/crypto/tests/unit/test_crypto_deriveKey.js
+++ /dev/null
@@ -1,28 +0,0 @@
-Components.utils.import("resource://services-crypto/WeaveCrypto.js");
-
-function run_test() {
-  let cryptoSvc = new WeaveCrypto();
-  // Extracted from test_utils_deriveKey.
-  let pp = "secret phrase";
-  let salt = "RE5YUHpQcGl3bg==";   // btoa("DNXPzPpiwn")
-
-  // 16-byte, extract key data.
-  let k = cryptoSvc.deriveKeyFromPassphrase(pp, salt, 16);
-  do_check_eq(16, k.length);
-  do_check_eq(btoa(k), "d2zG0d2cBfXnRwMUGyMwyg==");
-
-  // Test different key lengths.
-  k = cryptoSvc.deriveKeyFromPassphrase(pp, salt, 32);
-  do_check_eq(32, k.length);
-  do_check_eq(btoa(k), "d2zG0d2cBfXnRwMUGyMwyroRXtnrSIeLwSDvReSfcyA=");
-  let encKey = btoa(k);
-
-  // Test via encryption.
-  let iv = cryptoSvc.generateRandomIV();
-  do_check_eq(cryptoSvc.decrypt(cryptoSvc.encrypt("bacon", encKey, iv), encKey, iv), "bacon");
-
-  // Test default length (32).
-  k = cryptoSvc.deriveKeyFromPassphrase(pp, salt);
-  do_check_eq(32, k.length);
-  do_check_eq(encKey, btoa(k));
-}
--- a/services/crypto/tests/unit/xpcshell.ini
+++ b/services/crypto/tests/unit/xpcshell.ini
@@ -2,17 +2,16 @@
 head = head_helpers.js ../../../common/tests/unit/head_helpers.js
 firefox-appdir = browser
 support-files =
   !/services/common/tests/unit/head_helpers.js
 
 [test_load_modules.js]
 
 [test_crypto_crypt.js]
-[test_crypto_deriveKey.js]
 [test_crypto_random.js]
 # Bug 676977: test hangs consistently on Android
 skip-if = os == "android"
 [test_crypto_service.js]
 skip-if = (os == "android" || appname == 'thunderbird')
 [test_jwcrypto.js]
 skip-if = (os == "android" || appname == 'thunderbird')
 
--- a/services/sync/modules-testing/fakeservices.js
+++ b/services/sync/modules-testing/fakeservices.js
@@ -120,18 +120,13 @@ FakeCryptoService.prototype = {
     // A base64-encoded IV is 24 characters long
     return btoa("fake-fake-fake-random-iv");
   },
 
   expandData: function expandData(data, len) {
     return data;
   },
 
-  deriveKeyFromPassphrase: function deriveKeyFromPassphrase(passphrase,
-                                                            salt, keyLength) {
-    return "some derived key string composed of bytes";
-  },
-
   generateRandomBytes: function generateRandomBytes(byteCount) {
     return "not-so-random-now-are-we-HA-HA-HA! >:)".slice(byteCount);
   }
 };
 
--- a/services/sync/modules/util.js
+++ b/services/sync/modules/util.js
@@ -49,17 +49,16 @@ this.Utils = {
   digestBytes: CryptoUtils.digestBytes,
   sha1: CryptoUtils.sha1,
   sha1Base32: CryptoUtils.sha1Base32,
   sha256: CryptoUtils.sha256,
   makeHMACKey: CryptoUtils.makeHMACKey,
   makeHMACHasher: CryptoUtils.makeHMACHasher,
   hkdfExpand: CryptoUtils.hkdfExpand,
   pbkdf2Generate: CryptoUtils.pbkdf2Generate,
-  deriveKeyFromPassphrase: CryptoUtils.deriveKeyFromPassphrase,
   getHTTPMACSHA1Header: CryptoUtils.getHTTPMACSHA1Header,
 
   /**
    * The string to use as the base User-Agent in Sync requests.
    * This string will look something like
    *
    *   Firefox/49.0a1 (Windows NT 6.1; WOW64; rv:46.0) FxSync/1.51.0.20160516142357.desktop
    */
@@ -283,49 +282,16 @@ this.Utils = {
 
   decodeKeyBase32: function decodeKeyBase32(encoded) {
     return Utils.decodeBase32(
              Utils.base32FromFriendly(
                Utils.normalizePassphrase(encoded)))
            .slice(0, SYNC_KEY_DECODED_LENGTH);
   },
 
-  base64Key: function base64Key(keyData) {
-    return btoa(keyData);
-  },
-
-  /**
-   * N.B., salt should be base64 encoded, even though we have to decode
-   * it later!
-   */
-  derivePresentableKeyFromPassphrase: function derivePresentableKeyFromPassphrase(passphrase, salt, keyLength, forceJS) {
-    let k = CryptoUtils.deriveKeyFromPassphrase(passphrase, salt, keyLength,
-                                                forceJS);
-    return Utils.encodeKeyBase32(k);
-  },
-
-  /**
-   * N.B., salt should be base64 encoded, even though we have to decode
-   * it later!
-   */
-  deriveEncodedKeyFromPassphrase: function deriveEncodedKeyFromPassphrase(passphrase, salt, keyLength, forceJS) {
-    let k = CryptoUtils.deriveKeyFromPassphrase(passphrase, salt, keyLength,
-                                                forceJS);
-    return Utils.base64Key(k);
-  },
-
-  /**
-   * Take a base64-encoded 128-bit AES key, returning it as five groups of five
-   * uppercase alphanumeric characters, separated by hyphens.
-   * A.K.A. base64-to-base32 encoding.
-   */
-  presentEncodedKeyAsSyncKey: function presentEncodedKeyAsSyncKey(encodedKey) {
-    return Utils.encodeKeyBase32(atob(encodedKey));
-  },
-
   jsonFilePath(filePath) {
     return OS.Path.normalize(OS.Path.join(OS.Constants.Path.profileDir, "weave", filePath + ".json"));
   },
 
   /**
    * Load a JSON file from disk in the profile directory.
    *
    * @param filePath
@@ -447,83 +413,34 @@ this.Utils = {
                             ...(filePath + ".json").split("/"));
     if (that._log) {
       that._log.trace("Deleting " + path);
     }
     return OS.File.remove(path, { ignoreAbsent: true });
   },
 
   /**
-   * Generate 26 characters.
-   */
-  generatePassphrase: function generatePassphrase() {
-    // Note that this is a different base32 alphabet to the one we use for
-    // other tasks. It's lowercase, uses different letters, and needs to be
-    // decoded with decodeKeyBase32, not just decodeBase32.
-    return Utils.encodeKeyBase32(CryptoUtils.generateRandomBytes(16));
-  },
-
-  /**
    * The following are the methods supported for UI use:
    *
    * * isPassphrase:
    *     determines whether a string is either a normalized or presentable
    *     passphrase.
-   * * hyphenatePassphrase:
-   *     present a normalized passphrase for display. This might actually
-   *     perform work beyond just hyphenation; sorry.
-   * * hyphenatePartialPassphrase:
-   *     present a fragment of a normalized passphrase for display.
    * * normalizePassphrase:
    *     take a presentable passphrase and reduce it to a normalized
    *     representation for storage. normalizePassphrase can safely be called
    *     on normalized input.
    */
 
   isPassphrase(s) {
     if (s) {
       return /^[abcdefghijkmnpqrstuvwxyz23456789]{26}$/.test(Utils.normalizePassphrase(s));
     }
     return false;
   },
 
-  /**
-   * Hyphenate a passphrase (26 characters) into groups.
-   * abbbbccccddddeeeeffffggggh
-   * =>
-   * a-bbbbc-cccdd-ddeee-effff-ggggh
-   */
-  hyphenatePassphrase: function hyphenatePassphrase(passphrase) {
-    // For now, these are the same.
-    return Utils.hyphenatePartialPassphrase(passphrase, true);
-  },
-
-  hyphenatePartialPassphrase: function hyphenatePartialPassphrase(passphrase, omitTrailingDash) {
-    if (!passphrase)
-      return null;
-
-    // Get the raw data input. Just base32.
-    let data = passphrase.toLowerCase().replace(/[^abcdefghijkmnpqrstuvwxyz23456789]/g, "");
-
-    // This is the neatest way to do this.
-    if ((data.length == 1) && !omitTrailingDash)
-      return data + "-";
-
-    // Hyphenate it.
-    let y = data.substr(0, 1);
-    let z = data.substr(1).replace(/(.{1,5})/g, "-$1");
-
-    // Correct length? We're done.
-    if ((z.length == 30) || omitTrailingDash)
-      return y + z;
-
-    // Add a trailing dash if appropriate.
-    return (y + z.replace(/([^-]{5})$/, "$1-")).substr(0, SYNC_KEY_HYPHENATED_LENGTH);
-  },
-
   normalizePassphrase: function normalizePassphrase(pp) {
     // Short var name... have you seen the lines below?!
     // Allow leading and trailing whitespace.
     pp = pp.trim().toLowerCase();
 
     // 20-char sync key.
     if (pp.length == 23 &&
         [5, 11, 17].every(i => pp[i] == "-")) {
deleted file mode 100644
--- a/services/sync/tests/unit/test_utils_deriveKey.js
+++ /dev/null
@@ -1,66 +0,0 @@
-Cu.import("resource://services-crypto/WeaveCrypto.js");
-Cu.import("resource://services-sync/util.js");
-
-var cryptoSvc = new WeaveCrypto();
-
-function run_test() {
-  if (this.gczeal) {
-    _("Running deriveKey tests with gczeal(2).");
-    gczeal(2);
-  } else {
-    _("Running deriveKey tests with default gczeal.");
-  }
-
-  var iv = cryptoSvc.generateRandomIV();
-  var der_passphrase = "secret phrase";
-  var der_salt = "RE5YUHpQcGl3bg==";   // btoa("DNXPzPpiwn")
-
-  _("Testing deriveKeyFromPassphrase. Input is \"" + der_passphrase + "\", \"" + der_salt + "\" (base64-encoded).");
-
-  // Test friendly-ing.
-  do_check_eq("abcdefghijk8mn9pqrstuvwxyz234567",
-              Utils.base32ToFriendly("ABCDEFGHIJKLMNOPQRSTUVWXYZ234567"));
-  do_check_eq("ABCDEFGHIJKLMNOPQRSTUVWXYZ234567",
-              Utils.base32FromFriendly(
-                Utils.base32ToFriendly("ABCDEFGHIJKLMNOPQRSTUVWXYZ234567")));
-
-  // Test translation.
-  do_check_false(Utils.isPassphrase("o-5wmnu-o5tqc-7lz2h-amkbw-izqzi"));  // Wrong charset.
-  do_check_false(Utils.isPassphrase("O-5WMNU-O5TQC-7LZ2H-AMKBW-IZQZI"));  // Wrong charset.
-  do_check_true(Utils.isPassphrase("9-5wmnu-95tqc-78z2h-amkbw-izqzi"));
-  do_check_true(Utils.isPassphrase("9-5WMNU-95TQC-78Z2H-AMKBW-IZQZI"));   // isPassphrase normalizes.
-  do_check_true(Utils.isPassphrase(
-      Utils.normalizePassphrase("9-5WMNU-95TQC-78Z2H-AMKBW-IZQZI")));
-
-  // Base64. We don't actually use this in anger, particularly not with a 32-byte key.
-  var der_key = Utils.deriveEncodedKeyFromPassphrase(der_passphrase, der_salt);
-  _("Derived key in base64: " + der_key);
-  do_check_eq(cryptoSvc.decrypt(cryptoSvc.encrypt("bacon", der_key, iv), der_key, iv), "bacon");
-
-  // Base64, 16-byte output.
-  der_key = Utils.deriveEncodedKeyFromPassphrase(der_passphrase, der_salt, 16);
-  _("Derived key in base64: " + der_key);
-  do_check_eq("d2zG0d2cBfXnRwMUGyMwyg==", der_key);
-  do_check_eq(cryptoSvc.decrypt(cryptoSvc.encrypt("bacon", der_key, iv), der_key, iv), "bacon");
-
-  // Base32. Again, specify '16' to avoid it generating a 256-bit key string.
-  var b32key = Utils.derivePresentableKeyFromPassphrase(der_passphrase, der_salt, 16);
-  var hyphenated = Utils.hyphenatePassphrase(b32key);
-  do_check_true(Utils.isPassphrase(b32key));
-
-  _("Derived key in base32: " + b32key);
-  do_check_eq(b32key.length, 26);
-  do_check_eq(hyphenated.length, 31);  // 1 char, plus 5 groups of 5, hyphenated = 5 + (5*5) + 1 = 31.
-  do_check_eq(hyphenated, "9-5wmnu-95tqc-78z2h-amkbw-izqzi");
-
-  if (this.gczeal)
-    gczeal(0);
-
-  // Test the equivalence of our NSS and JS versions.
-  // Will only work on FF4, of course.
-  // Note that we don't add gczeal here: the pure-JS implementation is
-  // astonishingly slow, and this check takes five minutes to run.
-  do_check_eq(
-      Utils.deriveEncodedKeyFromPassphrase(der_passphrase, der_salt, 16, false),
-      Utils.deriveEncodedKeyFromPassphrase(der_passphrase, der_salt, 16, true));
-}
--- a/services/sync/tests/unit/test_utils_passphrase.js
+++ b/services/sync/tests/unit/test_utils_passphrase.js
@@ -1,56 +1,14 @@
 Cu.import("resource://services-sync/util.js");
 
 function run_test() {
-  _("Generated passphrase has length 26.");
-  let pp = Utils.generatePassphrase();
-  do_check_eq(pp.length, 26);
-
-  const key = "abcdefghijkmnpqrstuvwxyz23456789";
-  _("Passphrase only contains [" + key + "].");
-  do_check_true(pp.split("").every(chr => key.indexOf(chr) != -1));
-
-  _("Hyphenated passphrase has 5 hyphens.");
-  let hyphenated = Utils.hyphenatePassphrase(pp);
-  _("H: " + hyphenated);
-  do_check_eq(hyphenated.length, 31);
-  do_check_eq(hyphenated[1], "-");
-  do_check_eq(hyphenated[7], "-");
-  do_check_eq(hyphenated[13], "-");
-  do_check_eq(hyphenated[19], "-");
-  do_check_eq(hyphenated[25], "-");
-  do_check_eq(pp,
-      hyphenated.slice(0, 1) + hyphenated.slice(2, 7)
-      + hyphenated.slice(8, 13) + hyphenated.slice(14, 19)
-      + hyphenated.slice(20, 25) + hyphenated.slice(26, 31));
-
-  _("Arbitrary hyphenation.");
-  // We don't allow invalid characters for our base32 character set.
-  do_check_eq(Utils.hyphenatePassphrase("1234567"), "2-34567");  // Not partial, so no trailing dash.
-  do_check_eq(Utils.hyphenatePassphrase("1234567890"), "2-34567-89");
-  do_check_eq(Utils.hyphenatePassphrase("abcdeabcdeabcdeabcdeabcde"), "a-bcdea-bcdea-bcdea-bcdea-bcde");
-  do_check_eq(Utils.hyphenatePartialPassphrase("1234567"), "2-34567-");
-  do_check_eq(Utils.hyphenatePartialPassphrase("1234567890"), "2-34567-89");
-  do_check_eq(Utils.hyphenatePartialPassphrase("abcdeabcdeabcdeabcdeabcde"), "a-bcdea-bcdea-bcdea-bcdea-bcde");
-
-  do_check_eq(Utils.hyphenatePartialPassphrase("a"), "a-");
-  do_check_eq(Utils.hyphenatePartialPassphrase("1234567"), "2-34567-");
-  do_check_eq(Utils.hyphenatePartialPassphrase("a-bcdef-g"),
-              "a-bcdef-g");
-  do_check_eq(Utils.hyphenatePartialPassphrase("abcdefghijklmnop"),
-              "a-bcdef-ghijk-mnp");
-  do_check_eq(Utils.hyphenatePartialPassphrase("abcdefghijklmnopabcde"),
-              "a-bcdef-ghijk-mnpab-cde");
-  do_check_eq(Utils.hyphenatePartialPassphrase("a-bcdef-ghijk-LMNOP-ABCDE-Fg"),
-              "a-bcdef-ghijk-mnpab-cdefg-");
-  // Cuts off.
-  do_check_eq(Utils.hyphenatePartialPassphrase("aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa").length, 31);
-
   _("Normalize passphrase recognizes hyphens.");
+  const pp = "26ect2thczm599m2ffqarbicjq";
+  const hyphenated = "2-6ect2-thczm-599m2-ffqar-bicjq";
   do_check_eq(Utils.normalizePassphrase(hyphenated), pp);
 
   _("Skip whitespace.");
   do_check_eq("aaaaaaaaaaaaaaaaaaaaaaaaaa", Utils.normalizePassphrase("aaaaaaaaaaaaaaaaaaaaaaaaaa  "));
   do_check_eq("aaaaaaaaaaaaaaaaaaaaaaaaaa", Utils.normalizePassphrase("	 aaaaaaaaaaaaaaaaaaaaaaaaaa"));
   do_check_eq("aaaaaaaaaaaaaaaaaaaaaaaaaa", Utils.normalizePassphrase("    aaaaaaaaaaaaaaaaaaaaaaaaaa  "));
   do_check_eq("aaaaaaaaaaaaaaaaaaaaaaaaaa", Utils.normalizePassphrase("    a-aaaaa-aaaaa-aaaaa-aaaaa-aaaaa  "));
   do_check_true(Utils.isPassphrase("aaaaaaaaaaaaaaaaaaaaaaaaaa  "));
--- a/services/sync/tests/unit/xpcshell.ini
+++ b/services/sync/tests/unit/xpcshell.ini
@@ -21,17 +21,16 @@ support-files =
 
 # Ensure we can import everything.
 [test_load_modules.js]
 
 # util contains a bunch of functionality used throughout.
 [test_utils_catch.js]
 [test_utils_deepEquals.js]
 [test_utils_deferGetSet.js]
-[test_utils_deriveKey.js]
 [test_utils_keyEncoding.js]
 [test_utils_json.js]
 [test_utils_lock.js]
 [test_utils_makeGUID.js]
 [test_utils_notify.js]
 [test_utils_passphrase.js]
 
 # We have a number of other libraries that are pretty much standalone.