Bug 972582 - Fix broken base64UrlDecode in toolkit/identity. r=MattN, r=warner, sr=mossop

toolkit/identity/IdentityCryptoService.cpp

@@ -65,28 +65,6 @@ Base64UrlEncodeImpl(const nsACString & utf8Input, nsACString & result)
   return NS_OK;
 }
 
-nsresult
-Base64UrlDecodeImpl(const nsACString & base64Input, nsACString & result)
-{
-  nsresult rv = Base64Decode(base64Input, result);
-  NS_ENSURE_SUCCESS(rv, rv);
-
-  nsACString::char_type * out = result.BeginWriting();
-  nsACString::size_type length = result.Length();
-  // base64url encoding is defined in RFC 4648. It replaces the last two
-  // alphabet characters of base64 encoding with '-' and '_' respectively.
-  // Reverse that encoding here.
-  for (unsigned int i = 0; i < length; ++i) {
-    if (out[i] == '-') {
-      out[i] = '+';
-    } else if (out[i] == '_') {
-      out[i] = '/';
-    }
-  }
-
-  return NS_OK;
-}
-
 #define DSA_KEY_TYPE_STRING (NS_LITERAL_CSTRING("DS160"))
 #define RSA_KEY_TYPE_STRING (NS_LITERAL_CSTRING("RS256"))
 
@@ -260,13 +238,6 @@ IdentityCryptoService::Base64UrlEncode(const nsACString & utf8Input,
   return Base64UrlEncodeImpl(utf8Input, result);
 }
 
-NS_IMETHODIMP
-IdentityCryptoService::Base64UrlDecode(const nsACString & base64Input,
-                                       nsACString & result)
-{
-  return Base64UrlDecodeImpl(base64Input, result);
-}
-
 KeyPair::KeyPair(SECKEYPrivateKey * privateKey, SECKEYPublicKey * publicKey)
   : mPrivateKey(privateKey)
   , mPublicKey(publicKey)

toolkit/identity/nsIIdentityCryptoService.idl

@@ -38,14 +38,13 @@ interface nsIIdentitySignCallback;
  */
 
 // "@mozilla.org/identity/crypto-service;1"
