This is an automated email from the ASF dual-hosted git repository. smolnar82 pushed a commit to branch knox_idf in repository https://gitbox.apache.org/repos/asf/knox.git
commit 571a9d97f9b85003266f32378b9064c51c5e291d Author: Sandor Molnar <[email protected]> AuthorDate: Tue Aug 11 22:49:48 2026 +0200 KNOX-3414: bind federated id_token to the request via OIDC nonce (review finding H2) The federated broker leg sent no nonce to the external OP and never checked one on the callback, so a signature-valid id_token minted for a different (or attacker-initiated) authorization request could be injected/replayed at the callback. Add the OIDC nonce binding (OIDC Core 3.1.2.1): - New JVM-singleton FederatedNonceStore (mirrors FederatedOpConfigurationStore), single-use, keyed by the federated login-session id (== the state echoed by the OP). - WebSSOResource.federatedOpLogin mints a per-request nonce, stashes it, and passes it to the redirect builder. - KnoxIDFUtils.buildFederatedOpAuthRedirect now appends &nonce=<value> (percent-encoded). - AuthorizeResource.authCallback retrieves and single-use-removes the expected nonce and, only after the id_token's signature/issuer/audience are verified, requires its nonce claim to match (verifyFederatedNonce). Missing expected nonce or a mismatch fails the flow. Covered by AuthorizeResourceFederatedNonceTest (match passes; mismatch, missing claim, and missing expected nonce all rejected). Co-Authored-By: Claude Opus 4.8 <[email protected]> --- .../gateway/service/knoxidf/AuthorizeResource.java | 36 ++++++++++- .../AuthorizeResourceFederatedNonceTest.java | 75 ++++++++++++++++++++++ .../gateway/service/knoxsso/WebSSOResource.java | 10 ++- .../gateway/util/knoxidf/FederatedNonceStore.java | 42 ++++++++++++ .../knox/gateway/util/knoxidf/KnoxIDFUtils.java | 11 ++-- 5 files changed, 168 insertions(+), 6 deletions(-) diff --git a/gateway-service-knoxidf/src/main/java/org/apache/knox/gateway/service/knoxidf/AuthorizeResource.java b/gateway-service-knoxidf/src/main/java/org/apache/knox/gateway/service/knoxidf/AuthorizeResource.java index 8cff9a805..c2d063096 100644 --- a/gateway-service-knoxidf/src/main/java/org/apache/knox/gateway/service/knoxidf/AuthorizeResource.java +++ b/gateway-service-knoxidf/src/main/java/org/apache/knox/gateway/service/knoxidf/AuthorizeResource.java @@ -54,6 +54,7 @@ import org.apache.knox.gateway.services.security.token.impl.JWTToken; import org.apache.knox.gateway.util.JsonUtils; import org.apache.knox.gateway.util.knoxidf.AuthorizeRequestMetadata; import org.apache.knox.gateway.util.knoxidf.AuthorizeRequestMetadataStore; +import org.apache.knox.gateway.util.knoxidf.FederatedNonceStore; import org.apache.knox.gateway.util.knoxidf.FederatedOpConfiguration; import org.apache.knox.gateway.util.knoxidf.FederatedOpConfigurationStore; @@ -120,6 +121,7 @@ public class AuthorizeResource extends PasscodeTokenResourceBase { private static final String UTF_8 = StandardCharsets.UTF_8.name(); private AuthorizeRequestMetadataStore authorizeRequestMetadataStore; private final FederatedOpConfigurationStore federatedOpConfigurationStore = FederatedOpConfigurationStore.getInstance(120000L); + private final FederatedNonceStore federatedNonceStore = FederatedNonceStore.getInstance(120000L); @Context private HttpServletRequest request; @@ -291,9 +293,13 @@ public class AuthorizeResource extends PasscodeTokenResourceBase { return error("invalid_request", "No federated OP configuration available for the request"); } // The federated callback state is single-use: invalidate it in both stores now that it has - // been validated and captured, so a replayed callback with the same state is rejected. + // been validated and captured, so a replayed callback with the same state is rejected. The + // nonce Knox sent to the OP was stashed under the same key (the login-session id == state); + // retrieve and invalidate it too so it cannot be reused. authorizeRequestMetadataStore.remove(state); federatedOpConfigurationStore.remove(state); + final String expectedNonce = federatedNonceStore.get(state); + federatedNonceStore.remove(state); final Pair<String, String> federatedTokens = exchangeFederatedAuthCodeToTokens(federatedAuthCode, federatedOpConfiguration); if (StringUtils.isBlank(federatedTokens.getLeft())) { return error("invalid_request", "Federated OP did not return an id_token"); @@ -304,6 +310,15 @@ public class AuthorizeResource extends PasscodeTokenResourceBase { if (validationError != null) { return validationError; } + // Bind the (now signature-verified) id_token to this authorization request (OIDC Core 3.1.2.1): + // its nonce claim must equal the nonce Knox generated and sent to the OP for this login session. + // This is checked only after the token's authenticity is established, so a forged token cannot + // supply its own matching nonce. A missing expected nonce (e.g. expired/replayed state) or a + // mismatch fails the flow. + final Response nonceError = verifyFederatedNonce(expectedNonce, federatedIdToken); + if (nonceError != null) { + return nonceError; + } final FederatedIdentity federatedIdentity = resolveFederatedIdentity(federatedIdToken, federatedOpConfiguration.getName()); return getAuthCodeFromKnox(authorizeRequestMetadata, Pair.of(federatedIdentity.getId(), federatedTokens.getRight())); } @@ -581,6 +596,25 @@ public class AuthorizeResource extends PasscodeTokenResourceBase { return null; } + /** + * Verifies the OIDC {@code nonce} binding for a federated login (OIDC Core 3.1.2.1). The + * {@code expectedNonce} is the value Knox generated for this login session and sent to the OP; + * it must equal the {@code nonce} claim of the (already signature-verified) id_token. Callers + * must invoke this only after {@link #validateFederatedIdToken} succeeds so a forged token cannot + * assert its own nonce. + * + * @return an error {@link Response} on absence/mismatch, or {@code null} when the nonce matches. + */ + Response verifyFederatedNonce(final String expectedNonce, final JWT idToken) { + if (StringUtils.isBlank(expectedNonce)) { + return error("invalid_request", "Missing or expired federated login nonce"); + } + if (!expectedNonce.equals(idToken.getClaim(NONCE))) { + return error("invalid_request", "Federated id_token nonce mismatch"); + } + return null; + } + private FederatedIdentity resolveFederatedIdentity(final JWT jwt, String opName) { final String issuer = jwt.getIssuer(); final String subject = jwt.getSubject(); diff --git a/gateway-service-knoxidf/src/test/java/org/apache/knox/gateway/service/knoxidf/AuthorizeResourceFederatedNonceTest.java b/gateway-service-knoxidf/src/test/java/org/apache/knox/gateway/service/knoxidf/AuthorizeResourceFederatedNonceTest.java new file mode 100644 index 000000000..0c2ae1577 --- /dev/null +++ b/gateway-service-knoxidf/src/test/java/org/apache/knox/gateway/service/knoxidf/AuthorizeResourceFederatedNonceTest.java @@ -0,0 +1,75 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with this + * work for additional information regarding copyright ownership. The ASF + * licenses this file to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * <p> + * http://www.apache.org/licenses/LICENSE-2.0 + * <p> + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations under + * the License. + */ +package org.apache.knox.gateway.service.knoxidf; + +import static org.apache.knox.gateway.util.knoxidf.KnoxIDFConstants.NONCE; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; + +import javax.ws.rs.core.Response; + +import org.apache.knox.gateway.services.security.token.impl.JWT; +import org.easymock.EasyMock; +import org.junit.Test; + +/** + * Verifies the federated OIDC {@code nonce} binding (review finding H2). Knox mints a nonce for each + * federated login session, sends it to the OP, and — once the returned id_token's signature/issuer/ + * audience have been verified — requires the id_token's {@code nonce} claim to equal that value. This + * defeats id_token replay/injection: a token minted for a different (or attacker-initiated) request + * carries a different nonce and is rejected. + */ +public class AuthorizeResourceFederatedNonceTest { + + private static final String NONCE_VALUE = "6d1c7a90-4b2e-4c1a-9f3d-0a1b2c3d4e5f"; + + private static JWT idTokenWithNonce(final String nonce) { + final JWT idToken = EasyMock.createNiceMock(JWT.class); + EasyMock.expect(idToken.getClaim(NONCE)).andReturn(nonce).anyTimes(); + EasyMock.replay(idToken); + return idToken; + } + + @Test + public void testMatchingNoncePasses() { + final Response result = new AuthorizeResource().verifyFederatedNonce(NONCE_VALUE, idTokenWithNonce(NONCE_VALUE)); + assertNull("A matching nonce must pass (null == no error).", result); + } + + @Test + public void testMismatchedNonceIsRejected() { + final Response result = new AuthorizeResource().verifyFederatedNonce(NONCE_VALUE, idTokenWithNonce("some-other-nonce")); + assertNotNull("A mismatched nonce must be rejected.", result); + assertEquals(Response.Status.BAD_REQUEST.getStatusCode(), result.getStatus()); + } + + @Test + public void testMissingClaimInTokenIsRejected() { + final Response result = new AuthorizeResource().verifyFederatedNonce(NONCE_VALUE, idTokenWithNonce(null)); + assertNotNull("An id_token without a nonce claim must be rejected.", result); + assertEquals(Response.Status.BAD_REQUEST.getStatusCode(), result.getStatus()); + } + + @Test + public void testMissingExpectedNonceIsRejected() { + // No stored nonce (expired/replayed state) must fail closed rather than accept any token. + final Response result = new AuthorizeResource().verifyFederatedNonce(null, idTokenWithNonce(NONCE_VALUE)); + assertNotNull("A missing expected nonce must be rejected.", result); + assertEquals(Response.Status.BAD_REQUEST.getStatusCode(), result.getStatus()); + } +} diff --git a/gateway-service-knoxsso/src/main/java/org/apache/knox/gateway/service/knoxsso/WebSSOResource.java b/gateway-service-knoxsso/src/main/java/org/apache/knox/gateway/service/knoxsso/WebSSOResource.java index eea58f1dc..ce2787572 100644 --- a/gateway-service-knoxsso/src/main/java/org/apache/knox/gateway/service/knoxsso/WebSSOResource.java +++ b/gateway-service-knoxsso/src/main/java/org/apache/knox/gateway/service/knoxsso/WebSSOResource.java @@ -41,6 +41,7 @@ import org.apache.knox.gateway.util.SetCookieHeader; import org.apache.knox.gateway.util.Tokens; import org.apache.knox.gateway.util.Urls; import org.apache.knox.gateway.util.WhitelistUtils; +import org.apache.knox.gateway.util.knoxidf.FederatedNonceStore; import org.apache.knox.gateway.util.knoxidf.FederatedOpConfiguration; import org.apache.knox.gateway.util.knoxidf.FederatedOpConfigurationStore; import org.apache.knox.gateway.util.knoxidf.KnoxIDFUtils; @@ -70,6 +71,7 @@ import java.util.Map; import java.util.Map.Entry; import java.util.Optional; import java.util.Set; +import java.util.UUID; import static javax.ws.rs.core.MediaType.APPLICATION_JSON; import static javax.ws.rs.core.MediaType.APPLICATION_XML; @@ -120,6 +122,7 @@ public class WebSSOResource { private String tokenIssuer; private TokenStateService tokenStateService; private final FederatedOpConfigurationStore federatedOpConfigurationStore = FederatedOpConfigurationStore.getInstance(120000L); + private final FederatedNonceStore federatedNonceStore = FederatedNonceStore.getInstance(120000L); private String sameSiteValue; @@ -243,7 +246,12 @@ public class WebSSOResource { final FederatedOpConfiguration federatedOpConfiguration = federatedOpConfig.get(); //keep only the selected federated OP in the cache -> we can easily get it in the AuthorizeResource.authCallback endpoint federatedOpConfigurationStore.put(loginSessionId, Set.of(federatedOpConfiguration)); - final String federatedOpAuthRedirect = KnoxIDFUtils.buildFederatedOpAuthRedirect(federatedOpConfiguration, loginSessionId); + // Generate a per-request nonce, send it to the OP, and stash it keyed by the login-session id + // (== the state echoed back by the OP). AuthorizeResource.authCallback verifies the returned + // id_token's nonce claim against this value, binding the id_token to this authorization request. + final String nonce = UUID.randomUUID().toString(); + federatedNonceStore.put(loginSessionId, nonce); + final String federatedOpAuthRedirect = KnoxIDFUtils.buildFederatedOpAuthRedirect(federatedOpConfiguration, loginSessionId, nonce); return Response.seeOther(java.net.URI.create(federatedOpAuthRedirect)).build(); } else { return KnoxIDFUtils.error("invalid_request", "Cannot load federated op config associated with login session"); diff --git a/gateway-util-common/src/main/java/org/apache/knox/gateway/util/knoxidf/FederatedNonceStore.java b/gateway-util-common/src/main/java/org/apache/knox/gateway/util/knoxidf/FederatedNonceStore.java new file mode 100644 index 000000000..a3b30bf70 --- /dev/null +++ b/gateway-util-common/src/main/java/org/apache/knox/gateway/util/knoxidf/FederatedNonceStore.java @@ -0,0 +1,42 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with this + * work for additional information regarding copyright ownership. The ASF + * licenses this file to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * <p> + * http://www.apache.org/licenses/LICENSE-2.0 + * <p> + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations under + * the License. + */ +package org.apache.knox.gateway.util.knoxidf; + +/** + * Holds the OIDC {@code nonce} that Knox generates and sends to a federated OP, keyed by the + * federated login-session id (the {@code state} echoed by the OP). The value is written when the OP + * authorization redirect is built and read once when the OP callback is processed, so the returned + * id_token's {@code nonce} claim can be bound to this specific authorization request. Like the other + * KnoxIDF artifact stores this is a JVM singleton (the redirect is built in one topology/webapp and + * the callback handled in another, within the same JVM) and its entries are single-use: callers + * {@code remove()} the nonce after verifying it so a replayed callback cannot reuse it. + */ +public class FederatedNonceStore extends KnoxIDFArtifactStore<String> { + + private static FederatedNonceStore instance; + + private FederatedNonceStore(long ttl) { + super(ttl); + } + + public static synchronized FederatedNonceStore getInstance(long ttl) { + if (instance == null) { + instance = new FederatedNonceStore(ttl); + } + return instance; + } +} diff --git a/gateway-util-common/src/main/java/org/apache/knox/gateway/util/knoxidf/KnoxIDFUtils.java b/gateway-util-common/src/main/java/org/apache/knox/gateway/util/knoxidf/KnoxIDFUtils.java index 423ad37ad..f2f0e5ae3 100644 --- a/gateway-util-common/src/main/java/org/apache/knox/gateway/util/knoxidf/KnoxIDFUtils.java +++ b/gateway-util-common/src/main/java/org/apache/knox/gateway/util/knoxidf/KnoxIDFUtils.java @@ -102,17 +102,20 @@ public class KnoxIDFUtils { return new AuthorizeRequestMetadata(clientId, null, responseType, redirectUri, requestedScopes, state, nonce, codeChallenge, codeChallengeMethod); } - public static String buildFederatedOpAuthRedirect(final FederatedOpConfiguration federatedOpConfiguration, final String federatedState) { + public static String buildFederatedOpAuthRedirect(final FederatedOpConfiguration federatedOpConfiguration, final String federatedState, final String nonce) { // URL-encode every value placed into the query string. client_id and the callback URI - // (which itself contains ':' '/' '?' etc.) and the state must be percent-encoded or the - // OP receives a malformed/parameter-split URL. CODE_RESPONSE_TYPE and OPENID_SCOPE are + // (which itself contains ':' '/' '?' etc.), the state and the nonce must be percent-encoded + // or the OP receives a malformed/parameter-split URL. CODE_RESPONSE_TYPE and OPENID_SCOPE are // fixed "key=value" literals with no reserved characters, so they are appended as-is. + // The nonce binds the returned id_token to this authorization request (OIDC Core 3.1.2.1); + // it is verified against the id_token's nonce claim when the OP callback is processed. return federatedOpConfiguration.getAuthorizeEndpoint() + "?" + KnoxIDFConstants.CLIENT_ID + "=" + urlEncode(federatedOpConfiguration.getClientId()) + "&" + KnoxIDFConstants.REDIRECT_URI + "=" + urlEncode(federatedOpConfiguration.getAuthorizeCallback()) + "&" + KnoxIDFConstants.CODE_RESPONSE_TYPE + "&" + KnoxIDFConstants.OPENID_SCOPE - + "&" + KnoxIDFConstants.STATE + "=" + urlEncode(federatedState); + + "&" + KnoxIDFConstants.STATE + "=" + urlEncode(federatedState) + + "&" + KnoxIDFConstants.NONCE + "=" + urlEncode(nonce); } private static String urlEncode(final String value) {
