8000 Updating Secure Request Object to reject overflowing and malformed timestamps by PadzerButch · Pull Request #54907 · keycloak/keycloak · GitHub
[go: up one dir, main page]
Skip to content

Updating Secure Request Object to reject overflowing and malformed timestamps #54907

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
PadzerButch wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Updating Secure Request Object to reject overflowing and malformed timestamps #54907

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d52407a
Updating Secure Request Object to reject overflowing and malformed ti…
PadzerButch Nov 9, 2026
b16ee6f
Align zero-lifetime Request Object test with second boundary and exte…
PadzerButch Nov 10, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 30 additions & 7 deletions ...rc/main/java/org/keycloak/services/clientpolicy/executor/SecureRequestObjectExecutor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@ private void executeOnAuthorizationRequest(AuthorizationRequestContext context)
}

// check whether request object not expired
long exp = requestObject.get("exp").asLong();
long exp = getTimestampClaim(requestObject, "exp", context);
if (Time.currentTime() > exp) { // TODO: Time.currentTime() is int while exp is long...
logger.trace("request object expired.");
throw new ClientPolicyException(INVALID_REQUEST_OBJECT, "Request Expired");
Expand All @@ -207,25 +207,39 @@ private void executeOnAuthorizationRequest(AuthorizationRequestContext context)
}

