fix: Enforce webhook signature for Forgejo

Implemented signature validation for Gitea and Forgejo webhooks to
ensure the authenticity of incoming requests. This involves checking the
`X-Forgejo-Signature` and `X-Gitea-Signature` headers against a computed
HMAC-SHA256 hash of the request payload using a configured webhook
secret.

The validation ensures that only requests originating from a trusted
source with the correct secret are processed, enhancing the security of
webhook integrations. This change also includes necessary test cases to
verify the correct functioning of the validation logic.

Jira: https://issues.redhat.com/browse/SRVKP-10609
Signed-off-by: Chmouel Boudjnah <chmouel@redhat.com>

Author: chmouel

pkg/provider/gitea/gitea.go

@@ -2,7 +2,11 @@ package gitea
 
 import (
 	"context"
+	"crypto/hmac"
+	"crypto/sha256"
+	"crypto/subtle"
 	"encoding/base64"
+	"encoding/hex"
 	"encoding/json"
 	"fmt"
 	"net/http"
@@ -40,6 +44,9 @@ const (
 </td></tr>
 {{- end }}
 </table>`
+
+	ForgejoSignatureHeader = "X-Forgejo-Signature"
+	GiteaSignatureHeader   = "X-Gitea-Signature"
 )
 
 // validate the struct to interface.
@@ -122,9 +129,36 @@ func (v *Provider) SetLogger(logger *zap.SugaredLogger) {
 	v.Logger = logger
 }
 
-func (v *Provider) Validate(_ context.Context, _ *params.Run, _ *info.Event) error {
-	// TODO: figure out why gitea doesn't work with mac validation as github which seems to be the same
-	v.Logger.Debug("no secret and signature found, skipping validation for gitea")
+func (v *Provider) Validate(_ context.Context, _ *params.Run, event *info.Event) error {
+	signature := event.Request.Header.Get(ForgejoSignatureHeader)
+	if signature == "" {
+		signature = event.Request.Header.Get(GiteaSignatureHeader)
+	}
+	if signature == "" {
+		return fmt.Errorf("no signature has been detected, for security reason we are not allowing webhooks without a secret")
+	}
+
+	secret := event.Provider.WebhookSecret
+	if secret == "" {
+		return fmt.Errorf("no webhook secret has been set, in repository CR or secret")
+	}
+
+	return validateSignature(signature, event.Request.Payload, []byte(secret))
+}
+
+func validateSignature(signature string, payload, secret []byte) error {
+	signatureBytes, err := hex.DecodeString(signature)
+	if err != nil {
+		return fmt.Errorf("gitea/forgejo webhook signature is not valid hex: %w", err)
+	}
+
+	mac := hmac.New(sha256.New, secret)
+	mac.Write(payload)
+	expectedMAC := mac.Sum(nil)
+
+	if subtle.ConstantTimeCompare(signatureBytes, expectedMAC) != 1 {
+		return fmt.Errorf("gitea/forgejo webhook signature validation failed")
+	}
 	return nil
 }
 

pkg/provider/gitea/gitea_test.go

@@ -2,7 +2,9 @@ package gitea
 
 import (
 	"context"
+	"crypto/hmac"
 	"crypto/sha256"
+	"encoding/hex"
 	"fmt"
 	"io"
 	"net/http"
@@ -164,6 +166,115 @@ func TestProvider_CreateStatus(t *testing.T) {
 	}
 }
 
+func computeHMACSHA256(payload, secret []byte) string {
+	mac := hmac.New(sha256.New, secret)
+	mac.Write(payload)
+	return hex.EncodeToString(mac.Sum(nil))
+}
+
+func TestProvider_Validate(t *testing.T) {
+	testPayload := []byte(`{"ref":"refs/heads/main"}`)
+	testSecret := "mysecret"
+	validSignature := computeHMACSHA256(testPayload, []byte(testSecret))
+
+	tests := []struct {
+		name            string
+		signatureHeader string
+		signature       string
+		secret          string
+		payload         []byte
+		wantErr         string
+	}{
+		{
+			name:            "valid forgejo signature",
+			signatureHeader: ForgejoSignatureHeader,
+			signature:       validSignature,
+			secret:          testSecret,
+			payload:         testPayload,
+		},
+		{
+			name:            "valid gitea signature",
+			signatureHeader: GiteaSignatureHeader,
+			signature:       validSignature,
+			secret:          testSecret,
+			payload:         testPayload,
+		},
+		{
+			name:            "invalid signature mismatch",
+			signatureHeader: ForgejoSignatureHeader,
+			signature:       computeHMACSHA256([]byte("wrong payload"), []byte(testSecret)),
+			secret:          testSecret,
+			payload:         testPayload,
+			wantErr:         "gitea/forgejo webhook signature validation failed",
+		},
+		{
+			name:            "invalid hex in signature",
+			signatureHeader: ForgejoSignatureHeader,
+			signature:       "not-valid-hex!@#$",
+			secret:          testSecret,
+			payload:         testPayload,
+			wantErr:         "gitea/forgejo webhook signature is not valid hex",
+		},
+		{
+			name:            "signature present but no secret configured",
+			signatureHeader: ForgejoSignatureHeader,
+			signature:       validSignature,
+			secret:          "",
+			payload:         testPayload,
+			wantErr:         "no webhook secret has been set, in repository CR or secret",
+		},
+		{
+			name:            "secret configured but no signature",
+			signatureHeader: "",
+			signature:       "",
+			secret:          testSecret,
+			payload:         testPayload,
+			wantErr:         "no signature has been detected, for security reason we are not allowing webhooks without a secret",
+		},
+		{
+			name:            "no secret and no signature",
+			signatureHeader: "",
+			signature:       "",
+			secret:          "",
+			payload:         testPayload,
+			wantErr:         "no signature has been detected, for security reason we are not allowing webhooks without a secret",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			observer, _ := zapobserver.New(zap.InfoLevel)
+			logger := zap.New(observer).Sugar()
+
+			header := http.Header{}
+			if tt.signatureHeader != "" && tt.signature != "" {
+				header.Set(tt.signatureHeader, tt.signature)
+			}
+
+			event := &info.Event{
+				Provider: &info.Provider{
+					WebhookSecret: tt.secret,
+				},
+				Request: &info.Request{
+					Header:  header,
+					Payload: tt.payload,
+				},
+			}
+
+			p := &Provider{Logger: logger}
+			err := p.Validate(context.Background(), nil, event)
+
+			if tt.wantErr != "" {
+				assert.Assert(t, err != nil, "expected error but got nil")
+				assert.Assert(t, strings.Contains(err.Error(), tt.wantErr),
+					"expected error to contain %q, got %q", tt.wantErr, err.Error())
+			} else {
+				assert.NilError(t, err)
+			}
+		})
+	}
+}
+
 func TestProvider_GetFiles(t *testing.T) {
 	type args struct {
 		runevent *info.Event

test/gitea_test.go

@@ -625,7 +625,7 @@ func TestGiteaWithCLI(t *testing.T) {
 
 	output, err = tknpactest.ExecCommand(topts.ParamsRun, tknpacdelete.Root, "-n", topts.TargetNS, "repository", topts.TargetNS, "--cascade")
 	assert.NilError(t, err)
-	expectedOutput := fmt.Sprintf("secret %s has been deleted\nrepository %s has been deleted\n", topts.TargetNS, topts.TargetNS)
+	expectedOutput := fmt.Sprintf("secret %s has been deleted\nsecret webhook-secret has been deleted\nrepository %s has been deleted\n", topts.TargetNS, topts.TargetNS)
 	assert.Assert(t, output == expectedOutput, topts.TargetRefName, fmt.Sprintf("delete command should have this output: %s received: %s", expectedOutput, output))
 }
 

test/pkg/gitea/crd.go

@@ -2,6 +2,7 @@ package gitea
 
 import (
 	"context"
+	"os"
 
 	"codeberg.org/mvdkleijn/forgejo-sdk/forgejo/v2"
 	"github.com/openshift-pipelines/pipelines-as-code/pkg/apis/pipelinesascode/v1alpha1"
@@ -11,6 +12,8 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
+const webhookSecretName = "webhook-secret"
+
 // CreateToken creates gitea token with all scopes.
 func CreateToken(topts *TestOpts) (string, error) {
 	token, _, err := topts.GiteaCNX.Client().CreateAccessToken(forgejo.CreateAccessTokenOption{
@@ -48,14 +51,22 @@ func CreateCRD(ctx context.Context, topts *TestOpts, spec v1alpha1.RepositorySpe
 		}
 	}
 
+	_ = topts.ParamsRun.Clients.Kube.CoreV1().Secrets(ns).Delete(ctx, webhookSecretName, metav1.DeleteOptions{})
+	webhookSecret, _ := os.LookupEnv("TEST_EL_WEBHOOK_SECRET")
+	if err := secret.Create(ctx, topts.ParamsRun, map[string]string{"secret": webhookSecret}, ns, webhookSecretName); err != nil {
+		return err
+	}
+
+	if spec.GitProvider != nil {
+		spec.GitProvider.WebhookSecret = &v1alpha1.Secret{Name: webhookSecretName, Key: "secret"}
+	}
+	repoName := ns
+	if isGlobal {
+		repoName = info.DefaultGlobalRepoName
+	}
 	repository := &v1alpha1.Repository{
 		ObjectMeta: metav1.ObjectMeta{
-			Name: func() string {
-				if isGlobal {
-					return info.DefaultGlobalRepoName
-				}
-				return topts.TargetNS
-			}(),
+			Name: repoName,
 		},
 		Spec: spec,
 	}

test/pkg/gitea/scm.go

@@ -106,7 +106,7 @@ func GetIssueTimeline(ctx context.Context, topts *TestOpts) (Timelines, error) {
 	return tls, nil
 }
 
-func CreateGiteaRepo(giteaClient *forgejo.Client, user, name, defaultBranch, hookURL string, onOrg bool, logger *zap.SugaredLogger) (*forgejo.Repository, error) {
+func CreateGiteaRepo(giteaClient *forgejo.Client, user, name, defaultBranch, hookURL, webhookSecret string, onOrg bool, logger *zap.SugaredLogger) (*forgejo.Repository, error) {
 	var repo *forgejo.Repository
 	var err error
 	// Create a new repo
@@ -149,6 +149,7 @@ func CreateGiteaRepo(giteaClient *forgejo.Client, user, name, defaultBranch, hoo
 			"name":         "hook to smee url",
 			"url":          hookURL,
 			"content_type": "json",
+			"secret":       webhookSecret,
 		},
 		Events: []string{"push", "issue_comments", "pull_request"},
 	})

test/pkg/gitea/test.go

@@ -155,7 +155,11 @@ func TestPR(t *testing.T, topts *TestOpts) (context.Context, func()) {
 		topts.DefaultBranch = options.MainBranch
 	}
 
-	repoInfo, err := CreateGiteaRepo(topts.GiteaCNX.Client(), topts.Opts.Organization, topts.TargetRepoName, topts.DefaultBranch, hookURL, topts.OnOrg, topts.ParamsRun.Clients.Log)
+	webhookSecret := os.Getenv("TEST_EL_WEBHOOK_SECRET")
+	repoInfo, err := CreateGiteaRepo(topts.GiteaCNX.Client(), topts.Opts.Organization,
+		topts.TargetRepoName, topts.DefaultBranch, hookURL,
+		webhookSecret, topts.OnOrg,
+		topts.ParamsRun.Clients.Log)
 	assert.NilError(t, err)
 	topts.Opts.Repo = repoInfo.Name
 	topts.Opts.Organization = repoInfo.Owner.UserName