Skip to content

Commit cdab20f

Browse files
committed
Validate aud and resource claims in returned ID-JAG JWT
The XAA IdP exchange (RFC 8693) returned an ID-JAG JWT without validating its aud or resource claims against the values requested in the exchange. If the IdP minted an ID-JAG for a different audience or resource, ToolHive forwarded it to the target AS unchanged — a token- misbinding / confused-deputy risk. Add validateIDJAGClaims to check the aud claim (REQUIRED per draft §3.1, membership check per RFC 7519 §4.1.3) and the resource claim (OPTIONAL per §3.1, validated when one was requested) inside the returned ID-JAG JWT. A non-JWT assertion that fails to parse is now rejected rather than forwarded. The typ header check remains logged- only by design (the draft §3.1 MUST is on the issuer; ToolHive is the holder/presenter). Closes #5696
1 parent 7d08e2e commit cdab20f

2 files changed

Lines changed: 438 additions & 27 deletions

File tree

pkg/vmcp/auth/strategies/xaa.go

Lines changed: 100 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -224,7 +224,10 @@ func (*XAAStrategy) authenticateWithClientCredentials(
224224

225225
// performIDPExchange runs the RFC 8693 token exchange at the IdP to obtain an
226226
// ID-JAG. It verifies the response's issued_token_type matches the ID-JAG URN
227-
// per draft-ietf-oauth-identity-assertion-authz-grant §4.3.3.
227+
// (draft §4.3.3) and that token_type is N_A (draft §5.2), logs a mismatch on
228+
// the JWT typ header, rejects an empty or unparseable assertion, and validates
229+
// the aud/resource binding claims against what was requested (confused-deputy
230+
// defence; see validateIDJAGClaims).
228231
func (*XAAStrategy) performIDPExchange(
229232
ctx context.Context, idToken string, config *xaaParsedConfig,
230233
) (string, error) {
@@ -277,19 +280,107 @@ func (*XAAStrategy) performIDPExchange(
277280
const idJAGJWTType = "oauth-id-jag+jwt"
278281
parser := jwt.NewParser()
279282
parsed, _, parseErr := parser.ParseUnverified(assertion, jwt.MapClaims{})
280-
if parseErr == nil {
281-
if typ, _ := parsed.Header["typ"].(string); !strings.EqualFold(typ, idJAGJWTType) {
282-
slog.Debug("xaa: ID-JAG JWT typ header mismatch",
283-
"got", typ, "want", idJAGJWTType,
284-
)
285-
}
286-
} else {
287-
slog.Debug("xaa: could not parse ID-JAG JWT header for typ check", "error", parseErr)
283+
if parseErr != nil {
284+
// A non-parseable assertion cannot carry valid aud/resource claims and
285+
// therefore cannot be a valid ID-JAG. Fail fast rather than forwarding
286+
// an unvalidated assertion to the target AS.
287+
return "", fmt.Errorf("IdP exchange: ID-JAG assertion is not a valid JWT: %w", parseErr)
288+
}
289+
if typ, _ := parsed.Header["typ"].(string); !strings.EqualFold(typ, idJAGJWTType) {
290+
slog.Debug("xaa: ID-JAG JWT typ header mismatch",
291+
"got", typ, "want", idJAGJWTType,
292+
)
293+
}
294+
// The assertion cannot fail: ParseUnverified was called with a literal
295+
// jwt.MapClaims{}, which it decodes into in place. The !ok branch is
296+
// defensive.
297+
claims, ok := parsed.Claims.(jwt.MapClaims)
298+
if !ok {
299+
return "", fmt.Errorf("IdP exchange: ID-JAG claims are not MapClaims")
300+
}
301+
if err := validateIDJAGClaims(claims, config.targetAudience, config.targetResource); err != nil {
302+
return "", err
288303
}
289304

290305
return assertion, nil
291306
}
292307

308+
// validateIDJAGClaims checks the aud and resource claims of the returned ID-JAG
309+
// JWT against the values requested in the IdP exchange (draft §3.1, §5).
310+
//
311+
// Per draft §4.4.1, aud MUST be a single-value issuer identifier: either a JSON
312+
// string, or a JSON array containing exactly one element. A multi-element aud
313+
// array is malformed and is rejected (see audMatches). resource is checked
314+
// only when one was requested (the resource parameter is OPTIONAL per draft
315+
// §4.3); when one was requested, ToolHive requires the returned ID-JAG to echo
316+
// it — a deliberate strict-binding policy, stricter than the draft, which
317+
// leaves resource OPTIONAL in the response even when requested.
318+
//
319+
// This is defence-in-depth against token misbinding: if the IdP mints an ID-JAG
320+
// for a different audience/resource, the target AS should reject it — but
321+
// ToolHive fails fast rather than forwarding a mismatched assertion.
322+
//
323+
// Per RFC 7519 §4.1.3, the aud claim may be a string or an array of strings;
324+
// golang-jwt/v5 decodes a JSON array as []interface{} whose elements are
325+
// strings. resource legitimately allows an array of URIs per draft §3.1, so a
326+
// membership check (claimContainsString) is used there; aud does not permit
327+
// multiple values, so a dedicated single-value check (audMatches) is used
328+
// instead.
329+
func validateIDJAGClaims(claims jwt.MapClaims, wantAudience, wantResource string) error {
330+
// aud is REQUIRED and single-valued per draft §4.4.1.
331+
if !audMatches(claims, wantAudience) {
332+
return fmt.Errorf("IdP exchange: ID-JAG aud claim %v does not match requested audience %q", claims["aud"], wantAudience)
333+
}
334+
335+
// Deliberately stricter than the draft: resource is OPTIONAL in the ID-JAG
336+
// even when requested, but ToolHive rejects an ID-JAG that omits it once
337+
// the operator has configured a resource to bind to.
338+
if wantResource != "" && !claimContainsString(claims, "resource", wantResource) {
339+
return fmt.Errorf(
340+
"IdP exchange: ID-JAG resource claim %v does not contain requested resource %q",
341+
claims["resource"], wantResource,
342+
)
343+
}
344+
345+
return nil
346+
}
347+
348+
// audMatches reports whether the aud claim is a single-value issuer identifier
349+
// equal to want, per draft §4.4.1: either a JSON string, or a JSON array
350+
// containing exactly one element. Any other shape — missing, multi-element
351+
// array, empty array, wrong type, or a non-string element — is rejected.
352+
func audMatches(claims jwt.MapClaims, want string) bool {
353+
switch v := claims["aud"].(type) {
354+
case string:
355+
return v == want
356+
case []interface{}:
357+
if len(v) != 1 {
358+
return false
359+
}
360+
s, ok := v[0].(string)
361+
return ok && s == want
362+
default:
363+
return false
364+
}
365+
}
366+
367+
// claimContainsString reports whether the named claim contains want. Per RFC
368+
// 7519 the claim may be a single string or an array; golang-jwt/v5 decodes JSON
369+
// arrays as []interface{} whose elements are strings.
370+
func claimContainsString(claims jwt.MapClaims, key, want string) bool {
371+
switch v := claims[key].(type) {
372+
case string:
373+
return v == want
374+
case []interface{}:
375+
for _, e := range v {
376+
if s, ok := e.(string); ok && s == want {
377+
return true
378+
}
379+
}
380+
}
381+
return false
382+
}
383+
293384
// performTargetGrant runs the RFC 7523 JWT Bearer grant at the target AS using
294385
// the ID-JAG assertion returned by IdP exchange.
295386
func (*XAAStrategy) performTargetGrant(

0 commit comments

Comments
 (0)