From e95a6f224a1b5d773d3908fba4a88c40df8119aa Mon Sep 17 00:00:00 2001 From: Cykesiopka Date: Sat, 1 Oct 2016 09:29:28 +0800 Subject: [PATCH] Bug 1305531 - Use ACString instead of AUTF8String in nsISecretDecoderRing.idl to unbreak decrypting saved usernames and passwords. r=keeler Bug 1275841 switched some IDL types from "string" to "AUTF8String". This had the unintentional effect of breaking decryption of previously saved passwords that contained special characters. In particular, the AUTF8String type means XPConnect may convert any strings using that type to UTF-16 when crossing XPConnect boundaries. However, crypto-SDR.js (responsible for encrypting and decrypting for the password manager) expects to do conversions between UTF-16 and UTF-8 itself. What ends up happening is crypto-SDR.js decrypts a saved password and tries to convert from UTF-8 to UTF-16, but fails because the decrypted text is already UTF-16. The solution is to use ACString instead of AUTF8String. ACString does not result in automatic encoding changes, so the expectations of crypto-SDR.js are met again, and lets SecretDecoderRing.cpp keep the benefit of working with smart string types. This change probably breaks passwords saved after Bug 1275841 landed and before this patch landed, but the number of passwords this patch breaks is probably much lower than the number of passwords that would be broken if this patch did not land. MozReview-Commit-ID: 6Z01zfwJ6t7 --HG-- extra : rebase_source : 514e78f2e1c2cef3b3692656b20daf3b068a4fee --- security/manager/ssl/nsISecretDecoderRing.idl | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/security/manager/ssl/nsISecretDecoderRing.idl b/security/manager/ssl/nsISecretDecoderRing.idl index 90f0e5b3ff75..e9f67cbeeb9c 100644 --- a/security/manager/ssl/nsISecretDecoderRing.idl +++ b/security/manager/ssl/nsISecretDecoderRing.idl @@ -9,19 +9,27 @@ interface nsISecretDecoderRing: nsISupports { /** * Encrypt to Base64 output. + * Note that the input must basically be a byte array (i.e. the code points + * must be within the range [0, 255]). Hence, using this method directly to + * encrypt passwords (or any text, really) won't work as expected. + * Instead, use something like nsIScriptableUnicodeConverter to first convert + * the desired password or text to UTF-8, then encrypt that. Remember to + * convert back when calling decryptString(). * * @param text The text to encrypt. * @return The encrypted text, encoded as Base64. */ - ACString encryptString(in AUTF8String text); + ACString encryptString(in ACString text); /** * Decrypt Base64 input. + * See the encryptString() documentation - this method has basically the same + * limitations. * * @param encryptedBase64Text Encrypted input text, encoded as Base64. * @return The decoded text. */ - AUTF8String decryptString(in ACString encryptedBase64Text); + ACString decryptString(in ACString encryptedBase64Text); /** * Prompt the user to change the password on the SDR key.