fabiolb / fabio

Consul Load-Balancing made simple
https://fabiolb.net
MIT License
7.26k stars 616 forks source link

support for adding headers #168

Open danilobuerger opened 8 years ago

danilobuerger commented 8 years ago

Kind of like #110 but for arbitrary headers like HSTS or HPKP, etc.

magiconair commented 8 years ago

Do these headers have static or dynamic values, i.e. does the header value depend on the request?

danilobuerger commented 8 years ago

I would say this should be static only

jippi commented 7 years ago

:+1: on this, for my case static headers would also be sufficient :)

jippi commented 7 years ago

Well, static was a lie...

My usecase is proxy_set_header X-Request-Start "t=${msec}000"; from nginx. which is needed by https://docs.newrelic.com/docs/apm/applications-menu/features/configuring-request-queue-reporting to monitor routing latency :)

killcity commented 7 years ago

I am also wondering if this will eventually be supported. Static would be fine on my end.

Cheers

magiconair commented 7 years ago

If the headers need to be dynamic I might be able to re-use the formatting language I'm using for the access logger from #80. In this case I would need to wrap the values in {} or something similar to support the ${msec}000 use case. I'll think about that.

magiconair commented 7 years ago

I guess they should be configurable per route as well so this needs to become part of the urlprefix- tag options.

jippi commented 7 years ago

where is this ticket in the OSS pipeline @magiconair ? :)

magiconair commented 7 years ago

@jippi there is no pipeline. Generally, I pick up things that people are asking for and for which I have an idea on how to implement it. Adding a static option to add a set of headers to all requests is a no-brainer. But is that really enough? I think I have an idea on how to approach this but for this I need to understand what you need and I'd like to ask you to think a bit outside the box of your specific use-case, if possible.

I think it is reasonable to expect that people would want to modify headers per route. The main challenge with this approach is that if different services announce different options for the same route then the behavior is undefined. As long as that is documented I think I can live with that. This problem will remain for all other features that modify a route. fabio could detect these things and log them to make detection easier.

Also, I'm thinking that there are three operations: set, add and del for setting, adding and deleting headers, e.g.

urlprefix-/foo header_set="X-Request-Start=${time_us}" header_add="Strict-Transport-Security: max-age=60; includeSubDomains" header_del=X-Client-IP

Does that look like a reasonable approach?

This requires to first update the k/v parser to accept quoted values. Then I'd have to figure out which values would be needed and whether the access logging code could be re-used for that. Then I need to actually modify the headers.

Question: Should the route header modification happen before or after the normal header mangling (e.g. X-Forward-Proto, ...)

jippi commented 7 years ago

the use-cases i've had multiple times is only allowing pure forwarding of headers - by whitelisting - not injecting anything new :)

magiconair commented 7 years ago

@jippi like what? fabio doesn't remove headers AFAIK, or does it?

bkmit commented 7 years ago

@magiconair: We also need this facility for modifying response headers. And using logging formatting facility will allow send dynamic headers. IMHO: Headers modification need to be happen after request processed by upstream before sending response to client.

magiconair commented 7 years ago

@bkmit Can you provide a concrete example of what it is you're trying to do?

bkmit commented 7 years ago

@magiconair We use nginx (and we want to get rid) for adding Access-Control-*, Public-Key-Pins headers with different values depending on route. Also I want to add X-Request-Id generated by fabio and maybe others to response for debug/investigation purpose.

mafonso commented 7 years ago

I'm looking at fabio to replace a consul aware openresty setup. I would love to have the ability to inject headers (static at least) in order to be able to replace my custom gateway. Similar use cases as described above. CORS handling included.

magiconair commented 7 years ago

I need to think a bit on how to express that since I think I've moved myself somewhat into a corner with the config language. For example, should the header definition be part of the opts (which leads to quoting issues) or separate?

route add svc /foo 1.2.3.4 opts "foo=bar baz=bang hdr='x-foo: bar' hdr='del x-foo' hdr='add x-foo: baz'"
vs
route add svc /foo 1.2.3.4 opts "foo=bar baz=bang" hdr="x-foo: bar" hdr="del x-foo" hdr="add x-foo: baz"

How should I translate this from the urlprefix- tag which assumes every key=val pair after the path is part of opts?

urlprefix-/foo foo=bar baz=bang hdr="x-foo: bar" hdr="del x-foo" hdr="add x-foo: baz"
tino commented 7 years ago

