Skip to content

Commit

Permalink
Extend secret refresh allow list to all api/app key
Browse files Browse the repository at this point in the history
  • Loading branch information
hush-hush committed Feb 26, 2025
1 parent 153cd14 commit 2dbdf2a
Show file tree
Hide file tree
Showing 5 changed files with 73 additions and 41 deletions.
11 changes: 2 additions & 9 deletions comp/api/api/apiimpl/internal/config/endpoint.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ import (

api "github.com/DataDog/datadog-agent/comp/api/api/def"
"github.com/DataDog/datadog-agent/pkg/config/model"
pkgconfigsetup "github.com/DataDog/datadog-agent/pkg/config/setup"
util "github.com/DataDog/datadog-agent/pkg/util/common"
"github.com/DataDog/datadog-agent/pkg/util/log"
)
Expand Down Expand Up @@ -109,14 +108,8 @@ func (c *configEndpoint) getAllConfigValuesHandler(w http.ResponseWriter, r *htt

// GetConfigEndpointMuxCore builds and returns the mux for the config endpoint with default values
// for the core agent
func GetConfigEndpointMuxCore() *gorilla.Router {
return GetConfigEndpointMux(pkgconfigsetup.Datadog(), api.AuthorizedConfigPathsCore, "core")
}

// GetConfigEndpointMux builds and returns the mux for the config endpoint, with the given config,
// authorized paths, and expvar namespace
func GetConfigEndpointMux(cfg model.Reader, authorizedConfigPaths api.AuthorizedSet, expvarNamespace string) *gorilla.Router {
mux, _ := getConfigEndpoint(cfg, authorizedConfigPaths, expvarNamespace)
func GetConfigEndpointMuxCore(cfg model.Reader) *gorilla.Router {
mux, _ := getConfigEndpoint(cfg, api.AuthorizedConfigPathsCore, "core")
return mux
}

Expand Down
2 changes: 1 addition & 1 deletion comp/api/api/apiimpl/server_ipc.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ func (server *apiServer) startIPCServer(ipcServerAddr string, tmf observability.
return err
}

configEndpointMux := configendpoint.GetConfigEndpointMuxCore()
configEndpointMux := configendpoint.GetConfigEndpointMuxCore(server.cfg)
configEndpointMux.Use(validateToken)

ipcMux := http.NewServeMux()
Expand Down
25 changes: 23 additions & 2 deletions comp/api/api/def/component.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,29 @@ type AuthorizedSet map[string]struct{}
// AuthorizedConfigPathsCore is the the set of authorized config keys authorized for the
// config API.
var AuthorizedConfigPathsCore = buildAuthorizedSet(
"api_key", "site", "dd_url", "logs_config.dd_url",
"additional_endpoints", "logs_config.additional_endpoints", "apm_config.additional_endpoints",
"api_key",
"app_key",
"site",
"dd_url",
"additional_endpoints",

"external_metrics_provider.api_key",
"external_metrics_provider.app_key",

"logs_config.additional_endpoints",
"apm_config.additional_endpoints",
"database_monitoring.samples.additional_endpoints",
"database_monitoring.metrics.additional_endpoints",
"database_monitoring.activity.additional_endpoints",
"network_devices.metadata.additional_endpoints",
"network_devices.snmp_traps.forwarder.additional_endpoints",
"network_devices.netflow.forwarder.additional_endpoints",
"network_path.forwarder.additional_endpoints",
"container_lifecycle.additional_endpoints",
"container_image.additional_endpoints",
"sbom.additional_endpoints",
"service_discovery.forwarder.additional_endpoints",
"runtime_security_config.endpoints.additional_endpoints",
)

func buildAuthorizedSet(paths ...string) AuthorizedSet {
Expand Down
51 changes: 38 additions & 13 deletions comp/core/secrets/secretsimpl/secrets.go
Original file line number Diff line number Diff line change
Expand Up @@ -366,20 +366,47 @@ func (r *secretResolver) Resolve(data []byte, origin string) ([]byte, error) {
return finalConfig, nil
}

// allowlistPaths restricts what config settings may be updated
// tests can override this to exercise functionality: by setting this to nil, allow all settings
// NOTE: Related feature to `authorizedConfigPathsCore` in `comp/api/api/apiimpl/internal/config/endpoint.go`
var allowlistPaths = map[string]struct{}{"api_key": {}, "app_key": {}, "external_metrics_provider/api_key": {}, "external_metrics_provider/app_key": {}}
// allowlistPaths restricts what config settings may be updated. Any secrets linked to a settings containing any of the
// following strings will be refreshed.
//
// For example, allowing "additional_endpoints" will trigger notifications for:
// - "additional_endpoints"
// - "logs_config.additional_endpoints"
// - "logs_config.additional_endpoints.url"
// - ...
//
// NOTE: Related feature to `authorizedConfigPathsCore` in `comp/api/api/def/component.go`
var (
allowlistPaths = []string{
"api_key",
"app_key",
"additional_endpoints",
}
// tests override this to test refresh logic
allowlistEnabled = true
)

func secretMatchesAllowlist(secretCtx secretContext) bool {
if !allowlistEnabled {
return true
}
for _, allowedKey := range allowlistPaths {
if slices.Contains(secretCtx.path, allowedKey) {
return true
}
}
return false
}

// matchesAllowlist returns whether the handle is allowed, by matching all setting paths that
// handle appears at against the allowlist
func (r *secretResolver) matchesAllowlist(handle string) bool {
// if allowlist is disabled, consider every handle a match
if allowlistPaths == nil {
if !allowlistEnabled {
return true
}
for _, secretCtx := range r.origin[handle] {
if _, ok := allowlistPaths[strings.Join(secretCtx.path, "/")]; ok {
if secretMatchesAllowlist(secretCtx) {
return true
}
}
Expand Down Expand Up @@ -411,15 +438,13 @@ func (r *secretResolver) processSecretResponse(secretResponse map[string]string,
places := make([]handlePlace, 0, len(r.origin[handle]))
for _, secretCtx := range r.origin[handle] {
for _, sub := range r.subscriptions {
secretPath := strings.Join(secretCtx.path, "/")
// only update setting paths that match the allowlist
if useAllowlist && allowlistPaths != nil {
if _, ok := allowlistPaths[secretPath]; !ok {
continue
}
if useAllowlist && !secretMatchesAllowlist(secretCtx) {
// only update setting paths that match the allowlist
continue
}
// notify subscribers that secret has changed
sub(handle, secretCtx.origin, secretCtx.path, oldValue, secretValue)
secretPath := strings.Join(secretCtx.path, "/")
places = append(places, handlePlace{Context: secretCtx.origin, Path: secretPath})
}
}
Expand All @@ -443,7 +468,7 @@ func (r *secretResolver) Refresh() (string, error) {

// get handles from the cache that match the allowlist
newHandles := maps.Keys(r.cache)
if allowlistPaths != nil {
if allowlistEnabled {
filteredHandles := make([]string, 0, len(newHandles))
for _, handle := range newHandles {
if r.matchesAllowlist(handle) {
Expand Down
25 changes: 9 additions & 16 deletions comp/core/secrets/secretsimpl/secrets_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -516,8 +516,6 @@ func TestResolve(t *testing.T) {
}

func TestResolveNestedWithSubscribe(t *testing.T) {
testConf := testConfNestedMultiple

tel := fxutil.Test[telemetry.Component](t, nooptelemetry.Module())
resolver := newEnabledSecretResolver(tel)
resolver.backendCommand = "some_command"
Expand Down Expand Up @@ -548,7 +546,7 @@ func TestResolveNestedWithSubscribe(t *testing.T) {
assert.Fail(t, "unknown yaml path: %s", path)
}
})
_, err := resolver.Resolve(testConf, "test")
_, err := resolver.Resolve(testConfNestedMultiple, "test")

require.NoError(t, err)
assert.Equal(t, 1, topLevelResolved, "'top_level' secret was not resolved or resolved multiple times")
Expand All @@ -559,8 +557,6 @@ func TestResolveNestedWithSubscribe(t *testing.T) {
}

func TestResolveCached(t *testing.T) {
testConf := testConfNested

tel := fxutil.Test[telemetry.Component](t, nooptelemetry.Module())
resolver := newEnabledSecretResolver(tel)
resolver.backendCommand = "some_command"
Expand All @@ -578,7 +574,7 @@ func TestResolveCached(t *testing.T) {
resolver.SubscribeToChanges(func(handle, _ string, _ []string, _, _ any) {
totalResolved = append(totalResolved, handle)
})
_, err := resolver.Resolve(testConf, "test")
_, err := resolver.Resolve(testConfNested, "test")

// Resolve doesn't need to fetch because value is cached, but subscription is still called
require.NoError(t, err)
Expand All @@ -587,12 +583,9 @@ func TestResolveCached(t *testing.T) {
}

func TestResolveThenRefresh(t *testing.T) {
testConf := testConfNestedMultiple

// disable the allowlist for the test, let any secret changes happen
originalAllowlistPaths := allowlistPaths
allowlistPaths = nil
defer func() { allowlistPaths = originalAllowlistPaths }()
allowlistEnabled = false
defer func() { allowlistEnabled = true }()

tel := fxutil.Test[telemetry.Component](t, nooptelemetry.Module())
resolver := newEnabledSecretResolver(tel)
Expand All @@ -619,7 +612,7 @@ func TestResolveThenRefresh(t *testing.T) {
}

// resolve the secrets the first time
_, err := resolver.Resolve(testConf, "test")
_, err := resolver.Resolve(testConfNestedMultiple, "test")
require.NoError(t, err)
slices.Sort(keysResolved)
assert.Equal(t, testConfNestedOriginMultiple, resolver.origin)
Expand Down Expand Up @@ -697,15 +690,15 @@ func TestRefreshAllowlist(t *testing.T) {
defer func() { allowlistPaths = originalAllowlistPaths }()

// only allow api_key config setting to change
allowlistPaths = map[string]struct{}{"api_key": {}}
allowlistPaths = []string{"api_key"}

// Refresh means nothing changes because allowlist doesn't allow it
_, err := resolver.Refresh()
require.NoError(t, err)
assert.Equal(t, changes, []string{})

// now allow the config setting under scrutiny to change
allowlistPaths = map[string]struct{}{"another/config/setting": {}}
allowlistPaths = []string{"setting"}

// Refresh sees the change to the handle
_, err = resolver.Refresh()
Expand Down Expand Up @@ -733,7 +726,7 @@ func TestRefreshAllowlistAppliesToEachSettingPath(t *testing.T) {

// set the allowlist so that only 1 of the settings matches, the 2nd does not
originalAllowlistPaths := allowlistPaths
allowlistPaths = map[string]struct{}{"instances/0/password": {}}
allowlistPaths = []string{"instances"}
defer func() { allowlistPaths = originalAllowlistPaths }()

// subscribe to changes made during Refresh, keep track of updated setting paths
Expand Down Expand Up @@ -761,7 +754,7 @@ func TestRefreshAddsToAuditFile(t *testing.T) {
assert.NoError(t, err)

originalAllowlistPaths := allowlistPaths
allowlistPaths = map[string]struct{}{"another/config/setting": {}}
allowlistPaths = []string{"setting"}
defer func() { allowlistPaths = originalAllowlistPaths }()

tel := fxutil.Test[telemetry.Component](t, nooptelemetry.Module())
Expand Down

0 comments on commit 2dbdf2a

Please sign in to comment.