[tbb-commits] [Git][tpo/applications/tor-browser][tor-browser-115.15.0esr-13.5-1] 2 commits: Bug 1760806 - WebCrypto: ECDH and ECDSA JWK import to check that the crv in...

Pier Angelo Vendrame (@pierov) git at gitlab.torproject.org
Tue Sep 3 07:12:05 UTC 2024



Pier Angelo Vendrame pushed to branch tor-browser-115.15.0esr-13.5-1 at The Tor Project / Applications / Tor Browser


Commits:
b81d3724 by Anna Weine at 2024-09-03T09:11:27+02:00
Bug 1760806 - WebCrypto: ECDH and ECDSA JWK import to check that the crv in params and crv in alg are the same r=keeler

https://treeherder.mozilla.org/jobs?repo=try&revision=ed7936b105dea8e588650feb6baf26a50a6439fc

Differential Revision: https://phabricator.services.mozilla.com/D217273

- - - - -
76781561 by Sam Foster at 2024-09-03T09:11:36+02:00
Bug 1909099 - Always clean up old session restore and sync log files. r=markh,sessionstore-reviewers,dao

Differential Revision: https://phabricator.services.mozilla.com/D217520
- - - - -


5 changed files:

- dom/crypto/WebCryptoTask.cpp
- dom/crypto/test/test-vectors.js
- dom/crypto/test/test_WebCrypto_ECDH.html
- dom/crypto/test/test_WebCrypto_ECDSA.html
- services/common/logmanager.sys.mjs


Changes:

=====================================
dom/crypto/WebCryptoTask.cpp
=====================================
@@ -1777,7 +1777,8 @@ class ImportEcKeyTask : public ImportKeyTask {
       return;
     }
 
