Repository navigation
KNOX-3534 - Add ingress source routing to the k8s destination audience validator - #1476
Open
hsheinblatt wants to merge 5 commits into
Open
hsheinblatt wants to merge 5 commits into
hsheinblatt wants to merge 5 commits into
Conversation
added 5 commits
October 27, 2026 23:19
… routing and early-rejection ordering Pins how K8sDestinationAudienceValidator routes a token by the presence or absence of its act claim, and the order in which its early rejections fire relative to one another. Several of those rejections differ only in their message, and nothing in the suite asserted which one wins when more than one applies. No production change.
…dience validator A single topology can now authenticate both tokens arriving at the API through an ingress gateway and delegation tokens used for service-to-service calls. Four new parameters, all defaulting to the previous behavior: request.audience.k8s.ingress.source.spiffe-id.exception.enabled request.audience.k8s.ingress.source.spiffe-ids request.audience.k8s.ingress.source.spiffe-ids.skip.trust.domain.match request.audience.k8s.ingress.source.spiffe-ids.skip.validation When the exception is enabled and a request's source SPIFFE id names one of the configured ingress identities, a token with no act claim is either accepted without any audience check or validated against the fixed configured allowed-audience list, rather than against the request destination. A token that does carry an act claim is never exempted; such a token arriving from an ingress identity is rejected, since it authorizes a service-to-service call and does not legitimately arrive through the ingress. Enabling the exception without the source SPIFFE id header and the ingress identity list fails topology startup. With the exception left disabled, every token is validated exactly as before. Comments: Issues with test naming and structure Naming is confusing, switch from 'exception'.
Refactor tests with better named helpers.
KNOX-3534 - Add ingress source routing to the k8s destination audience validator Tokens that reach the destination validator through an ingress gateway carry an audience that does not name an in-cluster destination, so the destination checks cannot be applied to them. Add an opt-in routing step, decided by the source SPIFFE id header, that sends such tokens to a different audience check. New parameters (all read by K8sDestinationAudienceValidator.init): - ingress.source.spiffe-id.routing.enabled (default false): master switch. - ingress.source.spiffe-ids: comma-separated ingress SPIFFE ids. Each entry is trimmed and must parse; an unparseable entry fails init even when routing is disabled, so a typo cannot lie dormant until routing is enabled. - ingress.source.spiffe-ids.routing.skip.trust.domain.match (default false): match an ingress id on namespace and service account only, ignoring trust domain. - ingress.source.spiffe-ids.routing.skip.validation (default false): see below. init() requirements: enabling routing without the source SPIFFE id header parameter, or without a non-blank ingress id list, fails topology startup naming the missing parameter. The two skip flags are inert when routing is disabled; with routing disabled every token is validated exactly as before. Behavior when routing is enabled and the request's source SPIFFE id matches a configured ingress id (matching is case-insensitive): - Token without an act claim, skip.validation=false: accepted only if its audience is in the configured knox.token.audiences list, using AbstractJWTFilter.matchesConfiguredAudiences (a null list accepts all, NONE admits a token with no aud, NONE is not a wildcard). A rejection from this check carries no message, unlike destination rejections. - Token without an act claim, skip.validation=true: accepted with no audience check. - Token with an act claim: rejected with a message naming the act claim, the ingress source and the source header. A delegation token authorizes a must not arrive through the ingress. Skip flags do not exempt it, and it is rejected before the act-sub binding check. A malformed act claim from an ingress source is rejected the same way. Both ingress routes run before the aud-presence and header-presence checks because the configured-list check has its own handling of a token with no aud. A request whose source header is missing or unparseable is not an ingress source and takes the existing flow, including the existing missing-source rejection. With validate.audiences.without.act.claim=false that flow already returns the configured-list result first, and this change does not alter it. Security note: the source SPIFFE id header is trusted at face value. Whoever can set it can have a request routed as ingress, so it must be set or overwritten by a trusted proxy. The configured-list route is only as strong as knox.token.audiences. The source SPIFFE id header is preexisting, just now used in another optional flow. Tests: K8sIngressSourceRoutingTest covers init requirements, each route proved by inputs that only that route accepts, trust-domain matching, the act claim rejection and its ordering, and that require-all-audiences-match and the namespace agreement check still apply to non-ingress sources. MatchesConfiguredAudiencesTest gains cases for a null configured list with a token without aud, a single matching audience, and NONE not matching a non-matching audience.
…alidator Describe the four request.audience.k8s.ingress.source.* parameters of the Kubernetes destination audience validator in the request audience validator docs: when a request's source SPIFFE id matches a configured ingress source, a token with an act claim is rejected, and a token with no act claim is checked only against the fixed knox.token.audiences list, or not audience-checked at all when skip.validation is true. Adds a parameter table, an "Ingress source routing" section with a full outcome table, startup failure conditions and a trust warning for the source header, and updates the example topology, enforcement table, example request, prerequisites and failure modes. Existing sentences that assumed every token reaches destination validation are corrected. Docs only.
Test Results 4 files 4 suites 1m 20s ⏱️ Results for commit b2b41dc. |
smolnar82
requested changes
Nov 2, 2026
smolnar82
left a comment
smolnar82
left a comment
Contributor
There was a problem hiding this comment.
About 140 of the ~225 lines added to K8sDestinationAudienceValidator are comments, and the four new param constants alone have ~100 lines of javadoc. Comments this long don't get read, they hide the ~20 lines of real logic from reviewers, and they repeat the same facts in 4–5 places (class javadoc, param javadocs, inline comments, test javadoc, .md), which will drift out of sync as soon as the behavior changes. Please keep it to a line or two per param, inline comments only for a non-obvious why, and the details once in the docs page.
Comment on lines
+601
to
+606
| } else if (fromIngressSource) { | ||
| // An act claim means the token authorizes one workload to call another inside the cluster. | ||
| // Arriving from an ingress identity means it was presented from outside, which no such call | ||
| // does. | ||
| return new AudienceValidationResult(false, "Token has an act claim but its source SPIFFE id in header " | ||
| + sourceSpiffeIdHeader + " is a configured ingress source: " + request.getHeader(sourceSpiffeIdHeader)); |
Contributor
There was a problem hiding this comment.
Diverges from the JIRA. KNOX-3534: "Delegation tokens are not allowed on that path, so even if they come from an ingress, still treat them as they are now." I read that as "don't send act tokens down the ingress route; destination-validate them as today", not "reject them".
With routing enabled, an RFC 8693-exchanged token presented through the ingress used to be destination-validated. Now it always gets a 400. That breaks clients outside the cluster that re-exchange a delegated token through the public ingress.
Could we either:
- drop this branch, so
acttokens fall through to destination validation as before, or - put the rejection behind its own flag that defaults to
false(e.g.request.audience.k8s.ingress.routing.reject.delegation.tokens)?
If the stricter behavior is intended, please update the JIRA so the two agree.
| return ingressSourceRoutingSkipValidation | ||
| ? AudienceValidationResult.of(true) | ||
| : AudienceValidationResult.of( | ||
| AbstractJWTFilter.matchesConfiguredAudiences(token, configuredAudiences)); |
Contributor
There was a problem hiding this comment.
matchesConfiguredAudiences(token, null) returns true, so with the default skip.validation=false and no knox.token.audiences, this route behaves exactly like skip.validation=true. The docs mention it, but nothing at runtime does. Could we log a WARN at startup when routing is enabled and the provider has no knox.token.audiences? (AbstractJWTFilter sees both.) Alternatively, consider a dedicated ingress.source.audiences list. Reusing knox.token.audiences also changes what mesh tokens without act accept when validate.audiences.without.act.claim=false, which makes it hard to restrict ingress tokens without affecting the rest.
Comment on lines
+387
to
+388
| public static final String INGRESS_SOURCE_SPIFFE_ID_ROUTING_ENABLED_PARAM = | ||
| PARAM_PREFIX + "ingress.source.spiffe-id.routing.enabled"; |
Contributor
There was a problem hiding this comment.
Naming nit, but these names become public config once released: this one says spiffe-id (singular) while the others say spiffe-ids. The names are also quite long. Something like this might be clearer: (current -> suggested):
ingress.source.spiffe-id.routing.enabled->ingress.routing.enabledingress.source.spiffe-ids->ingress.source.spiffe-idsingress.source.spiffe-ids.routing.skip.trust.domain.match->ingress.routing.skip.trust.domain.matchingress.source.spiffe-ids.routing.skip.validation->ingress.routing.skip.audience.validation
Comment on lines
+52
to
+58
| * <p>A token carrying a delegation {@code act} claim is always destination-validated; see | ||
| * {@link #validate(HttpServletRequest, JWT, List)}. The one departure is ingress source routing: | ||
| * when {@link #INGRESS_SOURCE_SPIFFE_ID_ROUTING_ENABLED_PARAM} is {@code true} and the request's | ||
| * source SPIFFE id names one of the {@link #INGRESS_SOURCE_SPIFFE_IDS_PARAM}, such a token is | ||
| * rejected outright, because a delegation token must not arrive through the ingress. | ||
| * A token with no {@code act} claim is destination-validated too, unless ingress source routing | ||
| * applies to it or {@link #VALIDATE_AUDIENCES_WITHOUT_ACT_CLAIM_PARAM} is {@code false}. |
Contributor
There was a problem hiding this comment.
If the act-token rejection changes (see the comment on validate()), this paragraph needs updating too.
smolnar82
reviewed
Nov 2, 2026
|
|
||
| Either way a token with no `act` claim from an ingress source does not reach destination validation, so the destination SPIFFE-id, server-name and path headers are not required for it, none of the matching described under How this validator matches applies, and `validate.audiences.without.act.claim` and `require-all-audiences-match` have no effect on it. | ||
|
|
||
| If `knox.token.audiences` is not configured, the fixed-list check accepts every token, including one with no `aud` claim. If it is configured, a token with an `aud` claim must share at least one entry with it, and a token with no `aud` claim is accepted only if the list contains `NONE`. With routing enabled and `skip.validation` left `false`, a token with no `act` claim from an ingress source is therefore checked only against `knox.token.audiences`; configure that list on the provider to restrict it at all. |
Contributor
There was a problem hiding this comment.
This is the most important operational caveat of the feature, but it's buried mid-section. Could it go into the skip.validation row of the param table as an explicit must configure knox.token.audiences note, or a warning callout? Otherwise the default looks safe while actually accepting every aud.
This branch has not been deployed
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-3534 - Add ingress source routing to the k8s destination audience validator
What changes were proposed in this pull request?
K8sDestinationAudienceValidator can now route a request that arrives from a
configured ingress gateway to a different audience check than validation
against the request destination. It is off by default. It is controlled by
four new request.audience.k8s.* parameters:
ingress.source.spiffe-id.routing.enabled (default false)
ingress.source.spiffe-ids (no default)
ingress.source.spiffe-ids.routing.skip.trust.domain.match (default false)
ingress.source.spiffe-ids.routing.skip.validation (default false)
A request is from an ingress source when the SPIFFE id in the configured
source header matches an entry of ingress.source.spiffe-ids. Namespace,
service account and trust domain are compared ignoring case; the trust domain
is left out of the comparison only when skip.trust.domain.match is true. A
missing or unparseable header is never an ingress source. With routing
enabled, such a request is routed by its token:
the header and its value), regardless of skip.validation, before any other
check;
against the fixed knox.token.audiences list, or, with skip.validation true,
not audience-checked at all. The destination, server-name and path headers
are not required for it.
Every other request is handled as before. Startup fails if routing is enabled
without the source SPIFFE id header or without ingress.source.spiffe-ids, or
if any ingress id entry is not a full parseable SPIFFE id (checked even when
routing is disabled).
The change also has a test-only part: regression coverage that pins the
existing routing of tokens with no act claim and the order of early
rejections before the new behavior was added (first commit of the series).
The parameters were later renamed and the javadoc, class and test names
updated (follow-up commits). knox-site/docs/config_request_audience_validator.md
documents the parameters, the outcome table, trust and security notes, and
corrects existing statements the feature made inaccurate.
How was this patch tested?
row of the outcome table in the class javadoc, trust-domain skip, case
insensitivity, multiple ingress ids, missing/unparseable headers, fixed
list semantics (null list, NONE entry), skip.validation, and ordering
(rejection before actor-subject binding, skip before path header check).
with no act claim with routing off, including fallback to the fixed list
and the ordering of the destination and source header rejections.
(checkstyle, PMD, SpotBugs, forbiddenapis and unit tests): 769 tests in
the module, 0 failures, BUILD SUCCESS.
Integration Tests
None
UI changes
N/A