Skip to content

Harden the RFC 8705 mTLS token endpoint - Supersedes 3792 - #4076

Open
fhanik wants to merge 65 commits into
cloudfoundry:developfrom
fhanik:review/pr3792-fix
Open

fhanik wants to merge 65 commits into
cloudfoundry:developfrom
fhanik:review/pr3792-fix

Conversation

@fhanik

@fhanik fhanik commented Sep 16, 2026 •

Copy link
Copy Markdown
Contributor

PR #4076 — description draft

Paste-ready text for the pull request. Keep it in step with pr4076-develop-merge-evaluation.md, which holds the
evidence behind every statement here.


RFC 8705 mutual-TLS client authentication (tls_client_auth), hardened

Builds on #3972. Adds tls_client_auth client authentication at a dedicated /oauth/mtls/token endpoint, with
certificate-bound access tokens (cnf.x5t#S256), per-client trust configuration and optional RFC 8707 resource
indicators. Off by default (uaa.mtls-enabled, default false).

What this adds beyond #3972

  • RFC 8705 section 2.1.2 subject binding. A client with tls-client-auth-ca must also register exactly one expected
    subject value (tls_client_auth_subject_dn or one of the four tls_client_auth_san_* parameters). Chain validation
    alone only proves who issued a certificate; on a CA shared across tenants (Diego's instance-identity CA) it let any
    certificate holder authenticate as any client trusting that CA.
  • The endpoint is POST-only, serves nothing below its path, issues only client_credentials, and is not an alias of
    /oauth/token.
  • CA rotation bundles are fully trusted; expired certificates and CA certificates used as the leaf are refused.
  • Claim mappings cannot target UAA-owned or authentication-context claims (amr, acr, cnf, ...), and a mapping's
    regex is bounded (256-character OU cap, 100 000-character match budget) so it cannot hang token issuance.
  • Optional RFC 8707 resource parameter against a per-client allow-list (tls-client-auth-allowed-resources).
  • Zone isolation in both zone-addressing modes.

Behaviour with the flag off

The endpoint answers 404, the Tomcat connector is untouched, no mTLS beans or filter chain are registered, the
registration APIs and oauth.clients bootstrap reject the keys, and discovery advertises nothing. The flag is read one
way everywhere (true, 1, yes, on), so it cannot half-enable.

Changes that reach deployments without the flag — please read

Change Who is affected
Discovery gains tls_client_certificate_bound_access_tokens (false when off; RFC 8705 section 3.3 treats omitted as false) everyone, additive
An access token issued by a refresh no longer carries granted_scopes. It records the full consented scope set and belongs on the refresh token only; copying it onto a deliberately narrowed access token disclosed more than the caller was given. deployments that register any UaaTokenEnhancer (stock UAA registers none unless this feature is on)
UaaTokenEnhancer gains a default method getLateOverrideClaims() (empty set). Other enhancers are not restricted or altered; MtlsClaimsEnhancer opts in for its own sub/aud. source-compatible for all implementations
ClientAdminEndpointsValidator, ClientAdminBootstrap, ClientDetailsAuthenticationProvider, ZoneEndpointsClientDetailsValidator gained constructor parameters only code that constructs them by hand
UaaAuthenticationDetails gains a field and has no serialVersionUID; session rows persisted by the previous version are expected to fail to deserialize after an upgrade every deployment, at upgrade time — see below
A client whose additionalInformation already contains a key named tls-client-auth-ca is now treated as mTLS-only none expected; nothing in UAA defined that key before

UaaAuthenticationDetails is the one item here with no mitigation yet. It has been analysed but not exercised against a
live rolling upgrade; making the new field transient, or pinning serialVersionUID, removes the concern.

Operational notes

  • Enabling the flag is connector-wide: every TLS handshake to the instance now carries a CertificateRequest with no
    issuer constraint, and the connector moves to the FIPS BouncyCastle JSSE provider (required for TLS 1.3
    client-certificate requests). Soak-test interop and performance before enabling on a live foundation.
  • UAA stamps cnf but does not enforce it; resource servers must compare the thumbprint with the certificate they see.
  • Not implemented: revocation checking (PKIX runs with revocation off), per-zone enablement, audit events for mTLS token
    issuance, a dedicated rate-limit mapping for /oauth/mtls/token, self_signed_tls_client_auth and public-client
    certificate binding.

Tests

New: roughly 330 test cases (222 in new files, the rest added to existing classes) across unit, Spring-context,
MockMvc (including the flag-off suite and a flag-consistency suite) and a real
embedded-Tomcat TLS integration test. No pre-existing test was deleted, renamed or disabled; the handful of changed
existing tests are listed with reasons in pr4076-code-review-guide.md section 2.

Documentation

docs/UAA-Client-Authentication.md (configuration, certificate-bound tokens, error responses, compatibility notes),
docs/UAA-Configuration-Reference.md (uaa.mtls-enabled), the generated API docs (discovery, token endpoint,
introspection and check-token cnf, client registration keys) and the slate source.

PR #4076 — code-review guide

Pull request: #4076 ·
base: cloudfoundry/develop (diff taken from merge-base 6cc037111, the tip of develop that the
branch forked from) · head: review/pr3792-fix ·
excluding documentation: 86 files changed, 12059 additions, 46 deletions.

How to read this

  • Every file and line reference is a link into the PR's "Files changed" tab (#diff-<hash> anchors,
    R<n> = line n on the new side). They open the file's diff and scroll to the range.
    GitHub collapses large diffs — if an anchor lands on a collapsed file, click Load diff.
  • The links assume the PR head contains the latest push. Line anchors move if the branch is rebased or new commits are
    added.
  • Flag column (uaa.mtls-enabled, default false):
Mark Meaning
✅ Gated does nothing observable when the flag is off
⚪ Inert not flag-gated, but cannot act unless something only the flag enables is present (e.g. path that 404s, a config key that can't be registered)
⚠️ Un-gated change behaviour changes without uaa.mtls-enabled being on — for every deployment, or (where stated) only for deployments that register a token enhancer. Review these hardest
🔧 Build/plumbing no runtime behaviour of its own

Summary of the ⚠️ items (details in §1): M1 granted_scopes is no longer copied from a refresh token onto the
refreshed access token — only for deployments that register a token enhancer (stock UAA with the flag off has none), but
then regardless of the flag (UaaTokenServices); M2 UaaAuthenticationDetails gains a field and
has no serialVersionUID (UaaAuthenticationDetails); widened UAA_SUPPORTED_METHODS
(ClientAuthentication) — contained by an external-IdP split;
tls-client-auth-ca is now an interpreted additionalInformation key
(ClientDetailsAuthenticationProvider).
Fuller reasoning: docs/PR 4076 Thoughts/pr4076-develop-merge-evaluation.md.


Section 1 — Changed production files

Grouped by module; within a module, ordered roughly by how much a reviewer should care.

1.1 Server — request path, security chain, token issuance

UaaTokenServices

server/src/main/java/org/cloudfoundry/identity/uaa/oauth/UaaTokenServices.java

whole file NON_ADDITIONAL_ROOT_CLAIMS lateOverrideClaims into createJWTAccessToken late overrides applied collect opt-ins from each enhancer
  • What changed: (1) GRANTED_SCOPES added to NON_ADDITIONAL_ROOT_CLAIMS. (2) Enhancer output is applied to the
    access token exactly as on develop — before UAA's defaults, which take precedence for the claims UAA sets. (3)
    New: after its defaults, UAA applies the claims an enhancer both named in getLateOverrideClaims() and returned
    (lateOverrideClaims, threaded through createCompositeToken / createJWTAccessToken; the refresh path passes an
    empty map).
  • Why: (1) A refresh must not copy the full consented-scope set onto a deliberately narrowed access token; the old
    refreshTokenClaims.remove(GRANTED_SCOPES) ran after the copy loop and never worked. (3) MtlsClaimsEnhancer's
    certificate-identity sub/aud must survive UAA's defaults. It is an opt-in so that no other enhancer is
    restricted or changed: a third-party enhancer is responsible for its own output. (An earlier version of this PR
    instead filtered protected claim names out of every enhancer's output and let any enhancer's sub/aud win; that
    was reverted — see pr4076-develop-merge-evaluation.md M3.)
  • Flag: (1) ⚠️ Not gated by the flag, gated by the enhancer list (M1). getAdditionalRootClaims copies
    refresh-token claims only when uaaTokenEnhancers is non-empty, so refreshed access tokens stop carrying
    granted_scopes for any deployment that registers a token enhancer — flag or no flag; stock UAA with the flag off
    has none and is unaffected. Kept by decision; belongs in the PR description. (2)/(3) ✅ no change for any enhancer
    that does not opt in, and nothing in the tree opts in except MtlsClaimsEnhancer (flag-gated).

UaaTokenEnhancer

server/src/main/java/org/cloudfoundry/identity/uaa/oauth/UaaTokenEnhancer.java

whole file getLateOverrideClaims
  • What changed: New default Set<String> getLateOverrideClaims() returning an empty set, with javadoc.
  • Why: The explicit opt-in that lets one enhancer (the mTLS one) keep claims like sub/aud past UAA's defaults
    without changing what any other enhancer can do.
  • Flag: ✅ Source- and behaviour-compatible. A default method: existing implementations (including
    closed-source ones) compile unchanged and get the empty set, i.e. exactly the previous behaviour. Only a new
    abstract method would have broken them.

ClientDetailsAuthenticationProvider

server/src/main/java/org/cloudfoundry/identity/uaa/authentication/ClientDetailsAuthenticationProvider.java

whole file mTLS branch in additionalAuthenticationChecks path check / validateTlsClientAuth / config parsing
  • What changed: Adds a TlsClientAuthentication collaborator (constructor signature change). A client whose
    additionalInformation has tls-client-auth-ca is exclusively mTLS: it must authenticate at /oauth/mtls/token
    with no credentials, must register exactly one RFC 8705 §2.1.2 subject value, and is refused at /oauth/token.
    Conversely /oauth/mtls/token refuses any client without that config (so it is not an alias of /oauth/token).
    Validation = PKIX chain → TlsClientAuthSubjectMatcher → required-claims. A client with tls-client-auth-ca whose
    configuration cannot be read is refused rather than treated as ordinary. InvalidClientDetailsException is
    converted to BadCredentialsException (previously a 500 on the Basic-auth path). isTlsClientAuthPath answers true
    if either the decoded servlet path or the raw request path resolves to the endpoint (fail closed; handles
    /oauth/%6dtls/token).
  • Why: RFC 8705 client authentication, and closing the cross-tenant impersonation hole where any cert chaining to
    a shared CA (Diego instance-identity) authenticated as any client trusting that CA.
  • Flag: ⚪ Inert when off, but not flag-gated in code. Reachability is removed upstream: the endpoint 404s and
    tls-client-auth-ca cannot be registered. Review point: a pre-existing client that already happens to carry
    an additionalInformation key literally named tls-client-auth-ca would now be treated as mTLS-only and refused at
    /oauth/token. Nothing in UAA defined that key before this PR; check there are no foreign users. Also a
    state-transition case: clients registered with the flag on become inert (not insecure) after it is turned off — test
    E8.

UaaAuthenticationDetails

server/src/main/java/org/cloudfoundry/identity/uaa/authentication/UaaAuthenticationDetails.java

whole file new field populated from request getter
  • What changed: New @JsonIgnore private final String servletPath (decoded servlet path) alongside requestPath,
    set in all three constructors, with a getter.
  • Why: requestPath comes from getRequestURI() (raw, undecoded); the mTLS gate must see the same decoded path
    every other gate uses (/oauth/%6dtls/token bypass, security review §2).
  • Flag: ⚠️ Un-gated (M2). The class implements Serializable and declares no serialVersionUID; adding a
    field changes the implicit one. It sits in the Spring-Session-JDBC-persisted security context, so session rows
    written by the previous version fail to deserialize after upgrade. Fix: make servletPath transient or pin the
    UID to develop's value.

UaaTokenEndpoint

server/src/main/java/org/cloudfoundry/identity/uaa/oauth/token/UaaTokenEndpoint.java

whole file @RequestMapping adds /oauth/mtls/token guards: grant type, RFC 8707 resource
  • What changed: Type-level @RequestMapping now also maps /oauth/mtls/token. doDelegateGet/Post call two new
    guards that act only when the servlet path is the mTLS endpoint: rejectNonWorkloadGrantAtMtlsEndpoint (only
    client_credentials) and enforceResourceIndicator (RFC 8707 resource: ≤1 value, absolute URI without fragment,
    must be in the client's tls-client-auth-allowed-resources; else invalid_target).
  • Why: Without the grant restriction a cert-authenticated client could trade a password for a user token stamped
    with cnf, asserting two identities in one token.
  • Flag: ⚪ Inert. The mapping exists unconditionally but MtlsEndpointAvailabilityFilter 404s the path when
    the flag is off; both guards early-return on any other path. Doc nit (L1): the javadoc on
    enforceResourceIndicator still claims this is the only path into the granter so the enhancer needs no second check
    — disproved by security-review finding 1.

OauthEndpointSecurityConfiguration

server/src/main/java/org/cloudfoundry/identity/uaa/oauth/beans/OauthEndpointSecurityConfiguration.java

whole file mtlsTokenEndpointSecurity chain
  • What changed: New mtlsTokenEndpointSecurity UaaFilterChain at FilterChainOrder.OAUTH_11, matching
    /oauth/mtls/token and below: client-auth filters as in the token endpoint chain, stateless, CSRF off, anonymous
    off, /** requires full authentication.
  • Why: Gives the endpoint its own OAuth chain instead of falling through to the catch-all browser (uiSecurity) chain.
  • Flag: ✅ Gated by @Conditional(MtlsEnabledCondition.class) — bean absent when off.
    (@ConditionalOnProperty was replaced because it only accepts the literal true; see MtlsEnabledCondition.)

SpringServletXmlFiltersConfiguration

server/src/main/java/org/cloudfoundry/identity/uaa/SpringServletXmlFiltersConfiguration.java

whole file three new FilterRegistrationBeans
  • What changed: Registers MtlsEndpointAvailabilityFilter (order −290), RawPeerCertificateCaptureFilter (−300)
    and clientCertificateMapperFilter (−200, wraps the buildpack ClientCertificateMapper in MtlsPathGuardedFilter,
    reflectively because the class is package-private).
  • Why: Order matters: capture the genuine TLS-peer cert before the mapper overwrites
    jakarta.servlet.request.X509Certificate with the XFCC-derived one; availability filter must run after zone-path
    rewriting and before Spring Security (−100).
  • Flag: ✅ availability filter (flag arg) and ✅ mapper (registration setEnabled(false) and the buildpack class is
    never loaded when off). ⚪ capture filter is ungated — it only copies one attribute, and only on paths that 404 when
    off.

OauthEndpointBeanConfiguration

server/src/main/java/org/cloudfoundry/identity/uaa/oauth/beans/OauthEndpointBeanConfiguration.java

whole file clientAuthenticationProvider wiring
  • What changed: Injects the TlsClientAuthentication component into ClientDetailsAuthenticationProvider.
  • Why: Constructor change above.
  • Flag: ⚪ TlsClientAuthentication is an ungated @Component but only used after the provider has seen a
    tls-client-auth-ca client on the mTLS path.

ClientCredentialsTokenGranter

server/src/main/java/org/cloudfoundry/identity/uaa/oauth/provider/client/ClientCredentialsTokenGranter.java

whole file ALLOWED_AUTH_METHODS
  • What changed: tls_client_auth added to the allowed client-auth methods for client_credentials; new static isAllowedAuthMethod.
  • Why: Otherwise a cert-authenticated client could not obtain a client_credentials token.
  • Flag: ⚪ Only reachable by a request whose client_auth_method is tls_client_auth, which only the mTLS path produces.

FilterChainOrder

server/src/main/java/org/cloudfoundry/identity/uaa/web/FilterChainOrder.java

whole file OAUTH_11
  • What changed: Adds constant OAUTH_11 = 211.
  • Why: Order slot for the new chain (after OAUTH_10, which has a disjoint matcher).
  • Flag: 🔧 constant only.

1.2 Server — client registration and bootstrap

ClientAdminEndpointsValidator

server/src/main/java/org/cloudfoundry/identity/uaa/client/ClientAdminEndpointsValidator.java

whole file mtlsEnabled ctor param validate() hooks checkMtlsClientConfigAllowed + claim-config validation
  • What changed: Constructor takes mtlsEnabled. validate now calls checkMtlsClientConfigAllowed (rejects
    tls-client-auth-ca/-trusted-proxy-ca when off; parses PEMs; requires exactly one of the five §2.1.2 subject
    parameters) and validateTlsClientAuthClaimConfig (claim-mapping fields, reserved/dotted-reserved claim names,
    pattern only on subject_ou and with a capture group, template length ≤256 and ≥1 placeholder, placeholders must
    reference declared claims, required-claims shape, RFC 8707 allow-list shape, allow-list ⊕ aud-templates).
  • Why: Registration-time enforcement so malformed or forgeable mTLS config never reaches token issuance.
  • Flag: ✅ Gated: both calls are no-ops unless one of the tls-client-auth-* keys is present, and the CA keys
    throw when the flag is off. Existing clients without those keys are unaffected. Claim-mapping pattern complexity
    is bounded at match time in TlsClientAuthentication (see §3), not here.

ZoneEndpointsClientDetailsValidator

server/src/main/java/org/cloudfoundry/identity/uaa/zone/ZoneEndpointsClientDetailsValidator.java

whole file ctor + flag calls + secret relaxation hasNonblankTlsClientAuthCa
  • What changed: Takes mtlsEnabled; on CREATE runs the same two checks; a client with a non-blank CA string may
    omit client_secret; getAdditionalInformation() null-safe.
  • Why: Zone-admin client API is a separate registration path that must obey the same flag.
  • Flag: ✅ Gated. The secret relaxation can only trigger if checkMtlsClientConfigAllowed has already passed,
    i.e. flag on.

ClientAdminBootstrap

server/src/main/java/org/cloudfoundry/identity/uaa/client/ClientAdminBootstrap.java

whole file ctor + flag checks
  • What changed: Takes mtlsEnabled; bootstrapped (BOSH oauth.clients) clients run the same two checks.
  • Why: Bootstrap bypasses the admin API, so it needs its own enforcement.
  • Flag: ✅ Gated (same no-op-without-keys behaviour).

SpringServletXmlBeansConfiguration

server/src/main/java/org/cloudfoundry/identity/uaa/SpringServletXmlBeansConfiguration.java

whole file clientDetailsValidator bean
  • What changed: Passes ${uaa.mtls-enabled:false} into ClientAdminEndpointsValidator.
  • Why: Wiring.
  • Flag: ✅ carries the flag.

1.3 Server — discovery and external IdPs

OpenIdConnectEndpoints

server/src/main/java/org/cloudfoundry/identity/uaa/account/OpenIdConnectEndpoints.java

whole file flag + discovery
  • What changed: Takes mtlsEnabled; builds the document with the 3-arg OpenIdConfiguration; when on, sets mtls_endpoint_aliases.token_endpoint.
  • Why: RFC 8705 §5 / §3.3 metadata.
  • Flag: ✅ Gated. Off → no mtls_endpoint_aliases, no tls_client_auth in
    token_endpoint_auth_methods_supported. One additive field is always emitted:
    tls_client_certificate_bound_access_tokens (false when off — the RFC default).

ExternalOAuthAuthenticationManager

server/src/main/java/org/cloudfoundry/identity/uaa/provider/oauth/ExternalOAuthAuthenticationManager.java

whole file code exchange oauthTokenRequest
  • What changed: Both outbound token-request sites throw ProviderConfigurationException if an IdP's authMethod
    is tls_client_auth.
  • Why: UAA-as-relying-party has no client-cert plumbing; a stale/forced value must fail loudly before any request
    is sent.
  • Flag: ⚠️→✅ ungated code, behaviour-preserving: external IdP accepts exactly the methods it accepted before.
    Tests pin the rejection of tls_client_auth.

ExternalOAuthIdentityProviderConfigValidator

server/src/main/java/org/cloudfoundry/identity/uaa/provider/oauth/ExternalOAuthIdentityProviderConfigValidator.java

whole file validator
  • What changed: Uses the new isExternalOAuthMethodSupported / EXTERNAL_OAUTH_SUPPORTED_METHODS (the old
    four-method list).
  • Why: Widening UAA_SUPPORTED_METHODS must not leak tls_client_auth into relying-party config.
  • Flag: ⚠️→✅ ungated code, behaviour-preserving: external IdP accepts exactly the methods it accepted before.
    Tests pin the rejection of tls_client_auth.

OauthIDPWrapperFactoryBean

server/src/main/java/org/cloudfoundry/identity/uaa/provider/oauth/OauthIDPWrapperFactoryBean.java

whole file authMethod check
  • What changed: Same switch to isExternalOAuthMethodSupported for BOSH-supplied IdP definitions.
  • Why: Same reason.
  • Flag: ⚠️→✅ ungated code, behaviour-preserving: external IdP accepts exactly the methods it accepted before.
    Tests pin the rejection of tls_client_auth.

1.4 Model module

ClientAuthentication

model/src/main/java/org/cloudfoundry/identity/uaa/constants/ClientAuthentication.java

whole file constants + lists isValidMethod getCalculatedMethod
  • What changed: Adds TLS_CLIENT_AUTH; adds it to UAA_SUPPORTED_METHODS; new
    EXTERNAL_OAUTH_SUPPORTED_METHODS (old list) and isExternalOAuthMethodSupported; 4-arg
    isValidMethod/getCalculatedMethod with hasCaConfig (3-arg overloads delegate with false).
  • Why: Registers the method and teaches the validity matrix that tls_client_auth needs a CA and forbids a secret/key.
  • Flag: ⚠️ Un-gated constant widening. Every caller of isMethodSupported/UAA_SUPPORTED_METHODS now treats
    tls_client_auth as known. The audited external-IdP callers are split off (above); re-check for any other caller
    when reviewing.

OpenIdConfiguration

model/src/main/java/org/cloudfoundry/identity/uaa/account/OpenIdConfiguration.java

whole file tokenAMR default new fields + 2-arg ctor 3-arg ctor
  • What changed: tokenAMR default gains tls_client_auth, removed again in the constructor when mtlsEnabled
    is false
    ; new mtls_endpoint_aliases (NON_NULL) and tls_client_certificate_bound_access_tokens; new 3-arg
    constructor; the pre-existing 2-arg constructor now delegates with false.
  • Why: Discovery metadata. The 2-arg default was true earlier on the branch (fail-open); fixed so the unchanged
    public signature produces the unchanged document.
  • Flag: ✅ Gated by the constructor argument; default fails closed. Wire diff on a default deployment: one
    extra field, tls_client_certificate_bound_access_tokens:false. Note the field initializer default still contains
    tls_client_auth — only the constructors strip it; @NoArgsConstructor (Jackson) bypasses the strip.

UaaClientDetails

model/src/main/java/org/cloudfoundry/identity/uaa/client/UaaClientDetails.java

whole file field copy ctor setter / getter equals hashCode
  • What changed: New @JsonIgnore TlsClientAuthConfiguration field; setter mirrors it into additionalInformation
    (and clears all keys on null); included in the copy constructor, equals, hashCode.
  • Why: Typed access to the mTLS config.
  • Flag: ⚪ Inert — L3: nothing in production calls the setter (JDBC clients carry the config in
    additionalInformation), so the “typed field first” fast paths in MtlsClaimsEnhancer and
    UaaTokenEndpoint.allowedResourcesFor are always a miss; the field is not transient while
    TlsClientAuthConfiguration is not Serializable; and it makes equal-config clients compare unequal depending on
    how they were built.

OAuth2Exception

model/src/main/java/org/cloudfoundry/identity/uaa/oauth/common/exceptions/OAuth2Exception.java

whole file constant fromMap
  • What changed: Adds INVALID_TARGET and maps it to the new InvalidTargetException.
  • Why: RFC 8707 error code.
  • Flag: ⚪ additive constant/mapping; nothing emits it unless the mTLS path runs.

TokenConstants

model/src/main/java/org/cloudfoundry/identity/uaa/oauth/token/TokenConstants.java

whole file constant
  • What changed: CLIENT_AUTH_TLS_CLIENT_AUTH.
  • Why: Name for the client_auth_method claim value.
  • Flag: ⚪ additive constant/mapping; nothing emits it unless the mTLS path runs.

1.5 Build files

File Change Flag
libs.versions.toml adds java-buildpack-client-certificate-mapper-jakarta 2.0.1 and spring-boot-tomcat aliases 🔧 (mapper jar is on the classpath for everyone but only loaded when the flag is on; ClientCertificateMapperAutoConfigurationExclusionTest guards against it self-registering)
build.gradle.kts implementation of both (the Tomcat customizer needs spring-boot-tomcat; the filter needs the mapper) 🔧 (mapper jar is on the classpath for everyone but only loaded when the flag is on; ClientCertificateMapperAutoConfigurationExclusionTest guards against it self-registering)
build.gradle.kts testImplementation(bouncyCastlePkixFips) for cert generation in docs/MockMvc tests 🔧 (mapper jar is on the classpath for everyone but only loaded when the flag is on; ClientCertificateMapperAutoConfigurationExclusionTest guards against it self-registering)
index.html.md.erb API-docs source: the mTLS token-endpoint section (curl example, request parameters, response fields) and sub-sections on certificate-bound tokens (cnf, who enforces it), registering a client, and the endpoint's error responses 🔧 (mapper jar is on the classpath for everyone but only loaded when the flag is on; ClientCertificateMapperAutoConfigurationExclusionTest guards against it self-registering)

Section 2 — Changed existing test files (the test itself changed)

Purely additive test methods are in §4a. This section is only for tests whose existing lines or configuration
changed. Across the PR, no pre-existing @Test/@ParameterizedTest/@Nested was deleted, renamed or disabled
(checked with the grep in pr4076-backwards-compatibility-audit.md §1).

2.1 Behavioural / expectation changes — read these

Test Change Why Impact
UaaClientDetailsTest:279 hashCode()).isPositive() → .isNotZero() hashCode now mixes in the new field and may be negative; isPositive asserted something Object.hashCode never promised Weakens a smoke assertion only; equals/hashCode consistency is covered elsewhere.
OpenIdConfiguration.json:77 adds "tls_client_certificate_bound_access_tokens": false fixture is the serialized default discovery document The default discovery document grows by exactly one field (additive; RFC 8705 §3.3 treats omitted as false). tls_client_auth is not in the fixture — default stays mTLS-free.
TokenEndpointDocs:138 @TestPropertySource gains uaa.mtls-enabled=true for the whole class; autowires rawPeerCertificateCaptureFilterRegistration (:217) the new mTLS docs example cannot document a disabled endpoint Enabling the flag registers MtlsClaimsEnhancer, which flips UaaTokenServices onto its enhancer code path for all 30 examples. The class javadoc records a measurement: 29 other examples' response key-sets and field tables are identical on/off. If the enhancer ever contributes claims for non-mTLS callers this guarantee breaks → move the example to its own class.
OpenIdConnectEndpointDocs:47 ui_locales_supported line gains a trailing comma; two fieldWithPath rows added (mtls_endpoint_aliases.token_endpoint optional/STRING, tls_client_certificate_bound_access_tokens) published discovery example Class stays on the default flag, so the published example shows what most deployments return; the alias is documented as conditional. The comma change is mechanical.
OpenIdConnectEndpointsMockMvcTests:77 and …ZonePathTests:91 extra assertion getMtlsEndpointAliases()).isNull() in the existing default-discovery test earlier on the branch these classes were switched to mtls-enabled=true, which silently removed the default-deployment regression; they were reverted and strengthened Pins that a default deployment (default zone and per-zone, both addressing modes) advertises no alias. Existing assertions untouched.

2.2 Mechanical updates for changed production signatures (no assertion changed)

Test Edit Because
UaaClientAuthenticationProviderTest.java (L50, L62, L68) adds a TlsClientAuthentication mock and passes it to the new 4-arg ClientDetailsAuthenticationProvider constructor ClientDetailsAuthenticationProvider ctor
ClientAdminEndpointsTests.java (L124) new ClientAdminEndpointsValidator(..., false) validator ctor gained mtlsEnabled
ClientAdminEndpointsValidatorTests.java (L107) same one-line ctor edit (the rest of the file is additive → §4a) validator ctor
ClientAdminBootstrapMultipleSecretsTest.java (L70) trailing , false argument ClientAdminBootstrap ctor gained mtlsEnabled
ClientAdminBootstrapMultipleSecretsUpdateTests.java (L64) trailing , false argument same
ClientAdminBootstrapTests.java (L112, L135, L156, L193, L404, L493) 5 ctor call sites get false; @BeforeEach registers BouncyCastleFipsProvider (the new tests parse real PEMs) same; (rest additive → §4a)
ClientAdminBootstrapProdEncoderTest.java (L88) trailing , false argument (integration-test source set) same
ZoneEndpointsClientDetailsValidatorTests.java (L72) @InjectMocks removed in favour of explicit construction with the flag; BC FIPS provider registered ZoneEndpointsClientDetailsValidator ctor gained mtlsEnabled (Mockito can't pick the primitive)

2.3 Documentation classes: field descriptors added, no assertion changed

These are REST-docs classes (run by docsTestRestDocs, which feeds the slate API docs). Each only gains optional
field descriptors; the requests, responses and assertions are untouched, and the descriptors are optional so the
strict field checks still pass.

Class Added Why
ClientAdminEndpointDocs.java (L6, L73-96) the 14 tls-client-auth-* / tls_client_auth_* client keys, to idempotentFields (so create, update, get, list and tx all show them) client registration is where an operator meets these keys; the private_key_jwt equivalent was already documented here
IntrospectTokenEndpointDocs.java (L18, L76-77) optional cnf (RFC 8705 section 3.2) and client_auth_method response fields introspection returns the confirmation claim for a bound token; a resource server needs to know
CheckTokenEndpointDocs.java (L20, L71-72) the same two optional response fields /check_token returns them too

Source-compat note: these ctor changes remove the old public signatures (no deprecated overloads). Internal
Spring-wired classes, but anyone constructing them by hand must add the argument.


Section 3 — New production files

All new, so the diff is the whole file. Flag column as in the legend.

Module File Summary Flag
server TlsClientAuthentication.java @Component. PKIX validation of a client chain against the per-client CA (all PEMs in a bundle are anchors → CA rotation), end-entity constraints (not a CA, KeyUsage digitalSignature, EKU clientAuth/any), trusted-proxy validation for XFCC, claim-mapping extraction (CN/OU/O, escaped commas, multi-valued RDNs) with the regex work bounded — an OU over 256 characters is not matched, a match reading more than 100 000 characters is abandoned (that mapping then yields no claim), and each pattern is compiled once in a bounded cache — required-claims check, and selection of the right chain per trust model (direct vs proxy-only). Revocation is off (setRevocationEnabled(false)). ⚪
server TlsClientAuthSubjectMatcher.java RFC 8705 §2.1.2 exact binding: tls_client_auth_subject_dn (LdapName compare, order-significant), …_san_dns/uri/ip/email (typed SAN match; IP compared in binary form). Fails closed unless exactly one binding is configured. This is the headline security fix over #3972. ⚪
server MtlsClaimsEnhancer.java UaaTokenEnhancer. For tls_client_auth requests only: certificate-derived claims (dot-notation nesting, reserved names skipped), cnf.x5t#S256, sub/aud templates, and the RFC 8707 resource → aud (re-checked against the allow-list even though the endpoint already did). Fails closed — the token request fails — if cnf cannot be computed, if the client's configuration cannot be resolved, or if no certificate can be resolved, for a request that authenticated with one. Overrides getLateOverrideClaims() to return {sub, aud} — its only opt-in past UAA's defaults. L2: unguarded (UaaClientDetails) cast and a comment referring to handling that isn't there. ✅ @Conditional(MtlsEnabledCondition) (via the bean definition)
server MtlsEnabledCondition.java Spring Condition reading uaa.mtls-enabled via Environment.getProperty(.., Boolean.class, false) so 1/yes/on mean the same as for @Value boolean injection — prevents a half-enabled feature. ✅ is the gate
server MtlsEndpointAvailabilityFilter.java Order −290. Off → 404; any path below /oauth/mtls/token → 404 (closes the /oauth/mtls/token/oauth/token routing bypass); non-POST → 405 + Allow: POST. Only ever denies. ✅
server RawPeerCertificateCaptureFilter.java Order −300. Copies the genuine handshake peer cert into a private attribute before the mapper overwrites the standard one; exposes isMtlsTokenPath used across the feature (prefix semantics). ⚪ ungated, harmless
server MtlsPathGuardedFilter.java Wraps the buildpack ClientCertificateMapper so it only runs on the effective (post zone-rewrite) mTLS path. ✅ only constructed when on
server MtlsClientAuthTomcatCustomizer.java WebServerFactoryCustomizer: when on, installs BCJSSESslImplementation, sets certificateVerification=optionalNoCA and NoAcceptedIssuersTrustManager on every SSLHostConfig; idempotently registers BCFIPS + BCJSSE and fails fast if impostor/non-FIPS providers hold those names. Connector-wide, not per-endpoint — every TLS handshake now gets a CertificateRequest with no issuer list. ✅ returns before touching the connector when off
server BCJSSEUtil.java Tomcat JSSEUtil backed by BCJSSE; advertises TLS 1.3 renegotiable auth; sources implemented protocols/ciphers from BCJSSE (SunJSSE's set includes SSLv2Hello, which BCJSSE rejects). ✅ (only reachable via the customizer)
server BCJSSESSLContext.java Tomcat SSLContext adapter over javax.net.ssl.SSLContext from the BCJSSE provider. ✅
server BCJSSESslImplementation.java JSSEImplementation returning BCJSSEUtil; named via sslImplementationName so only this connector uses BCJSSE. ✅
server NoAcceptedIssuersTrustManager.java X509TrustManager with no-op checks and an empty accepted-issuers list, so Go-based clients (Gorouter) aren't told “only public CAs” and don't send an empty Certificate. ✅
model TlsClientAuthConfiguration.java Model for all tls-client-auth-* keys (CA, trusted-proxy CA, claim mappings, sub/aud templates, required claims, allowed resources, five §2.1.2 subject parameters), the reserved-claim set (isReservedClaimName also catches dotted children), isConfigured, configuredSubjectBindings. ⚪ data class
model InvalidTargetException.java RFC 8707 invalid_target, HTTP 400. ⚪

Section 4 — New tests

4a. Tests added to existing test classes

OpenIdConfigurationTests.java

Discovery model: default advertises nothing; flag-on advertises tls_client_auth; the 2-arg ctor fails closed.

Tests:

UaaClientDetailsTest.java

Copy-constructor keeps flat tls-client-auth-* info when typed config is null; JSON round trip; clearing removes all
six keys.

Tests:

ClientAuthenticationTest.java

The isValidMethod/getCalculatedMethod matrix with hasCaConfig; external-IdP list excludes tls_client_auth.

Tests:

UaaClientAuthenticationProviderTest.java

A secret presented for a tls-client-auth-ca client is refused (BadCredentialsException).

Tests:

ClientAdminBootstrapTests.java

BOSH-bootstrap path honours the flag and the §2.1.2 / claim-config rules (flag off rejects; on accepts; zero/two/blank
subject bindings rejected; SAN binding OK; ordinary client unaffected when off).

Tests:

ClientAdminEndpointsValidatorTests.java

Largest block (~560 lines): flag gating, the five-parameter §2.1.2 rule (parameterised), PEM validation, and ~30
validateTlsClientAuthClaimConfig_* cases (patterns only on subject_ou with a capture group, reserved and
dotted-reserved claims, templates, required-claims, length bounds). Several are labelled COPILOT REVIEW from the
upstream review round.

Added at:

Tests:

ZoneEndpointsClientDetailsValidatorTests.java

Zone-client API: flag gating, secretless clients allowed only with a valid non-blank CA string (JSON-map, non-string,
blank and unsupported values rejected), supplied secrets still policy-checked.

Tests:

ClientCredentialsTokenGranterTests.java

tls_client_auth is allowed for client_credentials.

Tests:

UaaTokenEndpointTests.java

Reflection test that the controller maps /oauth/mtls/token and its descendants (the premise of the routing-bypass fix).

Tests:

ExternalOAuthAuthenticationManagerTest.java

A stale tls_client_auth on an external IdP fails before any request is sent (both code-exchange and token-request paths).

Tests:

ExternalOAuthIdentityProviderConfigValidatorTest.java

Validator rejects tls_client_auth on an external IdP.

Tests:

OauthIdentityProviderDefinitionFactoryBeanTest.java

BOSH IdP factory rejects tls_client_auth.

Tests:

TokenEndpointDocs.java

REST-docs example for the mTLS client-credentials grant (generates a CA + leaf in-test).

Tests:

UaaTokenServicesTests.java

Three groups: M1 — refresh with a narrowed scope must not leak granted_scopes; the enhancer contract — an
opted-in enhancer's sub/aud win, protected claims cannot be overridden, custom claims still apply; and
coexistence (WhenMtlsClaimsEnhancerSharesTheEnhancerList) — the real MtlsClaimsEnhancer is inert for non-mTLS
requests and opts in to sub/aud only, claims from several enhancers coexist in either order, the later enhancer
wins a collision, and another enhancer is unrestricted: claims UAA does not set for the grant survive, and its
sub/aud do not displace UAA's, exactly as on develop. These two were committed red (448d872a9) and fail on the
branch without 3383975fe.

Tests:

4b. New test files

Counts are @Test/@ParameterizedTest annotations in the file. Ordered unit → Spring-context → MockMvc/integration.

File Tests What it proves
TlsClientAuthConfigurationTest.java 18 Model: reserved/dotted-reserved claim names, JSON round trips of every key, isConfigured, equality.
TlsClientAuthSubjectMatcherTest.java 17 §2.1.2 matching: DN (case/spacing-insensitive, order-sensitive, unparseable → no match), each SAN type, cross-type SAN confusion rejected, IP in binary form, exactly-one-binding rule.
TlsClientAuthenticationTest.java 45 PKIX validation (expired, intermediates, trust anchor in/out of chain, CA-as-leaf, EKU/KeyUsage), trusted-proxy checks, claim extraction incl. multi-valued RDNs, regex step budget / OU length cap / pattern cache, required-claims, chain selection for direct vs XFCC, mapper-silently-failed guard.
MtlsClaimsEnhancerTest.java 29 Claim mapping, nesting, reserved names, sub/aud templates, cnf thumbprint, only-for-tls_client_auth, fail-closed on lookup/encoding failure and on an unresolvable configuration or certificate, RFC 8707 allow-list re-check.
MtlsEnabledConditionTest.java 3 All truthy spellings match; falsy and absent don't — pins the one-meaning flag.
RawPeerCertificateCaptureFilterTest.java 2 Peer cert copied to the private attribute (or null).
RawPeerCertificateCaptureFilterRegistrationTest.java 6 Filter ordering vs the mapper; effective-path matching incl. zone-path; captured attribute survives the mapper overwriting the standard one.
ClientCertificateMapperFilterTest.java 5 Mapper registered only for the mTLS path, not constructed when off, populates the cert attribute before Security runs.
ClientCertificateMapperAutoConfigurationExclusionTest.java 2 Fails the build if the mapper jar ever ships an auto-config or ServletContainerInitializer registration.
OauthEndpointSecurityConfigurationTests.java 1 The mTLS security chain exists only when the flag is on.
ClientDetailsAuthenticationProviderTests.java 14 mTLS path detection (decoded servlet path vs raw URI, descendants), config parsing from flat additionalInformation, required-claims enforced against a client sharing the same CA, and a client whose configuration cannot be read is refused.
OpenIdConnectEndpointsTest.java 4 Discovery alias and the three advertisements move together, on and off.
MtlsClientAuthTomcatCustomizerTest.java 11 Connector settings, no-op when off, BCFIPS/BCJSSE registration idempotence and impostor/non-FIPS fail-fast.
BCJSSEUtilTest.java 1 Enabled protocols exclude SSLv2Hello, include TLS 1.3.
BCJSSESSLContextTest.java 2 TLS 1.2/1.3 from BCJSSE; clear error when provider missing.
MtlsClientAuthTomcatCustomizerIntegrationTest.java 6 Real embedded Tomcat: unauthenticated cert accepted & requested, TLS 1.2/1.3 negotiated, BCJSSE actually serving, nothing requested when off.
MtlsDisabledTokenEndpointMockMvcTests.java 8 The off-switch suite: endpoint 404 (default zone and both zone-addressing modes), registration API rejects TLS config, discovery advertises nothing, mapper filter not registered, a previously-persisted mTLS client cannot obtain a bound token (E8).
MtlsFlagConsistencyMockMvcTests.java 1 uaa.mtls-enabled=1 must not half-enable the feature.
MtlsTokenEndpointHardeningMockMvcTests.java 39 End-to-end security matrix in nested groups: credential confusion, grant types, validation failures, claim mapping/forgery, cert↔client binding (§2.1.2), cnf via introspect/check_token (JWT+opaque), CA rotation, POST-only, RFC 8707 resources, descendant-path routing bypass, discovery when on.
MtlsTokenEndpointMockMvcZonePathTests.java 7 Zone isolation in both zone-addressing modes: own-zone issuance and issuer, cross-zone clients unusable, per-zone CA for the same client id, zone-specific alias.

Generated from git diff 6cc037111..HEAD (develop merge-base); docs under docs/PR 4076 Thoughts/ are excluded from
all four sections. Companion documents: pr4076-develop-merge-evaluation.md (findings M1–M2, L1–L4),
pr4076-security-review.md, pr4076-backwards-compatibility-audit.md, pr4076-mtls-enabled-flag-audit.md.

@fhanik fhanik mentioned this pull request Sep 16, 2026
@fhanik fhanik changed the title Example fixes for 3792 Harden the RFC 8705 mTLS token endpoint - Supersedes 3792 Sep 24, 2026
@fhanik
fhanik force-pushed the review/pr3792-fix branch 2 times, most recently from a3235f7 to 6d0bf7a Compare September 24, 2026 13:48
@fhanik
fhanik marked this pull request as ready for review September 24, 2026 13:48
@strehle
strehle requested a lite review from Copilot September 24, 2026 14:06

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

Unresolved critical endpoint-path rejection and moderate dotted reserved-claim protection gaps remain.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 1 High severity · 2 Medium severity

Open (3)
What changed in this PR

Adds and hardens RFC 8705 mTLS token-endpoint support, including authentication, claim protections, endpoint gating, TLS integration, discovery metadata, documentation, and tests.

Changes:

  • Adds certificate-based client authentication, client_credentials restrictions, disabled-endpoint handling, and protected claim mapping.
  • Adds FIPS BCJSSE/Tomcat support, discovery metadata, configuration/API documentation, and extensive tests.
  • Final findings: a critical regular-endpoint rejection and moderate dotted reserved-claim validation gaps remain.
File Reviewed change
uaa/​src/​test/​java/​org/​cloudfoundry/​identity/​uaa/​scim/​endpoints/​OpenIdConnectEndpointsMockMvcZonePathTests.java Tests zone-path discovery metadata.
uaa/​src/​test/​java/​org/​cloudfoundry/​identity/​uaa/​scim/​endpoints/​OpenIdConnectEndpointsMockMvcTests.java Tests mTLS discovery metadata.
uaa/​src/​test/​java/​org/​cloudfoundry/​identity/​uaa/​scim/​endpoints/​OpenIdConnectEndpointDocs.java Documents discovery aliases.
uaa/​src/​test/​java/​org/​cloudfoundry/​identity/​uaa/​oauth/​UaaTokenServicesTests.java Tests claim protection and overrides.
uaa/​src/​test/​java/​org/​cloudfoundry/​identity/​uaa/​mock/​token/​MtlsDisabledTokenEndpointMockMvcTests.java Tests disabled mTLS behavior.
uaa/​src/​test/​java/​org/​cloudfoundry/​identity/​uaa/​login/​TokenEndpointDocs.java Documents mTLS token requests.
uaa/​slateCustomizations/​source/​index.html.md.erb Adds mTLS API documentation.
uaa/​build.gradle.kts Adds certificate test dependencies.
server/​src/​test/​java/​org/​cloudfoundry/​identity/​uaa/​web/​tomcat/​MtlsClientAuthTomcatCustomizerTest.java Tests Tomcat mTLS customization.
server/​src/​test/​java/​org/​cloudfoundry/​identity/​uaa/​web/​tomcat/​BCJSSEUtilTest.java Tests BCJSSE protocols.
server/​src/​test/​java/​org/​cloudfoundry/​identity/​uaa/​web/​tomcat/​BCJSSESSLContextTest.java Tests BCJSSE contexts.
server/​src/​test/​java/​org/​cloudfoundry/​identity/​uaa/​provider/​oauth/​OauthIdentityProviderDefinitionFactoryBeanTest.java Tests unsupported TLS authentication rejection.
server/​src/​test/​java/​org/​cloudfoundry/​identity/​uaa/​provider/​oauth/​ExternalOAuthIdentityProviderConfigValidatorTest.java Tests external OAuth validation.
server/​src/​test/​java/​org/​cloudfoundry/​identity/​uaa/​provider/​oauth/​ExternalOAuthAuthenticationManagerTest.java Tests stale TLS configuration rejection.
server/​src/​test/​java/​org/​cloudfoundry/​identity/​uaa/​oauth/​token/​UaaTokenEndpointTests.java Tests mTLS endpoint mappings.
server/​src/​test/​java/​org/​cloudfoundry/​identity/​uaa/​oauth/​tls/​RawPeerCertificateCaptureFilterTest.java Tests peer certificate capture.
server/​src/​test/​java/​org/​cloudfoundry/​identity/​uaa/​oauth/​tls/​RawPeerCertificateCaptureFilterRegistrationTest.java Tests filter registration and ordering.
server/​src/​test/​java/​org/​cloudfoundry/​identity/​uaa/​oauth/​tls/​ClientCertificateMapperFilterTest.java Tests certificate mapper behavior.
server/​src/​test/​java/​org/​cloudfoundry/​identity/​uaa/​oauth/​provider/​client/​ClientCredentialsTokenGranterTests.java Tests TLS client-credentials grants.
server/​src/​test/​java/​org/​cloudfoundry/​identity/​uaa/​oauth/​beans/​OauthEndpointSecurityConfigurationTests.java Tests conditional mTLS security wiring.
server/​src/​test/​java/​org/​cloudfoundry/​identity/​uaa/​client/​ClientAdminEndpointsTests.java Updates validator construction.
server/​src/​test/​java/​org/​cloudfoundry/​identity/​uaa/​client/​ClientAdminBootstrapTests.java Tests bootstrap mTLS validation.
server/​src/​test/​java/​org/​cloudfoundry/​identity/​uaa/​client/​ClientAdminBootstrapMultipleSecretsUpdateTests.java Updates bootstrap fixtures.
server/​src/​test/​java/​org/​cloudfoundry/​identity/​uaa/​client/​ClientAdminBootstrapMultipleSecretsTest.java Updates bootstrap fixtures.
server/​src/​test/​java/​org/​cloudfoundry/​identity/​uaa/​authentication/​UaaClientAuthenticationProviderTest.java Tests TLS client authentication rejection.
server/​src/​test/​java/​org/​cloudfoundry/​identity/​uaa/​authentication/​ClientDetailsAuthenticationProviderTests.java Tests certificate authentication and required claims.
server/​src/​test/​java/​org/​cloudfoundry/​identity/​uaa/​account/​OpenIdConnectEndpointsTest.java Tests conditional discovery metadata.
server/​src/​main/​java/​org/​cloudfoundry/​identity/​uaa/​zone/​ZoneEndpointsClientDetailsValidator.java Validates zone client mTLS configuration.
server/​src/​main/​java/​org/​cloudfoundry/​identity/​uaa/​web/​tomcat/​NoAcceptedIssuersTrustManager.java Provides TLS trust-manager handling.
server/​src/​main/​java/​org/​cloudfoundry/​identity/​uaa/​web/​tomcat/​MtlsClientAuthTomcatCustomizer.java Configures mTLS connectors.
server/​src/​main/​java/​org/​cloudfoundry/​identity/​uaa/​web/​tomcat/​BCJSSEUtil.java Integrates BCJSSE with Tomcat.
server/​src/​main/​java/​org/​cloudfoundry/​identity/​uaa/​web/​tomcat/​BCJSSESslImplementation.java Selects BCJSSE implementation.
server/​src/​main/​java/​org/​cloudfoundry/​identity/​uaa/​web/​tomcat/​BCJSSESSLContext.java Wraps BCJSSE SSL contexts.
server/​src/​main/​java/​org/​cloudfoundry/​identity/​uaa/​web/​FilterChainOrder.java Adds mTLS filter ordering.
server/​src/​main/​java/​org/​cloudfoundry/​identity/​uaa/​SpringServletXmlFiltersConfiguration.java Registers mTLS filters.
server/​src/​main/​java/​org/​cloudfoundry/​identity/​uaa/​SpringServletXmlBeansConfiguration.java Wires mTLS configuration.
server/​src/​main/​java/​org/​cloudfoundry/​identity/​uaa/​provider/​oauth/​OauthIDPWrapperFactoryBean.java Excludes TLS authentication from external IdPs.
server/​src/​main/​java/​org/​cloudfoundry/​identity/​uaa/​provider/​oauth/​ExternalOAuthIdentityProviderConfigValidator.java Validates external OAuth methods.
server/​src/​main/​java/​org/​cloudfoundry/​identity/​uaa/​provider/​oauth/​ExternalOAuthAuthenticationManager.java Rejects unsupported TLS authentication.
server/​src/​main/​java/​org/​cloudfoundry/​identity/​uaa/​oauth/​UaaTokenServices.java Protects UAA-owned JWT claims.
server/​src/​main/​java/​org/​cloudfoundry/​identity/​uaa/​oauth/​token/​UaaTokenEndpoint.java Restricts mTLS grants.
server/​src/​main/​java/​org/​cloudfoundry/​identity/​uaa/​oauth/​tls/​RawPeerCertificateCaptureFilter.java Captures genuine peer certificates.
server/​src/​main/​java/​org/​cloudfoundry/​identity/​uaa/​oauth/​tls/​MtlsPathGuardedFilter.java Scopes certificate mapping to mTLS paths.
server/​src/​main/​java/​org/​cloudfoundry/​identity/​uaa/​oauth/​tls/​MtlsEndpointAvailabilityFilter.java Hides the disabled mTLS endpoint.
server/​src/​main/​java/​org/​cloudfoundry/​identity/​uaa/​oauth/​tls/​MtlsClaimsEnhancer.java Adds certificate-derived claims; dotted reserved parents remain unresolved.
server/​src/​main/​java/​org/​cloudfoundry/​identity/​uaa/​oauth/​provider/​client/​ClientCredentialsTokenGranter.java Allows TLS client authentication.
server/​src/​main/​java/​org/​cloudfoundry/​identity/​uaa/​oauth/​beans/​OauthEndpointSecurityConfiguration.java Adds the mTLS security chain.
server/​src/​main/​java/​org/​cloudfoundry/​identity/​uaa/​oauth/​beans/​OauthEndpointBeanConfiguration.java Wires TLS authentication.
server/​src/​main/​java/​org/​cloudfoundry/​identity/​uaa/​client/​ClientAdminBootstrap.java Validates bootstrapped mTLS clients.
server/​src/​main/​java/​org/​cloudfoundry/​identity/​uaa/​authentication/​ClientDetailsAuthenticationProvider.java Enforces mTLS authentication; regular /oauth/token handling has a critical rejection issue.
server/​src/​main/​java/​org/​cloudfoundry/​identity/​uaa/​account/​OpenIdConnectEndpoints.java Advertises mTLS discovery metadata.
server/​src/​integrationTest/​java/​org/​cloudfoundry/​identity/​uaa/​client/​ClientAdminBootstrapProdEncoderTest.java Updates bootstrap integration coverage.
server/​build.gradle.kts Adds Tomcat and certificate-mapper dependencies.
model/​src/​test/​resources/​org/​cloudfoundry/​identity/​uaa/​account/​OpenIdConfiguration.json Updates discovery fixture.
model/​src/​test/​java/​org/​cloudfoundry/​identity/​uaa/​constants/​ClientAuthenticationTest.java Tests TLS authentication constants.
model/​src/​test/​java/​org/​cloudfoundry/​identity/​uaa/​client/​UaaClientDetailsTest.java Tests TLS configuration persistence.
model/​src/​test/​java/​org/​cloudfoundry/​identity/​uaa/​client/​TlsClientAuthConfigurationTest.java Tests TLS configuration serialization.
model/​src/​test/​java/​org/​cloudfoundry/​identity/​uaa/​account/​OpenIdConfigurationTests.java Tests conditional discovery fields.
model/​src/​main/​java/​org/​cloudfoundry/​identity/​uaa/​oauth/​token/​TokenConstants.java Adds the TLS authentication constant.
model/​src/​main/​java/​org/​cloudfoundry/​identity/​uaa/​constants/​ClientAuthentication.java Adds TLS authentication support.
model/​src/​main/​java/​org/​cloudfoundry/​identity/​uaa/​client/​UaaClientDetails.java Stores TLS client configuration.
model/​src/​main/​java/​org/​cloudfoundry/​identity/​uaa/​client/​TlsClientAuthConfiguration.java Defines TLS configuration and protected claims.
model/​src/​main/​java/​org/​cloudfoundry/​identity/​uaa/​account/​OpenIdConfiguration.java Adds mTLS discovery fields.
gradle/​libs.versions.toml Adds dependency aliases and versions.
docs/​UAA-Configuration-Reference.md Documents the mTLS feature flag.
docs/​UAA-Client-Authentication.md Documents RFC 8705 client authentication.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🔵 Needs a closer look

Unresolved moderate issues affect reserved-claim protection, client equality/hash semantics, and malformed configuration handling.

Review effort: Balanced
Findings: 1 High severity · 2 Medium severity

Open (3)

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

The mapper dependency can auto-register an earlier unguarded filter that defeats genuine peer-certificate capture, and configuration validation/documentation also have unresolved gaps.

Get a fresh assessment by requesting another Copilot review.

Review effort: Balanced
Findings: 1 High severity · 1 Medium severity · 1 Low severity

Open (3)
Resolved since last review (3)

Comment thread server/build.gradle.kts
Comment thread docs/UAA-Client-Authentication.md Outdated
@strehle
strehle requested a balanced review from Copilot September 24, 2026 18:55

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

fhanik added a commit to fhanik/uaa that referenced this pull request Sep 25, 2026
…, docs

Three findings from the latest Copilot review of PR cloudfoundry#4076
(review 5306465064), verified against the code before implementing:

1. HIGH - "Prevent competing certificate mapper from overwriting the
   peer certificate" (server/build.gradle.kts:44). Verified by
   decompiling the java-buildpack-client-certificate-mapper-jakarta
   jar: it ships a @ConditionalOnCloudPlatform(CLOUD_FOUNDRY)
   @configuration class, ClientCertificateMapperAutoConfiguration, that
   registers its own ClientCertificateMapper filter at
   Ordered.HIGHEST_PRECEDENCE -- ahead of RawPeerCertificateCaptureFilter
   (-300). If ever activated, it would overwrite the servlet
   X509Certificate attribute with XFCC data before the genuine TLS peer
   certificate is captured, defeating direct-connection mTLS
   authentication. The jar carries no spring.factories /
   AutoConfiguration.imports entry naming it, so nothing triggers this
   today, but a routine dependency bump could add one with no code-level
   signal. UaaBootApplication now excludes it by name (excludeName,
   since the jar is implementation-scoped in the server module and not
   on uaa's compile classpath) as permanent, version-independent
   insurance.

2. MEDIUM - "Reject unsupported subject patterns and captureless OU
   patterns" (ClientAdminEndpointsValidator.java:533). Verified against
   TlsClientAuthentication.extractClaimMappingValues: pattern is applied
   only when field == "subject_ou" (the subject_cn/subject_o switch arms
   ignore mapping.getPattern() entirely), and matchFirstOu only returns
   a value via m.group(1), guarded by m.groupCount() >= 1. The validator
   accepted both shapes as syntactically valid, so a client config could
   be registered that would never produce the claim it declares.
   validateTlsClientAuthClaimConfig now rejects a pattern on
   subject_cn/subject_o and a subject_ou pattern with no capturing
   group.

3. LOW - "Document template length, placeholder, and mapping
   constraints" (docs/UAA-Client-Authentication.md:180). The
   tls-client-auth-claim-mappings / -sub-template / -aud-templates rows
   didn't mention the validator's enforced constraints (256-character
   cap, at least one {claim} placeholder, placeholder must name a
   declared claim, plus the pattern constraints from #2). Documented.

Two existing tests (validateTlsClientAuthClaimConfig_acceptsValid
NativeClaimMappings / _acceptsValidJsonStringClaimMappings) used a
pattern on subject_cn -- exactly the now-rejected shape -- and were
updated to use subject_ou instead, which still exercises the pattern
path meaningfully.

Full ./gradlew clean test passes.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
fhanik added a commit to fhanik/uaa that referenced this pull request Sep 25, 2026
…, docs

Three findings from the latest Copilot review of PR cloudfoundry#4076
(review 5306465064), verified against the code before implementing:

1. HIGH - "Prevent competing certificate mapper from overwriting the
   peer certificate" (server/build.gradle.kts:44). Verified by
   decompiling the java-buildpack-client-certificate-mapper-jakarta
   jar: it ships a @ConditionalOnCloudPlatform(CLOUD_FOUNDRY)
   @configuration class, ClientCertificateMapperAutoConfiguration, that
   registers its own ClientCertificateMapper filter at
   Ordered.HIGHEST_PRECEDENCE -- ahead of RawPeerCertificateCaptureFilter
   (-300). If ever activated, it would overwrite the servlet
   X509Certificate attribute with XFCC data before the genuine TLS peer
   certificate is captured, defeating direct-connection mTLS
   authentication. The jar carries no spring.factories /
   AutoConfiguration.imports entry naming it, so nothing triggers this
   today, but a routine dependency bump could add one with no code-level
   signal. UaaBootApplication now excludes it by name (excludeName,
   since the jar is implementation-scoped in the server module and not
   on uaa's compile classpath) as permanent, version-independent
   insurance.

2. MEDIUM - "Reject unsupported subject patterns and captureless OU
   patterns" (ClientAdminEndpointsValidator.java:533). Verified against
   TlsClientAuthentication.extractClaimMappingValues: pattern is applied
   only when field == "subject_ou" (the subject_cn/subject_o switch arms
   ignore mapping.getPattern() entirely), and matchFirstOu only returns
   a value via m.group(1), guarded by m.groupCount() >= 1. The validator
   accepted both shapes as syntactically valid, so a client config could
   be registered that would never produce the claim it declares.
   validateTlsClientAuthClaimConfig now rejects a pattern on
   subject_cn/subject_o and a subject_ou pattern with no capturing
   group.

3. LOW - "Document template length, placeholder, and mapping
   constraints" (docs/UAA-Client-Authentication.md:180). The
   tls-client-auth-claim-mappings / -sub-template / -aud-templates rows
   didn't mention the validator's enforced constraints (256-character
   cap, at least one {claim} placeholder, placeholder must name a
   declared claim, plus the pattern constraints from #2). Documented.

Two existing tests (validateTlsClientAuthClaimConfig_acceptsValid
NativeClaimMappings / _acceptsValidJsonStringClaimMappings) used a
pattern on subject_cn -- exactly the now-rejected shape -- and were
updated to use subject_ou instead, which still exercises the pattern
path meaningfully.
@fhanik
fhanik force-pushed the review/pr3792-fix branch 2 times, most recently from f52ba97 to dfc6578 Compare September 25, 2026 16:31
fhanik added a commit to fhanik/uaa that referenced this pull request Sep 29, 2026
…other machine

Claude Code sessions are local JSONL transcripts keyed to the absolute project
path, so this session cannot be resumed on a different computer. This document
is the substitute: enough state, topology and hard-won environment detail to
pick the work up cold.

Records, verified against the repo rather than recalled:

- branch/PR state, the remotes to re-add (including rkoster's fork, since
  refs/pull/3972/head on the cloudfoundry remote is stale and will not update),
  and a commit-by-commit map of the branch
- the load-bearing fact that cloudfoundry#4076 implements RFC 8705 section 2.1.2 subject
  binding and cloudfoundry#3972 does not, so on that branch any certificate chaining to the
  configured CA authenticates as the client
- that the two branches have diverged (his 14 follow-ups start exactly where
  our squash ends) and cannot simply be merged
- verification commands and the current green state, including that the two
  remaining integrationTest failures are both SCIM rate-limiter artifacts
- environment traps that cost real time: piped gradle output hiding exit codes,
  ChromeDriver/Chrome major-version coupling and the unmanaged Homebrew binary,
  gradle daemons pinning a stale PATH, the Jackson 3 TypeReference package, and
  a local process that rewrites commits and strips trailers
- zone-aware MockMvc conventions, especially why servletPath may only be
  pre-set when no /z/{subdomain} prefix is present
- open items here and across all three approaches, plus a document index

Also corrects a stale assumption while writing it: origin/review/pr3968 is
twelve commits ahead of what was local, and includes two red/green pairs adding
JWT-SVID audit events and default-zone-only signing. Those close two findings
that pr3968-security-review.md still lists as unaddressed, so the handoff says
to take the remote and treat the old local branch as superseded.
fhanik added a commit to fhanik/uaa that referenced this pull request Sep 29, 2026
Removes the JWT-SVID/SPIFFE material, which belongs to a separate branch that
will not be picked up on the new machine and was only noise for continuing
PR cloudfoundry#4076:

- drops the section on that branch's divergence from origin entirely
- section 10 becomes a two-way comparison of this branch against the Service
  Accounts RFC, which stays because that design depends on
  tls_client_auth_san_dns and therefore consumes this branch rather than
  competing with it
- the document index lists only the mTLS-relevant analysis docs
- the "verify, do not assert" agreement now cites mTLS examples: the seven
  stale FINDING labels, the dotted-claim protection we already had, and the CA
  rotation gap we actually did not
- the @ConditionalOnProperty note now describes how the mTLS suites switch the
  feature on, rather than pointing at the other branch's fixtures
fhanik added a commit to fhanik/uaa that referenced this pull request Sep 30, 2026
…other machine

Claude Code sessions are local JSONL transcripts keyed to the absolute project
path, so this session cannot be resumed on a different computer. This document
is the substitute: enough state, topology and hard-won environment detail to
pick the work up cold.

Records, verified against the repo rather than recalled:

- branch/PR state, the remotes to re-add (including rkoster's fork, since
  refs/pull/3972/head on the cloudfoundry remote is stale and will not update),
  and a commit-by-commit map of the branch
- the load-bearing fact that cloudfoundry#4076 implements RFC 8705 section 2.1.2 subject
  binding and cloudfoundry#3972 does not, so on that branch any certificate chaining to the
  configured CA authenticates as the client
- that the two branches have diverged (his 14 follow-ups start exactly where
  our squash ends) and cannot simply be merged
- verification commands and the current green state, including that the two
  remaining integrationTest failures are both SCIM rate-limiter artifacts
- environment traps that cost real time: piped gradle output hiding exit codes,
  ChromeDriver/Chrome major-version coupling and the unmanaged Homebrew binary,
  gradle daemons pinning a stale PATH, the Jackson 3 TypeReference package, and
  a local process that rewrites commits and strips trailers
- zone-aware MockMvc conventions, especially why servletPath may only be
  pre-set when no /z/{subdomain} prefix is present
- open items here and across all three approaches, plus a document index

Also corrects a stale assumption while writing it: origin/review/pr3968 is
twelve commits ahead of what was local, and includes two red/green pairs adding
JWT-SVID audit events and default-zone-only signing. Those close two findings
that pr3968-security-review.md still lists as unaddressed, so the handoff says
to take the remote and treat the old local branch as superseded.
fhanik added a commit to fhanik/uaa that referenced this pull request Sep 30, 2026
Removes the JWT-SVID/SPIFFE material, which belongs to a separate branch that
will not be picked up on the new machine and was only noise for continuing
PR cloudfoundry#4076:

- drops the section on that branch's divergence from origin entirely
- section 10 becomes a two-way comparison of this branch against the Service
  Accounts RFC, which stays because that design depends on
  tls_client_auth_san_dns and therefore consumes this branch rather than
  competing with it
- the document index lists only the mTLS-relevant analysis docs
- the "verify, do not assert" agreement now cites mTLS examples: the seven
  stale FINDING labels, the dotted-claim protection we already had, and the CA
  rotation gap we actually did not
- the @ConditionalOnProperty note now describes how the mTLS suites switch the
  feature on, rather than pointing at the other branch's fixtures
fhanik added 8 commits October 2, 2026 10:05
… pattern once (green)

A claim mapping's pattern could hang token issuance: Java's regex engine has no
timeout and cannot be interrupted, and the registration validator accepts
nested-quantifier patterns that are exponential on a short input. Whoever
registers a client with its own tls-client-auth-ca also chooses the certificates
it is matched against.

- The OU is wrapped in a CharSequence that allows 100 000 charAt calls. The
  engine reads its input only through charAt, so this bounds the work whatever
  the pattern is; linear patterns use a few hundred. Exceeding it abandons that
  mapping, which then yields no claim, and logs a warning. A claim that
  tls-client-auth-required-claims depends on is therefore absent and
  authentication is refused. Other mappings are unaffected.
- An OU longer than 256 characters is not matched against a pattern. Real
  instance identities are under 60. A mapping with no pattern is untouched.
- Each distinct pattern is compiled once, in a bounded (256) access-ordered
  cache, instead of on every token request. A pattern that does not compile still
  throws and is not cached.

The step budget is the real fix; the length cap alone would not be (28 characters
is already enough to hang on ((a+)+)+b) and the cache is not a security measure.

Verified: the red commit's timeout and length-cap tests now pass, plus three
cache tests; 180 tests across the tls, validator and mTLS MockMvc suites pass.
L4 is marked fixed. The finding is also corrected in two ways that matter:

- the example was wrong: (a+)+b is optimised by current JDKs and is not
  dangerous; the shapes that are, measured on JDK 25, are ((a+)+)+b and
  (.*a){12}b;
- it was under-rated: whoever can register a client with tls-client-auth-ca also
  chooses the certificates the pattern is matched against, so in a multi-zone
  deployment a zone admin could hang token issuance for every zone. Re-rated
  medium for multi-zone deployments, low otherwise.

The client-authentication reference now states the limits (256-character OU cap,
100 000-character match budget, what happens to the claim and to
required-claims when a match is abandoned) in place of "avoid catastrophic
backtracking". The review guide and session handoff are updated to match.
…T docs

The generated API docs said nothing about how to register a tls_client_auth client
or what a resource server should look for in a bound token, although the
private_key_jwt equivalent was already documented in the same classes.

- ClientAdminEndpointDocs: the 14 tls-client-auth-* / tls_client_auth_* keys are
  added to the shared client field list, so create, update, get, list and the
  transaction example all show them. They are optional descriptors with no change
  to the requests or assertions, so the strict field checks still pass; the
  descriptions state the rules (exactly one subject parameter, mutually exclusive
  pairs, the regex limits, that the keys need uaa.mtls-enabled).
- IntrospectTokenEndpointDocs and CheckTokenEndpointDocs: optional cnf (RFC 8705
  section 3.2) and client_auth_method response fields, saying that the resource
  server, not UAA, has to compare the thumbprint with the certificate it sees.

Run with docsTestRestDocs; the generated snippets contain the new fields.
…rade notes

UAA-Client-Authentication.md gains four sections:

- registering a tls_client_auth client, by path (client admin API, zone client
  API where a secretless client is allowed, oauth.clients bootstrap) and what
  happens with the flag off;
- certificate-bound access tokens: the claims, how a resource server verifies
  cnf.x5t#S256 (with a command to compute it), that UAA stamps the binding but
  does not enforce it, and that /introspect and /check_token return it;
- the error responses at /oauth/mtls/token, by status and cause;
- compatibility and upgrade notes: what the flag-off deployment sees, the
  granted_scopes change for deployments with their own token enhancers, the
  constructor changes, and how MtlsClaimsEnhancer coexists with other enhancers
  including the new getLateOverrideClaims() opt-in.

The slate source gets the same three user-facing topics. generateDocs could not
be run here (the gem install step fails in this environment), so the slate
rendering is unverified; the additions are plain markdown and add no render()
calls.

Also a paste-ready PR description that lists the changes reaching deployments
without the flag, the review guide's counts are now computed rather than
written, it gains a section for the documentation classes, and the handoff index
lists the evaluation, guide and description.
…unbound token (red)

Three ways a tls_client_auth client could end up with a token that is not
certificate-bound, found while checking the review comments for "malformed
configuration handling":

- ClientDetailsAuthenticationProvider: if a client with tls-client-auth-ca has
  configuration that cannot be read (claim-mappings that are not JSON, say, in a
  row written before validation existed or straight to the database),
  getTlsClientAuthConfiguration swallowed the error and returned null, so the
  client was treated as an ordinary client and authenticated with its
  client_secret at /oauth/token -- exactly what an mTLS-only registration forbids.

- MtlsClaimsEnhancer: it re-reads additionalInformation itself, and parses keys
  (tls-client-auth-allowed-resources) that the authentication step does not.
  When that failed it returned an empty map: a token with no cnf for a request
  that authenticated with a certificate.

- MtlsClaimsEnhancer: the same empty map when no certificate could be resolved
  for a client that had just authenticated with one.

All three tests fail on the branch as it stands. The validators reject such
configuration at registration, so this is reachable only for stored data that
bypassed them; the point is that the failure mode should be a refusal, not a
silent downgrade.
…nd token (green)

- ClientDetailsAuthenticationProvider: a client that sets tls-client-auth-ca but
  whose configuration cannot be read is refused (BadCredentialsException) instead
  of being treated as an ordinary client that may use its client_secret.
- MtlsClaimsEnhancer: for a request that authenticated as tls_client_auth, a
  configuration or certificate that cannot be resolved now fails the token request
  (IllegalStateException, naming the client) instead of returning an empty map and
  issuing a token without cnf. Same fail-closed choice as the existing cnf
  computation.

The registration validators already reject configuration that cannot be read, so
this only affects stored data that bypassed them; the change makes the failure
mode a refusal rather than a silent downgrade.

Checked: the red commit's three tests pass; 805 tests across the server
oauth/authentication/client suites and the uaa mTLS MockMvc and token-services
suites pass.
Every Copilot finding on the PR (six inline comments across four reviews) is
checked against the code as it now stands, with where it is handled and the test
that pins it: two were fixed (dotted reserved claims in the validator and the
enhancer), two fixed earlier (pattern validation, template documentation), one
is not reachable and guarded by a canary test (the mapper jar's auto-configuration
-- verified against the jar itself), and one is a design decision (a client with
tls-client-auth-ca is exclusive to the mTLS endpoint). Suggested replies are
included; none has been posted.

Following the review summary's "malformed configuration handling" turned up three
fail-open paths, fixed in the previous two commits; recorded as L6 in the
evaluation. The guide and handoff index are updated.
@github-actions

github-actions Bot commented Oct 2, 2026

Copy link
Copy Markdown

This PR exceeds the recommended size of 1000 lines. Please make sure you are NOT addressing multiple issues with one PR. Note this PR might be rejected due to its size.

@rkoster rkoster left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we honor the RFC 8705 client metadata tls_client_certificate_bound_access_tokens independently of tls_client_auth authentication?

At the current PR head (3faed8183), MtlsClaimsEnhancer.enhance() unconditionally adds cnf.x5t#S256. The metadata name appears in discovery, but I could not find client-policy handling for it in token issuance. We also observed cnf in live tokens from this revision in the service-account lab.

RFC 8705 §1 explicitly separates mutual-TLS client authentication from certificate-bound access tokens. §3.4 defines the client metadata tls_client_certificate_bound_access_tokens and says its default is false. Authenticating with a certificate therefore does not itself require issuing a certificate-bound token.

The service-account draft, §§9–9.1 needs certificate authentication at issuance followed by ordinary bearer JWT use at CAPI and external consumers. Its managed-client registration explicitly specifies:

"tls_client_certificate_bound_access_tokens": false

Suggested behavior/coverage:

  • false (or absent, per the RFC default): keep certificate possession, chain validation, and exact registered subject/SAN matching, but omit cnf from the resulting JWT.
  • true: retain the current certificate-thumbprint cnf behavior.
  • Validate the setting as a boolean and verify it survives client create/read/update and persisted-client loading; applications must not be able to override the policy with token-request parameters or claim templates.
  • Add token-endpoint tests for both policies, including invalid-certificate and wrong-SAN rejection when bearer issuance is selected.

Our CAPI prototype also needs to include the explicit false field; that is a consumer-side follow-up. Simply adding it to registration cannot fix the unconditional enhancer behavior here. This request is about separating the two RFC profiles, not removing mTLS authentication or weakening its validation.

…lient_certificate_bound_access_tokens (red)

RFC 8705 section 1 separates mutual-TLS client authentication from certificate-bound
access tokens, and section 3.4 defaults the client metadata to false. A client that
authenticates with a certificate but does not ask for bound tokens must get a bearer JWT
without cnf, while subject binding and chain validation are unchanged.
…lient_certificate_bound_access_tokens (green)

MtlsClaimsEnhancer stamps cnf.x5t#S256 only for a client registered with
tls_client_certificate_bound_access_tokens=true (RFC 8705 section 3.4; absent means false).
Certificate possession, chain validation and exact subject binding are unchanged. The
setting is validated as a boolean at registration and read from the stored client only,
so neither a token request parameter nor a claim template can change it.

Existing tests that assert cnf now register their client with the setting enabled.
…ral-only SAN IP binding, shellcheck (green)

- UaaClientDetails.tlsClientAuthConfiguration is a derived view of additionalInformation and is
  transient like it, so a populated client can be serialized.
- A default-constructed OpenIdConfiguration no longer advertises tls_client_auth; it is added only
  when mtlsEnabled is explicitly true.
- tls_client_auth_san_ip must be an IPv4/IPv6 literal at registration, and the matcher parses
  literals only, so authentication never depends on DNS.
- boot-with-tls.sh quotes its expansions (SC2086).
@fhanik

fhanik commented Oct 12, 2026

Copy link
Copy Markdown
Contributor Author

@rkoster I have implemented the opt-in 5f60130

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment on lines 608 to 609
for (String excludedClaim : getExcludedClaims()) {
claims.remove(excludedClaim);
Comment on lines +329 to +338
putOrRemove(TlsClientAuthConfiguration.TLS_CLIENT_AUTH_REQUIRED_CLAIMS,
tlsClientAuthConfiguration.getRequiredClaims());
} else {
this.additionalInformation.remove(TlsClientAuthConfiguration.TLS_CLIENT_AUTH_CA);
this.additionalInformation.remove(TlsClientAuthConfiguration.TLS_CLIENT_AUTH_CLAIM_MAPPINGS);
this.additionalInformation.remove(TlsClientAuthConfiguration.TLS_CLIENT_AUTH_SUB_TEMPLATE);
this.additionalInformation.remove(TlsClientAuthConfiguration.TLS_CLIENT_AUTH_AUD_TEMPLATES);
this.additionalInformation.remove(TlsClientAuthConfiguration.TLS_CLIENT_AUTH_TRUSTED_PROXY_CA);
this.additionalInformation.remove(TlsClientAuthConfiguration.TLS_CLIENT_AUTH_REQUIRED_CLAIMS);
}
Comment on lines +307 to +311
#### Certificate-bound tokens

A token issued here is bound to the client certificate ([RFC 8705 section 3](https://www.rfc-editor.org/rfc/rfc8705#section-3)).
It carries `cnf` with `x5t#S256`, the base64url (no padding) SHA-256 thumbprint of the certificate's DER encoding, and
`client_auth_method: tls_client_auth`. Only `client_credentials` is issued and no refresh token is returned.
…_tokens

Invalid stored value fails closed, direct parser tests, YAML bootstrap persistence and
rejection, and a create/read/update round trip through the client admin API.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

Connector-wide TLS changes need human security and rollout review, and client-configuration defects remain unresolved.

5 open findings
1 resolved since last review
Previously missed (3)

In code that hasn't changed since last review

Medium severity Persist all TLS client authentication settings

model/​src/​main/​java/​org/​cloudfoundry/​identity/​uaa/​client/​UaaClientDetails.java:319

tlsClientAuthConfiguration is ignored by JSON and is transient, so the flat additionalInformation map must hold every setting needed after reload. This setter copies only six settings and omits all five RFC 8705 subject bindings and tls-client-auth-allowed-resources; the typed model also cannot express the bound-access-token setting. A client configured through this public setter can therefore lose its registered identity or resource policy on persistence, and may fail authentication after reload. Mirror and clear every supported key, add the missing typed setting, and cover a complete round trip.

Medium severity Base equality on persisted TLS configuration

model/​src/​main/​java/​org/​cloudfoundry/​identity/​uaa/​client/​UaaClientDetails.java:394

tlsClientAuthConfiguration is transient and is not reconstructed from additionalInformation when a client is loaded. Two clients with identical persisted settings therefore compare unequal if one was built with the typed setter and the other was loaded from JDBC or JSON; their hash codes also differ. This makes equality-based cache or change-detection behavior depend on construction path. Compare only the canonical persisted settings in both equals and hashCode.

Low severity Clarify certificate-bound access token requirements

uaa/​slateCustomizations/​source/​index.html.md.erb:311

This section says every token issued at /oauth/mtls/token carries cnf, but MtlsClaimsEnhancer adds it only when the client registers tls_client_certificate_bound_access_tokens: true; the default is a bearer token. As written, resource-server operators may assume binding that is not present. State the registration requirement and distinguish certificate authentication from certificate-bound access tokens.

🧠 Review effort: Lite

Comment on lines +304 to +305
Object rawConfig = info.get(TlsClientAuthConfiguration.TLS_CLIENT_AUTH_CA);
if (rawConfig instanceof String pem) {

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

Development

Successfully merging this pull request may close these issues.

3 participants