Repository navigation
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Comment on lines
+49
to
+59
| SecurityFilterChain chain = http | ||
| .securityMatcher("/jwt-svid/**") | ||
| .authorizeHttpRequests(auth -> { | ||
| auth.requestMatchers("/**").hasAuthority("uaa.resource"); | ||
| auth.anyRequest().denyAll(); | ||
| }) | ||
| .authenticationManager(clientAuthenticationManager) | ||
| .sessionManagement(session -> session.sessionCreationPolicy(SessionCreationPolicy.STATELESS)) | ||
| .addFilterAt(clientAuthenticationFilter.getFilter(), BasicAuthenticationFilter.class) | ||
| .anonymous(AnonymousConfigurer::disable) | ||
| .csrf(CsrfConfigurer::disable) |
Adds POST /jwt-svid/sign so UAA can act as a SPIFFE identity server for
Cloud Foundry workloads via offline OIDC federation.
Squashed from 9 commits:
- feat(spiffe): add SpiffeProperties config binding
- feat(spiffe): add SpiffeId formatter and CfInstanceIdentity record
- feat(spiffe): parse org/space/app GUIDs from instance cert OUs
- feat(spiffe): verify instance cert against configured CA and validity
- feat(spiffe): verify proof-of-possession signature and freshness
- feat(spiffe): sign JWT-SVID with UAA active key and OIDC issuer
- feat(spiffe): add JWT-SVID signing endpoint and DTOs
- feat(spiffe): wire SPIFFE properties, CA bean, and security chain
- fix(spiffe): validate jwt-svid request inputs and lock fail-closed paths
The final commit's message, retained in full:
Reject process_type not matching [A-Za-z0-9_-]{1,63} and audience that is
blank, over 512 chars, or contains control characters (400). process_type
is concatenated into the SPIFFE ID path and every proof-of-possession
message field must be newline-free, so this closes SPIFFE-ID injection and
PoP message ambiguity. Adds characterization tests pinning existing
fail-closed behaviour: malformed PEM, future-dated and malformed-base64
PoP, and a not-yet-valid instance cert.
…d/sign
The feature shipped with unit tests per collaborator but no test that drives
the endpoint through the real filter chain, so nothing covered authentication,
authorization, HTTP status mapping, or the actual signed claim set.
Adds 23 end-to-end tests over the real security chain with real BCFIPS
certificates and real proof-of-possession signatures, grouped as:
A: issuance, and the exact claim set that goes out (sub/aud/exp/iat/jti/cf)
B: endpoint auth -- unauthenticated 401, missing uaa.resource 403, bad secret
C: certificate verification -- untrusted CA, expired, not-yet-valid, bad PEM
D: proof-of-possession -- wrong key, stale timestamp, audience swap,
process_type swap, malformed base64
E: request validation -- process_type path separator, audience blank,
over-long, and control characters
F: a JWT-SVID must not double as a UAA access token
Because every SPIFFE bean is gated behind @ConditionalOnProperty, the CA is
generated at runtime and injected via an ApplicationContextInitializer, which
also gives these tests their own context instead of mutating the shared one.
Two tests are RED and stay red until the following commit. They assert correct
behaviour and currently document live defects, both confirmed by running them:
E5 the configured CA's own self-signed certificate passes chain verification
and is refused only later, incidentally, for missing CF OUs (400 not 401).
The trust anchor is usable as a workload identity.
E6 org/space/app OUs are concatenated into the SPIFFE ID without validation,
so a certificate bearing OU=app:a/process/admin yields
spiffe://td/cf/org/o/space/s/app/a/process/admin/process/web -- returns
200. process_type was hardened against exactly this, but the other three
path segments were not.
Group F documents a mitigating factor worth keeping: a JWT-SVID is signed with
the same key, same iss and same typ/kid header as a real access token, and only
the absence of client_id/cid/scope stops UAA's own introspection accepting one.
…ce gaps
Turns the two red tests from the previous commit green and hardens four
adjacent issues found reviewing the same code paths.
SPIFFE-ID injection via certificate OUs (was: HTTP 200)
process_type was validated against [A-Za-z0-9_-]{1,63} precisely because it
is concatenated into the SPIFFE ID path, but org/space/app -- which land in
the same path, and in the same newline-delimited proof-of-possession message
-- were interpolated unchecked straight from certificate OU attributes. A
cert bearing OU=app:a/process/admin produced
spiffe://td/cf/org/o/space/s/app/a/process/admin/process/web, so a relying
party authorizing on a path prefix could be given a different workload's
identity. SpiffeId now enforces the spec's own rule on every segment --
"Individual path segments MUST contain only letters, numbers, dots, dashes,
and underscores" -- rejects '.' and '..', and caps the ID at the specified
2048 bytes. Segment errors do not echo the offending certificate value.
The trust anchor was usable as a workload identity (was: HTTP 400)
A self-signed certificate verifies against its own public key, so checking
only "signed by the CA key" accepted the configured CA certificate as if it
were a leaf. It was refused solely because it happens to lack CF OUs. The CA
certificate is public configuration. InstanceIdentityVerifier now rejects
self-issued and CA-flagged certificates outright, and also checks the CA's
own validity dates -- an expired trust anchor must stop anchoring trust.
Application-wide @ControllerAdvice
JwtSvidController.ExceptionHandling was unscoped, and @ControllerAdvice is
meta-annotated @component while the nested class carries no
@ConditionalOnProperty of its own. Under
@componentscan("org.cloudfoundry.identity.uaa") it therefore registered for
every controller in UAA even with SPIFFE switched off, mapping any
IllegalArgumentException anywhere in the application to a 400 carrying the
raw exception message. Now scoped with assignableTypes, matching the only
other advice in the codebase, HttpMethodNotSupportedAdvice.
Ambiguous duplicate OUs
Two OUs sharing a prefix silently resolved to whichever the encoder placed
last, making identity depend on RDN ordering. Now refused.
Unset trust domain
Only instance-identity-ca gates the feature on, so trust_domain could be
omitted and every workload on the foundation would be issued an identity
under spiffe://null/, surfacing only at a relying party. Now a boot failure.
Also corrects the IllegalStateException text, which named a property
spelling (instance_identity_ca) that is valid in uaa.yml but is not the
canonical form, and logs a loud warning when pop-enabled is false, since
that setting lets any uaa.resource client impersonate any workload.
Verified: 23 end-to-end MockMvc tests and 80 server unit tests pass.
The feature shipped with no operator- or consumer-facing documentation. API reference Adds JwtSvidEndpointDocs, generating Slate snippets for POST /jwt-svid/sign from a real request, and registers them in slateCustomizations' index under a new "JWT-SVID" section -- snippets are only rendered if explicitly included there, so generating them alone would have produced nothing. Documents every request and response field, the required Authorization header, and the 400 / 401 / 403 cases. Like the MockMvc suite, it supplies a runtime-generated CA via an ApplicationContextInitializer, since the beans are conditional. Feature guide (docs/UAA-SPIFFE-JWT-SVID.md) Explains what the feature is for -- turning the Diego instance certificate CF already issues into a credential that AWS/GCP/Azure workload identity federation can consume -- the SPIFFE concepts behind it, the SPIFFE ID format and its per-segment character rules, and the full request flow. The section worth reading is "the two identities involved": the caller (the SPIFFE Agent, authenticated by client credentials, needing uaa.resource) is deliberately distinct from the subject (the workload, attested by the certificate plus proof of possession). The agent's credentials authorize using the endpoint, never minting a particular identity, which is what makes one shared agent client safe. Also records the operational caveats found during review rather than leaving them implicit: bearer-token lifetime versus SPIRE's 5-minute default, replay within the proof-of-possession freshness window, the breadth of uaa.resource, absence of revocation, default-bucket-only rate limiting, and that SPIFFE config is global while the issued iss is per-zone. Explains why an instance certificate is not a secret and therefore why pop_enabled must stay on. Configuration reference Documents all five uaa.spiffe.* properties in both halves of the reference (quick table and detailed descriptions) plus the ToC, noting that instance_identity_ca is the feature gate and that only a directly-issued leaf is supported. Verified: generateDocs succeeds and the rendered HTML contains the section, its nav entry, and the field tables, with no unrendered ERB.
Review comment cloudfoundry#13 on PR cloudfoundry#3968: the endpoint mints a credential with no audit record, so there is no trail of which agent obtained which workload's identity, when. Introduces the JwtSvidIssuedEvent audit event (AuditEventType code 43) and wires an ApplicationEventPublisher into JwtSvidController, but the controller does not yet publish the event -- the new AuditEvents test is intentionally red until the next commit.
Wires the ApplicationEventPublisher added in the previous commit into JwtSvidController.sign(): every successful issuance now publishes a JwtSvidIssuedEvent, giving UAA's audit framework a record of which agent client obtained which SPIFFE ID, for which audience, and when. Closes review comment cloudfoundry#13 on PR cloudfoundry#3968. Also injects IdentityZoneManager to source the event's zone id, since the static IdentityZoneHolder is deprecated.
…ning Review comment cloudfoundry#14 on PR cloudfoundry#3968: uaa.spiffe.* configuration is global, but the endpoint is reachable from any identity zone via subdomain routing, and JwtSvidSigner takes the issuer from IdentityZoneHolder, so the same trust domain gets served under differing issuers per zone. Adds JwtSvidController.ZoneNotSupportedException (404, so the endpoint's existence isn't revealed outside the default zone) plus a unit test and an end-to-end MockMvc test asserting a request from a non-default zone is refused. Both are red: the controller does not yet perform the check, and the end-to-end test proves that today a request from another zone is actually signed and returned (200).
Wires the ZoneNotSupportedException added in the previous commit into JwtSvidController.sign(): a request that arrives via a non-default zone's subdomain is now refused with 404 before any other processing. Closes review comment cloudfoundry#14 on PR cloudfoundry#3968 -- uaa.spiffe.* stays a global, default-zone-only feature rather than being silently reachable, and re-issuable under a different issuer, from every zone.
fhanik
force-pushed
the
review/pr3968
branch
from
September 28, 2026 19:41
33ff44d to
6105a83
Compare
Contributor
Author
|
Closing, as we will not do spiffe for now. Also note that I accidentally changed the PR description with another PR's description. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
PR #4076 — code-review guide
Pull request: #4076 ·
base:
cloudfoundry/develop(diff taken from merge-base6cc037111, the tip of develop that thebranch forked from) · head:
review/pr3792-fixat1b5b51b68+ ·overall: 94 files, +13 935 / −53, of which docs are excluded below.
How to read this
#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.
1b5b51b68). Line anchors move if the branch is rebased.uaa.mtls-enabled, defaultfalse):Summary of the⚠️ items (details in §1): M1
granted_scopesdropped from refresh-issued access tokens(UaaTokenServices); M2
UaaAuthenticationDetailsgains a field and has noserialVersionUID(UaaAuthenticationDetails); widenedUAA_SUPPORTED_METHODS(ClientAuthentication) — contained by an external-IdP split;tls-client-auth-cais now an interpretedadditionalInformationkey(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.javaGRANTED_SCOPESadded toNON_ADDITIONAL_ROOT_CLAIMS. (2) IncreateJWTToken,enhancer-supplied claims are now applied before UAA's own defaults and skip every protected claim name, then
sub/audalone are re-applied at the end.refreshTokenClaims.remove(GRANTED_SCOPES)ran after the copy loop and never worked. (2)MtlsClaimsEnhanceroutput is certificate-derived and client-configurable, so it must not be able to overwrite
scope,client_id,iss,authorities… ; onlysub/audare meant to be overridable.NON_ADDITIONAL_ROOT_CLAIMSis also read by the pre-existinggetAdditionalRootClaims, which everygrant_type=refresh_tokencall goes through → refreshed access tokens stopcarrying
granted_scopeson every deployment. Correct behaviour, but a signed-token change on the busiest non-mTLSpath. (2) ✅ Gated: the loop only sees entries when
uaaTokenEnhancersis non-empty, i.e. whenMtlsClaimsEnhanceris a bean (flag on).
ClientDetailsAuthenticationProvider
server/src/main/java/org/cloudfoundry/identity/uaa/authentication/ClientDetailsAuthenticationProvider.javaTlsClientAuthenticationcollaborator (constructor signature change). A client whoseadditionalInformationhastls-client-auth-cais exclusively mTLS: it must authenticate at/oauth/mtls/tokenwith no credentials, must register exactly one RFC 8705 §2.1.2 subject value, and is refused at
/oauth/token.Conversely
/oauth/mtls/tokenrefuses any client without that config (so it is not an alias of/oauth/token).Validation = PKIX chain →
TlsClientAuthSubjectMatcher→ required-claims.InvalidClientDetailsExceptionisconverted to
BadCredentialsException(previously a 500 on the Basic-auth path).isTlsClientAuthPathanswers trueif either the decoded servlet path or the raw request path resolves to the endpoint (fail closed; handles
/oauth/%6dtls/token).a shared CA (Diego instance-identity) authenticated as any client trusting that CA.
tls-client-auth-cacannot be registered. Review point: a pre-existing client that already happens to carryan
additionalInformationkey literally namedtls-client-auth-cawould 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 astate-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@JsonIgnore private final String servletPath(decoded servlet path) alongsiderequestPath,set in all three constructors, with a getter.
requestPathcomes fromgetRequestURI()(raw, undecoded); the mTLS gate must see the same decoded pathevery other gate uses (
/oauth/%6dtls/tokenbypass, security review §2).implements Serializableand declares noserialVersionUID; adding afield 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
servletPathtransientor pin theUID to develop's value.
UaaTokenEndpoint
server/src/main/java/org/cloudfoundry/identity/uaa/oauth/token/UaaTokenEndpoint.java@RequestMappingnow also maps/oauth/mtls/token.doDelegateGet/Postcall two newguards that act only when the servlet path is the mTLS endpoint:
rejectNonWorkloadGrantAtMtlsEndpoint(onlyclient_credentials) andenforceResourceIndicator(RFC 8707resource: ≤1 value, absolute URI without fragment,must be in the client's
tls-client-auth-allowed-resources; elseinvalid_target).with
cnf, asserting two identities in one token.MtlsEndpointAvailabilityFilter404s the path whenthe flag is off; both guards early-return on any other path. Doc nit (L1): the javadoc on
enforceResourceIndicatorstill 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.javamtlsTokenEndpointSecurityUaaFilterChainatFilterChainOrder.OAUTH_11, matching/oauth/mtls/tokenand below: client-auth filters as in the token endpoint chain, stateless, CSRF off, anonymousoff,
/**requires full authentication.uiSecurity) chain.@Conditional(MtlsEnabledCondition.class)— bean absent when off.(
@ConditionalOnPropertywas replaced because it only accepts the literaltrue; seeMtlsEnabledCondition.)SpringServletXmlFiltersConfiguration
server/src/main/java/org/cloudfoundry/identity/uaa/SpringServletXmlFiltersConfiguration.javaMtlsEndpointAvailabilityFilter(order −290),RawPeerCertificateCaptureFilter(−300)and
clientCertificateMapperFilter(−200, wraps the buildpackClientCertificateMapperinMtlsPathGuardedFilter,reflectively because the class is package-private).
jakarta.servlet.request.X509Certificatewith the XFCC-derived one; availability filter must run after zone-pathrewriting and before Spring Security (−100).
setEnabled(false)and the buildpack class isnever 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.javaTlsClientAuthenticationcomponent intoClientDetailsAuthenticationProvider.TlsClientAuthenticationis an ungated@Componentbut only used after the provider has seen atls-client-auth-caclient on the mTLS path.ClientCredentialsTokenGranter
server/src/main/java/org/cloudfoundry/identity/uaa/oauth/provider/client/ClientCredentialsTokenGranter.javatls_client_authadded to the allowed client-auth methods forclient_credentials; new staticisAllowedAuthMethod.client_auth_methodistls_client_auth, which only the mTLS path produces.FilterChainOrder
server/src/main/java/org/cloudfoundry/identity/uaa/web/FilterChainOrder.javaOAUTH_11 = 211.OAUTH_10, which has a disjoint matcher).1.2 Server — client registration and bootstrap
ClientAdminEndpointsValidator
server/src/main/java/org/cloudfoundry/identity/uaa/client/ClientAdminEndpointsValidator.javamtlsEnabled.validatenow callscheckMtlsClientConfigAllowed(rejectstls-client-auth-ca/-trusted-proxy-cawhen off; parses PEMs; requires exactly one of the five §2.1.2 subjectparameters) and
validateTlsClientAuthClaimConfig(claim-mapping fields, reserved/dotted-reserved claim names,patternonly onsubject_ouand with a capture group, template length ≤256 and ≥1 placeholder, placeholders mustreference declared claims, required-claims shape, RFC 8707 allow-list shape, allow-list ⊕ aud-templates).
tls-client-auth-*keys is present, and the CA keysthrow when the flag is off. Existing clients without those keys are unaffected. L4: claim-mapping
patternhasno complexity bound, unlike templates.
ZoneEndpointsClientDetailsValidator
server/src/main/java/org/cloudfoundry/identity/uaa/zone/ZoneEndpointsClientDetailsValidator.javamtlsEnabled; on CREATE runs the same two checks; a client with a non-blank CA string mayomit
client_secret;getAdditionalInformation()null-safe.checkMtlsClientConfigAllowedhas already passed,i.e. flag on.
ClientAdminBootstrap
server/src/main/java/org/cloudfoundry/identity/uaa/client/ClientAdminBootstrap.javamtlsEnabled; bootstrapped (BOSHoauth.clients) clients run the same two checks.SpringServletXmlBeansConfiguration
server/src/main/java/org/cloudfoundry/identity/uaa/SpringServletXmlBeansConfiguration.java${uaa.mtls-enabled:false}intoClientAdminEndpointsValidator.1.3 Server — discovery and external IdPs
OpenIdConnectEndpoints
server/src/main/java/org/cloudfoundry/identity/uaa/account/OpenIdConnectEndpoints.javamtlsEnabled; builds the document with the 3-argOpenIdConfiguration; when on, setsmtls_endpoint_aliases.token_endpoint.mtls_endpoint_aliases, notls_client_authintoken_endpoint_auth_methods_supported. One additive field is always emitted:tls_client_certificate_bound_access_tokens(falsewhen off — the RFC default).ExternalOAuthAuthenticationManager
server/src/main/java/org/cloudfoundry/identity/uaa/provider/oauth/ExternalOAuthAuthenticationManager.javaProviderConfigurationExceptionif an IdP'sauthMethodis
tls_client_auth.is sent.
Tests pin the rejection of
tls_client_auth.ExternalOAuthIdentityProviderConfigValidator
server/src/main/java/org/cloudfoundry/identity/uaa/provider/oauth/ExternalOAuthIdentityProviderConfigValidator.javaisExternalOAuthMethodSupported/EXTERNAL_OAUTH_SUPPORTED_METHODS(the oldfour-method list).
UAA_SUPPORTED_METHODSmust not leaktls_client_authinto relying-party config.Tests pin the rejection of
tls_client_auth.OauthIDPWrapperFactoryBean
server/src/main/java/org/cloudfoundry/identity/uaa/provider/oauth/OauthIDPWrapperFactoryBean.javaisExternalOAuthMethodSupportedfor BOSH-supplied IdP definitions.Tests pin the rejection of
tls_client_auth.1.4 Model module
ClientAuthentication
model/src/main/java/org/cloudfoundry/identity/uaa/constants/ClientAuthentication.javaTLS_CLIENT_AUTH; adds it toUAA_SUPPORTED_METHODS; newEXTERNAL_OAUTH_SUPPORTED_METHODS(old list) andisExternalOAuthMethodSupported; 4-argisValidMethod/getCalculatedMethodwithhasCaConfig(3-arg overloads delegate withfalse).tls_client_authneeds a CA and forbids a secret/key.isMethodSupported/UAA_SUPPORTED_METHODSnow treatstls_client_authas known. The audited external-IdP callers are split off (above); re-check for any other callerwhen reviewing.
OpenIdConfiguration
model/src/main/java/org/cloudfoundry/identity/uaa/account/OpenIdConfiguration.javatokenAMRdefault gainstls_client_auth, removed again in the constructor whenmtlsEnabledis false; new
mtls_endpoint_aliases(NON_NULL) andtls_client_certificate_bound_access_tokens; new 3-argconstructor; the pre-existing 2-arg constructor now delegates with
false.trueearlier on the branch (fail-open); fixed so the unchangedpublic signature produces the unchanged document.
extra field,
tls_client_certificate_bound_access_tokens:false. Note the field initializer default still containstls_client_auth— only the constructors strip it;@NoArgsConstructor(Jackson) bypasses the strip.UaaClientDetails
model/src/main/java/org/cloudfoundry/identity/uaa/client/UaaClientDetails.java@JsonIgnore TlsClientAuthConfigurationfield; setter mirrors it intoadditionalInformation(and clears all keys on
null); included in the copy constructor,equals,hashCode.additionalInformation), so the “typed field first” fast paths inMtlsClaimsEnhancerandUaaTokenEndpoint.allowedResourcesForare always a miss; the field is nottransientwhileTlsClientAuthConfigurationis notSerializable; and it makes equal-config clients compare unequal depending onhow they were built.
OAuth2Exception
model/src/main/java/org/cloudfoundry/identity/uaa/oauth/common/exceptions/OAuth2Exception.javaINVALID_TARGETand maps it to the newInvalidTargetException.TokenConstants
model/src/main/java/org/cloudfoundry/identity/uaa/oauth/token/TokenConstants.javaCLIENT_AUTH_TLS_CLIENT_AUTH.client_auth_methodclaim value.1.5 Build files
java-buildpack-client-certificate-mapper-jakarta2.0.1 andspring-boot-tomcataliasesClientCertificateMapperAutoConfigurationExclusionTestguards against it self-registering)implementationof both (the Tomcat customizer needsspring-boot-tomcat; the filter needs the mapper)ClientCertificateMapperAutoConfigurationExclusionTestguards against it self-registering)testImplementation(bouncyCastlePkixFips)for cert generation in docs/MockMvc testsClientCertificateMapperAutoConfigurationExclusionTestguards against it self-registering)mtls_endpoint_aliases/tls_client_certificate_bound_access_tokensrows and the mTLS token-endpoint sectionClientCertificateMapperAutoConfigurationExclusionTestguards 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/@Nestedwas deleted, renamed or disabled(checked with the grep in
pr4076-backwards-compatibility-audit.md§1).2.1 Behavioural / expectation changes — read these
hashCode()).isPositive()→.isNotZero()hashCodenow mixes in the new field and may be negative;isPositiveasserted somethingObject.hashCodenever promised"tls_client_certificate_bound_access_tokens": falsefalse).tls_client_authis not in the fixture — default stays mTLS-free.@TestPropertySourcegainsuaa.mtls-enabled=truefor the whole class; autowiresrawPeerCertificateCaptureFilterRegistration(:217)MtlsClaimsEnhancer, which flipsUaaTokenServicesonto 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.ui_locales_supportedline gains a trailing comma; twofieldWithPathrows added (mtls_endpoint_aliases.token_endpointoptional/STRING,tls_client_certificate_bound_access_tokens)getMtlsEndpointAliases()).isNull()in the existing default-discovery testmtls-enabled=true, which silently removed the default-deployment regression; they were reverted and strengthened2.2 Mechanical updates for changed production signatures (no assertion changed)
TlsClientAuthenticationmock and passes it to the new 4-argClientDetailsAuthenticationProviderconstructorClientDetailsAuthenticationProviderctornew ClientAdminEndpointsValidator(..., false)mtlsEnabled, falseargumentClientAdminBootstrapctor gainedmtlsEnabled, falseargumentfalse;@BeforeEachregistersBouncyCastleFipsProvider(the new tests parse real PEMs), falseargument (integration-test source set)@InjectMocksremoved in favour of explicit construction with the flag; BC FIPS provider registeredZoneEndpointsClientDetailsValidatorctor gainedmtlsEnabled(Mockito can't pick the primitive)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.
@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), required-claims check, and selection of the right chain per trust model (direct vs proxy-only). Revocation is off (setRevocationEnabled(false)).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.UaaTokenEnhancer. Fortls_client_authrequests only: certificate-derived claims (dot-notation nesting, reserved names skipped),cnf.x5t#S256,sub/audtemplates, and the RFC 8707resource→aud(re-checked against the allow-list even though the endpoint already did). Fails closed ifcnfcannot be computed. L2: unguarded(UaaClientDetails)cast and a comment referring to handling that isn't there.@Conditional(MtlsEnabledCondition)(via the bean definition)Conditionreadinguaa.mtls-enabledviaEnvironment.getProperty(.., Boolean.class, false)so1/yes/onmean the same as for@Value booleaninjection — prevents a half-enabled feature./oauth/mtls/token→ 404 (closes the/oauth/mtls/token/oauth/tokenrouting bypass); non-POST → 405 +Allow: POST. Only ever denies.isMtlsTokenPathused across the feature (prefix semantics).ClientCertificateMapperso it only runs on the effective (post zone-rewrite) mTLS path.WebServerFactoryCustomizer: when on, installsBCJSSESslImplementation, setscertificateVerification=optionalNoCAandNoAcceptedIssuersTrustManageron everySSLHostConfig; 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.JSSEUtilbacked by BCJSSE; advertises TLS 1.3 renegotiable auth; sources implemented protocols/ciphers from BCJSSE (SunJSSE's set includesSSLv2Hello, which BCJSSE rejects).SSLContextadapter overjavax.net.ssl.SSLContextfrom the BCJSSE provider.JSSEImplementationreturningBCJSSEUtil; named viasslImplementationNameso only this connector uses BCJSSE.X509TrustManagerwith 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.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 (isReservedClaimNamealso catches dotted children),isConfigured,configuredSubjectBindings.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:
mtlsEndpointAliasesIsNullByDefaultmtlsEndpointAliasesCanBeSettheConstructorWithoutAnMtlsFlagAdvertisesNoMtlsSupporttlsClientAuthIsExcludedWhenMtlsDisabledtlsClientAuthIsIncludedWhenMtlsEnabledUaaClientDetailsTest.java
Copy-constructor keeps flat
tls-client-auth-*info when typed config is null; JSON round trip; clearing removes allsix keys.
Tests:
copiesFlatTlsClientAuthAdditionalInformationWhenTypedConfigurationIsNulltlsClientAuthConfigRoundTripsViaJsonsetTlsClientAuthConfiguration_whenCleared_removesAllPersistedSettingsClientAuthenticationTest.java
The
isValidMethod/getCalculatedMethodmatrix withhasCaConfig; external-IdP list excludestls_client_auth.Tests:
externalOAuthMethodsSupportStandardMethodsButNotTlsClientAuthtlsClientAuthIsARecognisedMethodtlsClientAuthDoesNotRequireASecrettlsClientAuthIsCalculatedWhenHasCaConfigtlsClientAuthIsValidMethodWhenHasCaConfigtlsClientAuthIsInvalidWithoutCaConfigtlsClientAuthIsInvalidWhenHasSecrettlsClientAuthIsInvalidWhenHasKeyConfigtlsClientAuthIsValidMethodWhenHasCaConfigAndMethodUnsettlsClientAuthIsInvalidWhenMethodUnsetAndHasSecretAndCaConfigtlsClientAuthIsInvalidWhenMethodUnsetAndHasKeyConfigAndCaConfigUaaClientAuthenticationProviderTest.java
A secret presented for a
tls-client-auth-caclient is refused (BadCredentialsException).Tests:
provider_rejectsClientSecretForClientConfiguredForTlsClientAuthClientAdminBootstrapTests.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:
mtlsClientConfigRejectedWhenMtlsDisabledmtlsClientTrustedProxyConfigRejectedWhenMtlsDisabledmtlsClientConfigAllowedWhenMtlsEnablednestedTlsClientAuthConfigurationIsRejectedDuringBootstrapinvalidTlsClientAuthClaimConfigIsRejectedDuringBootstrapmtlsClientWithNoSubjectBindingIsRejectedDuringBootstrapmtlsClientWithTwoSubjectBindingsIsRejectedDuringBootstrapmtlsClientWithBlankSubjectBindingIsRejectedDuringBootstrapmtlsClientWithASanBindingBootstrapsSuccessfullymtlsClientWithRequiredClaimsButNoSubjectBindingIsRejectedDuringBootstrapordinaryClientBootstrapsSuccessfullyWhenMtlsDisabledClientAdminEndpointsValidatorTests.java
Largest block (~560 lines): flag gating, the five-parameter §2.1.2 rule (parameterised), PEM validation, and ~30
validateTlsClientAuthClaimConfig_*cases (patterns only onsubject_ouwith a capture group, reserved anddotted-reserved claims, templates, required-claims, length bounds). Several are labelled
COPILOT REVIEWfrom theupstream review round.
Added at:
Tests:
rejectsTlsClientAuthCaWhenMtlsDisabledrejectsTlsClientAuthTrustedProxyCaWhenMtlsDisabledallowsTlsClientAuthCaWithExactlyOneSubjectBindingacceptsEachOfTheFiveSubjectParametersrejectsTlsClientAuthCaWithNoSubjectBindingrejectsTlsClientAuthCaWithMoreThanOneSubjectBindingrejectsBlankSubjectBindingValuerequiredClaimsAloneDoesNotSatisfySubjectBindingallowsRequiredClaimsAlongsideASubjectBindingrejectsNestedTlsClientAuthConfigurationWhenMtlsEnabledrejectsBlankTlsClientAuthTrustedProxyCaWhenMtlsEnabledrejectsMalformedTlsClientAuthTrustedProxyCaWhenMtlsEnabledrejectsMalformedTlsClientAuthCaWhenMtlsEnabledallowsClientWithoutMtlsFieldsWhenMtlsDisabledvalidateTlsClientAuthClaimConfig_rejectsReservedClaimvalidateTlsClientAuthClaimConfig_rejectsDottedClaimWhoseParentIsReservedvalidateTlsClientAuthClaimConfig_rejectsPatternOnSubjectCnvalidateTlsClientAuthClaimConfig_rejectsCaptureGrouplessOuPatternvalidateTlsClientAuthClaimConfig_rejectsSubTemplateExceedingMaxLengthvalidateTlsClientAuthClaimConfig_rejectsPlaceholderlessSubTemplateAtMaxLengthQuicklyvalidateTlsClientAuthClaimConfig_acceptsFullyValidConfigZoneEndpointsClientDetailsValidatorTests.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:
rejectsSecretlessClientCredentialsClientWhenAdditionalInformationIsNullrejectsTlsClientAuthCaWhenMtlsDisabledallowsTlsClientAuthCaWhenMtlsEnabledallowsSecretlessClientCredentialsClientWhenTlsClientAuthCaConfiguredrejectsSecretlessClientCredentialsClientWhenTlsClientAuthCaIsJsonMaprejectsSecretlessClientCredentialsClientWhenTlsClientAuthCaMapHasUnsupportedCaValuerejectsSecretlessClientCredentialsClientWhenTlsClientAuthCaMapIsMalformedrejectsSecretlessClientCredentialsClientWhenTlsClientAuthCaIsBlankrejectsSecretlessClientCredentialsClientWhenTlsClientAuthCaIsNotAStringstillValidatesSuppliedSecretWhenTlsClientAuthCaConfiguredallowsClientWithoutMtlsFieldsWhenMtlsDisabledClientCredentialsTokenGranterTests.java
tls_client_authis allowed for client_credentials.Tests:
tlsClientAuthIsAllowedForClientCredentialsUaaTokenEndpointTests.java
Reflection test that the controller maps
/oauth/mtls/tokenand its descendants (the premise of the routing-bypass fix).Tests:
mapsMtlsTokenEndpointAndItsDescendantsExternalOAuthAuthenticationManagerTest.java
A stale
tls_client_authon an external IdP fails before any request is sent (both code-exchange and token-request paths).Tests:
oauthTokenRequestRejectsStaleTlsClientAuthMethodBeforeSendingRequestauthorizationCodeExchangeRejectsStaleTlsClientAuthMethodBeforeSendingRequestExternalOAuthIdentityProviderConfigValidatorTest.java
Validator rejects
tls_client_authon an external IdP.Tests:
configWithTlsClientAuthMethod_ThrowsExceptionOauthIdentityProviderDefinitionFactoryBeanTest.java
BOSH IdP factory rejects
tls_client_auth.Tests:
authMethodSetToTlsClientAuthIsRejectedTokenEndpointDocs.java
REST-docs example for the mTLS client-credentials grant (generates a CA + leaf in-test).
Tests:
getTokenUsingClientCredentialGrantWithTlsClientAuthUaaTokenServicesTests.java
Two groups: M1 — refresh with a narrowed scope must not leak
granted_scopes(this is the test that proves theun-gated change); and the enhancer contract —
sub/audoverride, protected claims cannot be overridden, customclaims still apply.
Tests:
refreshWithNarrowedScopeMustNotLeakGrantedScopesIntoTheAccessTokenenhancerSubAndAudClaimsWinOverUaaDefaultsenhancerCannotOverrideProtectedClaimsenhancerCanStillAddCustomClaims4b. New test files
Counts are
@Test/@ParameterizedTestannotations in the file. Ordered unit → Spring-context → MockMvc/integration.isConfigured, equality.cnfthumbprint, only-for-tls_client_auth, fail-closed on lookup/encoding failure, RFC 8707 allow-list re-check.ServletContainerInitializerregistration.additionalInformation, required-claims enforced against a client sharing the same CA.SSLv2Hello, include TLS 1.3.uaa.mtls-enabled=1must not half-enable the feature.cnfvia introspect/check_token (JWT+opaque), CA rotation, POST-only, RFC 8707 resources, descendant-path routing bypass, discovery when on.Generated from
git diff 6cc037111..HEAD(develop merge-base); docs underdocs/PR 4076 Thoughts/are excluded fromall 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.