Today we looked into what of our applications where vulnerable to the "covert redirect" vulnerability in the news today. We found that our OpenID provider, FedOAuth, was not generally vulnerable to the server side aspects. But about half of our clients had some issues with redirection.
If an attacker somehow tricked another user into visiting a link like this: https://copr.fedoraproject.org/login/?next=http://evilsite.com Copr would send the user to evilsite.com after successfully authenticating with fedoauth.
The patch for this, at first glance, is very simple:
-oid = OpenID(app, app.config["OPENID_STORE"]) +oid = OpenID(app, app.config["OPENID_STORE"], safe_roots=[])
However, the "safe_roots" argument to OpenID was only introduced in python-openid-1.2. The copr frontend is running on Fedora 19 and so is, for the moment, stuck with python-openid-1.0.1. Upgrading is probably the best bet, but it may cause other unforeseen issues.
As a workaround, I've attached a second patch that tells copr to just ignore the 'next' parameter and redirect always back to the copr root url. This second workaround patch is applied as a 'hotfix' in ansible and deployed to the copr cloud node.
http://infrastructure.fedoraproject.org/cgit/ansible.git/commit/?id=602405b5...
Dne 3.5.2014 04:28, Ralph Bean napsal(a):
As a workaround, I've attached a second patch that tells copr to just ignore the 'next' parameter and redirect always back to the copr root url. This second workaround patch is applied as a 'hotfix' in ansible and deployed to the copr cloud node.
return flask.redirect(oid.get_next_url())
return flask.redirect(flask.request.url_root)
Is it not possible to check if oid.get_next_url() starts with '/' and use it only then?
if oid.get_next_url().starstwith('/'): return flask.redirect(oid.get_next_url()) else: return flask.redirect(flask.request.url_root)
If oid.get_next_url() is an absolute URL (as it sounds to me) the check might be a bit more complex (i.e. allow both http and https and both copr-fe.cloud.fedoraproject.org and copr.fedoraproject.org).
On Sat, May 03, 2014 at 09:19:56AM +0200, Miro Hrončok wrote:
Dne 3.5.2014 04:28, Ralph Bean napsal(a):
As a workaround, I've attached a second patch that tells copr to just ignore the 'next' parameter and redirect always back to the copr root url. This second workaround patch is applied as a 'hotfix' in ansible and deployed to the copr cloud node.
return flask.redirect(oid.get_next_url())
return flask.redirect(flask.request.url_root)Is it not possible to check if oid.get_next_url() starts with '/' and use it only then?
if oid.get_next_url().starstwith('/'): return flask.redirect(oid.get_next_url()) else: return flask.redirect(flask.request.url_root)If oid.get_next_url() is an absolute URL (as it sounds to me) the check might be a bit more complex (i.e. allow both http and https and both copr-fe.cloud.fedoraproject.org and copr.fedoraproject.org).
I pointed Patrick to your reply (since he is not on the list) and his answer is: """ that should work, as that's basically the fix that you do when you enable safe_roots, but that they should consider upgrading anyway as there's been more security and bug fixes """
Cheers, Pierre
On 05/03/2014 04:28 AM, Ralph Bean wrote:
The patch for this, at first glance, is very simple:
-oid = OpenID(app, app.config["OPENID_STORE"]) +oid = OpenID(app, app.config["OPENID_STORE"], safe_roots=[])
However, the "safe_roots" argument to OpenID was only introduced in python-openid-1.2. The copr frontend is running on Fedora 19 and so is, for the moment, stuck with python-openid-1.0.1. Upgrading is probably the best bet, but it may cause other unforeseen issues.
Fedora 19 have python-openid-2.2.5-5.fc19.noarch, which seems much higher then 1.2 to me :)
So which version support safe_roots=[] ?
On 05/03/2014 04:28 AM, Ralph Bean wrote:
Upgrading is probably the best bet, but it may cause other unforeseen issues.
As a workaround, I've attached a second patch that tells copr to just ignore the 'next' parameter and redirect always back to the copr root url. This second workaround patch is applied as a 'hotfix' in ansible and deployed to the copr cloud node.
I commited the first patch. I would like to avoid that second one and rather do it properly and if it means upgrading to F20 then let it be.
I planned to do it for some time anyway. I will upgrade -dev instance and if everything will be ok, I will upgrade prod next week.
On Mon, May 05, 2014 at 08:55:53AM +0200, Miroslav Suchý wrote:
On 05/03/2014 04:28 AM, Ralph Bean wrote:
The patch for this, at first glance, is very simple:
-oid = OpenID(app, app.config["OPENID_STORE"]) +oid = OpenID(app, app.config["OPENID_STORE"], safe_roots=[])
However, the "safe_roots" argument to OpenID was only introduced in python-openid-1.2. The copr frontend is running on Fedora 19 and so is, for the moment, stuck with python-openid-1.0.1. Upgrading is probably the best bet, but it may cause other unforeseen issues.
Fedora 19 have python-openid-2.2.5-5.fc19.noarch, which seems much higher then 1.2 to me :)
So which version support safe_roots=[] ?
My apologies, I had the version number correct but typoed the package name. python-flask-openid is the package to look at:
~❯ pkgwat releases python-flask-openid +---------------+----------------+-----------------+ | release | stable_version | testing_version | +---------------+----------------+-----------------+ | Rawhide | 1.2-1.fc21 | None | | Fedora 20 | 1.2.1-1.fc20 | None | | Fedora 19 | 1.0.1-5.fc19 | None | | Fedora EPEL 7 | None | None | | Fedora EPEL 6 | 1.2-1.el6 | None | | Fedora EPEL 5 | None | None | +---------------+----------------+-----------------+
copr-devel@lists.fedorahosted.org
-
Miro Hrončok -
Miroslav Suchý -
Pierre-Yves Chibon -
Ralph Bean