Skip to content
Merged
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
7 changes: 6 additions & 1 deletion cmd/conformance-as/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,10 @@ import (
// httpFetchTimeout bounds a single outbound client-JWKS fetch.
const httpFetchTimeout = 10 * time.Second

// dpopNonceLifetime bounds how long a DPoP nonce issued under
// -dpop-nonce-challenge remains valid.
const dpopNonceLifetime = time.Minute

// readHeaderTimeout bounds how long the server waits to receive a
// request's headers — without it, a client that trickles headers one
// byte at a time can hold a connection (and its goroutine) open
Expand Down Expand Up @@ -47,6 +51,7 @@ func main() {
keyOverride := flag.String("key", "", "override tls.key_file from the config file")
insecureHTTP := flag.Bool("insecure-http", false, "serve plaintext HTTP instead of TLS (loopback listen_addr only)")
accessTokenFormat := flag.String("access-token-format", string(AccessTokenFormatJWT), "access token format to issue/verify: jwt or opaque")
dpopNonceChallenge := flag.Bool("dpop-nonce-challenge", false, "require and rotate a DPoP nonce on /accounts and /userinfo (RFC 9449 §8/§9) — off by default, since the OIDF suite's own protected-resource caller may not retry on the challenge")
flag.Parse()

if *configPath == "" {
Expand Down Expand Up @@ -75,7 +80,7 @@ func main() {
log.Fatal("conformance-as: -insecure-http requires a loopback listen_addr (e.g. 127.0.0.1:8443)")
}

mux, err := newServerMux(resolved, *insecureHTTP)
mux, err := newServerMux(resolved, *insecureHTTP, *dpopNonceChallenge)
if err != nil {
log.Fatalf("conformance-as: %v", err)
}
Expand Down
13 changes: 13 additions & 0 deletions cmd/conformance-as/resource.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,9 @@
writeResourceError(w, err)
return
}
if authCtx.NextDPoPNonce != "" {
w.Header().Set("DPoP-Nonce", authCtx.NextDPoPNonce)

Check failure on line 65 in cmd/conformance-as/resource.go

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Define a constant instead of duplicating this literal "DPoP-Nonce" 3 times.

See more on https://sonarcloud.io/project/issues?id=IDFoundry_FAPIgo&issues=AaBEBfqTUJwT2axngd90&open=AaBEBfqTUJwT2axngd90&pullRequest=166
}
w.Header().Set("Content-Type", "application/json")
_ = json.NewEncoder(w).Encode(map[string]any{
"subject": authCtx.Subject,
Expand Down Expand Up @@ -138,6 +141,9 @@
}
body["sub"] = authCtx.Subject

if authCtx.NextDPoPNonce != "" {
w.Header().Set("DPoP-Nonce", authCtx.NextDPoPNonce)
}
w.Header().Set("Content-Type", "application/json")
_ = json.NewEncoder(w).Encode(body)
}
Expand All @@ -149,6 +155,13 @@
http.Error(w, "internal error", http.StatusInternalServerError)
return
}
// Set before writeResourceErrorRaw's own WriteHeader call — a
// header can't be added after that. Only ErrorUseDPoPNonce ever
// carries a nonce (Error.Nonce's own doc comment), so this is a
// no-op for every other rejection.
if resErr.Nonce() != "" {
w.Header().Set("DPoP-Nonce", resErr.Nonce())
}
writeResourceErrorRaw(w, resErr.HTTPStatus(), string(resErr.Code()), resErr.PublicDescription())
}

Expand Down
57 changes: 56 additions & 1 deletion cmd/conformance-as/smoke_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,10 @@ type smokeHarness struct {
}

func newSmokeHarness(t *testing.T, format AccessTokenFormat) *smokeHarness {
return newSmokeHarnessWithNonceChallenge(t, format, false)
}

