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
61fda8c
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
Loading
Loading
0