// check whether request object not yet being processed
long nbf = requestObject.get("nbf").asLong();
if (Time.currentTime() < nbf - configuration.getAllowedClockSkew().intValue()) { // TODO: Time.currentTime() is int while nbf is long...
// "currentTime + skew" is computed instead of "nbf - skew" so that a crafted nbf cannot overflow
long nbf = getTimestampClaim(requestObject, "nbf", context);
if ((long) Time.currentTime() + configuration.getAllowedClockSkew().intValue() < nbf) { // TODO: Time.currentTime() is int while nbf is long...
logger.trace("request object not yet being processed.");
throwClientPolicyException(INVALID_REQUEST_OBJECT, "Request not yet being processed", context);
}

// check whether request object expires before nbf
if (exp < nbf) {
logger.trace("request object request expires before the lower bound nbf.");
throwClientPolicyException(INVALID_REQUEST_OBJECT, "Invalid parameter in the 'request' object: exp is before nbf", context);
}

// check whether request object's available period is short
int availablePeriod = Optional.ofNullable(configuration.getAvailablePeriod()).orElse(DEFAULT_AVAILABLE_PERIOD).intValue();
if (exp - nbf > availablePeriod) {
long lifetime;
try {
lifetime = Math.subtractExact(exp, nbf);
} catch (ArithmeticException e) {
//does not even fit into a long, so defo longer than the available period
lifetime = Long.MAX_VALUE;
}
if (lifetime > availablePeriod) {
logger.trace("request object's available period is long.");
throwClientPolicyException(INVALID_REQUEST_OBJECT, "Request's available period is long", context);
}
}

// check "iat" with clock skew
if (requestObject.get("iat") != null) {
long iat = requestObject.get("iat").asLong();
if (Time.currentTime() < iat - configuration.getAllowedClockSkew().intValue()) { // TODO: Time.currentTime() is int while nbf is long...
logger.trace("request object issed in the future.");
long iat = getTimestampClaim(requestObject, "iat", context);
if ((long) Time.currentTime() + configuration.getAllowedClockSkew().intValue() < iat) {// TODO: Time.currentTime() is int while nbf is long...
logger.trace("request object issued in the future.");
throwClientPolicyException(INVALID_REQUEST_OBJECT, "Request issued in the future", context);
}
}
Expand Down Expand Up @@ -281,6 +295,15 @@ private boolean isSameParameterIncluded(String param, String value, JsonNode req
return false;
}

private long getTimestampClaim(JsonNode requestObject, String claim, AuthorizationRequestContext context) throws ClientPolicyException {
JsonNode value = requestObject.get(claim);
if (!value.isIntegralNumber() || !value.canConvertToLong()) {
logger.tracef("request object claim '%s' is not a valid timestamp.", claim);
throwClientPolicyException(INVALID_REQUEST_OBJECT, "Invalid parameter in the 'request' object: " + claim, context);
}
return value.asLong();
}

private void throwClientPolicyException(String error, String message,
AuthorizationRequestContext context) throws ClientPolicyException {
if (context.isParRequest() && INVALID_REQUEST_OBJECT.equals(error)) {
Expand Down
225 changes: 225 additions & 0 deletions ...ase/src/test/java/org/keycloak/tests/client/policies/SecureRequestObjectExecutorTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,225 @@
/*
* Copyright 2026 Red Hat, Inc. and/or its affiliates
* and other contributors as indicated by the @author tags.
*
* Licensed 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
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* 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.keycloak.tests.client.policies;

import java.security.KeyPair;

import org.keycloak.OAuth2Constants;
import org.keycloak.OAuthErrorException;
import org.keycloak.common.util.KeyUtils;
import org.keycloak.common.util.Time;
import org.keycloak.crypto.Algorithm;
import org.keycloak.crypto.ECDSASignatureSignerContext;
import org.keycloak.crypto.KeyUse;
import org.keycloak.crypto.KeyWrapper;
import org.keycloak.jose.jwk.JSONWebKeySet;
import org.keycloak.jose.jwk.JWK;
import org.keycloak.jose.jwk.JWKBuilder;
import org.keycloak.jose.jws.JWSBuilder;
import org.keycloak.models.utils.KeycloakModelUtils;
import org.keycloak.protocol.oidc.OIDCAdvancedConfigWrapper;
import org.keycloak.protocol.oidc.OIDCLoginProtocol;
import org.keycloak.representations.idm.ClientRepresentation;
import org.keycloak.services.clientpolicy.condition.AnyClientConditionFactory;
import org.keycloak.services.clientpolicy.executor.SecureRequestObjectExecutorFactory;
import org.keycloak.testframework.annotations.InjectRealm;
import org.keycloak.testframework.annotations.KeycloakIntegrationTest;
import org.keycloak.testframework.oauth.OAuthClient;
import org.keycloak.testframework.oauth.annotations.InjectOAuthClient;
import org.keycloak.testframework.realm.ManagedRealm;
import org.keycloak.tests.common.BasicRealmWithUserConfig;
import org.keycloak.testsuite.util.oauth.AuthorizationEndpointResponse;
import org.keycloak.util.JsonSerialization;

import com.fasterxml.jackson.databind.node.ObjectNode;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.CsvSource;
import org.junit.jupiter.params.provider.ValueSource;

import static org.keycloak.tests.utils.ClientPoliciesUtil.createAnyClientConditionConfig;
import static org.keycloak.tests.utils.ClientPoliciesUtil.createSecureRequestObjectExecutorConfig;

import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNotNull;

/**
* Exercises signed request objects through the authorization endpoint, including timestamp validation errors.
*/
@KeycloakIntegrationTest
public class SecureRequestObjectExecutorTest extends AbstractClientPoliciesTest {

private static final String SCOPE = "openid";
private static final String RESPONSE_TYPE = "code";

@InjectRealm(config = BasicRealmWithUserConfig.class)
ManagedRealm realm;

@InjectOAuthClient
OAuthClient oauth;

private KeyWrapper signingKey;


@Test
public void validEs256RequestObjectIsAccepted() throws Exception {
setupClientAndPolicy();

oauth.loginForm().request(sign(validRequestObject())).open();
oauth.fillLoginForm(BasicRealmWithUserConfig.USERNAME, BasicRealmWithUserConfig.PASSWORD);
assertNotNull(oauth.parseLoginResponse().getCode());
}

@ParameterizedTest
@ValueSource(strings = {"exp", "nbf"})
public void missingRequiredTimestampIsRejected(String claim) throws Exception {
setupClientAndPolicy();

ObjectNode requestObject = validRequestObject();
requestObject.remove(claim);
assertErrorForRequest(requestObject, "Missing parameter in the 'request' object: " + claim);
}

@Test
public void incorrectAudienceIsRejected() throws Exception {
setupClientAndPolicy();

ObjectNode requestObject = validRequestObject();
requestObject.put("aud", oauth.getEndpoints().getIssuer() + "-other");
assertErrorForRequest(requestObject, "Invalid parameter in the 'request' object: aud");
}

@Test
public void expiredRequestObjectIsRejected() throws Exception {
setupClientAndPolicy();

ObjectNode requestObject = validRequestObject();
requestObject.put("exp", 0L);
assertErrorForRequest(requestObject, "Request Expired");
}

@Test
public void futureNbfIsRejected() throws Exception {
setupClientAndPolicy();

ObjectNode requestObject = validRequestObject();
requestObject.put("nbf", (long) Time.currentTime() + 600);
assertErrorForRequest(requestObject, "Request not yet being processed");
}

@ParameterizedTest
@ValueSource(longs = {Long.MIN_VALUE, Long.MIN_VALUE + 15})
public void overflowingLifetimeIsRejected(long nbf) throws Exception {
setupClientAndPolicy();

ObjectNode requestObject = validRequestObject();
requestObject.put("nbf", nbf);
assertErrorForRequest(requestObject, "Request's available period is long");
}

@Test
public void expirationBeforeNbfIsRejected() throws Exception {
setupClientAndPolicy();

ObjectNode requestObject = validRequestObject();
long now = Time.currentTime();
requestObject.put("nbf", now + 10);
requestObject.put("exp", now + 5);
assertErrorForRequest(requestObject, "Invalid parameter in the 'request' object: exp is before nbf");
}

@ParameterizedTest
@CsvSource({
"nbf, 9223372036854775817",
"nbf, -9223372036854775818",
"nbf, 1.5",
"nbf, \"123\"",
"exp, 9223372036854775817",
"exp, -9223372036854775818",
"exp, 1.5",
"exp, \"123\"",
"iat, 9223372036854775817",
"iat, -9223372036854775818",
"iat, 1.5",
"iat, \"123\""
})
public void malformedTimestampIsRejected(String claim, String jsonValue) throws Exception {
setupClientAndPolicy();

ObjectNode requestObject = validRequestObject();
requestObject.set(claim, JsonSerialization.mapper.readTree(jsonValue));
assertErrorForRequest(requestObject, "Invalid parameter in the 'request' object: " + claim);
D2DB }

@Test
public void overflowingFutureIatIsRejected() throws Exception {
setupClientAndPolicy();

ObjectNode requestObject = validRequestObject();
requestObject.put("iat", Long.MAX_VALUE);
assertErrorForRequest(requestObject, "Request issued in the future");
}

private void setupClientAndPolicy() throws Exception {
KeyPair keyPair = KeyUtils.generateEcKeyPair("secp256r1");
signingKey = new KeyWrapper();
signingKey.setAlgorithm(Algorithm.ES256);
signingKey.setPrivateKey(keyPair.getPrivate());

JSONWebKeySet jwks = new JSONWebKeySet();
jwks.setKeys(new JWK[] {
JWKBuilder.create().algorithm(Algorithm.ES256).kid(KeycloakModelUtils.generateId()).ec(keyPair.getPublic(), KeyUse.SIG)
});

ClientRepresentation clientRep = oauth.clientResource().toRepresentation();
OIDCAdvancedConfigWrapper.fromClientRepresentation(clientRep).setRequestObjectSignatureAlg(Algorithm.ES256);
OIDCAdvancedConfigWrapper.fromClientRepresentation(clientRep).setUseJwksString(true);
OIDCAdvancedConfigWrapper.fromClientRepresentation(clientRep).setJwksString(JsonSerialization.writeValueAsString(jwks));
oauth.clientResource().update(clientRep);

setupPolicy(realm, SecureRequestObjectExecutorFactory.PROVIDER_ID, createSecureRequestObjectExecutorConfig(60, true),
AnyClientConditionFactory.PROVIDER_ID, createAnyClientConditionConfig());
}

private ObjectNode validRequestObject() {
long now = Time.currentTime();
ObjectNode token = JsonSerialization.createObjectNode();
token.put("jti", KeycloakModelUtils.generateId());
token.put("iat", now);
token.put("nbf", now);
token.put("exp", now + 30);
token.put("iss", oauth.getClientId());
token.put("aud", oauth.getEndpoints().getIssuer());
token.put(OIDCLoginProtocol.CLIENT_ID_PARAM, oauth.getClientId());
token.put(OIDCLoginProtocol.RESPONSE_TYPE_PARAM, RESPONSE_TYPE);
token.put(OIDCLoginProtocol.REDIRECT_URI_PARAM, oauth.getRedirectUri());
token.put(OAuth2Constants.SCOPE, SCOPE);
token.put(OIDCLoginProtocol.NONCE_PARAM, KeycloakModelUtils.generateId());
return token;
}

private String sign(ObjectNode token) {
return new JWSBuilder().jsonContent(token).sign(new ECDSASignatureSignerContext(signingKey));
}

private void assertErrorForRequest(ObjectNode requestObject, String expectedDescription) {
oauth.loginForm().request(sign(requestObject)).open();
AuthorizationEndpointResponse response = oauth.parseLoginResponse();
assertEquals(OAuthErrorException.INVALID_REQUEST_OBJECT, response.getError());
assertEquals(expectedDescription, response.getErrorDescription());
}
}
Loading
0