diff --git a/browserid/static/dialog/controllers/authenticate_controller.js b/browserid/static/dialog/controllers/authenticate_controller.js index b9164e931e21cc885641a3d80c1154c60f439629..0054f37c37efc626e98afa6d5eccc40c7a86089b 100644 --- a/browserid/static/dialog/controllers/authenticate_controller.js +++ b/browserid/static/dialog/controllers/authenticate_controller.js @@ -192,7 +192,7 @@ this._super(el, { bodyTemplate: "authenticate.ejs", bodyVars: { - sitename: user.getOrigin(), + sitename: user.getHostname(), email: options.email || "" } }); diff --git a/browserid/static/dialog/controllers/dialog_controller.js b/browserid/static/dialog/controllers/dialog_controller.js index 1f369bdba719e4f562bb4a5e57b15fe2f52e9fe8..491d983fabfa5bced7d4e27424bf97ac90c57715 100644 --- a/browserid/static/dialog/controllers/dialog_controller.js +++ b/browserid/static/dialog/controllers/dialog_controller.js @@ -63,7 +63,7 @@ user.setOrigin(origin_url); // get the cleaned origin. - $("#sitename").text(user.getOrigin()); + $("#sitename").text(user.getHostname()); this.doCheckAuth(); diff --git a/browserid/static/dialog/resources/user.js b/browserid/static/dialog/resources/user.js index b78bc6fb54c3db1f082e11daf338b165f7b5e814..27105f7f4e798961bd9669063cb850501671b9d1 100644 --- a/browserid/static/dialog/resources/user.js +++ b/browserid/static/dialog/resources/user.js @@ -98,10 +98,6 @@ BrowserID.User = (function() { } } - function filterOrigin(origin) { - return origin.replace(/^.*:\/\//, ""); - } - function registrationPoll(checkFunc, email, onSuccess, onFailure) { function poll() { checkFunc(email, function(status) { @@ -201,8 +197,8 @@ BrowserID.User = (function() { * @method setOrigin * @param {string} origin */ - setOrigin: function(unfilteredOrigin) { - origin = filterOrigin(unfilteredOrigin); + setOrigin: function(originArg) { + origin = originArg; }, /** @@ -214,6 +210,15 @@ BrowserID.User = (function() { return origin; }, + /** + * Get the hostname for the set origin + * @method getHostname + * @returns {string} + */ + getHostname: function() { + return origin.replace(/^.*:\/\//, "").replace(/:\d*$/, ""); + }, + /** * Create a user account - this creates an user account that must be verified. * @method createUser @@ -575,9 +580,7 @@ BrowserID.User = (function() { */ clearStoredEmailKeypairs: function() { storage.clear(); - }, - - + } }; User.setOrigin(document.location.host); diff --git a/browserid/static/dialog/test/qunit/resources/user_unit_test.js b/browserid/static/dialog/test/qunit/resources/user_unit_test.js index ae2791b7e1d5444961c9965a117cc1dd235adaa7..7e0b15155355d61d644a5aa3a48ff7d92079ece7 100644 --- a/browserid/static/dialog/test/qunit/resources/user_unit_test.js +++ b/browserid/static/dialog/test/qunit/resources/user_unit_test.js @@ -221,6 +221,14 @@ steal.plugins("jquery", "funcunit/qunit").then("/dialog/resources/user", functio equal(lib.getOrigin(), testOrigin); }); + test("setOrigin, getHostname", function() { + var origin = "http://testorigin.com:10001"; + lib.setOrigin(origin); + + var hostname = lib.getHostname(); + equal(hostname, "testorigin.com", "getHostname returns only the hostname"); + }); + test("getStoredEmailKeypairs", function() { var identities = lib.getStoredEmailKeypairs(); equal("object", typeof identities, "we have some identities"); diff --git a/rp/index.html b/rp/index.html index 06b071670dd11d27a44625bc8ac7127cb466d3fc..2b258e7c52cd1b38a553122f52f88e82954784f8 100644 --- a/rp/index.html +++ b/rp/index.html @@ -94,7 +94,7 @@ a:hover { border-bottom: 2px solid black ; } var url = "https://browserid.org/verify" var data = { assertion: assertion, - audience: window.location.host + audience: window.location.protocol + "//" + window.location.host }; $("#oVerificationRequest").empty().text("POST " + url + "\n" + JSON.stringify(data)); diff --git a/verifier/app.js b/verifier/app.js index d28dc2173e61641ed85928cb53f5254ef9602112..8a7d95e62ca09e286dfe23b7f68b130079193f05 100644 --- a/verifier/app.js +++ b/verifier/app.js @@ -68,25 +68,25 @@ function doVerify(req, resp, next) { certassertion.verify( assertion, audience, - function(email, audience, expires, issuer) { + function(email, audienceFromAssertion, expires, issuer) { resp.json({ status : "okay", email : email, - audience : audience, + audience : audience, // NOTE: we return the audience formatted as the RP provided it, not normalized in any way. expires : expires.valueOf(), issuer: issuer }); metrics.report('verify', { result: 'success', - rp: audience + rp: audienceFromAssertion }); }, function(error) { resp.json({"status":"failure", reason: (error ? error.toString() : "unknown")}); metrics.report('verify', { result: 'failure', - rp: audience + rp: audienceFromAssertion }); }); diff --git a/verifier/lib/certassertion.js b/verifier/lib/certassertion.js index 748d287d3398a44c68d9ec3e8094c9f4f757006e..2bbf6c9857606d1474f0a58d1f274fb3b1454bb2 100644 --- a/verifier/lib/certassertion.js +++ b/verifier/lib/certassertion.js @@ -118,15 +118,49 @@ function retrieveHostPublicKey(host, successCB, errorCB) { // cache it publicKeys[host] = pk; - + return successCB(pk); }); }); - + parser.parseString(hostmeta); }, errorCB); } +// compare two audiences: +// *want* is what was extracted from the assertion (it's trusted, we +// generated it! +// *got* is what was provided by the RP, so depending on their implementation +// it might be strangely formed. +function compareAudiences(want, got) { + try { + // issue #82 - for a limited time, let's allow got to be sloppy and omit scheme + // in which case we guess a scheme based on port + if (!/^https?:\/\//.test(got)) { + var x = got.split(':'); + var scheme = "http"; + if (x.length === 2 && x[1] === '443') scheme = "https"; + got = scheme + "://" + got; + } + + // now parse and compare + function normalizeParsedURL(u) { + if (!u.port) u.port = u.protocol === 'https:' ? 443 : 80; + return u; + } + + want = normalizeParsedURL(url.parse(want)); + + got = normalizeParsedURL(url.parse(got)); + + return (want.protocol === got.protocol && + want.hostname === got.hostname && + want.port === got.port); + } catch(e) { + return false; + } +} + // verify the tuple certList, assertion, audience // // assertion is a bundle of the underlying assertion and the cert list @@ -161,7 +195,7 @@ function verify(assertion, audience, successCB, errorCB, pkRetriever) { tok.parse(bundle.assertion); // audience must match! - if (tok.audience != audience) { + if (!compareAudiences(tok.audience, audience)) { logger.debug("verification failure, audience mismatch: '" + tok.audience + "' != '" + audience + "'"); return errorCB("audience mismatch"); @@ -174,7 +208,7 @@ function verify(assertion, audience, successCB, errorCB, pkRetriever) { } }, errorCB); } - + exports.retrieveHostPublicKey = retrieveHostPublicKey; exports.verify = verify; \ No newline at end of file