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

Authorization header leaking to the backend. #671

Closed
tecnobrat opened this issue Jul 11, 2019 · 10 comments
Closed

Authorization header leaking to the backend. #671

tecnobrat opened this issue Jul 11, 2019 · 10 comments

Comments

@tecnobrat
Copy link

tecnobrat commented Jul 11, 2019

We're running into an issue where we have basic auth enabled on a fabio route (fabio doing the auth) ... but then fabio is passing that Authorization header to the backend as well ... and we'd like it to not do that :)

The route in question doesn't have basic auth on it, but other routes for the same domain do. Which means that your browser sends it regardless if fabio requires it for that route path.

For example domain.com requires basic auth, but domain.com/thing does not. However your browser (browsers default to this behaviour since its the same "domain") sends the header to domain.com/thing, which passes it on to the backend.

May be related to #168 which talks about removing arbitrary headers but @leprechau asked me to open a new issue.

@tecnobrat tecnobrat changed the title Authentication header leaking Authentication header leaking to the backend. Jul 11, 2019
@pschultz
Copy link
Member

pschultz commented Jul 15, 2019

I assume you mean the Authorization header, not Authentication.

If I understand correctly you have routes configured somewhat like this:

route add test-service /         http://127.0.0.1:3000/ weight 1 opts "auth=mybasicauth"
route add test-service /thing http://127.0.0.1:3000/ weight 1

I don't think fabio should remove the Authorization header for either of these routes.

It would be totally unreasonable to do it for /thing. We can't just blindly assume the backend doesn't need a particular header and remove it, especially the Authorization header. Backends that handle authorization themselves are the norm, not an exception.

Removing the Authorization header for the / route is also not normal behaviour for a proxy. nginx doesn't do this, haproxy doesn't do this, and, if memory serves, Apache doesn't do this either. For basic auth in particular the backend may very well look at which user authenticated.

I understand the desire to hide some headers from backends, but this is not a separate issue from #168; the Authorization header isn't special.

@tecnobrat
Copy link
Author

@pschultz correct on all counts. That was my opinion too. @leprechau thought it may be a separate issue, and asked me to open it as such.

@aaronhurt
Copy link
Member

The one difference here that I think may warrant special handling is that in the first case Fabio itself is sending the WWW-Authenticate header challenge that causes the browser to send the Authorization header. If the backend is/was expecting the Authorization header why would Fabio be handling/prompting the authentication for the route?

To the difference between / and /thing/ above Fabio could possibly keep some state on what Host headers it has seen match the auth tagged route and then optionally filter the browser Authorization header from transiting to the backend.

Is this completely separate from #168, no absolutely not. Should the functions that may be created to deal with #168 be used to remove Authorization headers for routes where Fabio itself handles authentication? Maybe? I think it's worth of a conversation which is why I thought it may need to be tracked in a separate issue.

@tecnobrat tecnobrat changed the title Authentication header leaking to the backend. Authorization header leaking to the backend. Jul 15, 2019
@pschultz
Copy link
Member

pschultz commented Jul 15, 2019

It's not that simple. / and /thing may not be related at all. Perhaps this example makes it obvious:

route add test-service /ui     http://127.0.0.1:3000/ opts "auth=mybasicauth allow=ip:10.0.0.0/8"
route add test-service /api    http://127.0.0.1:3001/

It's easy to imagine that the backend for /ui is some crappy PHP script that can't (or won't) do its own authentication, and the backend for /api expects typical Bearer tokens.

Some routes sharing a Host header doesn't mean those routes all go to the same application.

Should the functions that may be created to deal with #168 be used to remove Authorization headers for routes where Fabio itself handles authentication?

Yes, absolutely.

@pschultz
Copy link
Member

Fabio could possibly keep some state on what Host headers it has seen match the auth tagged route and then optionally filter the browser Authorization header from transiting to the backend.

I suppose the keyword here is "optionally", but I feel that this would just be an exact subset of whatever #168 requires. Or did you have something in mind that isn't?

@tecnobrat
Copy link
Author

It's easy to imagine that the backend for /ui is some crappy PHP script that can't (or won't) do its own authentication, and the backend for /api expects typical Bearer tokens.

We have this exact use-case.

@tecnobrat
Copy link
Author

For my use-case, if I can could be like:

route add test-service /thing http://127.0.0.1:3000/ weight 1 opts "removeheader=Authorization"

Then my problem is solved, and things like /api would still allow Authorization headers to be passed to the backend.

@aaronhurt
Copy link
Member

aaronhurt commented Jul 15, 2019

@tecnobrat So then this has nothing to do with fabio itself requesting authentication? In that case yes, this is exactly the same as #168 and cab be closed. My original understanding was that you were asking fabio to handle authentication because the backend service(s) weren't capable.

I still think there may be a case for preventing responses to fabio issued challenges from reaching the backend. However, if this is not the issue here and (to my knowledge) hasn't been brought up by anyone else since we added that feature to fabio this issue can be closed.

@tecnobrat
Copy link
Author

tecnobrat commented Jul 15, 2019

@leprechau its both, we have basic auth through fabio since the service cannot.

However, if the service was requesting auth itself, it would be the same issue since the browser is "naive", and once the WWW-Authorization header has been sent to the browser, it will send the authorization header on any request to that domain, regardless if the service requests it or not on that specific path.

I purely need to remove the header from going to the backend on specific requests. In my use-case we have an endpoint called /downloads which goes to an s3 bucket. Since the browser sends the authorization header by default, and we're sending get params to the /download endpoint (for s3 presigned URLs) .. s3 freaks out because it received both a Authorization header and a get param for authorization. I want to stop the Authorization header from being passed to that backend on those requests.

This is not a case of the authorization header going to the backend on requests that we specifically want to make sure basic auth is on... but I see your point there.

@aaronhurt
Copy link
Member

@tecnobrat Thank you, that helps. So this is not an issue caused by fabio prompted authentication specifically but a more general purpose need for header manipulation.

I'll close issue. Thanks again for the detailed description. That's a great use case for pushing #168 up the todo list.

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

No branches or pull requests

3 participants