From 649c42f75b845f614654eff82d0628d526fbee29 Mon Sep 17 00:00:00 2001 From: Simone Gotti Date: Tue, 7 May 2019 18:29:31 +0200 Subject: [PATCH] gitsources: create secret and webhook secret Use the webhook secret on webhook creation and check it and webhook receive --- internal/gitsources/agolagit/agolagit.go | 5 -- internal/gitsources/agolagit/parse.go | 32 +++++------ internal/gitsources/gitea/parse.go | 53 +++++++++++------- internal/gitsources/gitlab/gitlab.go | 1 + internal/gitsources/gitlab/parse.go | 56 +++++++++---------- internal/gitsources/gitsource.go | 2 +- .../services/configstore/action/project.go | 2 + internal/services/gateway/action/project.go | 2 +- internal/services/gateway/webhook.go | 4 +- internal/services/types/types.go | 10 +++- 10 files changed, 91 insertions(+), 76 deletions(-) diff --git a/internal/gitsources/agolagit/agolagit.go b/internal/gitsources/agolagit/agolagit.go index 3d48591..9a05662 100644 --- a/internal/gitsources/agolagit/agolagit.go +++ b/internal/gitsources/agolagit/agolagit.go @@ -28,7 +28,6 @@ import ( "github.com/pkg/errors" gitsource "github.com/sorintlab/agola/internal/gitsources" - "github.com/sorintlab/agola/internal/services/types" ) var jsonContent = http.Header{"content-type": []string{"application/json"}} @@ -166,10 +165,6 @@ func (c *Client) CreateCommitStatus(repopath, commitSHA string, status gitsource return nil } -func (c *Client) ParseWebhook(r *http.Request) (*types.WebhookData, error) { - return parseWebhook(r) -} - func (c *Client) ListUserRepos() ([]*gitsource.RepoInfo, error) { return nil, nil } diff --git a/internal/gitsources/agolagit/parse.go b/internal/gitsources/agolagit/parse.go index a4ec5c9..fbf1016 100644 --- a/internal/gitsources/agolagit/parse.go +++ b/internal/gitsources/agolagit/parse.go @@ -18,6 +18,7 @@ import ( "encoding/json" "fmt" "io" + "io/ioutil" "net/http" "path" "strconv" @@ -40,31 +41,25 @@ const ( prActionSync = "synchronized" ) -func parseWebhook(r *http.Request) (*types.WebhookData, error) { +func (c *Client) ParseWebhook(r *http.Request, secret string) (*types.WebhookData, error) { + data, err := ioutil.ReadAll(io.LimitReader(r.Body, 10*1024*1024)) + if err != nil { + return nil, err + } + switch r.Header.Get(hookEvent) { case hookPush: - return parsePushHook(r.Body) + return parsePushHook(data) case hookPullRequest: - return parsePullRequestHook(r.Body) + return parsePullRequestHook(data) default: return nil, errors.Errorf("unknown webhook event type: %q", r.Header.Get(hookEvent)) } } -func parsePush(r io.Reader) (*pushHook, error) { +func parsePushHook(data []byte) (*types.WebhookData, error) { push := new(pushHook) - err := json.NewDecoder(r).Decode(push) - return push, err -} - -func parsePullRequest(r io.Reader) (*pullRequestHook, error) { - pr := new(pullRequestHook) - err := json.NewDecoder(r).Decode(pr) - return pr, err -} - -func parsePushHook(payload io.Reader) (*types.WebhookData, error) { - push, err := parsePush(payload) + err := json.Unmarshal(data, push) if err != nil { return nil, err } @@ -72,8 +67,9 @@ func parsePushHook(payload io.Reader) (*types.WebhookData, error) { return webhookDataFromPush(push) } -func parsePullRequestHook(payload io.Reader) (*types.WebhookData, error) { - prhook, err := parsePullRequest(payload) +func parsePullRequestHook(data []byte) (*types.WebhookData, error) { + prhook := new(pullRequestHook) + err := json.Unmarshal(data, prhook) if err != nil { return nil, err } diff --git a/internal/gitsources/gitea/parse.go b/internal/gitsources/gitea/parse.go index cba3c63..31e310b 100644 --- a/internal/gitsources/gitea/parse.go +++ b/internal/gitsources/gitea/parse.go @@ -15,9 +15,13 @@ package gitea import ( + "crypto/hmac" + "crypto/sha256" + "encoding/hex" "encoding/json" "fmt" "io" + "io/ioutil" "net/http" "path" "strconv" @@ -29,7 +33,8 @@ import ( ) const ( - hookEvent = "X-Gitea-Event" + hookEvent = "X-Gitea-Event" + signatureHeader = "X-Gitea-Signature" hookPush = "push" hookPullRequest = "pull_request" @@ -40,31 +45,40 @@ const ( prActionSync = "synchronized" ) -func (c *Client) ParseWebhook(r *http.Request) (*types.WebhookData, error) { +func (c *Client) ParseWebhook(r *http.Request, secret string) (*types.WebhookData, error) { + data, err := ioutil.ReadAll(io.LimitReader(r.Body, 10*1024*1024)) + if err != nil { + return nil, err + } + + // verify signature + if secret != "" { + signature := r.Header.Get(signatureHeader) + ds, err := hex.DecodeString(signature) + if err != nil { + return nil, errors.Errorf("wrong webhook signature") + } + h := hmac.New(sha256.New, []byte(secret)) + h.Write(data) + cs := h.Sum(nil) + if !hmac.Equal(cs, ds) { + return nil, errors.Errorf("wrong webhook signature") + } + } + switch r.Header.Get(hookEvent) { case hookPush: - return parsePushHook(r.Body) + return parsePushHook(data) case hookPullRequest: - return parsePullRequestHook(r.Body) + return parsePullRequestHook(data) default: return nil, errors.Errorf("unknown webhook event type: %q", r.Header.Get(hookEvent)) } } -func parsePush(r io.Reader) (*pushHook, error) { +func parsePushHook(data []byte) (*types.WebhookData, error) { push := new(pushHook) - err := json.NewDecoder(r).Decode(push) - return push, err -} - -func parsePullRequest(r io.Reader) (*pullRequestHook, error) { - pr := new(pullRequestHook) - err := json.NewDecoder(r).Decode(pr) - return pr, err -} - -func parsePushHook(payload io.Reader) (*types.WebhookData, error) { - push, err := parsePush(payload) + err := json.Unmarshal(data, push) if err != nil { return nil, err } @@ -72,8 +86,9 @@ func parsePushHook(payload io.Reader) (*types.WebhookData, error) { return webhookDataFromPush(push) } -func parsePullRequestHook(payload io.Reader) (*types.WebhookData, error) { - prhook, err := parsePullRequest(payload) +func parsePullRequestHook(data []byte) (*types.WebhookData, error) { + prhook := new(pullRequestHook) + err := json.Unmarshal(data, prhook) if err != nil { return nil, err } diff --git a/internal/gitsources/gitlab/gitlab.go b/internal/gitsources/gitlab/gitlab.go index 5cfb702..920c9e8 100644 --- a/internal/gitsources/gitlab/gitlab.go +++ b/internal/gitsources/gitlab/gitlab.go @@ -219,6 +219,7 @@ func (c *Client) CreateRepoWebhook(repopath, url, secret string) error { PushEvents: gitlab.Bool(true), TagPushEvents: gitlab.Bool(true), MergeRequestsEvents: gitlab.Bool(true), + Token: gitlab.String(secret), } _, _, err := c.client.Projects.AddProjectHook(repopath, opts) diff --git a/internal/gitsources/gitlab/parse.go b/internal/gitsources/gitlab/parse.go index 5dbaacb..35228da 100644 --- a/internal/gitsources/gitlab/parse.go +++ b/internal/gitsources/gitlab/parse.go @@ -18,6 +18,7 @@ import ( "encoding/json" "fmt" "io" + "io/ioutil" "net/http" "strconv" "strings" @@ -28,7 +29,8 @@ import ( ) const ( - hookEvent = "X-Gitlab-Event" + hookEvent = "X-Gitlab-Event" + tokenHeader = "X-Gitlab-Token" hookPush = "Push Hook" hookTagPush = "Tag Push Hook" @@ -40,33 +42,36 @@ const ( prActionSync = "synchronized" ) -func (c *Client) ParseWebhook(r *http.Request) (*types.WebhookData, error) { +func (c *Client) ParseWebhook(r *http.Request, secret string) (*types.WebhookData, error) { + data, err := ioutil.ReadAll(io.LimitReader(r.Body, 10*1024*1024)) + if err != nil { + return nil, err + } + + // verify token (gitlab doesn't sign the payload but just returns the provided + // secret) + if secret != "" { + token := r.Header.Get(tokenHeader) + if token != secret { + return nil, errors.Errorf("wrong webhook token") + } + } + switch r.Header.Get(hookEvent) { case hookPush: - return parsePushHook(r.Body) + return parsePushHook(data) case hookTagPush: - return parsePushHook(r.Body) + return parsePushHook(data) case hookPullRequest: - return parsePullRequestHook(r.Body) + return parsePullRequestHook(data) default: return nil, errors.Errorf("unknown webhook event type: %q", r.Header.Get(hookEvent)) } } -func parsePush(r io.Reader) (*pushHook, error) { +func parsePushHook(data []byte) (*types.WebhookData, error) { push := new(pushHook) - err := json.NewDecoder(r).Decode(push) - return push, err -} - -func parsePullRequest(r io.Reader) (*pullRequestHook, error) { - pr := new(pullRequestHook) - err := json.NewDecoder(r).Decode(pr) - return pr, err -} - -func parsePushHook(payload io.Reader) (*types.WebhookData, error) { - push, err := parsePush(payload) + err := json.Unmarshal(data, push) if err != nil { return nil, err } @@ -79,20 +84,15 @@ func parsePushHook(payload io.Reader) (*types.WebhookData, error) { return webhookDataFromPush(push) } -func parsePullRequestHook(payload io.Reader) (*types.WebhookData, error) { - prhook, err := parsePullRequest(payload) +func parsePullRequestHook(data []byte) (*types.WebhookData, error) { + prhook := new(pullRequestHook) + err := json.Unmarshal(data, prhook) if err != nil { return nil, err } - // // skip non open pull requests - // if prhook.PullRequest.State != prStateOpen { - // return nil, nil - // } - // // only accept actions that have new commits - // if prhook.Action != prActionOpen && prhook.Action != prActionSync { - // return nil, nil - // } + // TODO(sgotti) skip non open pull requests + // TODO(sgotti) only accept actions that have new commits return webhookDataFromPullRequest(prhook), nil } diff --git a/internal/gitsources/gitsource.go b/internal/gitsources/gitsource.go index f5e5d10..6885199 100644 --- a/internal/gitsources/gitsource.go +++ b/internal/gitsources/gitsource.go @@ -37,7 +37,7 @@ type GitSource interface { UpdateDeployKey(repopath, title, pubKey string, readonly bool) error DeleteRepoWebhook(repopath, url string) error CreateRepoWebhook(repopath, url, secret string) error - ParseWebhook(r *http.Request) (*types.WebhookData, error) + ParseWebhook(r *http.Request, secret string) (*types.WebhookData, error) CreateCommitStatus(repopath, commitSHA string, status CommitStatus, targetURL, description, context string) error ListUserRepos() ([]*RepoInfo, error) } diff --git a/internal/services/configstore/action/project.go b/internal/services/configstore/action/project.go index 0c3e900..7944469 100644 --- a/internal/services/configstore/action/project.go +++ b/internal/services/configstore/action/project.go @@ -133,7 +133,9 @@ func (h *ActionHandler) CreateProject(ctx context.Context, project *types.Projec project.ID = uuid.NewV4().String() project.Parent.Type = types.ConfigTypeProjectGroup + // generate the Secret and the WebhookSecret project.Secret = util.EncodeSha1Hex(uuid.NewV4().String()) + project.WebhookSecret = util.EncodeSha1Hex(uuid.NewV4().String()) pcj, err := json.Marshal(project) if err != nil { diff --git a/internal/services/gateway/action/project.go b/internal/services/gateway/action/project.go index 490617c..c70a7b4 100644 --- a/internal/services/gateway/action/project.go +++ b/internal/services/gateway/action/project.go @@ -194,7 +194,7 @@ func (h *ActionHandler) SetupProject(ctx context.Context, rs *types.RemoteSource return errors.Wrapf(err, "failed to delete repository webhook") } h.log.Infof("creating webhook to url: %s", webhookURL) - if err := gitsource.CreateRepoWebhook(project.RepositoryPath, webhookURL.String(), ""); err != nil { + if err := gitsource.CreateRepoWebhook(project.RepositoryPath, webhookURL.String(), project.WebhookSecret); err != nil { return errors.Wrapf(err, "failed to create repository webhook") } diff --git a/internal/services/gateway/webhook.go b/internal/services/gateway/webhook.go index 2ba3b66..138a8c1 100644 --- a/internal/services/gateway/webhook.go +++ b/internal/services/gateway/webhook.go @@ -155,7 +155,7 @@ func (h *webhooksHandler) handleWebhook(r *http.Request) (int, string, error) { skipSSHHostKeyCheck = project.SkipSSHHostKeyCheck } runType = types.RunTypeProject - webhookData, err = gitSource.ParseWebhook(r) + webhookData, err = gitSource.ParseWebhook(r, project.WebhookSecret) if err != nil { return http.StatusBadRequest, "", errors.Wrapf(err, "failed to parse webhook") } @@ -213,7 +213,7 @@ func (h *webhooksHandler) handleWebhook(r *http.Request) (int, string, error) { } else { gitSource = agolagit.New(h.apiExposedURL + "/repos") var err error - webhookData, err = gitSource.ParseWebhook(r) + webhookData, err = gitSource.ParseWebhook(r, "") if err != nil { return http.StatusBadRequest, "", errors.Wrapf(err, "failed to parse webhook") } diff --git a/internal/services/types/types.go b/internal/services/types/types.go index 9da6d3d..2a979e9 100644 --- a/internal/services/types/types.go +++ b/internal/services/types/types.go @@ -73,7 +73,8 @@ type User struct { Name string `json:"name,omitempty"` - // A secret string that could be used for signing or other purposes + // Secret is a secret that could be used for signing or other purposes. It + // should never be directly exposed to external services Secret string `json:"secret,omitempty"` LinkedAccounts map[string]*LinkedAccount `json:"linked_accounts,omitempty"` @@ -236,7 +237,8 @@ type Project struct { ID string `json:"id,omitempty"` Name string `json:"name,omitempty"` - // A secret string that could be used for signing or other purposes + // Secret is a secret that could be used for signing or other purposes. It + // should never be directly exposed to external services Secret string `json:"secret,omitempty"` Parent Parent `json:"parent,omitempty"` @@ -263,6 +265,10 @@ type Project struct { SSHPrivateKey string `json:"ssh_private_key,omitempty"` // PEM Encoded private key SkipSSHHostKeyCheck bool `json:"skip_ssh_host_key_check,omitempty"` + + // Webhooksecret is the secret passed to git sources that support a + // secret/token for signing or verifying the webhook payload + WebhookSecret string `json:"webhook_secret,omitempty"` } type SecretType string