Some checks failed
check / check (push) Failing after 2m31s
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. Under the GitLab scheme the signature header is the secret rather than a digest over the request, so an accepted request's headers are cloned and the configured scheme's credential header dropped before they are serialized onto the event. Stored headers are persisted verbatim in the per-webhook database and replayed onto every outbound delivery, so keeping the token would put it in every backup and hand every target operator the means to forge signed requests to the entrypoint it authenticates. Stripping sits once above the first write rather than at each egress, and is driven by the scheme's own description with stripping as the default: a scheme added later is covered unless it declares its header a digest, as GitHub's HMAC over the body does. An entrypoint holding one half of the pair now renders as misconfigured rather than as unverified, and the scheme selector follows the stored scheme so such a row no longer marks two options selected.
341 lines
7.8 KiB
Go
341 lines
7.8 KiB
Go
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)
|
|
}
|