From 6dfdd307759ef6830a03585b11b114324abfa787 Mon Sep 17 00:00:00 2001 From: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com> Date: Wed, 24 Jan 2024 14:45:09 -0500 Subject: [PATCH] fix(server): allow disabling content-type check (#16959) * fix(server): allow disabling content-type check Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com> * fix spacing Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com> --------- Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com> --- cmd/argocd-server/commands/argocd_server.go | 9 +- .../operator-manual/argocd-cmd-params-cm.yaml | 3 + .../base/server/argocd-server-deployment.yaml | 294 +++++++++--------- manifests/ha/install.yaml | 6 + manifests/ha/namespace-install.yaml | 6 + manifests/install.yaml | 11 + manifests/namespace-install.yaml | 6 + server/server.go | 2 + server/server_test.go | 43 +++ 9 files changed, 234 insertions(+), 146 deletions(-) diff --git a/cmd/argocd-server/commands/argocd_server.go b/cmd/argocd-server/commands/argocd_server.go index 6ec66801cc317..646ecd6a2aabe 100644 --- a/cmd/argocd-server/commands/argocd_server.go +++ b/cmd/argocd-server/commands/argocd_server.go @@ -172,6 +172,11 @@ func NewCommand() *cobra.Command { baseHRef = rootPath } + var contentTypesList []string + if contentTypes != "" { + contentTypesList = strings.Split(contentTypes, ";") + } + argoCDOpts := server.ArgoCDServerOpts{ Insecure: insecure, ListenPort: listenPort, @@ -187,7 +192,7 @@ func NewCommand() *cobra.Command { DexServerAddr: dexServerAddress, DexTLSConfig: dexTlsConfig, DisableAuth: disableAuth, - ContentTypes: strings.Split(contentTypes, ";"), + ContentTypes: contentTypesList, EnableGZip: enableGZip, TLSConfigCustomizer: tlsConfigCustomizer, Cache: cache, @@ -243,7 +248,7 @@ func NewCommand() *cobra.Command { command.Flags().StringVar(&repoServerAddress, "repo-server", env.StringFromEnv("ARGOCD_SERVER_REPO_SERVER", common.DefaultRepoServerAddr), "Repo server address") command.Flags().StringVar(&dexServerAddress, "dex-server", env.StringFromEnv("ARGOCD_SERVER_DEX_SERVER", common.DefaultDexServerAddr), "Dex server address") command.Flags().BoolVar(&disableAuth, "disable-auth", env.ParseBoolFromEnv("ARGOCD_SERVER_DISABLE_AUTH", false), "Disable client authentication") - command.Flags().StringVar(&contentTypes, "api-content-types", "application/json", "Semicolon separated list of allowed content types for non GET api requests. Any content type is allowed if empty.") + command.Flags().StringVar(&contentTypes, "api-content-types", env.StringFromEnv("ARGOCD_API_CONTENT_TYPES", "application/json"), "Semicolon separated list of allowed content types for non GET api requests. Any content type is allowed if empty.") command.Flags().BoolVar(&enableGZip, "enable-gzip", env.ParseBoolFromEnv("ARGOCD_SERVER_ENABLE_GZIP", true), "Enable GZIP compression") command.AddCommand(cli.NewVersionCmd(cliName)) command.Flags().StringVar(&listenHost, "address", env.StringFromEnv("ARGOCD_SERVER_LISTEN_ADDRESS", common.DefaultAddressAPIServer), "Listen on given address") diff --git a/docs/operator-manual/argocd-cmd-params-cm.yaml b/docs/operator-manual/argocd-cmd-params-cm.yaml index dac955a9662de..3cb79d85f3150 100644 --- a/docs/operator-manual/argocd-cmd-params-cm.yaml +++ b/docs/operator-manual/argocd-cmd-params-cm.yaml @@ -90,6 +90,9 @@ data: server.k8sclient.retry.max: "0" # The initial backoff delay on the first retry attempt in ms. Subsequent retries will double this backoff time up to a maximum threshold server.k8sclient.retry.base.backoff: "100" + # Semicolon-separated list of content types allowed on non-GET requests. Set an empty string to allow all. Be aware + # that allowing content types besides application/json may make your API more vulnerable to CSRF attacks. + server.api.content.types: "application/json" # Set the logging format. One of: text|json (default "text") server.log.format: "text" diff --git a/manifests/base/server/argocd-server-deployment.yaml b/manifests/base/server/argocd-server-deployment.yaml index 6df5f9701713f..0ebeb70e08531 100644 --- a/manifests/base/server/argocd-server-deployment.yaml +++ b/manifests/base/server/argocd-server-deployment.yaml @@ -25,136 +25,136 @@ spec: env: - name: ARGOCD_SERVER_INSECURE valueFrom: - configMapKeyRef: - name: argocd-cmd-params-cm - key: server.insecure - optional: true + configMapKeyRef: + name: argocd-cmd-params-cm + key: server.insecure + optional: true - name: ARGOCD_SERVER_BASEHREF valueFrom: - configMapKeyRef: - name: argocd-cmd-params-cm - key: server.basehref - optional: true + configMapKeyRef: + name: argocd-cmd-params-cm + key: server.basehref + optional: true - name: ARGOCD_SERVER_ROOTPATH valueFrom: - configMapKeyRef: - name: argocd-cmd-params-cm - key: server.rootpath - optional: true + configMapKeyRef: + name: argocd-cmd-params-cm + key: server.rootpath + optional: true - name: ARGOCD_SERVER_LOGFORMAT valueFrom: - configMapKeyRef: - name: argocd-cmd-params-cm - key: server.log.format - optional: true + configMapKeyRef: + name: argocd-cmd-params-cm + key: server.log.format + optional: true - name: ARGOCD_SERVER_LOG_LEVEL valueFrom: - configMapKeyRef: - name: argocd-cmd-params-cm - key: server.log.level - optional: true + configMapKeyRef: + name: argocd-cmd-params-cm + key: server.log.level + optional: true - name: ARGOCD_SERVER_REPO_SERVER valueFrom: - configMapKeyRef: - name: argocd-cmd-params-cm - key: repo.server - optional: true + configMapKeyRef: + name: argocd-cmd-params-cm + key: repo.server + optional: true - name: ARGOCD_SERVER_DEX_SERVER valueFrom: - configMapKeyRef: - name: argocd-cmd-params-cm - key: server.dex.server - optional: true + configMapKeyRef: + name: argocd-cmd-params-cm + key: server.dex.server + optional: true - name: ARGOCD_SERVER_DISABLE_AUTH valueFrom: - configMapKeyRef: - name: argocd-cmd-params-cm - key: server.disable.auth - optional: true + configMapKeyRef: + name: argocd-cmd-params-cm + key: server.disable.auth + optional: true - name: ARGOCD_SERVER_ENABLE_GZIP valueFrom: - configMapKeyRef: - name: argocd-cmd-params-cm - key: server.enable.gzip - optional: true + configMapKeyRef: + name: argocd-cmd-params-cm + key: server.enable.gzip + optional: true - name: ARGOCD_SERVER_REPO_SERVER_TIMEOUT_SECONDS valueFrom: - configMapKeyRef: - name: argocd-cmd-params-cm - key: server.repo.server.timeout.seconds - optional: true + configMapKeyRef: + name: argocd-cmd-params-cm + key: server.repo.server.timeout.seconds + optional: true - name: ARGOCD_SERVER_X_FRAME_OPTIONS valueFrom: - configMapKeyRef: - name: argocd-cmd-params-cm - key: server.x.frame.options - optional: true + configMapKeyRef: + name: argocd-cmd-params-cm + key: server.x.frame.options + optional: true - name: ARGOCD_SERVER_CONTENT_SECURITY_POLICY valueFrom: - configMapKeyRef: - name: argocd-cmd-params-cm - key: server.content.security.policy - optional: true + configMapKeyRef: + name: argocd-cmd-params-cm + key: server.content.security.policy + optional: true - name: ARGOCD_SERVER_REPO_SERVER_PLAINTEXT valueFrom: - configMapKeyRef: - name: argocd-cmd-params-cm - key: server.repo.server.plaintext - optional: true + configMapKeyRef: + name: argocd-cmd-params-cm + key: server.repo.server.plaintext + optional: true - name: ARGOCD_SERVER_REPO_SERVER_STRICT_TLS valueFrom: - configMapKeyRef: - name: argocd-cmd-params-cm - key: server.repo.server.strict.tls - optional: true + configMapKeyRef: + name: argocd-cmd-params-cm + key: server.repo.server.strict.tls + optional: true - name: ARGOCD_SERVER_DEX_SERVER_PLAINTEXT valueFrom: - configMapKeyRef: - name: argocd-cmd-params-cm - key: server.dex.server.plaintext - optional: true + configMapKeyRef: + name: argocd-cmd-params-cm + key: server.dex.server.plaintext + optional: true - name: ARGOCD_SERVER_DEX_SERVER_STRICT_TLS valueFrom: - configMapKeyRef: - name: argocd-cmd-params-cm - key: server.dex.server.strict.tls - optional: true + configMapKeyRef: + name: argocd-cmd-params-cm + key: server.dex.server.strict.tls + optional: true - name: ARGOCD_TLS_MIN_VERSION valueFrom: - configMapKeyRef: - name: argocd-cmd-params-cm - key: server.tls.minversion - optional: true + configMapKeyRef: + name: argocd-cmd-params-cm + key: server.tls.minversion + optional: true - name: ARGOCD_TLS_MAX_VERSION valueFrom: - configMapKeyRef: - name: argocd-cmd-params-cm - key: server.tls.maxversion - optional: true + configMapKeyRef: + name: argocd-cmd-params-cm + key: server.tls.maxversion + optional: true - name: ARGOCD_TLS_CIPHERS valueFrom: - configMapKeyRef: - name: argocd-cmd-params-cm - key: server.tls.ciphers - optional: true + configMapKeyRef: + name: argocd-cmd-params-cm + key: server.tls.ciphers + optional: true - name: ARGOCD_SERVER_CONNECTION_STATUS_CACHE_EXPIRATION valueFrom: - configMapKeyRef: - name: argocd-cmd-params-cm - key: server.connection.status.cache.expiration - optional: true + configMapKeyRef: + name: argocd-cmd-params-cm + key: server.connection.status.cache.expiration + optional: true - name: ARGOCD_SERVER_OIDC_CACHE_EXPIRATION valueFrom: - configMapKeyRef: - name: argocd-cmd-params-cm - key: server.oidc.cache.expiration - optional: true + configMapKeyRef: + name: argocd-cmd-params-cm + key: server.oidc.cache.expiration + optional: true - name: ARGOCD_SERVER_LOGIN_ATTEMPTS_EXPIRATION valueFrom: - configMapKeyRef: - name: argocd-cmd-params-cm - key: server.login.attempts.expiration - optional: true + configMapKeyRef: + name: argocd-cmd-params-cm + key: server.login.attempts.expiration + optional: true - name: ARGOCD_SERVER_STATIC_ASSETS valueFrom: configMapKeyRef: @@ -163,16 +163,16 @@ spec: optional: true - name: ARGOCD_APP_STATE_CACHE_EXPIRATION valueFrom: - configMapKeyRef: - name: argocd-cmd-params-cm - key: server.app.state.cache.expiration - optional: true + configMapKeyRef: + name: argocd-cmd-params-cm + key: server.app.state.cache.expiration + optional: true - name: REDIS_SERVER valueFrom: - configMapKeyRef: - name: argocd-cmd-params-cm - key: redis.server - optional: true + configMapKeyRef: + name: argocd-cmd-params-cm + key: redis.server + optional: true - name: REDIS_COMPRESSION valueFrom: configMapKeyRef: @@ -181,76 +181,82 @@ spec: optional: true - name: REDISDB valueFrom: - configMapKeyRef: - name: argocd-cmd-params-cm - key: redis.db - optional: true + configMapKeyRef: + name: argocd-cmd-params-cm + key: redis.db + optional: true - name: ARGOCD_DEFAULT_CACHE_EXPIRATION valueFrom: - configMapKeyRef: - name: argocd-cmd-params-cm - key: server.default.cache.expiration - optional: true + configMapKeyRef: + name: argocd-cmd-params-cm + key: server.default.cache.expiration + optional: true - name: ARGOCD_MAX_COOKIE_NUMBER valueFrom: - configMapKeyRef: - name: argocd-cmd-params-cm - key: server.http.cookie.maxnumber - optional: true + configMapKeyRef: + name: argocd-cmd-params-cm + key: server.http.cookie.maxnumber + optional: true - name: ARGOCD_SERVER_LISTEN_ADDRESS valueFrom: - configMapKeyRef: - name: argocd-cmd-params-cm - key: server.listen.address - optional: true + configMapKeyRef: + name: argocd-cmd-params-cm + key: server.listen.address + optional: true - name: ARGOCD_SERVER_METRICS_LISTEN_ADDRESS valueFrom: - configMapKeyRef: - name: argocd-cmd-params-cm - key: server.metrics.listen.address - optional: true + configMapKeyRef: + name: argocd-cmd-params-cm + key: server.metrics.listen.address + optional: true - name: ARGOCD_SERVER_OTLP_ADDRESS valueFrom: - configMapKeyRef: - name: argocd-cmd-params-cm - key: otlp.address - optional: true + configMapKeyRef: + name: argocd-cmd-params-cm + key: otlp.address + optional: true - name: ARGOCD_SERVER_OTLP_INSECURE valueFrom: - configMapKeyRef: - name: argocd-cmd-params-cm - key: otlp.insecure - optional: true + configMapKeyRef: + name: argocd-cmd-params-cm + key: otlp.insecure + optional: true - name: ARGOCD_SERVER_OTLP_HEADERS valueFrom: - configMapKeyRef: - name: argocd-cmd-params-cm - key: otlp.headers - optional: true + configMapKeyRef: + name: argocd-cmd-params-cm + key: otlp.headers + optional: true - name: ARGOCD_APPLICATION_NAMESPACES valueFrom: - configMapKeyRef: - name: argocd-cmd-params-cm - key: application.namespaces - optional: true + configMapKeyRef: + name: argocd-cmd-params-cm + key: application.namespaces + optional: true - name: ARGOCD_SERVER_ENABLE_PROXY_EXTENSION valueFrom: - configMapKeyRef: - name: argocd-cmd-params-cm - key: server.enable.proxy.extension - optional: true + configMapKeyRef: + name: argocd-cmd-params-cm + key: server.enable.proxy.extension + optional: true - name: ARGOCD_K8SCLIENT_RETRY_MAX valueFrom: - configMapKeyRef: - name: argocd-cmd-params-cm - key: server.k8sclient.retry.max - optional: true + configMapKeyRef: + name: argocd-cmd-params-cm + key: server.k8sclient.retry.max + optional: true - name: ARGOCD_K8SCLIENT_RETRY_BASE_BACKOFF valueFrom: - configMapKeyRef: - name: argocd-cmd-params-cm - key: server.k8sclient.retry.base.backoff - optional: true + configMapKeyRef: + name: argocd-cmd-params-cm + key: server.k8sclient.retry.base.backoff + optional: true + - name: ARGOCD_API_CONTENT_TYPES + valueFrom: + configMapKeyRef: + name: argocd-cmd-params-cm + key: server.api.content.types + optional: true volumeMounts: - name: ssh-known-hosts mountPath: /app/config/ssh diff --git a/manifests/ha/install.yaml b/manifests/ha/install.yaml index e343330050855..a092e4d205efd 100644 --- a/manifests/ha/install.yaml +++ b/manifests/ha/install.yaml @@ -23327,6 +23327,12 @@ spec: key: server.k8sclient.retry.base.backoff name: argocd-cmd-params-cm optional: true + - name: ARGOCD_API_CONTENT_TYPES + valueFrom: + configMapKeyRef: + key: server.api.content.types + name: argocd-cmd-params-cm + optional: true image: quay.io/argoproj/argocd:latest imagePullPolicy: Always livenessProbe: diff --git a/manifests/ha/namespace-install.yaml b/manifests/ha/namespace-install.yaml index ccac170de7e19..2c1def5603cc8 100644 --- a/manifests/ha/namespace-install.yaml +++ b/manifests/ha/namespace-install.yaml @@ -2593,6 +2593,12 @@ spec: key: server.k8sclient.retry.base.backoff name: argocd-cmd-params-cm optional: true + - name: ARGOCD_API_CONTENT_TYPES + valueFrom: + configMapKeyRef: + key: server.api.content.types + name: argocd-cmd-params-cm + optional: true image: quay.io/argoproj/argocd:latest imagePullPolicy: Always livenessProbe: diff --git a/manifests/install.yaml b/manifests/install.yaml index e65d42b5c66a0..926d8ef84d5d1 100644 --- a/manifests/install.yaml +++ b/manifests/install.yaml @@ -22372,8 +22372,19 @@ spec: key: server.k8sclient.retry.base.backoff name: argocd-cmd-params-cm optional: true +<<<<<<< HEAD image: argocd:latest imagePullPolicy: IfNotPresent +======= + - name: ARGOCD_API_CONTENT_TYPES + valueFrom: + configMapKeyRef: + key: server.api.content.types + name: argocd-cmd-params-cm + optional: true + image: quay.io/argoproj/argocd:latest + imagePullPolicy: Always +>>>>>>> 8932036d5 (fix(server): allow disabling content-type check (#16959)) livenessProbe: httpGet: path: /healthz?full=true diff --git a/manifests/namespace-install.yaml b/manifests/namespace-install.yaml index ab6e3b63348fd..d9cc590df7861 100644 --- a/manifests/namespace-install.yaml +++ b/manifests/namespace-install.yaml @@ -1637,6 +1637,12 @@ spec: key: server.k8sclient.retry.base.backoff name: argocd-cmd-params-cm optional: true + - name: ARGOCD_API_CONTENT_TYPES + valueFrom: + configMapKeyRef: + key: server.api.content.types + name: argocd-cmd-params-cm + optional: true image: quay.io/argoproj/argocd:latest imagePullPolicy: Always livenessProbe: diff --git a/server/server.go b/server/server.go index 8de2ecb9eff9c..8f6aafc689e94 100644 --- a/server/server.go +++ b/server/server.go @@ -993,6 +993,8 @@ func (a *ArgoCDServer) newHTTPServer(ctx context.Context, port int, grpcWebHandl } if len(a.ContentTypes) > 0 { handler = enforceContentTypes(handler, a.ContentTypes) + } else { + log.WithField(common.SecurityField, common.SecurityHigh).Warnf("Content-Type enforcement is disabled, which may make your API vulnerable to CSRF attacks") } mux.Handle("/api/", handler) diff --git a/server/server_test.go b/server/server_test.go index acfb32e57e5d4..c4f4153f24d89 100644 --- a/server/server_test.go +++ b/server/server_test.go @@ -1526,3 +1526,46 @@ func TestReplaceBaseHRef(t *testing.T) { }) } } + +func Test_enforceContentTypes(t *testing.T) { + getBaseHandler := func(t *testing.T, allow bool) http.Handler { + return http.HandlerFunc(func(writer http.ResponseWriter, request *http.Request) { + assert.True(t, allow, "http handler was hit when it should have been blocked by content type enforcement") + writer.WriteHeader(200) + }) + } + + t.Parallel() + + t.Run("GET - not providing a content type, should still succeed", func(t *testing.T) { + handler := enforceContentTypes(getBaseHandler(t, true), []string{"application/json"}).(http.HandlerFunc) + req := httptest.NewRequest("GET", "/", nil) + w := httptest.NewRecorder() + handler(w, req) + resp := w.Result() + assert.Equal(t, 200, resp.StatusCode) + }) + + t.Run("POST", func(t *testing.T) { + handler := enforceContentTypes(getBaseHandler(t, true), []string{"application/json"}).(http.HandlerFunc) + req := httptest.NewRequest("POST", "/", nil) + w := httptest.NewRecorder() + handler(w, req) + resp := w.Result() + assert.Equal(t, 415, resp.StatusCode, "didn't provide a content type, should have gotten an error") + + req = httptest.NewRequest("POST", "/", nil) + req.Header = map[string][]string{"Content-Type": {"application/json"}} + w = httptest.NewRecorder() + handler(w, req) + resp = w.Result() + assert.Equal(t, 200, resp.StatusCode, "should have passed, since an allowed content type was provided") + + req = httptest.NewRequest("POST", "/", nil) + req.Header = map[string][]string{"Content-Type": {"not-allowed"}} + w = httptest.NewRecorder() + handler(w, req) + resp = w.Result() + assert.Equal(t, 415, resp.StatusCode, "should not have passed, since a disallowed content type was provided") + }) +}