fabio icon indicating copy to clipboard operation
fabio copied to clipboard

support for adding headers

Open danilobuerger opened this issue 7 years ago • 32 comments

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

danilobuerger avatar Sep 28 '16 08:09 danilobuerger

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

magiconair avatar Sep 29 '16 21:09 magiconair

I would say this should be static only

danilobuerger avatar Sep 30 '16 09:09 danilobuerger

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

jippi avatar Jan 16 '17 14:01 jippi

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 :)

jippi avatar Jan 17 '17 13:01 jippi

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

Cheers

killcity avatar Feb 22 '17 22:02 killcity

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 avatar Mar 28 '17 11:03 magiconair

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

magiconair avatar Mar 28 '17 11:03 magiconair

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

jippi avatar Mar 28 '17 11:03 jippi

@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, ...)

magiconair avatar Mar 28 '17 12:03 magiconair

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

jippi avatar Mar 28 '17 13:03 jippi

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

magiconair avatar Mar 28 '17 13:03 magiconair

@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.

bkmit avatar May 31 '17 17:05 bkmit

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

magiconair avatar May 31 '17 19:05 magiconair

@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.

bkmit avatar Jun 01 '17 10:06 bkmit

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.

mafonso avatar Jul 12 '17 12:07 mafonso

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"

magiconair avatar Jul 13 '17 01:07 magiconair

What about a second tag?

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

tino avatar Jul 13 '17 06:07 tino

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

thijs avatar Oct 27 '17 08:10 thijs

please

sielaq avatar Oct 27 '17 11:10 sielaq

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

samm-git avatar Aug 07 '18 07:08 samm-git

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

danlsgiga avatar Mar 06 '19 20:03 danlsgiga

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

fsuste avatar Mar 21 '19 08:03 fsuste

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.

tecnobrat avatar Jul 11 '19 22:07 tecnobrat

@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?

aaronhurt avatar Jul 11 '19 22:07 aaronhurt

@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.

tecnobrat avatar Jul 11 '19 22:07 tecnobrat

@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.

aaronhurt avatar Jul 11 '19 22:07 aaronhurt

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

  • set request header (replacing all existing values)
  • add request header (preserving existing values)
  • set response header (replacing all existing values)
  • add response header (preserving existing values)

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

  • add HTTP/1.1 fabio to the Via request header value
  • set the X-Request-Start request header value to the current time, removing any existing value if necessary
  • remove any existing X-Powered-By response header field (because the value is empty)
  • add HTTP/1.1 fabio to the Via response header value

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?

pschultz avatar Jul 16 '19 12:07 pschultz

It'd be amazing 😅

scalp42 avatar Jan 14 '20 18:01 scalp42

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?

sbrl avatar Jul 15 '20 23:07 sbrl

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!

descalzi avatar Oct 22 '20 15:10 descalzi