Repository: caddyserver/caddy (language: go)

Issue:
go 1.20.6 host validation breaks setting Host to a unix socket address thus `caddy reload` over UDS
For those using caddy admin API over a unix domain socket, `caddy reload` now fails with `Error: sending configuration to instance: performing request: Post "http://%20/load": http: invalid Host header`. See upstream issue https://github.com/golang/go/issues/61431


Discussion / hints:
Oh lovely. This issue looks like a sticky mess. Will keep an eye on it.
I'm not really sure how to work around this yet.

I don't want to set a Host header at all because RFC 2616 Section 14.26, says:

> A client MUST include a Host header field in all HTTP/1.1 request
> messages. If the requested URI does not include an Internet host
> name for the service being requested, then the Host header field MUST
> be given with an empty value.

If we disregard the RFC (sigh), and set a Host header, what should it be?

If we use a DNS name I'm worried it could enable CORS violations or DNS rebinding attacks, which is what we're trying to prevent.

If we use an IP address, it might not work because the use of Unix sockets implies the lack of support for IP.

In the meantime, the closest I've come is to change the hostname from ` ` (a space) to `unix:127.0.0.1` -- basically a bogus hostname but with an invalid port. Unfortunately, `curl` won't accept this, so you wouldn't be able to use `curl` as a client. It works with `caddy reload` but the point is to enable other HTTP clients to use the API too.

Sigh, I don't know.
What does `empty value` mean? ` ` (a space) certainly does not sound as empty as `` (well, empty).
@NickCao Though I have not checked the code history (thus I could be wrong), I believe the URL host component of ` ` was enough to pass Go's "is not empty" check, and then I presume spaces were trimmed at some point to make it empty afterward. In any case, it used to work. Now we need to figure out a Host value when there is no Host...
Infosec people are telling me I don't need to worry about DNS or CORS in Unix socket servers. So maybe I will just disable those checks for Unix sockets...
Eh, there's no great solution here. If we don't check the Host header it could be anything, which seems stupid, but if we do check it we have to rely on either DNS or IP, which is also stupid.

However, as I'm coding up this patch I'm thinking that maybe we should at least require a host of `127.0.0.1` or `::1` -- this may appear to require IP, which I know is not guaranteed in Unix socket contexts -- but if IP is truly irrelevant to Unix sockets, then this shouldn't matter. And if, for some reason, in some part of a system, it IS used, at least it will resolve to the loopback interface. Even `localhost` has the potential of being resolved to non-loopback (which is dumb, but it's reality; just edit your `/etc/hosts` file accordingly). So at least by requiring `127.0.0.1` or `::1`, we don't lean on DNS to be trustworthy, and we don't necessarily _require_ IP, but if IP is used we should be able to trust it was at least the local machine. We can still enforce CORS, then, in case browsers decide to do something careless in the future.

Fix patch:
diff --git a/admin.go b/admin.go
index 4a1d23b60ec..1966556a5b5 100644
--- a/admin.go
+++ b/admin.go
@@ -318,7 +318,32 @@ func (admin AdminConfig) allowedOrigins(addr NetworkAddress) []*url.URL {
 				// messages. If the requested URI does not include an Internet host
 				// name for the service being requested, then the Host header field MUST
 				// be given with an empty value."
+				//
+				// UPDATE July 2023: Go broke this by patching a minor security bug in 1.20.6.
+				// Understandable, but frustrating. See:
+				// https://github.com/golang/go/issues/60374
+				// See also the discussion here:
+				// https://github.com/golang/go/issues/61431
+				//
+				// We can no longer conform to RFC 2616 Section 14.26 from either Go or curl
+				// in purity. (Curl allowed no host between 7.40 and 7.50, but now requires a
+				// bogus host; see https://superuser.com/a/925610.) If we disable Host/Origin
+				// security checks, the infosec community assures me that it is secure to do
+				// so, because:
+				// 1) Browsers do not allow access to unix sockets
+				// 2) DNS is irrelevant to unix sockets
+				//
+				// I am not quite ready to trust either of those external factors, so instead
+				// of disabling Host/Origin checks, we now allow specific Host values when
+				// accessing the admin endpoint over unix sockets. I definitely don't trust
+				// DNS (e.g. I don't trust 'localhost' to always resolve to the local host),
+				// and IP shouldn't even be used, but if it is for some reason, I think we can
+				// at least be reasonably assured that 127.0.0.1 and ::1 route to the local
+				// machine, meaning that a hypothetical browser origin would have to be on the
+				// local machine as well.
 				uniqueOrigins[""] = struct{}{}
+				uniqueOrigins["127.0.0.1"] = struct{}{}
+				uniqueOrigins["::1"] = struct{}{}
 			} else {
 				uniqueOrigins[net.JoinHostPort("localhost", addr.port())] = struct{}{}
 				uniqueOrigins[net.JoinHostPort("::1", addr.port())] = struct{}{}
diff --git a/cmd/commandfuncs.go b/cmd/commandfuncs.go
index 77a4cdf4a80..68b099ef727 100644
--- a/cmd/commandfuncs.go
+++ b/cmd/commandfuncs.go
@@ -610,7 +610,7 @@ func AdminAPIRequest(adminAddr, method, uri string, headers http.Header, body io
 	}
 	origin := "http://" + parsedAddr.JoinHostPort(0)
 	if parsedAddr.IsUnixNetwork() {
-		origin = "http://unixsocket" // hack so that http.NewRequest() is happy
+		origin = "http://127.0.0.1" // bogus host is a hack so that http.NewRequest() is happy
 	}
 
 	// form the request
@@ -619,20 +619,24 @@ func AdminAPIRequest(adminAddr, method, uri string, headers http.Header, body io
 		return nil, fmt.Errorf("making request: %v", err)
 	}
 	if parsedAddr.IsUnixNetwork() {
-		// When listening on a unix socket, the admin endpoint doesn't
-		// accept any Host header because there is no host:port for
-		// a unix socket's address. The server's host check is fairly
-		// strict for security reasons, so we don't allow just any
-		// Host header. For unix sockets, the Host header must be
-		// empty. Unfortunately, Go makes it impossible to make HTTP
-		// requests with an empty Host header... except with this one
-		// weird trick. (Hopefully they don't fix it. It's already
-		// hard enough to use HTTP over unix sockets.)
+		// We used to conform to RFC 2616 Section 14.26 which requires
+		// an empty host header when there is no host, as is the case
+		// with unix sockets. However, Go required a Host value so we
+		// used a hack of a space character as the host (it would see
+		// the Host was non-empty, then trim the space later). As of
+		// Go 1.20.6 (July 2023), this hack no longer works. See:
+		// https://github.com/golang/go/issues/60374
+		// See also the discussion here:
+		// https://github.com/golang/go/issues/61431
 		//
-		// An equivalent curl command would be something like:
-		// $ curl --unix-socket caddy.sock http:/:$REQUEST_URI
-		req.URL.Host = " "
-		req.Host = ""
+		// After that, we now require a Host value of either 127.0.0.1
+		// or ::1 if one is set. Above I choose to use 127.0.0.1. Even
+		// though the value should be completely irrelevant (it could be
+		// "srldkjfsd"), if for some reason the Host *is* used, at least
+		// we can have some reasonable assurance it will stay on the local
+		// machine and that browsers, if they ever allow access to unix
+		// sockets, can still enforce CORS, ensuring it is still coming
+		// from the local machine.
 	} else {
 		req.Header.Set("Origin", origin)
 	}


Test patch: