Caddy: Proxy health check does not respect upstream headers

[francislavoie]

This is a followup issue to https://github.com/mholt/caddy/issues/1556

My upstream server expects headers from the transparent directive to be passed through for the health check to work. My upstream server is a caddy server listening on a specific hostname (i.e. not 0.0.0.0 but instead for the domain name).

I'm trying to write the fix, essentially we just need to mutate the headers similarly to https://github.com/mholt/caddy/blob/7d15435361c95aa029be244f962b47950487a539/caddyhttp/proxy/proxy.go#L185-L191, but the problem I see is that we don't have a request as a base for the replacer when doing a health check, since it's an out-of-band check.

We need a request to pass into this:

replacer := httpserver.NewReplacer(req, nil, "")

Any ideas?

Side question, since I'm quite new to Go, it feels gross to be copying the mutateHeadersByRules func from proxy.go into upstream.go for this, what would be the ideal way to re-use that code in this case?

[JuliCodes]

I think the most used case is the redirection of the host name. So maybe we could read the domain from the Caddy configuration and pass this to the upstream?

[francislavoie]

Sounds good. I wouldn't mind getting pushed in the right direction - how should I get the current hostname? I'm not sure where to look in the codebase.

This is what I have so far, at line 375-ish in upstream.go:

        // set up request, needed to be able to modify headers
        req, _ := http.NewRequest("GET", hostURL, nil)

        // set headers for request going upstream
        if u.upstreamHeaders != nil {
            hostHeader := u.upstreamHeaders.Get("Host")
            if strings.Contains(hostHeader, "{host}") {
                replacement := strings.Replace(hostHeader, "{host}", <header here>)
                req.Header.Set("Host", replacement)
            }
        }

        if r, err := u.HealthCheck.Client.Do(req); err == nil {

[francislavoie]

Alright I think I have an idea:

In proxy/setup.go, line 17, I'll pass the host from the config:

upstreams, err := NewStaticUpstreams(c.Dispenser, httpserver.GetConfig(c).Host())

Changing the func signature in proxy/upstream.go:

func NewStaticUpstreams(c caddyfile.Dispenser, host string) ([]Upstream, error) {

Then add the host to the staticUpstream struct...

I don't like this though, cause I have to change the function signature which means all the tests need to change. Anyone got a better idea? :disappointed:

[francislavoie]

Alright I'm just confused now. I have it implemented (with the noted changes in the previous comment), but it still doesn't seem to work. Pretty odd.

caddy_1  | Request: &{GET https://10.0.1.5/ HTTP/1.1 1 1 map[Host:[<snip>]] <nil> <nil> 0 [] false 10.0.1.5 map[] map[] <nil> map[]   <nil> <nil> <nil> <nil>}
caddy_1  | Upstream status: &{404 Not Found 404 HTTP/1.1 1 1 map[Date:[Fri, 14 Apr 2017 02:34:51 GMT] Content-Length:[20] Content-Type:[text/plain; charset=utf-8] Server:[Caddy] X-Content-Type-Options:[nosniff]] 0xc42024bda0 20 [] false false map[] 0xc420197800 0xc420085a20}

So, I can see the headers in the request before it gets sent out, but the behaviour I see is the same as if I do a wget without setting the header:

$ wget https://10.0.1.5/ --no-check-certificate
--2017-04-14 02:33:58--  https://10.0.1.5/
Connecting to 10.0.1.5:443... connected.
WARNING: cannot verify 10.0.1.5's certificate, issued by ‘O=Caddy Self-Signed’:
  Unable to locally verify the issuer's authority.
    WARNING: certificate common name ‘’ doesn't match requested host name ‘10.0.1.5’.
HTTP request sent, awaiting response... 404 Not Found
2017-04-14 02:33:58 ERROR 404: Not Found.

$ wget https://10.0.1.5/ --no-check-certificate --header="Host: <snip>"
--2017-04-14 02:34:30--  https://10.0.1.5/
Connecting to 10.0.1.5:443... connected.
WARNING: cannot verify 10.0.1.5's certificate, issued by ‘O=Caddy Self-Signed’:
  Unable to locally verify the issuer's authority.
    WARNING: certificate common name ‘’ doesn't match requested host name ‘10.0.1.5’.
HTTP request sent, awaiting response... 200 OK
Length: 1031 (1.0K) [text/html]
Saving to: ‘index.html’

index.html                               100%[================================================================================>]   1.01K  --.-KB/s    in 0s      

2017-04-14 02:34:30 (26.8 MB/s) - ‘index.html’ saved [1031/1031]

My changes so far, here: https://github.com/mholt/caddy/compare/master...francislavoie:fix-%231574

[mholt]

@francislavoie You've made great progress so far! Sorry I haven't responded lately, been juggling a lot of things this week but I think school is giving me the calm before the storm now before finals.

🤔 Does perhaps the Host field of the http.Request need to be set to the right host too? The Go docs say:

  // For client requests Host optionally overrides the Host
  // header to send. If empty, the Request.Write method uses
  // the value of URL.Host. Host may contain an international
  // domain name.

So, I suspect setting one or the other will work, but not both. In fact, leaving both blank may still work if the URL is formed using the hostname rather than the IP.

[francislavoie]

@mholt oh man, it worked! That was it! Whoo!

So, how would you suggest that I write a test for this? Not sure how that would work. Also, are you OK with that signature change to NewStaticUpstreams and adding Host to the HealthCheck struct? Trying to tick all the boxes before I make that PR. Or should I make the PR first so you can review?

Thanks :smile:

Good luck on your finals and the event next Thursday! I'm excited to see what's next. I'm a huge fan of this project.

[mholt]

Nice work!

Make a PR and then we'll review. Doing code reviews in issues is tedious. Expect several revisions, that's normal. We'll help you get it to a mergeable state. (Yes, I'm behind on PRs, but we have great collaborators who will help if they're available.)

[francislavoie]

Closing this now that I made the PR :smile: