agent: verify local proxy tokens for CA leaf + tests

agent/acl.go

@@ -8,6 +8,7 @@ import (
 	"github.com/armon/go-metrics"
 	"github.com/hashicorp/consul/acl"
 	"github.com/hashicorp/consul/agent/config"
+	"github.com/hashicorp/consul/agent/local"
 	"github.com/hashicorp/consul/agent/structs"
 	"github.com/hashicorp/consul/types"
 	"github.com/hashicorp/golang-lru"
@@ -239,6 +240,18 @@ func (a *Agent) resolveToken(id string) (acl.ACL, error) {
 	return a.acls.lookupACL(a, id)
 }
 
+// resolveProxyToken attempts to resolve an ACL ID to a local proxy token.
+// If a local proxy isn't found with that token, nil is returned.
+func (a *Agent) resolveProxyToken(id string) *local.ManagedProxy {
+	for _, p := range a.State.Proxies() {
+		if p.ProxyToken == id {
+			return p
+		}
+	}
+
+	return nil
+}
+
 // vetServiceRegister makes sure the service registration action is allowed by
 // the given token.
 func (a *Agent) vetServiceRegister(token string, service *structs.NodeService) error {

agent/agent.go

@@ -2115,24 +2115,53 @@ func (a *Agent) RemoveProxy(proxyID string, persist bool) error {
 	return nil
 }
 
-// verifyProxyToken takes a proxy service ID and a token and verifies
+// verifyProxyToken takes a token and attempts to verify it against the
-// that the token is allowed to access proxy-related information (leaf
+// targetService name. If targetProxy is specified, then the local proxy
+// token must exactly match the given proxy ID.
 // cert, config, etc.).
 //
 // The given token may be a local-only proxy token or it may be an ACL
 // token. We will attempt to verify the local proxy token first.
-func (a *Agent) verifyProxyToken(proxyId, token string) error {
+func (a *Agent) verifyProxyToken(token, targetService, targetProxy string) error {
-	proxy := a.State.Proxy(proxyId)
+	// If we specify a target proxy, we look up that proxy directly. Otherwise,
-	if proxy == nil {
+	// we resolve with any proxy we can find.
-		return fmt.Errorf("unknown proxy service ID: %q", proxyId)
+	var proxy *local.ManagedProxy
+	if targetProxy != "" {
+		proxy = a.State.Proxy(targetProxy)
+		if proxy == nil {
+			return fmt.Errorf("unknown proxy service ID: %q", targetProxy)
+		}
+
+		// If the token DOESN'T match, then we reset the proxy which will
+		// cause the logic below to fall back to normal ACLs. Otherwise,
+		// we keep the proxy set because we also have to verify that the
+		// target service matches on the proxy.
+		if token != proxy.ProxyToken {
+			proxy = nil
+		}
+	} else {
+		proxy = a.resolveProxyToken(token)
 	}
 
-	// Easy case is if the token just matches our local proxy token.
+	// The existence of a token isn't enough, we also need to verify
-	// If this happens we can return without any requests.
+	// that the service name of the matching proxy matches our target
-	if token == proxy.ProxyToken {
+	// service.
+	if proxy != nil {
+		if proxy.Proxy.TargetServiceID != targetService {
+			return acl.ErrPermissionDenied
+		}
+
 		return nil
 	}
 
+	// Retrieve the service specified. This should always exist because
+	// we only call this function for proxies and leaf certs and both can
+	// only be called for local services.
+	service := a.State.Service(targetService)
+	if service == nil {
+		return fmt.Errorf("unknown service ID: %s", targetService)
+	}
+
 	// Doesn't match, we have to do a full token resolution. The required
 	// permission for any proxy-related endpont is service:write, since
 	// to register a proxy you require that permission and sensitive data
@@ -2141,7 +2170,7 @@ func (a *Agent) verifyProxyToken(proxyId, token string) error {
 	if err != nil {
 		return err
 	}
-	if rule != nil && !rule.ServiceWrite(proxy.Proxy.TargetServiceID, nil) {
+	if rule != nil && !rule.ServiceWrite(service.Service, nil) {
 		return acl.ErrPermissionDenied
 	}
 

agent/agent_endpoint.go

@@ -925,15 +925,12 @@ func (s *HTTPServer) AgentConnectCALeafCert(resp http.ResponseWriter, req *http.
 	}
 	args.MinQueryIndex = qOpts.MinQueryIndex
 
-	// Validate token
+	// Verify the proxy token. This will check both the local proxy token
-	// TODO(banks): support correct proxy token checking too
+	// as well as the ACL if the token isn't local.
-	rule, err := s.agent.resolveToken(qOpts.Token)
+	err := s.agent.verifyProxyToken(qOpts.Token, id, "")
 	if err != nil {
 		return nil, err
 	}
-	if rule != nil && !rule.ServiceWrite(service.Service, nil) {
-		return nil, acl.ErrPermissionDenied
-	}
 
 	raw, err := s.agent.cache.Get(cachetype.ConnectCALeafName, &args)
 	if err != nil {
@@ -985,7 +982,8 @@ func (s *HTTPServer) AgentConnectProxyConfig(resp http.ResponseWriter, req *http
 			}
 
 			// Validate the ACL token
-			if err := s.agent.verifyProxyToken(id, token); err != nil {
+			err := s.agent.verifyProxyToken(token, proxy.Proxy.TargetServiceID, id)
+			if err != nil {
 				return "", nil, err
 			}
 

agent/agent_endpoint_test.go

@@ -2195,6 +2195,282 @@ func TestAgentConnectCARoots_list(t *testing.T) {
 	}
 }
 
+func TestAgentConnectCALeafCert_aclDefaultDeny(t *testing.T) {
+	t.Parallel()
+
+	require := require.New(t)
+	a := NewTestAgent(t.Name(), TestACLConfig()+`
+	connect {
+		enabled = true
+	}
+	`)
+	defer a.Shutdown()
+
+	// Register a service with a managed proxy
+	{
+		reg := &structs.ServiceDefinition{
+			ID:      "test-id",
+			Name:    "test",
+			Address: "127.0.0.1",
+			Port:    8000,
+			Check: structs.CheckType{
+				TTL: 15 * time.Second,
+			},
+			Connect: &structs.ServiceDefinitionConnect{
+				Proxy: &structs.ServiceDefinitionConnectProxy{},
+			},
+		}
+
+		req, _ := http.NewRequest("PUT", "/v1/agent/service/register?token=root", jsonReader(reg))
+		resp := httptest.NewRecorder()
+		_, err := a.srv.AgentRegisterService(resp, req)
+		require.NoError(err)
+		require.Equal(200, resp.Code, "body: %s", resp.Body.String())
+	}
+
+	req, _ := http.NewRequest("GET", "/v1/agent/connect/ca/leaf/test-id", nil)
+	resp := httptest.NewRecorder()
+	_, err := a.srv.AgentConnectCALeafCert(resp, req)
+	require.Error(err)
+	require.True(acl.IsErrPermissionDenied(err))
+}
+
+func TestAgentConnectCALeafCert_aclProxyToken(t *testing.T) {
+	t.Parallel()
+
+	require := require.New(t)
+	a := NewTestAgent(t.Name(), TestACLConfig()+`
+	connect {
+		enabled = true
+	}
+	`)
+	defer a.Shutdown()
+
+	// Register a service with a managed proxy
+	{
+		reg := &structs.ServiceDefinition{
+			ID:      "test-id",
+			Name:    "test",
+			Address: "127.0.0.1",
+			Port:    8000,
+			Check: structs.CheckType{
+				TTL: 15 * time.Second,
+			},
+			Connect: &structs.ServiceDefinitionConnect{
+				Proxy: &structs.ServiceDefinitionConnectProxy{},
+			},
+		}
+
+		req, _ := http.NewRequest("PUT", "/v1/agent/service/register?token=root", jsonReader(reg))
+		resp := httptest.NewRecorder()
+		_, err := a.srv.AgentRegisterService(resp, req)
+		require.NoError(err)
+		require.Equal(200, resp.Code, "body: %s", resp.Body.String())
+	}
+
+	// Get the proxy token from the agent directly, since there is no API.
+	proxy := a.State.Proxy("test-id-proxy")
+	require.NotNil(proxy)
+	token := proxy.ProxyToken
+	require.NotEmpty(token)
+
+	req, _ := http.NewRequest("GET", "/v1/agent/connect/ca/leaf/test-id?token="+token, nil)
+	resp := httptest.NewRecorder()
+	obj, err := a.srv.AgentConnectCALeafCert(resp, req)
+	require.NoError(err)
+
+	// Get the issued cert
+	_, ok := obj.(*structs.IssuedCert)
+	require.True(ok)
+}
+
+func TestAgentConnectCALeafCert_aclProxyTokenOther(t *testing.T) {
+	t.Parallel()
+
+	require := require.New(t)
+	a := NewTestAgent(t.Name(), TestACLConfig()+`
+	connect {
+		enabled = true
+	}
+	`)
+	defer a.Shutdown()
+
+	// Register a service with a managed proxy
+	{
+		reg := &structs.ServiceDefinition{
+			ID:      "test-id",
+			Name:    "test",
+			Address: "127.0.0.1",
+			Port:    8000,
+			Check: structs.CheckType{
+				TTL: 15 * time.Second,
+			},
+			Connect: &structs.ServiceDefinitionConnect{
+				Proxy: &structs.ServiceDefinitionConnectProxy{},
+			},
+		}
+
+		req, _ := http.NewRequest("PUT", "/v1/agent/service/register?token=root", jsonReader(reg))
+		resp := httptest.NewRecorder()
+		_, err := a.srv.AgentRegisterService(resp, req)
+		require.NoError(err)
+		require.Equal(200, resp.Code, "body: %s", resp.Body.String())
+	}
+
+	// Register another service
+	{
+		reg := &structs.ServiceDefinition{
+			ID:      "wrong-id",
+			Name:    "wrong",
+			Address: "127.0.0.1",
+			Port:    8000,
+			Check: structs.CheckType{
+				TTL: 15 * time.Second,
+			},
+			Connect: &structs.ServiceDefinitionConnect{
+				Proxy: &structs.ServiceDefinitionConnectProxy{},
+			},
+		}
+
+		req, _ := http.NewRequest("PUT", "/v1/agent/service/register?token=root", jsonReader(reg))
+		resp := httptest.NewRecorder()
+		_, err := a.srv.AgentRegisterService(resp, req)
+		require.NoError(err)
+		require.Equal(200, resp.Code, "body: %s", resp.Body.String())
+	}
+
+	// Get the proxy token from the agent directly, since there is no API.
+	proxy := a.State.Proxy("wrong-id-proxy")
+	require.NotNil(proxy)
+	token := proxy.ProxyToken
+	require.NotEmpty(token)
+
+	req, _ := http.NewRequest("GET", "/v1/agent/connect/ca/leaf/test-id?token="+token, nil)
+	resp := httptest.NewRecorder()
+	_, err := a.srv.AgentConnectCALeafCert(resp, req)
+	require.Error(err)
+	require.True(acl.IsErrPermissionDenied(err))
+}
+
+func TestAgentConnectCALeafCert_aclServiceWrite(t *testing.T) {
+	t.Parallel()
+
+	require := require.New(t)
+	a := NewTestAgent(t.Name(), TestACLConfig()+`
+	connect {
+		enabled = true
+	}
+	`)
+	defer a.Shutdown()
+
+	// Register a service with a managed proxy
+	{
+		reg := &structs.ServiceDefinition{
+			ID:      "test-id",
+			Name:    "test",
+			Address: "127.0.0.1",
+			Port:    8000,
+			Check: structs.CheckType{
+				TTL: 15 * time.Second,
+			},
+			Connect: &structs.ServiceDefinitionConnect{
+				Proxy: &structs.ServiceDefinitionConnectProxy{},
+			},
+		}
+
+		req, _ := http.NewRequest("PUT", "/v1/agent/service/register?token=root", jsonReader(reg))
+		resp := httptest.NewRecorder()
+		_, err := a.srv.AgentRegisterService(resp, req)
+		require.NoError(err)
+		require.Equal(200, resp.Code, "body: %s", resp.Body.String())
+	}
+
+	// Create an ACL with service:write for our service
+	var token string
+	{
+		args := map[string]interface{}{
+			"Name":  "User Token",
+			"Type":  "client",
+			"Rules": `service "test" { policy = "write" }`,
+		}
+		req, _ := http.NewRequest("PUT", "/v1/acl/create?token=root", jsonReader(args))
+		resp := httptest.NewRecorder()
+		obj, err := a.srv.ACLCreate(resp, req)
+		if err != nil {
+			t.Fatalf("err: %v", err)
+		}
+		aclResp := obj.(aclCreateResponse)
+		token = aclResp.ID
+	}
+
+	req, _ := http.NewRequest("GET", "/v1/agent/connect/ca/leaf/test-id?token="+token, nil)
+	resp := httptest.NewRecorder()
+	obj, err := a.srv.AgentConnectCALeafCert(resp, req)
+	require.NoError(err)
+
+	// Get the issued cert
+	_, ok := obj.(*structs.IssuedCert)
+	require.True(ok)
+}
+
+func TestAgentConnectCALeafCert_aclServiceReadDeny(t *testing.T) {
+	t.Parallel()
+
+	require := require.New(t)
+	a := NewTestAgent(t.Name(), TestACLConfig()+`
+	connect {
+		enabled = true
+	}
+	`)
+	defer a.Shutdown()
+
+	// Register a service with a managed proxy
+	{
+		reg := &structs.ServiceDefinition{
+			ID:      "test-id",
+			Name:    "test",
+			Address: "127.0.0.1",
+			Port:    8000,
+			Check: structs.CheckType{
+				TTL: 15 * time.Second,
+			},
+			Connect: &structs.ServiceDefinitionConnect{
+				Proxy: &structs.ServiceDefinitionConnectProxy{},
+			},
+		}
+
+		req, _ := http.NewRequest("PUT", "/v1/agent/service/register?token=root", jsonReader(reg))
+		resp := httptest.NewRecorder()
+		_, err := a.srv.AgentRegisterService(resp, req)
+		require.NoError(err)
+		require.Equal(200, resp.Code, "body: %s", resp.Body.String())
+	}
+
+	// Create an ACL with service:read for our service
+	var token string
+	{
+		args := map[string]interface{}{
+			"Name":  "User Token",
+			"Type":  "client",
+			"Rules": `service "test" { policy = "read" }`,
+		}
+		req, _ := http.NewRequest("PUT", "/v1/acl/create?token=root", jsonReader(args))
+		resp := httptest.NewRecorder()
+		obj, err := a.srv.ACLCreate(resp, req)
+		if err != nil {
+			t.Fatalf("err: %v", err)
+		}
+		aclResp := obj.(aclCreateResponse)
+		token = aclResp.ID
+	}
+
+	req, _ := http.NewRequest("GET", "/v1/agent/connect/ca/leaf/test-id?token="+token, nil)
+	resp := httptest.NewRecorder()
+	_, err := a.srv.AgentConnectCALeafCert(resp, req)
+	require.Error(err)
+	require.True(acl.IsErrPermissionDenied(err))
+}
+
 func TestAgentConnectCALeafCert_good(t *testing.T) {
 	t.Parallel()
 
@@ -2554,7 +2830,6 @@ func TestAgentConnectProxyConfig_aclDefaultDeny(t *testing.T) {
 	resp := httptest.NewRecorder()
 	_, err := a.srv.AgentConnectProxyConfig(resp, req)
 	require.True(acl.IsErrPermissionDenied(err))
-
 }
 
 func TestAgentConnectProxyConfig_aclProxyToken(t *testing.T) {