diff --git a/agent/agent_endpoint.go b/agent/agent_endpoint.go index 4c62354c1..70073bdcc 100644 --- a/agent/agent_endpoint.go +++ b/agent/agent_endpoint.go @@ -944,14 +944,22 @@ func (s *HTTPServer) AgentConnectCALeafCert(resp http.ResponseWriter, req *http. Service: serviceName, // Need name not ID } var qOpts structs.QueryOptions + // Store DC in the ConnectCALeafRequest but query opts separately - if done := s.parse(resp, req, &args.Datacenter, &qOpts); done { + // Don't resolve a proxy token to a real token that will be + // done with a call to verifyProxyToken later along with + // other security relevant checks. + if done := s.parseWithoutResolvingProxyToken(resp, req, &args.Datacenter, &qOpts); done { return nil, nil } args.MinQueryIndex = qOpts.MinQueryIndex // Verify the proxy token. This will check both the local proxy token - // as well as the ACL if the token isn't local. + // as well as the ACL if the token isn't local. The checks done in + // verifyProxyToken are still relevant because a leaf cert can be cached + // verifying the proxy token matches the service id or that a real + // acl token still is valid and has ServiceWrite is necessary or + // that cached cert is potentially unprotected. effectiveToken, _, err := s.agent.verifyProxyToken(qOpts.Token, serviceName, "") if err != nil { return nil, err @@ -989,9 +997,11 @@ func (s *HTTPServer) AgentConnectProxyConfig(resp http.ResponseWriter, req *http return nil, nil } - // Parse the token + // Parse the token - don't resolve a proxy token to a real token + // that will be done with a call to verifyProxyToken later along with + // other security relevant checks. var token string - s.parseToken(req, &token) + s.parseTokenWithoutResolvingProxyToken(req, &token) // Parse hash specially since it's only this endpoint that uses it currently. // Eventually this should happen in parseWait and end up in QueryOptions but I @@ -1018,7 +1028,12 @@ func (s *HTTPServer) AgentConnectProxyConfig(resp http.ResponseWriter, req *http return "", nil, nil } - // Validate the ACL token + // Validate the ACL token - because this endpoint uses data local to a single + // agent, this function is responsible for all enforcement regarding + // protection of the configuration. verifyProxyToken will match the proxies + // token to the correct service or in the case of being provide a real ACL + // token it will ensure that the requester has ServiceWrite privileges + // for this service. _, isProxyToken, err := s.agent.verifyProxyToken(token, target.Service, id) if err != nil { return "", nil, err diff --git a/agent/http.go b/agent/http.go index 1f8919539..1a0586f28 100644 --- a/agent/http.go +++ b/agent/http.go @@ -563,22 +563,43 @@ func (s *HTTPServer) parseDC(req *http.Request, dc *string) { } } -// parseToken is used to parse the ?token query param or the X-Consul-Token header -func (s *HTTPServer) parseToken(req *http.Request, token *string) { +// parseTokenInternal is used to parse the ?token query param or the X-Consul-Token header and +// optionally resolve proxy tokens to real ACL tokens. If no token is specified it will populate +// the token with the agents UserToken (acl_token in the consul configuration) +func (s *HTTPServer) parseTokenInternal(req *http.Request, token *string, resolveProxyToken bool) { + tok := "" if other := req.URL.Query().Get("token"); other != "" { - *token = other + tok = other + } else if other := req.Header.Get("X-Consul-Token"); other != "" { + tok = other + } + + if tok != "" { + if resolveProxyToken { + if p := s.agent.resolveProxyToken(tok); p != nil { + *token = s.agent.State.ServiceToken(p.Proxy.TargetServiceID) + return + } + } + + *token = tok return } - if other := req.Header.Get("X-Consul-Token"); other != "" { - *token = other - return - } - - // Set the default ACLToken *token = s.agent.tokens.UserToken() } +// parseToken is used to parse the ?token query param or the X-Consul-Token header and +// resolve proxy tokens to real ACL tokens +func (s *HTTPServer) parseToken(req *http.Request, token *string) { + s.parseTokenInternal(req, token, true) +} + +// parseTokenWithoutResolvingProxyToken is used to parse the ?token query param or the X-Consul-Token header +func (s *HTTPServer) parseTokenWithoutResolvingProxyToken(req *http.Request, token *string) { + s.parseTokenInternal(req, token, false) +} + func sourceAddrFromRequest(req *http.Request) string { xff := req.Header.Get("X-Forwarded-For") forwardHosts := strings.Split(xff, ",") @@ -631,13 +652,24 @@ func (s *HTTPServer) parseMetaFilter(req *http.Request) map[string]string { return nil } -// parse is a convenience method for endpoints that need +// parseInternal is a convenience method for endpoints that need // to use both parseWait and parseDC. -func (s *HTTPServer) parse(resp http.ResponseWriter, req *http.Request, dc *string, b *structs.QueryOptions) bool { +func (s *HTTPServer) parseInternal(resp http.ResponseWriter, req *http.Request, dc *string, b *structs.QueryOptions, resolveProxyToken bool) bool { s.parseDC(req, dc) - s.parseToken(req, &b.Token) + s.parseTokenInternal(req, &b.Token, resolveProxyToken) if s.parseConsistency(resp, req, b) { return true } return parseWait(resp, req, b) } + +// parse is a convenience method for endpoints that need +// to use both parseWait and parseDC. +func (s *HTTPServer) parse(resp http.ResponseWriter, req *http.Request, dc *string, b *structs.QueryOptions) bool { + return s.parseInternal(resp, req, dc, b, true) +} + +// parseWithoutResolvingProxyToken is a convenience method similar to parse except that it disables resolving proxy tokens +func (s *HTTPServer) parseWithoutResolvingProxyToken(resp http.ResponseWriter, req *http.Request, dc *string, b *structs.QueryOptions) bool { + return s.parseInternal(resp, req, dc, b, false) +} diff --git a/agent/http_test.go b/agent/http_test.go index ffe560d12..8338af037 100644 --- a/agent/http_test.go +++ b/agent/http_test.go @@ -22,6 +22,7 @@ import ( "github.com/hashicorp/consul/api" "github.com/hashicorp/consul/testutil" "github.com/hashicorp/go-cleanhttp" + "github.com/stretchr/testify/require" "golang.org/x/net/http2" ) @@ -786,6 +787,97 @@ func TestEnableWebUI(t *testing.T) { } } +func TestParseToken_ProxyTokenResolve(t *testing.T) { + t.Parallel() + + type endpointCheck struct { + endpoint string + handler func(s *HTTPServer, resp http.ResponseWriter, req *http.Request) (interface{}, error) + } + + // This is not an exhaustive list of all of our endpoints and is only testing GET endpoints + // right now. However it provides decent coverage that the proxy token resolution + // is happening properly + tests := []endpointCheck{ + {"/v1/acl/info/root", (*HTTPServer).ACLGet}, + {"/v1/agent/self", (*HTTPServer).AgentSelf}, + {"/v1/agent/metrics", (*HTTPServer).AgentMetrics}, + {"/v1/agent/services", (*HTTPServer).AgentServices}, + {"/v1/agent/checks", (*HTTPServer).AgentChecks}, + {"/v1/agent/members", (*HTTPServer).AgentMembers}, + {"/v1/agent/connect/ca/roots", (*HTTPServer).AgentConnectCARoots}, + {"/v1/agent/connect/ca/leaf/test", (*HTTPServer).AgentConnectCALeafCert}, + {"/v1/agent/connect/ca/proxy/test", (*HTTPServer).AgentConnectProxyConfig}, + {"/v1/catalog/connect", (*HTTPServer).CatalogConnectServiceNodes}, + {"/v1/catalog/datacenters", (*HTTPServer).CatalogDatacenters}, + {"/v1/catalog/nodes", (*HTTPServer).CatalogNodes}, + {"/v1/catalog/node/" + t.Name(), (*HTTPServer).CatalogNodeServices}, + {"/v1/catalog/services", (*HTTPServer).CatalogServices}, + {"/v1/catalog/service/test", (*HTTPServer).CatalogServiceNodes}, + {"/v1/connect/ca/configuration", (*HTTPServer).ConnectCAConfiguration}, + {"/v1/connect/ca/roots", (*HTTPServer).ConnectCARoots}, + {"/v1/connect/intentions", (*HTTPServer).IntentionEndpoint}, + {"/v1/coordinate/datacenters", (*HTTPServer).CoordinateDatacenters}, + {"/v1/coordinate/nodes", (*HTTPServer).CoordinateNodes}, + {"/v1/coordinate/node/" + t.Name(), (*HTTPServer).CoordinateNode}, + {"/v1/event/list", (*HTTPServer).EventList}, + {"/v1/health/node/" + t.Name(), (*HTTPServer).HealthNodeChecks}, + {"/v1/health/checks/test", (*HTTPServer).HealthNodeChecks}, + {"/v1/health/state/passing", (*HTTPServer).HealthChecksInState}, + {"/v1/health/service/test", (*HTTPServer).HealthServiceNodes}, + {"/v1/health/connect/test", (*HTTPServer).HealthConnectServiceNodes}, + {"/v1/operator/raft/configuration", (*HTTPServer).OperatorRaftConfiguration}, + // keyring endpoint has issues with returning errors if you haven't enabled encryption + // {"/v1/operator/keyring", (*HTTPServer).OperatorKeyringEndpoint}, + {"/v1/operator/autopilot/configuration", (*HTTPServer).OperatorAutopilotConfiguration}, + {"/v1/operator/autopilot/health", (*HTTPServer).OperatorServerHealth}, + {"/v1/query", (*HTTPServer).PreparedQueryGeneral}, + {"/v1/session/list", (*HTTPServer).SessionList}, + {"/v1/status/leader", (*HTTPServer).StatusLeader}, + {"/v1/status/peers", (*HTTPServer).StatusPeers}, + } + + a := NewTestAgent(t.Name(), TestACLConfig()+testAllowProxyConfig()) + 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.ServiceConnect{ + 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(t, err) + require.Equal(t, 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(t, proxy) + token := proxy.ProxyToken + require.NotEmpty(t, token) + + for _, check := range tests { + t.Run(fmt.Sprintf("GET(%s)", check.endpoint), func(t *testing.T) { + req, _ := http.NewRequest("GET", fmt.Sprintf("%s?token=%s", check.endpoint, token), nil) + resp := httptest.NewRecorder() + _, err := check.handler(a.srv, resp, req) + require.NoError(t, err) + }) + } +} + // assertIndex tests that X-Consul-Index is set and non-zero func assertIndex(t *testing.T, resp *httptest.ResponseRecorder) { header := resp.Header().Get("X-Consul-Index")