summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorMatt Holt <mholt@users.noreply.github.com>2023-08-02 11:13:52 -0600
committerGitHub <noreply@github.com>2023-08-02 11:13:52 -0600
commitf66493efef4d909fdeb68a2ce8131d58e17333b3 (patch)
tree708db84035c22ba9c676293ad3b965d6b7c8bc4e
parent5c51c1db2ce450a3fa003834097ad010b3844673 (diff)
core: Allow loopback hosts for admin endpoint (fix #5650) (#5664)
-rw-r--r--admin.go25
-rw-r--r--cmd/commandfuncs.go32
2 files changed, 43 insertions, 14 deletions
diff --git a/admin.go b/admin.go
index 4a1d23b..1966556 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 77a4cdf..68b099e 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)
}