diff --git a/docs/operator-manual/argocd-cm.yaml b/docs/operator-manual/argocd-cm.yaml index 61b8c3faa5d4f..16cfe24b48f2e 100644 --- a/docs/operator-manual/argocd-cm.yaml +++ b/docs/operator-manual/argocd-cm.yaml @@ -420,3 +420,5 @@ data: cluster: name: some-cluster server: https://some-cluster + # The maximum size of the payload that can be sent to the webhook server. + webhook.maxPayloadSizeMB: 1024 \ No newline at end of file diff --git a/docs/operator-manual/webhook.md b/docs/operator-manual/webhook.md index a0e6c8deba1b2..92789e983d3b3 100644 --- a/docs/operator-manual/webhook.md +++ b/docs/operator-manual/webhook.md @@ -19,6 +19,8 @@ URL configured in the Git provider should use the `/api/webhook` endpoint of you (e.g. `https://argocd.example.com/api/webhook`). If you wish to use a shared secret, input an arbitrary value in the secret. This value will be used when configuring the webhook in the next step. +To prevent DDoS attacks with unauthenticated webhook events (the `/api/webhook` endpoint currently lacks rate limiting protection), it is recommended to limit the payload size. You can achieve this by configuring the `argocd-cm` ConfigMap with the `webhook.maxPayloadSizeMB` attribute. The default value is 1GB. + ## Github ![Add Webhook](../assets/webhook-config.png "Add Webhook") diff --git a/server/server.go b/server/server.go index 862ad4075c4f9..163e8d3595592 100644 --- a/server/server.go +++ b/server/server.go @@ -1049,7 +1049,7 @@ func (a *ArgoCDServer) newHTTPServer(ctx context.Context, port int, grpcWebHandl // Webhook handler for git events (Note: cache timeouts are hardcoded because API server does not write to cache and not really using them) argoDB := db.NewDB(a.Namespace, a.settingsMgr, a.KubeClientset) - acdWebhookHandler := webhook.NewHandler(a.Namespace, a.ArgoCDServerOpts.ApplicationNamespaces, a.AppClientset, a.settings, a.settingsMgr, a.RepoServerCache, a.Cache, argoDB) + acdWebhookHandler := webhook.NewHandler(a.Namespace, a.ArgoCDServerOpts.ApplicationNamespaces, a.AppClientset, a.settings, a.settingsMgr, a.RepoServerCache, a.Cache, argoDB, a.settingsMgr.GetMaxWebhookPayloadSize()) mux.HandleFunc("/api/webhook", acdWebhookHandler.Handler) diff --git a/util/settings/settings.go b/util/settings/settings.go index 1f036ddeff7c6..3651dfade6872 100644 --- a/util/settings/settings.go +++ b/util/settings/settings.go @@ -434,6 +434,8 @@ const ( settingsWebhookAzureDevOpsUsernameKey = "webhook.azuredevops.username" // settingsWebhookAzureDevOpsPasswordKey is the key for Azure DevOps webhook password settingsWebhookAzureDevOpsPasswordKey = "webhook.azuredevops.password" + // settingsWebhookMaxPayloadSize is the key for the maximum payload size for webhooks in MB + settingsWebhookMaxPayloadSizeMB = "webhook.maxPayloadSizeMB" // settingsApplicationInstanceLabelKey is the key to configure injected app instance label key settingsApplicationInstanceLabelKey = "application.instanceLabelKey" // settingsResourceTrackingMethodKey is the key to configure tracking method for application resources @@ -517,6 +519,11 @@ const ( RespectRBACValueNormal = "normal" ) +const ( + // default max webhook payload size is 1GB + defaultMaxWebhookPayloadSize = int64(1) * 1024 * 1024 * 1024 +) + var sourceTypeToEnableGenerationKey = map[v1alpha1.ApplicationSourceType]string{ v1alpha1.ApplicationSourceTypeKustomize: "kustomize.enable", v1alpha1.ApplicationSourceTypeHelm: "helm.enable", @@ -2257,3 +2264,22 @@ func (mgr *SettingsManager) GetExcludeEventLabelKeys() []string { } return labelKeys } + +func (mgr *SettingsManager) GetMaxWebhookPayloadSize() int64 { + argoCDCM, err := mgr.getConfigMap() + if err != nil { + return defaultMaxWebhookPayloadSize + } + + if argoCDCM.Data[settingsWebhookMaxPayloadSizeMB] == "" { + return defaultMaxWebhookPayloadSize + } + + maxPayloadSizeMB, err := strconv.ParseInt(argoCDCM.Data[settingsWebhookMaxPayloadSizeMB], 10, 64) + if err != nil { + log.Warnf("Failed to parse '%s' key: %v", settingsWebhookMaxPayloadSizeMB, err) + return defaultMaxWebhookPayloadSize + } + + return maxPayloadSizeMB * 1024 * 1024 +} diff --git a/util/webhook/webhook.go b/util/webhook/webhook.go index b20657fb4567b..65ed848ac4fd2 100644 --- a/util/webhook/webhook.go +++ b/util/webhook/webhook.go @@ -61,9 +61,10 @@ type ArgoCDWebhookHandler struct { azuredevopsAuthHandler func(r *http.Request) error gogs *gogs.Webhook settingsSrc settingsSource + maxWebhookPayloadSizeB int64 } -func NewHandler(namespace string, applicationNamespaces []string, appClientset appclientset.Interface, set *settings.ArgoCDSettings, settingsSrc settingsSource, repoCache *cache.Cache, serverCache *servercache.Cache, argoDB db.ArgoDB) *ArgoCDWebhookHandler { +func NewHandler(namespace string, applicationNamespaces []string, appClientset appclientset.Interface, set *settings.ArgoCDSettings, settingsSrc settingsSource, repoCache *cache.Cache, serverCache *servercache.Cache, argoDB db.ArgoDB, maxWebhookPayloadSizeB int64) *ArgoCDWebhookHandler { githubWebhook, err := github.New(github.Options.Secret(set.WebhookGitHubSecret)) if err != nil { log.Warnf("Unable to init the GitHub webhook") @@ -113,6 +114,7 @@ func NewHandler(namespace string, applicationNamespaces []string, appClientset a repoCache: repoCache, serverCache: serverCache, db: argoDB, + maxWebhookPayloadSizeB: maxWebhookPayloadSizeB, } return &acdWebhook @@ -393,6 +395,8 @@ func (a *ArgoCDWebhookHandler) Handler(w http.ResponseWriter, r *http.Request) { var payload interface{} var err error + r.Body = http.MaxBytesReader(w, r.Body, a.maxWebhookPayloadSizeB) + switch { case r.Header.Get("X-Vss-Activityid") != "": if err = a.azuredevopsAuthHandler(r); err != nil { @@ -435,6 +439,14 @@ func (a *ArgoCDWebhookHandler) Handler(w http.ResponseWriter, r *http.Request) { } if err != nil { + // If the error is due to a large payload, return a more user-friendly error message + if err.Error() == "error parsing payload" { + msg := fmt.Sprintf("Webhook processing failed: The payload is either too large or corrupted. Please check the payload size (must be under %v MB) and ensure it is valid JSON", a.maxWebhookPayloadSizeB/1024/1024) + log.WithField(common.SecurityField, common.SecurityHigh).Warn(msg) + http.Error(w, msg, http.StatusBadRequest) + return + } + log.Infof("Webhook processing failed: %s", err) status := http.StatusBadRequest if r.Method != http.MethodPost { diff --git a/util/webhook/webhook_test.go b/util/webhook/webhook_test.go index 2e00e599fce40..bc0351fdcecc4 100644 --- a/util/webhook/webhook_test.go +++ b/util/webhook/webhook_test.go @@ -56,6 +56,11 @@ type reactorDef struct { } func NewMockHandler(reactor *reactorDef, applicationNamespaces []string, objects ...runtime.Object) *ArgoCDWebhookHandler { + defaultMaxPayloadSize := int64(1) * 1024 * 1024 * 1024 + return NewMockHandlerWithPayloadLimit(reactor, applicationNamespaces, defaultMaxPayloadSize, objects...) +} + +func NewMockHandlerWithPayloadLimit(reactor *reactorDef, applicationNamespaces []string, maxPayloadSize int64, objects ...runtime.Object) *ArgoCDWebhookHandler { appClientset := appclientset.NewSimpleClientset(objects...) if reactor != nil { defaultReactor := appClientset.ReactionChain[0] @@ -72,7 +77,7 @@ func NewMockHandler(reactor *reactorDef, applicationNamespaces []string, objects 1*time.Minute, 1*time.Minute, 10*time.Second, - ), servercache.NewCache(appstate.NewCache(cacheClient, time.Minute), time.Minute, time.Minute, time.Minute), &mocks.ArgoDB{}) + ), servercache.NewCache(appstate.NewCache(cacheClient, time.Minute), time.Minute, time.Minute, time.Minute), &mocks.ArgoDB{}, maxPayloadSize) } func TestGitHubCommitEvent(t *testing.T) { @@ -393,7 +398,7 @@ func TestInvalidEvent(t *testing.T) { w := httptest.NewRecorder() h.Handler(w, req) assert.Equal(t, http.StatusBadRequest, w.Code) - expectedLogResult := "Webhook processing failed: error parsing payload" + expectedLogResult := "Webhook processing failed: The payload is either too large or corrupted. Please check the payload size (must be under 1024 MB) and ensure it is valid JSON" assert.Equal(t, expectedLogResult, hook.LastEntry().Message) assert.Equal(t, expectedLogResult+"\n", w.Body.String()) hook.Reset() @@ -604,3 +609,20 @@ func Test_getWebUrlRegex(t *testing.T) { }) } } + +func TestGitHubCommitEventMaxPayloadSize(t *testing.T) { + hook := test.NewGlobal() + maxPayloadSize := int64(100) + h := NewMockHandlerWithPayloadLimit(nil, []string{}, maxPayloadSize) + req := httptest.NewRequest(http.MethodPost, "/api/webhook", nil) + req.Header.Set("X-GitHub-Event", "push") + eventJSON, err := os.ReadFile("testdata/github-commit-event.json") + require.NoError(t, err) + req.Body = io.NopCloser(bytes.NewReader(eventJSON)) + w := httptest.NewRecorder() + h.Handler(w, req) + assert.Equal(t, http.StatusBadRequest, w.Code) + expectedLogResult := "Webhook processing failed: The payload is either too large or corrupted. Please check the payload size (must be under 0 MB) and ensure it is valid JSON" + assert.Equal(t, expectedLogResult, hook.LastEntry().Message) + hook.Reset() +}