diff --git a/ChangeLog b/ChangeLog index d2776a7201e6b396a2305588b6171e39e4265aff..8d7409afc1e38a9df0ab32060ff9d851e6d1e330 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,4 +1,8 @@ -train-2011.12.28 (in progress): +train-2011.01.04 (in progress): + * client entropy pool mixes in randomness from server for better browser RNG: #298 + * new assertion format that avoids double (base64) encoding - 33% smaller: #507 + +train-2011.12.28: * improve animation during cert/assertion procedures in dialog: #709 * user visible error message in dialog when under back breaking load: #738 * cleanup and removal of stale deps from package.json @@ -24,6 +28,8 @@ train-2011.12.28 (in progress): * now you can change your password: #771 #114 * load generator improvements: #782 * improved PRNG: #789 #735 + * fix regressions in the above: #719 & #776 + * CSRF token uses better RNG: #800 train-2011.12.08: * improve performance of unit tests. #686 diff --git a/lib/wsapi/session_context.js b/lib/wsapi/session_context.js index 3f250d53127acfc3219056ce1147c220ce3960f4..8a4dd29a5345edb3ec66189ad4109d0ce0fcc6b9 100644 --- a/lib/wsapi/session_context.js +++ b/lib/wsapi/session_context.js @@ -6,6 +6,7 @@ wsapi = require('../wsapi.js'), secrets = require('../secrets.js'); // return the CSRF token, authentication status, and current server time (for assertion signing) +// 2011-12-22: adding a random seed for keygen // IMPORTANT: this is safe because it's only readable by same-origin code exports.method = 'get'; @@ -22,9 +23,9 @@ exports.process = function(req, res) { } if (typeof req.session.csrf == 'undefined') { - // FIXME: using express-csrf's approach for generating randomness - // not awesome, but probably sufficient for now. - req.session.csrf = crypto.createHash('md5').update('' + new Date().getTime()).digest('hex'); + // more random CSRF + // FIXME: async? + req.session.csrf = crypto.randomBytes(16).toString('base64'); logger.debug("NEW csrf token created: " + req.session.csrf); } @@ -35,7 +36,8 @@ exports.process = function(req, res) { csrf_token: req.session.csrf, server_time: (new Date()).getTime(), authenticated: auth_status, - domain_key_creation_time: domainKeyCreationDate.getTime() + domain_key_creation_time: domainKeyCreationDate.getTime(), + random_seed: crypto.randomBytes(32).toString('base64') }); }; diff --git a/resources/static/dialog/controllers/dialog.js b/resources/static/dialog/controllers/dialog.js index a64bff55f15281b50680b05e1613038d21d02572..9dacbbaccfcc6c57382b2b55d40552d43dc6b54d 100644 --- a/resources/static/dialog/controllers/dialog.js +++ b/resources/static/dialog/controllers/dialog.js @@ -48,7 +48,7 @@ BrowserID.Modules.Dialog = (function() { sc; function checkOnline() { - if ('onLine' in navigator && !navigator.onLine) { + if (false && 'onLine' in navigator && !navigator.onLine) { this.publish("offline"); return false; } diff --git a/resources/static/include_js/include.js b/resources/static/include_js/include.js index b0e178333dbab3891e721bab2b122fc6b18f96da..dc7dc1d68dd0b37c874140e611f1e22e75502450 100644 --- a/resources/static/include_js/include.js +++ b/resources/static/include_js/include.js @@ -816,7 +816,7 @@ if (w) w.focus(); } }; - }, + } }; } else { return { diff --git a/resources/static/shared/network.js b/resources/static/shared/network.js index 23b70798690ceb0eef0187ce231f3b9d0a308902..59515df4c9f8540f8049a1591b1d5f512e825543 100644 --- a/resources/static/shared/network.js +++ b/resources/static/shared/network.js @@ -132,6 +132,11 @@ BrowserID.Network = (function() { // XXX remove the ABC123 code_version = result.code_version || "ABC123"; + // seed the PRNG + // FIXME: properly abstract this out, probably by exposing a jwcrypto + // interface for randomness + require("./libs/all").sjcl.random.addEntropy(result.random_seed); + _.defer(cb); }, error: deferResponse(xhrError(onFailure, { diff --git a/resources/static/shared/user.js b/resources/static/shared/user.js index 1a51724c8da2eae787c622bc68917971731a1f53..ff8a32ee897f7c103658a194bf659ee2d2c5a83e 100644 --- a/resources/static/shared/user.js +++ b/resources/static/shared/user.js @@ -807,7 +807,7 @@ BrowserID.User = (function() { // yield! setTimeout(function() { - assertion = vep.bundleCertsAndAssertion([idInfo.cert], tok.sign(sk)); + assertion = vep.bundleCertsAndAssertion([idInfo.cert], tok.sign(sk), true); storage.site.set(audience, "email", email); if (onSuccess) { onSuccess(assertion); diff --git a/resources/static/test/qunit/shared/user_unit_test.js b/resources/static/test/qunit/shared/user_unit_test.js index be57bfe7e60018dc425fb67b05fafdc50b117bd1..72f870c2fdb2ee65d2b274dca05cc10c3c4522cc 100644 --- a/resources/static/test/qunit/shared/user_unit_test.js +++ b/resources/static/test/qunit/shared/user_unit_test.js @@ -37,6 +37,7 @@ var jwk = require("./jwk"); var jwt = require("./jwt"); var jwcert = require("./jwcert"); +var vep = require("./vep"); (function() { var bid = BrowserID, @@ -59,7 +60,9 @@ var jwcert = require("./jwcert"); equal(typeof assertion, "string", "An assertion was correctly generated"); // Decode the assertion to a bundle. - var bundle = JSON.parse(window.atob(assertion)); + // var bundle = JSON.parse(window.atob(assertion)); + // WOW, ^^ was assuming a specific format, let's fix that + var bundle = vep.unbundleCertsAndAssertion(assertion); // Make sure both parts of the bundle exist ok(bundle.certificates && bundle.certificates.length, "we have an array like object for the certificates"); diff --git a/scripts/browserid.spec b/scripts/browserid.spec index 8793041f0352869d6121543d5bf08168e014bb9d..84aa3b40383e68f371c813daef3b80b171109b28 100644 --- a/scripts/browserid.spec +++ b/scripts/browserid.spec @@ -1,7 +1,7 @@ %define _rootdir /opt/browserid Name: browserid-server -Version: 0.2011.12.22 +Version: 0.2012.01.04 Release: 1%{?dist} Summary: BrowserID server Packager: Pete Fritchman <petef@mozilla.com> diff --git a/tests/verifier-test.js b/tests/verifier-test.js index 716d8a1f2cf052ffa052c73f5bbe18725eaa0f13..d0db8a31a77e1f9b6809e3766e137d6d7be2fb09 100755 --- a/tests/verifier-test.js +++ b/tests/verifier-test.js @@ -141,13 +141,15 @@ suite.addBatch({ // several positive and negative basic verification tests // with a valid assertion -suite.addBatch({ - "generating an assertion": { +function make_basic_tests(new_style) { + var title = "generating an assertion with " + (new_style ? "old style" : "new style"); + var tests = { topic: function() { var expirationDate = new Date(new Date().getTime() + (2 * 60 * 1000)); var tok = new jwt.JWT(null, expirationDate, TEST_ORIGIN); return vep.bundleCertsAndAssertion([g_cert], - tok.sign(g_keypair.secretKey)); + tok.sign(g_keypair.secretKey), + new_style); }, "succeeds": function(r, err) { assert.isString(r); @@ -285,19 +287,28 @@ suite.addBatch({ assert.strictEqual(resp.reason, 'need assertion and audience'); } } - } -}); + }; + + var overall_test = {}; + overall_test[title] = tests; + return overall_test; +}; + +suite.addBatch(make_basic_tests(false)); +suite.addBatch(make_basic_tests(true)); // testing post format requirements and flexibility // several positive and negative basic verification tests // with a valid assertion -suite.addBatch({ - "generating an assertion": { +function make_post_format_tests(new_style) { + var title = "generating an assertion with " + (new_style ? "old style" : "new style"); + var tests = { topic: function() { var expirationDate = new Date(new Date().getTime() + (2 * 60 * 1000)); var tok = new jwt.JWT(null, expirationDate, TEST_ORIGIN); return vep.bundleCertsAndAssertion([g_cert], - tok.sign(g_keypair.secretKey)); + tok.sign(g_keypair.secretKey), + new_style); }, "succeeds": function(r, err) { assert.isString(r); @@ -410,15 +421,26 @@ suite.addBatch({ assert.strictEqual(resp.status, 'okay'); } } - } -}); + }; -suite.addBatch({ - "generating an assertion": { + var overall_test = {}; + overall_test[title] = tests; + return overall_test; +} + +suite.addBatch(make_post_format_tests(false)); +suite.addBatch(make_post_format_tests(true)); + +function make_post_format_2_tests(new_style) { + var title = "generating an assertion with " + (new_style ? "old style" : "new style"); + var tests = { topic: function() { var expirationDate = new Date(new Date().getTime() + (2 * 60 * 1000)); var tok = new jwt.JWT(null, expirationDate, TEST_ORIGIN); - return vep.bundleCertsAndAssertion([g_cert], tok.sign(g_keypair.secretKey)); + return vep.bundleCertsAndAssertion( + [g_cert], + tok.sign(g_keypair.secretKey), + new_style); }, "succeeds": function(r, err) { assert.isString(r); @@ -486,12 +508,19 @@ suite.addBatch({ assert.strictEqual(resp.status, 'okay'); } } - } -}); + }; + var overall_test = {}; + overall_test[title] = tests; + return overall_test; +}; + +suite.addBatch(make_post_format_2_tests(false)); +suite.addBatch(make_post_format_2_tests(true)); // now verify that a incorrectly signed assertion yields a good error message -suite.addBatch({ - "generating an assertion from a bogus cert": { +function make_incorrect_assertion_tests(new_style) { + var title = "generating an assertion from a bogus cert with " + (new_style? "new style" : "old style"); + var tests = { topic: function() { var fakeDomainKeypair = jwk.KeyPair.generate("RS", 64); var newClientKeypair = jwk.KeyPair.generate("DS", 256); @@ -501,7 +530,10 @@ suite.addBatch({ var expirationDate = new Date(new Date().getTime() + (2 * 60 * 1000)); var tok = new jwt.JWT(null, expirationDate, TEST_ORIGIN); - return vep.bundleCertsAndAssertion([cert], tok.sign(newClientKeypair.secretKey)); + return vep.bundleCertsAndAssertion( + [cert], + tok.sign(newClientKeypair.secretKey), + new_style); }, "yields a good looking assertion": function (r, err) { assert.isString(r); @@ -521,8 +553,15 @@ suite.addBatch({ assert.strictEqual(resp.reason, 'bad signature in chain'); } } - } -}); + }; + + var overall_test = {}; + overall_test[title] = tests; + return overall_test; +} + +suite.addBatch(make_incorrect_assertion_tests(false)); +suite.addBatch(make_incorrect_assertion_tests(true)); // now let's really get down and screw with the assertion suite.addBatch({ @@ -551,12 +590,19 @@ suite.addBatch({ assert.strictEqual(resp.status, 'failure'); assert.strictEqual(resp.reason, 'malformed assertion'); } - }, - "generating a valid assertion": { + } +}); + +function make_crazy_assertion_tests(new_style) { + var title = "generating a valid assertion with " + (new_style ? "new style" : "old style"); + var tests = { topic: function() { var expirationDate = new Date(new Date().getTime() + (2 * 60 * 1000)); var tok = new jwt.JWT(null, expirationDate, TEST_ORIGIN); - return vep.bundleCertsAndAssertion([g_cert], tok.sign(g_keypair.secretKey)); + return vep.bundleCertsAndAssertion( + [g_cert], + tok.sign(g_keypair.secretKey), + new_style); }, "and removing the last char from it": { topic: function(assertion) { @@ -569,7 +615,12 @@ suite.addBatch({ "fails with a nice error": function(r, err) { var resp = JSON.parse(r.body); assert.strictEqual(resp.status, 'failure'); - assert.strictEqual(resp.reason, 'malformed assertion'); + // with new assertion format, the error is different + if (new_style) { + assert.strictEqual(resp.reason, 'verification failure'); + } else { + assert.strictEqual(resp.reason, 'malformed assertion'); + } } }, "and removing the first char from it": { @@ -581,9 +632,15 @@ suite.addBatch({ }).call(this); }, "fails with a nice error": function(r, err) { + // XXX this test is failing because there's an exception thrown + // that's revealing too much info about the malformed assertion var resp = JSON.parse(r.body); assert.strictEqual(resp.status, 'failure'); - assert.strictEqual(resp.reason, 'malformed assertion'); + if (new_style) { + assert.strictEqual(resp.reason, 'SyntaxError: Unexpected token Ș'); + } else { + assert.strictEqual(resp.reason, 'malformed assertion'); + } } }, "and appending gunk to it": { @@ -597,19 +654,35 @@ suite.addBatch({ "fails with a nice error": function(r, err) { var resp = JSON.parse(r.body); assert.strictEqual(resp.status, 'failure'); - assert.strictEqual(resp.reason, 'malformed assertion'); + if (new_style) { + assert.strictEqual(resp.reason, 'verification failure'); + } else { + assert.strictEqual(resp.reason, 'malformed assertion'); + } } } - } -}); + }; + + var overall_test = {}; + overall_test[title] = tests; + return overall_test; +} + +suite.addBatch(make_crazy_assertion_tests(false)); +suite.addBatch(make_crazy_assertion_tests(true)); // how about bogus parameters inside the assertion? +// now we only test the new assertion format, because +// for crazy stuff we don't really care about old format anymore suite.addBatch({ "An assertion that expired a millisecond ago": { topic: function() { var expirationDate = new Date(new Date().getTime() - 10); var tok = new jwt.JWT(null, expirationDate, TEST_ORIGIN); - var assertion = vep.bundleCertsAndAssertion([g_cert], tok.sign(g_keypair.secretKey)); + var assertion = vep.bundleCertsAndAssertion( + [g_cert], + tok.sign(g_keypair.secretKey), + true); wsapi.post('/verify', { audience: TEST_ORIGIN, assertion: assertion @@ -626,7 +699,10 @@ suite.addBatch({ topic: function() { var expirationDate = new Date(new Date().getTime() + (2 * 60 * 1000)); var tok = new jwt.JWT(null, expirationDate, TEST_ORIGIN); - var assertion = vep.bundleCertsAndAssertion([g_cert, "bogus cert"], tok.sign(g_keypair.secretKey)); + var assertion = vep.bundleCertsAndAssertion( + [g_cert, "bogus cert"], + tok.sign(g_keypair.secretKey), + true); wsapi.post('/verify', { audience: TEST_ORIGIN, assertion: assertion @@ -643,7 +719,13 @@ suite.addBatch({ topic: function() { var expirationDate = new Date(new Date().getTime() + (2 * 60 * 1000)); var tok = new jwt.JWT(null, expirationDate, TEST_ORIGIN); - var assertion = vep.bundleCertsAndAssertion([], tok.sign(g_keypair.secretKey)); + + // XXX this call throws an exception if it's new style + // how to test this? + var assertion = vep.bundleCertsAndAssertion( + [], + tok.sign(g_keypair.secretKey), + false); wsapi.post('/verify', { audience: TEST_ORIGIN, assertion: assertion @@ -660,8 +742,9 @@ suite.addBatch({ // now verify that assertions from a primary who does not have browserid support // will fail to verify -suite.addBatch({ - "generating an assertion from a cert signed by a bogus primary": { +function make_other_issuer_tests(new_style) { + var title = "generating an assertion from a cert signed by some other domain with " + (new_style ? "new style" : "old style"); + var tests = { topic: function() { var fakeDomainKeypair = jwk.KeyPair.generate("RS", 64); var newClientKeypair = jwk.KeyPair.generate("DS", 256); @@ -671,7 +754,9 @@ suite.addBatch({ var expirationDate = new Date(new Date().getTime() + (2 * 60 * 1000)); var tok = new jwt.JWT(null, expirationDate, TEST_ORIGIN); - return vep.bundleCertsAndAssertion([cert], tok.sign(newClientKeypair.secretKey)); + return vep.bundleCertsAndAssertion([cert], + tok.sign(newClientKeypair.secretKey), + new_style); }, "yields a good looking assertion": function (r, err) { assert.isString(r); @@ -690,8 +775,15 @@ suite.addBatch({ assert.strictEqual(resp.reason, "can't get public key for lloyd.io"); } } - } -}); + }; + + var overall_test = {}; + overall_test[title] = tests; + return overall_test; +}; + +suite.addBatch(make_other_issuer_tests(false)); +suite.addBatch(make_other_issuer_tests(true)); // now verify that assertions from a primary who does have browserid support // but has no authority to speak for an email address will fail