Bug 1305531 - Use ACString instead of AUTF8String in nsISecretDecoderRing.idl to unbreak decrypting saved usernames and passwords. r=keeler draft
authorCykesiopka <cykesiopka.bmo@gmail.com>
Sat, 01 Oct 2016 09:29:28 +0800
changeset 419840 514e78f2e1c2cef3b3692656b20daf3b068a4fee
parent 419839 c8b0d217e5a4a903975eb2a104a3fb5e56899e6f
child 419841 6aba7627e3c0c79ea633933ce7f140c20b16c9d9
push id31023
push usercykesiopka.bmo@gmail.com
push dateSat, 01 Oct 2016 01:30:33 +0000
reviewerskeeler
bugs1305531, 1275841
milestone52.0a1
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
security/manager/ssl/nsISecretDecoderRing.idl
--- a/security/manager/ssl/nsISecretDecoderRing.idl
+++ b/security/manager/ssl/nsISecretDecoderRing.idl
@@ -4,29 +4,37 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "nsISupports.idl"
 
 [scriptable, uuid(0EC80360-075C-11d4-9FD4-00C04F1B83D8)]
 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.
    */
   void changePassword();
 
   /**
    * Logout of the security device that protects the SDR key.