From 26e11a8c361e182f79af649531a6537fc760485a Mon Sep 17 00:00:00 2001
From: Mark Hammond <mhammond@skippinet.com.au>
Date: Thu, 15 Feb 2018 11:55:32 +1100
Subject: [PATCH] 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

--HG--
extra : rebase_source : d595d3f3d0ed9f6186ab93ca86a2833593de5d33
---
 services/crypto/modules/utils.js              |  5 ---
 .../tests/unit/test_utils_hkdfExpand.js       | 32 +++++++++++++++++++
 2 files changed, 32 insertions(+), 5 deletions(-)

diff --git a/services/crypto/modules/utils.js b/services/crypto/modules/utils.js
index 7a7ef6d814b4..f26f7f50165d 100644
--- a/services/crypto/modules/utils.js
+++ b/services/crypto/modules/utils.js
@@ -144,11 +144,6 @@ this.CryptoUtils = {
    * 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);
diff --git a/services/crypto/tests/unit/test_utils_hkdfExpand.js b/services/crypto/tests/unit/test_utils_hkdfExpand.js
index fd2245b77cf3..3dfbfbefcf03 100644
--- a/services/crypto/tests/unit/test_utils_hkdfExpand.js
+++ b/services/crypto/tests/unit/test_utils_hkdfExpand.js
@@ -101,6 +101,32 @@ function hkdf_hex(ikm, salt, info, len) {
   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);
@@ -117,4 +143,10 @@ function run_test() {
   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);
+  }
 }