Skip to content

Commit

Permalink
Reapply "Add webhook secret bootstrap and verification for Gitlab" (#…
Browse files Browse the repository at this point in the history
…4506)

* Reapply "Add webhook secret bootstrap and verification for Gitlab (#4492)" (#4504)

This reverts commit a06e930.

* Handle embedded structurs in our config parsing

our config parsing sets the defaults and registers options in viper. It
did not handle the case of embedded structures (or squashed tags). This
adds that functionality.

Signed-off-by: Juan Antonio Osorio <[email protected]>

---------

Signed-off-by: Juan Antonio Osorio <[email protected]>
  • Loading branch information
JAORMX authored Sep 17, 2024
1 parent a06e930 commit ac573e7
Show file tree
Hide file tree
Showing 11 changed files with 327 additions and 27 deletions.
2 changes: 1 addition & 1 deletion cmd/dev/app/rule_type/rttst.go
Original file line number Diff line number Diff line change
Expand Up @@ -438,7 +438,7 @@ func getProvider(pstr string, token string, providerConfigFile string) (provifv1
}

// We may pass a "fake" webhook URL here as it is not used in the test
client, err := gitlab.New(credentials.NewGitLabTokenCredential(token), cfg, "fake")
client, err := gitlab.New(credentials.NewGitLabTokenCredential(token), cfg, "fake", "fake")
if err != nil {
return nil, fmt.Errorf("error instantiating gitlab provider: %w", err)
}
Expand Down
6 changes: 6 additions & 0 deletions internal/config/server/gitlab.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,12 @@ package server
type GitLabConfig struct {
OAuthClientConfig `mapstructure:",squash"`

// WebhookSecrets is the configuration for the GitLab webhook secrets
// setup and verification. This is used to verify incoming webhook requests
// from GitLab, as well as to generate the webhook URL for GitLab to send
// events to.
WebhookSecrets `mapstructure:",squash"`

// Scopes is the list of scopes to request from the GitLab OAuth provider
Scopes []string `mapstructure:"scopes"`
}
14 changes: 12 additions & 2 deletions internal/config/server/webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,20 @@ import (

// WebhookConfig is the configuration for our webhook capabilities
type WebhookConfig struct {
// WebhookSecrets is the configuration for the webhook secrets.
// This is embedded in the WebhookConfig so that the secrets can be
// used in the WebhookConfig, as the GitHub provider needs for now.
WebhookSecrets `mapstructure:",squash"`
// ExternalWebhookURL is the URL that we will send our webhook to
ExternalWebhookURL string `mapstructure:"external_webhook_url"`
// ExternalPingURL is the URL that we will send our ping to
ExternalPingURL string `mapstructure:"external_ping_url"`
}

// WebhookSecrets is the configuration for the webhook secrets. this is useful
// to import in whatever provider configuration that needs to use some webhook
// secrets.
type WebhookSecrets struct {
// WebhookSecret is the secret that we will use to sign our webhook
WebhookSecret string `mapstructure:"webhook_secret"`
// WebhookSecretFile is the location of the file containing the webhook secret
Expand All @@ -39,7 +49,7 @@ type WebhookConfig struct {

// GetPreviousWebhookSecrets retrieves the previous webhook secrets from a file specified in the WebhookConfig.
// It reads the contents of the file, splits the data by whitespace, and returns it as a slice of strings.
func (wc *WebhookConfig) GetPreviousWebhookSecrets() ([]string, error) {
func (wc *WebhookSecrets) GetPreviousWebhookSecrets() ([]string, error) {
data, err := os.ReadFile(wc.PreviousWebhookSecretFile)
if err != nil {
return nil, fmt.Errorf("failed to read previous webhook secrets from file: %w", err)
Expand All @@ -51,6 +61,6 @@ func (wc *WebhookConfig) GetPreviousWebhookSecrets() ([]string, error) {
}

// GetWebhookSecret returns the GitHub App's webhook secret
func (wc *WebhookConfig) GetWebhookSecret() (string, error) {
func (wc *WebhookSecrets) GetWebhookSecret() (string, error) {
return fileOrArg(wc.WebhookSecretFile, wc.WebhookSecret, "webhook secret")
}
13 changes: 12 additions & 1 deletion internal/config/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,18 @@ func SetViperStructDefaults(v *viper.Viper, prefix string, s any) {
// Error, need a tag
panic(fmt.Sprintf("Untagged config struct field %q", field.Name))
}
valueName := strings.ToLower(prefix + field.Tag.Get("mapstructure"))

var valueName string
// Check if the tag is "squash" and if so, don't add the field name to the prefix
if field.Tag.Get("mapstructure") == ",squash" {
if strings.HasSuffix(prefix, ".") {
valueName = strings.ToLower(prefix[:len(prefix)-1])
} else {
valueName = strings.ToLower(prefix)
}
} else {
valueName = strings.ToLower(prefix + field.Tag.Get("mapstructure"))
}
fieldType := field.Type

// Extract a default value the `default` struct tag
Expand Down
6 changes: 5 additions & 1 deletion internal/controlplane/fuzz_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,11 @@ func FuzzGitHubEventParsers(f *testing.F) {
}
defer os.Remove(whSecretFile.Name())

whConfig := &server.WebhookConfig{WebhookSecretFile: whSecretFile.Name()}
whConfig := &server.WebhookConfig{
WebhookSecrets: server.WebhookSecrets{
WebhookSecretFile: whSecretFile.Name(),
},
}

s := &Server{}
ctx := context.Background()
Expand Down
20 changes: 15 additions & 5 deletions internal/providers/gitlab/gitlab.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,11 +57,20 @@ type gitlabClient struct {
glcfg *minderv1.GitLabProviderConfig
webhookURL string
gitConfig config.GitConfig

// secret for the webhook. This is stored in the
// structure to allow efficient fetching.
currentWebhookSecret string
}

// New creates a new GitLab provider
// Note that the webhook URL should already contain the provider class in the path
func New(cred provifv1.GitLabCredential, cfg *minderv1.GitLabProviderConfig, webhookURL string) (*gitlabClient, error) {
func New(
cred provifv1.GitLabCredential,
cfg *minderv1.GitLabProviderConfig,
webhookURL string,
currentWebhookSecret string,
) (*gitlabClient, error) {
// TODO: We need a context here.
cli := oauth2.NewClient(context.Background(), cred.GetAsOAuth2TokenSource())

Expand All @@ -74,10 +83,11 @@ func New(cred provifv1.GitLabCredential, cfg *minderv1.GitLabProviderConfig, web
}

return &gitlabClient{
cred: cred,
cli: cli,
glcfg: cfg,
webhookURL: webhookURL,
cred: cred,
cli: cli,
glcfg: cfg,
webhookURL: webhookURL,
currentWebhookSecret: currentWebhookSecret,
// TODO: Add git config
}, nil
}
Expand Down
36 changes: 30 additions & 6 deletions internal/providers/gitlab/manager/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ import (
"net/url"
"slices"

"github.com/rs/zerolog"

"github.com/stacklok/minder/internal/config/server"
"github.com/stacklok/minder/internal/crypto"
"github.com/stacklok/minder/internal/db"
Expand All @@ -39,6 +41,12 @@ type providerClassManager struct {
glpcfg *server.GitLabConfig
webhookURL string
parentContext context.Context

// secrets for the webhook. These are stored in the
// structure to allow efficient fetching. Rotation
// requires a process restart.
currentWebhookSecret string
previousWebhookSecrets []string
}

// NewGitLabProviderClassManager creates a new provider class manager for the dockerhub provider
Expand All @@ -50,17 +58,33 @@ func NewGitLabProviderClassManager(
return nil, errors.New("webhook URL is required")
}

if cfg == nil {
return nil, errors.New("gitlab config is required")
}

webhookURL, err := url.JoinPath(webhookURLBase, url.PathEscape(string(db.ProviderClassGitlab)))
if err != nil {
return nil, fmt.Errorf("error joining webhook URL: %w", err)
}

whSecret, err := cfg.GetWebhookSecret()
if err != nil {
return nil, fmt.Errorf("error getting webhook secret: %w", err)
}

previousSecrets, err := cfg.GetPreviousWebhookSecrets()
if err != nil {
zerolog.Ctx(ctx).Error().Err(err).Msg("previous secrets not loaded")
}

return &providerClassManager{
store: store,
crypteng: crypteng,
glpcfg: cfg,
webhookURL: webhookURL,
parentContext: ctx,
store: store,
crypteng: crypteng,
glpcfg: cfg,
webhookURL: webhookURL,
parentContext: ctx,
currentWebhookSecret: whSecret,
previousWebhookSecrets: previousSecrets,
}, nil
}

Expand Down Expand Up @@ -91,7 +115,7 @@ func (g *providerClassManager) Build(ctx context.Context, config *db.Provider) (
return nil, fmt.Errorf("error parsing gitlab config: %w", err)
}

cli, err := gitlab.New(creds, cfg, g.webhookURL)
cli, err := gitlab.New(creds, cfg, g.webhookURL, g.currentWebhookSecret)
if err != nil {
return nil, fmt.Errorf("error creating gitlab client: %w", err)
}
Expand Down
59 changes: 57 additions & 2 deletions internal/providers/gitlab/manager/webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,16 +15,22 @@
package manager

import (
"errors"
"fmt"
"net/http"
"strings"

"github.com/google/uuid"
"github.com/rs/zerolog"

"github.com/stacklok/minder/internal/providers/gitlab/webhooksecret"
)

// GetWebhookHandler implements the ProviderManager interface
// Note that this is where the whole webhook handler is defined and
// will live.
func (m *providerClassManager) GetWebhookHandler() http.Handler {
return http.HandlerFunc(func(_ http.ResponseWriter, r *http.Request) {
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
l := zerolog.Ctx(m.parentContext).With().
Str("webhook", "gitlab").
Str("method", r.Method).
Expand All @@ -34,8 +40,57 @@ func (m *providerClassManager) GetWebhookHandler() http.Handler {
Str("content-type", r.Header.Get("Content-Type")).
Logger()

// TODO: Implement webhook handler
// Validate the webhook secret
if err := m.validateRequest(r); err != nil {
l.Error().Err(err).Msg("invalid webhook request")
http.Error(w, "invalid webhook request", http.StatusUnauthorized)
return
}

l.Debug().Msg("received webhook")
})
}

func (m *providerClassManager) validateRequest(r *http.Request) error {
// Validate the webhook secret
gltok := r.Header.Get("X-Gitlab-Token")
if gltok == "" {
return errors.New("missing X-Gitlab-Token header")
}

if err := m.validateToken(gltok, r); err != nil {
return fmt.Errorf("invalid X-Gitlab-Token header: %w", err)
}

return nil
}

// validateToken validates the incoming GitLab webhook token
// Validation takes the secret from the GitLab webhook configuration
// appens the last element of the path to the URL (which is unique per entity)
func (m *providerClassManager) validateToken(token string, req *http.Request) error {
// Extract the unique ID from the URL path
path := req.URL.Path
uniq := path[strings.LastIndex(path, "/")+1:]

// uniq must be a valid UUID
_, err := uuid.Parse(uniq)
if err != nil {
return errors.New("invalid unique ID")
}

// Generate the expected secret
if valid := webhooksecret.Verify(m.currentWebhookSecret, uniq, token); valid {
// If the secret is valid, we can return
return nil
}

// Check the previous secrets
for _, prev := range m.previousWebhookSecrets {
if valid := webhooksecret.Verify(prev, uniq, token); valid {
return nil
}
}

return errors.New("invalid webhook token")
}
27 changes: 18 additions & 9 deletions internal/providers/gitlab/registration.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import (
"github.com/xanzy/go-gitlab"

"github.com/stacklok/minder/internal/entities/properties"
"github.com/stacklok/minder/internal/providers/gitlab/webhooksecret"
"github.com/stacklok/minder/internal/util/ptr"
minderv1 "github.com/stacklok/minder/pkg/api/protobuf/go/minder/v1"
)
Expand Down Expand Up @@ -57,8 +58,11 @@ func (c *gitlabClient) RegisterEntity(

whprops, err := c.createWebhook(ctx, upstreamID)
if err != nil {
// There is already enough context in the error message
return nil, err
zerolog.Ctx(ctx).Error().
Str("upstreamID", upstreamID).
Str("provider-class", Class).
Err(err).Msg("failed to create webhook")
return nil, errors.New("failed to create webhook")
}

return props.Merge(whprops), nil
Expand Down Expand Up @@ -102,15 +106,20 @@ func (c *gitlabClient) createWebhook(ctx context.Context, upstreamID string) (*p
return nil, fmt.Errorf("failed to join URL path for webhook: %w", err)
}

sec, err := webhooksecret.New(c.currentWebhookSecret, hookUUID.String())
if err != nil {
return nil, fmt.Errorf("failed to create webhook secret: %w", err)
}

trve := ptr.Ptr(true)
hreq := &gitlab.AddProjectHookOptions{
URL: &webhookUniqueURL,
// TODO: Add secret
PushEvents: trve,
TagPushEvents: trve,
MergeRequestsEvents: trve,
ReleasesEvents: trve,
// TODO: Enable SSL verification
URL: &webhookUniqueURL,
Token: &sec,
PushEvents: trve,
TagPushEvents: trve,
MergeRequestsEvents: trve,
ReleasesEvents: trve,
EnableSSLVerification: trve,
}

hook, err := c.doCreateWebhook(ctx, createHookPath, hreq)
Expand Down
57 changes: 57 additions & 0 deletions internal/providers/gitlab/webhooksecret/whsecret.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
//
// Copyright 2024 Stacklok, Inc.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

// Package webhooksecret provides a way to generate and verify secrets for GitLab webhooks.
package webhooksecret

import (
sum "crypto/sha512"
"encoding/hex"
"errors"
"fmt"
)

var (
// ErrEmptyBaseOrUniq is returned when the base or uniq strings are empty.
ErrEmptyBaseOrUniq = errors.New("base or uniq strings are empty")
)

// New creates a new secret for usage in the gitlab webhook.
// The secret is generated by combining the base and uniq strings
// and then hashing the result.
func New(base string, uniq string) (string, error) {
if base == "" || uniq == "" {
return "", ErrEmptyBaseOrUniq
}

hash := sum.New()
_, err := hash.Write([]byte(base + uniq))
if err != nil {
return "", fmt.Errorf("failed to write secret: %w", err)
}

return hex.EncodeToString(hash.Sum(nil)), nil
}

// Verify checks if the given secret is valid for the given base and uniq strings.
func Verify(base string, uniq string, secret string) bool {
s, err := New(base, uniq)
if err != nil {
// If we can't generate the secret, we can't verify it.
return false
}

return s == secret
}
Loading

0 comments on commit ac573e7

Please sign in to comment.