admin: Remove host checking for UDS (close #6832)

The consensus is that host enforcement on unix sockets is ineffective, frustrating, and confusing. (Unix sockets have their own OS-level permissions system.)

admin.go

@@ -221,7 +221,8 @@ func (admin *AdminConfig) newAdminHandler(addr NetworkAddress, remote bool, _ Co
 	if remote {
 		muxWrap.remoteControl = admin.Remote
 	} else {
-		muxWrap.enforceHost = !addr.isWildcardInterface()
+		// see comment in allowedOrigins() as to why we disable the host check for unix/fd networks
+		muxWrap.enforceHost = !addr.isWildcardInterface() && !addr.IsUnixNetwork() && !addr.IsFdNetwork()
 		muxWrap.allowedOrigins = admin.allowedOrigins(addr)
 		muxWrap.enforceOrigin = admin.EnforceOrigin
 	}
@@ -310,9 +311,6 @@ func (admin AdminConfig) allowedOrigins(addr NetworkAddress) []*url.URL {
 	for _, o := range admin.Origins {
 		uniqueOrigins[o] = struct{}{}
 	}
-	if admin.Origins == nil {
-		if addr.isLoopback() {
-			if addr.IsUnixNetwork() || addr.IsFdNetwork() {
 	// RFC 2616, Section 14.26:
 	// "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
@@ -330,27 +328,26 @@ func (admin AdminConfig) allowedOrigins(addr NetworkAddress) []*url.URL {
 	// 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
+	// If either of those two statements ever fail to hold true, it is not the
-				// of disabling Host/Origin checks, we now allow specific Host values when
+	// fault of Caddy.
-				// 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),
+	// Thus, we do not fill out allowed origins and do not enforce Host
-				// and IP shouldn't even be used, but if it is for some reason, I think we can
+	// requirements for unix sockets. Enforcing it leads to confusion and
-				// at least be reasonably assured that 127.0.0.1 and ::1 route to the local
+	// frustration, when UDS have their own permissions from the OS.
-				// machine, meaning that a hypothetical browser origin would have to be on the
+	// Enforcing host requirements here is effectively security theater,
-				// local machine as well.
+	// and a false sense of security.
-				uniqueOrigins[""] = struct{}{}
+	//
-				uniqueOrigins["127.0.0.1"] = struct{}{}
+	// See also the discussion in #6832.
-				uniqueOrigins["::1"] = struct{}{}
+	if admin.Origins == nil && !addr.IsUnixNetwork() && !addr.IsFdNetwork() {
-			} else {
+		if addr.isLoopback() {
 			uniqueOrigins[net.JoinHostPort("localhost", addr.port())] = struct{}{}
 			uniqueOrigins[net.JoinHostPort("::1", addr.port())] = struct{}{}
 			uniqueOrigins[net.JoinHostPort("127.0.0.1", addr.port())] = struct{}{}
-			}
+		} else {
-		}
-		if !addr.IsUnixNetwork() && !addr.IsFdNetwork() {
 			uniqueOrigins[addr.JoinHostPort(0)] = struct{}{}
 		}
 	}

admin_test.go

@@ -531,6 +531,7 @@ func TestAdminRouterProvisioning(t *testing.T) {
 }
 
 func TestAllowedOriginsUnixSocket(t *testing.T) {
+	// see comment in allowedOrigins() as to why we do not fill out allowed origins for UDS
 	tests := []struct {
 		name          string
 		addr          NetworkAddress
@@ -544,11 +545,7 @@ func TestAllowedOriginsUnixSocket(t *testing.T) {
 				Host:    "/tmp/caddy.sock",
 			},
 			origins:       nil, // default origins
-			expectOrigins: []string{
+			expectOrigins: []string{},
-				"", // empty host as per RFC 2616
-				"127.0.0.1",
-				"::1",
-			},
 		},
 		{
 			name: "unix socket with custom origins",
@@ -578,7 +575,7 @@ func TestAllowedOriginsUnixSocket(t *testing.T) {
 		},
 	}
 
-	for _, test := range tests {
+	for i, test := range tests {
 		t.Run(test.name, func(t *testing.T) {
 			admin := AdminConfig{
 				Origins: test.origins,
@@ -592,7 +589,7 @@ func TestAllowedOriginsUnixSocket(t *testing.T) {
 			}
 
 			if len(gotOrigins) != len(test.expectOrigins) {
-				t.Errorf("Expected %d origins but got %d", len(test.expectOrigins), len(gotOrigins))
+				t.Errorf("%d: Expected %d origins but got %d", i, len(test.expectOrigins), len(gotOrigins))
 				return
 			}
 
@@ -607,7 +604,7 @@ func TestAllowedOriginsUnixSocket(t *testing.T) {
 			}
 
 			if !reflect.DeepEqual(expectMap, gotMap) {
-				t.Errorf("Origins mismatch.\nExpected: %v\nGot: %v", test.expectOrigins, gotOrigins)
+				t.Errorf("%d: Origins mismatch.\nExpected: %v\nGot: %v", i, test.expectOrigins, gotOrigins)
 			}
 		})
 	}