From 4bf1c2e4f7398782faa5a8f0fb439f374d20487a Mon Sep 17 00:00:00 2001
From: Matt Keeler <mjkeeler7@gmail.com>
Date: Fri, 31 Jan 2020 09:57:38 -0500
Subject: [PATCH] agent: add ACL enforcement to the v1/agent/health/service/*
 endpoints

This adds acl enforcement to the two endpoints that were missing it.

Note that in the case of getting a services health by its id, we still
must first lookup the service so we still "leak" information about a
service with that ID existing. There isn't really a way around it though
as ACLs are meant to check service names.
---
 agent/agent_endpoint.go      | 27 ++++++++++++++++++
 agent/agent_endpoint_test.go | 53 ++++++++++++++++++++++++++++++++++++
 2 files changed, 80 insertions(+)

diff --git a/agent/agent_endpoint.go b/agent/agent_endpoint.go
index 2fde1421c..7a35af71f 100644
--- a/agent/agent_endpoint.go
+++ b/agent/agent_endpoint.go
@@ -737,10 +737,23 @@ func (s *HTTPServer) AgentHealthServiceByID(resp http.ResponseWriter, req *http.
 		return nil, err
 	}
 
+	var token string
+	s.parseToken(req, &token)
+
+	// need to resolve to default the meta
+	var authzContext acl.AuthorizerContext
+	authz, err := s.agent.resolveTokenAndDefaultMeta(token, &entMeta, &authzContext)
+	if err != nil {
+		return nil, err
+	}
+
 	var sid structs.ServiceID
 	sid.Init(serviceID, &entMeta)
 
 	if service := s.agent.State.Service(sid); service != nil {
+		if authz != nil && authz.ServiceRead(service.Service, &authzContext) != acl.Allow {
+			return nil, acl.ErrPermissionDenied
+		}
 		code, status, healthChecks := agentHealthService(sid, s)
 		if returnTextPlain(req) {
 			return status, CodeWithPayloadError{StatusCode: code, Reason: status, ContentType: "text/plain"}
@@ -777,6 +790,20 @@ func (s *HTTPServer) AgentHealthServiceByName(resp http.ResponseWriter, req *htt
 		return nil, err
 	}
 
+	var token string
+	s.parseToken(req, &token)
+
+	// need to resolve to default the meta
+	var authzContext acl.AuthorizerContext
+	authz, err := s.agent.resolveTokenAndDefaultMeta(token, &entMeta, &authzContext)
+	if err != nil {
+		return nil, err
+	}
+
+	if authz != nil && authz.ServiceRead(serviceName, &authzContext) != acl.Allow {
+		return nil, acl.ErrPermissionDenied
+	}
+
 	code := http.StatusNotFound
 	status := fmt.Sprintf("ServiceName %s Not Found", serviceName)
 	services := s.agent.State.Services(&entMeta)
diff --git a/agent/agent_endpoint_test.go b/agent/agent_endpoint_test.go
index e94ac2e5f..eb8c8d530 100644
--- a/agent/agent_endpoint_test.go
+++ b/agent/agent_endpoint_test.go
@@ -1081,6 +1081,59 @@ func TestAgent_HealthServiceByName(t *testing.T) {
 	})
 }
 
+func TestAgent_HealthServicesACLEnforcement(t *testing.T) {
+	t.Parallel()
+	a := NewTestAgent(t, t.Name(), TestACLConfigWithParams(nil))
+	defer a.Shutdown()
+
+	service := &structs.NodeService{
+		ID:      "mysql1",
+		Service: "mysql",
+	}
+	require.NoError(t, a.AddService(service, nil, false, "", ConfigSourceLocal))
+
+	service = &structs.NodeService{
+		ID:      "foo1",
+		Service: "foo",
+	}
+	require.NoError(t, a.AddService(service, nil, false, "", ConfigSourceLocal))
+
+	// no token
+	t.Run("no-token-health-by-id", func(t *testing.T) {
+		req, err := http.NewRequest("GET", "/v1/agent/health/service/id/mysql1", nil)
+		require.NoError(t, err)
+		resp := httptest.NewRecorder()
+		_, err = a.srv.AgentHealthServiceByID(resp, req)
+		require.Equal(t, acl.ErrPermissionDenied, err)
+	})
+
+	t.Run("no-token-health-by-name", func(t *testing.T) {
+		req, err := http.NewRequest("GET", "/v1/agent/health/service/name/mysql", nil)
+		require.NoError(t, err)
+		resp := httptest.NewRecorder()
+		_, err = a.srv.AgentHealthServiceByName(resp, req)
+		require.Equal(t, acl.ErrPermissionDenied, err)
+	})
+
+	t.Run("root-token-health-by-id", func(t *testing.T) {
+		req, err := http.NewRequest("GET", "/v1/agent/health/service/id/foo1", nil)
+		require.NoError(t, err)
+		req.Header.Add("X-Consul-Token", TestDefaultMasterToken)
+		resp := httptest.NewRecorder()
+		_, err = a.srv.AgentHealthServiceByID(resp, req)
+		require.NotEqual(t, acl.ErrPermissionDenied, err)
+	})
+
+	t.Run("root-token-health-by-name", func(t *testing.T) {
+		req, err := http.NewRequest("GET", "/v1/agent/health/service/name/foo", nil)
+		require.NoError(t, err)
+		req.Header.Add("X-Consul-Token", TestDefaultMasterToken)
+		resp := httptest.NewRecorder()
+		_, err = a.srv.AgentHealthServiceByName(resp, req)
+		require.NotEqual(t, acl.ErrPermissionDenied, err)
+	})
+}
+
 func TestAgent_Checks_ACLFilter(t *testing.T) {
 	t.Parallel()
 	a := NewTestAgent(t, t.Name(), TestACLConfig())