Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 14 additions & 10 deletions adcp/v3/signing/MIGRATION.md
Original file line number Diff line number Diff line change
Expand Up @@ -65,18 +65,22 @@ Success signal: signed requests arrive, `signing.VerifiedSignerFromContext(ctx)`

Move the operation to `warn_for`. Verification still runs and failures are logged; traffic is unaffected. Watch your failure rate and walk down the long tail of "some counterparty is misbehaving" before flipping to reject.

This is the spec's shadow-mode stop. The SDK exposes it via `MiddlewareOptions.ObserveOnly` (tracked in [#53](https://github.com/adcontextprotocol/adcp-go/issues/53)); until that lands, approximate it with an `OnReject` that logs-and-passes-through:
This is the spec's shadow-mode stop. The SDK exposes it via `MiddlewareOptions.ObserveOnly`: verification runs on every signed request, but a failure — including an unsigned request to an operation you've also (mistakenly, since `warn_for` and `required_for` are disjoint) left in `RequiredFor` — is logged at INFO instead of rejected, and the request reaches your handler with no `VerifiedSigner` in its context:

```go
// WARNING: step-B shim only. Delete before enabling RequiredFor in step C —
// this turns every required-signed op into an unsigned op.
OnReject: func(w http.ResponseWriter, r *http.Request, e *signing.Error) {
logger.Warn("signature would reject", "code", e.Code, "detail", e.Detail)
// fall through to the next handler — request is NOT rejected.
},
mw := signing.Middleware(signing.MiddlewareOptions{
Resolver: jwksResolver,
Replay: signing.NewMemoryReplayStore(0),
Revocation: signing.NewStaticRevocationList(nil),
OperationResolver: signing.DefaultOperationResolver,
ObserveOnly: true, // step-B: verify, log, never reject
Logger: logger,
})
```

Success signal: grep your logs for the `request signature rejected` message (field `code` = `request_signature_*`) and watch the rate fall to zero — or to a known-and-tolerated set of counterparties — over a window long enough to cover your slowest integrator's deploy cadence.
One case still hard-rejects even under `ObserveOnly`: a partial or malformed `Signature`/`Signature-Input` header pair. Per spec, that pair "cannot be safely interpreted as either signed or unsigned traffic," so it 401s regardless of rollout stage — only a *well-formed* signature that fails verification (bad crypto, unknown key, expired window, replay, ...) is observed-and-passed-through.

Success signal: grep your logs for `"signature verification failed (ObserveOnly...)"` (field `code` = `request_signature_*`, `observe_only=true`) and watch the rate fall to zero — or to a known-and-tolerated set of counterparties — over a window long enough to cover your slowest integrator's deploy cadence.

### Step C — `required_for`

