luxolus
/
open-consul
2
0
Fork 0
Code Issues 1 Pull Requests Packages Releases Wiki Activity

agent: leaf endpoint accepts name, not service ID 

This change is important so that requests can made representing a
service that may not be registered with the same local agent.
This commit is contained in:
Mitchell Hashimoto 2018-05-18 23:27:02 -07:00
parent 1476745bdc
commit 9249662c6c
No known key found for this signature in database
GPG Key ID: 744E147AA52F5B0A
3 changed files with 133 additions and 32 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

23
agent/agent.go
View File

@ -2177,21 +2177,22 @@ func (a *Agent) verifyProxyToken(token, targetService, targetProxy string) (stri
// that the service name of the matching proxy matches our target
// service.
if proxy != nil {
if proxy.Proxy.TargetServiceID != targetService {
// Get the target service since we only have the name. The nil
// check below should never be true since a proxy token always
// represents the existence of a local service.
target := a.State.Service(proxy.Proxy.TargetServiceID)
if target == nil {
return "", fmt.Errorf("proxy target service not found: %q",
proxy.Proxy.TargetServiceID)
}
if target.Service != targetService {
return "", acl.ErrPermissionDenied
}
// Resolve the actual ACL token used to register the proxy/service and
// return that for use in RPC calls.
return a.State.ServiceToken(targetService), 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)
return a.State.ServiceToken(proxy.Proxy.TargetServiceID), nil
}
// Doesn't match, we have to do a full token resolution. The required
@ -2202,7 +2203,7 @@ func (a *Agent) verifyProxyToken(token, targetService, targetProxy string) (stri
if err != nil {
return "", err
}
if rule != nil && !rule.ServiceWrite(service.Service, nil) {
if rule != nil && !rule.ServiceWrite(targetService, nil) {
return "", acl.ErrPermissionDenied
}

24
agent/agent_endpoint.go
View File

@ -908,16 +908,10 @@ func (s *HTTPServer) AgentConnectCARoots(resp http.ResponseWriter, req *http.Req
// instance. This supports blocking queries to update the returned bundle.
func (s *HTTPServer) AgentConnectCALeafCert(resp http.ResponseWriter, req *http.Request) (interface{}, error) {
// Get the service ID. Note that this is the ID of a service instance.
id := strings.TrimPrefix(req.URL.Path, "/v1/agent/connect/ca/leaf/")
// Retrieve the service specified
service := s.agent.State.Service(id)
if service == nil {
return nil, fmt.Errorf("unknown service ID: %s", id)
}
serviceName := strings.TrimPrefix(req.URL.Path, "/v1/agent/connect/ca/leaf/")
args := cachetype.ConnectCALeafRequest{
Service: service.Service, // Need name not ID
Service: serviceName, // Need name not ID
}
var qOpts structs.QueryOptions
// Store DC in the ConnectCALeafRequest but query opts separately
@ -928,7 +922,7 @@ func (s *HTTPServer) AgentConnectCALeafCert(resp http.ResponseWriter, req *http.
// Verify the proxy token. This will check both the local proxy token
// as well as the ACL if the token isn't local.
effectiveToken, err := s.agent.verifyProxyToken(qOpts.Token, id, "")
effectiveToken, err := s.agent.verifyProxyToken(qOpts.Token, serviceName, "")
if err != nil {
return nil, err
}
@ -983,12 +977,6 @@ func (s *HTTPServer) AgentConnectProxyConfig(resp http.ResponseWriter, req *http
return "", nil, nil
}
// Validate the ACL token
_, err := s.agent.verifyProxyToken(token, proxy.Proxy.TargetServiceID, id)
if err != nil {
return "", nil, err
}
// Lookup the target service as a convenience
target := s.agent.State.Service(proxy.Proxy.TargetServiceID)
if target == nil {
@ -999,6 +987,12 @@ func (s *HTTPServer) AgentConnectProxyConfig(resp http.ResponseWriter, req *http
return "", nil, nil
}
// Validate the ACL token
_, err := s.agent.verifyProxyToken(token, target.Service, id)
if err != nil {
return "", nil, err
}
// Watch the proxy for changes
ws.Add(proxy.WatchCh)

118
agent/agent_endpoint_test.go
View File

@ -2238,7 +2238,7 @@ func TestAgentConnectCALeafCert_aclDefaultDeny(t *testing.T) {
require.Equal(200, resp.Code, "body: %s", resp.Body.String())
}
req, _ := http.NewRequest("GET", "/v1/agent/connect/ca/leaf/test-id", nil)
req, _ := http.NewRequest("GET", "/v1/agent/connect/ca/leaf/test", nil)
resp := httptest.NewRecorder()
_, err := a.srv.AgentConnectCALeafCert(resp, req)
require.Error(err)
@ -2284,7 +2284,7 @@ func TestAgentConnectCALeafCert_aclProxyToken(t *testing.T) {
token := proxy.ProxyToken
require.NotEmpty(token)
req, _ := http.NewRequest("GET", "/v1/agent/connect/ca/leaf/test-id?token="+token, nil)
req, _ := http.NewRequest("GET", "/v1/agent/connect/ca/leaf/test?token="+token, nil)
resp := httptest.NewRecorder()
obj, err := a.srv.AgentConnectCALeafCert(resp, req)
require.NoError(err)
@ -2355,7 +2355,7 @@ func TestAgentConnectCALeafCert_aclProxyTokenOther(t *testing.T) {
token := proxy.ProxyToken
require.NotEmpty(token)
req, _ := http.NewRequest("GET", "/v1/agent/connect/ca/leaf/test-id?token="+token, nil)
req, _ := http.NewRequest("GET", "/v1/agent/connect/ca/leaf/test?token="+token, nil)
resp := httptest.NewRecorder()
_, err := a.srv.AgentConnectCALeafCert(resp, req)
require.Error(err)
@ -2413,7 +2413,7 @@ func TestAgentConnectCALeafCert_aclServiceWrite(t *testing.T) {
token = aclResp.ID
}
req, _ := http.NewRequest("GET", "/v1/agent/connect/ca/leaf/test-id?token="+token, nil)
req, _ := http.NewRequest("GET", "/v1/agent/connect/ca/leaf/test?token="+token, nil)
resp := httptest.NewRecorder()
obj, err := a.srv.AgentConnectCALeafCert(resp, req)
require.NoError(err)
@ -2474,7 +2474,7 @@ func TestAgentConnectCALeafCert_aclServiceReadDeny(t *testing.T) {
token = aclResp.ID
}
req, _ := http.NewRequest("GET", "/v1/agent/connect/ca/leaf/test-id?token="+token, nil)
req, _ := http.NewRequest("GET", "/v1/agent/connect/ca/leaf/test?token="+token, nil)
resp := httptest.NewRecorder()
_, err := a.srv.AgentConnectCALeafCert(resp, req)
require.Error(err)
@ -2517,7 +2517,113 @@ func TestAgentConnectCALeafCert_good(t *testing.T) {
}
// List
req, _ := http.NewRequest("GET", "/v1/agent/connect/ca/leaf/foo", nil)
req, _ := http.NewRequest("GET", "/v1/agent/connect/ca/leaf/test", nil)
resp := httptest.NewRecorder()
obj, err := a.srv.AgentConnectCALeafCert(resp, req)
require.NoError(err)
// Get the issued cert
issued, ok := obj.(*structs.IssuedCert)
assert.True(ok)
// Verify that the cert is signed by the CA
requireLeafValidUnderCA(t, issued, ca1)
// Verify blocking index
assert.True(issued.ModifyIndex > 0)
assert.Equal(fmt.Sprintf("%d", issued.ModifyIndex),
resp.Header().Get("X-Consul-Index"))
// That should've been a cache miss, so no hit change
require.Equal(cacheHits, a.cache.Hits())
// Test caching
{
// Fetch it again
obj2, err := a.srv.AgentConnectCALeafCert(httptest.NewRecorder(), req)
require.NoError(err)
require.Equal(obj, obj2)
// Should cache hit this time and not make request
require.Equal(cacheHits+1, a.cache.Hits())
cacheHits++
}
// Test that caching is updated in the background
{
// Set a new CA
ca := connect.TestCAConfigSet(t, a, nil)
retry.Run(t, func(r *retry.R) {
// Try and sign again (note no index/wait arg since cache should update in
// background even if we aren't actively blocking)
obj, err := a.srv.AgentConnectCALeafCert(httptest.NewRecorder(), req)
r.Check(err)
issued2 := obj.(*structs.IssuedCert)
if issued.CertPEM == issued2.CertPEM {
r.Fatalf("leaf has not updated")
}
// Got a new leaf. Sanity check it's a whole new key as well as differnt
// cert.
if issued.PrivateKeyPEM == issued2.PrivateKeyPEM {
r.Fatalf("new leaf has same private key as before")
}
// Verify that the cert is signed by the new CA
requireLeafValidUnderCA(t, issued2, ca)
})
// Should be a cache hit! The data should've updated in the cache
// in the background so this should've been fetched directly from
// the cache.
if v := a.cache.Hits(); v < cacheHits+1 {
t.Fatalf("expected at least one more cache hit, still at %d", v)
}
cacheHits = a.cache.Hits()
}
}
// Test we can request a leaf cert for a service we have permission for
// but is not local to this agent.
func TestAgentConnectCALeafCert_goodNotLocal(t *testing.T) {
t.Parallel()
assert := assert.New(t)
require := require.New(t)
a := NewTestAgent(t.Name(), "")
defer a.Shutdown()
// CA already setup by default by NewTestAgent but force a new one so we can
// verify it was signed easily.
ca1 := connect.TestCAConfigSet(t, a, nil)
// Grab the initial cache hit count
cacheHits := a.cache.Hits()
{
// Register a non-local service (central catalog)
args := &structs.RegisterRequest{
Node: "foo",
Address: "127.0.0.1",
Service: &structs.NodeService{
Service: "test",
Address: "127.0.0.1",
Port: 8080,
},
}
req, _ := http.NewRequest("PUT", "/v1/catalog/register", jsonReader(args))
resp := httptest.NewRecorder()
_, err := a.srv.CatalogRegister(resp, req)
require.NoError(err)
if !assert.Equal(200, resp.Code) {
t.Log("Body: ", resp.Body.String())
}
}
// List
req, _ := http.NewRequest("GET", "/v1/agent/connect/ca/leaf/test", nil)
resp := httptest.NewRecorder()
obj, err := a.srv.AgentConnectCALeafCert(resp, req)
require.NoError(err)