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=ed7936b105dea8e588...

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/fdbe54d...