func newSmokeHarnessWithNonceChallenge(t *testing.T, format AccessTokenFormat, dpopNonceChallenge bool) *smokeHarness {
t.Helper()

cert, pool := selfSignedCert(t)
Expand All @@ -140,6 +144,10 @@ func newSmokeHarness(t *testing.T, format AccessTokenFormat) *smokeHarness {
if err != nil {
t.Fatalf("build endpoints: %v", err)
}
userinfoURL, err := buildResourceURL(issuer, false, "/userinfo")
if err != nil {
t.Fatalf("build userinfo url: %v", err)
}

clientKeyManager, err := ephemeral.NewKeyManager(map[keys.SigningPurpose]fapi.SignatureAlgorithm{
keys.ClientAuthentication: fapi.ES256,
Expand Down Expand Up @@ -188,7 +196,7 @@ func newSmokeHarness(t *testing.T, format AccessTokenFormat) *smokeHarness {
AdvertisedScopes: []string{"openid", "accounts", "offline_access"},
}

mux, err := newServerMux(resolved, false)
mux, err := newServerMux(resolved, false, dpopNonceChallenge)
if err != nil {
t.Fatalf("build server mux: %v", err)
}
Expand Down Expand Up @@ -226,6 +234,7 @@ func newSmokeHarness(t *testing.T, format AccessTokenFormat) *smokeHarness {
Authorization: endpoints.Authorization,
Token: endpoints.Token,
PushedAuthorizationRequest: endpoints.PushedAuthorizationRequest,
UserInfo: userinfoURL,
},
Profile: client.ProfileFAPISecurity,
Algorithms: client.Algorithms{
Expand Down Expand Up @@ -386,6 +395,52 @@ func testSmokeAuthorizationCodeFlow(t *testing.T, format AccessTokenFormat) {
}
}

// TestSmokeUserInfoWithDPoPNonceChallenge confirms this binary's
// -dpop-nonce-challenge wiring actually interoperates with the client
// package's own nonce-retry logic (client/resource.go's
// ResourceClient.Do), not just each side's own unit tests in isolation:
// FetchUserInfo must succeed transparently even though the very first
// call is challenged and retried entirely inside the client, with no
// special handling from this test.
func TestSmokeUserInfoWithDPoPNonceChallenge(t *testing.T) {
h := newSmokeHarnessWithNonceChallenge(t, AccessTokenFormatJWT, true)
ctx := context.Background()
scope := []string{"openid", "accounts"}

handle := h.runToConsent(ctx, scope)
rawQuery := h.submitDecision(ctx, handle, "approve", scope)

result, err := h.client.CompleteAuthorization(ctx, client.AuthorizationCallback{RawQuery: rawQuery})
if err != nil {
t.Fatalf("CompleteAuthorization: %v", err)
}
success, ok := result.(client.CompletionSuccess)
if !ok {
t.Fatalf("CompleteAuthorization result = %T, want client.CompletionSuccess", result)
}

info, err := h.client.FetchUserInfo(ctx, success.Tokens)
if err != nil {
t.Fatalf("FetchUserInfo (should retry the nonce challenge transparently): %v", err)
}
if info.Subject != smokeSubject {
t.Fatalf("UserInfo.Subject = %q, want %q", info.Subject, smokeSubject)
}

// A second, independent call must succeed too — confirming the
// server's single-use nonce store doesn't end up in some broken
// state after one full challenge/consume/reissue cycle. (client
// itself has no cross-call nonce cache — ResourceClient.Do only
// reacts to a challenge within the one call it's already making —
// so this still exercises its own full challenge/retry round trip,
// same as the first call above; the server's proactive
// NextDPoPNonce only benefits a caller that tracks and reuses it
// itself, which this client doesn't do.)
if _, err := h.client.FetchUserInfo(ctx, success.Tokens); err != nil {
t.Fatalf("FetchUserInfo (second call): %v", err)
}
}

func TestSmokeAuthorizationDenied(t *testing.T) {
h := newSmokeHarness(t, AccessTokenFormatJWT)
ctx := context.Background()
Expand Down
20 changes: 16 additions & 4 deletions cmd/conformance-as/wiring.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import (
// Factored out so the end-to-end smoke test can stand up the exact same
// wiring main.go uses, against its own TLS listener, without going
// through flags or a config file on disk.
func newServerMux(resolved ResolvedConfig, allowLoopbackHTTP bool) (*http.ServeMux, error) {
func newServerMux(resolved ResolvedConfig, allowLoopbackHTTP bool, dpopNonceChallenge bool) (*http.ServeMux, error) {
endpoints, err := buildEndpoints(resolved.Issuer, allowLoopbackHTTP)
if err != nil {
return nil, err
Expand Down Expand Up @@ -146,17 +146,29 @@ func newServerMux(resolved ResolvedConfig, allowLoopbackHTTP bool) (*http.ServeM
return nil, err
}

resourceVerifier, err := fapires.NewVerifier(fapires.Config{
resourceCfg := fapires.Config{
Limits: fapires.Limits{
MaxDPoPProofAge: resolved.Limits.MaxDPoPProofAge,
MaxClockSkew: resolved.Limits.MaxClockSkew,
},
}, fapires.Dependencies{
}
resourceDeps := fapires.Dependencies{
AccessTokens: resourceAccessTokens,
Replay: replayStore,
Revocation: revocationStore,
Clock: fapires.SystemClock{},
})
}
// Off by default (main.go's -dpop-nonce-challenge flag): the OIDF
// suite's own AS-plan protected-resource caller isn't guaranteed to
// implement the client-side nonce-challenge retry the way this
// module's own client package does, so turning this on
// unconditionally would risk breaking unrelated AS conformance.
if dpopNonceChallenge {
resourceCfg.Limits.DPoPNonceLifetime = dpopNonceLifetime
resourceDeps.Nonces = memstore.NewNonceStore()
resourceDeps.Random = rand.Reader
}
resourceVerifier, err := fapires.NewVerifier(resourceCfg, resourceDeps)
if err != nil {
return nil, err
}
Expand Down
41 changes: 41 additions & 0 deletions internal/dpop/dpop_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -283,6 +283,47 @@ func TestVerifyNonceBinding(t *testing.T) {
}
}

// TestVerifyReturnsProofNonce confirms VerifiedProof.Nonce surfaces the
// proof's own "nonce" claim unconditionally — even with no RequiredNonce
// set — since a caller implementing single-use, issued-per-challenge
// nonces (unlike RequiredNonce's compare-to-one-known-value check) needs
// the presented value itself to look up, not just a pass/fail verdict.
func TestVerifyReturnsProofNonce(t *testing.T) {
key := generateKey(t)
now := time.Now()
target := mustURL(t, "https://as.example/token")

withNonce, err := CreateProof(ProofRequest{
Signer: key, Algorithm: fapi.ES256, Method: "POST", URL: target, Now: now, Nonce: "presented-nonce",
})
if err != nil {
t.Fatalf("CreateProof: %v", err)
}
verified, err := Verify(context.Background(), VerifyRequest{
Proof: withNonce, Method: "POST", URL: target, Now: now, MaxProofAge: time.Minute,
})
if err != nil {
t.Fatalf("Verify(with nonce): %v", err)
}
if verified.Nonce != "presented-nonce" {
t.Fatalf("VerifiedProof.Nonce = %q, want %q", verified.Nonce, "presented-nonce")
}

withoutNonce, err := CreateProof(ProofRequest{Signer: key, Algorithm: fapi.ES256, Method: "POST", URL: target, Now: now})
if err != nil {
t.Fatalf("CreateProof: %v", err)
}
verified, err = Verify(context.Background(), VerifyRequest{
Proof: withoutNonce, Method: "POST", URL: target, Now: now, MaxProofAge: time.Minute,
})
if err != nil {
t.Fatalf("Verify(without nonce): %v", err)
}
if verified.Nonce != "" {
t.Fatalf("VerifiedProof.Nonce = %q, want empty", verified.Nonce)
}
}

func TestVerifyRequiresMaxProofAge(t *testing.T) {
key := generateKey(t)
now := time.Now()
Expand Down
13 changes: 11 additions & 2 deletions internal/dpop/verify.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,10 +63,19 @@ type VerifyRequest struct {

// VerifiedProof is everything about a DPoP proof worth retaining once it
// has been verified: the thumbprint of the key it was signed with (for
// binding against a token's cnf.jkt) and when it was issued.
// binding against a token's cnf.jkt), when it was issued, and the
// proof's own "nonce" claim (empty if absent).
//
// Nonce is handed back unconditionally, regardless of RequiredNonce —
// unlike RequiredNonce's own compare-to-one-known-value check, a caller
// implementing single-use, issued-per-challenge nonces (RFC 9449 §8,
// §9) doesn't know the expected value in advance; it has to look up
// whether the presented value was one it actually issued, which only it
// (not this package) can do.
type VerifiedProof struct {
Thumbprint jose.Thumbprint
IssuedAt time.Time
Nonce string
}

// Verify checks a DPoP proof against req. On success, the returned
Expand Down Expand Up @@ -157,5 +166,5 @@ func Verify(ctx context.Context, req VerifyRequest) (VerifiedProof, error) {
if err != nil {
return VerifiedProof{}, fmt.Errorf("dpop: %w", err)
}
return VerifiedProof{Thumbprint: thumbprint, IssuedAt: iat}, nil
return VerifiedProof{Thumbprint: thumbprint, IssuedAt: iat, Nonce: c.Nonce}, nil
}
7 changes: 7 additions & 0 deletions resource/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,13 @@ type Limits struct {
// extends how long past a token's own expiry it's still accepted.
// Zero means no tolerance.
MaxClockSkew time.Duration

// DPoPNonceLifetime bounds how long an issued DPoP nonce remains
// valid. Required only when Dependencies.Nonces is non-nil — see
// its own doc comment; NewVerifier rejects a zero value in that
// case, exactly like MaxDPoPProofAge above, but leaves it
// unvalidated when nonce-challenge support is disabled.
DPoPNonceLifetime time.Duration
}

// Config is this verifier's immutable configuration. It is copied by
Expand Down
27 changes: 25 additions & 2 deletions resource/dependencies.go
Original file line number Diff line number Diff line change
@@ -1,12 +1,21 @@
package resource

import (
"io"

"github.com/idfoundry/fapigo/storage"
)

// Dependencies are this verifier's injected collaborators. NewVerifier
// rejects a nil value for any field — there is no implicit fallback (no
// default clock, no silently-installed in-memory replay store).
// rejects a nil value for any field required unconditionally — there is
// no implicit fallback (no default clock, no silently-installed
// in-memory replay store). Nonces/Random are the one exception: unlike
// every other field, DPoP nonce-challenge support (RFC 9449 §8, §9) is
// genuinely optional protocol behavior, not a security check this
// module considers non-negotiable — leaving Nonces nil disables it
// entirely, with no visible-opt-out sentinel needed the way
// Revocation's NoRevocation{} is, because declining it is the normal,
// fully spec-compliant default, not a choice that needs to be visible.
type Dependencies struct {
// AccessTokens resolves a presented access token's claims — see
// AccessTokenResolver. Required — pass JWTAccessTokens{...} (the
Expand All @@ -25,4 +34,18 @@ type Dependencies struct {

// Clock supplies the current time.
Clock Clock

// Nonces persists DPoP nonces this verifier issues and consumes.
// Nil disables DPoP nonce-challenge support entirely — Verify never
// requires or checks a nonce, exactly like today. Set it (and
// Random, and Config.Limits.DPoPNonceLifetime) to have Verify
// challenge a request whose proof carries no valid nonce, and
// proactively reissue a fresh one on every successful call.
Nonces storage.NonceStore

// Random is the source of randomness for DPoP nonce generation.
// Required only when Nonces is non-nil — NewVerifier rejects nil
// Random in that case, matching server.Dependencies.Random's own
// "no implicit fallback" rule.
Random io.Reader
}
16 changes: 16 additions & 0 deletions resource/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,15 @@ const (
ErrorInvalidRequest ErrorCode = "invalid_request"
ErrorInvalidToken ErrorCode = "invalid_token"
ErrorServerError ErrorCode = "server_error"

// ErrorUseDPoPNonce indicates the presented DPoP proof was otherwise
// valid but carried no nonce, or one this verifier didn't just issue
// (unknown, already consumed, or expired) — RFC 9449 §8's own error
// value, distinct from ErrorInvalidToken (the token itself is fine;
// the caller just needs to retry with the nonce this error's own
// Nonce method returns). Only ever returned when
// Dependencies.Nonces is configured.
ErrorUseDPoPNonce ErrorCode = "use_dpop_nonce"
)

// Error is the error type Verify returns. Code and PublicDescription
Expand All @@ -23,6 +32,7 @@ type Error struct {
httpStatus int
description string
cause error
nonce string
}

func newError(code ErrorCode, httpStatus int, description string, cause error) *Error {
Expand All @@ -35,6 +45,12 @@ func (e *Error) Code() ErrorCode { return e.code }
// PublicDescription returns a short, safe-to-expose description.
func (e *Error) PublicDescription() string { return e.description }

// Nonce returns the nonce a caller should present on retry, alongside
// this error's own DPoP challenge — non-empty only when Code is
// ErrorUseDPoPNonce, in which case it belongs in the response's own
// DPoP-Nonce header (RFC 9449 §8).
func (e *Error) Nonce() string { return e.nonce }

// HTTPStatus returns the HTTP status code an adapter should respond
// with.
func (e *Error) HTTPStatus() int { return e.httpStatus }
Expand Down
Loading
Loading