From 80c5d8a11cc120f37ea101ca213571ff8bd351c0 Mon Sep 17 00:00:00 2001 From: Lloyd Hilaiel <lloyd@hilaiel.com> Date: Wed, 19 Oct 2011 18:41:44 -0600 Subject: [PATCH] include full origin (inc. scheme) in generated assertions. To preserve compatibility, allow the rp to omit scheme in post data, and subsequent to verification, return whatever the rp sent us. issue #82 --- browserid/static/dialog/resources/user.js | 12 ++----- rp/index.html | 2 +- verifier/app.js | 8 ++--- verifier/lib/certassertion.js | 42 ++++++++++++++++++++--- 4 files changed, 46 insertions(+), 18 deletions(-) diff --git a/browserid/static/dialog/resources/user.js b/browserid/static/dialog/resources/user.js index b78bc6fb5..57a491795 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; }, /** @@ -575,9 +571,7 @@ BrowserID.User = (function() { */ clearStoredEmailKeypairs: function() { storage.clear(); - }, - - + } }; User.setOrigin(document.location.host); diff --git a/rp/index.html b/rp/index.html index 06b071670..2b258e7c5 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 d28dc2173..8a7d95e62 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 748d287d3..2bbf6c985 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 -- GitLab