lmccay commented on code in PR #1476:
URL: https://github.com/apache/knox/pull/1476#discussion_r4472683345
##########
gateway-provider-security-jwt/src/main/java/org/apache/knox/gateway/provider/federation/jwt/filter/K8sDestinationAudienceValidator.java:
##########
@@ -399,19 +526,84 @@ public void init(final FilterConfig filterConfig) throws
Exception {
enforceActSubIsServiceAccount =
Boolean.parseBoolean(filterConfig.getInitParameter(ENFORCE_ACT_SUB_IS_SERVICE_ACCOUNT_PARAM));
+ ingressSourceRoutingEnabled = Boolean.parseBoolean(
+
filterConfig.getInitParameter(INGRESS_SOURCE_SPIFFE_ID_ROUTING_ENABLED_PARAM));
+ ingressSourceRoutingSkipTrustDomainMatch = Boolean.parseBoolean(
+
filterConfig.getInitParameter(INGRESS_SOURCE_SPIFFE_IDS_ROUTING_SKIP_TRUST_DOMAIN_MATCH_PARAM));
+ ingressSourceRoutingSkipValidation = Boolean.parseBoolean(
+
filterConfig.getInitParameter(INGRESS_SOURCE_SPIFFE_IDS_ROUTING_SKIP_VALIDATION_PARAM));
+
+ // Parsed strictly whether or not routing is enabled, so a malformed entry
fails the
+ // topology at startup instead of sitting inert.
+ final String ingressSourceSpiffeIdsParam =
+
blankToNull(filterConfig.getInitParameter(INGRESS_SOURCE_SPIFFE_IDS_PARAM));
+ ingressSourceSpiffeIds = new ArrayList<>();
+ if (ingressSourceSpiffeIdsParam != null) {
+ for (final String entry :
parseCommaSeparated(ingressSourceSpiffeIdsParam)) {
+ final Optional<SpiffeId> parsed = SpiffeId.parse(entry);
+ if (parsed.isEmpty()) {
+ throw new ServletException(String.format(Locale.ROOT,
+ "%s contains an invalid SPIFFE id entry: %s",
INGRESS_SOURCE_SPIFFE_IDS_PARAM, entry));
+ }
+ ingressSourceSpiffeIds.add(lowerCased(parsed.get()));
+ }
+ }
+
if (enforceActSubMatchesSourceSpiffeId && sourceSpiffeIdHeader == null) {
throw new ServletException(String.format(Locale.ROOT,
"%s is true but %s is not configured",
ENFORCE_ACT_SUB_MATCHES_SOURCE_SPIFFE_ID_PARAM,
SOURCE_SPIFFE_ID_HEADER_PARAM));
}
+
+ if (ingressSourceRoutingEnabled) {
+ if (sourceSpiffeIdHeader == null) {
+ throw new ServletException(String.format(Locale.ROOT,
+ "%s is true but %s is not configured",
INGRESS_SOURCE_SPIFFE_ID_ROUTING_ENABLED_PARAM,
+ SOURCE_SPIFFE_ID_HEADER_PARAM));
+ }
+ if (ingressSourceSpiffeIds.isEmpty()) {
+ throw new ServletException(String.format(Locale.ROOT,
+ "%s is true but %s is not configured",
INGRESS_SOURCE_SPIFFE_ID_ROUTING_ENABLED_PARAM,
+ INGRESS_SOURCE_SPIFFE_IDS_PARAM));
+ }
+ }
}
@Override
public AudienceValidationResult validate(final HttpServletRequest request,
final JWT token,
final List<String> configuredAudiences) {
final boolean hasActClaim = token.getClaimAsObject(JWTToken.ACT_CLAIM) !=
null;
- if (!hasActClaim && !validateAudiencesWithoutActClaim) {
- return
AudienceValidationResult.of(AbstractJWTFilter.matchesConfiguredAudiences(token,
configuredAudiences));
+
+ // Resolved before the early returns below because the routing decision
needs it. An
+ // absent value is not rejected here: that rejection happens further down,
after the destination
+ // header and server-name checks, so a request whose source header is
missing or unparseable
+ // is reported only once those earlier checks have passed.
+ final Optional<SpiffeId> sourceSpiffeId = sourceSpiffeIdHeader == null
+ ? Optional.empty() : sourceSpiffeIdFromHeader(request);
+ final boolean fromIngressSource = ingressSourceRoutingEnabled
+ && isConfiguredIngressSource(sourceSpiffeId);
+
+ if (!hasActClaim) {
+ if (fromIngressSource) {
+ // A token with no act claim arriving through the ingress names no
in-cluster destination,
+ // so its audience is either not checked at all or checked against the
fixed configured
+ // list. Both decisions are made before the aud claim is required to
be present, because
+ // the fixed-list check has its own handling for a token that carries
no aud claim.
+ return ingressSourceRoutingSkipValidation
+ ? AudienceValidationResult.of(true)
+ : AudienceValidationResult.of(
+ AbstractJWTFilter.matchesConfiguredAudiences(token,
configuredAudiences));
+ }
+ if (!validateAudiencesWithoutActClaim) {
+ return AudienceValidationResult.of(
+ AbstractJWTFilter.matchesConfiguredAudiences(token,
configuredAudiences));
+ }
+ } 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));
Review Comment:
@smolnar82 - I agree. This will break existing clients. We can't do this
now, could be made to be a configurable pattern but I don't know that we really
need to treat ingress differently at this point even. I'll continue to review.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
