Skip to content

Commit

Permalink
Add "generic-hmac" Provider
Browse files Browse the repository at this point in the history
This commit adds the "generic-hmac" Provider type for authenticating
webhook requests coming from notification-controller. I extended the
`Forwarder` notifier to accept an optional key used for generating the
HMAC. If the key is nil or empty no HMAC header is generated and the
forwarder behaves as before. If it is provided an `X-Signature` HTTP
header is added to the request carrying the HMAC.

I transformed the `TestForwarder_Post` test into a table-driven test
so that we can use the same setup and testing code for testing HMAC
and non-HMAC forwarder instances.

refs #99

Signed-off-by: Max Jonas Werner <max@e13.dev>
  • Loading branch information
makkes committed Sep 30, 2022
1 parent d7c31de commit 70b804b
Show file tree
Hide file tree
Showing 6 changed files with 164 additions and 44 deletions.
3 changes: 2 additions & 1 deletion api/v1beta1/provider_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ const (
// ProviderSpec defines the desired state of Provider
type ProviderSpec struct {
// Type of provider
// +kubebuilder:validation:Enum=slack;discord;msteams;rocket;generic;github;gitlab;bitbucket;azuredevops;googlechat;webex;sentry;azureeventhub;telegram;lark;matrix;opsgenie;alertmanager;grafana;githubdispatch;
// +kubebuilder:validation:Enum=slack;discord;msteams;rocket;generic;generic-hmac;github;gitlab;bitbucket;azuredevops;googlechat;webex;sentry;azureeventhub;telegram;lark;matrix;opsgenie;alertmanager;grafana;githubdispatch;
// +required
Type string `json:"type"`

Expand Down Expand Up @@ -78,6 +78,7 @@ type ProviderSpec struct {

const (
GenericProvider string = "generic"
GenericHMACProvider string = "generic-hmac"
SlackProvider string = "slack"
GrafanaProvider string = "grafana"
DiscordProvider string = "discord"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@ spec:
- msteams
- rocket
- generic
- generic-hmac
- github
- gitlab
- bitbucket
Expand Down
83 changes: 66 additions & 17 deletions docs/spec/v1beta1/provider.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ Spec:
```go
type ProviderSpec struct {
// Type of provider
// +kubebuilder:validation:Enum=slack;discord;msteams;rocket;generic;github;gitlab;bitbucket;azuredevops;googlechat;webex;sentry;azureeventhub;telegram;lark;matrix;opsgenie;githubdispatch
// +kubebuilder:validation:Enum=slack;discord;msteams;rocket;generic;generic-hmac;github;gitlab;bitbucket;azuredevops;googlechat;webex;sentry;azureeventhub;telegram;lark;matrix;opsgenie;githubdispatch
// +required
Type string `json:"type"`

Expand Down Expand Up @@ -51,22 +51,23 @@ Notification providers:

| Provider | Type |
| --------------- | -------------- |
| Alertmanager | alertmanager |
| Azure Event Hub | azureeventhub |
| Discord | discord |
| Generic webhook | generic |
| GitHub dispatch | githubdispatch |
| Google Chat | googlechat |
| Grafana | grafana |
| Lark | lark |
| Matrix | matrix |
| Microsoft Teams | msteams |
| Opsgenie | opsgenie |
| Rocket | rocket |
| Sentry | sentry |
| Slack | slack |
| Telegram | telegram |
| WebEx | webex |
| Alertmanager | alertmanager |
| Azure Event Hub | azureeventhub |
| Discord | discord |
| Generic webhook | generic |
| Generic webhook with HMAC | generic-hmac |
| GitHub dispatch | githubdispatch |
| Google Chat | googlechat |
| Grafana | grafana |
| Lark | lark |
| Matrix | matrix |
| Microsoft Teams | msteams |
| Opsgenie | opsgenie |
| Rocket | rocket |
| Sentry | sentry |
| Slack | slack |
| Telegram | telegram |
| WebEx | webex |

Git commit status providers:

Expand Down Expand Up @@ -211,6 +212,54 @@ stringData:
X-Forwarded-Proto: https
```
### Generic webhook with HMAC
If you set the `.spec.type` of a `Provider` resource to `generic-hmac` then the HTTP request sent to the webhook will include the `X-Signature` HTTP header carrying the HMAC of the request body. This allows the webhook server to authenticate the request. The key used for the HMAC must be supplied in the `token` field of the Secret resource referenced in `.spec.secretRef`. The HTTP header value has the following format:

```
X-Signature: HASH_FUNC=HASH
```
`HASH_FUNC` denotes the Hash function used to generate the HMAC and currently defaults to `sha256` but may change in the future. You must make sure to take this value into account when verifying the HMAC. `HASH` is the hex-encoded HMAC value. The following Go code illustrates how the header is parsed and verified:
```go
func verifySignature(sig string, payload, key []byte) error {
sigHdr := strings.Split(sig, "=")
if len(shgHdr) != 2 {
return fmt.Errorf("invalid signature value")
}
var newF func() hash.Hash
switch sigHdr[0] {
case "sha224":
newF = sha256.New224
case "sha256":
newF = sha256.New
case "sha384":
newF = sha512.New384
case "sha512":
newF = sha512.New
default:
return fmt.Errorf("unsupported signature algorithm %q", sigHdr[0])
}
mac := hmac.New(newF, key)
if _, err := mac.Write(payload); err != nil {
return fmt.Errorf("error MAC'ing payload: %w", err)
}
sum := fmt.Sprintf("%x", mac.Sum(nil))
if sum != sigHdr[1] {
return fmt.Errorf("HMACs don't match: %#v != %#v", sum, sigHdr[1])
}
return nil
}
[...]
key := []byte("b1fad212fb1b87a56c79e5da48018650b85ab7cf")
if len(r.Header["X-Signature"]) > 0 {
if err := verifySignature(r.Header["X-Signature"][0], body, key); err != nil {
// handle signature verification failure here
}
}
```

### Self-signed certificates

The `certSecretRef` field names a secret with TLS certificate data. This is for the purpose
Expand Down
4 changes: 3 additions & 1 deletion internal/notifier/factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,9 @@ func (f Factory) Notifier(provider string) (Interface, error) {
var err error
switch provider {
case v1beta1.GenericProvider:
n, err = NewForwarder(f.URL, f.ProxyURL, f.Headers, f.CertPool)
n, err = NewForwarder(f.URL, f.ProxyURL, f.Headers, f.CertPool, nil)
case v1beta1.GenericHMACProvider:
n, err = NewForwarder(f.URL, f.ProxyURL, f.Headers, f.CertPool, []byte(f.Token))
case v1beta1.SlackProvider:
n, err = NewSlack(f.URL, f.ProxyURL, f.Token, f.CertPool, f.Username, f.Channel)
case v1beta1.DiscordProvider:
Expand Down
28 changes: 27 additions & 1 deletion internal/notifier/forwarder.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,10 @@ package notifier

import (
"context"
"crypto/hmac"
"crypto/sha256"
"crypto/x509"
"encoding/json"
"fmt"
"net/url"

Expand All @@ -38,23 +41,46 @@ type Forwarder struct {
ProxyURL string
Headers map[string]string
CertPool *x509.CertPool
HMACKey []byte
}

func NewForwarder(hookURL string, proxyURL string, headers map[string]string, certPool *x509.CertPool) (*Forwarder, error) {
func NewForwarder(hookURL string, proxyURL string, headers map[string]string, certPool *x509.CertPool, hmacKey []byte) (*Forwarder, error) {
if _, err := url.ParseRequestURI(hookURL); err != nil {
return nil, fmt.Errorf("invalid hook URL %s: %w", hookURL, err)
}

if hmacKey != nil && len(hmacKey) == 0 {
return nil, fmt.Errorf("HMAC key is empty")
}

return &Forwarder{
URL: hookURL,
ProxyURL: proxyURL,
Headers: headers,
CertPool: certPool,
HMACKey: hmacKey,
}, nil
}

func sign(payload, key []byte) string {
h := hmac.New(sha256.New, key)
h.Write(payload)
return fmt.Sprintf("%x", h.Sum(nil))
}

func (f *Forwarder) Post(ctx context.Context, event events.Event) error {
var sig string
if len(f.HMACKey) != 0 {
eventJSON, err := json.Marshal(event)
if err != nil {
return fmt.Errorf("failed marshalling event: %w", err)
}
sig = fmt.Sprintf("sha256=%s", sign(eventJSON, f.HMACKey))
}
err := postMessage(ctx, f.URL, f.ProxyURL, f.CertPool, event, func(req *retryablehttp.Request) {
if sig != "" {
req.Header.Set("X-Signature", sig)
}
req.Header.Set(NotificationHeader, event.ReportingController)
for key, val := range f.Headers {
req.Header.Set(key, val)
Expand Down
89 changes: 65 additions & 24 deletions internal/notifier/forwarder_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,42 +25,83 @@ import (
"net/http"
"net/http/httptest"
"testing"
"time"

fuzz "github.com/AdaLogics/go-fuzz-headers"
"github.com/fluxcd/pkg/runtime/events"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"

"github.com/stretchr/testify/require"
)

func TestForwarder_Post(t *testing.T) {
ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
b, err := io.ReadAll(r.Body)
require.NoError(t, err)

require.Equal(t, "source-controller", r.Header.Get("gotk-component"))
require.Equal(t, "token", r.Header.Get("Authorization"))
var payload = events.Event{}
err = json.Unmarshal(b, &payload)
require.NoError(t, err)
require.Equal(t, "webapp", payload.InvolvedObject.Name)
require.Equal(t, "metadata", payload.Metadata["test"])
}))
defer ts.Close()

headers := make(map[string]string)
headers["Authorization"] = "token"
forwarder, err := NewForwarder(ts.URL, "", headers, nil)
require.NoError(t, err)

err = forwarder.Post(context.TODO(), testEvent())
require.NoError(t, err)
tests := []struct {
name string
hmacKey []byte
hmacHeader string
}{
{
name: "happy path with nil HMAC key",
hmacKey: nil,
hmacHeader: "",
},
{
name: "happy path with empty HMAC key",
hmacKey: []byte{},
hmacHeader: "",
},
{
name: "happy path with HMAC key from empty string",
hmacKey: []byte(""),
hmacHeader: "",
},
{
name: "non-empty HMAC key adds signature header",
hmacKey: []byte("7152fed34dd6149a7c75a276c510da27cb6f82b0"),
hmacHeader: "sha256=65b018549b1254e7226d1c08f9567ee45bc9de0fc4e7b1a40253f9a018b08be7",
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
b, err := io.ReadAll(r.Body)
require.NoError(t, err)

require.Equal(t, "source-controller", r.Header.Get("gotk-component"))
require.Equal(t, "token", r.Header.Get("Authorization"))
if tt.hmacHeader == "" {
_, ok := r.Header["X-Signature"]
require.Equal(t, false, ok, "expected signature header to be absent but it was present")
} else {
require.Equal(t, tt.hmacHeader, r.Header.Get("X-Signature"))
}
var payload = events.Event{}
err = json.Unmarshal(b, &payload)
require.NoError(t, err)
require.Equal(t, "webapp", payload.InvolvedObject.Name)
require.Equal(t, "metadata", payload.Metadata["test"])
}))
defer ts.Close()

headers := make(map[string]string)
headers["Authorization"] = "token"
forwarder, err := NewForwarder(ts.URL, "", headers, nil, tt.hmacKey)
require.NoError(t, err)

ev := testEvent()
ev.Timestamp = metav1.NewTime(time.Unix(1664520029, 0))
err = forwarder.Post(context.TODO(), ev)
require.NoError(t, err)
})
}
}

func Fuzz_Forwarder(f *testing.F) {
f.Add("", []byte{}, []byte{})
f.Add("", []byte{}, []byte{}, []byte{})

f.Fuzz(func(t *testing.T,
urlSuffix string, seed, response []byte) {
urlSuffix string, seed, response, hmacKey []byte) {
ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.Write(response)
io.Copy(io.Discard, r.Body)
Expand All @@ -74,7 +115,7 @@ func Fuzz_Forwarder(f *testing.F) {
header := make(map[string]string)
_ = fuzz.NewConsumer(seed).FuzzMap(&header)

forwarder, err := NewForwarder(fmt.Sprintf("%s/%s", ts.URL, urlSuffix), "", header, &cert)
forwarder, err := NewForwarder(fmt.Sprintf("%s/%s", ts.URL, urlSuffix), "", header, &cert, hmacKey)
if err != nil {
return
}
Expand Down

0 comments on commit 70b804b

Please sign in to comment.