Limit proxy telemetry config to only be visible with authenticated with a proxy token

agent/agent.go

@@ -2212,19 +2212,22 @@ func (a *Agent) RemoveProxy(proxyID string, persist bool) error {
 // 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.
 //
-// The effective ACL token is returned along with any error. In the case the
+// The effective ACL token is returned along with a boolean which is true if the
-// token matches a proxy token, then the ACL token used to register that proxy's
+// match was against a proxy token rather than an ACL token, and any error. In
-// target service is returned for use in any RPC calls the proxy needs to make
+// the case the token matches a proxy token, then the ACL token used to register
-// on behalf of that service. If the token was an ACL token already then it is
+// that proxy's target service is returned for use in any RPC calls the proxy
-// always returned. Provided error is nil, a valid ACL token is always returned.
+// needs to make on behalf of that service. If the token was an ACL token
-func (a *Agent) verifyProxyToken(token, targetService, targetProxy string) (string, error) {
+// already then it is always returned. Provided error is nil, a valid ACL token
+// is always returned.
+func (a *Agent) verifyProxyToken(token, targetService,
+	targetProxy string) (string, bool, 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)
+			return "", false, fmt.Errorf("unknown proxy service ID: %q", targetProxy)
 		}
 
 		// If the token DOESN'T match, then we reset the proxy which will
@@ -2247,32 +2250,32 @@ func (a *Agent) verifyProxyToken(token, targetService, targetProxy string) (stri
 		// 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",
+			return "", false, fmt.Errorf("proxy target service not found: %q",
 				proxy.Proxy.TargetServiceID)
 		}
 
 		if target.Service != targetService {
-			return "", acl.ErrPermissionDenied
+			return "", false, 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(proxy.Proxy.TargetServiceID), nil
+		return a.State.ServiceToken(proxy.Proxy.TargetServiceID), true, nil
 	}
 
 	// Doesn't match, we have to do a full token resolution. The required
-	// permission for any proxy-related endpont is service:write, since
+	// permission for any proxy-related endpoint is service:write, since
 	// to register a proxy you require that permission and sensitive data
 	// is usually present in the configuration.
 	rule, err := a.resolveToken(token)
 	if err != nil {
-		return "", err
+		return "", false, err
 	}
 	if rule != nil && !rule.ServiceWrite(targetService, nil) {
-		return "", acl.ErrPermissionDenied
+		return "", false, acl.ErrPermissionDenied
 	}
 
-	return token, nil
+	return token, false, nil
 }
 
 func (a *Agent) cancelCheckMonitors(checkID types.CheckID) {

agent/agent_endpoint.go

@@ -952,7 +952,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, serviceName, "")
+	effectiveToken, _, err := s.agent.verifyProxyToken(qOpts.Token, serviceName, "")
 	if err != nil {
 		return nil, err
 	}
@@ -1019,7 +1019,7 @@ func (s *HTTPServer) AgentConnectProxyConfig(resp http.ResponseWriter, req *http
 			}
 
 			// Validate the ACL token
-			_, err := s.agent.verifyProxyToken(token, target.Service, id)
+			_, isProxyToken, err := s.agent.verifyProxyToken(token, target.Service, id)
 			if err != nil {
 				return "", nil, err
 			}
@@ -1063,6 +1063,15 @@ func (s *HTTPServer) AgentConnectProxyConfig(resp http.ResponseWriter, req *http
 					target.Port)
 			}
 
