[
https://issues.apache.org/jira/browse/KNOX-3460?focusedWorklogId=1042463&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-1042463
]
ASF GitHub Bot logged work on KNOX-3460:
----------------------------------------
Author: ASF GitHub Bot
Created on: 17/Sep/26 19:49
Start Date: 17/Sep/26 19:49
Worklog Time Spent: 10m
Work Description: hsheinblatt commented on code in PR #1409:
URL: https://github.com/apache/knox/pull/1409#discussion_r4040712435
##########
gateway-provider-security-jwt/src/main/java/org/apache/knox/gateway/provider/federation/jwt/filter/TokenExchangeHandler.java:
##########
Review Comment:
I think every handleValidationError call needs an audit call first in this
design, so missing here.
##########
gateway-provider-security-jwt/src/main/java/org/apache/knox/gateway/provider/federation/jwt/filter/TokenExchangeHandler.java:
##########
@@ -348,6 +232,165 @@ ActionOutcome.SUCCESS, auditMessage(policyDecision,
actorIdentity, subjectToken,
}
}
+ private boolean validateExchangeRequestParameters(HttpServletRequest
request, HttpServletResponse response,
+ HttpServletRequest
bodyRequest, String subjectTokenValue, boolean hasActorToken) throws
IOException {
+ final String subjectTokenType =
bodyRequest.getParameter(SUBJECT_TOKEN_TYPE);
+ final String actorTokenType = bodyRequest.getParameter(ACTOR_TOKEN_TYPE);
+ final boolean hasActorTokenType = actorTokenType != null &&
!actorTokenType.isEmpty();
+
+ // RFC 8693 section 2.1: subject_token and subject_token_type are REQUIRED.
+ if (subjectTokenValue == null || subjectTokenValue.isEmpty()) {
+ filter.handleValidationError(request, response,
HttpServletResponse.SC_BAD_REQUEST,
Review Comment:
I think all these need audit messages with them. caller just returns,
doesn't conditionally audit.
##########
gateway-provider-security-jwt/src/test/java/org/apache/knox/gateway/provider/federation/jwt/filter/TokenExchangeHandlerTest.java:
##########
@@ -297,6 +320,77 @@ public void
testSameSubjectExchangeNeverCallsPolicyEvaluation() throws Exception
assertNull(filter.capturedPolicyCheckRequest);
}
+ @Test
+ public void testSameSubjectExchangeAuditsSuccess() throws Exception {
+ // A plain same-subject exchange previously emitted no TOKEN_EXCHANGE
record (only the generic
+ // AUTHENTICATION audit). It must now audit SUCCESS with the subject
acting as its own actor.
+ filter.valid.put("subtok", jwt("alice", "KNOXSSO"));
+ final Capture<String> auditMessage = expectAudit(Action.TOKEN_EXCHANGE,
"USER/alice",
+ ResourceType.PRINCIPAL, ActionOutcome.SUCCESS);
+ handler.handle(request("subtok", JWT_TYPE, null, null), response, chain);
+
+ assertTrue(filter.continued);
+ EasyMock.verify(auditor);
+
assertTrue(auditMessage.getValue().contains("event_type=token_exchange_allowed"));
+ assertTrue(auditMessage.getValue().contains("actor_authority=USER"));
+ assertTrue(auditMessage.getValue().contains("actor_id=alice"));
+ assertTrue(auditMessage.getValue().contains("subject_token_iss=KNOXSSO"));
+ assertTrue(auditMessage.getValue().contains("subject_token_sub=alice"));
+ }
+
+ @Test
+ public void
testSameSubjectExchangeWithAuthorizedRequestedAudienceAuditsSuccess() throws
Exception {
+ // Honoring on and the requested audience is authorized (carried by the
subject token): the
+ // exchange succeeds and audits SUCCESS once.
+ filter.tokenExchangeSameSubjectRequestedAudienceEnabled = true;
+ filter.valid.put("subtok", jwtWithAudiences("alice", "KNOXSSO",
"service-a"));
+ final Capture<String> auditMessage = expectAudit(Action.TOKEN_EXCHANGE,
"USER/alice",
+ ResourceType.PRINCIPAL, ActionOutcome.SUCCESS);
+ handler.handle(exchangeRequest("subtok", null, new String[]
{"service-a"}), response, chain);
+
+ assertTrue(filter.continued);
+ EasyMock.verify(auditor);
+
assertTrue(auditMessage.getValue().contains("event_type=token_exchange_allowed"));
+
assertTrue(auditMessage.getValue().contains("requested_resources=[service-a]"));
+ assertTrue(auditMessage.getValue().contains("audiences_honored=true"));
+ }
+
+ @Test
+ public void
testSameSubjectRequestedAudienceDroppedWhenHonoringDisabledAuditsNotHonored()
throws Exception {
+ // Honoring off but the request carried audiences: they are silently
dropped. The SUCCESS record
+ // still lists what was requested but must report audiences_honored=false,
so a reader never
+ // mistakes the listed-but-dropped requested_resources for honored ones.
+ filter.valid.put("subtok", jwtWithAudiences("alice", "KNOXSSO",
"service-a"));
+ final Capture<String> auditMessage = expectAudit(Action.TOKEN_EXCHANGE,
"USER/alice",
+ ResourceType.PRINCIPAL, ActionOutcome.SUCCESS);
+ handler.handle(exchangeRequest("subtok", null, new String[]
{"service-a"}), response, chain);
+
+ assertTrue(filter.continued);
+ EasyMock.verify(auditor);
+
assertTrue(auditMessage.getValue().contains("event_type=token_exchange_allowed"));
+
assertTrue(auditMessage.getValue().contains("requested_resources=[service-a]"));
+ assertTrue(auditMessage.getValue().contains("audiences_honored=false"));
+ }
+
+ @Test
+ public void testSameSubjectRequestedAudienceRejectionAuditsFailure() throws
Exception {
+ // Honoring on but the requested audience is not among the subject token's
own aud claim: the
+ // exchange is rejected as invalid_target and must audit FAILURE with the
deny reason.
+ filter.tokenExchangeSameSubjectRequestedAudienceEnabled = true;
+ filter.valid.put("subtok", jwtWithAudiences("alice", "KNOXSSO",
"service-a"));
+ final Capture<String> auditMessage = expectAudit(Action.TOKEN_EXCHANGE,
"USER/alice",
+ ResourceType.PRINCIPAL, ActionOutcome.FAILURE);
+ 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);
+ EasyMock.verify(auditor);
+
assertTrue(auditMessage.getValue().contains("event_type=token_exchange_denied"));
+
assertTrue(auditMessage.getValue().contains("deny_reason=requested_audience_not_authorized"));
+ assertTrue(auditMessage.getValue().contains("subject_token_sub=alice"));
+ }
+
Review Comment:
If we add audits for all the other failure cases, then each needs a test
too. Possibly we can simplify those to invalid requests with a simpler message,
but it would be good to have the actor who requested an invalid request and for
whom.
##########
gateway-provider-security-jwt/src/main/java/org/apache/knox/gateway/provider/federation/jwt/filter/TokenExchangeHandler.java:
##########
@@ -348,6 +232,165 @@ ActionOutcome.SUCCESS, auditMessage(policyDecision,
actorIdentity, subjectToken,
}
}
+ private boolean validateExchangeRequestParameters(HttpServletRequest
request, HttpServletResponse response,
+ HttpServletRequest
bodyRequest, String subjectTokenValue, boolean hasActorToken) throws
IOException {
+ final String subjectTokenType =
bodyRequest.getParameter(SUBJECT_TOKEN_TYPE);
+ final String actorTokenType = bodyRequest.getParameter(ACTOR_TOKEN_TYPE);
+ final boolean hasActorTokenType = actorTokenType != null &&
!actorTokenType.isEmpty();
+
+ // RFC 8693 section 2.1: subject_token and subject_token_type are REQUIRED.
+ if (subjectTokenValue == null || subjectTokenValue.isEmpty()) {
+ filter.handleValidationError(request, response,
HttpServletResponse.SC_BAD_REQUEST,
+ "invalid_request", "the subject_token parameter is required");
+ return false;
+ }
+ if (subjectTokenType == null || subjectTokenType.isEmpty()) {
+ filter.handleValidationError(request, response,
HttpServletResponse.SC_BAD_REQUEST,
+ "invalid_request", "the subject_token_type parameter is
required");
+ return false;
+ }
+ // RFC 8693 section 2.1: actor_token_type is REQUIRED when actor_token is
present and MUST NOT
+ // be present otherwise.
+ if (hasActorToken && !hasActorTokenType) {
+ filter.handleValidationError(request, response,
HttpServletResponse.SC_BAD_REQUEST,
+ "invalid_request", "actor_token_type is required when
actor_token is present");
+ return false;
+ }
+ if (!hasActorToken && hasActorTokenType) {
+ filter.handleValidationError(request, response,
HttpServletResponse.SC_BAD_REQUEST,
+ "invalid_request", "actor_token_type must not be present without
actor_token");
+ return false;
+ }
+ // Only JWT-family token types are supported.
+ if (isNotSupportedTokenType(subjectTokenType)) {
+ filter.handleValidationError(request, response,
HttpServletResponse.SC_BAD_REQUEST,
+ "invalid_request", "unsupported subject_token_type " +
subjectTokenType);
+ return false;
+ }
+ if (hasActorToken && isNotSupportedTokenType(actorTokenType)) {
+ filter.handleValidationError(request, response,
HttpServletResponse.SC_BAD_REQUEST,
+ "invalid_request", "unsupported actor_token_type " +
actorTokenType);
+ return false;
+ }
+ return true;
+ }
+
+ private DelegationTokenExchangeOutcome
handleDelegationExchange(HttpServletRequest request,
+ HttpServletResponse response, FilterChain chain, JWT subjectToken,
boolean hasActorToken,
+ String actorTokenValue, String requestedSubjectValue, boolean
requestedSubjectDiffersFromSubject,
+ List<String> requestedAudiences)
+ throws IOException, ServletException, ParseException,
UnknownTokenException {
+ final Set<String> uniqueRequestedAudiences =
distinctNonBlankValues(requestedAudiences);
+ if (!validateDelegationExchangeRequest(request, response, hasActorToken,
+ requestedSubjectDiffersFromSubject, requestedSubjectValue,
uniqueRequestedAudiences)) {
+ // Rejected; error response already sent.
+ return DelegationTokenExchangeOutcome.rejected();
+ }
+
+ // The actor for this exchange is the actor_token's identity when an
actor_token is
+ // present, or the subject_token's identity when this is a headless
delegation
+ // exchange. Either way it must be parsed/validated before the policy
check below; when
+ // an actor_token is present it is reused for Subject construction further
down.
+ JWT actorToken = null;
+ if (hasActorToken) {
+ actorToken = filter.parseAndValidateJWT(request, response, chain,
actorTokenValue);
+ if (actorToken == null) {
+ // Validation failed, error response already sent
+ return DelegationTokenExchangeOutcome.rejected();
+ }
+ }
+ final int actChainDepth = hasActorToken ?
TokenUtils.extractActorChain(subjectToken).size() : 0;
+ final JWT actorIdentitySource = hasActorToken ? actorToken : subjectToken;
+ final ActorIdentity actorIdentity =
ActorIdentity.fromJwt(actorIdentitySource);
+
+ // A single policy-evaluation call per exchange, carrying the full
validated requested-
+ // resource set and an always-empty requestedScopes set (scope enforcement
is deferred).
+ final PolicyCheckRequest policyCheckRequest = new PolicyCheckRequest(
+ actorIdentity.actorAuthority, actorIdentity.actorId,
+ requestedSubjectDiffersFromSubject ? requestedSubjectValue :
subjectToken.getSubject(),
+ uniqueRequestedAudiences, Collections.emptySet(),
requestedSubjectDiffersFromSubject);
+
+ // Policy evaluation resolves canActFor.users and canActFor.groups (the
latter via an LDAP
+ // group lookup on the impersonated subject) and returns a decision; a
subject that matches
+ // neither is reported as a denial below, not as an error.
+ final PolicyDecision policyDecision;
+ try {
+ policyDecision = filter.evaluateDelegationPolicy(policyCheckRequest);
+ } catch (DelegationGroupLookupUnavailableException e) {
+ // The policy is group-based but its canActFor.groups rule could not be
evaluated: LDAP is
+ // either disabled/absent or the group lookup itself failed. This is a
server-side
+ // condition, not a policy denial, so surface a server_error directing
the operator to LDAP
+ // rather than a misleading rejection. The underlying cause is logged by
the policy service.
+ // Audit it as an UNAVAILABLE outcome so every delegation exchange
(allow/deny/unavailable)
+ // leaves a TOKEN_EXCHANGE record.
+ auditing.unavailable(actorIdentity, subjectToken, requestedSubjectValue,
+ "delegation_group_lookup_unavailable", uniqueRequestedAudiences,
actChainDepth);
+ filter.handleValidationError(request, response,
HttpServletResponse.SC_INTERNAL_SERVER_ERROR,
+ "server_error", "This delegation policy restricts canActFor by
group, which requires "
+ + "the LDAP service to resolve group membership; ensure the LDAP
service is enabled "
+ + "and reachable to evaluate group-based delegation policies");
+ return DelegationTokenExchangeOutcome.rejected();
+ }
+
+ if (policyDecision.getDenyReason() != null) {
+ auditing.denied(actorIdentity, subjectToken, requestedSubjectValue,
+ policyDecision.getDenyReason(), uniqueRequestedAudiences,
actChainDepth);
+ // A single, generic denial that does not identify which requested value
failed.
+ filter.handleValidationError(request, response,
HttpServletResponse.SC_BAD_REQUEST,
+ "invalid_request", "The token exchange request is rejected by
policy");
+ return DelegationTokenExchangeOutcome.rejected();
+ }
+ auditing.allowed(actorIdentity, subjectToken, requestedSubjectValue,
uniqueRequestedAudiences,
+ !requestedAudiences.isEmpty(), actChainDepth);
+
+ request.setAttribute(CommonTokenConstants.REQUESTED_TTL_REQUEST_ATTR,
policyDecision.getEffectiveTtlSec());
+
+ return DelegationTokenExchangeOutcome.authorized(actorToken,
!requestedAudiences.isEmpty());
+ }
+
+ private boolean validateDelegationExchangeRequest(HttpServletRequest
request, HttpServletResponse response,
+ boolean hasActorToken, boolean requestedSubjectDiffersFromSubject,
String requestedSubjectValue,
+ Set<String> uniqueRequestedAudiences) throws IOException {
+ // Delegation exchanges are default denied unless
DELEGATION_SERVER_ENABLED is set to true. When
+ // true, only authorized token exchanges will be permitted. Otherwise, any
actor could impersonate
+ // any subject without authorization.
+ if (!filter.isDelegationServerEnabled()) {
+ filter.handleValidationError(request, response,
HttpServletResponse.SC_BAD_REQUEST,
Review Comment:
I think all these calls to handleValidationError need audit calls before
them. The caller doesn't do it.
Issue Time Tracking
-------------------
Worklog Id: (was: 1042463)
Time Spent: 40m (was: 0.5h)
> Audit all token-exchange flows including minted token fields
> ------------------------------------------------------------
>
> Key: KNOX-3460
> URL: https://issues.apache.org/jira/browse/KNOX-3460
> Project: Apache Knox
> Issue Type: Sub-task
> Components: Server
> Affects Versions: 3.1.0
> Reporter: Sandor Molnar
> Assignee: Sandor Molnar
> Priority: Major
> Fix For: 3.1.0
>
> Time Spent: 40m
> Remaining Estimate: 0h
>
> Only the delegation policy decision is audited today (TokenExchangeHandler
> success/failure inside the delegation branch). The same-subject exchange path
> emits no TOKEN_EXCHANGE audit record at all (only the generic AUTHENTICATION
> audit from the shared {{{}continueWithEstablishedSecurityContext{}}}).
> Minted-token fields — jti, expiry, issued_subject — are unauditable from the
> filter because they only exist after TokenResource mints the token, and
> TokenResource has no auditing whatsoever today.
> *Scope:*
> # Audit all three exchange paths in the handler with the fields known
> pre-mint (incl. same-subject).
> # Add a TOKEN_EXCHANGE mint-time audit in TokenResource.getJWT() for
> exchange-originated mints, emitting jti/expiry/issued_subject
> *Explicitly out of scope:* auditing ordinary (non-exchange) token issuance.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
