[
https://issues.apache.org/jira/browse/KNOX-3461?focusedWorklogId=1042116&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-1042116
]
ASF GitHub Bot logged work on KNOX-3461:
----------------------------------------
Author: ASF GitHub Bot
Created on: 16/Sep/26 21:06
Start Date: 16/Sep/26 21:06
Worklog Time Spent: 10m
Work Description: hsheinblatt commented on code in PR #1405:
URL: https://github.com/apache/knox/pull/1405#discussion_r4030623241
##########
gateway-provider-security-jwt/src/main/java/org/apache/knox/gateway/provider/federation/jwt/filter/JWTFederationFilter.java:
##########
@@ -131,6 +131,18 @@ public class JWTFederationFilter extends AbstractJWTFilter
{
// Has no effect on a same-subject exchange.
public static final String DELEGATION_ENFORCE_REQUESTED_AUDIENCE_MAX_ONE =
"delegation.enforce.requested.audience.max.one";
+ // Topology provider param (default false/absent). Gates whether a
+ // same-subject token exchange may honor a requested audience/resource at
+ // all. While off (the fail-safe default), the requested audience is ignored
+ // for same-subject exchanges, so a passthrough audience validator cannot
+ // mint an arbitrarily-audienced token without authorization. While on, the
+ // requested audience is honored but must be authorized against the subject
+ // token's own aud claim; any requested value the subject token does not
+ // already carry is rejected. Unlike the delegation.* flags above, this one
+ // affects only same-subject exchanges (delegation exchanges are authorized
+ // by the delegation policy regardless of this flag).
+ public static final String
DELEGATION_SAME_SUBJECT_REQUESTED_AUDIENCE_ENABLED =
"delegation.same.subject.requested.audience.enabled";
Review Comment:
I had been using 'delegation' to mean cross-subject token exchange
specifically. For consistency, I wouldn't call this 'delegation'. I can see how
it's mostly relevant if one is using delegation, but in principle one could
enforce same subject token exchange audience validation with delegation
exchange turned off. So, I suggest a different prefix for the name here,
possibly "token-exchange.same.subject.requested.audience.enabled" or
'token.exchange' or 'rfc8693.token.exchange' since it only applies for the
specific OIDC grant type.
##########
gateway-provider-security-jwt/src/main/java/org/apache/knox/gateway/provider/federation/jwt/filter/TokenExchangeHandler.java:
##########
@@ -316,7 +316,29 @@ ActionOutcome.SUCCESS, auditMessage(policyDecision,
actorIdentity, subjectToken,
// present so that KNOXTOKEN falls back to its resource query parameter
otherwise; when set,
// the body value takes precedence over the query parameter.
if (!requestedAudiences.isEmpty()) {
-
request.setAttribute(CommonTokenConstants.REQUESTED_AUDIENCES_REQUEST_ATTR,
requestedAudiences);
+ final boolean sameSubjectExchange = !hasActorToken &&
!requestedSubjectDiffersFromSubject;
Review Comment:
nit: I hadn't computed the boolean delegationExchange above for the big
branch that handles delegation, because it was only used once, so I just
inlined the AND expression. It branches off for the delegation authorization
logic. But if we're using it again here, we can compute it up there and share
it.
nit: Arguably this logic, or at least the enforcement logic could be moved
into the above conditional, where we already have an if delegation block. We
can add an else block to enforce the audiences are valid there, so all the
enforcement is done before the subject setting. That is, if we later add more
options for how to validate same-subject audience requests, we'd have a more
complex validation step there. Grouping it with setting the attribute is a bit
confusing.
##########
gateway-provider-security-jwt/src/test/java/org/apache/knox/gateway/provider/federation/jwt/filter/TokenExchangeHandlerTest.java:
##########
@@ -571,6 +560,74 @@ public void
testNoResourceOrAudienceLeavesRequestAttributeUnset() throws Excepti
assertFalse(requestedAudiencesAttr.hasCaptured());
}
+ @Test
+ public void testSameSubjectRequestedAudienceIgnoredWhenFlagDisabled() throws
Exception {
+ // Fail-safe default: for a same-subject exchange the requested audience
is dropped entirely
+ // (not conveyed downstream), so a passthrough audience validator cannot
mint an
+ // arbitrarily-audienced token without authorization. The exchange still
succeeds.
+ filter.valid.put("subtok", jwtWithAudiences("alice", "KNOXSSO",
"service-a"));
+ handler.handle(exchangeRequest("subtok", null, new String[]
{"service-a"}), response, chain);
+
+ assertTrue(filter.continued);
+ assertFalse(requestedAudiencesAttr.hasCaptured());
+ }
+
+ @Test
+ public void
testSameSubjectRequestedAudienceAuthorizedWhenSubsetOfSubjectTokenAudience()
throws Exception {
+ // Honoring on: a requested audience the subject token already carries is
authorized and
+ // conveyed, even when the subject token carries additional audiences.
+ filter.delegationSameSubjectRequestedAudienceEnabled = true;
+ filter.valid.put("subtok", jwtWithAudiences("alice", "KNOXSSO",
"service-a", "service-b", "service-c"));
+ handler.handle(exchangeRequest("subtok", null, new String[]
{"service-a"}), response, chain);
+
+ assertTrue(filter.continued);
+ assertEquals(Arrays.asList("service-a"),
requestedAudiencesAttr.getValue());
+ }
+
+ @Test
+ public void
testSameSubjectRequestedAudienceRejectedWhenNotInSubjectTokenAudience() throws
Exception {
+ // Honoring on but the requested audience is not among the subject token's
own aud claim:
+ // rejected as invalid_target and never conveyed.
+ filter.delegationSameSubjectRequestedAudienceEnabled = true;
+ filter.valid.put("subtok", jwtWithAudiences("alice", "KNOXSSO",
"service-a"));
+ handler.handle(exchangeRequest("subtok", null, new String[]
{"service-b"}), response, chain);
+
+ assertFalse(filter.continued);
+ assertEquals(HttpServletResponse.SC_BAD_REQUEST, filter.errorStatus);
+ assertEquals("invalid_target", filter.error);
+ assertFalse(requestedAudiencesAttr.hasCaptured());
+ }
+
+ @Test
+ public void
testSameSubjectRequestedAudienceRejectedWhenSubjectTokenHasNoAudience() throws
Exception {
+ // Honoring on and a requested audience present, but the subject token
carries no aud claim:
+ // there is nothing to authorize against, so the request is rejected.
+ filter.delegationSameSubjectRequestedAudienceEnabled = true;
+ filter.valid.put("subtok", jwt("alice", "KNOXSSO"));
+ handler.handle(exchangeRequest("subtok", null, new String[]
{"service-a"}), response, chain);
+
+ assertFalse(filter.continued);
+ assertEquals(HttpServletResponse.SC_BAD_REQUEST, filter.errorStatus);
+ assertEquals("invalid_target", filter.error);
+ assertFalse(requestedAudiencesAttr.hasCaptured());
+ }
Review Comment:
I'd add another boundary test when the request has some audiences in the
subject token claim and some not, to show it checks all.
Issue Time Tracking
-------------------
Worklog Id: (was: 1042116)
Time Spent: 50m (was: 40m)
> Same subject validation for requested audience
> ----------------------------------------------
>
> Key: KNOX-3461
> URL: https://issues.apache.org/jira/browse/KNOX-3461
> Project: Apache Knox
> Issue Type: Sub-task
> Components: JWT
> Reporter: Harrison Sheinblatt
> Assignee: Sandor Molnar
> Priority: Major
> Time Spent: 50m
> Remaining Estimate: 0h
>
> When delegation is enabled, we'll want to set the knoxidf audience strategy
> to passthrough so we rely on the delegation policy to authorize a requested
> audience instead of the hardcoded list. But once that's set, then the
> same-subject requested audience is unauthorized at all. So we either need to
> not allow requested audiences just for same-subject exchanges with a setting,
> or add enforcement, or both.
> The simplest thing is to add a setting to skip reading the requested audience
> only for same-subject token exchanges.
> The natural authz is to validate the original subject token has the audience
> requested to allow the requested audience. So if the flag to allow reading
> the audience for same-subject exchange is on, we'd authorize on the subject
> token. If this proves a problem, we can turn it off or figure out a way to
> add more audiences to the user tokens knox issues.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
