Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Specific redirect URI for each OP #291

Closed
remi-cc opened this issue Aug 21, 2017 · 17 comments
Closed

Specific redirect URI for each OP #291

remi-cc opened this issue Aug 21, 2017 · 17 comments

Comments

@remi-cc
Copy link

remi-cc commented Aug 21, 2017

Dear Mr Zandbelt,

I have a new feature request for security reasons.
In order to mitigate Mixup-Attack, the OpenID foundation advise to specify a redirect_uri different for each OP, like :
www.myapacheserveur.com/callback1 for the first OP;
www.myapacheserveur.com/callback2 for the second OP.

Then, RP must verify that the authorization response has been received on the specific URI associated with the OP of the user session.

All informations are available on OpenID Foundation website :
http://openid.net/2016/07/16/preventing-mix-up-attacks-with-openid-connect/
"Register a different redirect URI for each OP, and record the redirect URI used in the outgoing authorization request in the user’s session together with state and nonce. On receiving the authorization code in the response, verify that the user’s session contains the state value of the response, and that the redirect URI of the response matches the one used in the request."

Do you think that you can implement this?

@zandbelt
Copy link
Member

zandbelt commented Aug 22, 2017

There's an existing mitigation mechanism in place for that:

one can add iss and/or client_id (with the correct values for a particular OP) to the Redirect URI that you register with the OP and mod_auth_openidc will verify those values when the authorization response comes back. See: https://github.com/pingidentity/mod_auth_openidc/blob/master/src/proto.c#L2719

So you'd register:
www.myapacheserveur.com/callback?iss=op1 for the first OP;
www.myapacheserveur.com/callback?iss=op2 for the second OP;

where op1 is the issuer identifier of the first OP and op2 that of the 2nd OP.

This mechanism was one of the proposed solutions that were determined by the OAuth working group when analyzing this attack.

@remi-cc
Copy link
Author

remi-cc commented Aug 22, 2017

Ok, i registered :
OIDCRedirectURI https://www.myRP.com/callback in the RP apache conf.
Then, i registered :
https://www.myRP.com/callback?iss=https://www.myOP1.com as redirect_uri in my first OP where https://www.myOP1.com is the «iss» value of OP1 given in metadata OP1.provider file.
I registered for the second OP this redirect_uri :
https://www.myRP.com/callback?iss=https://www.myOP2.com

But the authorization request send by mod_auth contains this static redirect_uri parameter :
https://www.myRP.com/callback (url encoded)

So this redirect_uri value doesn't match exactly with the one registered in my OPs and the authorization request is refused.

How can i configure mod_auth to build dynamicaly the good redirect_uri param in the authorization request?

@zandbelt
Copy link
Member

It requires matching on the OP side to ignore request parameters.

@remi-cc
Copy link
Author

remi-cc commented Aug 22, 2017

But in the OAuth2 / OIDC specs, the authorization request validation (OP side) specify that the redirect_uri param must match exactly with the one registered.

Don't you think that mod_auth can set this param dynamicaly to respect specs?

I think for example :
OIDCRedirectURI https://www.myRP.com/callback?iss=
And mod auth will determine the good redirect_uri to send.

@zandbelt
Copy link
Member

The query parameter may be ignored but if your OP does not support that then there's no alternative currently. I'll keep this as a feature request but prioritizing it would require a commercial discussion.

zandbelt added a commit that referenced this issue Aug 29, 2017
used in multi-provider setups to mitigate the IDP mixup attack by
setting "issuer_specific_redirect_uri" in the .conf file; this will add
a parameter "iss" to the redirect URI with the url-encoded issuer value;
thanks @remi-cc

Signed-off-by: Hans Zandbelt <hans.zandbelt@zmartzone.eu>
@zandbelt
Copy link
Member

In a167873 I've added the option to add the "iss" parameter to the redirect URI in multi-provider setups. This is less preferred than adding it in the redirect URI path but a lot easier to implement and backwards compatible. Would you be able to verify this?

@remi-cc
Copy link
Author

remi-cc commented Aug 29, 2017

Let me verify it with pleasure

@zandbelt
Copy link
Member

I realized that OpenID Connect indeed requires simple string comparison as the redirect URI matching algorithm and as such the existing mitigation is not enough by spec though it may work against various providers, e.g. Google was tested successfully earlier.