-    if (mFormat.EqualsLiteral(WEBCRYPTO_KEY_FORMAT_RAW)) {
+    if (mFormat.EqualsLiteral(WEBCRYPTO_KEY_FORMAT_RAW) ||
+        mFormat.EqualsLiteral(WEBCRYPTO_KEY_FORMAT_JWK)) {
       RootedDictionary<EcKeyImportParams> params(aCx);
       mEarlyRv = Coerce(aCx, params, aAlgorithm);
       if (NS_FAILED(mEarlyRv) || !params.mNamedCurve.WasPassed()) {
@@ -1882,11 +1883,21 @@ class ImportEcKeyTask : public ImportKeyTask {
       return NS_ERROR_DOM_NOT_SUPPORTED_ERR;
     }
 
-    // Extract 'crv' parameter from JWKs.
+    // Checking the 'crv' consistency
     if (mFormat.EqualsLiteral(WEBCRYPTO_KEY_FORMAT_JWK)) {
-      if (!NormalizeToken(mJwk.mCrv.Value(), mNamedCurve)) {
+      // the curve stated in 'crv field'
+      nsString namedCurveFromCrv;
+      if (!NormalizeToken(mJwk.mCrv.Value(), namedCurveFromCrv)) {
         return NS_ERROR_DOM_NOT_SUPPORTED_ERR;
       }
+
+      // https://w3c.github.io/webcrypto/#ecdh-operations
+      // https://w3c.github.io/webcrypto/#ecdsa-operations
+      // If namedCurve is not equal to the namedCurve member of
+      // normalizedAlgorithm (mNamedCurve in our case), throw a DataError.
+      if (!mNamedCurve.Equals(namedCurveFromCrv)) {
+        return NS_ERROR_DOM_DATA_ERR;
+      }
     }
     return NS_OK;
   }


=====================================
dom/crypto/test/test-vectors.js
=====================================
@@ -901,6 +901,13 @@ let tv = {
       y: "9M8HWzlAXdHxresJAQftz7K0ljc52HZ54wVssFV9Ct8",
     },
 
+    jwk_different_crv: {
+      kty: "EC",
+      crv: "P-521",
+      x: "XOe4bjsyZgQD5jcS7wmY3q4QJ_rsPBvp92-TTf61jpg",
+      y: "9M8HWzlAXdHxresJAQftz7K0ljc52HZ54wVssFV9Ct8",
+    },
+
     // The crv parameter is missing.
     jwk_missing_crv: {
       kty: "EC",
@@ -1017,6 +1024,18 @@ let tv = {
     },
   },
 
+  // An ECDSA key in JWK format, which an "crv" field doesn't match the alg's crv.
+  ecdsa_jwk_crv_mismatch: {
+    pub_jwk: {
+      kty: "EC",
+      crv: "P-256",
+      alg: "ECDSA",
+
+      x: "XOe4bjsyZgQD5jcS7wmY3q4QJ_rsPBvp92-TTf61jpg",
+      y: "9M8HWzlAXdHxresJAQftz7K0ljc52HZ54wVssFV9Ct8",
+    },
+  },
+
   ecdsa_bad: {
     pub_jwk: {
       kty: "EC",


=====================================
dom/crypto/test/test_WebCrypto_ECDH.html
=====================================
@@ -152,12 +152,24 @@ TestArray.addTest(
   }
 );
 
+// -----------------------------------------------------------------------------
+TestArray.addTest(
+  "Verify that ECDH import fails with a key with a mismatched 'crv' field",
+  function() {
+    var that = this;
+    var alg = { name: "ECDH", namedCurve: "P-521"};
+
+    crypto.subtle.importKey("jwk", tv.ecdsa_jwk_crv_mismatch.pub_jwk, alg, true, ["verify"])
+      .then(error(that), complete(that));
+  }
+);
+
 // -----------------------------------------------------------------------------
 TestArray.addTest(
   "JWK import an ECDH public and private key and derive bits (P-256)",
   function() {
     var that = this;
-    var alg = { name: "ECDH" };
+    var alg = { name: "ECDH", namedCurve: "P-256" };
 
     var pubKey, privKey;
     function setPub(x) { pubKey = x; }
@@ -182,7 +194,7 @@ TestArray.addTest(
   "JWK import an ECDH public and private key and derive bits (P-384)",
   function() {
     var that = this;
-    var alg = { name: "ECDH" };
+    var alg = { name: "ECDH", namedCurve: "P-384"};
 
     var pubKey, privKey;
     function setPub(x) { pubKey = x; }
@@ -207,7 +219,7 @@ TestArray.addTest(
   "JWK import an ECDH public and private key and derive bits (P-521)",
   function() {
     var that = this;
-    var alg = { name: "ECDH" };
+    var alg = { name: "ECDH", namedCurve : "P-521" };
 
     var pubKey, privKey;
     function setPub(x) { pubKey = x; }
@@ -232,7 +244,7 @@ TestArray.addTest(
   "JWK import/export roundtrip with ECDH (P-256)",
   function() {
     var that = this;
-    var alg = { name: "ECDH" };
+    var alg = { name: "ECDH", namedCurve : "P-256" };
 
     var pubKey, privKey;
     function setPub(x) { pubKey = x; }
@@ -277,7 +289,7 @@ TestArray.addTest(
   "PKCS8 import/export roundtrip with ECDH (P-256)",
   function() {
     var that = this;
-    var alg = { name: "ECDH",  namedCurve: "P-256" };
+    var alg = { name: "ECDH", namedCurve: "P-256" };
 
     function doExportPriv(x) {
       return crypto.subtle.exportKey("pkcs8", x);
@@ -296,7 +308,7 @@ TestArray.addTest(
   "Test that importing bad JWKs fails",
   function() {
     var that = this;
-    var alg = { name: "ECDH" };
+    var alg = { name: "ECDH", namedCurve: "P-256" };
     var tvs = tv.ecdh_p256_negative;
 
     function doTryImport(jwk) {
@@ -306,6 +318,7 @@ TestArray.addTest(
     }
 
     doTryImport(tvs.jwk_bad_crv)()
+      .then(error(that), doTryImport(tvs.jwk_different_crv))
       .then(error(that), doTryImport(tvs.jwk_missing_crv))
       .then(error(that), doTryImport(tvs.jwk_missing_x))
       .then(error(that), doTryImport(tvs.jwk_missing_y))
@@ -349,7 +362,7 @@ TestArray.addTest(
   "Derive an HMAC key from two ECDH keys and test sign/verify",
   function() {
     var that = this;
-    var alg = { name: "ECDH" };
+    var alg = { name: "ECDH", namedCurve: "P-521" };
     var algDerived = { name: "HMAC", hash: {name: "SHA-1"} };
 
     var pubKey, privKey;
@@ -391,6 +404,28 @@ TestArray.addTest(
   }
 );
 
+// -----------------------------------------------------------------------------
+TestArray.addTest(
+  "Derive an HKDF key from two ECDH keys and derive an HMAC key from that",
+  function() {
+    var that = this;
+    var alg = { name: "ECDH", namedCurve: "P-256" };
+
+    async function doTest() {
+      let privKey = await crypto.subtle.importKey("jwk", tv.ecdh_p256.jwk_priv, alg, false, ["deriveKey"]);
+      let pubKey = await crypto.subtle.importKey("jwk", tv.ecdh_p256.jwk_pub, alg, false, []);
+      let ecdhAlg = { name: "ECDH", public: pubKey };
+      let hkdfAlg = { name: "HKDF", hash: "SHA-256", salt: new Uint8Array(), info: new Uint8Array() };
+      let hkdfKey = await crypto.subtle.deriveKey(ecdhAlg, privKey, hkdfAlg, false, ["deriveKey"]);
+      let hmacAlg = { name: "HMAC", hash: "SHA-256" };
+      let hmacKey = await crypto.subtle.deriveKey(hkdfAlg, hkdfKey, hmacAlg, false, ["sign"]);
+      return crypto.subtle.sign("HMAC", hmacKey, new Uint8Array());
+    }
+    const expected = util.hex2abv("acf62832fa93469824cd997593bc963b28a68e6f73f4516bbe51b35942fe9811");
+    doTest().then(memcmp_complete(that, expected), error(that));
+  }
+);
+
 // -----------------------------------------------------------------------------
 TestArray.addTest(
   "SPKI import/export of public ECDH keys (P-256)",
@@ -433,7 +468,7 @@ TestArray.addTest(
   "SPKI/JWK import ECDH keys (P-256) and derive a known secret",
   function() {
     var that = this;
-    var alg = { name: "ECDH" };
+    var alg = { name: "ECDH", namedCurve: "P-256" };
 
     var pubKey, privKey;
     function setPub(x) { pubKey = x; }


=====================================
dom/crypto/test/test_WebCrypto_ECDSA.html
=====================================
@@ -91,7 +91,7 @@ TestArray.addTest(
   "ECDSA JWK import and reject a known-bad signature",
   function() {
     var that = this;
-    var alg = { name: "ECDSA", namedCurve: "P-256", hash: "SHA-256" };
+    var alg = { name: "ECDSA", namedCurve: "P-521", hash: "SHA-512" };
 
     function doVerify(x) {
       return crypto.subtle.verify(alg, x, tv.ecdsa_verify.sig_tampered,
@@ -141,6 +141,18 @@ TestArray.addTest(
   }
 );
 
+// -----------------------------------------------------------------------------
+TestArray.addTest(
+  "Verify that ECDSA import fails with a key with a mismatched 'crv' field",
+  function() {
+    var that = this;
+    var alg = { name: "ECDSA", namedCurve: "P-521", hash: "SHA-512" };
+
+    crypto.subtle.importKey("jwk", tv.ecdsa_jwk_crv_mismatch.pub_jwk, alg, true, ["verify"])
+      .then(error(that), complete(that));
+  }
+);
+
 // -----------------------------------------------------------------------------
 TestArray.addTest(
   "Verify that ECDSA import fails with a known-bad public key",


=====================================
services/common/logmanager.sys.mjs
=====================================
@@ -363,12 +363,7 @@ LogManager.prototype = {
         filename,
         this._log
       );
-      // It's not completely clear to markh why we only do log cleanups
-      // for errors, but for now the Sync semantics have been copied...
-      // (one theory is that only cleaning up on error makes it less
-      // likely old error logs would be removed, but that's not true if
-      // there are occasional errors - let's address this later!)
-      if (reason == this.ERROR_LOG_WRITTEN && !this._cleaningUpFileLogs) {
+      if (!this._cleaningUpFileLogs) {
         this._log.trace("Running cleanup.");
         try {
           await this.cleanupLogs();



View it on GitLab: https://gitlab.torproject.org/tpo/applications/tor-browser/-/compare/fdbe54d55678935528d9387639c333c9202bd732...767815613bc846201de67c15e979121fdec3058c

-- 
View it on GitLab: https://gitlab.torproject.org/tpo/applications/tor-browser/-/compare/fdbe54d55678935528d9387639c333c9202bd732...767815613bc846201de67c15e979121fdec3058c
You're receiving this email because of your account on gitlab.torproject.org.


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.torproject.org/pipermail/tbb-commits/attachments/20240903/c56a75c6/attachment-0001.htm>


More information about the tbb-commits mailing list