Expand Down Expand Up @@ -142,7 +146,7 @@ Ordering is different — the old kid must stop being trusted *before* anything
- **Clock skew > 60s.** Verifiers reject with `request_signature_window_invalid` when `created` is > 60s in the future or `expires` is > 60s in the past. NTP-sync both sides; investigate container hosts that drift after suspend/resume.
- **Custom `HTTPClient` losing SSRF protection.** If you supply `HTTPJWKSResolver.HTTPClient`, start from `signing.NewSafeHTTPClient()` — otherwise you lose the DNS-rebinding / private-IP / loopback guards, and an attacker who controls a `jwks_uri` can pivot against your internal network.
- **Per-keyid replay cap.** The default in-memory replay store caps at 1,000,000 entries per keyid. Sustained > 3k QPS per signing key will trip `request_signature_rate_abuse`. Deploy a distributed replay store (Redis or equivalent, tracked in [#54](https://github.com/adcontextprotocol/adcp-go/issues/54)) before you get there.
- **`OnReject` that logs-and-passes-through past step B.** The step-B shim above is a footgun if it survives into production — it silently turns every required-signed op into an unsigned op. Before flipping `RequiredFor`, grep your middleware wiring for `OnReject` and confirm the shim is gone. Months later, if you inherit the repo, re-grep.
- **`ObserveOnly: true` that survives past step B.** It's a footgun if it survives into production unnoticed — it silently turns every required-signed op back into an unsigned op. Before flipping `RequiredFor` to enforce, grep your middleware wiring for `ObserveOnly` and confirm it's gone (or scoped to a different middleware instance covering only the operations still in `warn_for`). Months later, if you inherit the repo, re-grep.

## 5. Verification checklist before enforcing

Expand All @@ -152,6 +156,6 @@ Ordering is different — the old kid must stop being trusted *before* anything
- [ ] Revocation source is configured (not `nil` — the middleware logs a warning when `RequiredFor` is non-empty and `Revocation` is nil).
- [ ] Replay store is either in-memory (single instance) or a shared backing store (distributed).
- [ ] Logs from step B show zero unexpected failures over at least one full deploy cycle of your slowest counterparty.
- [ ] No `OnReject` pass-through shim remains in the middleware wiring (grep for `OnReject`).
- [ ] No `ObserveOnly: true` remains on the middleware instance you're about to enforce on (grep for `ObserveOnly`).
- [ ] `CheckRedirect` is set on every signing client.
- [ ] Clock sync monitoring is in place on signer and verifier hosts.
15 changes: 15 additions & 0 deletions adcp/v3/signing/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,21 @@ The package is validated against the spec's [conformance vectors](https://adcont

Vectors live under `testdata/request-signing/`; tests are in `conformance_test.go`.

## Testing handlers that expect signed requests

The [`signingtest`](./signingtest) subpackage collapses the keypair + JWK +
`StaticJWKSResolver` + `NewMemoryReplayStore` boilerplate a handler test
otherwise has to hand-roll:

```go
signer, opts := signingtest.NewTestAgent(t)
opts.OperationResolver = signing.DefaultOperationResolver
opts.RequiredFor = []string{"create_media_buy"}
handler := signing.Middleware(opts)(yourHandler)

resp := signingtest.SignAndSend(t, signer, handler, req)
```

## Signing (buyer side)

```go
Expand Down
14 changes: 14 additions & 0 deletions adcp/v3/signing/doc.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,4 +47,18 @@
// }
// // v.KeyID, v.AgentURL, v.VerifiedAt, v.Algorithm available for audit
// }
//
// # Shadow-mode rollout
//
// MiddlewareOptions.ObserveOnly maps to the spec's warn_for rollout stop
// between supported_for and required_for: verification still runs, but a
// failing request passes to next.ServeHTTP anyway (no VerifiedSigner
// attached), and the failure is logged at INFO instead of rejected. See
// MIGRATION.md's "Step B — warn_for" for the full staged-enforcement recipe.
//
// # Testing handlers that expect signed requests
//
// See the signingtest subpackage for NewTestAgent and SignAndSend, which
// collapse the keypair + JWK + resolver + replay-store setup a handler test
// otherwise has to hand-roll.
package signing
45 changes: 44 additions & 1 deletion adcp/v3/signing/middleware.go
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,32 @@ type MiddlewareOptions struct {
// Set this when your reverse proxy terminates TLS and the verifier sees
// plain HTTP.
SchemeOverride string

// ObserveOnly puts this middleware instance in shadow mode. Verification
// still runs, but a request that fails it — including an unsigned
// request to an operation in RequiredFor — is NOT rejected: next is
// invoked with no VerifiedSigner in the context, exactly as if the
// request had arrived unsigned. The failure is logged at INFO level
// (instead of the usual WARN + 401) via Logger, so operators can watch
// the failure rate before promoting the operation to full enforcement.
//
// This maps to the spec's `warn_for` rollout stop — the shadow-mode
// bridge between `supported_for` (verified when present, never required)
// and `required_for` (verified and mandatory). Wire a dedicated
// Middleware instance with ObserveOnly=true for the operations you've
// moved to `warn_for`; RequiredFor should normally be empty on that
// instance, since `warn_for` and `required_for` are disjoint per spec —
// see https://adcontextprotocol.org/docs/building/implementation/security#transport-capability-advertisement.
//
// One case still hard-rejects even under ObserveOnly: a partial or
// malformed Signature/Signature-Input header pair (one header present
// without the other, or either header present but unparseable) —
// surfaced as *Error{Code: CodeHeaderMalformed}. The spec requires this
// because a broken pair "cannot be safely interpreted as either signed
// or unsigned traffic"; a well-formed signature that merely fails
// verification (bad crypto, unknown key, expired window, replay, ...) is
// the case ObserveOnly is for and passes through.
ObserveOnly bool
}

// Middleware returns an http.Handler middleware that verifies incoming AdCP
Expand Down Expand Up @@ -187,13 +213,30 @@ func Middleware(opts MiddlewareOptions) func(http.Handler) http.Handler {
if parsed, perr := parseSignatureInput(r.Header.Get(signatureInputHeader)); perr == nil {
keyid = parsed.keyID
}
logger.Warn("signature rejected",
// ObserveOnly never overrides a malformed header pair — that
// case cannot be safely treated as unsigned traffic (see the
// ObserveOnly doc comment).
observing := opts.ObserveOnly && e.Code != CodeHeaderMalformed
level := slog.LevelWarn
msg := "signature rejected"
if observing {
level = slog.LevelInfo
msg = "signature verification failed (ObserveOnly: request allowed through unverified)"
}
logger.Log(r.Context(), level, msg,
"code", e.WireCode(profile),
"detail", e.Detail,
"op", opName,
"keyid", keyid,
"profile", profile.Tag,
"observe_only", observing,
)
if observing {
// No VerifiedSigner is attached — downstream sees this
// exactly as an unsigned request.
next.ServeHTTP(w, r)
return
}
if opts.OnReject != nil {
opts.OnReject(w, r, e)
return
Expand Down
Loading
Loading