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

Consider disabling all the OIDC endpoints by default to reduce the attack surface #312

Closed
PinpointTownes opened this Issue Jul 9, 2016 · 2 comments

Comments

2 participants
@PinpointTownes
Member

PinpointTownes commented Jul 9, 2016

Unlike OAuthAuthorizationServerMiddleware, ASOS' endpoints are all enabled by default.

To reduce the attack surface of ASOS deployments, I'm considering removing all the default endpoint paths in beta7 (except the configuration and cryptography endpoints, that are required for OIDC discovery to work properly).

I'd love to hear your thoughts about this change.

@PinpointTownes PinpointTownes added this to the 1.0.0-beta7 milestone Jul 9, 2016

@PinpointTownes PinpointTownes self-assigned this Jul 9, 2016

@PinpointTownes PinpointTownes changed the title from Consider disabling all the OIDC endpoints by default to reduce the surface attack to Consider disabling all the OIDC endpoints by default to reduce the attack surface Jul 9, 2016

@MartinMason

This comment has been minimized.

Show comment
Hide comment
@MartinMason

MartinMason Jul 9, 2016

I'm all for it. In my constant head banging trying to get Aurelia and OpenIddict to work together, ended up coming to the point where the only endpoints needed was the authorization endpoint and userinfo endpoint. If resource owner password credentials grant was the only flow needed, it would have been only the token endpoint and userinfo endpoints. Are you thinking that to enable endpoints, you'd explicitly need to call .EnableAuthorizationEndpoint("~/connect/authorize") in the ConfigureServices method of StartUp. Only reservation would be the number of questions you'd get on the gitter chat.

MartinMason commented Jul 9, 2016

I'm all for it. In my constant head banging trying to get Aurelia and OpenIddict to work together, ended up coming to the point where the only endpoints needed was the authorization endpoint and userinfo endpoint. If resource owner password credentials grant was the only flow needed, it would have been only the token endpoint and userinfo endpoints. Are you thinking that to enable endpoints, you'd explicitly need to call .EnableAuthorizationEndpoint("~/connect/authorize") in the ConfigureServices method of StartUp. Only reservation would be the number of questions you'd get on the gitter chat.

@PinpointTownes

This comment has been minimized.

Show comment
Hide comment
@PinpointTownes

PinpointTownes Jul 9, 2016

Member

Only reservation would be the number of questions you'd get on the gitter chat.

Very good point 👍

My OpenIddict PR includes a few checks to ensure that at least one flow is registered (which should warn existing users about this new requirement) and that the final configuration is coherent.

image

So if you allow a flow to be used without enabling the corresponding endpoints, an exception is thrown during startup (e.g for the password flow, the token endpoint must be enabled ; for the authorization code flow, both the authorization and the token endpoints must be enabled).

image

image

I hope it will be clear enough.

Member

PinpointTownes commented Jul 9, 2016

Only reservation would be the number of questions you'd get on the gitter chat.

Very good point 👍

My OpenIddict PR includes a few checks to ensure that at least one flow is registered (which should warn existing users about this new requirement) and that the final configuration is coherent.

image

So if you allow a flow to be used without enabling the corresponding endpoints, an exception is thrown during startup (e.g for the password flow, the token endpoint must be enabled ; for the authorization code flow, both the authorization and the token endpoints must be enabled).

image

image

I hope it will be clear enough.

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