And since the spec allows for a query parameter to the Redirect URI that must be retained (https://tools.ietf.org/html/rfc6749#section-3.1.2), any spec-compliant Provider should now work with the new mitigation implementation.

FWIW: binary packages are available from http://mod-auth-openidc.org/download/?C=M;O=D; you'll need 2.3.2rc6.

@remi-cc
Copy link
Author

remi-cc commented Aug 29, 2017

I installed binary packages and set up "issuer_specific_redirect_uri" in the OP.conf file.
But my proof of concept use WSO2 IS as OP (product not certified by OpenID foundation).
It seems that i can register redirect_uri with query parameter but it doesn't accept url encoded character, like "%3A"...

Anyway, when i intercept myself the authorization request, mod_auth_openidc adds in the redirect_uri parameter the issuer as URL encoded query parameter.

Like you said, OP spec-compliant should accept it, which it's not the case of WSO2 IS product.
I will give you results if i test it with an another OP.

Thanks for your great work.

@prabath
Copy link

prabath commented Aug 29, 2017

Hi,

I am an architect from the WSO2 Identity Server product. Interesting discussion!

WSO2 IS respects the query parameters in the redirect_uri sent by the client - and once you configure it in WSO2 IS with the same. The issue here seems to be from mod_auth_openidc - it does URL encode the redirect_uri, before placing it in the request. Ideally if you just place the url as it is (and expect the browser to do the URL encoding this will work fine).

So the issue here is in WSO2 IS when you define the redirect_uri, you cannot define it as an URL encode format - because of the characters like % - and it's not allowed.

So, I do not think this is a OP spec-compliant issue... Anyway thanks for bringing this up...

Thanks & regards,
-Prabath

@zandbelt
Copy link
Member

zandbelt commented Aug 29, 2017

Thanks @prabath ; the point is that the query parameter - which is a URL in itself - needs to be preserved as part of the callback to the RP, not as part of the authorization request to the authorization endpoint of the OP. Hence url-encoding is required for this parameter just like it is for the others, e.g. to preserve spaces in the scope. The server should (and will) url-decode before processing.

Note that even when including the iss value in the path of the redirect URI one would run into the same issue.

@prabath
Copy link

prabath commented Aug 29, 2017

Hi @zandbelt,

This is a quick test I did with WSO2 IS..

When you send the following authorization request (where the iss value is not URL encoded, but browser will do that) - and IS will completely URL-decode it.

https://localhost:9443/oauth2/authorize?client_id=jjIO3DoNbIbq5TdLcdHLkUnr5xka&redirect_uri=http://127.0.0.1:5000?iss=http://test.com&response_type=code

This will return to the following URL (preserving the query parameter)..

http://127.0.0.1:5000/?iss=http://test.com&code=d64afedb-2c9d-3f5b-ac32-890634b30f57

Now, RP will completely URL decode the request from the browser.

I think the issue is, if you send the initial request from the mod_auth_openidc , then it places the URL as the following, where part of it is URL encoded by the module and expects the rest to be done by the browser..

https://localhost:9443/oauth2/authorize?client_id=jjIO3DoNbIbq5TdLcdHLkUnr5xka&redirect_uri=http://127.0.0.1:5000?iss=http%3A%2F%2Ftest.com&response_type=code

Also from WSO2 side, we will consider allowing % character when defining the redirect_uri at the OP side in our next release (since its a valid character) - so you can define the redirect_uri there as http%3A%2F%2Ftest.com instead of http://test.com.

Thanks & regards,
-Prabath

@zandbelt
Copy link
Member

I'm not sure which version you used but mod_auth_openidc always url-encodes the value of the redirect_uri parameter, as it should. Now when then redirect URI contains a query parameter in itself, that will be doubly URL-encoded, as it should.

@remi-cc
Copy link
Author

remi-cc commented Aug 29, 2017

In my point of view (but i'm not a web expert), the doubly URL-encoded for iss query parameter in the redirect_uri seems to be the right thing to do.
@prabath : i understand that issue in WSO2 (% character not allowed) will be fixed in the next release. So there is no need to add a bug issue, right?

@prabath
Copy link

prabath commented Aug 29, 2017

@remi-cc yes - I think the issue is the restriction we enforce on while defining the redirect_uri at the IS side. Yes - we will fix that for IS 5.4.0.

@zandbelt
Copy link
Member

@remi-cc you may be able to hack around the url-encoding in your own fork for the time being and try to achieve what @prabath describes until 5.4.0 is out but that comes with a warning: it is not really straight-forward since the module automatically encodes all parameters and values so you'd have to make an exception for the redirect URI.

@zandbelt
Copy link
Member

@remi-cc I have validated this with my own OP so I'm closing this. You can re-open if needed for some reason.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants