Bug 1368560 part 1 - Remove un-used legacy crypto methods. r?markh draft
authorEdouard Oger <eoger@fastmail.com>
Mon, 29 May 2017 17:27:13 -0400
changeset 587167 30e71e2eb7be081d68a62f371efa5e4ad5ef59dd
parent 586973 7b8937970f9ca85db88cb2496f2112175fd847c8
child 587168 ad3281dbc49371307dd7388c9e2fe7d35f61d806
child 587939 a3abb7fce54b0d35e5f1877e86727a52405ff81e
child 587953 4623bc1e6cb2b680ac146b5c93a0b5d0b6a591e5
push id61642
push userbmo:eoger@fastmail.com
push dateWed, 31 May 2017 17:11:22 +0000
reviewersmarkh
bugs1368560
milestone55.0a1
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.