Detect and remove double encoded redirects#6690
Conversation
Some hosts seem to re-encode parameters in a http=>https redirect, which causes problems with Jetpack auth. This checks the ‘redirect_to’ query param and if it detects a double-encoded value removes one level of encoding.
georgestephanis
left a comment
There was a problem hiding this comment.
I'd personally prefer to update the messaging, and contact hosts, (perhaps with some notification system to logstash or the like with the IP of the server so we can verify and reach out to the hosts) -- but I don't think I'm opposed to this way if it really is the best fix.
class.jetpack.php
Outdated
| } | ||
|
|
||
| static function is_redirect_encoded( $redirect_url ) { | ||
| return wp_startswith( $redirect_url, 'https%3A%2F%2F' ); |
There was a problem hiding this comment.
This only catches https urls -- would it be wiser to catch /https?/i starting (plus the encoded slashes)? -- some could capitalize the protocol (unlikely, but possible and may as well hit the edge cases)
There was a problem hiding this comment.
The redirect_url is (as far as I know) public-api.wordpress.com and so should be set by us and never http, but added the extra checks just in case!
Swap to a regex to catch weird edge-cases where protocol may not be lower case, or where the redirect_url may not be https
|
I've split off the detection part into #6704, which detects the problem and shows a specific message. It might be better to start there, see how wide-spread the problem is, and then circle back to this PR if it's big enough of a problem that silently fixing it makes sense. I'll close this for now. |
When WordPress.com starts authenticating with a Jetpack site it accesses the site via http (I assume because it doesn't know if the site supports SSL).
Some hosts redirect http to https, but are somehow misconfigured so that query parameters are re-encoded. This causes signature matching to fail, and this message to be shown:
Someone may be trying to trick you into giving them access to your siteI'm currently unsure as to the exact cause of the problem - it's possibly a misconfiguration with Apache, or maybe even a plugin.
Added: Really Simple SSL adds this to
.htaccesswhich is known to cause the problem on some (but not all) hosts.Adding
NEto the list of options on the rewrite rule fixes it.Testing the problem
You can check the problem with this:
curl -v http://lireo.com/?s=%40The server incorrectly redirects you to:
https://www.lireo.com/?s=%2540Note that the
%40has been double-encoded to%2540. The correct target should be:https://www.lireo.com/?s=%40I created this plugin that fakes the problem to make it easier to test fixes with.
Changes proposed in this Pull Request:
This PR checks the
redirect_toquery param and if it detects a double-encoded value removes one level of encoding. This means that the signature matching succeeds, and theredirect_togoes to the right place.This also has to be performed with the details saved to cookies in the SSO code.
Ideally the best solution is that the problem is removed from the server and we don't need this somewhat hacky solution. This PR silently removes the problem, but doesn't prevent the same problem occurring elsewhere.
An alternative to silently fixing the problem is that we change the 'someone may be trying to trick you' message to a full explanation of the problem, hoping that the user will be able to fix it properly - I don't know how realistic this is.
Also, maybe Jetpack could check for this and show an error in the dashboard. We may even be able to track statistics for how many people are affected.
Also it should be noted that I'm not familiar with the auth process, and there may be a better way of doing it. This is more to start the conversation. If a root cause can be discovered then it may help towards a better fix.
Testing instructions:
Requires a host that double-encodes the redirect (real or via above mentioned plugin):
Someone may be trying to trick you into giving them access to your siteerrorTry the same with this PR and the auth process should complete (SSO and normal login).
@georgeh