Bug 1437416 - remove dead code that attempted to provide a default salt and verify the code actually works regardless. r=rfkelly,rnewman
authorMark Hammond <mhammond@skippinet.com.au>
Thu, 15 Feb 2018 11:55:32 +1100
changeset 759103 d3363d47276bf078c00d6d3bd9d231ebbce4c4b4
parent 759102 946a3f94fc70a7f1ba31467c21a24710a7958921
child 759104 9d9d274ee33699c3c1a85ec689e0491d71431f3f
push id100272
push userrwood@mozilla.com
push dateFri, 23 Feb 2018 18:27:33 +0000
reviewersrfkelly, rnewman
bugs1437416
milestone60.0a1
Bug 1437416 - remove dead code that attempted to provide a default salt and verify the code actually works regardless. r=rfkelly,rnewman MozReview-Commit-ID: GKII0jjVtSf
services/crypto/modules/utils.js
services/crypto/tests/unit/test_utils_hkdfExpand.js
--- a/services/crypto/modules/utils.js
+++ b/services/crypto/modules/utils.js
@@ -139,21 +139,16 @@ this.CryptoUtils = {
     hasher.init(type, key);
     return hasher;
   },
 
   /**
    * HMAC-based Key Derivation (RFC 5869).
    */
   hkdf: function hkdf(ikm, xts, info, len) {
-    if (typeof xts === undefined)
-      xts = String.fromCharCode(0, 0, 0, 0, 0, 0, 0, 0,
-                                0, 0, 0, 0, 0, 0, 0, 0,
-                                0, 0, 0, 0, 0, 0, 0, 0,
-                                0, 0, 0, 0, 0, 0, 0, 0);
     let h = CryptoUtils.makeHMACHasher(Ci.nsICryptoHMAC.SHA256,
                                        CryptoUtils.makeHMACKey(xts));
     let prk = CryptoUtils.digestBytes(ikm, h);
     return CryptoUtils.hkdfExpand(prk, info, len);
   },
 
   /**
    * HMAC-based Key Derivation Step 2 according to RFC 5869.
--- a/services/crypto/tests/unit/test_utils_hkdfExpand.js
+++ b/services/crypto/tests/unit/test_utils_hkdfExpand.js
@@ -96,25 +96,57 @@ function expand_hex(prk, info, len) {
 function hkdf_hex(ikm, salt, info, len) {
   ikm = _hexToString(ikm);
   if (salt)
     salt = _hexToString(salt);
   info = _hexToString(info);
   return CommonUtils.bytesAsHex(CryptoUtils.hkdf(ikm, salt, info, len));
 }
 
+// In bug 1437416 we thought we supplied a default for the salt but
+// actually ended up calling the platform c++ code with undefined as the
+// salt - which still ended up doing the right thing. Let's try and
+// codify that behaviour.
+let hkdf_tc1 = {
+  ikm: "foo",
+  info: "bar",
+  salt: undefined,
+  len: 64,
+  // As all inputs are known, we can pre-calculate the expected result:
+  // >>> tokenlib.utils.HKDF("foo", None, "bar", 64).encode("hex")
+  // 'f037f3ab189f485d0d93249f432def681a0305e39ef85f810e2f0b74d2078861fbd34318934b49de822c6148c8bb0785613e4b01176b47634e25eecd5e94ff3b'
+  result: "f037f3ab189f485d0d93249f432def681a0305e39ef85f810e2f0b74d2078861fbd34318934b49de822c6148c8bb0785613e4b01176b47634e25eecd5e94ff3b",
+};
+
+// same inputs, but this time with the default salt explicitly defined.
+// should give the same result.
+let hkdf_tc2 = {
+  ikm: "foo",
+  info: "bar",
+  salt: String.fromCharCode(0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+                            0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0),
+  len: 64,
+  result: hkdf_tc1.result,
+};
+
 function run_test() {
   _("Verifying Test Case 1");
   Assert.equal(extract_hex(tc1.salt, tc1.IKM), tc1.PRK);
   Assert.equal(expand_hex(tc1.PRK, tc1.info, tc1.L), tc1.OKM);
   Assert.equal(hkdf_hex(tc1.IKM, tc1.salt, tc1.info, tc1.L), tc1.OKM);
 
   _("Verifying Test Case 2");
   Assert.equal(extract_hex(tc2.salt, tc2.IKM), tc2.PRK);
   Assert.equal(expand_hex(tc2.PRK, tc2.info, tc2.L), tc2.OKM);
   Assert.equal(hkdf_hex(tc2.IKM, tc2.salt, tc2.info, tc2.L), tc2.OKM);
 
   _("Verifying Test Case 3");
   Assert.equal(extract_hex(tc3.salt, tc3.IKM), tc3.PRK);
   Assert.equal(expand_hex(tc3.PRK, tc3.info, tc3.L), tc3.OKM);
   Assert.equal(hkdf_hex(tc3.IKM, tc3.salt, tc3.info, tc3.L), tc3.OKM);
   Assert.equal(hkdf_hex(tc3.IKM, undefined, tc3.info, tc3.L), tc3.OKM);
+
+  _("Verifying hkdf semantics");
+  for (let tc of [hkdf_tc1, hkdf_tc2]) {
+    let result = CommonUtils.bytesAsHex(CryptoUtils.hkdf(tc.ikm, tc.salt, tc.info, tc.len));
+    Assert.equal(result, tc.result);
+  }
 }