+			// Only merge in telemetry config from agent if the requested is
+			// authorized with a proxy token. This prevents us leaking potentially
+			// sensitive config like Circonus API token via a public endpoint. Proxy
+			// tokens are only ever generated in-memory and passed via ENV to a child
+			// proxy process so potential for abuse here seems small. This endpoint in
+			// general is only useful for managed proxies now so it should _always_ be
+			// true that auth is via a proxy token but inconvenient for testing if we
+			// lock it down so strictly.
+			if isProxyToken {
 				// Add telemetry config. Copy the global config so we can customize the
 				// prefix.
 				telemetryCfg := s.agent.config.Telemetry
@@ -1093,6 +1102,7 @@ func (s *HTTPServer) AgentConnectProxyConfig(resp http.ResponseWriter, req *http
 					// Add agent telemetry config.
 					config["telemetry"] = telemetryCfg
 				}
+			}
 
 			reply := &api.ConnectProxyConfig{
 				ProxyServiceID:    proxy.Proxy.ProxyService.ID,

agent/agent_endpoint_test.go

@@ -3274,13 +3274,16 @@ func TestAgentConnectProxyConfig_ConfigHandling(t *testing.T) {
 		Check: structs.CheckType{
 			TTL: 15 * time.Second,
 		},
-		Connect: &structs.ServiceConnect{},
+		Connect: &structs.ServiceConnect{
+			// Proxy is populated with the definition in the table below.
+		},
 	}
 
 	tests := []struct {
 		name         string
 		globalConfig string
 		proxy        structs.ServiceDefinitionConnectProxy
+		useToken     string
 		wantMode     api.ProxyExecMode
 		wantCommand  []string
 		wantConfig   map[string]interface{}
@@ -3502,6 +3505,58 @@ func TestAgentConnectProxyConfig_ConfigHandling(t *testing.T) {
 				},
 			},
 		},
+		{
+			name:     "reg passed through with no agent config added if not proxy token auth",
+			useToken: "foo", // no actual ACLs set so this any token will work but has to be non-empty to be used below
+			globalConfig: `
+			bind_addr = "0.0.0.0"
+			connect {
+				enabled = true
+				proxy {
+					allow_managed_api_registration = true
+				}
+				proxy_defaults = {
+					exec_mode = "daemon"
+					daemon_command = ["daemon.sh"]
+					script_command = ["script.sh"]
+					config = {
+						connect_timeout_ms = 1000
+					}
+				}
+			}
+			ports {
+				proxy_min_port = 10000
+				proxy_max_port = 10000
+			}
+			telemetry {
+				statsite_address = "localhost:8989"
+			}
+			`,
+			proxy: structs.ServiceDefinitionConnectProxy{
+				ExecMode: "script",
+				Command:  []string{"foo.sh"},
+				Config: map[string]interface{}{
+					"connect_timeout_ms":    2000,
+					"bind_address":          "127.0.0.1",
+					"bind_port":             1024,
+					"local_service_address": "127.0.0.1:9191",
+					"telemetry": map[string]interface{}{
+						"metrics_prefix": "foo",
+					},
+				},
+			},
+			wantMode:    api.ProxyExecModeScript,
+			wantCommand: []string{"foo.sh"},
+			wantConfig: map[string]interface{}{
+				"bind_address":          "127.0.0.1",
+				"bind_port":             float64(1024),
+				"local_service_address": "127.0.0.1:9191",
+				"connect_timeout_ms":    float64(2000),
+				"telemetry": map[string]interface{}{ // No defaults merged
+					"metrics_prefix": "foo",
+				},
+			},
+		},
 	}
 
 	for _, tt := range tests {
@@ -3522,7 +3577,16 @@ func TestAgentConnectProxyConfig_ConfigHandling(t *testing.T) {
 				require.Equal(200, resp.Code, "body: %s", resp.Body.String())
 			}
 
+			proxy := a.State.Proxy("test-id-proxy")
+			require.NotNil(proxy)
+			require.NotEmpty(proxy.ProxyToken)
+
 			req, _ := http.NewRequest("GET", "/v1/agent/connect/proxy/test-id-proxy", nil)
+			if tt.useToken != "" {
+				req.Header.Set("X-Consul-Token", tt.useToken)
+			} else {
+				req.Header.Set("X-Consul-Token", proxy.ProxyToken)
+			}
 			resp := httptest.NewRecorder()
 			obj, err := a.srv.AgentConnectProxyConfig(resp, req)
 			require.NoError(err)