diff --git a/agent/acl.go b/agent/acl.go index 0bf4180eb4..e266feafa8 100644 --- a/agent/acl.go +++ b/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 { diff --git a/agent/agent.go b/agent/agent.go index ff840d1626..21ada77cd1 100644 --- a/agent/agent.go +++ b/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 -// that the token is allowed to access proxy-related information (leaf +// verifyProxyToken takes a token and attempts to verify it against the +// 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 { - proxy := a.State.Proxy(proxyId) - if proxy == nil { - return fmt.Errorf("unknown proxy service ID: %q", proxyId) +func (a *Agent) verifyProxyToken(token, targetService, targetProxy string) error { + // If we specify a target proxy, we look up that proxy directly. Otherwise, + // we resolve with any proxy we can find. + 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. - // If this happens we can return without any requests. - if token == proxy.ProxyToken { + // The existence of a token isn't enough, we also need to verify + // that the service name of the matching proxy matches our target + // 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 } diff --git a/agent/agent_endpoint.go b/agent/agent_endpoint.go index de1c0d48ea..32b326867b 100644 --- a/agent/agent_endpoint.go +++ b/agent/agent_endpoint.go @@ -925,15 +925,12 @@ func (s *HTTPServer) AgentConnectCALeafCert(resp http.ResponseWriter, req *http. } args.MinQueryIndex = qOpts.MinQueryIndex - // Validate token - // TODO(banks): support correct proxy token checking too - rule, err := s.agent.resolveToken(qOpts.Token) + // Verify the proxy token. This will check both the local proxy token + // as well as the ACL if the token isn't local. + 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 } diff --git a/agent/agent_endpoint_test.go b/agent/agent_endpoint_test.go index 66ebc59ef0..d4b55a50f0 100644 --- a/agent/agent_endpoint_test.go +++ b/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) {