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
 - 
76781561
by Sam Foster at 2024-09-03T09:11:36+02:00
 
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:
| ... | ... | @@ -1777,7 +1777,8 @@ class ImportEcKeyTask : public ImportKeyTask { | 
| 1777 | 1777 |        return;
 | 
| 1778 | 1778 |      }
 | 
| 1779 | 1779 | |
| 1780 | -    if (mFormat.EqualsLiteral(WEBCRYPTO_KEY_FORMAT_RAW)) {
 | 
|
| 1780 | +    if (mFormat.EqualsLiteral(WEBCRYPTO_KEY_FORMAT_RAW) ||
 | 
|
| 1781 | +        mFormat.EqualsLiteral(WEBCRYPTO_KEY_FORMAT_JWK)) {
 | 
|
| 1781 | 1782 |        RootedDictionary<EcKeyImportParams> params(aCx);
 | 
| 1782 | 1783 |        mEarlyRv = Coerce(aCx, params, aAlgorithm);
 | 
| 1783 | 1784 |        if (NS_FAILED(mEarlyRv) || !params.mNamedCurve.WasPassed()) {
 | 
| ... | ... | @@ -1882,11 +1883,21 @@ class ImportEcKeyTask : public ImportKeyTask { | 
| 1882 | 1883 |        return NS_ERROR_DOM_NOT_SUPPORTED_ERR;
 | 
| 1883 | 1884 |      }
 | 
| 1884 | 1885 | |
| 1885 | -    // Extract 'crv' parameter from JWKs.
 | 
|
| 1886 | +    // Checking the 'crv' consistency
 | 
|
| 1886 | 1887 |      if (mFormat.EqualsLiteral(WEBCRYPTO_KEY_FORMAT_JWK)) {
 | 
| 1887 | -      if (!NormalizeToken(mJwk.mCrv.Value(), mNamedCurve)) {
 | 
|
| 1888 | +      // the curve stated in 'crv field'
 | 
|
| 1889 | +      nsString namedCurveFromCrv;
 | 
|
| 1890 | +      if (!NormalizeToken(mJwk.mCrv.Value(), namedCurveFromCrv)) {
 | 
|
| 1888 | 1891 |          return NS_ERROR_DOM_NOT_SUPPORTED_ERR;
 | 
| 1889 | 1892 |        }
 | 
| 1893 | +  | 
|
| 1894 | +      // https://w3c.github.io/webcrypto/#ecdh-operations
 | 
|
| 1895 | +      // https://w3c.github.io/webcrypto/#ecdsa-operations
 | 
|
| 1896 | +      // If namedCurve is not equal to the namedCurve member of
 | 
|
| 1897 | +      // normalizedAlgorithm (mNamedCurve in our case), throw a DataError.
 | 
|
| 1898 | +      if (!mNamedCurve.Equals(namedCurveFromCrv)) {
 | 
|
| 1899 | +        return NS_ERROR_DOM_DATA_ERR;
 | 
|
| 1900 | +      }
 | 
|
| 1890 | 1901 |      }
 | 
| 1891 | 1902 |      return NS_OK;
 | 
| 1892 | 1903 |    }
 | 
| ... | ... | @@ -901,6 +901,13 @@ let tv = { | 
| 901 | 901 |        y: "9M8HWzlAXdHxresJAQftz7K0ljc52HZ54wVssFV9Ct8",
 | 
| 902 | 902 |      },
 | 
| 903 | 903 | |
| 904 | +    jwk_different_crv: {
 | 
|
| 905 | +      kty: "EC",
 | 
|
| 906 | +      crv: "P-521",
 | 
|
| 907 | +      x: "XOe4bjsyZgQD5jcS7wmY3q4QJ_rsPBvp92-TTf61jpg",
 | 
|
| 908 | +      y: "9M8HWzlAXdHxresJAQftz7K0ljc52HZ54wVssFV9Ct8",
 | 
|
| 909 | +    },
 | 
|
| 910 | +  | 
|
| 904 | 911 |      // The crv parameter is missing.
 | 
| 905 | 912 |      jwk_missing_crv: {
 | 
| 906 | 913 |        kty: "EC",
 | 
| ... | ... | @@ -1017,6 +1024,18 @@ let tv = { | 
| 1017 | 1024 |      },
 | 
| 1018 | 1025 |    },
 | 
| 1019 | 1026 | |
| 1027 | +  // An ECDSA key in JWK format, which an "crv" field doesn't match the alg's crv.
 | 
|
| 1028 | +  ecdsa_jwk_crv_mismatch: {
 | 
|
| 1029 | +    pub_jwk: {
 | 
|
| 1030 | +      kty: "EC",
 | 
|
| 1031 | +      crv: "P-256",
 | 
|
| 1032 | +      alg: "ECDSA",
 | 
|
| 1033 | +  | 
|
| 1034 | +      x: "XOe4bjsyZgQD5jcS7wmY3q4QJ_rsPBvp92-TTf61jpg",
 | 
|
| 1035 | +      y: "9M8HWzlAXdHxresJAQftz7K0ljc52HZ54wVssFV9Ct8",
 | 
|
| 1036 | +    },
 | 
|
| 1037 | +  },
 | 
|
| 1038 | +  | 
|
| 1020 | 1039 |    ecdsa_bad: {
 | 
| 1021 | 1040 |      pub_jwk: {
 | 
| 1022 | 1041 |        kty: "EC",
 | 
| ... | ... | @@ -152,12 +152,24 @@ TestArray.addTest( | 
| 152 | 152 |    }
 | 
| 153 | 153 |  );
 | 
| 154 | 154 | |
| 155 | +// -----------------------------------------------------------------------------
 | 
|
| 156 | +TestArray.addTest(
 | 
|
| 157 | +  "Verify that ECDH import fails with a key with a mismatched 'crv' field",
 | 
|
| 158 | +  function() {
 | 
|
| 159 | +    var that = this;
 | 
|
| 160 | +    var alg = { name: "ECDH", namedCurve: "P-521"};
 | 
|
| 161 | +  | 
|
| 162 | +    crypto.subtle.importKey("jwk", tv.ecdsa_jwk_crv_mismatch.pub_jwk, alg, true, ["verify"])
 | 
|
| 163 | +      .then(error(that), complete(that));
 | 
|
| 164 | +  }
 | 
|
| 165 | +);
 | 
|
| 166 | +  | 
|
| 155 | 167 |  // -----------------------------------------------------------------------------
 | 
| 156 | 168 |  TestArray.addTest(
 | 
| 157 | 169 |    "JWK import an ECDH public and private key and derive bits (P-256)",
 | 
| 158 | 170 |    function() {
 | 
| 159 | 171 |      var that = this;
 | 
| 160 | -    var alg = { name: "ECDH" };
 | 
|
| 172 | +    var alg = { name: "ECDH", namedCurve: "P-256" };
 | 
|
| 161 | 173 | |
| 162 | 174 |      var pubKey, privKey;
 | 
| 163 | 175 |      function setPub(x) { pubKey = x; }
 | 
| ... | ... | @@ -182,7 +194,7 @@ TestArray.addTest( | 
| 182 | 194 |    "JWK import an ECDH public and private key and derive bits (P-384)",
 | 
| 183 | 195 |    function() {
 | 
| 184 | 196 |      var that = this;
 | 
| 185 | -    var alg = { name: "ECDH" };
 | 
|
| 197 | +    var alg = { name: "ECDH", namedCurve: "P-384"};
 | 
|
| 186 | 198 | |
| 187 | 199 |      var pubKey, privKey;
 | 
| 188 | 200 |      function setPub(x) { pubKey = x; }
 | 
| ... | ... | @@ -207,7 +219,7 @@ TestArray.addTest( | 
| 207 | 219 |    "JWK import an ECDH public and private key and derive bits (P-521)",
 | 
| 208 | 220 |    function() {
 | 
| 209 | 221 |      var that = this;
 | 
| 210 | -    var alg = { name: "ECDH" };
 | 
|
| 222 | +    var alg = { name: "ECDH", namedCurve : "P-521" };
 | 
|
| 211 | 223 | |
| 212 | 224 |      var pubKey, privKey;
 | 
| 213 | 225 |      function setPub(x) { pubKey = x; }
 | 
| ... | ... | @@ -232,7 +244,7 @@ TestArray.addTest( | 
| 232 | 244 |    "JWK import/export roundtrip with ECDH (P-256)",
 | 
| 233 | 245 |    function() {
 | 
| 234 | 246 |      var that = this;
 | 
| 235 | -    var alg = { name: "ECDH" };
 | 
|
| 247 | +    var alg = { name: "ECDH", namedCurve : "P-256" };
 | 
|
| 236 | 248 | |
| 237 | 249 |      var pubKey, privKey;
 | 
| 238 | 250 |      function setPub(x) { pubKey = x; }
 | 
| ... | ... | @@ -277,7 +289,7 @@ TestArray.addTest( | 
| 277 | 289 |    "PKCS8 import/export roundtrip with ECDH (P-256)",
 | 
| 278 | 290 |    function() {
 | 
| 279 | 291 |      var that = this;
 | 
| 280 | -    var alg = { name: "ECDH",  namedCurve: "P-256" };
 | 
|
| 292 | +    var alg = { name: "ECDH", namedCurve: "P-256" };
 | 
|
| 281 | 293 | |
| 282 | 294 |      function doExportPriv(x) {
 | 
| 283 | 295 |        return crypto.subtle.exportKey("pkcs8", x);
 | 
| ... | ... | @@ -296,7 +308,7 @@ TestArray.addTest( | 
| 296 | 308 |    "Test that importing bad JWKs fails",
 | 
| 297 | 309 |    function() {
 | 
| 298 | 310 |      var that = this;
 | 
| 299 | -    var alg = { name: "ECDH" };
 | 
|
| 311 | +    var alg = { name: "ECDH", namedCurve: "P-256" };
 | 
|
| 300 | 312 |      var tvs = tv.ecdh_p256_negative;
 | 
| 301 | 313 | |
| 302 | 314 |      function doTryImport(jwk) {
 | 
| ... | ... | @@ -306,6 +318,7 @@ TestArray.addTest( | 
| 306 | 318 |      }
 | 
| 307 | 319 | |
| 308 | 320 |      doTryImport(tvs.jwk_bad_crv)()
 | 
| 321 | +      .then(error(that), doTryImport(tvs.jwk_different_crv))
 | 
|
| 309 | 322 |        .then(error(that), doTryImport(tvs.jwk_missing_crv))
 | 
| 310 | 323 |        .then(error(that), doTryImport(tvs.jwk_missing_x))
 | 
| 311 | 324 |        .then(error(that), doTryImport(tvs.jwk_missing_y))
 | 
| ... | ... | @@ -349,7 +362,7 @@ TestArray.addTest( | 
| 349 | 362 |    "Derive an HMAC key from two ECDH keys and test sign/verify",
 | 
| 350 | 363 |    function() {
 | 
| 351 | 364 |      var that = this;
 | 
| 352 | -    var alg = { name: "ECDH" };
 | 
|
| 365 | +    var alg = { name: "ECDH", namedCurve: "P-521" };
 | 
|
| 353 | 366 |      var algDerived = { name: "HMAC", hash: {name: "SHA-1"} };
 | 
| 354 | 367 | |
| 355 | 368 |      var pubKey, privKey;
 | 
| ... | ... | @@ -391,6 +404,28 @@ TestArray.addTest( | 
| 391 | 404 |    }
 | 
| 392 | 405 |  );
 | 
| 393 | 406 | |
| 407 | +// -----------------------------------------------------------------------------
 | 
|
| 408 | +TestArray.addTest(
 | 
|
| 409 | +  "Derive an HKDF key from two ECDH keys and derive an HMAC key from that",
 | 
|
| 410 | +  function() {
 | 
|
| 411 | +    var that = this;
 | 
|
| 412 | +    var alg = { name: "ECDH", namedCurve: "P-256" };
 | 
|
| 413 | +  | 
|
| 414 | +    async function doTest() {
 | 
|
| 415 | +      let privKey = await crypto.subtle.importKey("jwk", tv.ecdh_p256.jwk_priv, alg, false, ["deriveKey"]);
 | 
|
| 416 | +      let pubKey = await crypto.subtle.importKey("jwk", tv.ecdh_p256.jwk_pub, alg, false, []);
 | 
|
| 417 | +      let ecdhAlg = { name: "ECDH", public: pubKey };
 | 
|
| 418 | +      let hkdfAlg = { name: "HKDF", hash: "SHA-256", salt: new Uint8Array(), info: new Uint8Array() };
 | 
|
| 419 | +      let hkdfKey = await crypto.subtle.deriveKey(ecdhAlg, privKey, hkdfAlg, false, ["deriveKey"]);
 | 
|
| 420 | +      let hmacAlg = { name: "HMAC", hash: "SHA-256" };
 | 
|
| 421 | +      let hmacKey = await crypto.subtle.deriveKey(hkdfAlg, hkdfKey, hmacAlg, false, ["sign"]);
 | 
|
| 422 | +      return crypto.subtle.sign("HMAC", hmacKey, new Uint8Array());
 | 
|
| 423 | +    }
 | 
|
| 424 | +    const expected = util.hex2abv("acf62832fa93469824cd997593bc963b28a68e6f73f4516bbe51b35942fe9811");
 | 
|
| 425 | +    doTest().then(memcmp_complete(that, expected), error(that));
 | 
|
| 426 | +  }
 | 
|
| 427 | +);
 | 
|
| 428 | +  | 
|
| 394 | 429 |  // -----------------------------------------------------------------------------
 | 
| 395 | 430 |  TestArray.addTest(
 | 
| 396 | 431 |    "SPKI import/export of public ECDH keys (P-256)",
 | 
| ... | ... | @@ -433,7 +468,7 @@ TestArray.addTest( | 
| 433 | 468 |    "SPKI/JWK import ECDH keys (P-256) and derive a known secret",
 | 
| 434 | 469 |    function() {
 | 
| 435 | 470 |      var that = this;
 | 
| 436 | -    var alg = { name: "ECDH" };
 | 
|
| 471 | +    var alg = { name: "ECDH", namedCurve: "P-256" };
 | 
|
| 437 | 472 | |
| 438 | 473 |      var pubKey, privKey;
 | 
| 439 | 474 |      function setPub(x) { pubKey = x; }
 | 
| ... | ... | @@ -91,7 +91,7 @@ TestArray.addTest( | 
| 91 | 91 |    "ECDSA JWK import and reject a known-bad signature",
 | 
| 92 | 92 |    function() {
 | 
| 93 | 93 |      var that = this;
 | 
| 94 | -    var alg = { name: "ECDSA", namedCurve: "P-256", hash: "SHA-256" };
 | 
|
| 94 | +    var alg = { name: "ECDSA", namedCurve: "P-521", hash: "SHA-512" };
 | 
|
| 95 | 95 | |
| 96 | 96 |      function doVerify(x) {
 | 
| 97 | 97 |        return crypto.subtle.verify(alg, x, tv.ecdsa_verify.sig_tampered,
 | 
| ... | ... | @@ -141,6 +141,18 @@ TestArray.addTest( | 
| 141 | 141 |    }
 | 
| 142 | 142 |  );
 | 
| 143 | 143 | |
| 144 | +// -----------------------------------------------------------------------------
 | 
|
| 145 | +TestArray.addTest(
 | 
|
| 146 | +  "Verify that ECDSA import fails with a key with a mismatched 'crv' field",
 | 
|
| 147 | +  function() {
 | 
|
| 148 | +    var that = this;
 | 
|
| 149 | +    var alg = { name: "ECDSA", namedCurve: "P-521", hash: "SHA-512" };
 | 
|
| 150 | +  | 
|
| 151 | +    crypto.subtle.importKey("jwk", tv.ecdsa_jwk_crv_mismatch.pub_jwk, alg, true, ["verify"])
 | 
|
| 152 | +      .then(error(that), complete(that));
 | 
|
| 153 | +  }
 | 
|
| 154 | +);
 | 
|
| 155 | +  | 
|
| 144 | 156 |  // -----------------------------------------------------------------------------
 | 
| 145 | 157 |  TestArray.addTest(
 | 
| 146 | 158 |    "Verify that ECDSA import fails with a known-bad public key",
 | 
| ... | ... | @@ -363,12 +363,7 @@ LogManager.prototype = { | 
| 363 | 363 |          filename,
 | 
| 364 | 364 |          this._log
 | 
| 365 | 365 |        );
 | 
| 366 | -      // It's not completely clear to markh why we only do log cleanups
 | 
|
| 367 | -      // for errors, but for now the Sync semantics have been copied...
 | 
|
| 368 | -      // (one theory is that only cleaning up on error makes it less
 | 
|
| 369 | -      // likely old error logs would be removed, but that's not true if
 | 
|
| 370 | -      // there are occasional errors - let's address this later!)
 | 
|
| 371 | -      if (reason == this.ERROR_LOG_WRITTEN && !this._cleaningUpFileLogs) {
 | 
|
| 366 | +      if (!this._cleaningUpFileLogs) {
 | 
|
| 372 | 367 |          this._log.trace("Running cleanup.");
 | 
| 373 | 368 |          try {
 | 
| 374 | 369 |            await this.cleanupLogs();
 |