-
-
Save wojtha/989903 to your computer and use it in GitHub Desktop.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Ok! | |
If you (like me) are wondering what's up with there being 3 separate OpenID critical issues between D8 and D7 and what on earth it all means, and more importantly how to fix it, you're in luck! wojtha kindly spent about an hour with me in IRC today walking me through the state of things. | |
Here's the skinny: | |
There are two main issues where core's OpenID module is violating spec, and this leads to consequences ranging from certain OpenID providers just plain not working to possible impersonation risks: | |
[#575810]: When you enter an OpenID provider like "webchick.openid.com", Drupal normalize it into a fully-qualified domain for you, like "http://webchick.openid.com", sets it as a user Claimed Ideintifier and sends a request to that URL to retrieve supported services from the provider and that kind of stuff. The problem is that all public providers (and probably all of the providers around the world) redirects "http://webchick.openid.com" to more secure https://webchick.openid.com or sometimes to completely different URL like https://openid.com/webchick. According to specs Drupal should change that Claimed ID to that final URL and use it in the authentication request which he sends to the provider. However Drupal is <strong>completely ignoring</strong> this recommendation and instead of this it further uses the normalized user entered Claimed ID in the response (http://webchick.openid.com) no matter if that URL was redirected. We need to fix that. | |
[#1076366]: Another issue is related to Claim IDs. When you attach a new OpenID provider to your account, you're given a fragment identifier, something like a form token, to tie it uniquely to your openid account. For example, https://webchick.openid.com/#FUSGDKS. If webchick deletes his openid account and other webchick creates the new one, the new account will have different fragment. Drupal is erroneously stripping this fragment identifier from the external auth tables, which could lead to impersonation attacks. | |
THEN you have the problem of existing D6 and D7 sites where these values have already been stored all screwed up, and they need to be fixed (D7 needs a workaround for both of these issues; D6 only the redirect one because [#728278] did not yet get committed to D6). The only reasonable place to cut in and do this is during the login, because it requires manual intervention with a user's OpenID provider. But this involves sticking in some pretty nasty code in openid_authenticate() that needs to run on each openid login request. Furthermore API change for D7 and D6 is needed to fix [#575810] since Claimed ID were designed here as immutable. | |
Openid standard is pretty complex so both issues targets significant part of providers, but sure not all of them. | |
So. That is where we're at. The order of operations in terms of reviewing/committing these patches is: | |
1. Redirects for D8 | |
2. Fragments for D8 | |
3. Redirects for D7 | |
4. Fragments for D7 | |
5. Workarounds for D7 (must be committed along #3 and #4, or else no one with invalid openids can log in) | |
6. Redirects for D6 | |
7. Workaround for D6 (must be committed along with #6) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment