fix: Sanitize x-client_cert header for IAS proof-token validation (4.0.8)#1996
Open
NiklasHerrmann21 wants to merge 3 commits into
Open
fix: Sanitize x-client_cert header for IAS proof-token validation (4.0.8)#1996NiklasHerrmann21 wants to merge 3 commits into
NiklasHerrmann21 wants to merge 3 commits into
Conversation
…0.8) SapIdJwtSignatureValidator was passing the raw PEM (including -----BEGIN CERTIFICATE-----/-----END CERTIFICATE----- delimiters and CR/LF line breaks) as the x-client_cert header value. Since 4.0.0 the token-client HTTP transport uses Java 11's HttpClient, which enforces RFC 7230 and rejects any header value containing CR/LF with IllegalArgumentException. Every authenticated request behind Istio with credential-type: X509_GENERATED failed proof-token validation as a result. Strip PEM delimiters and all whitespace before placing the value on the wire via a new X509Certificate#getPEMHeaderValue() accessor; the IAS JWKS endpoint accepts bare base64-encoded DER. The existing getPEM() contract is unchanged. Also add a defensive CR/LF check in JavaHttpClientAdapter that fails fast with a message naming the offending header key (not the value, which may be sensitive), turning any future regression from a misleading "invalid header value: <PEM>" into an actionable transport-layer error. Bump project version 4.0.7 -> 4.0.8 across all modules, READMEs and samples. Move the dependency updates and junit-bom import fix from the 4.1.0 CHANGELOG section into 4.0.8.
kuntzed
previously approved these changes
Jul 2, 2026
Extend the header sanitization to handle a PEM containing multiple concatenated CERTIFICATE blocks: keep only the first (leaf) block before stripping delimiters and whitespace. This is consistent with getSubjectDN() and getThumbprint(), which already operate on the first certificate (X509Parser.parseCertificate uses generateCertificate in singular form and silently ignores trailing blocks). Without this, a Multi-PEM header value would be flattened into a concatenated base64 blob that no downstream PEM parser could split back into individual certificates — strictly worse than the pre-4.0 behavior, where PEM delimiters at least preserved the structure. Rename getPEMHeaderValue() to getLeafCertificateAsHeaderValue() to reflect the extended behavior. Update the call site in SapIdJwtSignatureValidator and add a Multi-PEM test in X509CertificateTest.
kuntzed
reviewed
Jul 2, 2026
| } | ||
| } | ||
| return pem | ||
| .replace("-----BEGIN CERTIFICATE-----", "") |
Contributor
There was a problem hiding this comment.
wir haben dafür konstanten, siehe X509Parser.BEGIN_CERTIFICATE und ich finde es unnötig, die erst mit rauszuschneiden, nur um sie dann zu löschen.
…ssor Address review feedback: reuse the existing PEM delimiter constants from X509Parser instead of hardcoded string literals, and slice between them in a single substring() instead of first including the markers and then stripping them again.
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
2 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.
Summary
Fixes a regression introduced in 4.0.0 that breaks IAS proof-token validation in Kyma/Istio environments with
credential-type: X509_GENERATED.SapIdJwtSignatureValidatorwas setting thex-client_certrequest header directly fromX509Certificate#getPEM(). When the certificate originates from Istio'sx-forwarded-client-cert(XFCC) header, the PEM includes-----BEGIN CERTIFICATE-----/-----END CERTIFICATE-----delimiters and CR/LF line breaks.HttpClient(JavaHttpClientAdapter), which enforces RFC 7230 and rejects any header value containing CR/LF withIllegalArgumentException(surfaced as"Token signature can not be validated because: invalid header value: <PEM>").Fix
X509Certificate#getPEMHeaderValue()accessor that returns bare base64-encoded DER (PEM delimiters and all whitespace stripped). The IAS JWKS endpoint accepts this form.getPEM()'s existing contract is unchanged so other callers aren't affected.SapIdJwtSignatureValidatornow usesgetPEMHeaderValue()when populating thex-client_certheader.JavaHttpClientAdapter: any header value containing CR or LF fails fast with a clear message naming the offending header key (not the value, which may be sensitive), so future regressions surface as an actionable transport-layer error instead of a misleading"invalid header value: <full PEM dumped>".Release
Prepares 4.0.8:
junit-bomimport fix that were previously staged for 4.1.0 are now part of 4.0.8.Test plan
mvn -pl java-security,token-client test— 327 tests, 0 failures, 0 errors, 2 skipped (pre-existing), BUILD SUCCESSX509CertificateTest: new tests for multi-line PEM and XFCC-wrapped URL-encoded PEM inputsSapIdJwtSignatureValidatorTest: new test feeding a multi-line PEM cert through the proof-token path, asserting the capturedx-client_certheader contains no-----BEGIN, whitespace,\n, or\rJavaHttpClientAdapterTest: new file with tests for\nand\rrejection with clear error messagecredential-type: X509_GENERATED