-[scriptable, builtinclass, uuid(17e227c4-2c31-4167-9dd4-f55ddee6a53a)]
+[scriptable, builtinclass, uuid(f087e6bc-dd33-4f6c-a106-dd786e052ee9)]
 interface nsIIdentityCryptoService : nsISupports
 {
   void generateKeyPair(in AUTF8String algorithm,
                        in nsIIdentityKeyGenCallback callback);
 
   ACString base64UrlEncode(in AUTF8String toEncode);
-  ACString base64UrlDecode(in AUTF8String toDecode);
 };
 
 /**

toolkit/identity/tests/unit/head_identity.js

@@ -8,6 +8,9 @@ const Cr = Components.results;
 
 Cu.import("resource://testing-common/httpd.js");
 
+// XXX until bug 937114 is fixed
+Cu.importGlobalProperties(['atob']);
+
 // The following boilerplate makes sure that XPCom calls
 // that use the profile directory work.
 
@@ -68,6 +71,29 @@ function uuid() {
   return uuidGenerator.generateUUID().toString();
 }
 
+function base64UrlDecode(s) {
+  s = s.replace(/-/g, '+');
+  s = s.replace(/_/g, '/');
+
+  // Replace padding if it was stripped by the sender.
+  // See http://tools.ietf.org/html/rfc4648#section-4
+  switch (s.length % 4) {
+    case 0:
+      break; // No pad chars in this case
+    case 2:
+      s += "==";
+      break; // Two pad chars
+    case 3:
+      s += "=";
+      break; // One pad char
+    default:
+      throw new InputException("Illegal base64url string!");
+  }
+
+  // With correct padding restored, apply the standard base64 decoder
+  return atob(s);
+}
+
 // create a mock "doc" object, which the Identity Service
 // uses as a pointer back into the doc object
 function mock_doc(aIdentity, aOrigin, aDoFunc) {

toolkit/identity/tests/unit/test_crypto_service.js

@@ -13,6 +13,25 @@ const idService = Cc["@mozilla.org/identity/crypto-service;1"]
 const ALG_DSA = "DS160";
 const ALG_RSA = "RS256";
 
+const BASE64_URL_ENCODINGS = [
+  // The vectors from RFC 4648 are very silly, but we may as well include them.
+  ["", ""],
+  ["f", "Zg=="],
+  ["fo", "Zm8="],
+  ["foo", "Zm9v"],
+  ["foob", "Zm9vYg=="],
+  ["fooba", "Zm9vYmE="],
+  ["foobar", "Zm9vYmFy"],
+
+  // It's quite likely you could get a string like this in an assertion audience
+  ["i-like-pie.com", "aS1saWtlLXBpZS5jb20="],
+
+  // A few extra to be really sure
+  ["andré@example.com", "YW5kcsOpQGV4YW1wbGUuY29t"],
+  ["πόλλ' οἶδ' ἀλώπηξ, ἀλλ' ἐχῖνος ἓν μέγα",
+   "z4DPjM67zrsnIM6_4by2zrQnIOG8gM67z47PgM63zr4sIOG8gM67zrsnIOG8kM-H4b-Wzr3Ov8-CIOG8k869IM68zq3Os86x"],
+];
+
 // When the output of an operation is a
 function do_check_eq_or_slightly_less(x, y) {
   do_check_true(x >= y - (3 * 8));
@@ -21,7 +40,7 @@ function do_check_eq_or_slightly_less(x, y) {
 function test_base64_roundtrip() {
   let message = "Attack at dawn!";
   let encoded = idService.base64UrlEncode(message);
-  let decoded = idService.base64UrlDecode(encoded);
+  let decoded = base64UrlDecode(encoded);
   do_check_neq(message, encoded);
   do_check_eq(decoded, message);
   run_next_test();
@@ -70,9 +89,33 @@ function test_rsa() {
   });
 }
 
+function test_base64UrlEncode() {
+  for (let [source, target] of BASE64_URL_ENCODINGS) {
+    do_check_eq(target, idService.base64UrlEncode(source));
+  }
+  run_next_test();
+}
+
+function test_base64UrlDecode() {
+  let utf8Converter = Cc["@mozilla.org/intl/scriptableunicodeconverter"]
+                        .createInstance(Ci.nsIScriptableUnicodeConverter);
+  utf8Converter.charset = "UTF-8";
+
+  // We know the encoding of our inputs - on conversion back out again, make
+  // sure they're the same.
+  for (let [source, target] of BASE64_URL_ENCODINGS) {
+    let result = utf8Converter.ConvertToUnicode(base64UrlDecode(target));
+    result += utf8Converter.Finish();
+    do_check_eq(source, result);
+  }
+  run_next_test();
+}
+
 add_test(test_base64_roundtrip);
 add_test(test_dsa);
 add_test(test_rsa);
+add_test(test_base64UrlEncode);
+add_test(test_base64UrlDecode);
 
 function run_test() {
   run_next_test();

toolkit/identity/tests/unit/test_jwcrypto.js

@@ -204,6 +204,29 @@ function test_assertion_lifetime() {
   );
 }
 
+function test_audience_encoding_bug972582() {
+  let audience = "i-like-pie.com";
+
+  jwcrypto.generateKeyPair(
+    "DS160",
+    function(err, kp) {
+      do_check_null(err);
+      jwcrypto.generateAssertion("fake-cert", kp, audience,
+        function(err, backedAssertion) {
+          do_check_null(err);
+
+          let [cert, assertion] = backedAssertion.split("~");
+          let components = extractComponents(assertion);
+          do_check_eq(components.payload.aud, audience);
+
+          do_test_finished();
+          run_next_test();
+        }
+      );
+    }
+  );
+}
+
 // End of tests
 // Helper function follow
 
@@ -221,8 +244,8 @@ function extractComponents(signedObject) {
   let payloadSegment = parts[1];
   let cryptoSegment = parts[2];
 
-  let header = JSON.parse(CryptoService.base64UrlDecode(headerSegment));
-  let payload = JSON.parse(CryptoService.base64UrlDecode(payloadSegment));
+  let header = JSON.parse(base64UrlDecode(headerSegment));
+  let payload = JSON.parse(base64UrlDecode(payloadSegment));
 
   // Ensure well-formed header
   do_check_eq(Object.keys(header).length, 1);
@@ -246,6 +269,7 @@ let TESTS = [
   test_get_assertion,
   test_get_assertion_with_offset,
   test_assertion_lifetime,
+  test_audience_encoding_bug972582,
 ];
 
 TESTS = TESTS.concat([test_rsa, test_dsa]);