Repository navigation
KNOX-3516: Log and describe JWT rejections caused by an untrusted issuer - #1457
Merged
Merged
Conversation
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
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.
KNOX-3516 - Log and describe JWT rejections caused by an untrusted issuer
What changes were proposed in this pull request?
When a JWT's
issis neither injwt.expected.issuernor a runtime-registered trusted issuer,AbstractJWTFilter.validateToken(...)rejected the request with a 401 and logged nothing. It also passed anullerror message, so the client got a bare 401 on the plain path and{"error":"invalid_request"}with noerror_descriptionon the token-exchange path. The failure could not be seen on either side.This PR:
Rejected token {0} ({1}) with unexpected issuer {2}: .... It uses the redacted token display text and token id, the same as the other rejection paths. The display text is always present, which matters because foreign tokens (K8s service-account tokens, external OIDC) usually have noknox.id, so the token id isnull.Token issuer '<iss>' is not trusted. On token exchange it is returned aserror_description; on the plain path it is passed tosendError(401, ...).TokenUtils.getIssuerDisplayText(String). The issuer is read before the signature is verified, so the caller controls it. Control characters are replaced to prevent log forging, and the length is capped at 256 characters to limit log flooding.SSOCookieFederationFilter.isInvalidSsoCookie()decides by the end of the error string (" is disabled"/" exceeded idle timeout"). If the issuer ended the message, a craftedisscould trigger cookie removal, session invalidation and global logout.How was this patch tested?
New and updated unit tests;
mvn verifypasses ongateway-spiandgateway-provider-security-jwt, including checkstyle.UnexpectedIssuerRejectionTest(new):knox.id) from an untrusted issuer produces the log record with the expected arguments and the expected 401 message;issis neutralized in both the log and the client message;" is disabled"/" exceeded idle timeout"cannot produce an error string with that ending.TokenUtilsTest:getIssuerDisplayTextwith a normal issuer, null, C0/C1 control characters, truncation, and a value exactly at the length limit.JWTFederationFilterTokenExchangeTest:testUntrustedIssuerRejectedNoHttpCallandtestServiceUnavailablenow require theerror_description;testNonTokenExchangeRegistryIssuerRejectednow verifies thesendErrormessage.Integration Tests
N/A