Caddy: Add ability to overwrite headers

[stp-ip]

As prerequisite we have the header content-security-policy: policy

Right now one can either strip a header:
header / -content-security-policy
Resulting in no content-security-policy header

Or add something to a header:
header / content-security-policy "some policy"
Resulting in content-security-policy: policy some policy

But one can not overwrite a header:
header / content-security-policy "some policy"
Resulting in content-security-policy: some policy

My suggestion would be to have -content-security-policy strip the header, +content-security-policy add to a header already available and content-security-policy to overwrite it. This would break compatibility, but in my opinion makes it clearer.

[mholt]

Resulting in content-security-policy: policy some policy

Silly question, but are you sure? I've never heard of header fields being concatenated like that.

Also, I lied in Slack, since I was going on memory. The header directive uses w.Header().Set(header.Name, header.Value) which overwrites any previous value; it doesn't use Add() to make a potentially duplicate header field.

[stp-ip]

I thought I was quite sure, but will check this again with a clean setup and in more detail tomorrow.
Will report back.

[stp-ip]

I just triple checked:

My caddyfile:

https://testing.example.com {
        proxy / localhost:8080 {
                proxy_header Host {host}
                proxy_header X-Real-IP {remote}
                proxy_header X-Forwarded-Proto {scheme}
        }
        header / -Cache-Control 
        header / X-header-usage "true"
        header / X-Robots-Tag "redacted"

        tls [email protected]
}

Headers before reloading with the headers added:

Cache-Control: max-age=7200, public
Server: Caddy, Apache
X-Robots-Tag: none

Headers with new headers added to caddy:

Cache-Control: max-age=7200, public
Server: Caddy, Apache
X-Robots-Tag: redacted, none
X-header-usage: true

header / -Cache-Control <-- header is still there
header / X-header-usage "true" <-- header was successfully added
header / X-Robots-Tag "redacted" <-- was added at the beginning of the header

[mjeri]

I had a look together with @DirtyHairy into the issue and the related code, the proxy and headers middlewares. Based on that knowledge I can not understand the case you describe. I also did setup a sample scenario and can not reproduce the behaviour but experience a quite different one. Which version of caddy do you use? I tried the stuff with the latest master version.

In general I think there is currently no way to modify headers coming from a proxy response. The headers middleware is executed before the proxy middleware and does not act in the "response" chain. So additional headers configured are added twice, the one you configure to remove are not removed from the proxy response.

Even though I find this would be a very cool feature and I could imagine to implement it, if there is further need and acceptance of that idea by the caddy maintainers. Also I agree with the semantics of your specification, though here package maintainers maybe can give a hint how we should work with the backwards compatibility breaks. Though I am not sure what to do, e.g. when you want to remove a header, when this header is existent twice, which is fine by the http specification imho.

For documentation the setup I used to reproduce, at least to try :D, is the following:

Static Website

http://localhost:9092 {
  root /home/maja/src/caddy-website/static

  header / Cache-Control "max-age=7200, public"
  header / X-Robots-Tag "none"
}

Proxy Server

http://localhost:9090 {

  proxy / localhost:9092 {
    proxy_header Host {host}
    proxy_header X-Real-IP {remote}
    proxy_header X-Forwarded-Proto {scheme}
  }

  header / -Cache-Control
  header / X-Header-Usage "true"
  header / X-Robots-Tag "redacted"
}

Headers I get when requesting http://localhost:9090/Sample.txt:

Cache-Control:max-age=7200, public
Server:Caddy
Server:Caddy
X-Header-Usage:true
X-Robots-Tag:redacted
X-Robots-Tag:none

[stp-ip]

Thanks for testing. I think it validates what I saw.
Headers are not removed from a proxied source.
Headers are not overwritten from a proxied source.
Headers are duplicated or added. (depends on the client, if it is seen as a duplicate header or the headers get combined)

