From e79127161a71d144edf4ee6c428d73aa40d57348 Mon Sep 17 00:00:00 2001 From: Daniel Tafoya Date: Mon, 9 Dec 2024 14:29:43 -0500 Subject: [PATCH 1/9] Add validation to process agent config endpoint --- cmd/process-agent/api/config_set.go | 21 +++++++++ cmd/process-agent/api/server.go | 3 +- .../subcommands/config/config.go | 5 ++ comp/process/apiserver/apiserver_test.go | 46 +++++++++++++++++++ 4 files changed, 73 insertions(+), 2 deletions(-) create mode 100644 cmd/process-agent/api/config_set.go diff --git a/cmd/process-agent/api/config_set.go b/cmd/process-agent/api/config_set.go new file mode 100644 index 0000000000000..22f6a0fcf3d5e --- /dev/null +++ b/cmd/process-agent/api/config_set.go @@ -0,0 +1,21 @@ +// Unless explicitly stated otherwise all files in this repository are licensed +// under the Apache License Version 2.0. +// This product includes software developed at Datadog (https://www.datadoghq.com/). +// Copyright 2016-present Datadog, Inc. + +package api + +import ( + "net/http" + + "github.com/DataDog/datadog-agent/pkg/api/util" +) + +func configSetHandler(deps APIServerDeps, w http.ResponseWriter, r *http.Request) { + if err := util.Validate(w, r); err != nil { + deps.Log.Warnf("invalid auth token for %s request to %s: %s", r.Method, r.RequestURI, err) + return + } + + deps.Settings.SetValue(w, r) +} diff --git a/cmd/process-agent/api/server.go b/cmd/process-agent/api/server.go index bdf4b1d4d105f..bfdf3155a2a8c 100644 --- a/cmd/process-agent/api/server.go +++ b/cmd/process-agent/api/server.go @@ -43,8 +43,7 @@ func SetupAPIServerHandlers(deps APIServerDeps, r *mux.Router) { r.HandleFunc("/config/all", deps.Settings.GetFullConfig("")).Methods("GET") // Get all fields from process-agent Config object r.HandleFunc("/config/list-runtime", deps.Settings.ListConfigurable).Methods("GET") r.HandleFunc("/config/{setting}", deps.Settings.GetValue).Methods("GET") - r.HandleFunc("/config/{setting}", deps.Settings.SetValue).Methods("POST") - + r.HandleFunc("/config/{setting}", injectDeps(deps, configSetHandler)).Methods("POST") r.HandleFunc("/agent/status", injectDeps(deps, statusHandler)).Methods("GET") r.HandleFunc("/agent/tagger-list", injectDeps(deps, getTaggerList)).Methods("GET") r.HandleFunc("/agent/workload-list/short", func(w http.ResponseWriter, _ *http.Request) { diff --git a/cmd/process-agent/subcommands/config/config.go b/cmd/process-agent/subcommands/config/config.go index 1acd0ec15e5e2..a46bd0291a7b7 100644 --- a/cmd/process-agent/subcommands/config/config.go +++ b/cmd/process-agent/subcommands/config/config.go @@ -181,6 +181,11 @@ func getConfigValue(deps dependencies, args []string) error { } func getClient(cfg model.Reader) (settings.Client, error) { + err := util.SetAuthToken(pkgconfigsetup.Datadog()) + if err != nil { + return nil, err + } + httpClient := apiutil.GetClient(false) ipcAddress, err := pkgconfigsetup.GetIPCAddress(pkgconfigsetup.Datadog()) diff --git a/comp/process/apiserver/apiserver_test.go b/comp/process/apiserver/apiserver_test.go index a0d94db4db4c2..b53464485032c 100644 --- a/comp/process/apiserver/apiserver_test.go +++ b/comp/process/apiserver/apiserver_test.go @@ -11,10 +11,12 @@ import ( "time" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "go.uber.org/fx" "github.com/DataDog/datadog-agent/comp/api/authtoken/fetchonlyimpl" "github.com/DataDog/datadog-agent/comp/core" + "github.com/DataDog/datadog-agent/comp/core/config" "github.com/DataDog/datadog-agent/comp/core/settings/settingsimpl" "github.com/DataDog/datadog-agent/comp/core/status" "github.com/DataDog/datadog-agent/comp/core/status/statusimpl" @@ -22,6 +24,8 @@ import ( taggerfx "github.com/DataDog/datadog-agent/comp/core/tagger/fx" workloadmeta "github.com/DataDog/datadog-agent/comp/core/workloadmeta/def" workloadmetafx "github.com/DataDog/datadog-agent/comp/core/workloadmeta/fx" + "github.com/DataDog/datadog-agent/pkg/api/util" + pkgconfigsetup "github.com/DataDog/datadog-agent/pkg/config/setup" "github.com/DataDog/datadog-agent/pkg/util/fxutil" ) @@ -29,6 +33,9 @@ func TestLifecycle(t *testing.T) { _ = fxutil.Test[Component](t, fx.Options( Module(), core.MockBundle(), + fx.Replace(config.MockParams{Overrides: map[string]interface{}{ + "process_config.cmd_port": 43424, + }}), workloadmetafx.Module(workloadmeta.NewParams()), fx.Supply( status.Params{ @@ -53,3 +60,42 @@ func TestLifecycle(t *testing.T) { return res.StatusCode == http.StatusOK }, 5*time.Second, time.Second) } + +func TestPostAuthentication(t *testing.T) { + _ = fxutil.Test[Component](t, fx.Options( + Module(), + core.MockBundle(), + fx.Replace(config.MockParams{Overrides: map[string]interface{}{ + "process_config.cmd_port": 43424, + }}), + workloadmetafx.Module(workloadmeta.NewParams()), + fx.Supply( + status.Params{ + PythonVersionGetFunc: func() string { return "n/a" }, + }, + ), + taggerfx.Module(tagger.Params{ + UseFakeTagger: true, + }), + statusimpl.Module(), + settingsimpl.MockModule(), + )) + + assert.EventuallyWithT(t, func(c *assert.CollectT) { + // No authentication + req, err := http.NewRequest("POST", "http://localhost:43424/config/log_level?value=debug", nil) + require.NoError(c, err) + res, err := util.GetClient(false).Do(req) + require.NoError(c, err) + defer res.Body.Close() + assert.Equal(c, http.StatusUnauthorized, res.StatusCode) + + // With authentication + util.CreateAndSetAuthToken(pkgconfigsetup.Datadog()) + req.Header.Set("Authorization", "Bearer "+util.GetAuthToken()) + res, err = util.GetClient(false).Do(req) + require.NoError(c, err) + defer res.Body.Close() + assert.Equal(c, http.StatusOK, res.StatusCode) + }, 5*time.Second, time.Second) +} From 5fd830acc01cfad906e01cbbb6cb6e7ce21b6357 Mon Sep 17 00:00:00 2001 From: Daniel Tafoya Date: Fri, 13 Dec 2024 14:53:39 -0500 Subject: [PATCH 2/9] reno --- .../process-agent-config-auth-09a3123ac6496052.yaml | 11 +++++++++++ 1 file changed, 11 insertions(+) create mode 100644 releasenotes/notes/process-agent-config-auth-09a3123ac6496052.yaml diff --git a/releasenotes/notes/process-agent-config-auth-09a3123ac6496052.yaml b/releasenotes/notes/process-agent-config-auth-09a3123ac6496052.yaml new file mode 100644 index 0000000000000..f33381d26dfd1 --- /dev/null +++ b/releasenotes/notes/process-agent-config-auth-09a3123ac6496052.yaml @@ -0,0 +1,11 @@ +# Each section from every release note are combined when the +# CHANGELOG.rst is rendered. So the text needs to be worded so that +# it does not depend on any information only available in another +# section. This may mean repeating some details, but each section +# must be readable independently of the other. +# +# Each section note must be formatted as reStructuredText. +--- +enhancements: + - | + The process agent endpoint for setting config values now uses authentication. From ccc084e1e9a1d68ce04ed1db9a9f4313bd83a708 Mon Sep 17 00:00:00 2001 From: Daniel Tafoya Date: Fri, 13 Dec 2024 15:32:25 -0500 Subject: [PATCH 3/9] Fix wrong port --- comp/process/apiserver/apiserver_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/comp/process/apiserver/apiserver_test.go b/comp/process/apiserver/apiserver_test.go index b53464485032c..9b6dd4f32d2af 100644 --- a/comp/process/apiserver/apiserver_test.go +++ b/comp/process/apiserver/apiserver_test.go @@ -51,7 +51,7 @@ func TestLifecycle(t *testing.T) { )) assert.Eventually(t, func() bool { - res, err := http.Get("http://localhost:6162/config") + res, err := http.Get("http://localhost:43424/config") if err != nil { return false } From aaca14acead0a5b264dc5384b6d42ef37878dd4f Mon Sep 17 00:00:00 2001 From: Daniel Tafoya Date: Tue, 17 Dec 2024 00:51:52 -0500 Subject: [PATCH 4/9] ues auth for all endpoints --- comp/process/apiserver/apiserver.go | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/comp/process/apiserver/apiserver.go b/comp/process/apiserver/apiserver.go index 24bbe6353d06f..9c37154296f64 100644 --- a/comp/process/apiserver/apiserver.go +++ b/comp/process/apiserver/apiserver.go @@ -16,8 +16,10 @@ import ( "github.com/DataDog/datadog-agent/cmd/process-agent/api" "github.com/DataDog/datadog-agent/comp/api/authtoken" - log "github.com/DataDog/datadog-agent/comp/core/log/def" + logComp "github.com/DataDog/datadog-agent/comp/core/log/def" + "github.com/DataDog/datadog-agent/pkg/api/util" pkgconfigsetup "github.com/DataDog/datadog-agent/pkg/config/setup" + "github.com/DataDog/datadog-agent/pkg/util/log" ) var _ Component = (*apiserver)(nil) @@ -31,7 +33,7 @@ type dependencies struct { Lc fx.Lifecycle - Log log.Component + Log logComp.Component At authtoken.Component @@ -41,6 +43,7 @@ type dependencies struct { //nolint:revive // TODO(PROC) Fix revive linter func newApiServer(deps dependencies) Component { r := mux.NewRouter() + r.Use(validateToken) api.SetupAPIServerHandlers(deps.APIServerDeps, r) // Set up routes addr, err := pkgconfigsetup.GetProcessAPIAddressPort(pkgconfigsetup.Datadog()) @@ -84,3 +87,13 @@ func newApiServer(deps dependencies) Component { return apiserver } + +func validateToken(next http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if err := util.Validate(w, r); err != nil { + log.Warnf("invalid auth token for %s request to %s: %s", r.Method, r.RequestURI, err) + return + } + next.ServeHTTP(w, r) + }) +} From b5487fc66316c0d04ee6dda9e7aaf1abe099a56a Mon Sep 17 00:00:00 2001 From: Daniel Tafoya Date: Tue, 17 Dec 2024 01:06:44 -0500 Subject: [PATCH 5/9] fix lifecycle --- comp/process/apiserver/apiserver_test.go | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/comp/process/apiserver/apiserver_test.go b/comp/process/apiserver/apiserver_test.go index 9b6dd4f32d2af..4160cae594127 100644 --- a/comp/process/apiserver/apiserver_test.go +++ b/comp/process/apiserver/apiserver_test.go @@ -50,14 +50,15 @@ func TestLifecycle(t *testing.T) { fetchonlyimpl.MockModule(), )) - assert.Eventually(t, func() bool { - res, err := http.Get("http://localhost:43424/config") - if err != nil { - return false - } + assert.EventuallyWithT(t, func(c *assert.CollectT) { + req, err := http.NewRequest("POST", "http://localhost:43424/config", nil) + require.NoError(c, err) + util.CreateAndSetAuthToken(pkgconfigsetup.Datadog()) + req.Header.Set("Authorization", "Bearer "+util.GetAuthToken()) + res, err := util.GetClient(false).Do(req) + require.NoError(c, err) defer res.Body.Close() - - return res.StatusCode == http.StatusOK + assert.Equal(c, http.StatusOK, res.StatusCode) }, 5*time.Second, time.Second) } From 15f45364e6fb08156a97e54dc9f9e67534da9439 Mon Sep 17 00:00:00 2001 From: Daniel Tafoya Date: Tue, 17 Dec 2024 22:52:31 -0500 Subject: [PATCH 6/9] Add addintional auth token checks --- cmd/process-agent/subcommands/config/config.go | 2 +- cmd/process-agent/subcommands/status/status.go | 6 ++++++ cmd/process-agent/subcommands/taggerlist/tagger_list.go | 5 +++++ comp/process/apiserver/apiserver_test.go | 2 +- pkg/flare/archive.go | 6 ++++++ 5 files changed, 19 insertions(+), 2 deletions(-) diff --git a/cmd/process-agent/subcommands/config/config.go b/cmd/process-agent/subcommands/config/config.go index a46bd0291a7b7..f7a9996ac10ab 100644 --- a/cmd/process-agent/subcommands/config/config.go +++ b/cmd/process-agent/subcommands/config/config.go @@ -181,7 +181,7 @@ func getConfigValue(deps dependencies, args []string) error { } func getClient(cfg model.Reader) (settings.Client, error) { - err := util.SetAuthToken(pkgconfigsetup.Datadog()) + err := util.SetAuthToken(cfg) if err != nil { return nil, err } diff --git a/cmd/process-agent/subcommands/status/status.go b/cmd/process-agent/subcommands/status/status.go index 8826c7547fb61..f98877e1979a2 100644 --- a/cmd/process-agent/subcommands/status/status.go +++ b/cmd/process-agent/subcommands/status/status.go @@ -21,6 +21,7 @@ import ( log "github.com/DataDog/datadog-agent/comp/core/log/def" compStatus "github.com/DataDog/datadog-agent/comp/core/status" "github.com/DataDog/datadog-agent/comp/process" + "github.com/DataDog/datadog-agent/pkg/api/util" apiutil "github.com/DataDog/datadog-agent/pkg/api/util" "github.com/DataDog/datadog-agent/pkg/collector/python" pkgconfigsetup "github.com/DataDog/datadog-agent/pkg/config/setup" @@ -147,6 +148,11 @@ func runStatus(deps dependencies) error { return err } + err = util.SetAuthToken(deps.Config) + if err != nil { + return err + } + getAndWriteStatus(deps.Log, statusURL, os.Stdout) return nil } diff --git a/cmd/process-agent/subcommands/taggerlist/tagger_list.go b/cmd/process-agent/subcommands/taggerlist/tagger_list.go index e7ec238efdeea..235a5a2b958df 100644 --- a/cmd/process-agent/subcommands/taggerlist/tagger_list.go +++ b/cmd/process-agent/subcommands/taggerlist/tagger_list.go @@ -18,6 +18,7 @@ import ( "github.com/DataDog/datadog-agent/comp/core/config" log "github.com/DataDog/datadog-agent/comp/core/log/def" "github.com/DataDog/datadog-agent/comp/core/tagger/api" + "github.com/DataDog/datadog-agent/pkg/api/util" pkgconfigsetup "github.com/DataDog/datadog-agent/pkg/config/setup" "github.com/DataDog/datadog-agent/pkg/util/fxutil" ) @@ -58,6 +59,10 @@ func taggerList(deps dependencies) error { return err } + err = util.SetAuthToken(deps.Config) + if err != nil { + return err + } return api.GetTaggerList(color.Output, taggerURL) } diff --git a/comp/process/apiserver/apiserver_test.go b/comp/process/apiserver/apiserver_test.go index 4160cae594127..d4aded7ca7e23 100644 --- a/comp/process/apiserver/apiserver_test.go +++ b/comp/process/apiserver/apiserver_test.go @@ -51,7 +51,7 @@ func TestLifecycle(t *testing.T) { )) assert.EventuallyWithT(t, func(c *assert.CollectT) { - req, err := http.NewRequest("POST", "http://localhost:43424/config", nil) + req, err := http.NewRequest("GET", "http://localhost:43424/agent/status", nil) require.NoError(c, err) util.CreateAndSetAuthToken(pkgconfigsetup.Datadog()) req.Header.Set("Authorization", "Bearer "+util.GetAuthToken()) diff --git a/pkg/flare/archive.go b/pkg/flare/archive.go index b61c6d9dd6ed2..2bf349dc35654 100644 --- a/pkg/flare/archive.go +++ b/pkg/flare/archive.go @@ -27,6 +27,7 @@ import ( flaretypes "github.com/DataDog/datadog-agent/comp/core/flare/types" workloadmeta "github.com/DataDog/datadog-agent/comp/core/workloadmeta/def" "github.com/DataDog/datadog-agent/pkg/api/security" + "github.com/DataDog/datadog-agent/pkg/api/util" apiutil "github.com/DataDog/datadog-agent/pkg/api/util" pkgconfigsetup "github.com/DataDog/datadog-agent/pkg/config/setup" "github.com/DataDog/datadog-agent/pkg/diagnose" @@ -394,6 +395,11 @@ func getProcessAgentTaggerList() ([]byte, error) { return nil, fmt.Errorf("wrong configuration to connect to process-agent") } + err = util.SetAuthToken(pkgconfigsetup.Datadog()) + if err != nil { + return nil, err + } + taggerListURL := fmt.Sprintf("http://%s/agent/tagger-list", addressPort) return getTaggerList(taggerListURL) } From f7b106f1a4096549ad1a30460a7a245f7f01e186 Mon Sep 17 00:00:00 2001 From: Daniel Tafoya Date: Wed, 18 Dec 2024 00:07:10 -0500 Subject: [PATCH 7/9] Add component --- comp/process/apiserver/apiserver_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/comp/process/apiserver/apiserver_test.go b/comp/process/apiserver/apiserver_test.go index d4aded7ca7e23..34736ef24d695 100644 --- a/comp/process/apiserver/apiserver_test.go +++ b/comp/process/apiserver/apiserver_test.go @@ -80,6 +80,7 @@ func TestPostAuthentication(t *testing.T) { }), statusimpl.Module(), settingsimpl.MockModule(), + fetchonlyimpl.MockModule(), )) assert.EventuallyWithT(t, func(c *assert.CollectT) { From 3a35a38e3030d0bdfc08872325087f603028dc58 Mon Sep 17 00:00:00 2001 From: Daniel Tafoya Date: Wed, 18 Dec 2024 02:20:11 -0500 Subject: [PATCH 8/9] Remove dupe import --- pkg/flare/archive.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/pkg/flare/archive.go b/pkg/flare/archive.go index 2bf349dc35654..8c2ad86688e97 100644 --- a/pkg/flare/archive.go +++ b/pkg/flare/archive.go @@ -27,7 +27,6 @@ import ( flaretypes "github.com/DataDog/datadog-agent/comp/core/flare/types" workloadmeta "github.com/DataDog/datadog-agent/comp/core/workloadmeta/def" "github.com/DataDog/datadog-agent/pkg/api/security" - "github.com/DataDog/datadog-agent/pkg/api/util" apiutil "github.com/DataDog/datadog-agent/pkg/api/util" pkgconfigsetup "github.com/DataDog/datadog-agent/pkg/config/setup" "github.com/DataDog/datadog-agent/pkg/diagnose" @@ -395,7 +394,7 @@ func getProcessAgentTaggerList() ([]byte, error) { return nil, fmt.Errorf("wrong configuration to connect to process-agent") } - err = util.SetAuthToken(pkgconfigsetup.Datadog()) + err = apiutil.SetAuthToken(pkgconfigsetup.Datadog()) if err != nil { return nil, err } From 5519fbf5ab0124fc2cbde28b1ae64c88326f3e49 Mon Sep 17 00:00:00 2001 From: Daniel Tafoya Date: Wed, 18 Dec 2024 10:28:44 -0500 Subject: [PATCH 9/9] Fix e2e test --- test/new-e2e/tests/process/linux_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/new-e2e/tests/process/linux_test.go b/test/new-e2e/tests/process/linux_test.go index 7b5e093d5bf83..6955ff308a3d1 100644 --- a/test/new-e2e/tests/process/linux_test.go +++ b/test/new-e2e/tests/process/linux_test.go @@ -117,7 +117,7 @@ func (s *linuxTestSuite) TestProcessChecksInCoreAgent() { // Verify that the process agent is not running assert.EventuallyWithT(t, func(c *assert.CollectT) { - status := s.Env().RemoteHost.MustExecute("/opt/datadog-agent/embedded/bin/process-agent status") + status := s.Env().RemoteHost.MustExecute("sudo /opt/datadog-agent/embedded/bin/process-agent status") assert.Contains(c, status, "The Process Agent is not running") }, 1*time.Minute, 5*time.Second)