What about a second tag?

headers-/foo hdr="x-foo: bar" hdr="del x-foo" hdr="add x-foo: baz"

thijs commented 6 years ago

I like the idea of add and delete (set could just be delete and add, right?), but currently my main issue is adding a static header to the request that is passed on

sielaq commented 6 years ago

please

samm-git commented 6 years ago

Adding my use-case, as suggested in https://github.com/fabiolb/fabio/issues/528

I would like to remove some of the custom headers added by my load balancer to not expose their values to the registered services. In my configuration it would be enough to set value of this headers globally, but probably per-route configuration could be even better

danlsgiga commented 5 years ago

I'd like to be able to remove response headers as well like "Server" or "X-Powered-By" added by some servers.

fsuste commented 5 years ago

I'd like to add custom headers such as CSP headers. Those can be static headers or maybe even pulled from Consul.

tecnobrat commented 5 years ago

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 Authentication header to the backend as well ... and we'd like it to not do that :)

So we'd like to strip that before it hits the backend.

aaronhurt commented 5 years ago

@tecnobrat That sounds like a separate issue with the implementation of the basic authorization handler on routes. Fabio doesn't add that header but maybe it should strip it from being passed to the backend if it's handling the authentication. Could you open a separate issue for that?

tecnobrat commented 5 years ago

@leprechau 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 sends the header to domain.com/thing, which passes it on to the backend.

Happy to open a new issue, but I just want to make sure its really a separate thing.

aaronhurt commented 5 years ago

@tecnobrat if the route on which fabio is handling the authentication also sends the header I think that's a separate issue related to leaking Authentication headers past the point where they are authenticated. Routes where fabio does not handle authentication but you still desire header manipulation/stripping might fall here. I could also see that being something that should be handled by fabio though since it's the one sending the WWW-Authenticate header and should not leak the request to the backend since the backend didn't request the authentication. Thoughts? Probably better to discuss this in a separate issue.

pschultz commented 5 years ago

To summarize:

Users want to set and remove both request and response headers. Some use-cases require the values to be dynamic (e.g. X-Request-Start). The route options would be the obvious place to configure this, since that's where other request modifications happen as well (such as stripping a path prefix), and it translates easily to Consul tags.

So we would like to add four options

Removing headers can be covered with the set option and an empty value. That's what nginx does, for instance.

Reusing the syntax for access logs would be nice, but it somewhat collides with the options syntax and makes quoting difficult.

That being said, I don't think I have ever seen a literal single quote appear in an HTTP header. Perhaps it is acceptable to start with the limitation that single quotes are not supported.

route add svc / http://localhost:8000 opts \
    "addreqhdr='Via:HTTP/1.1 fabio' setreqhdr=X-Request-Start:${time_unix_ms} setreshdr=X-Powered-By: addreshdr='Via:HTTP/1.1 fabio'"

This would

parseOpts would become significantly more complex because it will have to deal with single-quoted values. Empty values are not supported and, as mentioned before, neither are literal single quotes in header values.

Is this agreeable?

scalp42 commented 4 years ago

It'd be amazing 😅

sbrl commented 4 years ago

What's the status of this please? I'm just setting up a fabio instance that's backed by Nomad + Consul, and I need to be able to set a custom HTTP header for a given route.

Reading this through, it sounds like there are multiple sets of http header-related features here. Perhaps this could be made more manageable by breaking it down into multiple stages?

descalzi commented 3 years ago

What's the status of this please? I cant use fabio for web gRPC requests as they get blocked by cors allow origin, so being able to add the headers would great!

lemon-li commented 3 years ago

Any progress? Support CORS Header?

ziazon commented 2 years ago

Also, I'm thinking that there are three operations: set, add and del for setting, adding and deleting headers, e.g.

urlprefix-/foo header_set="X-Request-Start=${time_us}" header_add="Strict-Transport-Security: max-age=60; includeSubDomains" header_del=X-Client-IP

Does that look like a reasonable approach?

Yes! My use case:

We deploy services via nomad, so it would be great to be able to add custom headers so that when we inspect the the headers of a request (say one of the instances of a server is not behaving as expected) we can see via the custom headers which instance of that service it's coming from, rather than have to go in and tail all the logs of all the services to see which one is having issues with certain requests as a way to simply identify which node is having issues.