I think this is in my opinion a case of "now working as one would expect".

[mholt]

So are we good here? Feel free to close the issue if so, or I will. :smile: Or, if you could clarify what change needs to happen, that would be helpful.

[stp-ip]

I still think this is unintuative and basically a bug.

For me what needs to happen to see it as resolved would be:

• setting a header should overwrite the header both in proxy and non proxy situations
• removing a header should remove the header both in proxy and non proxy situations
• (optional - feature) adding a method to add, but not overwrite a header as suggested as +content-security-policy

Hope that makes it clearer.

[DirtyHairy]

I'll take the liberty to chime in :) While I don't have any stakes here, I am the other guy who analyzed the code together with @marco-jantke. Some observations:

1. If the upstream request succeeds, the proxy middleware will return immediatelly without executing next, effectively terminating the middleware flow and going back up the stack.
2. From the position of the header middleware in the stack, it is clear that there is currently no way how header could mutate or remove headers set by any middleware below, including proxy. The headers are set before the request is propagated down the stack, and no processing is done when the request moves back up.
3. The header middleware uses Set to apply headers, so it indeed does overwrite, but proxy uses Add to copy the headers from the upsteam response and thus is responsible for the doubling.

(1) and (3) look pretty counterintuitive to me; @mholt : are these bugs or a features :smirk:? As for (2), it would seem more intuitive to me if the header middleware did it's thing when the request goes back up the stack (after calling next), but this is a major change and I am not sure about the potential for breakage. I think this would also achieve @stp-ip's goal:

• Add or overwrite a header: x-some-header: "some value"
• Append to a header: x-some-header: "{>x-some-header}, some other value"
• Delete a header: -x-some-header

I fail to see the usecase for setting a single header multiple times; in addition, the presence of multiple declarations of a single header make the interpretation of the {>x-some-header} placeholder a bit murky. Still, if desired, this could be achieved with a +x-some-header: "some value" syntax extension.

[mholt]

@DirtyHairy: 1) is intended behavior. You may be right about 2), about how header needs to do its processing after the call to Next.ServeHTTP() rather than before. And 3) is just a decision I made early on, figuring it should replace rather than add.

Duplicate header fields _are_ apparently allowed as long as they can be combined into a single valid field with each value being separated by a comma.

We should probably define a consistent way to add, remove, and replace header fields in Caddy: both with the header directive and also the proxy directive related to headers upstream. I like where the + syntax was going for extending a header.

I have to attend to some major restructuring right now, so I'm inviting others to make a clear proposal about how this should be implemented/corrected so that it works well with upstreams and also gives the user the needed control over headers without becoming too complex. Again, I'm okay with the - and + syntax in front of the header name as needed; and even using placeholders is fine... let me know what you'd suggest and somebody make it happen! :smile:

[p4tin]

If I get this right the consensus from the above discussions was these test cases for headers (proxied or not)

1. header name value (caddyFile config - name only)
   
   
   
   • if exists replace
   
   
   • else create
   
   
2. header -name (caddyFile config - '-' before the name)
   
   
   
   • remove header always
   
   
3. header +name value (caddyFile config - '+' before the name)
   
   
   
   • if exists add value to current header with a comma in between values
   
   
   • else create with value
   
   

If we agree on the test cases then I will try to code a solution.

Updated for inside proxy only!!!

[mholt]

I like it, but maybe we should hear back from @stp-ip to see if that's agreeable with what he was asking about too.

Remember, this will be inside the proxy directive only, so we'll have syntax like this: proxy_header Name Value instead of header / Name Value.

[stp-ip]

Thanks for keeping me involved. Yes I agree with @p4tin. The general idea is great. The only thing to take into account is the sequence of caddy and middleware plugins/modules/add-ons. Headers set in caddy itself need to be applied last. Middleware can only remove, add or replace headers, but only before caddy's headers are set. So If a header is removed/set/or added in caddy it needs to overwrite the middleware.

