Bug 972582 - Fix broken base64UrlDecode in toolkit/identity. r=MattN, r=warner, sr=mossop
authorJed Parsons <jedp@mozilla.com>
Wed, 19 Feb 2014 13:53:12 -0800
changeset 170043 953fda48369a77edcf4555611c560637e991893b
parent 170042 7b08710ab4e2c235e0a59bdc6cf3218f93182887
child 170044 5dea7efc75151f95b622108be5b5be48b46da0ea
push id270
push userpvanderbeken@mozilla.com
push dateThu, 06 Mar 2014 09:24:21 +0000
reviewersMattN, warner, mossop
bugs972582
milestone30.0a1
Bug 972582 - Fix broken base64UrlDecode in toolkit/identity. r=MattN, r=warner, sr=mossop
toolkit/identity/IdentityCryptoService.cpp
toolkit/identity/nsIIdentityCryptoService.idl
toolkit/identity/tests/unit/head_identity.js
toolkit/identity/tests/unit/test_crypto_service.js
toolkit/identity/tests/unit/test_jwcrypto.js
--- a/toolkit/identity/IdentityCryptoService.cpp
+++ b/toolkit/identity/IdentityCryptoService.cpp
@@ -60,38 +60,16 @@ Base64UrlEncodeImpl(const nsACString & u
     } else if (out[i] == '/') {
       out[i] = '_';
     }
   }
 
   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"))
 
 class KeyPair : public nsIIdentityKeyPair, public nsNSSShutDownObject
 {
 public:
   NS_DECL_THREADSAFE_ISUPPORTS
   NS_DECL_NSIIDENTITYKEYPAIR
@@ -255,23 +233,16 @@ IdentityCryptoService::GenerateKeyPair(
 
 NS_IMETHODIMP
 IdentityCryptoService::Base64UrlEncode(const nsACString & utf8Input,
                                        nsACString & result)
 {
   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)
 {
   MOZ_ASSERT(!NS_IsMainThread());
 }
 
 NS_IMETHODIMP
--- a/toolkit/identity/nsIIdentityCryptoService.idl
+++ b/toolkit/identity/nsIIdentityCryptoService.idl
@@ -33,24 +33,23 @@ interface nsIIdentitySignCallback;
  *
  * "DS160": DSA with SHA-1. A 1024-bit prime and a 160-bit subprime with SHA-1.
  *
  * we use these abbreviated algorithm names as per the JWA spec
  * http://tools.ietf.org/html/draft-ietf-jose-json-web-algorithms-02
  */
 
 // "@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);
 };
 
 /**
  * This interface provides a keypair and signing interface for Identity functionality
  */
 [scriptable, uuid(73962dc7-8ee7-4346-a12b-b039e1d9b54d)]
 interface nsIIdentityKeyPair : nsISupports
 {
--- a/toolkit/identity/tests/unit/head_identity.js
+++ b/toolkit/identity/tests/unit/head_identity.js
@@ -3,16 +3,19 @@
 
 const Cc = Components.classes;
 const Ci = Components.interfaces;
 const Cu = Components.utils;
 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.
 
 Cu.import("resource://gre/modules/XPCOMUtils.jsm");
 Cu.import("resource://gre/modules/Services.jsm");
 
 XPCOMUtils.defineLazyModuleGetter(this, "jwcrypto",
                                   "resource://gre/modules/identity/jwcrypto.jsm");
@@ -63,16 +66,39 @@ function partial(fn) {
     return fn.apply(this, args.concat(Array.prototype.slice.call(arguments)));
   };
 }
 
 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) {
   let mockedDoc = {};
   mockedDoc.id = uuid();
   mockedDoc.loggedInUser = aIdentity;
   mockedDoc.origin = aOrigin;
   mockedDoc['do'] = aDoFunc;
@@ -182,18 +208,18 @@ function setup_provisioning(identity, af
   IDService.IDP._provisionFlows[provId] = {
     identity : identity,
     idpParams: TEST_IDPPARAMS,
     callback: function(err) {
       if (doneProvisioningCallback)
         doneProvisioningCallback(err);
     },
     sandbox: {
-	// Emulate the free() method on the iframe sandbox
-	free: function() {}
+  // Emulate the free() method on the iframe sandbox
+  free: function() {}
     }
   };
 
   let caller = {};
   caller.id = provId;
   caller.doBeginProvisioningCallback = function(id, duration_s) {
     if (callerCallbacks && callerCallbacks.beginProvisioningCallback)
       callerCallbacks.beginProvisioningCallback(id, duration_s);
--- a/toolkit/identity/tests/unit/test_crypto_service.js
+++ b/toolkit/identity/tests/unit/test_crypto_service.js
@@ -8,25 +8,44 @@ Cu.import("resource://gre/modules/XPCOMU
 Cu.import('resource://gre/modules/identity/LogUtils.jsm');
 
 const idService = Cc["@mozilla.org/identity/crypto-service;1"]
                     .getService(Ci.nsIIdentityCryptoService);
 
 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));
 }
 
 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();
 }
 
 function test_dsa() {
   idService.generateKeyPair(ALG_DSA, function (rv, keyPair) {
     log("DSA generateKeyPair finished ", rv);
@@ -65,15 +84,39 @@ function test_rsa() {
       log("RSA sign finished ", rv, signature);
       do_check_true(Components.isSuccessCode(rv));
       do_check_true(signature.length > 1);
       run_next_test();
     });
   });
 }
 
+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();
 }
--- a/toolkit/identity/tests/unit/test_jwcrypto.js
+++ b/toolkit/identity/tests/unit/test_jwcrypto.js
@@ -199,16 +199,39 @@ function test_assertion_lifetime() {
           do_test_finished();
           run_next_test();
         }
       );
     }
   );
 }
 
+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
 
 function extractComponents(signedObject) {
   if (typeof(signedObject) != 'string') {
     throw new Error("malformed signature " + typeof(signedObject));
   }
 
@@ -216,18 +239,18 @@ function extractComponents(signedObject)
   if (parts.length != 3) {
     throw new Error("signed object must have three parts, this one has " + parts.length);
   }
 
   let headerSegment = parts[0];
   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);
   do_check_true(!!header.alg);
 
   // Ensure well-formed payload
   for (let field of ["exp", "aud"]) {
     do_check_true(!!payload[field]);
@@ -241,16 +264,17 @@ function extractComponents(signedObject)
 };
 
 let TESTS = [
   test_sanity,
   test_generate,
   test_get_assertion,
   test_get_assertion_with_offset,
   test_assertion_lifetime,
+  test_audience_encoding_bug972582,
 ];
 
 TESTS = TESTS.concat([test_rsa, test_dsa]);
 
 TESTS.forEach(add_test);
 
 function run_test() {
   run_next_test();