Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(server): allow disabling content-type check #16959

Merged
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 7 additions & 2 deletions cmd/argocd-server/commands/argocd_server.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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,
Expand Down Expand Up @@ -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")
Expand Down
3 changes: 3 additions & 0 deletions docs/operator-manual/argocd-cmd-params-cm.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
6 changes: 6 additions & 0 deletions manifests/base/server/argocd-server-deployment.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -251,6 +251,12 @@ spec:
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
Comment on lines +254 to +259
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not: spacing seems to be off

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed! The new block was okay but a bunch of others were indented by 4, which is weird.

volumeMounts:
- name: ssh-known-hosts
mountPath: /app/config/ssh
Expand Down
6 changes: 6 additions & 0 deletions manifests/ha/install.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
6 changes: 6 additions & 0 deletions manifests/ha/namespace-install.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
6 changes: 6 additions & 0 deletions manifests/install.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -22371,6 +22371,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:
Expand Down
6 changes: 6 additions & 0 deletions manifests/namespace-install.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
2 changes: 2 additions & 0 deletions server/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
43 changes: 43 additions & 0 deletions server/server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
})
}
Loading