[p4tin]

ok I think i got it let me see what I can do. Thanks for the clarifications!

[stp-ip]

Thanks for jumping in and contributing. \o/

[mholt]

@p4tin How is this going? Any way we can help?

[p4tin]

@mholt - Sorry have not had time to look at it yet will pick it up tomorrow morning and I am sure I will have question - I will be on Gitter.

[Burmudar]

Hi guys,

I've started looking into this (hope you don't mind @p4tin). I just need some slight clarification on where this header modification should happen.

1. Should headers for the request sent to the upstream backend be modified (Add, Replaced, Removed) ?
2. Should the headers of the response received from the upstream backend be modified (Add, Replaced, Removed)?

[mholt]

@Burmudar Sorry I didn't get back to this sooner:

1. I think the proxy directive is the place to handle headers for the _request_ sent upstream.
2. I believe, but haven't verified, that the regular header directive should work for managing headers of the _response_ received from upstream. If so, then it should be modified to support adding, replacing, and removing headers in the same way the proxy directive can with your PR.

[Burmudar]

@mholt No problem! I appreciate the feedback in any case.

I'll get right on investigating the header directive and see whether it can be used. I hope to get to this before the weekend.

[Burmudar]

@mholt I've looked into this and found the following.

The header directive does not get the response from the upstream. It adds the headers to the response before the proxy directive starts.

A possible solution is letting the header directive be last in the chain. I still have to check what the ramifications are of doing that.

[mholt]

@Burmudar I think we can re-arrange the logic a bit in the header directive. Right now it processes headers before passing the request to the next middleware in the chain (Next.ServeHTTP). Perhaps we should process headers after calling the Next handler, at which point the response headers will already be set. We shouldn't need to move it down to the end of the chain at all.

[Burmudar]

@mholt I moved the processing as you suggested. Modifying the headers after the Next.ServeHTTP changes the ResponseWriters Header() but has no effect on the headers sent with the response.

As soon as WriteHeader or Write (which calls WriteHeader implicitly) is called the headers are effectively locked(You can still add Trailer headers as per the golang docs but have to declare them before hand). You can see this behaviour by making the header directive WriteHeader(http.StatusOK) as its first instruction.

The golang net/http docs also recommend that the ResponseWriter should not be used after ServeHTTP :disappointed:

[mholt]

Ah, of course, that's because the header is already written because on the way down, the response body has already been written (presumably).

[mholt]

Maybe we should try moving header to the end of the chain as you suggested. Do you want to try it?

[Burmudar]

Knowing what I know now, I don't think that would work anymore. Specifically in the case with the proxy directive the reverse proxy immediately writes the upstream response to the current response :cry:

To accomplish the goal here, from my limited experience, I think right when the reverse proxy receives the upstream response the headers should be modified. Which brings us back to my earlier suggestion of something like proxy_resp_header ?

[mholt]

Alright, how about header_upstream and header_downstream?

[Burmudar]

Sounds good :smile:

Just to make 100% sure those are specifically to the proxy directive so proxy_header_upstream and proxy_header_downstream

[mholt]

I thought about those, but it's kind of long, and can only be used inside proxy anyway.

[Burmudar]

Yeah good point. I'll start making the required changes as soon as possible. Thanks for your input!

[virtualdxs]

I'm not able to overwrite headers set by PHP.

    header / content-security-policy "frame-ancestors 'self' k7dxs.net" # Has no effect

    header / content-security-policy2 "frame-ancestors 'self' k7dxs.net" # Adds this other header

    header / -content-security-policy
    header / content-security-policy "frame-ancestors 'self' k7dxs.net" # Results in NO version of this header

Is this by design? Am I doing something wrong?

[francislavoie]

@virtualdxs you'll get a quicker answer by asking here: https://caddy.community/

[tobya]

@virtualdxs This is an outstanding issue. #1829