Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

notification/webhook: all webhooks are signed by a new set of signing keys #4225

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
1 change: 1 addition & 0 deletions app/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,7 @@ type App struct {
SessionKeyring keyring.Keyring
APIKeyring keyring.Keyring
AuthLinkKeyring keyring.Keyring
WebhookKeyring keyring.Keyring

NonceStore *nonce.Store
LabelStore *label.Store
Expand Down
12 changes: 12 additions & 0 deletions app/initstores.go
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,18 @@ func (app *App) initStores(ctx context.Context) error {
return errors.Wrap(err, "init API keyring")
}

if app.WebhookKeyring == nil {
app.WebhookKeyring, err = keyring.NewDB(ctx, app.cfg.LegacyLogger, app.db, &keyring.Config{
Name: "webhook-signature-keys",
MaxOldKeys: 100,
Keys: app.cfg.EncryptionKeys,
})

if err != nil {
return errors.Wrap(err, "init webhook keyring")
}
}

if app.AuthLinkStore == nil {
app.AuthLinkStore, err = authlink.NewStore(ctx, app.db, app.AuthLinkKeyring)
}
Expand Down
2 changes: 1 addition & 1 deletion app/startup.go
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ func (app *App) startup(ctx context.Context) error {
app.DestRegistry.RegisterProvider(ctx, app.slackChan)
app.DestRegistry.RegisterProvider(ctx, app.slackChan.DMSender())
app.DestRegistry.RegisterProvider(ctx, app.slackChan.UserGroupSender())
app.DestRegistry.RegisterProvider(ctx, webhook.NewSender(ctx))
app.DestRegistry.RegisterProvider(ctx, webhook.NewSender(ctx, app.WebhookKeyring))
if app.cfg.StubNotifiers {
app.DestRegistry.StubNotifiers()
}
Expand Down
37 changes: 37 additions & 0 deletions keyring/store.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,11 +38,14 @@ func init() {

// A Keyring allows signing and verifying messages.
type Keyring interface {
CurrentPublicKey() (*ecdsa.PublicKey, error)
RotateKeys(ctx context.Context) error

Sign(p []byte) ([]byte, error)
Verify(p []byte, signature []byte) (valid, oldKey bool)

SignASN1(p []byte) ([]byte, error)

SignJWT(jwt.Claims) (string, error)
VerifyJWT(token string, c jwt.Claims, iss, aud string) (bool, error)

Expand Down Expand Up @@ -468,6 +471,20 @@ func (db *DB) refreshAndRotateKeys(ctx context.Context, forceRotation bool) erro
return nil
}

// CurrentPublicKey retrives the most recent public key in the DB
func (db *DB) CurrentPublicKey() (*ecdsa.PublicKey, error) {
db.mx.Lock()
defer db.mx.Unlock()

pubKey, ok := db.verificationKeys[byte(db.rotationCount%256)]

if !ok {
return nil, errors.New("public key unavailable")
}

return &pubKey, nil
}

func (db *DB) SignJWT(c jwt.Claims) (string, error) {
db.mx.RLock()
defer db.mx.RUnlock()
Expand Down Expand Up @@ -604,3 +621,23 @@ func (db *DB) Verify(p []byte, signature []byte) (valid, oldKey bool) {
oldKey = hdr.KeyIndex != byte(db.rotationCount) && hdr.KeyIndex != byte(db.rotationCount+1)
return true, oldKey
}

// SignASN1 will sign the message `p` and produce a signature in ASN1 format
// This function uses the SHA512/224 hashing algorithm to produce the digest for the signature
func (db *DB) SignASN1(p []byte) ([]byte, error) {
db.mx.RLock()
defer db.mx.RUnlock()

if db.signingKey == nil {
return nil, errors.New("signing key unavailable")
}

sum := sha512.Sum512_224(p)
output, err := ecdsa.SignASN1(rand.Reader, db.signingKey, sum[:])

if err != nil {
return nil, err
}

return output, nil
}
19 changes: 16 additions & 3 deletions notification/webhook/sender.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,10 @@ package webhook
import (
"bytes"
"context"
"encoding/base64"
"encoding/json"
"fmt"
"github.com/target/goalert/keyring"
"net/http"
"time"

Expand All @@ -13,7 +15,9 @@ import (
"github.com/target/goalert/notification/nfydest"
)

type Sender struct{}
type Sender struct {
signingKeyring keyring.Keyring
}

// POSTDataAlert represents fields in outgoing alert notification.
type POSTDataAlert struct {
Expand Down Expand Up @@ -83,8 +87,10 @@ type POSTDataTest struct {
Type string
}

func NewSender(ctx context.Context) *Sender {
return &Sender{}
func NewSender(ctx context.Context, keyring keyring.Keyring) *Sender {
return &Sender{
signingKeyring: keyring,
}
}

var _ nfydest.MessageSender = &Sender{}
Expand Down Expand Up @@ -155,6 +161,12 @@ func (s *Sender) SendMessage(ctx context.Context, msg notification.Message) (*no
return nil, err
}

signature, err := s.signingKeyring.SignASN1(data)
if err != nil {
return nil, err
}
signatureBase64 := base64.StdEncoding.EncodeToString(signature)

ctx, cancel := context.WithTimeout(ctx, time.Second*3)
defer cancel()

Expand All @@ -173,6 +185,7 @@ func (s *Sender) SendMessage(ctx context.Context, msg notification.Message) (*no
}

req.Header.Add("Content-Type", "application/json")
req.Header.Add("X-Webhook-Signature", signatureBase64)

_, err = http.DefaultClient.Do(req)
if err != nil {
Expand Down
101 changes: 101 additions & 0 deletions test/smoke/webhook_signing_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,101 @@
package smoke

import (
"context"
"crypto/ecdsa"
"crypto/sha512"
"encoding/base64"
"github.com/stretchr/testify/require"
"io"
"net/http"
"net/http/httptest"
"testing"
"time"

"github.com/stretchr/testify/assert"
"github.com/target/goalert/test/smoke/harness"
)

type WebhookTestingSign struct {
Body []byte
Signature string
}

func TestWebhookSigning(t *testing.T) {
t.Parallel()

ch := make(chan WebhookTestingSign, 1)

ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
data, err := io.ReadAll(r.Body)
if !assert.NoError(t, err) {
return
}

ch <- WebhookTestingSign{
Body: data,
Signature: r.Header.Get("X-Webhook-Signature"),
}
}))

defer ts.Close()

sql := `
insert into users (id, name, email)
values
({{uuid "user"}}, 'bob', 'joe');
insert into user_contact_methods (id, user_id, name, type, value)
values
({{uuid "cm1"}}, {{uuid "user"}}, 'personal', 'WEBHOOK', '` + ts.URL + `');

insert into user_notification_rules (user_id, contact_method_id, delay_minutes)
values
({{uuid "user"}}, {{uuid "cm1"}}, 0);

insert into escalation_policies (id, name)
values
({{uuid "eid"}}, 'esc policy');
insert into escalation_policy_steps (id, escalation_policy_id)
values
({{uuid "esid"}}, {{uuid "eid"}});
insert into escalation_policy_actions (escalation_policy_step_id, user_id)
values
({{uuid "esid"}}, {{uuid "user"}});

insert into services (id, escalation_policy_id, name, description)
values
({{uuid "sid"}}, {{uuid "eid"}}, 'service', 'testing');

insert into alerts (service_id, summary, details, status, dedup_key)
values
({{uuid "sid"}}, 'testing summary', 'testing details', 'triggered', 'auto:1:foo');
`

h := harness.NewHarness(t, sql, "webhook-user-contact-method-type")
defer h.Close()

timeout, cancellation := context.WithTimeout(context.Background(), 10*time.Second)

select {
case alert := <-ch:
cancellation()
// convert alert.Signature from base64 to byte slice
signatureBytes, err := base64.StdEncoding.DecodeString(alert.Signature)
require.NoError(t, err)

key, err := h.App().WebhookKeyring.CurrentPublicKey()
require.NoError(t, err)

// given a public key, this is how you'd validate the signature is valid
sum := sha512.Sum512_224(alert.Body)
valid := ecdsa.VerifyASN1(key, sum[:], signatureBytes)

if !assert.True(t, valid, "webhook signature invalid") {
return
}
case <-timeout.Done():
cancellation()
t.Fatal("webhook timeout")
}

}
Loading