Authentication/Authorization Bypass, Replay Attack

PoC (conceptual, assumes a Grafana provisioning instance exposed at https://grafana.example.com with a configured webhook secret): Prerequisites: - A Grafana provisioning instance with a webhook secret configured for GitHub webhooks. - Ability to reach the webhook endpoint (e.g., /webhook). - Attacker can capture a legitimate webhook payload from GitHub (payload.json). 1) Capture a legitimate webhook payload from GitHub for an event you have access to (e.g., a push event) and sign it with the shared secret to produce a valid signature. This simulates an authenticated delivery. 2) Replay the same payload with the same signature to demonstrate replay protection. Python PoC to send a signed payload twice (second delivery should be treated as a replay and dropped by the server): import json import time import hmac import hashlib import requests # Replace with your Grafana webhook URL webhook_url = "https://grafana.example.com/webhook" secret = "webhook-secret" # same secret configured on the Grafana instance payload = json.dumps({ "action": "push", "repository": {"full_name": "grafana/git-ui-sync-demo"}, "ref": "refs/heads/main", "commits": [] }) def sign(p): mac = hmac.new(secret.encode(), p.encode(), hashlib.sha256) return "sha256=" + mac.hexdigest() headers = { "X-Hub-Signature-256": sign(payload), "X-GitHub-Event": "push", "Content-Type": "application/json", "X-GitHub-Delivery": "delivery-1" } # First delivery (should be accepted and trigger provisioning as appropriate) r1 = requests.post(webhook_url, data=payload, headers=headers) print("first delivery:", r1.status_code, r1.text) # Replay the same payload with the same signature # Ensure same body and signature; delivery can be same or different depending on implementation r2 = requests.post(webhook_url, data=payload, headers=headers) print("replay delivery:", r2.status_code, r2.text) Notes: - The second delivery should be detected as a replay by the new replay protection (seenOrAdd) and dropped or handled as a replay event, rather than triggering another provisioning action. - If you test without the fix (on older versions), you may observe the second delivery triggering another provisioning job or being processed again.

diff --git a/apps/provisioning/pkg/repository/github/webhook.go b/apps/provisioning/pkg/repository/github/webhook.go index 6a38cb8197863..c6c7a3fed68a9 100644 --- a/apps/provisioning/pkg/repository/github/webhook.go +++ b/apps/provisioning/pkg/repository/github/webhook.go @@ -7,6 +7,7 @@ import ( "log/slog" "net/http" "slices" + "strings" "time" "github.com/google/go-github/v82/github" @@ -24,7 +25,6 @@ var subscribedEvents = []string{"pull_request", "push"} // same order as slices. type GithubWebhookRepository interface { GithubRepository repository.Hooks - repository.WebhookRepository } @@ -32,14 +32,14 @@ var _ repository.WebhookRepository = (*githubWebhookRepository)(nil) type githubWebhookRepository struct { GithubRepository - config *provisioning.Repository - owner string - repo string - secret common.RawSecureValue - gh Client - webhookURL string - incrementalPolicy repository.IncrementalSyncPolicy - replayCache *replayCache + *repository.WebhookHandler + config *provisioning.Repository + owner string + repo string + gh Client + webhookURL string + secret common.RawSecureValue + replayCache *replayCache } func NewGithubWebhookRepository( @@ -54,32 +54,33 @@ func NewGithubWebhookRepository( if replay == nil { replay = newReplayCache(defaultReplayCacheTTL) } - return &githubWebhookRepository{ - GithubRepository: basic, - config: basic.Config(), - owner: basic.Owner(), - repo: basic.Repo(), - gh: basic.Client(), - webhookURL: webhookURL, - secret: secret, - incrementalPolicy: incrementalPolicy, - replayCache: replay, - } + cfg := basic.Config() + slug := fmt.Sprintf("%s/%s", basic.Owner(), basic.Repo()) + r := &githubWebhookRepository{ + GithubRepository: basic, + config: cfg, + owner: basic.Owner(), + repo: basic.Repo(), + gh: basic.Client(), + webhookURL: webhookURL, + secret: secret, + replayCache: replay, + } + r.WebhookHandler = repository.NewWebhookHandler( + r.processRequest, cfg.Status.Webhook, cfg.GetName(), slug, + cfg.Spec.GitHub.Branch, cfg.Spec.Sync.Enabled, incrementalPolicy, + ) + return r } -// Webhook implements Repository. -func (r *githubWebhookRepository) Webhook(ctx context.Context, req *http.Request) (*provisioning.WebhookResponse, error) { - if r.config.Status.Webhook == nil { - return nil, fmt.Errorf("unexpected webhook request") - } - +func (r *githubWebhookRepository) processRequest(ctx context.Context, req *http.Request) (repository.WebhookEvent, error) { if r.secret.IsZero() { - return nil, fmt.Errorf("missing webhook secret") + return repository.WebhookEvent{}, fmt.Errorf("missing webhook secret") } payload, err := github.ValidatePayload(req, []byte(r.secret)) if err != nil { - return nil, apierrors.NewUnauthorized("invalid signature") + return repository.WebhookEvent{}, apierrors.NewUnauthorized("invalid signature") } // Replay protection: key on the validated signature, not the @@ -100,130 +101,65 @@ func (r *githubWebhookRepository) Webhook(ctx context.Context, req *http.Request } if r.replayCache.seenOrAdd(signature) { logging.FromContext(ctx).Debug("dropping replayed webhook delivery", "delivery_id", github.DeliveryID(req)) - return &provisioning.WebhookResponse{Code: http.StatusOK, Message: "ok"}, nil + return repository.WebhookEvent{Type: repository.WebhookEventReplay}, nil } - return r.parseWebhook(ctx, github.WebHookType(req), payload) -} - -// This method does not include context because it does delegate any more requests -func (r *githubWebhookRepository) parseWebhook(ctx context.Context, messageType string, payload []byte) (*provisioning.WebhookResponse, error) { - event, err := github.ParseWebHook(messageType, payload) + event, err := github.ParseWebHook(github.WebHookType(req), payload) if err != nil { - return nil, apierrors.NewBadRequest("invalid payload") + return repository.WebhookEvent{}, apierrors.NewBadRequest("invalid payload") } switch event := event.(type) { case *github.PushEvent: - return r.parsePushEvent(ctx, event) + if event.GetRepo() == nil { + return repository.WebhookEvent{}, fmt.Errorf("missing repository in push event") + } + var deletedPaths []string + var totalChanges int + for _, change := range event.GetCommits() { + totalChanges += len(change.Added) + len(change.Modified) + len(change.Removed) + deletedPaths = append(deletedPaths, change.Removed...) + } + return repository.WebhookEvent{ + Type: repository.WebhookEventPush, + RepoSlug: event.GetRepo().GetFullName(), + Branch: strings.TrimPrefix(event.GetRef(), "refs/heads/"), + DeletedPaths: deletedPaths, + TotalChanges: totalChanges, + }, nil case *github.PullRequestEvent: - return r.parsePullRequestEvent(event) - case *github.PingEvent: - return &provisioning.WebhookResponse{ - Code: http.StatusOK, - Message: "ping received", + if event.GetRepo() == nil { + return repository.WebhookEvent{}, fmt.Errorf("missing repository in pull request event") + } + pr := event.GetPullRequest() + if pr == nil { + return repository.WebhookEvent{}, fmt.Errorf("expected PR in event") + } + return repository.WebhookEvent{ + Type: repository.WebhookEventPullRequest, + RepoSlug: event.GetRepo().GetFullName(), + Branch: pr.GetBase().GetRef(), + Action: normalizeGitHubAction(event.GetAction()), + PRNumber: pr.GetNumber(), + PRURL: pr.GetHTMLURL(), + SourceRef: pr.GetHead().GetRef(), + Hash: pr.GetHead().GetSHA(), }, nil + case *github.PingEvent: + return repository.WebhookEvent{Type: repository.WebhookEventPing}, nil default: - return &provisioning.WebhookResponse{ - Code: http.StatusNotImplemented, - Message: fmt.Sprintf("unsupported messageType: %s", messageType), + return repository.WebhookEvent{ + Type: repository.WebhookEventUnsupported, + Message: fmt.Sprintf("unsupported messageType: %s", github.WebHookType(req)), }, nil } } -func (r *githubWebhookRepository) parsePushEvent(ctx context.Context, event *github.PushEvent) (*provisioning.WebhookResponse, error) { - _, logger := r.logger(ctx, "") - - if event.GetRepo() == nil { - return nil, fmt.Errorf("missing repository in push event") - } - expected := fmt.Sprintf("%s/%s", r.owner, r.repo) - if event.GetRepo().GetFullName() != expected { - logger.Warn("webhook push event repository mismatch", "expected", expected, "got", event.GetRepo().GetFullName()) - return nil, repository.ErrRepositoryMismatch - } - - // No need to sync if not enabled - if !r.config.Spec.Sync.Enabled { - return &provisioning.WebhookResponse{Code: http.StatusOK}, nil - } - - // Skip silently if the event is not for the main/master branch - // as we cannot configure the webhook to only publish events for the main branch - if event.GetRef() != fmt.Sprintf("refs/heads/%s", r.config.Spec.GitHub.Branch) { - return &provisioning.WebhookResponse{Code: http.StatusOK}, nil - } - - var deletedPaths []string - var totalChanges int - for _, change := range event.GetCommits() { - totalChanges += len(change.Added) + len(change.Modified) + len(change.Removed) - deletedPaths = append(deletedPaths, change.Removed...) - } - - incremental := r.incrementalPolicy.CanUseIncrementalSync(deletedPaths, totalChanges) - - return &provisioning.WebhookResponse{ - Code: http.StatusAccepted, - Job: &provisioning.JobSpec{ - Repository: r.config.GetName(), - Action: provisioning.JobActionPull, - Pull: &provisioning.SyncJobOptions{ - Incremental: incremental, - }, - }, - }, nil -} - -func (r *githubWebhookRepository) parsePullRequestEvent(event *github.PullRequestEvent) (*provisioning.WebhookResponse, error) { - if event.GetRepo() == nil { - return nil, fmt.Errorf("missing repository in pull request event") - } - cfg := r.config.Spec.GitHub - if cfg == nil { - return nil, fmt.Errorf("missing GitHub config") - } - - expected := fmt.Sprintf("%s/%s", r.owner, r.repo) - if event.GetRepo().GetFullName() != expected { - slog.Warn("webhook pull request event repository mismatch", "expected", expected, "got", event.GetRepo().GetFullName()) - return nil, repository.ErrRepositoryMismatch +func normalizeGitHubAction(action string) repository.PullRequestAction { + if action == "synchronize" { + return repository.PullRequestActionUpdated } - pr := event.GetPullRequest() - if pr == nil { - return nil, fmt.Errorf("expected PR in event") - } - - if pr.GetBase().GetRef() != r.config.Spec.GitHub.Branch { - return &provisioning.WebhookResponse{ - Code: http.StatusOK, - Message: fmt.Sprintf("ignoring pull request event as %s is not the configured branch", pr.GetBase().GetRef()), - }, nil - } - - action := event.GetAction() - if action != "opened" && action != "reopened" && action != "synchronize" { - return &provisioning.WebhookResponse{ - Code: http.StatusOK, // Nothing needed - Message: fmt.Sprintf("ignore pull request event: %s", action), - }, nil - } - - // Queue an async job that will parse files - return &provisioning.WebhookResponse{ - Code: http.StatusAccepted, // Nothing needed - Message: fmt.Sprintf("pull request: %s", action), - Job: &provisioning.JobSpec{ - Repository: r.config.GetName(), - Action: provisioning.JobActionPullRequest, - PullRequest: &provisioning.PullRequestJobOptions{ - URL: pr.GetHTMLURL(), - PR: pr.GetNumber(), - Ref: pr.GetHead().GetRef(), - Hash: pr.GetHead().GetSHA(), - }, - }, - }, nil + return repository.PullRequestAction(action) } // CommentPullRequest adds a comment to a pull request. diff --git a/apps/provisioning/pkg/repository/github/webhook_test.go b/apps/provisioning/pkg/repository/github/webhook_test.go index 07d2c4e08be8e..b3f612cba6cde 100644 --- a/apps/provisioning/pkg/repository/github/webhook_test.go +++ b/apps/provisioning/pkg/repository/github/webhook_test.go @@ -1,7 +1,6 @@ package github import ( - "context" "crypto/hmac" "crypto/sha256" "encoding/hex" @@ -122,10 +121,17 @@ func TestParseWebhooks(t *testing.T) { GenerateDashboardPreviews: true, }, }, + Status: provisioning.RepositoryStatus{ + Webhook: &provisioning.WebhookStatus{}, + }, }, - owner: "grafana", - repo: "git-ui-sync-demo", + owner: "grafana", + repo: "git-ui-sync-demo", + secret: common.RawSecureValue("webhook-secret"), + replayCache: newReplayCache(time.Hour), } + gh.WebhookHandler = repo.NewWebhookHandler(gh.processRequest, gh.config.Status.Webhook, gh.config.GetName(), + "grafana/git-ui-sync-demo", gh.config.Spec.GitHub.Branch, gh.config.Spec.Sync.Enabled, repo.IncrementalSyncPolicy{}) for _, tt := range tests { name := fmt.Sprintf("webhook-%s-%s.json", tt.messageType, tt.name) @@ -134,7 +140,7 @@ func TestParseWebhooks(t *testing.T) { payload, err := os.ReadFile(path.Join("testdata", name)) require.NoError(t, err) - rsp, err := gh.parseWebhook(context.Background(), tt.messageType, payload) + rsp, err := gh.Webhook(t.Context(), signedWebhookRequest(t, tt.messageType, "webhook-secret", "", string(payload))) require.NoError(t, err) require.Equal(t, tt.expected.Code, rsp.Code) @@ -158,17 +164,23 @@ func TestParsePushEvent_LargeDiffForcesFullSync(t *testing.T) { Branch: "main", }, }, + Status: provisioning.RepositoryStatus{ + Webhook: &provisioning.WebhookStatus{}, + }, }, - owner: "grafana", - repo: "git-ui-sync-demo", - incrementalPolicy: repo.NewIncrementalSyncPolicy(false, 5), + owner: "grafana", + repo: "git-ui-sync-demo", + secret: common.RawSecureValue("webhook-secret"), + replayCache: newReplayCache(time.Hour), } + gh.WebhookHandler = repo.NewWebhookHandler(gh.processRequest, gh.config.Status.Webhook, gh.config.GetName(), + "grafana/git-ui-sync-demo", gh.config.Spec.GitHub.Branch, gh.config.Spec.Sync.Enabled, repo.NewIncrementalSyncPolicy(false, 5)) // nolint:gosec payload, err := os.ReadFile(path.Join("testdata", "webhook-push-large_diff.json")) require.NoError(t, err) - rsp, err := gh.parseWebhook(context.Background(), "push", payload) + rsp, err := gh.Webhook(t.Context(), signedWebhookRequest(t, "push", "webhook-secret", "", string(payload))) require.NoError(t, err) require.Equal(t, http.StatusAccepted, rsp.Code) @@ -196,27 +208,8 @@ func TestGitHubRepository_Webhook_ReplayProtection(t *testing.T) { const defaultSecret = "webhook-secret" - // newSignedRequest signs payload with secret and sets the headers GitHub - // sends. deliveryID populates X-GitHub-Delivery; it is intentionally - // independent of the signature so tests can vary it freely. - newSignedRequest := func(payload, deliveryID, secret string) *http.Request { - req, _ := http.NewRequest("POST", "/webhook", strings.NewReader(payload)) - req.Header.Set("X-GitHub-Event", "push") - req.Header.Set("Content-Type", "application/json") - if deliveryID != "" { - req.Header.Set("X-GitHub-Delivery", deliveryID) - } - - mac := hmac.New(sha256.New, []byte(secret)) - mac.Write([]byte(payload)) - signature := hex.EncodeToString(mac.Sum(nil)) - req.Header.Set("X-Hub-Signature-256", "sha256="+signature) - - return req - } - newRepo := func(cache *replayCache, secret string) *githubWebhookRepository { - return &githubWebhookRepository{ + r := &githubWebhookRepository{ config: &provisioning.Repository{ ObjectMeta: metav1.ObjectMeta{Name: "test-repo"}, Spec: provisioning.RepositorySpec{ @@ -232,12 +225,15 @@ func TestGitHubRepository_Webhook_ReplayProtection(t *testing.T) { secret: common.RawSecureValue(secret), replayCache: cache, } + r.WebhookHandler = repo.NewWebhookHandler(r.processRequest, r.config.Status.Webhook, r.config.GetName(), + "grafana/grafana", r.config.Spec.GitHub.Branch, r.config.Spec.Sync.Enabled, repo.IncrementalSyncPolicy{}) + return r } t.Run("first delivery is accepted", func(t *testing.T) { gh := newRepo(newReplayCache(time.Hour), defaultSecret) - rsp, err := gh.Webhook(context.Background(), newSignedRequest(pushPayload, "delivery-1", defaultSecret)) + rsp, err := gh.Webhook(t.Context(), signedWebhookRequest(t, "push", defaultSecret, "delivery-1", pushPayload)) require.NoError(t, err) require.Equal(t, http.StatusAccepted, rsp.Code) }) @@ -246,14 +242,14 @@ func TestGitHubRepository_Webhook_ReplayProtection(t *testing.T) { gh := newRepo(newReplayCache(time.Hour), defaultSecret) // First delivery succeeds with the normal accepted-job response. - first, err := gh.Webhook(context.Background(), newSignedRequest(pushPayload, "delivery-dup", defaultSecret)) + first, err := gh.Webhook(t.Context(), signedWebhookRequest( ... [truncated]