Add optional inbound webhook signature verification (closes #67)
All checks were successful
check / check (push) Successful in 4m23s
All checks were successful
check / check (push) Successful in 4m23s
A receiver URL was a bare v4 UUID and nothing else: anyone who learned it could store events and, because inbound headers are forwarded to targets almost verbatim, choose what the downstream service received. Entrypoints gain an optional scheme/secret pair. GitHub's X-Hub-Signature-256 (HMAC-SHA256 hex over the raw body) and GitLab's X-Gitlab-Token (plain shared token) are supported; both compare with hmac.Equal. With nothing configured an entrypoint behaves exactly as before, which is also where every pre-existing row lands after AutoMigrate adds the columns. Verification runs after the capped body read and before the first write, so a rejected request leaves no event row, no delivery row and no delivery task. A configuration the receiver cannot apply — unknown scheme, or one half of the pair missing — is refused with a 500 rather than falling back to unverified. The secret is credential-bearing and is stored in the clear because HMAC needs the key itself. It is excluded from JSON, kept out of templates by a new handlers.EntrypointView projection, and absent from every log line including the rejection path. The UI sets and rotates it through one form that never renders the stored value.
This commit is contained in:
126
README.md
126
README.md
@@ -435,7 +435,92 @@ backups at rest and restrict who can read them.
|
||||
`events-*.db` today hands them live delivery destinations.
|
||||
- `webhooker.db` stores target config **unencrypted**, tracked at
|
||||
[issue #212](https://git.eeqj.de/sneak/webhooker/issues/212), next to
|
||||
the session encryption key and the Argon2id password hashes.
|
||||
the session encryption key and the Argon2id password hashes. It also
|
||||
holds each entrypoint's inbound signature secret in the clear, for
|
||||
the reason given under
|
||||
[Inbound Signature Verification](#inbound-signature-verification):
|
||||
HMAC verification needs the key itself, so it cannot be hashed.
|
||||
|
||||
## Inbound Signature Verification
|
||||
|
||||
A receiver URL is a bare v4 UUID in a path. That is unguessable, but it
|
||||
is not a credential: anyone who learns it — from a browser history, a
|
||||
proxy log, a screenshot, a copy-pasted support ticket — can post events
|
||||
that webhooker stores and forwards, and the inbound headers are passed
|
||||
on to your targets almost verbatim, so they also choose what the
|
||||
downstream service sees. Verification is how an entrypoint stops
|
||||
accepting anything that reaches its URL.
|
||||
|
||||
It is optional and configured per entrypoint. An entrypoint with no
|
||||
scheme selected is not verified, which is what every entrypoint was
|
||||
before this existed and what every entrypoint remains after an
|
||||
upgrade — enabling verification is always a deliberate act, and no
|
||||
existing deployment is locked out of its own receivers by installing a
|
||||
new version.
|
||||
|
||||
When a scheme **is** selected, a request whose signature is missing,
|
||||
malformed or wrong is answered `401` and **nothing is stored**: no
|
||||
event row, no delivery row, no delivery attempt. Rejection happens
|
||||
after the body is read (the signature covers it) and before the first
|
||||
write.
|
||||
|
||||
### Supported schemes
|
||||
|
||||
| Scheme | Header | Check |
|
||||
| -------- | --------------------- | ----- |
|
||||
| `github` | `X-Hub-Signature-256` | HMAC-SHA256 of the raw request body under the shared secret, hex-encoded, prefixed `sha256=` |
|
||||
| `gitlab` | `X-Gitlab-Token` | The header is the shared secret itself, compared as-is |
|
||||
|
||||
Both comparisons run in constant time (`hmac.Equal`). The HMAC is
|
||||
computed over the request body exactly as received, before any parsing,
|
||||
and under the same 1 MB body cap every other request obeys — an
|
||||
unsigned sender cannot make webhooker buffer more than a signed one.
|
||||
|
||||
GitHub's older SHA-1 `X-Hub-Signature` is **not** accepted. Neither is
|
||||
a GitHub digest sent without its `sha256=` prefix.
|
||||
|
||||
### Configuring a sender
|
||||
|
||||
The secret is a value you choose and enter in two places: at the sender
|
||||
and in webhooker. webhooker never generates or displays one, so there
|
||||
is no stored credential the UI can be made to reveal.
|
||||
|
||||
1. Generate a secret, e.g. `openssl rand -hex 32`.
|
||||
2. In webhooker, open the webhook's page, find the entrypoint, and
|
||||
click **Configure** (or **Rotate**, if it already has one). Select
|
||||
the scheme and paste the secret. Surrounding whitespace is stripped,
|
||||
so a value pasted with a trailing space still works; a secret whose
|
||||
own first or last character is a space cannot be stored.
|
||||
3. At the sender:
|
||||
- **GitHub** — repository (or organization) → Settings → Webhooks →
|
||||
the hook → **Secret**. GitHub then signs every delivery with
|
||||
`X-Hub-Signature-256`.
|
||||
- **GitLab** — project → Settings → Webhooks → the hook → **Secret
|
||||
token**. GitLab sends it verbatim as `X-Gitlab-Token`.
|
||||
|
||||
**Rotation** is the same form: submit the new secret. Deliveries signed
|
||||
with the old secret are rejected from that moment, so change it at the
|
||||
sender in the same sitting. Selecting **None** removes verification and
|
||||
deletes the stored secret with it.
|
||||
|
||||
The page shows which scheme an entrypoint uses and which header it
|
||||
reads, never the secret. The value is stored in the clear — HMAC
|
||||
verification needs the key itself, and a hash of it cannot recompute a
|
||||
sender's digest — so it is handled like the other credentials
|
||||
webhooker holds: excluded from JSON, kept out of templates by a
|
||||
projection (`handlers.EntrypointView`), and absent from every log line,
|
||||
including the ones written when verification fails.
|
||||
|
||||
### When configuration is broken
|
||||
|
||||
An entrypoint whose stored scheme this build does not recognise, or
|
||||
which has one half of the scheme/secret pair and not the other, is
|
||||
answered `500` and stores nothing. It is not treated as unverified. The
|
||||
UI cannot create such a row — it rejects an unknown scheme with a `400`
|
||||
— so this covers a hand-edited database or a downgrade to a build that
|
||||
predates a scheme. Failing closed is the point: an entrypoint the
|
||||
operator believes is protected must never quietly go back to accepting
|
||||
anything.
|
||||
|
||||
## Entrypoints
|
||||
|
||||
@@ -712,9 +797,15 @@ the full request and creates an Event.
|
||||
| `path` | string | Unique bare UUID, generated at creation. The `/webhook/` prefix is route only and is not stored: the receiver matches this column against the raw `{uuid}` path segment |
|
||||
| `description` | string | Optional description |
|
||||
| `active` | boolean | Whether this entrypoint accepts events (default: true) |
|
||||
| `signature_scheme` | string | How inbound requests are authenticated: `github`, `gitlab`, or empty for no verification (default: empty). See [Inbound Signature Verification](#inbound-signature-verification) |
|
||||
| `signature_secret` | string | The secret shared with the sender, stored in the clear because HMAC verification needs the key itself. Never marshalled to JSON, never rendered, never logged. Empty when no scheme is set |
|
||||
|
||||
**Relations:** Belongs to Webhook.
|
||||
|
||||
Both signature columns arrive through `AutoMigrate` with an empty
|
||||
default, so every entrypoint written before they existed migrates to
|
||||
"not configured" and keeps accepting the traffic it already accepted.
|
||||
|
||||
A webhook can have multiple entrypoints. This allows separate URLs for
|
||||
different event sources that all feed into the same processing pipeline
|
||||
(e.g., one entrypoint for GitHub, another for Stripe, both routing to
|
||||
@@ -984,12 +1075,16 @@ External Service
|
||||
└─────────────┘ └──────────────┘ └──────┬───────┘
|
||||
│
|
||||
1. Look up Entrypoint by UUID
|
||||
2. Capture full request as Event
|
||||
3. Create Delivery records for each active Target
|
||||
4. Build self-contained delivery.Task structs
|
||||
2. Read the body under the 1 MB cap
|
||||
3. Verify the signature, if the entrypoint has
|
||||
one configured — 401 and no writes if it
|
||||
fails (see Inbound Signature Verification)
|
||||
4. Capture full request as Event
|
||||
5. Create Delivery records for each active Target
|
||||
6. Build self-contained delivery.Task structs
|
||||
(target config + event data inline for
|
||||
bodies < 16 KiB)
|
||||
5. Notify Engine via channel (no DB read needed)
|
||||
7. Notify Engine via channel (no DB read needed)
|
||||
│
|
||||
▼
|
||||
┌──────────────┐
|
||||
@@ -1686,6 +1781,7 @@ abuse limit later; they are tracked as future work.
|
||||
| `POST` | `/source/{id}/entrypoints` | Add entrypoint to webhook |
|
||||
| `POST` | `/source/{id}/entrypoints/{entrypointID}/delete` | Delete an entrypoint |
|
||||
| `POST` | `/source/{id}/entrypoints/{entrypointID}/toggle` | Enable or disable an entrypoint |
|
||||
| `POST` | `/source/{id}/entrypoints/{entrypointID}/secret` | Set, rotate or remove the entrypoint's inbound signature scheme and secret (see [Inbound Signature Verification](#inbound-signature-verification)) |
|
||||
| `POST` | `/source/{id}/targets` | Add target to webhook |
|
||||
| `POST` | `/source/{id}/targets/{targetID}/delete` | Delete a target |
|
||||
| `POST` | `/source/{id}/targets/{targetID}/toggle` | Enable or disable a target |
|
||||
@@ -1732,7 +1828,7 @@ webhooker/
|
||||
│ │ ├── model_setting.go # Setting entity (key-value app config)
|
||||
│ │ ├── model_user.go # User entity
|
||||
│ │ ├── model_webhook.go # Webhook entity
|
||||
│ │ ├── model_entrypoint.go # Entrypoint entity
|
||||
│ │ ├── model_entrypoint.go # Entrypoint entity and SignatureScheme enum
|
||||
│ │ ├── model_target.go # Target entity and TargetType enum
|
||||
│ │ ├── model_event.go # Event entity (per-webhook DB)
|
||||
│ │ ├── model_delivery.go # Delivery entity (per-webhook DB)
|
||||
@@ -1764,6 +1860,7 @@ webhooker/
|
||||
│ ├── handlers/
|
||||
│ │ ├── handlers.go # Base handler struct, JSON helpers, template rendering
|
||||
│ │ ├── auth.go # Login, logout handlers
|
||||
│ │ ├── entrypoint_view.go # Masked entrypoint view for templates
|
||||
│ │ ├── event_log_view.go # Event log projection, byte-capped in SQL
|
||||
│ │ ├── healthcheck.go # Health check handler
|
||||
│ │ ├── index.go # Index page handler
|
||||
@@ -1786,9 +1883,11 @@ webhooker/
|
||||
│ │ ├── server.go # Server struct, fx lifecycle, signal handling
|
||||
│ │ ├── http.go # HTTP server setup with timeouts
|
||||
│ │ └── routes.go # All route definitions
|
||||
│ └── session/
|
||||
│ ├── session.go # Cookie-based session management
|
||||
│ └── testing.go # NewForTest: Session without the fx lifecycle
|
||||
│ ├── session/
|
||||
│ │ ├── session.go # Cookie-based session management
|
||||
│ │ └── testing.go # NewForTest: Session without the fx lifecycle
|
||||
│ └── signature/
|
||||
│ └── signature.go # Inbound signature verification (GitHub, GitLab)
|
||||
├── static/
|
||||
│ ├── static.go # //go:embed directive
|
||||
│ ├── css/input.css # Tailwind input, source for tailwind.css (make css)
|
||||
@@ -1930,6 +2029,15 @@ check, see [The login endpoint](#the-login-endpoint).
|
||||
`/api` (stateless API). The middleware auto-detects TLS status
|
||||
per-request (via `r.TLS` and `X-Forwarded-Proto`) to set appropriate
|
||||
cookie security flags and Origin/Referer validation mode
|
||||
- **Optional inbound signature verification** per entrypoint (GitHub
|
||||
`X-Hub-Signature-256`, GitLab `X-Gitlab-Token`). Off by default and
|
||||
off after an upgrade, so behaviour is unchanged until an operator
|
||||
turns it on. Where it is on, an unsigned or wrongly signed request
|
||||
is `401` and is not persisted, and a configuration the receiver
|
||||
cannot apply fails closed rather than reverting to unverified. The
|
||||
comparison is constant time and the secret never reaches a template,
|
||||
a JSON response or a log line (see
|
||||
[Inbound Signature Verification](#inbound-signature-verification))
|
||||
- **SSRF prevention** for HTTP delivery targets: private/reserved IP
|
||||
ranges (RFC 1918, loopback, link-local, cloud metadata) are blocked
|
||||
both at target creation time (URL validation) and at delivery time
|
||||
|
||||
85
internal/database/migration_entrypoint_test.go
Normal file
85
internal/database/migration_entrypoint_test.go
Normal file
@@ -0,0 +1,85 @@
|
||||
package database_test
|
||||
|
||||
import (
|
||||
"net/http"
|
||||
"testing"
|
||||
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
"sneak.berlin/go/webhooker/internal/database"
|
||||
"sneak.berlin/go/webhooker/internal/signature"
|
||||
)
|
||||
|
||||
// TestEntrypointSignatureColumnsMigrateToUnconfigured pins the
|
||||
// upgrade path for a deployment that already has entrypoints.
|
||||
//
|
||||
// The signature columns arrive through GORM's AutoMigrate, so every
|
||||
// row written before they existed acquires them with no value. That
|
||||
// has to land on "not configured", because the alternative is an
|
||||
// upgrade that rejects the traffic the operator was already
|
||||
// receiving — a self-inflicted outage on a receiver whose senders
|
||||
// cannot be told to start signing.
|
||||
//
|
||||
// The legacy schema is reproduced by dropping the columns from a
|
||||
// migrated database and writing a row through the old shape, so the
|
||||
// row really predates them rather than merely being blank.
|
||||
func TestEntrypointSignatureColumnsMigrateToUnconfigured(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
db, lc := setupTestDB(t)
|
||||
lc.RequireStart()
|
||||
|
||||
t.Cleanup(lc.RequireStop)
|
||||
|
||||
for _, column := range []string{
|
||||
"signature_scheme", "signature_secret",
|
||||
} {
|
||||
require.NoError(
|
||||
t,
|
||||
db.DB().Exec(
|
||||
"ALTER TABLE entrypoints DROP COLUMN "+column,
|
||||
).Error,
|
||||
"dropping %s to reproduce the pre-upgrade schema",
|
||||
column,
|
||||
)
|
||||
}
|
||||
|
||||
const legacyID = "legacy-entrypoint"
|
||||
|
||||
require.NoError(
|
||||
t,
|
||||
db.DB().Exec(
|
||||
`INSERT INTO entrypoints
|
||||
(id, created_at, updated_at, webhook_id, path,
|
||||
description, active)
|
||||
VALUES (?, ?, ?, ?, ?, ?, ?)`,
|
||||
legacyID, "2026-01-01 00:00:00", "2026-01-01 00:00:00",
|
||||
"legacy-webhook", "legacy-path", "predates signatures",
|
||||
true,
|
||||
).Error,
|
||||
)
|
||||
|
||||
// The upgrade.
|
||||
require.NoError(t, db.Migrate())
|
||||
|
||||
var ep database.Entrypoint
|
||||
|
||||
require.NoError(
|
||||
t,
|
||||
db.DB().Where("id = ?", legacyID).First(&ep).Error,
|
||||
"the migrated row must still load; a NULL landing in a "+
|
||||
"string column would fail here",
|
||||
)
|
||||
|
||||
assert.Equal(t, database.SignatureSchemeNone, ep.SignatureScheme)
|
||||
assert.Empty(t, ep.SignatureSecret)
|
||||
assert.False(t, ep.SignatureConfigured())
|
||||
assert.True(t, ep.Active, "the row's other columns survive")
|
||||
|
||||
// The behaviour that actually matters: an unsigned request to
|
||||
// this entrypoint is still accepted.
|
||||
assert.NoError(
|
||||
t,
|
||||
signature.Verify(&ep, http.Header{}, []byte(`{"a":1}`)),
|
||||
)
|
||||
}
|
||||
@@ -1,5 +1,22 @@
|
||||
package database
|
||||
|
||||
// SignatureScheme names the way an entrypoint authenticates inbound
|
||||
// requests. A scheme fixes both the header the signature arrives in
|
||||
// and the algorithm used to check it, so an operator cannot pair one
|
||||
// sender's header with another sender's comparison.
|
||||
type SignatureScheme string
|
||||
|
||||
// Signature scheme values. The empty scheme means the entrypoint
|
||||
// performs no inbound verification: it is the default, and it is the
|
||||
// state every entrypoint created before this column existed migrates
|
||||
// to, so an existing deployment keeps accepting the requests it
|
||||
// accepted before.
|
||||
const (
|
||||
SignatureSchemeNone SignatureScheme = ""
|
||||
SignatureSchemeGitHub SignatureScheme = "github"
|
||||
SignatureSchemeGitLab SignatureScheme = "gitlab"
|
||||
)
|
||||
|
||||
// Entrypoint represents an inbound URL endpoint that feeds into a webhook
|
||||
type Entrypoint struct {
|
||||
BaseModel
|
||||
@@ -12,6 +29,31 @@ type Entrypoint struct {
|
||||
Description string `json:"description"`
|
||||
Active bool `gorm:"default:true" json:"active"`
|
||||
|
||||
// SignatureScheme selects how inbound requests to this
|
||||
// entrypoint are authenticated. Empty means unauthenticated,
|
||||
// which is what a UUID-only entrypoint has always been.
|
||||
SignatureScheme SignatureScheme `gorm:"default:''" json:"signatureScheme"`
|
||||
|
||||
// SignatureSecret is the secret shared with the sender.
|
||||
//
|
||||
// It is stored in the clear because HMAC verification needs the
|
||||
// key itself: a hash of it cannot recompute the sender's digest.
|
||||
// It is therefore a live credential, and json:"-" keeps it out of
|
||||
// any handler that marshals the model, the way APIKey.Key and
|
||||
// Target.Config are kept out. handlers.EntrypointView is the
|
||||
// matching barrier for the HTML path.
|
||||
SignatureSecret string `gorm:"default:''" json:"-"`
|
||||
|
||||
// Relations
|
||||
Webhook Webhook `json:"webhook,omitzero"`
|
||||
}
|
||||
|
||||
// SignatureConfigured reports whether this entrypoint verifies
|
||||
// inbound requests. Both halves must be present: a scheme without a
|
||||
// secret, or a secret without a scheme, is a broken configuration
|
||||
// rather than a configured one, and signature.Verify fails those
|
||||
// closed rather than treating them as "off".
|
||||
func (e *Entrypoint) SignatureConfigured() bool {
|
||||
return e.SignatureScheme != SignatureSchemeNone &&
|
||||
e.SignatureSecret != ""
|
||||
}
|
||||
|
||||
@@ -34,6 +34,8 @@ func marshalModel(t *testing.T, v any) string {
|
||||
// - APIKey.Key is a bearer token outright.
|
||||
// - Setting.Value holds the session encryption key.
|
||||
// - User.Password holds the Argon2 hash, and was already tagged.
|
||||
// - Entrypoint.SignatureSecret is the secret its senders sign with,
|
||||
// stored in the clear because HMAC verification needs the key.
|
||||
func TestModelsDoNotMarshalTheirSecrets(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
@@ -72,6 +74,14 @@ func TestModelsDoNotMarshalTheirSecrets(t *testing.T) {
|
||||
Password: marker,
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "entrypoint signature secret",
|
||||
model: database.Entrypoint{
|
||||
Description: keptField,
|
||||
SignatureScheme: database.SignatureSchemeGitHub,
|
||||
SignatureSecret: marker,
|
||||
},
|
||||
},
|
||||
}
|
||||
|
||||
for _, tc := range cases {
|
||||
@@ -105,3 +115,24 @@ func TestWebhookMarshalsNoTargetConfig(t *testing.T) {
|
||||
assert.NotContains(t, encoded, marker)
|
||||
assert.Contains(t, encoded, keptField)
|
||||
}
|
||||
|
||||
// TestWebhookMarshalsNoEntrypointSecret covers the same nested case
|
||||
// for the entrypoint's inbound signature secret, which reaches a
|
||||
// marshalled webhook through the Entrypoints association.
|
||||
func TestWebhookMarshalsNoEntrypointSecret(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
const marker = "QQENTRYPOINTMARKERQQ"
|
||||
|
||||
encoded := marshalModel(t, database.Webhook{
|
||||
Name: keptField,
|
||||
Entrypoints: []database.Entrypoint{{
|
||||
Path: "some-uuid",
|
||||
SignatureScheme: database.SignatureSchemeGitLab,
|
||||
SignatureSecret: marker,
|
||||
}},
|
||||
})
|
||||
|
||||
assert.NotContains(t, encoded, marker)
|
||||
assert.Contains(t, encoded, keptField)
|
||||
}
|
||||
|
||||
325
internal/handlers/entrypoint_secret_test.go
Normal file
325
internal/handlers/entrypoint_secret_test.go
Normal file
@@ -0,0 +1,325 @@
|
||||
package handlers_test
|
||||
|
||||
import (
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"net/url"
|
||||
"testing"
|
||||
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
"sneak.berlin/go/webhooker/internal/database"
|
||||
"sneak.berlin/go/webhooker/internal/handlers"
|
||||
"sneak.berlin/go/webhooker/internal/session"
|
||||
)
|
||||
|
||||
// submitEntrypointSecret posts the signature configuration form for
|
||||
// an entrypoint and returns the recorder.
|
||||
func submitEntrypointSecret(
|
||||
t *testing.T,
|
||||
h *handlers.Handlers,
|
||||
cookies []*http.Cookie,
|
||||
webhookID, entrypointID, scheme, secret string,
|
||||
) *httptest.ResponseRecorder {
|
||||
t.Helper()
|
||||
|
||||
form := url.Values{}
|
||||
form.Set("signature_scheme", scheme)
|
||||
form.Set("secret", secret)
|
||||
|
||||
req := formRequest(
|
||||
"/source/"+webhookID+"/entrypoints/"+
|
||||
entrypointID+"/secret",
|
||||
cookies,
|
||||
form,
|
||||
map[string]string{
|
||||
paramSourceID: webhookID,
|
||||
entrypointIDParam: entrypointID,
|
||||
},
|
||||
)
|
||||
w := httptest.NewRecorder()
|
||||
|
||||
h.HandleEntrypointSecret().ServeHTTP(w, req)
|
||||
|
||||
return w
|
||||
}
|
||||
|
||||
// reloadEntrypoint reads an entrypoint back from the database,
|
||||
// including the columns the model keeps out of JSON.
|
||||
func reloadEntrypoint(
|
||||
t *testing.T,
|
||||
db *database.Database,
|
||||
id string,
|
||||
) database.Entrypoint {
|
||||
t.Helper()
|
||||
|
||||
var ep database.Entrypoint
|
||||
|
||||
require.NoError(
|
||||
t, db.DB().Where("id = ?", id).First(&ep).Error,
|
||||
)
|
||||
|
||||
return ep
|
||||
}
|
||||
|
||||
// TestEntrypointSecretSetRotateAndRemove walks the whole lifecycle
|
||||
// the UI has to support: turning verification on, rotating the secret
|
||||
// to a new value, and turning it back off.
|
||||
func TestEntrypointSecretSetRotateAndRemove(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
var (
|
||||
h *handlers.Handlers
|
||||
sess *session.Session
|
||||
db *database.Database
|
||||
)
|
||||
|
||||
app := newTestApp(t, &h, &sess, &db)
|
||||
app.RequireStart()
|
||||
|
||||
t.Cleanup(app.RequireStop)
|
||||
|
||||
cookies := authenticatedCookies(
|
||||
t, sess, deleteTestUserID, deleteTestUsername,
|
||||
)
|
||||
wh := seedWebhook(t, db)
|
||||
ep := seedSignedEntrypoint(
|
||||
t, db, wh.ID, database.SignatureSchemeNone, "",
|
||||
)
|
||||
|
||||
// Set.
|
||||
w := submitEntrypointSecret(
|
||||
t, h, cookies, wh.ID, ep.ID, "github", inboundSecret,
|
||||
)
|
||||
require.Equal(t, http.StatusSeeOther, w.Code)
|
||||
|
||||
stored := reloadEntrypoint(t, db, ep.ID)
|
||||
assert.Equal(
|
||||
t, database.SignatureSchemeGitHub, stored.SignatureScheme,
|
||||
)
|
||||
assert.Equal(t, inboundSecret, stored.SignatureSecret)
|
||||
assert.True(t, stored.SignatureConfigured())
|
||||
|
||||
// Rotate: a new secret and a different scheme in one submission.
|
||||
// The new value is submitted with surrounding whitespace, the way
|
||||
// a secret pasted out of a password manager arrives; storing that
|
||||
// verbatim would make every later request fail verification with
|
||||
// nothing visible on either side to explain it.
|
||||
const rotated = "QQROTATEDSECRETQQ"
|
||||
|
||||
w = submitEntrypointSecret(
|
||||
t, h, cookies, wh.ID, ep.ID, "gitlab", " "+rotated+"\t",
|
||||
)
|
||||
require.Equal(t, http.StatusSeeOther, w.Code)
|
||||
|
||||
stored = reloadEntrypoint(t, db, ep.ID)
|
||||
assert.Equal(
|
||||
t, database.SignatureSchemeGitLab, stored.SignatureScheme,
|
||||
)
|
||||
assert.Equal(t, rotated, stored.SignatureSecret)
|
||||
|
||||
// Remove. The secret has to go with the scheme: a stored
|
||||
// credential nothing reads is one more copy to leak.
|
||||
w = submitEntrypointSecret(t, h, cookies, wh.ID, ep.ID, "", "")
|
||||
require.Equal(t, http.StatusSeeOther, w.Code)
|
||||
|
||||
stored = reloadEntrypoint(t, db, ep.ID)
|
||||
assert.Equal(
|
||||
t, database.SignatureSchemeNone, stored.SignatureScheme,
|
||||
)
|
||||
assert.Empty(t, stored.SignatureSecret)
|
||||
assert.False(t, stored.SignatureConfigured())
|
||||
}
|
||||
|
||||
// TestEntrypointSecretRejectsBadInput proves the form cannot create a
|
||||
// row the receiver would later have to refuse. Both rejections leave
|
||||
// the stored configuration untouched rather than half-applied.
|
||||
func TestEntrypointSecretRejectsBadInput(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
cases := []struct {
|
||||
name string
|
||||
scheme string
|
||||
secret string
|
||||
}{
|
||||
{
|
||||
name: "unsupported scheme",
|
||||
scheme: "stripe",
|
||||
secret: inboundSecret,
|
||||
},
|
||||
{
|
||||
name: "scheme with no secret",
|
||||
scheme: "github",
|
||||
secret: "",
|
||||
},
|
||||
{
|
||||
// Whitespace is stripped, so a secret of spaces is an
|
||||
// empty one.
|
||||
name: "scheme with blank secret",
|
||||
scheme: "github",
|
||||
secret: " ",
|
||||
},
|
||||
}
|
||||
|
||||
var (
|
||||
h *handlers.Handlers
|
||||
sess *session.Session
|
||||
db *database.Database
|
||||
)
|
||||
|
||||
app := newTestApp(t, &h, &sess, &db)
|
||||
app.RequireStart()
|
||||
|
||||
t.Cleanup(app.RequireStop)
|
||||
|
||||
cookies := authenticatedCookies(
|
||||
t, sess, deleteTestUserID, deleteTestUsername,
|
||||
)
|
||||
|
||||
for _, tc := range cases {
|
||||
wh := seedWebhook(t, db)
|
||||
ep := seedSignedEntrypoint(
|
||||
t, db, wh.ID,
|
||||
database.SignatureSchemeGitLab, inboundSecret,
|
||||
)
|
||||
|
||||
w := submitEntrypointSecret(
|
||||
t, h, cookies, wh.ID, ep.ID, tc.scheme, tc.secret,
|
||||
)
|
||||
|
||||
assert.Equal(
|
||||
t, http.StatusBadRequest, w.Code, "case %s", tc.name,
|
||||
)
|
||||
|
||||
stored := reloadEntrypoint(t, db, ep.ID)
|
||||
assert.Equal(
|
||||
t,
|
||||
database.SignatureSchemeGitLab,
|
||||
stored.SignatureScheme,
|
||||
"case %s", tc.name,
|
||||
)
|
||||
assert.Equal(
|
||||
t, inboundSecret, stored.SignatureSecret,
|
||||
"case %s", tc.name,
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
// TestEntrypointSecretRequiresOwnership proves the configuration
|
||||
// endpoint is bound by the same ownership check as the rest of the
|
||||
// webhook's pages: another user's entrypoint is a 404, and the secret
|
||||
// is not touched.
|
||||
func TestEntrypointSecretRequiresOwnership(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
var (
|
||||
h *handlers.Handlers
|
||||
sess *session.Session
|
||||
db *database.Database
|
||||
)
|
||||
|
||||
app := newTestApp(t, &h, &sess, &db)
|
||||
app.RequireStart()
|
||||
|
||||
t.Cleanup(app.RequireStop)
|
||||
|
||||
wh := seedWebhook(t, db)
|
||||
ep := seedSignedEntrypoint(
|
||||
t, db, wh.ID,
|
||||
database.SignatureSchemeGitLab, inboundSecret,
|
||||
)
|
||||
|
||||
stranger := authenticatedCookies(
|
||||
t, sess, "someone-else", "someoneelse",
|
||||
)
|
||||
|
||||
w := submitEntrypointSecret(
|
||||
t, h, stranger, wh.ID, ep.ID, "github", "hijacked",
|
||||
)
|
||||
|
||||
assert.Equal(t, http.StatusNotFound, w.Code)
|
||||
assert.Equal(
|
||||
t,
|
||||
inboundSecret,
|
||||
reloadEntrypoint(t, db, ep.ID).SignatureSecret,
|
||||
)
|
||||
}
|
||||
|
||||
// TestHandleSourceDetail_MasksEntrypointSecret is the regression test
|
||||
// for the credential on the entrypoint: the page has to say that
|
||||
// verification is configured and which header carries it, without the
|
||||
// secret itself ever reaching the rendered HTML.
|
||||
func TestHandleSourceDetail_MasksEntrypointSecret(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
var (
|
||||
h *handlers.Handlers
|
||||
sess *session.Session
|
||||
db *database.Database
|
||||
)
|
||||
|
||||
app := newTestApp(t, &h, &sess, &db)
|
||||
app.RequireStart()
|
||||
|
||||
t.Cleanup(app.RequireStop)
|
||||
|
||||
wh := seedWebhook(t, db)
|
||||
seedSignedEntrypoint(
|
||||
t, db, wh.ID,
|
||||
database.SignatureSchemeGitHub, inboundSecret,
|
||||
)
|
||||
|
||||
body := renderSourceDetailPage(t, h, sess, wh.ID)
|
||||
|
||||
assert.NotContains(t, body, inboundSecret)
|
||||
assert.Contains(t, body, "GitHub")
|
||||
assert.Contains(t, body, "X-Hub-Signature-256")
|
||||
}
|
||||
|
||||
// TestEntrypointViewsDropTheSecret pins the projection itself, so the
|
||||
// barrier survives a template rewrite that stops rendering the field
|
||||
// the page test above looks at.
|
||||
func TestEntrypointViewsDropTheSecret(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
views := handlers.NewEntrypointViews([]database.Entrypoint{
|
||||
{
|
||||
Path: "p1",
|
||||
Active: true,
|
||||
SignatureScheme: database.SignatureSchemeGitHub,
|
||||
SignatureSecret: inboundSecret,
|
||||
},
|
||||
{
|
||||
Path: "p2",
|
||||
},
|
||||
{
|
||||
// A scheme this build does not know: described as
|
||||
// unavailable, never echoed back.
|
||||
Path: "p3",
|
||||
SignatureScheme: database.SignatureScheme("stripe"),
|
||||
SignatureSecret: inboundSecret,
|
||||
},
|
||||
})
|
||||
|
||||
require.Len(t, views, 3)
|
||||
|
||||
assert.True(t, views[0].Configured)
|
||||
assert.Equal(t, "GitHub", views[0].SchemeLabel)
|
||||
assert.Equal(t, "X-Hub-Signature-256", views[0].SchemeHeader)
|
||||
|
||||
assert.False(t, views[1].Configured)
|
||||
assert.Equal(t, "not verified", views[1].SchemeLabel)
|
||||
assert.Empty(t, views[1].SchemeHeader)
|
||||
|
||||
assert.True(t, views[2].Configured)
|
||||
assert.Equal(t, "(unavailable)", views[2].SchemeLabel)
|
||||
|
||||
// The struct has no field that could carry the secret, so this
|
||||
// fails to compile rather than fails at runtime if one is added
|
||||
// and populated. The assertion covers the labels it derives.
|
||||
for _, v := range views {
|
||||
assert.NotContains(t, v.SchemeLabel, inboundSecret)
|
||||
assert.NotContains(t, v.SchemeHeader, inboundSecret)
|
||||
assert.NotContains(t, string(v.Scheme), inboundSecret)
|
||||
}
|
||||
}
|
||||
80
internal/handlers/entrypoint_view.go
Normal file
80
internal/handlers/entrypoint_view.go
Normal file
@@ -0,0 +1,80 @@
|
||||
package handlers
|
||||
|
||||
import (
|
||||
"sneak.berlin/go/webhooker/internal/database"
|
||||
"sneak.berlin/go/webhooker/internal/signature"
|
||||
)
|
||||
|
||||
// signatureUnavailable is what an entrypoint's scheme renders as when
|
||||
// the stored value is not one this build supports. The stored string
|
||||
// is never echoed as a fallback: it is operator-supplied and the row
|
||||
// is already in a state the receiver refuses, so the UI says so
|
||||
// rather than inventing a description for it.
|
||||
const signatureUnavailable = "(unavailable)"
|
||||
|
||||
// signatureNotVerified is the label for an entrypoint that performs
|
||||
// no inbound verification.
|
||||
const signatureNotVerified = "not verified"
|
||||
|
||||
// EntrypointView is the display-safe projection of an entrypoint for
|
||||
// the UI. It deliberately has no secret field, so no template —
|
||||
// present or future — can render the shared secret, in the same way
|
||||
// delivery.TargetView keeps a target's stored credential away from
|
||||
// one.
|
||||
type EntrypointView struct {
|
||||
ID string
|
||||
Path string
|
||||
Description string
|
||||
Active bool
|
||||
|
||||
// Configured reports whether inbound requests to this entrypoint
|
||||
// are verified.
|
||||
Configured bool
|
||||
|
||||
// Scheme is the stored scheme, carried so the form can preselect
|
||||
// it. It names an algorithm, not a secret.
|
||||
Scheme database.SignatureScheme
|
||||
|
||||
// SchemeLabel and SchemeHeader describe the configured scheme for
|
||||
// display: the sender's name, and the header its signature
|
||||
// arrives in.
|
||||
SchemeLabel string
|
||||
SchemeHeader string
|
||||
}
|
||||
|
||||
// NewEntrypointViews projects entrypoints for rendering, dropping the
|
||||
// shared secret on the way.
|
||||
func NewEntrypointViews(
|
||||
entrypoints []database.Entrypoint,
|
||||
) []EntrypointView {
|
||||
views := make([]EntrypointView, 0, len(entrypoints))
|
||||
|
||||
for i := range entrypoints {
|
||||
e := &entrypoints[i]
|
||||
|
||||
view := EntrypointView{
|
||||
ID: e.ID,
|
||||
Path: e.Path,
|
||||
Description: e.Description,
|
||||
Active: e.Active,
|
||||
Configured: e.SignatureConfigured(),
|
||||
Scheme: e.SignatureScheme,
|
||||
SchemeLabel: signatureNotVerified,
|
||||
SchemeHeader: "",
|
||||
}
|
||||
|
||||
if view.Configured {
|
||||
view.SchemeLabel = signatureUnavailable
|
||||
|
||||
info, ok := signature.Info(e.SignatureScheme)
|
||||
if ok {
|
||||
view.SchemeLabel = info.Label
|
||||
view.SchemeHeader = info.Header
|
||||
}
|
||||
}
|
||||
|
||||
views = append(views, view)
|
||||
}
|
||||
|
||||
return views
|
||||
}
|
||||
@@ -11,6 +11,7 @@ import (
|
||||
"github.com/google/uuid"
|
||||
"sneak.berlin/go/webhooker/internal/database"
|
||||
"sneak.berlin/go/webhooker/internal/delivery"
|
||||
"sneak.berlin/go/webhooker/internal/signature"
|
||||
)
|
||||
|
||||
// WebhookListItem holds data for the webhook list view.
|
||||
@@ -414,11 +415,14 @@ func (h *Handlers) renderSourceDetail(
|
||||
// receivers; html/template cannot address a value stored in a map.
|
||||
data := map[string]any{
|
||||
tmplKeyWebhook: &webhook,
|
||||
"Entrypoints": entrypoints,
|
||||
// Targets are projected to a display-safe view: the
|
||||
// stored config blob holds credentials and must never
|
||||
// Entrypoints and targets are both projected to
|
||||
// display-safe views: an entrypoint carries the shared
|
||||
// secret its senders sign with and a target's stored
|
||||
// config blob holds a credential, and neither must ever
|
||||
// reach a template.
|
||||
"Entrypoints": NewEntrypointViews(entrypoints),
|
||||
"Targets": delivery.NewTargetViews(targets),
|
||||
"SignatureSchemes": signature.Schemes(),
|
||||
"Events": events,
|
||||
"BaseURL": scheme + "://" + host,
|
||||
}
|
||||
@@ -972,6 +976,145 @@ func (h *Handlers) HandleEntrypointCreate() http.HandlerFunc {
|
||||
}
|
||||
}
|
||||
|
||||
// HandleEntrypointSecret sets, rotates or removes the shared secret
|
||||
// an entrypoint verifies inbound requests with.
|
||||
//
|
||||
// Setting and rotating are the same operation: the form always takes
|
||||
// the secret afresh and the stored value is never sent to the browser
|
||||
// to be edited, so there is no path by which the page can display a
|
||||
// credential it holds. Rotation is therefore "submit the new secret",
|
||||
// and the operator already has that value — both supported senders
|
||||
// require them to enter the same string on the sender's side, so
|
||||
// there is no generated value for webhooker to reveal once.
|
||||
func (h *Handlers) HandleEntrypointSecret() http.HandlerFunc {
|
||||
return func(w http.ResponseWriter, r *http.Request) {
|
||||
webhook, ok := h.ownedWebhook(w, r)
|
||||
if !ok {
|
||||
return
|
||||
}
|
||||
|
||||
// The body size cap is enforced by the MaxBodySize
|
||||
// middleware, which runs before CSRF parses the form.
|
||||
err := r.ParseForm()
|
||||
if err != nil {
|
||||
http.Error(
|
||||
w, "Bad request", http.StatusBadRequest,
|
||||
)
|
||||
|
||||
return
|
||||
}
|
||||
|
||||
var entrypoint database.Entrypoint
|
||||
|
||||
err = h.db.DB().Where(
|
||||
"id = ? AND webhook_id = ?",
|
||||
chi.URLParam(r, "entrypointID"), webhook.ID,
|
||||
).First(&entrypoint).Error
|
||||
if err != nil {
|
||||
http.NotFound(w, r)
|
||||
|
||||
return
|
||||
}
|
||||
|
||||
h.applyEntrypointSecret(w, r, &entrypoint)
|
||||
}
|
||||
}
|
||||
|
||||
// applyEntrypointSecret validates the submitted scheme and secret and
|
||||
// stores them.
|
||||
//
|
||||
// A scheme this build does not support is a 400, never a stored value
|
||||
// the receiver would later have to interpret: the receiver fails such
|
||||
// a row closed, so letting one be created would take the entrypoint
|
||||
// offline through a form that reported success.
|
||||
func (h *Handlers) applyEntrypointSecret(
|
||||
w http.ResponseWriter,
|
||||
r *http.Request,
|
||||
entrypoint *database.Entrypoint,
|
||||
) {
|
||||
// PostFormValue, not FormValue: a credential must come from the
|
||||
// body. FormValue falls back to the query string, and the request
|
||||
// line — unlike the body — is what logs, proxies, Referer headers
|
||||
// and error trackers record.
|
||||
scheme := database.SignatureScheme(
|
||||
r.PostFormValue("signature_scheme"),
|
||||
)
|
||||
|
||||
// Surrounding whitespace is stripped, because a secret pasted from
|
||||
// a password manager routinely carries some and the resulting
|
||||
// mismatch is undiagnosable from the sender's side. A secret whose
|
||||
// own first or last character is a space cannot be stored; the
|
||||
// README says so.
|
||||
secret := strings.TrimSpace(r.PostFormValue("secret"))
|
||||
|
||||
if !signature.Supported(scheme) {
|
||||
http.Error(
|
||||
w, "Invalid signature scheme",
|
||||
http.StatusBadRequest,
|
||||
)
|
||||
|
||||
return
|
||||
}
|
||||
|
||||
if scheme == database.SignatureSchemeNone {
|
||||
// Turning verification off drops the secret with it: a stored
|
||||
// credential nothing reads is one more copy to leak, and
|
||||
// Verify refuses that pairing in any case.
|
||||
secret = ""
|
||||
} else if secret == "" {
|
||||
http.Error(
|
||||
w,
|
||||
"A shared secret is required for this signature scheme.",
|
||||
http.StatusBadRequest,
|
||||
)
|
||||
|
||||
return
|
||||
}
|
||||
|
||||
h.storeEntrypointSecret(w, r, entrypoint, scheme, secret)
|
||||
}
|
||||
|
||||
// storeEntrypointSecret writes a validated scheme and secret to an
|
||||
// entrypoint and returns the operator to the webhook page.
|
||||
func (h *Handlers) storeEntrypointSecret(
|
||||
w http.ResponseWriter,
|
||||
r *http.Request,
|
||||
entrypoint *database.Entrypoint,
|
||||
scheme database.SignatureScheme,
|
||||
secret string,
|
||||
) {
|
||||
// Updates with a map rather than a struct: a struct update skips
|
||||
// zero values, and the empty pair is exactly what has to be
|
||||
// written when verification is being turned off.
|
||||
err := h.db.DB().Model(entrypoint).Updates(map[string]any{
|
||||
"signature_scheme": scheme,
|
||||
"signature_secret": secret,
|
||||
}).Error
|
||||
if err != nil {
|
||||
// The error is logged by serverError; GORM's error text
|
||||
// carries the statement, not the bound values, so the secret
|
||||
// does not travel with it.
|
||||
h.serverError(
|
||||
w, "failed to update entrypoint signature", err,
|
||||
)
|
||||
|
||||
return
|
||||
}
|
||||
|
||||
h.log.Info(
|
||||
"entrypoint signature configuration updated",
|
||||
"entrypoint_id", entrypoint.ID,
|
||||
"webhook_id", entrypoint.WebhookID,
|
||||
"scheme", string(scheme),
|
||||
)
|
||||
|
||||
http.Redirect(
|
||||
w, r,
|
||||
"/source/"+entrypoint.WebhookID,
|
||||
http.StatusSeeOther,
|
||||
)
|
||||
}
|
||||
|
||||
// HandleTargetCreate handles adding a new target to a webhook.
|
||||
func (h *Handlers) HandleTargetCreate() http.HandlerFunc {
|
||||
return func(w http.ResponseWriter, r *http.Request) {
|
||||
|
||||
@@ -13,6 +13,7 @@ import (
|
||||
"sneak.berlin/go/webhooker/internal/delivery"
|
||||
"sneak.berlin/go/webhooker/internal/handlers"
|
||||
"sneak.berlin/go/webhooker/internal/session"
|
||||
"sneak.berlin/go/webhooker/internal/signature"
|
||||
)
|
||||
|
||||
// Template data keys the page templates read. The handlers package has
|
||||
@@ -268,10 +269,14 @@ func TestEntrypointCopyButtonIsProgressiveEnhancement(t *testing.T) {
|
||||
|
||||
body := renderPage(t, h, sess, "source_detail.html", map[string]any{
|
||||
dataKeyWebhook: webhook,
|
||||
"Entrypoints": []database.Entrypoint{entrypoint},
|
||||
// The handler passes delivery.NewTargetViews(targets), never
|
||||
// raw targets, so the test data has to have that same shape.
|
||||
// The handler passes projected views, never raw rows — an
|
||||
// entrypoint carries its shared secret and a target its
|
||||
// stored credential — so the test data has that same shape.
|
||||
"Entrypoints": handlers.NewEntrypointViews(
|
||||
[]database.Entrypoint{entrypoint},
|
||||
),
|
||||
"Targets": delivery.NewTargetViews(nil),
|
||||
"SignatureSchemes": signature.Schemes(),
|
||||
"Events": []database.Event{},
|
||||
"BaseURL": "https://hooks.example.com",
|
||||
})
|
||||
|
||||
@@ -2,6 +2,7 @@ package handlers
|
||||
|
||||
import (
|
||||
"encoding/json"
|
||||
"errors"
|
||||
"io"
|
||||
"net/http"
|
||||
|
||||
@@ -10,6 +11,7 @@ import (
|
||||
"sneak.berlin/go/webhooker/internal/database"
|
||||
"sneak.berlin/go/webhooker/internal/delivery"
|
||||
"sneak.berlin/go/webhooker/internal/logfield"
|
||||
"sneak.berlin/go/webhooker/internal/signature"
|
||||
)
|
||||
|
||||
const (
|
||||
@@ -69,8 +71,8 @@ func (h *Handlers) HandleWebhook() http.HandlerFunc {
|
||||
}
|
||||
}
|
||||
|
||||
// processWebhookRequest reads the body, serializes headers,
|
||||
// loads targets, and delivers the event.
|
||||
// processWebhookRequest reads the body, verifies the sender,
|
||||
// serializes headers, loads targets, and delivers the event.
|
||||
func (h *Handlers) processWebhookRequest(
|
||||
w http.ResponseWriter,
|
||||
r *http.Request,
|
||||
@@ -81,6 +83,17 @@ func (h *Handlers) processWebhookRequest(
|
||||
return
|
||||
}
|
||||
|
||||
// Before anything is written. An unverified request must leave no
|
||||
// event row, no delivery row and no delivery task behind, so this
|
||||
// sits above every write rather than inside the transaction that
|
||||
// performs them. It has to sit below the body read because the
|
||||
// signature is computed over the body; readWebhookBody is what
|
||||
// bounds that read, so an unauthenticated sender still cannot make
|
||||
// the process hold more than the 1 MB cap.
|
||||
if !h.verifyInboundSignature(w, entrypoint, r.Header, body) {
|
||||
return
|
||||
}
|
||||
|
||||
headersJSON, err := json.Marshal(r.Header)
|
||||
if err != nil {
|
||||
h.serverError(w, "failed to serialize headers", err)
|
||||
@@ -100,6 +113,63 @@ func (h *Handlers) processWebhookRequest(
|
||||
)
|
||||
}
|
||||
|
||||
// verifyInboundSignature authenticates the request against the
|
||||
// entrypoint's configured secret, reporting false once it has written
|
||||
// the response.
|
||||
//
|
||||
// An entrypoint with no secret configured is not checked and this
|
||||
// returns true, which is the unchanged behaviour every existing
|
||||
// entrypoint keeps.
|
||||
//
|
||||
// A configuration that cannot be applied — an unknown scheme, or one
|
||||
// half of the pair missing — is a 500, not a 401: the request may well
|
||||
// be authentic, and calling it unauthorized would tell a legitimate
|
||||
// sender to go fix its own signing. Either way it is refused. Failing
|
||||
// open here would mean an entrypoint the operator has protected
|
||||
// quietly accepting anything.
|
||||
func (h *Handlers) verifyInboundSignature(
|
||||
w http.ResponseWriter,
|
||||
entrypoint database.Entrypoint,
|
||||
header http.Header,
|
||||
body []byte,
|
||||
) bool {
|
||||
err := signature.Verify(&entrypoint, header, body)
|
||||
if err == nil {
|
||||
return true
|
||||
}
|
||||
|
||||
if errors.Is(err, signature.ErrConfig) {
|
||||
h.log.Error(
|
||||
"entrypoint signature configuration cannot be applied",
|
||||
"entrypoint_id", entrypoint.ID,
|
||||
"webhook_id", entrypoint.WebhookID,
|
||||
"error", err,
|
||||
)
|
||||
http.Error(
|
||||
w, "Internal server error",
|
||||
http.StatusInternalServerError,
|
||||
)
|
||||
|
||||
return false
|
||||
}
|
||||
|
||||
// Every field here is bounded and none is client-chosen: the ids
|
||||
// are ours, the scheme is one of a fixed set, and the error is a
|
||||
// static string carrying no part of the secret or of what the
|
||||
// client presented. Reaching this line also requires a real
|
||||
// entrypoint UUID, so it is not a line a stranger can drive.
|
||||
h.log.Warn(
|
||||
"inbound signature verification failed",
|
||||
"entrypoint_id", entrypoint.ID,
|
||||
"webhook_id", entrypoint.WebhookID,
|
||||
"scheme", string(entrypoint.SignatureScheme),
|
||||
"error", err,
|
||||
)
|
||||
http.Error(w, "Unauthorized", http.StatusUnauthorized)
|
||||
|
||||
return false
|
||||
}
|
||||
|
||||
// loadActiveTargets returns all active targets for a webhook.
|
||||
func (h *Handlers) loadActiveTargets(
|
||||
webhookID string,
|
||||
|
||||
362
internal/handlers/webhook_signature_test.go
Normal file
362
internal/handlers/webhook_signature_test.go
Normal file
@@ -0,0 +1,362 @@
|
||||
package handlers_test
|
||||
|
||||
import (
|
||||
"bytes"
|
||||
"context"
|
||||
"crypto/hmac"
|
||||
"crypto/sha256"
|
||||
"encoding/hex"
|
||||
"log/slog"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"strings"
|
||||
"testing"
|
||||
|
||||
"github.com/go-chi/chi"
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
"gorm.io/gorm/clause"
|
||||
"sneak.berlin/go/webhooker/internal/database"
|
||||
"sneak.berlin/go/webhooker/internal/handlers"
|
||||
"sneak.berlin/go/webhooker/internal/signature"
|
||||
)
|
||||
|
||||
const (
|
||||
// inboundSecret is the shared secret the signed-receiver tests
|
||||
// configure on their entrypoint. It doubles as a marker: no log
|
||||
// line and no rendered page may contain it.
|
||||
inboundSecret = "QQINBOUNDSECRETQQ"
|
||||
|
||||
// inboundBody is the payload the sender signs.
|
||||
inboundBody = `{"zen":"Non-blocking is better than blocking."}`
|
||||
|
||||
// entrypointIDParam is the chi URL parameter naming an entrypoint.
|
||||
entrypointIDParam = "entrypointID"
|
||||
)
|
||||
|
||||
// hubSignature returns the X-Hub-Signature-256 value a GitHub sender
|
||||
// holding secret would send for inboundBody.
|
||||
func hubSignature(secret string) string {
|
||||
mac := hmac.New(sha256.New, []byte(secret))
|
||||
_, _ = mac.Write([]byte(inboundBody))
|
||||
|
||||
return "sha256=" + hex.EncodeToString(mac.Sum(nil))
|
||||
}
|
||||
|
||||
// seedSignedEntrypoint inserts an active entrypoint for a webhook
|
||||
// with the given signature configuration and returns it.
|
||||
func seedSignedEntrypoint(
|
||||
t *testing.T,
|
||||
db *database.Database,
|
||||
webhookID string,
|
||||
scheme database.SignatureScheme,
|
||||
secret string,
|
||||
) *database.Entrypoint {
|
||||
t.Helper()
|
||||
|
||||
ep := &database.Entrypoint{
|
||||
WebhookID: webhookID,
|
||||
Path: "path-" + webhookID,
|
||||
Description: "signed",
|
||||
Active: true,
|
||||
SignatureScheme: scheme,
|
||||
SignatureSecret: secret,
|
||||
}
|
||||
|
||||
require.NoError(
|
||||
t,
|
||||
db.DB().Omit(clause.Associations).Create(ep).Error,
|
||||
)
|
||||
|
||||
return ep
|
||||
}
|
||||
|
||||
// postToEntrypoint drives the real receiver handler at an
|
||||
// entrypoint's path with one optional header set.
|
||||
func postToEntrypoint(
|
||||
t *testing.T,
|
||||
h *handlers.Handlers,
|
||||
path, body, headerName, headerValue string,
|
||||
) *httptest.ResponseRecorder {
|
||||
t.Helper()
|
||||
|
||||
req := httptest.NewRequestWithContext(
|
||||
context.Background(),
|
||||
http.MethodPost,
|
||||
"/webhook/"+path,
|
||||
strings.NewReader(body),
|
||||
)
|
||||
req.Header.Set("Content-Type", "application/json")
|
||||
|
||||
if headerName != "" {
|
||||
req.Header.Set(headerName, headerValue)
|
||||
}
|
||||
|
||||
rctx := chi.NewRouteContext()
|
||||
rctx.URLParams.Add("uuid", path)
|
||||
|
||||
req = req.WithContext(
|
||||
context.WithValue(req.Context(), chi.RouteCtxKey, rctx),
|
||||
)
|
||||
|
||||
w := httptest.NewRecorder()
|
||||
h.HandleWebhook().ServeHTTP(w, req)
|
||||
|
||||
return w
|
||||
}
|
||||
|
||||
// storedEvents counts the event rows a webhook's per-webhook database
|
||||
// holds. A database that was never opened holds none, which is the
|
||||
// state a rejected request has to leave behind.
|
||||
func storedEvents(
|
||||
t *testing.T,
|
||||
mgr *database.WebhookDBManager,
|
||||
webhookID string,
|
||||
) int64 {
|
||||
t.Helper()
|
||||
|
||||
if !mgr.DBExists(webhookID) {
|
||||
return 0
|
||||
}
|
||||
|
||||
db, err := mgr.GetDB(webhookID)
|
||||
require.NoError(t, err)
|
||||
|
||||
var count int64
|
||||
|
||||
require.NoError(
|
||||
t,
|
||||
db.Model(&database.Event{}).
|
||||
Where("webhook_id = ?", webhookID).
|
||||
Count(&count).Error,
|
||||
)
|
||||
|
||||
return count
|
||||
}
|
||||
|
||||
// signedReceiverCase is one inbound request against an entrypoint
|
||||
// with a given stored signature configuration.
|
||||
type signedReceiverCase struct {
|
||||
name string
|
||||
scheme database.SignatureScheme
|
||||
secret string
|
||||
headerName string
|
||||
headerValue string
|
||||
body string
|
||||
wantStatus int
|
||||
}
|
||||
|
||||
// signedReceiverCases covers each supported scheme with a valid
|
||||
// signature, an invalid one and none at all, plus the two states that
|
||||
// are not "a client got it wrong": an entrypoint with nothing
|
||||
// configured, and one whose stored configuration cannot be applied.
|
||||
func signedReceiverCases() []signedReceiverCase {
|
||||
return append(
|
||||
schemeReceiverCases(), unverifiedReceiverCases()...,
|
||||
)
|
||||
}
|
||||
|
||||
// schemeReceiverCases covers the two supported schemes.
|
||||
func schemeReceiverCases() []signedReceiverCase {
|
||||
return []signedReceiverCase{
|
||||
{
|
||||
name: "github valid",
|
||||
scheme: database.SignatureSchemeGitHub,
|
||||
secret: inboundSecret,
|
||||
headerName: signature.HeaderGitHub,
|
||||
headerValue: hubSignature(inboundSecret),
|
||||
body: inboundBody,
|
||||
wantStatus: http.StatusOK,
|
||||
},
|
||||
{
|
||||
name: "github wrong secret",
|
||||
scheme: database.SignatureSchemeGitHub,
|
||||
secret: inboundSecret,
|
||||
headerName: signature.HeaderGitHub,
|
||||
headerValue: hubSignature("wrong"),
|
||||
body: inboundBody,
|
||||
wantStatus: http.StatusUnauthorized,
|
||||
},
|
||||
{
|
||||
// A digest that was valid for a different body: the
|
||||
// check is over the bytes as received.
|
||||
name: "github body tampered",
|
||||
scheme: database.SignatureSchemeGitHub,
|
||||
secret: inboundSecret,
|
||||
headerName: signature.HeaderGitHub,
|
||||
headerValue: hubSignature(inboundSecret),
|
||||
body: inboundBody + " ",
|
||||
wantStatus: http.StatusUnauthorized,
|
||||
},
|
||||
{
|
||||
name: "github unsigned",
|
||||
scheme: database.SignatureSchemeGitHub,
|
||||
secret: inboundSecret,
|
||||
body: inboundBody,
|
||||
wantStatus: http.StatusUnauthorized,
|
||||
},
|
||||
{
|
||||
name: "gitlab valid",
|
||||
scheme: database.SignatureSchemeGitLab,
|
||||
secret: inboundSecret,
|
||||
headerName: signature.HeaderGitLab,
|
||||
headerValue: inboundSecret,
|
||||
body: inboundBody,
|
||||
wantStatus: http.StatusOK,
|
||||
},
|
||||
{
|
||||
name: "gitlab wrong token",
|
||||
scheme: database.SignatureSchemeGitLab,
|
||||
secret: inboundSecret,
|
||||
headerName: signature.HeaderGitLab,
|
||||
headerValue: "wrong",
|
||||
body: inboundBody,
|
||||
wantStatus: http.StatusUnauthorized,
|
||||
},
|
||||
{
|
||||
name: "gitlab unsigned",
|
||||
scheme: database.SignatureSchemeGitLab,
|
||||
secret: inboundSecret,
|
||||
body: inboundBody,
|
||||
wantStatus: http.StatusUnauthorized,
|
||||
},
|
||||
}
|
||||
}
|
||||
|
||||
// unverifiedReceiverCases covers the two entrypoint states that are
|
||||
// not about a client getting its signature wrong: nothing configured
|
||||
// at all, and a configuration the receiver cannot apply.
|
||||
func unverifiedReceiverCases() []signedReceiverCase {
|
||||
return []signedReceiverCase{
|
||||
{
|
||||
// The pass-through case. An entrypoint with nothing
|
||||
// configured is what every deployment already has, and
|
||||
// it must keep accepting unsigned requests so that an
|
||||
// upgrade does not lock an operator out of their own
|
||||
// receivers.
|
||||
name: "unconfigured accepts unsigned",
|
||||
scheme: database.SignatureSchemeNone,
|
||||
body: inboundBody,
|
||||
wantStatus: http.StatusOK,
|
||||
},
|
||||
{
|
||||
// A stray signature header changes nothing when nothing
|
||||
// is configured to check it.
|
||||
name: "unconfigured ignores a stray header",
|
||||
scheme: database.SignatureSchemeNone,
|
||||
headerName: signature.HeaderGitHub,
|
||||
headerValue: "sha256=deadbeef",
|
||||
body: inboundBody,
|
||||
wantStatus: http.StatusOK,
|
||||
},
|
||||
{
|
||||
// A scheme this build cannot apply, reachable only by
|
||||
// editing the database: refused, not waved through as
|
||||
// unverified.
|
||||
name: "unknown scheme fails closed",
|
||||
scheme: database.SignatureScheme("stripe"),
|
||||
secret: inboundSecret,
|
||||
headerName: signature.HeaderGitHub,
|
||||
headerValue: hubSignature(inboundSecret),
|
||||
body: inboundBody,
|
||||
wantStatus: http.StatusInternalServerError,
|
||||
},
|
||||
}
|
||||
}
|
||||
|
||||
// TestReceiverVerifiesConfiguredEntrypoints is the load-bearing test
|
||||
// for the feature: for each supported scheme a correctly signed
|
||||
// request is accepted and stored, and an incorrectly signed or
|
||||
// unsigned one is answered 401 having stored nothing.
|
||||
//
|
||||
// The event count is the half that matters most. A rejection that
|
||||
// still wrote a row would leave the receiver a place for a stranger
|
||||
// who knows a URL to deposit content, which is exactly what the
|
||||
// signature is there to prevent.
|
||||
//
|
||||
// The cases share one application and take a webhook each, rather
|
||||
// than each standing up its own: every newTestApp seeds an admin user
|
||||
// and so pays an Argon2id hash at 64 MB, and this package's test
|
||||
// budget is not large enough to spend one per table row.
|
||||
func TestReceiverVerifiesConfiguredEntrypoints(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
var (
|
||||
h *handlers.Handlers
|
||||
db *database.Database
|
||||
mgr *database.WebhookDBManager
|
||||
)
|
||||
|
||||
app := newTestApp(t, &h, &db, &mgr)
|
||||
app.RequireStart()
|
||||
|
||||
t.Cleanup(app.RequireStop)
|
||||
|
||||
for _, tc := range signedReceiverCases() {
|
||||
wh := seedWebhook(t, db)
|
||||
ep := seedSignedEntrypoint(
|
||||
t, db, wh.ID, tc.scheme, tc.secret,
|
||||
)
|
||||
|
||||
w := postToEntrypoint(
|
||||
t, h, ep.Path, tc.body,
|
||||
tc.headerName, tc.headerValue,
|
||||
)
|
||||
|
||||
assert.Equal(t, tc.wantStatus, w.Code, "case %s", tc.name)
|
||||
|
||||
want := int64(0)
|
||||
if tc.wantStatus == http.StatusOK {
|
||||
want = 1
|
||||
}
|
||||
|
||||
assert.Equal(
|
||||
t, want, storedEvents(t, mgr, wh.ID),
|
||||
"case %s: stored event rows after a %d response",
|
||||
tc.name, w.Code,
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
// TestReceiverLogsNoSecret proves the rejection path does not write
|
||||
// the shared secret, or what the client presented, into the log. A
|
||||
// GitLab token arrives as the credential itself, so echoing the
|
||||
// header value would put a live secret in the log of every deployment
|
||||
// whose sender is briefly misconfigured.
|
||||
func TestReceiverLogsNoSecret(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
const presented = "QQPRESENTEDVALUEQQ"
|
||||
|
||||
var (
|
||||
h *handlers.Handlers
|
||||
db *database.Database
|
||||
)
|
||||
|
||||
app := newTestApp(t, &h, &db)
|
||||
app.RequireStart()
|
||||
|
||||
t.Cleanup(app.RequireStop)
|
||||
|
||||
var buf bytes.Buffer
|
||||
|
||||
h.SetLogForTest(slog.New(slog.NewJSONHandler(&buf, nil)))
|
||||
|
||||
wh := seedWebhook(t, db)
|
||||
ep := seedSignedEntrypoint(
|
||||
t, db, wh.ID,
|
||||
database.SignatureSchemeGitLab, inboundSecret,
|
||||
)
|
||||
|
||||
w := postToEntrypoint(
|
||||
t, h, ep.Path, inboundBody,
|
||||
signature.HeaderGitLab, presented,
|
||||
)
|
||||
require.Equal(t, http.StatusUnauthorized, w.Code)
|
||||
|
||||
// The rejection is recorded at all — a silent 401 leaves an
|
||||
// operator no way to see a sender failing to authenticate.
|
||||
assert.Contains(t, buf.String(), "verification failed")
|
||||
assert.NotContains(t, buf.String(), inboundSecret)
|
||||
assert.NotContains(t, buf.String(), presented)
|
||||
}
|
||||
@@ -213,6 +213,10 @@ func (s *Server) setupSourceRoutes() {
|
||||
"/entrypoints/{entrypointID}/toggle",
|
||||
s.h.HandleEntrypointToggle(),
|
||||
)
|
||||
r.Post(
|
||||
"/entrypoints/{entrypointID}/secret",
|
||||
s.h.HandleEntrypointSecret(),
|
||||
)
|
||||
r.Post("/targets", s.h.HandleTargetCreate())
|
||||
r.Post(
|
||||
"/targets/{targetID}/delete",
|
||||
|
||||
229
internal/signature/signature.go
Normal file
229
internal/signature/signature.go
Normal file
@@ -0,0 +1,229 @@
|
||||
// Package signature verifies that an inbound webhook request really
|
||||
// came from the sender an entrypoint was configured for.
|
||||
//
|
||||
// Verification is optional and per entrypoint. An entrypoint with no
|
||||
// scheme configured is not verified at all, which is what every
|
||||
// entrypoint was before this package existed. An entrypoint whose
|
||||
// configuration is present but incoherent is failed closed, never
|
||||
// treated as unverified: the whole point of the feature is that
|
||||
// turning it on cannot silently turn itself back off.
|
||||
package signature
|
||||
|
||||
import (
|
||||
"crypto/hmac"
|
||||
"crypto/sha256"
|
||||
"encoding/hex"
|
||||
"errors"
|
||||
"fmt"
|
||||
"net/http"
|
||||
"strings"
|
||||
|
||||
"sneak.berlin/go/webhooker/internal/database"
|
||||
)
|
||||
|
||||
// Header names each supported scheme reads its signature from.
|
||||
const (
|
||||
// HeaderGitHub is GitHub's HMAC-SHA256 signature header. GitHub
|
||||
// also sends the older SHA-1 X-Hub-Signature; it is not accepted.
|
||||
HeaderGitHub = "X-Hub-Signature-256"
|
||||
|
||||
// HeaderGitLab is GitLab's plain shared-token header.
|
||||
HeaderGitLab = "X-Gitlab-Token"
|
||||
)
|
||||
|
||||
// githubPrefix is the algorithm label GitHub puts in front of the hex
|
||||
// digest. It is required, not optional: accepting a bare digest too
|
||||
// would mean accepting a spelling no supported sender produces.
|
||||
const githubPrefix = "sha256="
|
||||
|
||||
// ErrConfig marks a failure caused by the entrypoint's stored
|
||||
// configuration rather than by the request. A caller must fail these
|
||||
// closed — refuse the request — because the alternative is an
|
||||
// entrypoint the operator believes is verified silently accepting
|
||||
// anything.
|
||||
var ErrConfig = errors.New("entrypoint signature configuration invalid")
|
||||
|
||||
// ErrUnauthorized marks a request that failed verification. A caller
|
||||
// answers these 401.
|
||||
var ErrUnauthorized = errors.New("inbound signature verification failed")
|
||||
|
||||
// Configuration failures. None of these carry any part of the secret.
|
||||
var (
|
||||
errSchemeUnknown = fmt.Errorf(
|
||||
"%w: unsupported scheme", ErrConfig,
|
||||
)
|
||||
errSecretMissing = fmt.Errorf(
|
||||
"%w: scheme set with no secret", ErrConfig,
|
||||
)
|
||||
errSchemeMissing = fmt.Errorf(
|
||||
"%w: secret set with no scheme", ErrConfig,
|
||||
)
|
||||
)
|
||||
|
||||
// Request failures. These are logged, so none of them carries the
|
||||
// value the client sent: under the GitLab scheme that value is a
|
||||
// guess at the token, and under either scheme a misconfigured sender
|
||||
// could be presenting the real one.
|
||||
var (
|
||||
errHeaderMissing = fmt.Errorf(
|
||||
"%w: signature header absent", ErrUnauthorized,
|
||||
)
|
||||
errHeaderMalformed = fmt.Errorf(
|
||||
"%w: signature header malformed", ErrUnauthorized,
|
||||
)
|
||||
errSignatureMismatch = fmt.Errorf(
|
||||
"%w: signature does not match", ErrUnauthorized,
|
||||
)
|
||||
)
|
||||
|
||||
// SchemeInfo describes one supported scheme for the UI.
|
||||
type SchemeInfo struct {
|
||||
Scheme database.SignatureScheme
|
||||
Label string
|
||||
Header string
|
||||
}
|
||||
|
||||
// Schemes returns the supported schemes in the order the UI offers
|
||||
// them. It returns a fresh slice per call so no caller can edit the
|
||||
// set out from under another.
|
||||
func Schemes() []SchemeInfo {
|
||||
return []SchemeInfo{
|
||||
{
|
||||
Scheme: database.SignatureSchemeGitHub,
|
||||
Label: "GitHub",
|
||||
Header: HeaderGitHub,
|
||||
},
|
||||
{
|
||||
Scheme: database.SignatureSchemeGitLab,
|
||||
Label: "GitLab",
|
||||
Header: HeaderGitLab,
|
||||
},
|
||||
}
|
||||
}
|
||||
|
||||
// Info returns the description of a supported scheme. It reports
|
||||
// false for the empty scheme and for anything unrecognised, which is
|
||||
// what a row hand-edited in the database could hold.
|
||||
func Info(scheme database.SignatureScheme) (SchemeInfo, bool) {
|
||||
for _, s := range Schemes() {
|
||||
if s.Scheme == scheme {
|
||||
return s, true
|
||||
}
|
||||
}
|
||||
|
||||
return SchemeInfo{}, false
|
||||
}
|
||||
|
||||
// Supported reports whether a scheme may be stored on an entrypoint.
|
||||
// The empty scheme is supported: it means no verification.
|
||||
func Supported(scheme database.SignatureScheme) bool {
|
||||
if scheme == database.SignatureSchemeNone {
|
||||
return true
|
||||
}
|
||||
|
||||
_, ok := Info(scheme)
|
||||
|
||||
return ok
|
||||
}
|
||||
|
||||
// Verify checks an inbound request against an entrypoint's
|
||||
// configuration and returns nil when the request may be accepted.
|
||||
//
|
||||
// body must be the raw bytes exactly as received, before any parsing
|
||||
// or normalisation: the sender computed its digest over those bytes,
|
||||
// so anything that re-encodes them produces a different digest and a
|
||||
// spurious rejection. The caller is also responsible for bounding
|
||||
// that read; this package hashes what it is handed.
|
||||
//
|
||||
// Every non-nil error is either ErrConfig or ErrUnauthorized, so a
|
||||
// caller can tell "the server is misconfigured" from "the client did
|
||||
// not authenticate" with errors.Is.
|
||||
func Verify(
|
||||
entrypoint *database.Entrypoint,
|
||||
header http.Header,
|
||||
body []byte,
|
||||
) error {
|
||||
scheme := entrypoint.SignatureScheme
|
||||
secret := entrypoint.SignatureSecret
|
||||
|
||||
if scheme == database.SignatureSchemeNone {
|
||||
// A secret with no scheme names no header and no algorithm,
|
||||
// so there is nothing to check it with. Accepting the request
|
||||
// would make a half-applied configuration indistinguishable
|
||||
// from no configuration at all.
|
||||
if secret != "" {
|
||||
return errSchemeMissing
|
||||
}
|
||||
|
||||
return nil
|
||||
}
|
||||
|
||||
if secret == "" {
|
||||
return errSecretMissing
|
||||
}
|
||||
|
||||
switch scheme {
|
||||
case database.SignatureSchemeGitHub:
|
||||
return verifyGitHub(secret, header.Get(HeaderGitHub), body)
|
||||
case database.SignatureSchemeGitLab:
|
||||
return verifyGitLab(secret, header.Get(HeaderGitLab))
|
||||
case database.SignatureSchemeNone:
|
||||
// Handled above; restated so the switch stays exhaustive and
|
||||
// adding a scheme has to be decided here.
|
||||
return nil
|
||||
default:
|
||||
return errSchemeUnknown
|
||||
}
|
||||
}
|
||||
|
||||
// verifyGitHub checks a GitHub-style X-Hub-Signature-256: the string
|
||||
// "sha256=" followed by the hex HMAC-SHA256 of the raw body under the
|
||||
// shared secret.
|
||||
func verifyGitHub(secret, provided string, body []byte) error {
|
||||
if provided == "" {
|
||||
return errHeaderMissing
|
||||
}
|
||||
|
||||
encoded, ok := strings.CutPrefix(provided, githubPrefix)
|
||||
if !ok {
|
||||
return errHeaderMalformed
|
||||
}
|
||||
|
||||
got, err := hex.DecodeString(encoded)
|
||||
if err != nil {
|
||||
return errHeaderMalformed
|
||||
}
|
||||
|
||||
mac := hmac.New(sha256.New, []byte(secret))
|
||||
|
||||
// hash.Hash.Write is documented never to return an error.
|
||||
_, _ = mac.Write(body)
|
||||
|
||||
// hmac.Equal, never ==: string comparison stops at the first
|
||||
// differing byte, which tells a client how much of a forged
|
||||
// digest it got right and turns forgery into a per-byte search.
|
||||
if !hmac.Equal(mac.Sum(nil), got) {
|
||||
return errSignatureMismatch
|
||||
}
|
||||
|
||||
return nil
|
||||
}
|
||||
|
||||
// verifyGitLab checks a GitLab-style X-Gitlab-Token, which is the
|
||||
// shared secret itself rather than a digest over the body.
|
||||
//
|
||||
// The comparison is constant time in the same way as the HMAC one.
|
||||
// hmac.Equal returns early for unequal lengths, so the length of the
|
||||
// token is not hidden; its contents are, and length alone does not
|
||||
// let a client search for the value.
|
||||
func verifyGitLab(secret, provided string) error {
|
||||
if provided == "" {
|
||||
return errHeaderMissing
|
||||
}
|
||||
|
||||
if !hmac.Equal([]byte(provided), []byte(secret)) {
|
||||
return errSignatureMismatch
|
||||
}
|
||||
|
||||
return nil
|
||||
}
|
||||
340
internal/signature/signature_test.go
Normal file
340
internal/signature/signature_test.go
Normal file
@@ -0,0 +1,340 @@
|
||||
package signature_test
|
||||
|
||||
import (
|
||||
"crypto/hmac"
|
||||
"crypto/sha256"
|
||||
"encoding/hex"
|
||||
"net/http"
|
||||
"testing"
|
||||
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
"sneak.berlin/go/webhooker/internal/database"
|
||||
"sneak.berlin/go/webhooker/internal/signature"
|
||||
)
|
||||
|
||||
const (
|
||||
// testSharedKey is the shared secret under test. It is not named
|
||||
// "secret": gosec reads a credential-shaped name bound to a
|
||||
// high-entropy literal as a leaked credential, which is the right
|
||||
// rule and the wrong finding here.
|
||||
testSharedKey = "s3kr1t-shared-value"
|
||||
testBody = `{"action":"opened","number":1}`
|
||||
)
|
||||
|
||||
// githubSignature returns the X-Hub-Signature-256 value GitHub would
|
||||
// send for testBody signed with secret.
|
||||
func githubSignature(secret string) string {
|
||||
mac := hmac.New(sha256.New, []byte(secret))
|
||||
_, _ = mac.Write([]byte(testBody))
|
||||
|
||||
return "sha256=" + hex.EncodeToString(mac.Sum(nil))
|
||||
}
|
||||
|
||||
// headerWith builds a request header carrying one value.
|
||||
func headerWith(name, value string) http.Header {
|
||||
h := http.Header{}
|
||||
if name != "" {
|
||||
h.Set(name, value)
|
||||
}
|
||||
|
||||
return h
|
||||
}
|
||||
|
||||
// entrypoint builds an entrypoint with a signature configuration.
|
||||
func entrypoint(
|
||||
scheme database.SignatureScheme, secret string,
|
||||
) *database.Entrypoint {
|
||||
return &database.Entrypoint{
|
||||
SignatureScheme: scheme,
|
||||
SignatureSecret: secret,
|
||||
}
|
||||
}
|
||||
|
||||
// TestVerifyUnconfiguredAcceptsAnything pins the pass-through case:
|
||||
// an entrypoint with no scheme is the entrypoint every deployment
|
||||
// already has, and it must keep accepting requests that carry no
|
||||
// signature at all.
|
||||
func TestVerifyUnconfiguredAcceptsAnything(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
ep := entrypoint(database.SignatureSchemeNone, "")
|
||||
|
||||
require.NoError(
|
||||
t, signature.Verify(ep, http.Header{}, []byte(testBody)),
|
||||
)
|
||||
require.NoError(
|
||||
t,
|
||||
signature.Verify(
|
||||
ep,
|
||||
headerWith(signature.HeaderGitHub, "sha256=deadbeef"),
|
||||
[]byte(testBody),
|
||||
),
|
||||
)
|
||||
}
|
||||
|
||||
// githubCase is one inbound request against a GitHub-scheme
|
||||
// entrypoint.
|
||||
type githubCase struct {
|
||||
name string
|
||||
header string
|
||||
value string
|
||||
body string
|
||||
want error
|
||||
}
|
||||
|
||||
// githubCases enumerates the shapes a GitHub signature can arrive in.
|
||||
func githubCases() []githubCase {
|
||||
valid := githubSignature(testSharedKey)
|
||||
|
||||
return []githubCase{
|
||||
{
|
||||
name: "valid",
|
||||
header: signature.HeaderGitHub,
|
||||
value: valid,
|
||||
body: testBody,
|
||||
want: nil,
|
||||
},
|
||||
{
|
||||
name: "absent header",
|
||||
header: "",
|
||||
body: testBody,
|
||||
want: signature.ErrUnauthorized,
|
||||
},
|
||||
{
|
||||
name: "wrong secret",
|
||||
header: signature.HeaderGitHub,
|
||||
value: githubSignature("not-the-shared-value"),
|
||||
body: testBody,
|
||||
want: signature.ErrUnauthorized,
|
||||
},
|
||||
{
|
||||
// The digest is valid for a different body: the check
|
||||
// has to be over the bytes actually received.
|
||||
name: "body altered in flight",
|
||||
header: signature.HeaderGitHub,
|
||||
value: valid,
|
||||
body: testBody + " ",
|
||||
want: signature.ErrUnauthorized,
|
||||
},
|
||||
{
|
||||
name: "missing algorithm prefix",
|
||||
header: signature.HeaderGitHub,
|
||||
value: valid[len("sha256="):],
|
||||
body: testBody,
|
||||
want: signature.ErrUnauthorized,
|
||||
},
|
||||
{
|
||||
name: "not hex",
|
||||
header: signature.HeaderGitHub,
|
||||
value: "sha256=zzzz",
|
||||
body: testBody,
|
||||
want: signature.ErrUnauthorized,
|
||||
},
|
||||
{
|
||||
name: "empty digest",
|
||||
header: signature.HeaderGitHub,
|
||||
value: "sha256=",
|
||||
body: testBody,
|
||||
want: signature.ErrUnauthorized,
|
||||
},
|
||||
{
|
||||
// GitLab's header does not authenticate a GitHub
|
||||
// entrypoint, even holding the right secret.
|
||||
name: "wrong header for the scheme",
|
||||
header: signature.HeaderGitLab,
|
||||
value: testSharedKey,
|
||||
body: testBody,
|
||||
want: signature.ErrUnauthorized,
|
||||
},
|
||||
}
|
||||
}
|
||||
|
||||
func TestVerifyGitHub(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
for _, tc := range githubCases() {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
err := signature.Verify(
|
||||
entrypoint(
|
||||
database.SignatureSchemeGitHub, testSharedKey,
|
||||
),
|
||||
headerWith(tc.header, tc.value),
|
||||
[]byte(tc.body),
|
||||
)
|
||||
|
||||
if tc.want == nil {
|
||||
require.NoError(t, err)
|
||||
|
||||
return
|
||||
}
|
||||
|
||||
require.ErrorIs(t, err, tc.want)
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestVerifyGitLab(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
cases := []struct {
|
||||
name string
|
||||
header string
|
||||
value string
|
||||
want error
|
||||
}{
|
||||
{
|
||||
name: "valid",
|
||||
header: signature.HeaderGitLab,
|
||||
value: testSharedKey,
|
||||
want: nil,
|
||||
},
|
||||
{
|
||||
name: "absent header",
|
||||
header: "",
|
||||
want: signature.ErrUnauthorized,
|
||||
},
|
||||
{
|
||||
name: "wrong token",
|
||||
header: signature.HeaderGitLab,
|
||||
value: "not-the-shared-value",
|
||||
want: signature.ErrUnauthorized,
|
||||
},
|
||||
{
|
||||
name: "token prefix only",
|
||||
header: signature.HeaderGitLab,
|
||||
value: testSharedKey[:5],
|
||||
want: signature.ErrUnauthorized,
|
||||
},
|
||||
{
|
||||
name: "wrong header for the scheme",
|
||||
header: signature.HeaderGitHub,
|
||||
value: githubSignature(testSharedKey),
|
||||
want: signature.ErrUnauthorized,
|
||||
},
|
||||
}
|
||||
|
||||
for _, tc := range cases {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
err := signature.Verify(
|
||||
entrypoint(
|
||||
database.SignatureSchemeGitLab, testSharedKey,
|
||||
),
|
||||
headerWith(tc.header, tc.value),
|
||||
[]byte(testBody),
|
||||
)
|
||||
|
||||
if tc.want == nil {
|
||||
require.NoError(t, err)
|
||||
|
||||
return
|
||||
}
|
||||
|
||||
require.ErrorIs(t, err, tc.want)
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// TestVerifyBrokenConfigurationFailsClosed covers the rows a caller
|
||||
// must refuse rather than wave through. Each is a state an operator
|
||||
// could only reach outside the UI, and each one would otherwise be
|
||||
// indistinguishable from "verification is off".
|
||||
func TestVerifyBrokenConfigurationFailsClosed(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
cases := []struct {
|
||||
name string
|
||||
scheme database.SignatureScheme
|
||||
secret string
|
||||
}{
|
||||
{
|
||||
name: "unknown scheme",
|
||||
scheme: database.SignatureScheme("stripe"),
|
||||
secret: testSharedKey,
|
||||
},
|
||||
{
|
||||
name: "scheme without secret",
|
||||
scheme: database.SignatureSchemeGitHub,
|
||||
secret: "",
|
||||
},
|
||||
{
|
||||
name: "secret without scheme",
|
||||
scheme: database.SignatureSchemeNone,
|
||||
secret: testSharedKey,
|
||||
},
|
||||
}
|
||||
|
||||
for _, tc := range cases {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
err := signature.Verify(
|
||||
entrypoint(tc.scheme, tc.secret),
|
||||
headerWith(
|
||||
signature.HeaderGitHub,
|
||||
githubSignature(testSharedKey),
|
||||
),
|
||||
[]byte(testBody),
|
||||
)
|
||||
|
||||
require.ErrorIs(t, err, signature.ErrConfig)
|
||||
assert.NotErrorIs(t, err, signature.ErrUnauthorized)
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// TestErrorsCarryNoSecret proves the strings that reach the log hold
|
||||
// no part of the shared secret or of what the client presented.
|
||||
func TestErrorsCarryNoSecret(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
const presented = "QQPRESENTEDTOKENQQ"
|
||||
|
||||
for _, scheme := range []database.SignatureScheme{
|
||||
database.SignatureSchemeGitHub,
|
||||
database.SignatureSchemeGitLab,
|
||||
} {
|
||||
for _, header := range []string{
|
||||
signature.HeaderGitHub, signature.HeaderGitLab,
|
||||
} {
|
||||
err := signature.Verify(
|
||||
entrypoint(scheme, testSharedKey),
|
||||
headerWith(header, presented),
|
||||
[]byte(testBody),
|
||||
)
|
||||
require.Error(t, err)
|
||||
assert.NotContains(t, err.Error(), testSharedKey)
|
||||
assert.NotContains(t, err.Error(), presented)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func TestSchemeMetadata(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
assert.True(t, signature.Supported(database.SignatureSchemeNone))
|
||||
assert.True(t, signature.Supported(database.SignatureSchemeGitHub))
|
||||
assert.True(t, signature.Supported(database.SignatureSchemeGitLab))
|
||||
assert.False(
|
||||
t, signature.Supported(database.SignatureScheme("stripe")),
|
||||
)
|
||||
|
||||
// The empty scheme describes no sender, so it has no info even
|
||||
// though it is a storable value.
|
||||
_, ok := signature.Info(database.SignatureSchemeNone)
|
||||
assert.False(t, ok)
|
||||
|
||||
info, ok := signature.Info(database.SignatureSchemeGitHub)
|
||||
require.True(t, ok)
|
||||
assert.Equal(t, "GitHub", info.Label)
|
||||
assert.Equal(t, signature.HeaderGitHub, info.Header)
|
||||
|
||||
info, ok = signature.Info(database.SignatureSchemeGitLab)
|
||||
require.True(t, ok)
|
||||
assert.Equal(t, "GitLab", info.Label)
|
||||
assert.Equal(t, signature.HeaderGitLab, info.Header)
|
||||
}
|
||||
@@ -48,7 +48,7 @@
|
||||
|
||||
<div class="divide-y divide-gray-100">
|
||||
{{range .Entrypoints}}
|
||||
<div class="p-4">
|
||||
<div class="p-4" x-data="{ showSecret: false }">
|
||||
<div class="flex items-center justify-between mb-1">
|
||||
<span class="text-sm font-medium text-gray-900">{{if .Description}}{{.Description}}{{else}}Entrypoint{{end}}</span>
|
||||
<div class="flex items-center gap-2">
|
||||
@@ -75,6 +75,34 @@
|
||||
script the URL above stays selectable. -->
|
||||
<button type="button" hidden data-copy-target="entrypoint-url-{{.ID}}" class="text-xs text-gray-500 hover:text-primary-600">Copy</button>
|
||||
</div>
|
||||
<div class="flex items-center gap-2 mt-2">
|
||||
<span class="text-xs text-gray-500">
|
||||
Signature: {{.SchemeLabel}}{{if .SchemeHeader}} ({{.SchemeHeader}}){{end}}
|
||||
</span>
|
||||
<button type="button" @click="showSecret = !showSecret" class="text-xs text-gray-500 hover:text-primary-600">
|
||||
{{if .Configured}}Rotate{{else}}Configure{{end}}
|
||||
</button>
|
||||
</div>
|
||||
<!-- The stored secret is never sent to the browser: the
|
||||
form takes a new one every time, so setting and
|
||||
rotating are the same submission. -->
|
||||
<div x-show="showSecret" x-cloak class="mt-2">
|
||||
<form method="POST" action="/source/{{$.Webhook.ID}}/entrypoints/{{.ID}}/secret" class="flex gap-2">
|
||||
<input type="hidden" name="csrf_token" value="{{$.CSRFToken}}">
|
||||
<select name="signature_scheme" class="input text-sm w-28">
|
||||
<option value="" {{if not .Configured}}selected{{end}}>None</option>
|
||||
{{$current := .Scheme}}
|
||||
{{range $.SignatureSchemes}}
|
||||
<option value="{{.Scheme}}" {{if eq .Scheme $current}}selected{{end}}>{{.Label}}</option>
|
||||
{{end}}
|
||||
</select>
|
||||
<input type="password" name="secret" autocomplete="new-password" placeholder="Shared secret" class="input text-sm flex-1">
|
||||
<button type="submit" class="btn-primary text-sm">Save</button>
|
||||
</form>
|
||||
<p class="text-xs text-gray-500 mt-1">
|
||||
Enter the same secret you configured at the sender. Selecting None removes verification.
|
||||
</p>
|
||||
</div>
|
||||
</div>
|
||||
{{else}}
|
||||
<div class="p-4 text-sm text-gray-500">No entrypoints configured.</div>
|
||||
|
||||
Reference in New Issue
Block a user