From 4f8199050b9ed7d32fb4f1561d39d900ccb7c947 Mon Sep 17 00:00:00 2001 From: Jared Wein Date: Fri, 28 Jun 2019 16:53:11 +0000 Subject: [PATCH] Bug 1560447 - Add a decryptMany method to crypto-SDR.js for batch decrypting of stored logins. r=keeler Differential Revision: https://phabricator.services.mozilla.com/D35879 --- security/manager/ssl/SecretDecoderRing.cpp | 75 ++++++++++++++++++- security/manager/ssl/nsISecretDecoderRing.idl | 11 +++ security/manager/ssl/tests/unit/test_sdr.js | 37 +++++++++ toolkit/components/passwordmgr/crypto-SDR.js | 40 ++++++++++ .../passwordmgr/nsILoginManagerCrypto.idl | 12 +++ 5 files changed, 173 insertions(+), 2 deletions(-) diff --git a/security/manager/ssl/SecretDecoderRing.cpp b/security/manager/ssl/SecretDecoderRing.cpp index 3ad88c2c0d25..97ec4b01e206 100644 --- a/security/manager/ssl/SecretDecoderRing.cpp +++ b/security/manager/ssl/SecretDecoderRing.cpp @@ -38,8 +38,7 @@ void BackgroundSdrEncryptStrings(const nsTArray& plaintexts, InfallibleTArray cipherTexts(plaintexts.Length()); nsresult rv = NS_ERROR_FAILURE; - for (uint32_t i = 0; i < plaintexts.Length(); ++i) { - const nsCString& plaintext = plaintexts[i]; + for (const auto& plaintext : plaintexts) { nsCString cipherText; rv = sdrService->EncryptString(plaintext, cipherText); @@ -63,6 +62,37 @@ void BackgroundSdrEncryptStrings(const nsTArray& plaintexts, NS_DispatchToMainThread(runnable.forget()); } +void BackgroundSdrDecryptStrings(const nsTArray& encryptedStrings, + RefPtr& aPromise) { + nsCOMPtr sdrService = + do_GetService(NS_SECRETDECODERRING_CONTRACTID); + InfallibleTArray plainTexts(encryptedStrings.Length()); + + nsresult rv = NS_ERROR_FAILURE; + for (const auto& encryptedString : encryptedStrings) { + nsCString plainText; + rv = sdrService->DecryptString(encryptedString, plainText); + + if (NS_WARN_IF(NS_FAILED(rv))) { + break; + } + + plainTexts.AppendElement(NS_ConvertASCIItoUTF16(plainText)); + } + + nsCOMPtr runnable( + NS_NewRunnableFunction("BackgroundSdrDecryptStringsResolve", + [rv, aPromise = std::move(aPromise), + plainTexts = std::move(plainTexts)]() { + if (NS_FAILED(rv)) { + aPromise->MaybeReject(rv); + } else { + aPromise->MaybeResolve(plainTexts); + } + })); + NS_DispatchToMainThread(runnable.forget()); +} + nsresult SecretDecoderRing::Encrypt(const nsACString& data, /*out*/ nsACString& result) { UniquePK11SlotInfo slot(PK11_GetInternalKeySlot()); @@ -147,6 +177,7 @@ SecretDecoderRing::AsyncEncryptStrings(const nsTArray& plaintexts, MOZ_RELEASE_ASSERT(NS_IsMainThread()); NS_ENSURE_ARG(!plaintexts.IsEmpty()); NS_ENSURE_ARG_POINTER(aCx); + NS_ENSURE_ARG_POINTER(aPromise); nsIGlobalObject* globalObject = xpc::CurrentNativeGlobal(aCx); if (NS_WARN_IF(!globalObject)) { @@ -196,6 +227,46 @@ SecretDecoderRing::DecryptString(const nsACString& encryptedBase64Text, return NS_OK; } +NS_IMETHODIMP +SecretDecoderRing::AsyncDecryptStrings( + const nsTArray& encryptedStrings, JSContext* aCx, + Promise** aPromise) { + MOZ_RELEASE_ASSERT(NS_IsMainThread()); + NS_ENSURE_ARG(!encryptedStrings.IsEmpty()); + NS_ENSURE_ARG_POINTER(aCx); + NS_ENSURE_ARG_POINTER(aPromise); + + nsIGlobalObject* globalObject = xpc::CurrentNativeGlobal(aCx); + if (NS_WARN_IF(!globalObject)) { + return NS_ERROR_UNEXPECTED; + } + + ErrorResult result; + RefPtr promise = Promise::Create(globalObject, result); + if (NS_WARN_IF(result.Failed())) { + return result.StealNSResult(); + } + + // encryptedStrings are expected to be base64. + nsCOMPtr runnable(NS_NewRunnableFunction( + "BackgroundSdrDecryptStrings", [promise, encryptedStrings]() mutable { + BackgroundSdrDecryptStrings(encryptedStrings, promise); + })); + + nsCOMPtr target( + do_GetService(NS_STREAMTRANSPORTSERVICE_CONTRACTID)); + if (!target) { + return NS_ERROR_FAILURE; + } + nsresult rv = target->Dispatch(runnable, NS_DISPATCH_NORMAL); + if (NS_WARN_IF(NS_FAILED(rv))) { + return rv; + } + + promise.forget(aPromise); + return NS_OK; +} + NS_IMETHODIMP SecretDecoderRing::ChangePassword() { UniquePK11SlotInfo slot(PK11_GetInternalKeySlot()); diff --git a/security/manager/ssl/nsISecretDecoderRing.idl b/security/manager/ssl/nsISecretDecoderRing.idl index 30f441f7c3ab..caa70b2f3b6d 100644 --- a/security/manager/ssl/nsISecretDecoderRing.idl +++ b/security/manager/ssl/nsISecretDecoderRing.idl @@ -45,6 +45,17 @@ interface nsISecretDecoderRing: nsISupports { [must_use] ACString decryptString(in ACString encryptedBase64Text); + /** + * Run decryptString on multiple strings, asynchronously. This will allow you + * to not jank the browser if you need to decrypt a large number of strings + * all at once. + * + * @param encryptedStrings the strings to decrypt, encoded as Base64. + * @return A promise that resolves with the list of decrypted strings in Unicode. + */ + [implicit_jscontext, must_use] + Promise asyncDecryptStrings(in Array encryptedStrings); + /** * Prompt the user to change the password on the SDR key. */ diff --git a/security/manager/ssl/tests/unit/test_sdr.js b/security/manager/ssl/tests/unit/test_sdr.js index d576dd67bab3..36d009f448f8 100644 --- a/security/manager/ssl/tests/unit/test_sdr.js +++ b/security/manager/ssl/tests/unit/test_sdr.js @@ -117,3 +117,40 @@ add_task(async function testAsyncEncryptStrings() { "decryptString(encryptString(input)) should return input"); } }); + +add_task(async function testAsyncDecryptStrings() { + let sdr = Cc["@mozilla.org/security/sdr;1"] + .getService(Ci.nsISecretDecoderRing); + + // Test valid inputs for encryptString() and decryptString(). + let testCases = [ + "", + " ", // First printable latin1 character (code point 32). + "foo", + "1234567890`~!@#$%^&*()-_=+{[}]|\\:;'\",<.>/?", + "¡äöüÿ", // Misc + last printable latin1 character (code point 255). + "aaa 一二三", // Includes Unicode with code points outside [0, 255]. + ]; + + let convertedTestCases = testCases.map(tc => { + let converter = Cc["@mozilla.org/intl/scriptableunicodeconverter"] + .createInstance(Ci.nsIScriptableUnicodeConverter); + converter.charset = "UTF-8"; + + let convertedInput = converter.ConvertFromUnicode(tc); + convertedInput += converter.Finish(); + return convertedInput; + }); + + let encryptedStrings = convertedTestCases.map(tc => sdr.encryptString(tc)); + let decrypteds = await sdr.asyncDecryptStrings(encryptedStrings); + for (let i = 0; i < encryptedStrings.length; i++) { + let decrypted = decrypteds[i]; + let expectedDecryptedString = convertedTestCases[i]; + + equal(decrypted, expectedDecryptedString, + "decrypted string should match expected value"); + equal(sdr.decryptString(encryptedStrings[i]), decrypted, + "decryptString(encryptString(input)) should return the initial decrypted string value"); + } +}); diff --git a/toolkit/components/passwordmgr/crypto-SDR.js b/toolkit/components/passwordmgr/crypto-SDR.js index 8021dd820a96..8a71661af6d9 100644 --- a/toolkit/components/passwordmgr/crypto-SDR.js +++ b/toolkit/components/passwordmgr/crypto-SDR.js @@ -193,6 +193,46 @@ LoginManagerCrypto_SDR.prototype = { return plainText; }, + /* + * Decrypts the specified strings, using the SecretDecoderRing. + * + * Returns a promise which resolves with the the decrypted strings, + * or throws/rejects with an error if there was a problem. + */ + async decryptMany(cipherTexts) { + if (!Array.isArray(cipherTexts) || !cipherTexts.length) { + throw Components.Exception("Need at least one ciphertext to decrypt", + Cr.NS_ERROR_INVALID_ARG); + } + + let plainTexts = []; + + let wasLoggedIn = this.isLoggedIn; + let canceledMP = false; + + this._uiBusy = true; + try { + plainTexts = await this._decoderRing.asyncDecryptStrings(cipherTexts); + } catch (e) { + this.log("Failed to decrypt strings. (" + e.name + ")"); + // If the user clicks Cancel, we get NS_ERROR_NOT_AVAILABLE. + if (e.result == Cr.NS_ERROR_NOT_AVAILABLE) { + canceledMP = true; + throw Components.Exception("User canceled master password entry", Cr.NS_ERROR_ABORT); + } else { + throw Components.Exception("Couldn't decrypt strings: " + e.result, Cr.NS_ERROR_FAILURE); + } + } finally { + this._uiBusy = false; + // If we triggered a master password prompt, notify observers. + if (!wasLoggedIn && this.isLoggedIn) { + this._notifyObservers("passwordmgr-crypto-login"); + } else if (canceledMP) { + this._notifyObservers("passwordmgr-crypto-loginCanceled"); + } + } + return plainTexts; + }, /* * uiBusy diff --git a/toolkit/components/passwordmgr/nsILoginManagerCrypto.idl b/toolkit/components/passwordmgr/nsILoginManagerCrypto.idl index 7325ec49b42e..a7d6369359ca 100644 --- a/toolkit/components/passwordmgr/nsILoginManagerCrypto.idl +++ b/toolkit/components/passwordmgr/nsILoginManagerCrypto.idl @@ -52,6 +52,18 @@ interface nsILoginManagerCrypto : nsISupports { */ AString decrypt(in AString cipherText); + /** + * @param cipherTexts + * The strings to be decrypted. + * + * Decrypts the specified strings, returning the plaintext values. + * + * Can throw if the user cancels entry of their master password, or if the + * cipherText value can not be successfully decrypted (eg, if it was + * encrypted with some other key). + */ + jsval decryptMany(in jsval cipherTexts); + /** * uiBusy *