From 2b4a1899a3ef30b13a7ee631f4f5da18d69cd908 Mon Sep 17 00:00:00 2001 From: Valera V Harseko Date: Tue, 15 Sep 2026 08:03:06 +0300 Subject: [PATCH 1/4] [#1130] Verify client assertions by their own alg; default id_token_signed_response_alg OpenAMClientRegistration.verifyJwtIdentity chose HMAC vs. asymmetric verification from the client's id_token_signed_response_alg - the algorithm of the ID tokens *we* issue - so a private_key_jwt client whose ID-token algorithm was HS256 had its RS256 assertion pushed through the shared-secret verifier ("Unsupported Signing Algorithm, SHA256withRSA"). Dispatch on the JWS header of the presented JWT instead: HMAC uses the client secret only, anything else the client's registered public keys only, and "none" is refused. OpenAM-issued ID tokens (idtokeninfo, the OIDC SSO provider) are unaffected since their header matches the configured algorithm. getIDTokenSignedResponseAlgorithm() returned null when the attribute was never persisted (AgentsRepo reads without schema defaults, e.g. a client created via the realm-config REST endpoint or ssoadm), which NPE'd at the token endpoint for any openid request. Fall back to HS256, the default the schema, the console and dynamic registration already use. Fixes #1130 --- .../oauth2/OpenAMClientRegistration.java | 30 ++++- .../oauth2/OpenAMClientRegistrationTest.java | 126 ++++++++++++++++++ 2 files changed, 150 insertions(+), 6 deletions(-) diff --git a/openam-oauth2/src/main/java/org/forgerock/openam/oauth2/OpenAMClientRegistration.java b/openam-oauth2/src/main/java/org/forgerock/openam/oauth2/OpenAMClientRegistration.java index 846f06a26c..6944edd703 100644 --- a/openam-oauth2/src/main/java/org/forgerock/openam/oauth2/OpenAMClientRegistration.java +++ b/openam-oauth2/src/main/java/org/forgerock/openam/oauth2/OpenAMClientRegistration.java @@ -94,6 +94,9 @@ */ public class OpenAMClientRegistration implements OpenIdConnectClientRegistration { + /** Default of com.forgerock.openam.oauth2provider.idTokenSignedResponseAlg in AgentService.xml. */ + private static final String ID_TOKEN_SIGNED_RESPONSE_ALG_DEFAULT = "HS256"; + private static final String DELIMITER = "\\|"; /** Read/connect timeouts (ms) used when fetching a client's {@code jwks_uri}. @@ -512,10 +515,11 @@ public String getIDTokenSignedResponseAlgorithm() { } catch (Exception e) { throw Utils.createException(OAuth2Constants.OAuth2Client.IDTOKEN_SIGNED_RESPONSE_ALG, e, logger); } - if (set.iterator().hasNext()){ - return set.iterator().next(); - } - return null; + // AgentsRepo reads agent attributes without schema defaults, so a client created through + // the realm-config REST endpoint or ssoadm may have nothing persisted here. Fall back to + // the same default the schema, the console and dynamic registration use. + final String algorithm = CollectionUtils.getFirstItem(set); + return StringUtils.isEmpty(algorithm) ? ID_TOKEN_SIGNED_RESPONSE_ALG_DEFAULT : algorithm; } @Override @@ -652,7 +656,16 @@ public String getSubjectType() { @Override public boolean verifyJwtIdentity(final OAuth2Jwt jwt) { - final JwsAlgorithm signatureAlgorithm = JwsAlgorithm.valueOf(getIDTokenSignedResponseAlgorithm()); + // The JWT is either a client assertion (client_secret_jwt, private_key_jwt, jwt-bearer) + // signed by the client, or an ID token this server issued. In both cases the JWS header + // states how it was signed; id_token_signed_response_alg only describes the ID tokens we + // issue and says nothing about how the client signs its assertions. HMAC verification + // uses the client secret only and the asymmetric branch uses the client's public keys + // only, so the header cannot steer one key type into the other's verifier. + final JwsAlgorithm signatureAlgorithm = jwt.getSignedJwt().getHeader().getAlgorithm(); + if (signatureAlgorithm == null || signatureAlgorithm.getAlgorithmType() == JwsAlgorithmType.NONE) { + return false; + } if (signatureAlgorithm.getAlgorithmType() == JwsAlgorithmType.HMAC) { return verifyJwtBySharedSecret(jwt); } else { @@ -677,8 +690,13 @@ public boolean isConsentImplied() { } private boolean verifyJwtBySharedSecret(final OAuth2Jwt jwt) { + final String clientSecret = getClientSecret(); + if (StringUtils.isEmpty(clientSecret)) { + // A public client has no secret to verify an HMAC assertion with. + return false; + } final String issuer = jwt.getSignedJwt().getClaimsSet().getIssuer(); - OpenIdResolver resolver = new SharedSecretOpenIdResolverImpl(issuer, getClientSecret()); + OpenIdResolver resolver = new SharedSecretOpenIdResolverImpl(issuer, clientSecret); try { resolver.validateIdentity(jwt.getSignedJwt()); return jwt.isContentValid() && jwt.isIntendedForAudience(getClientId()); diff --git a/openam-oauth2/src/test/java/org/forgerock/openam/oauth2/OpenAMClientRegistrationTest.java b/openam-oauth2/src/test/java/org/forgerock/openam/oauth2/OpenAMClientRegistrationTest.java index 66354d4f9e..075b002f8b 100644 --- a/openam-oauth2/src/test/java/org/forgerock/openam/oauth2/OpenAMClientRegistrationTest.java +++ b/openam-oauth2/src/test/java/org/forgerock/openam/oauth2/OpenAMClientRegistrationTest.java @@ -13,6 +13,7 @@ * * Copyright 2014-2016 ForgeRock AS. * Portions Copyrighted 2015 Nomura Research Institute, Ltd. + * Portions Copyrighted 2026 3A Systems, LLC. */ package org.forgerock.openam.oauth2; @@ -30,21 +31,34 @@ import java.net.URI; import java.nio.charset.StandardCharsets; import java.security.Key; +import java.security.KeyPair; import java.security.KeyPairGenerator; import java.security.MessageDigest; import java.security.PublicKey; +import java.security.interfaces.RSAPrivateKey; +import java.security.interfaces.RSAPublicKey; import java.util.Arrays; import java.util.Collections; +import java.util.Date; import java.util.HashSet; import java.util.List; import java.util.Locale; import java.util.Map; import java.util.Set; +import java.util.UUID; import javax.crypto.spec.SecretKeySpec; import org.forgerock.jaspi.modules.openid.resolvers.service.OpenIdResolverService; +import org.forgerock.json.jose.builders.JwsHeaderBuilder; +import org.forgerock.json.jose.builders.JwtBuilderFactory; import org.forgerock.json.jose.jwe.JweAlgorithm; +import org.forgerock.json.jose.jws.JwsAlgorithm; +import org.forgerock.json.jose.jws.SigningManager; +import org.forgerock.json.jose.jws.handlers.NOPSigningHandler; +import org.forgerock.json.jose.jws.handlers.SigningHandler; +import org.forgerock.json.jose.jwt.JwtClaimsSet; +import org.forgerock.oauth2.core.OAuth2Jwt; import org.forgerock.oauth2.core.OAuth2ProviderSettings; import org.forgerock.oauth2.core.PEMDecoder; import org.forgerock.oauth2.core.exceptions.ClientAuthenticationFailureFactory; @@ -328,4 +342,116 @@ private void setUpAgentToThrowExceptionForAttribute(String attributeName) throws given(amIdentity.getAttribute(attributeName)) .willThrow(new SSOException("exception!")); } + + // --- #1130: id_token_signed_response_alg default and client-assertion dispatch ---------- + + /** + * AgentsRepo reads agent attributes without schema defaults, so a client created without + * the attribute (realm-config PUT, ssoadm) has no value persisted; the token endpoint then + * NPEs. The schema, the console and dynamic registration all default to HS256. + */ + @Test + public void idTokenSignedResponseAlgorithmDefaultsToHs256WhenUnset() throws Exception { + given(amIdentity.getAttribute(IDTOKEN_SIGNED_RESPONSE_ALG)).willReturn(Collections.emptySet()); + assertThat(clientRegistration.getIDTokenSignedResponseAlgorithm()).isEqualTo("HS256"); + + given(amIdentity.getAttribute(IDTOKEN_SIGNED_RESPONSE_ALG)).willReturn(null); + assertThat(clientRegistration.getIDTokenSignedResponseAlgorithm()).isEqualTo("HS256"); + + given(amIdentity.getAttribute(IDTOKEN_SIGNED_RESPONSE_ALG)).willReturn(singleton("RS256")); + assertThat(clientRegistration.getIDTokenSignedResponseAlgorithm()).isEqualTo("RS256"); + } + + /** + * A client_secret_jwt assertion is verified with the client secret whatever algorithm the + * client asked for its (outgoing) ID tokens. + */ + @Test + public void verifyJwtIdentityUsesClientSecretForHmacAssertionRegardlessOfIdTokenAlg() throws Exception { + String clientId = "client1"; + String secret = "a-client-secret-of-sufficient-length"; + given(amIdentity.getName()).willReturn(clientId); + given(amIdentity.getAttribute("userpassword")).willReturn(singleton(secret)); + given(amIdentity.getAttribute(IDTOKEN_SIGNED_RESPONSE_ALG)).willReturn(singleton("RS256")); + + SigningHandler signer = new SigningManager().newHmacSigningHandler(secret.getBytes(StandardCharsets.UTF_8)); + OAuth2Jwt assertion = assertion(clientId, signer, JwsAlgorithm.HS256, null); + + assertThat(clientRegistration.verifyJwtIdentity(assertion)).isTrue(); + + SigningHandler wrongSigner = new SigningManager().newHmacSigningHandler("wrong".getBytes(StandardCharsets.UTF_8)); + assertThat(clientRegistration.verifyJwtIdentity(assertion(clientId, wrongSigner, JwsAlgorithm.HS256, null))) + .isFalse(); + } + + /** + * A private_key_jwt assertion is verified with the client's registered public keys whatever + * algorithm the client asked for its (outgoing) ID tokens. + */ + @Test + public void verifyJwtIdentityUsesPublicKeysForAsymmetricAssertionRegardlessOfIdTokenAlg() throws Exception { + String clientId = "client1"; + KeyPair clientKeys = generateRsaKeyPair(); + String kid = UUID.randomUUID().toString(); + given(amIdentity.getName()).willReturn(clientId); + given(amIdentity.getAttribute("userpassword")).willReturn(singleton("unused-secret")); + given(amIdentity.getAttribute(IDTOKEN_SIGNED_RESPONSE_ALG)).willReturn(singleton("HS256")); + given(amIdentity.getAttribute(PUBLIC_KEY_SELECTOR)).willReturn(singleton("jwks")); + given(amIdentity.getAttribute(JWKS)).willReturn(singleton(jwks((RSAPublicKey) clientKeys.getPublic(), kid))); + + SigningHandler signer = new SigningManager().newRsaSigningHandler((RSAPrivateKey) clientKeys.getPrivate()); + assertThat(clientRegistration.verifyJwtIdentity(assertion(clientId, signer, JwsAlgorithm.RS256, kid))).isTrue(); + + KeyPair otherKeys = generateRsaKeyPair(); + SigningHandler otherSigner = new SigningManager().newRsaSigningHandler((RSAPrivateKey) otherKeys.getPrivate()); + assertThat(clientRegistration.verifyJwtIdentity(assertion(clientId, otherSigner, JwsAlgorithm.RS256, kid))) + .isFalse(); + } + + @Test + public void verifyJwtIdentityRejectsUnsignedAssertion() throws Exception { + String clientId = "client1"; + given(amIdentity.getName()).willReturn(clientId); + given(amIdentity.getAttribute("userpassword")).willReturn(singleton("a-client-secret")); + given(amIdentity.getAttribute(IDTOKEN_SIGNED_RESPONSE_ALG)).willReturn(singleton("HS256")); + + OAuth2Jwt unsigned = assertion(clientId, new NOPSigningHandler(), JwsAlgorithm.NONE, null); + + assertThat(clientRegistration.verifyJwtIdentity(unsigned)).isFalse(); + } + + private static OAuth2Jwt assertion(String clientId, SigningHandler signer, JwsAlgorithm alg, String kid) { + JwtClaimsSet claims = new JwtBuilderFactory().claims() + .iss(clientId) + .sub(clientId) + .aud(Collections.singletonList(clientId)) + .exp(new Date(System.currentTimeMillis() + 60_000L)) + .iat(new Date()) + .build(); + JwsHeaderBuilder headers = new JwtBuilderFactory().jws(signer).headers().alg(alg); + if (kid != null) { + headers = headers.kid(kid); + } + return OAuth2Jwt.create(headers.done().claims(claims).build()); + } + + private static KeyPair generateRsaKeyPair() throws Exception { + KeyPairGenerator gen = KeyPairGenerator.getInstance("RSA"); + gen.initialize(2048); + return gen.generateKeyPair(); + } + + private static String jwks(RSAPublicKey pk, String kid) { + return "{\"keys\":[{\"kty\":\"RSA\",\"use\":\"sig\",\"alg\":\"RS256\",\"kid\":\"" + kid + "\"," + + "\"n\":\"" + base64UrlUnsigned(pk.getModulus()) + "\"," + + "\"e\":\"" + base64UrlUnsigned(pk.getPublicExponent()) + "\"}]}"; + } + + private static String base64UrlUnsigned(java.math.BigInteger bi) { + byte[] full = bi.toByteArray(); + if (full.length > 1 && full[0] == 0) { + full = Arrays.copyOfRange(full, 1, full.length); + } + return java.util.Base64.getUrlEncoder().withoutPadding().encodeToString(full); + } } \ No newline at end of file From 3dd4b8044e4ac99986f35ca08f2480a259de712b Mon Sep 17 00:00:00 2001 From: Valera V Harseko Date: Thu, 17 Sep 2026 07:10:24 +0300 Subject: [PATCH 2/4] [#1130] Reject unknown alg, missing secret and key location cleanly; verify ID tokens by their configured alg Review round 2 on #1131. - OAuth2Jwt.getSigningAlgorithm(): null for an alg outside the JwsAlgorithm enum, including the wire spelling "none" (JwsHeader.getAlgorithm() is JwsAlgorithm.valueOf(alg), so "none"/"hs256" threw IllegalArgumentException instead of reaching the NONE guard; the builder-emitted "NONE" the test pinned never occurs on the wire). - Utils.getAttributeValueFromSet(): null for an absent or empty attribute, so a client without userpassword gets false from the HMAC branch (and past StatefulTokenStore:278) instead of NPE/NoSuchElementException. - getClientPublicKeySelector(): null when publicKeyLocation was never persisted or holds an unknown value; verifyJwtIdentity() returns false instead of a 500 + stack trace, since any client_id can now be sent an RS256 assertion. - byJWKs(): an oct JWK is never tried against an RS/ES-signed assertion. - New OpenIdConnectClientRegistration.verifyIdTokenIdentity(): an ID token we issued must carry id_token_signed_response_alg in its header; IdTokenInfo and OpenIdConnectSSOProvider use it, so the client-auth gate and the verifier reason about the same algorithm. Tests: wire "none" and "hs256" assertions; secret null/empty; key location null/empty/unknown; oct JWK + RS256; verifyIdTokenIdentity match/mismatch/none; SSO provider rejects a token verified only as a client assertion; OAuth2JwtTest. --- .../org/forgerock/oauth2/core/OAuth2Jwt.java | 17 +++ .../oauth2/AgentClientRegistration.java | 6 + .../oauth2/OpenAMClientRegistration.java | 37 +++-- .../org/forgerock/openam/oauth2/Utils.java | 6 +- .../OpenIdConnectClientRegistration.java | 13 ++ .../openidconnect/restlet/IdTokenInfo.java | 4 +- .../ssoprovider/OpenIdConnectSSOProvider.java | 4 +- .../forgerock/oauth2/core/OAuth2JwtTest.java | 59 ++++++++ .../oauth2/OpenAMClientRegistrationTest.java | 128 +++++++++++++++++- .../OpenIdConnectSSOProviderTest.java | 39 +++++- 10 files changed, 286 insertions(+), 27 deletions(-) create mode 100644 openam-oauth2/src/test/java/org/forgerock/oauth2/core/OAuth2JwtTest.java diff --git a/openam-oauth2/src/main/java/org/forgerock/oauth2/core/OAuth2Jwt.java b/openam-oauth2/src/main/java/org/forgerock/oauth2/core/OAuth2Jwt.java index 69dcda0a6a..b6d618de15 100644 --- a/openam-oauth2/src/main/java/org/forgerock/oauth2/core/OAuth2Jwt.java +++ b/openam-oauth2/src/main/java/org/forgerock/oauth2/core/OAuth2Jwt.java @@ -13,12 +13,14 @@ * * Copyright 2014-2016 ForgeRock AS. * Portions Copyrighted 2015 Nomura Research Institute, Ltd. + * Portions Copyrighted 2026 3A Systems, LLC. */ package org.forgerock.oauth2.core; import com.google.common.annotations.VisibleForTesting; import org.forgerock.json.jose.common.JwtReconstruction; +import org.forgerock.json.jose.jws.JwsAlgorithm; import org.forgerock.json.jose.jws.SignedJwt; import org.forgerock.json.jose.jws.handlers.SigningHandler; import org.forgerock.util.time.TimeService; @@ -157,4 +159,19 @@ public String getSubject() { public SignedJwt getSignedJwt() { return jwt; } + + /** + * Gets the signing algorithm named in the JWS header. + * + * @return The signing algorithm, or {@code null} if the header names one this server does not + * know, which includes the RFC 7518 spelling {@code "none"}: {@code JwsHeader.getAlgorithm()} + * is {@code JwsAlgorithm.valueOf(alg)} and only maps the enum's own upper-case names. + */ + public JwsAlgorithm getSigningAlgorithm() { + try { + return jwt.getHeader().getAlgorithm(); + } catch (IllegalArgumentException e) { + return null; + } + } } diff --git a/openam-oauth2/src/main/java/org/forgerock/openam/oauth2/AgentClientRegistration.java b/openam-oauth2/src/main/java/org/forgerock/openam/oauth2/AgentClientRegistration.java index a7bdf9d395..14ae9520bd 100644 --- a/openam-oauth2/src/main/java/org/forgerock/openam/oauth2/AgentClientRegistration.java +++ b/openam-oauth2/src/main/java/org/forgerock/openam/oauth2/AgentClientRegistration.java @@ -12,6 +12,7 @@ * information: "Portions copyright [year] [name of copyright owner]". * * Copyright 2016 ForgeRock AS. + * Portions Copyrighted 2026 3A Systems, LLC. */ package org.forgerock.openam.oauth2; @@ -146,6 +147,11 @@ public boolean verifyJwtIdentity(OAuth2Jwt jwt) { return false; } + @Override + public boolean verifyIdTokenIdentity(OAuth2Jwt idToken) { + return false; + } + @Override public String getIDTokenSignedResponseAlgorithm() { return "HS256"; diff --git a/openam-oauth2/src/main/java/org/forgerock/openam/oauth2/OpenAMClientRegistration.java b/openam-oauth2/src/main/java/org/forgerock/openam/oauth2/OpenAMClientRegistration.java index 6944edd703..a049fbd03e 100644 --- a/openam-oauth2/src/main/java/org/forgerock/openam/oauth2/OpenAMClientRegistration.java +++ b/openam-oauth2/src/main/java/org/forgerock/openam/oauth2/OpenAMClientRegistration.java @@ -656,21 +656,26 @@ public String getSubjectType() { @Override public boolean verifyJwtIdentity(final OAuth2Jwt jwt) { - // The JWT is either a client assertion (client_secret_jwt, private_key_jwt, jwt-bearer) - // signed by the client, or an ID token this server issued. In both cases the JWS header - // states how it was signed; id_token_signed_response_alg only describes the ID tokens we - // issue and says nothing about how the client signs its assertions. HMAC verification - // uses the client secret only and the asymmetric branch uses the client's public keys - // only, so the header cannot steer one key type into the other's verifier. - final JwsAlgorithm signatureAlgorithm = jwt.getSignedJwt().getHeader().getAlgorithm(); + // The client signed this JWT (client_secret_jwt, private_key_jwt, jwt-bearer), so the JWS + // header states how; id_token_signed_response_alg only describes the ID tokens we issue. + // HMAC verification uses the client secret only and the asymmetric branch the client's + // registered keys only, so the header cannot steer one key type into the other's verifier. + // An algorithm outside the JwsAlgorithm enum (including the wire spelling "none") is null. + final JwsAlgorithm signatureAlgorithm = jwt.getSigningAlgorithm(); if (signatureAlgorithm == null || signatureAlgorithm.getAlgorithmType() == JwsAlgorithmType.NONE) { return false; } if (signatureAlgorithm.getAlgorithmType() == JwsAlgorithmType.HMAC) { return verifyJwtBySharedSecret(jwt); } else { + final Client.PublicKeySelector selector = getClientPublicKeySelector(); + if (selector == null) { + // Nothing registered to verify an asymmetric assertion with: invalid_client, not + // server_error, now that any client_id can be sent an assertion with this alg. + return false; + } try { - switch (getClientPublicKeySelector()) { + switch (selector) { case JWKS: return byJWKs(jwt); case JWKS_URI: @@ -684,6 +689,14 @@ public boolean verifyJwtIdentity(final OAuth2Jwt jwt) { } } + @Override + public boolean verifyIdTokenIdentity(final OAuth2Jwt idToken) { + // We issued the ID token with id_token_signed_response_alg, so the header must say so; + // the algorithm, and hence the key the token is verified with, is not the presenter's to pick. + final JwsAlgorithm configured = JwsAlgorithm.valueOf(getIDTokenSignedResponseAlgorithm()); + return idToken.getSigningAlgorithm() == configured && verifyJwtIdentity(idToken); + } + @Override public boolean isConsentImplied() { return Boolean.parseBoolean(getAttribute(OAuth2Constants.OAuth2Client.IS_CONSENT_IMPLIED)); @@ -723,7 +736,9 @@ private boolean byJWKs(OAuth2Jwt jwt) throws IdRepoException, SSOException, final Key key = jwkMap.get(jwt.getSignedJwt().getHeader().getKeyId()); - return key != null && jwt.isValid(getSigningHandlerForKey(key)); + // Only asymmetric algorithms reach this branch; an oct JWK would be handed to an HMAC + // handler, which cannot verify them and would surface as a server error. + return key != null && !(key instanceof SecretKey) && jwt.isValid(getSigningHandlerForKey(key)); } /** @@ -832,7 +847,9 @@ private Client.PublicKeySelector getClientPublicKeySelector() { } catch (IdRepoException | SSOException e) { throw Utils.createException(OAuth2Constants.OAuth2Client.PUBLIC_KEY_SELECTOR, e, logger); } - return Client.PublicKeySelector.fromString(set.iterator().next()); + // As with idTokenSignedResponseAlg, the schema default is not applied to a client created + // through the realm-config REST endpoint or ssoadm; fromString() is null for an unknown value. + return Client.PublicKeySelector.fromString(CollectionUtils.getFirstItem(set)); } /** diff --git a/openam-oauth2/src/main/java/org/forgerock/openam/oauth2/Utils.java b/openam-oauth2/src/main/java/org/forgerock/openam/oauth2/Utils.java index 8f97278d4a..303e51186a 100644 --- a/openam-oauth2/src/main/java/org/forgerock/openam/oauth2/Utils.java +++ b/openam-oauth2/src/main/java/org/forgerock/openam/oauth2/Utils.java @@ -12,6 +12,7 @@ * information: "Portions copyright [year] [name of copyright owner]". * * Copyright 2016 ForgeRock AS. + * Portions Copyrighted 2026 3A Systems, LLC. */ package org.forgerock.openam.oauth2; @@ -19,6 +20,7 @@ import java.util.HashSet; import java.util.Set; +import org.forgerock.openam.utils.CollectionUtils; import org.restlet.Request; import com.sun.identity.idm.AMIdentity; @@ -103,7 +105,7 @@ static Set stripAttributeNameFromValue(Set attributeValues) { * @param identity The identity with the attribute. * @param attributeName The name of the attribute. * @param logger The logger used to log eventual exceptions. - * @return The attribute value. + * @return The attribute value, or {@code null} if the identity has no value for the attribute. */ static String getAttributeValueFromSet(AMIdentity identity, String attributeName, Debug logger) { Set values; @@ -112,7 +114,7 @@ static String getAttributeValueFromSet(AMIdentity identity, String attributeName } catch (Exception e) { throw createException(attributeName, e, logger); } - return values.iterator().next(); + return CollectionUtils.getFirstItem(values); } /** diff --git a/openam-oauth2/src/main/java/org/forgerock/openidconnect/OpenIdConnectClientRegistration.java b/openam-oauth2/src/main/java/org/forgerock/openidconnect/OpenIdConnectClientRegistration.java index 1ac25a8451..158bf926e0 100644 --- a/openam-oauth2/src/main/java/org/forgerock/openidconnect/OpenIdConnectClientRegistration.java +++ b/openam-oauth2/src/main/java/org/forgerock/openidconnect/OpenIdConnectClientRegistration.java @@ -13,6 +13,7 @@ * * Copyright 2014-2016 ForgeRock AS. * Portions Copyrighted 2015 Nomura Research Institute, Ltd. + * Portions Copyrighted 2026 3A Systems, LLC. */ package org.forgerock.openidconnect; @@ -21,6 +22,7 @@ import java.security.Key; import org.forgerock.oauth2.core.ClientRegistration; +import org.forgerock.oauth2.core.OAuth2Jwt; import org.forgerock.oauth2.core.OAuth2ProviderSettings; import org.forgerock.oauth2.core.exceptions.ServerException; @@ -38,6 +40,17 @@ public interface OpenIdConnectClientRegistration extends ClientRegistration { */ String getIDTokenSignedResponseAlgorithm(); + /** + * Verifies that the supplied JWT is an ID token this server issued for this client: signed with + * the client's {@link #getIDTokenSignedResponseAlgorithm() id_token_signed_response_alg} and + * verifiable with the key that algorithm implies. Unlike {@link #verifyJwtIdentity(OAuth2Jwt)}, + * which checks a JWT the client signed, the algorithm is not the sender's to choose. + * + * @param idToken The ID token. + * @return {@code true} if the ID token was issued for this client. + */ + boolean verifyIdTokenIdentity(OAuth2Jwt idToken); + /** * Determines if ID token encryption is enabled. * diff --git a/openam-oauth2/src/main/java/org/forgerock/openidconnect/restlet/IdTokenInfo.java b/openam-oauth2/src/main/java/org/forgerock/openidconnect/restlet/IdTokenInfo.java index 010584d0d8..5efa03387c 100644 --- a/openam-oauth2/src/main/java/org/forgerock/openidconnect/restlet/IdTokenInfo.java +++ b/openam-oauth2/src/main/java/org/forgerock/openidconnect/restlet/IdTokenInfo.java @@ -12,7 +12,7 @@ * information: "Portions copyright [year] [name of copyright owner]". * * Copyright 2016 ForgeRock AS. - * Portions copyright 2025 3A Systems LLC. + * Portions copyright 2025-2026 3A Systems LLC. */ package org.forgerock.openidconnect.restlet; @@ -171,7 +171,7 @@ OAuth2Jwt validateIdToken(OAuth2Request request) throws OAuth2Exception, RealmLo throw new BadRequestException("id_token has expired"); } - if (!clientRegistration.verifyJwtIdentity(idToken)) { + if (!clientRegistration.verifyIdTokenIdentity(idToken)) { throw new BadRequestException("invalid id_token"); } diff --git a/openam-oauth2/src/main/java/org/forgerock/openidconnect/ssoprovider/OpenIdConnectSSOProvider.java b/openam-oauth2/src/main/java/org/forgerock/openidconnect/ssoprovider/OpenIdConnectSSOProvider.java index 3b2b6eecf4..9183c6dbea 100644 --- a/openam-oauth2/src/main/java/org/forgerock/openidconnect/ssoprovider/OpenIdConnectSSOProvider.java +++ b/openam-oauth2/src/main/java/org/forgerock/openidconnect/ssoprovider/OpenIdConnectSSOProvider.java @@ -12,7 +12,7 @@ * information: "Portions copyright [year] [name of copyright owner]". * * Copyright 2016 ForgeRock AS. - * Portions copyright 2025 3A Systems LLC. + * Portions copyright 2025-2026 3A Systems LLC. */ package org.forgerock.openidconnect.ssoprovider; @@ -260,7 +260,7 @@ public String load(final @Nonnull String idTokenString) throws SSOException { throw new SSOException(e); } - if (!clientRegistration.verifyJwtIdentity(idToken)) { + if (!clientRegistration.verifyIdTokenIdentity(idToken)) { throw new SSOException("invalid id_token"); } diff --git a/openam-oauth2/src/test/java/org/forgerock/oauth2/core/OAuth2JwtTest.java b/openam-oauth2/src/test/java/org/forgerock/oauth2/core/OAuth2JwtTest.java new file mode 100644 index 0000000000..c0a6bfa2ef --- /dev/null +++ b/openam-oauth2/src/test/java/org/forgerock/oauth2/core/OAuth2JwtTest.java @@ -0,0 +1,59 @@ +/* + * The contents of this file are subject to the terms of the Common Development and + * Distribution License (the License). You may not use this file except in compliance with the + * License. + * + * You can obtain a copy of the License at legal/CDDLv1.0.txt. See the License for the + * specific language governing permission and limitations under the License. + * + * When distributing Covered Software, include this CDDL Header Notice in each file and include + * the License file at legal/CDDLv1.0.txt. If applicable, add the following below the CDDL + * Header, with the fields enclosed by brackets [] replaced by your own identifying + * information: "Portions copyright [year] [name of copyright owner]". + * + * Copyright 2026 3A Systems, LLC. + */ + +package org.forgerock.oauth2.core; + +import static org.assertj.core.api.Assertions.assertThat; + +import java.nio.charset.StandardCharsets; + +import org.forgerock.json.jose.builders.JwtBuilderFactory; +import org.forgerock.json.jose.jws.JwsAlgorithm; +import org.forgerock.json.jose.jws.SigningManager; +import org.forgerock.util.encode.Base64url; +import org.testng.annotations.Test; + +public class OAuth2JwtTest { + + @Test + public void getSigningAlgorithmReturnsHeaderAlgorithm() { + String jwt = new JwtBuilderFactory() + .jws(new SigningManager().newHmacSigningHandler("secret".getBytes(StandardCharsets.UTF_8))) + .headers().alg(JwsAlgorithm.HS256).done() + .claims(new JwtBuilderFactory().claims().sub("s").build()) + .build(); + + assertThat(OAuth2Jwt.create(jwt).getSigningAlgorithm()).isEqualTo(JwsAlgorithm.HS256); + } + + /** + * {@code JwsHeader.getAlgorithm()} is {@code JwsAlgorithm.valueOf(alg)} and throws for the RFC + * spelling {@code "none"}, for lower case and for anything it does not know; that is "no usable + * algorithm", not a server error. + */ + @Test + public void getSigningAlgorithmReturnsNullForAlgorithmOutsideTheEnum() { + assertThat(OAuth2Jwt.create(rawJwt("none")).getSigningAlgorithm()).isNull(); + assertThat(OAuth2Jwt.create(rawJwt("hs256")).getSigningAlgorithm()).isNull(); + assertThat(OAuth2Jwt.create(rawJwt("PS256")).getSigningAlgorithm()).isNull(); + } + + private static String rawJwt(String alg) { + String header = Base64url.encode(("{\"typ\":\"JWT\",\"alg\":\"" + alg + "\"}").getBytes(StandardCharsets.UTF_8)); + String payload = Base64url.encode("{\"sub\":\"s\"}".getBytes(StandardCharsets.UTF_8)); + return header + "." + payload + "."; + } +} diff --git a/openam-oauth2/src/test/java/org/forgerock/openam/oauth2/OpenAMClientRegistrationTest.java b/openam-oauth2/src/test/java/org/forgerock/openam/oauth2/OpenAMClientRegistrationTest.java index 075b002f8b..b58f593340 100644 --- a/openam-oauth2/src/test/java/org/forgerock/openam/oauth2/OpenAMClientRegistrationTest.java +++ b/openam-oauth2/src/test/java/org/forgerock/openam/oauth2/OpenAMClientRegistrationTest.java @@ -55,13 +55,13 @@ import org.forgerock.json.jose.jwe.JweAlgorithm; import org.forgerock.json.jose.jws.JwsAlgorithm; import org.forgerock.json.jose.jws.SigningManager; -import org.forgerock.json.jose.jws.handlers.NOPSigningHandler; import org.forgerock.json.jose.jws.handlers.SigningHandler; import org.forgerock.json.jose.jwt.JwtClaimsSet; import org.forgerock.oauth2.core.OAuth2Jwt; import org.forgerock.oauth2.core.OAuth2ProviderSettings; import org.forgerock.oauth2.core.PEMDecoder; import org.forgerock.oauth2.core.exceptions.ClientAuthenticationFailureFactory; +import org.forgerock.util.encode.Base64url; import org.mockito.Mock; import org.mockito.MockitoAnnotations; import org.testng.annotations.BeforeClass; @@ -408,16 +408,136 @@ public void verifyJwtIdentityUsesPublicKeysForAsymmetricAssertionRegardlessOfIdT .isFalse(); } + /** + * {@code JwsHeader.getAlgorithm()} is {@code JwsAlgorithm.valueOf(alg)}: the RFC spelling + * {@code "none"} (or anything outside the enum) throws rather than returning {@code NONE}, so + * the assertion is built as a raw compact serialisation, not through the builder, which would + * emit the enum name {@code "NONE"} that never occurs on the wire. + */ + @Test + public void verifyJwtIdentityRejectsWireAlgNone() throws Exception { + String clientId = "client1"; + given(amIdentity.getName()).willReturn(clientId); + given(amIdentity.getAttribute("userpassword")).willReturn(singleton("a-client-secret")); + given(amIdentity.getAttribute(IDTOKEN_SIGNED_RESPONSE_ALG)).willReturn(singleton("HS256")); + + assertThat(clientRegistration.verifyJwtIdentity(rawAssertion(clientId, "none"))).isFalse(); + } + @Test - public void verifyJwtIdentityRejectsUnsignedAssertion() throws Exception { + public void verifyJwtIdentityRejectsUnknownAlg() throws Exception { String clientId = "client1"; given(amIdentity.getName()).willReturn(clientId); given(amIdentity.getAttribute("userpassword")).willReturn(singleton("a-client-secret")); given(amIdentity.getAttribute(IDTOKEN_SIGNED_RESPONSE_ALG)).willReturn(singleton("HS256")); - OAuth2Jwt unsigned = assertion(clientId, new NOPSigningHandler(), JwsAlgorithm.NONE, null); + assertThat(clientRegistration.verifyJwtIdentity(rawAssertion(clientId, "hs256"))).isFalse(); + } + + /** A public client has no userpassword at all; an HMAC assertion is invalid_client, not an NPE. */ + @Test + public void verifyJwtIdentityReturnsFalseForHmacAssertionWhenClientHasNoSecret() throws Exception { + String clientId = "client1"; + given(amIdentity.getName()).willReturn(clientId); + given(amIdentity.getAttribute(IDTOKEN_SIGNED_RESPONSE_ALG)).willReturn(singleton("HS256")); + SigningHandler signer = new SigningManager().newHmacSigningHandler("any".getBytes(StandardCharsets.UTF_8)); + OAuth2Jwt assertion = assertion(clientId, signer, JwsAlgorithm.HS256, null); + + given(amIdentity.getAttribute("userpassword")).willReturn(null); + assertThat(clientRegistration.verifyJwtIdentity(assertion)).isFalse(); + + given(amIdentity.getAttribute("userpassword")).willReturn(Collections.emptySet()); + assertThat(clientRegistration.verifyJwtIdentity(assertion)).isFalse(); + } - assertThat(clientRegistration.verifyJwtIdentity(unsigned)).isFalse(); + /** + * AgentsRepo does not apply the schema default of publicKeyLocation either; a client with no + * registered key location cannot verify an asymmetric assertion, which is invalid_client rather + * than a server_error. + */ + @Test + public void verifyJwtIdentityReturnsFalseForAsymmetricAssertionWithoutPublicKeyLocation() throws Exception { + String clientId = "client1"; + given(amIdentity.getName()).willReturn(clientId); + given(amIdentity.getAttribute("userpassword")).willReturn(singleton("a-client-secret")); + given(amIdentity.getAttribute(IDTOKEN_SIGNED_RESPONSE_ALG)).willReturn(singleton("HS256")); + KeyPair clientKeys = generateRsaKeyPair(); + SigningHandler signer = new SigningManager().newRsaSigningHandler((RSAPrivateKey) clientKeys.getPrivate()); + OAuth2Jwt assertion = assertion(clientId, signer, JwsAlgorithm.RS256, "kid"); + + given(amIdentity.getAttribute(PUBLIC_KEY_SELECTOR)).willReturn(null); + assertThat(clientRegistration.verifyJwtIdentity(assertion)).isFalse(); + + given(amIdentity.getAttribute(PUBLIC_KEY_SELECTOR)).willReturn(Collections.emptySet()); + assertThat(clientRegistration.verifyJwtIdentity(assertion)).isFalse(); + + given(amIdentity.getAttribute(PUBLIC_KEY_SELECTOR)).willReturn(singleton("not-a-selector")); + assertThat(clientRegistration.verifyJwtIdentity(assertion)).isFalse(); + } + + /** + * A symmetric (oct) JWK can never verify an RS/ES-signed assertion; it must not be tried as an + * HMAC key. (JWKLookup keys an oct JWK's {@code alg} by the JCA name, hence {@code HmacSHA256}.) + */ + @Test + public void verifyJwtIdentityRejectsAsymmetricAssertionAgainstSymmetricJwk() throws Exception { + String clientId = "client1"; + String kid = UUID.randomUUID().toString(); + given(amIdentity.getName()).willReturn(clientId); + given(amIdentity.getAttribute("userpassword")).willReturn(singleton("a-client-secret")); + given(amIdentity.getAttribute(IDTOKEN_SIGNED_RESPONSE_ALG)).willReturn(singleton("HS256")); + given(amIdentity.getAttribute(PUBLIC_KEY_SELECTOR)).willReturn(singleton("jwks")); + given(amIdentity.getAttribute(JWKS)).willReturn(singleton("{\"keys\":[{\"kty\":\"oct\",\"alg\":\"HmacSHA256\",\"kid\":\"" + kid + + "\",\"k\":\"" + Base64url.encode("a-shared-key-of-sufficient-length".getBytes(StandardCharsets.UTF_8)) + + "\"}]}")); + KeyPair clientKeys = generateRsaKeyPair(); + SigningHandler signer = new SigningManager().newRsaSigningHandler((RSAPrivateKey) clientKeys.getPrivate()); + + assertThat(clientRegistration.verifyJwtIdentity(assertion(clientId, signer, JwsAlgorithm.RS256, kid))).isFalse(); + } + + /** + * An ID token this server issued carries id_token_signed_response_alg in its header; one signed + * with any other algorithm is not ours, whatever key it would otherwise verify with. + */ + @Test + public void verifyIdTokenIdentityRejectsAlgorithmOtherThanConfigured() throws Exception { + String clientId = "client1"; + String secret = "a-client-secret-of-sufficient-length"; + given(amIdentity.getName()).willReturn(clientId); + given(amIdentity.getAttribute("userpassword")).willReturn(singleton(secret)); + SigningHandler signer = new SigningManager().newHmacSigningHandler(secret.getBytes(StandardCharsets.UTF_8)); + OAuth2Jwt idToken = assertion(clientId, signer, JwsAlgorithm.HS256, null); + + given(amIdentity.getAttribute(IDTOKEN_SIGNED_RESPONSE_ALG)).willReturn(singleton("HS256")); + assertThat(clientRegistration.verifyIdTokenIdentity(idToken)).isTrue(); + + given(amIdentity.getAttribute(IDTOKEN_SIGNED_RESPONSE_ALG)).willReturn(singleton("RS256")); + assertThat(clientRegistration.verifyIdTokenIdentity(idToken)).isFalse(); + } + + @Test + public void verifyIdTokenIdentityRejectsWireAlgNone() throws Exception { + String clientId = "client1"; + given(amIdentity.getName()).willReturn(clientId); + given(amIdentity.getAttribute("userpassword")).willReturn(singleton("a-client-secret")); + given(amIdentity.getAttribute(IDTOKEN_SIGNED_RESPONSE_ALG)).willReturn(singleton("HS256")); + + assertThat(clientRegistration.verifyIdTokenIdentity(rawAssertion(clientId, "none"))).isFalse(); + } + + /** Compact serialisation with the given literal {@code alg} and an empty signature part. */ + private static OAuth2Jwt rawAssertion(String clientId, String alg) { + JwtClaimsSet claims = new JwtBuilderFactory().claims() + .iss(clientId) + .sub(clientId) + .aud(Collections.singletonList(clientId)) + .exp(new Date(System.currentTimeMillis() + 60_000L)) + .iat(new Date()) + .build(); + String header = Base64url.encode(("{\"typ\":\"JWT\",\"alg\":\"" + alg + "\"}").getBytes(StandardCharsets.UTF_8)); + String payload = Base64url.encode(claims.build().getBytes(StandardCharsets.UTF_8)); + return OAuth2Jwt.create(header + "." + payload + "."); } private static OAuth2Jwt assertion(String clientId, SigningHandler signer, JwsAlgorithm alg, String kid) { diff --git a/openam-oauth2/src/test/java/org/forgerock/openidconnect/ssoprovider/OpenIdConnectSSOProviderTest.java b/openam-oauth2/src/test/java/org/forgerock/openidconnect/ssoprovider/OpenIdConnectSSOProviderTest.java index 47d62cd3a1..7f0c19c63b 100644 --- a/openam-oauth2/src/test/java/org/forgerock/openidconnect/ssoprovider/OpenIdConnectSSOProviderTest.java +++ b/openam-oauth2/src/test/java/org/forgerock/openidconnect/ssoprovider/OpenIdConnectSSOProviderTest.java @@ -12,7 +12,7 @@ * information: "Portions copyright [year] [name of copyright owner]". * * Copyright 2016 ForgeRock AS. - * Portions copyright 2025 3A Systems LLC. + * Portions copyright 2025-2026 3A Systems LLC. */ package org.forgerock.openidconnect.ssoprovider; @@ -202,7 +202,32 @@ public void shouldRejectInvalidSignatures() throws Exception { given(mockProviderSettings.isOpenIDConnectSSOProviderEnabled()).willReturn(true); claimsSet.addAudience(clientId); given(mockClientStore.get(clientId, "/", null)).willReturn(mockClient); - given(mockClient.verifyJwtIdentity(mockJwt)).willReturn(false); + given(mockClient.verifyIdTokenIdentity(mockJwt)).willReturn(false); + + // When + ssoProvider.createSSOToken(tokenId); + + // Then - exception + } + + /** + * The id_token was issued by this server for the client, so it is checked as an ID token + * (header alg must be the client's id_token_signed_response_alg), not as a client assertion. + */ + @Test(expectedExceptions = SSOException.class, expectedExceptionsMessageRegExp = "invalid id_token") + public void shouldNotAcceptIdTokenVerifiedOnlyAsClientAssertion() throws Exception { + // Given + String tokenId = "a jwt signed with a key the client may use for assertions"; + String clientId = "client_id"; + given(mockTokenParser.parse(tokenId)).willReturn(mockJwt); + given(mockJwt.isExpired()).willReturn(false); + given(mockProviderSettingsFactory.getRealmProviderSettings("/")).willReturn(mockProviderSettings); + given(mockProviderSettings.isOpenIDConnectSSOProviderEnabled()).willReturn(true); + claimsSet.addAudience(clientId); + claimsSet.setClaim(OPS, "session identifier"); + given(mockClientStore.get(clientId, "/", null)).willReturn(mockClient); + given(mockClient.verifyJwtIdentity(mockJwt)).willReturn(true); + given(mockClient.verifyIdTokenIdentity(mockJwt)).willReturn(false); // When ssoProvider.createSSOToken(tokenId); @@ -222,7 +247,7 @@ public void shouldRejectJwtWithNoOpsClaim() throws Exception { claimsSet.addAudience(clientId); // no OPS claim given(mockClientStore.get(clientId, "/", null)).willReturn(mockClient); - given(mockClient.verifyJwtIdentity(mockJwt)).willReturn(true); + given(mockClient.verifyIdTokenIdentity(mockJwt)).willReturn(true); // When ssoProvider.createSSOToken(tokenId); @@ -243,7 +268,7 @@ public void shouldRejectJwtIfSessionNotFound() throws Exception { claimsSet.addAudience(clientId); claimsSet.setClaim(OPS, ops); given(mockClientStore.get(clientId, "/", null)).willReturn(mockClient); - given(mockClient.verifyJwtIdentity(mockJwt)).willReturn(true); + given(mockClient.verifyIdTokenIdentity(mockJwt)).willReturn(true); given(mockTokenStore.read(ops)).willReturn(null); // When @@ -266,7 +291,7 @@ public void shouldRejectJwtIfNoSessionLinked() throws Exception { claimsSet.addAudience(clientId); claimsSet.setClaim(OPS, ops); given(mockClientStore.get(clientId, "/", null)).willReturn(mockClient); - given(mockClient.verifyJwtIdentity(mockJwt)).willReturn(true); + given(mockClient.verifyIdTokenIdentity(mockJwt)).willReturn(true); given(mockTokenStore.read(ops)).willReturn(token); // When @@ -290,7 +315,7 @@ public void shouldUseStoredSessionIdWhenFound() throws Exception { claimsSet.addAudience(clientId); claimsSet.setClaim(OPS, ops); given(mockClientStore.get(clientId, "/", null)).willReturn(mockClient); - given(mockClient.verifyJwtIdentity(mockJwt)).willReturn(true); + given(mockClient.verifyIdTokenIdentity(mockJwt)).willReturn(true); given(mockTokenStore.read(ops)).willReturn(json(object(field(LEGACY_OPS, asList(sessionId))))); given(mockTokenManager.createSSOToken(sessionId)).willReturn(mockSsoToken); @@ -315,7 +340,7 @@ public void shouldUseSSOTokenClaimWhenPresent() throws Exception { claimsSet.addAudience(clientId); claimsSet.setClaim(SSOTOKEN, sessionId); given(mockClientStore.get(clientId, "/", null)).willReturn(mockClient); - given(mockClient.verifyJwtIdentity(mockJwt)).willReturn(true); + given(mockClient.verifyIdTokenIdentity(mockJwt)).willReturn(true); given(mockTokenManager.createSSOToken(sessionId)).willReturn(mockSsoToken); // When From 6d58828497e77754e3291f3f83f1616d4bf5b052 Mon Sep 17 00:00:00 2001 From: Valera V Harseko Date: Thu, 17 Sep 2026 10:20:00 +0300 Subject: [PATCH 3/4] [#1130] Fail closed on missing key material and alg/key mismatch; tolerate free-text id_token alg Review round 3 on #1131. - verifyJwtIdentity(), asymmetric arm: a registered publicKeyLocation with nothing behind it (jwks_uri without a URI, x509 without a certificate, jwks without a set) returns false instead of raising SERVER_ERROR inside the catch that logs a stack trace. AgentConfiguration.createAgent persists the schema default jwks_uri, so a console-, ssoadm- or /json/agents-created client is in this state until a URI is configured, and any client_id can be sent an assertion with an asymmetric alg. JwsSigningException and IllegalArgumentException from the handlers (RSA key vs. ES256 header, a symmetric key served at jwks_uri) are the same invalid_client; IdRepo/SSO and JWKS-fetch failures remain server errors. - verifyIdTokenIdentity(): idTokenSignedResponseAlg is a free-text attribute and StatefulTokenStore upper-cases it when issuing; the verifier now does the same and treats an unknown value as "not ours" instead of throwing. - openam-uma IdTokenClaimGatherer: getClientSecret() may be null for a public client since round 2; an RS256 id_token verifies with the provider key alone, an HMAC one without a secret is rejected instead of NPE. Tests: key location without material (three selectors); ES256 against an RSA JWK; symmetric key behind jwks_uri (resolver seeded via ClientJwksResolverCache); header without alg and enum-spelled "NONE"; configured "hs256" accepted and "not-an-alg" rejected; UMA gatherer with a null secret (RS256 / HS256); new IdTokenInfoTest (openam-core test-jar for RealmTestHelper) pinning that the endpoint uses verifyIdTokenIdentity. --- openam-oauth2/pom.xml | 6 + .../oauth2/OpenAMClientRegistration.java | 43 +++-- .../oauth2/OpenAMClientRegistrationTest.java | 127 ++++++++++++++- .../restlet/IdTokenInfoTest.java | 148 ++++++++++++++++++ .../openam/uma/IdTokenClaimGatherer.java | 9 +- .../openam/uma/IdTokenClaimGathererTest.java | 47 ++++++ 6 files changed, 360 insertions(+), 20 deletions(-) create mode 100644 openam-oauth2/src/test/java/org/forgerock/openidconnect/restlet/IdTokenInfoTest.java diff --git a/openam-oauth2/pom.xml b/openam-oauth2/pom.xml index 0d05abd169..cce6a1c790 100644 --- a/openam-oauth2/pom.xml +++ b/openam-oauth2/pom.xml @@ -166,6 +166,12 @@ test-jar test + + org.openidentityplatform.openam + openam-core + test-jar + test + org.openidentityplatform.openam openam-notifications-integration diff --git a/openam-oauth2/src/main/java/org/forgerock/openam/oauth2/OpenAMClientRegistration.java b/openam-oauth2/src/main/java/org/forgerock/openam/oauth2/OpenAMClientRegistration.java index a049fbd03e..212ac8e0fa 100644 --- a/openam-oauth2/src/main/java/org/forgerock/openam/oauth2/OpenAMClientRegistration.java +++ b/openam-oauth2/src/main/java/org/forgerock/openam/oauth2/OpenAMClientRegistration.java @@ -60,6 +60,7 @@ import org.forgerock.jaspi.modules.openid.resolvers.SharedSecretOpenIdResolverImpl; import org.forgerock.jaspi.modules.openid.resolvers.service.OpenIdResolverService; import org.forgerock.json.jose.exceptions.JweException; +import org.forgerock.json.jose.exceptions.JwsSigningException; import org.forgerock.json.jose.jwe.EncryptionMethod; import org.forgerock.json.jose.jwe.JweAlgorithm; import org.forgerock.json.jose.jwk.JWKSet; @@ -668,10 +669,12 @@ public boolean verifyJwtIdentity(final OAuth2Jwt jwt) { if (signatureAlgorithm.getAlgorithmType() == JwsAlgorithmType.HMAC) { return verifyJwtBySharedSecret(jwt); } else { + // Any client_id can be sent an assertion with this alg, so what the client has (not) + // registered decides between invalid_client and server_error: no key location, no key + // material behind it, or a key that cannot verify this alg is false; only failures to + // read the registration or fetch the JWKS are server errors. final Client.PublicKeySelector selector = getClientPublicKeySelector(); if (selector == null) { - // Nothing registered to verify an asymmetric assertion with: invalid_client, not - // server_error, now that any client_id can be sent an assertion with this alg. return false; } try { @@ -683,6 +686,10 @@ public boolean verifyJwtIdentity(final OAuth2Jwt jwt) { default: return byX509Key(jwt); } + } catch (JwsSigningException | IllegalArgumentException e) { + // The registered key does not fit the header's alg (RSA key vs. ES256, a symmetric + // key served at jwks_uri): the client cannot be verified with what it registered. + return false; } catch (Exception e) { throw Utils.createException("Client Bearer Jwt Public key", e, logger); } @@ -693,7 +700,14 @@ public boolean verifyJwtIdentity(final OAuth2Jwt jwt) { public boolean verifyIdTokenIdentity(final OAuth2Jwt idToken) { // We issued the ID token with id_token_signed_response_alg, so the header must say so; // the algorithm, and hence the key the token is verified with, is not the presenter's to pick. - final JwsAlgorithm configured = JwsAlgorithm.valueOf(getIDTokenSignedResponseAlgorithm()); + // idTokenSignedResponseAlg is a free-text attribute; StatefulTokenStore upper-cases it when + // issuing, so the same spelling verifies, and a value we could not have issued with is not ours. + final JwsAlgorithm configured; + try { + configured = JwsAlgorithm.valueOf(getIDTokenSignedResponseAlgorithm().toUpperCase()); + } catch (IllegalArgumentException e) { + return false; + } return idToken.getSigningAlgorithm() == configured && verifyJwtIdentity(idToken); } @@ -723,9 +737,8 @@ private boolean byJWKs(OAuth2Jwt jwt) throws IdRepoException, SSOException, Set set = amIdentity.getAttribute(OAuth2Constants.OAuth2Client.JWKS); final String jwkSetStr = CollectionUtils.getFirstItem(set); - if (jwkSetStr == null) { - throw OAuthProblemException.OAuthError.SERVER_ERROR.handle(Request.getCurrent(), - "No Client Bearer JWK set."); + if (StringUtils.isEmpty(jwkSetStr)) { + return false; } final JWKSet jwkSet = new JWKSet(JsonValueBuilder.toJsonValue(jwkSetStr) @@ -764,13 +777,13 @@ private SigningHandler getSigningHandlerForKey(final Key key) { private boolean byJWKsURI(OAuth2Jwt jwt) throws IdRepoException, SSOException, MalformedURLException { final Set set = amIdentity.getAttribute(OAuth2Constants.OAuth2Client.JWKS_URI); - if (set == null || set.isEmpty()) { - throw OAuthProblemException.OAuthError.SERVER_ERROR.handle(Request.getCurrent(), - "No Client Bearer JWKs_URI set."); + // The schema default of publicKeyLocation is jwks_uri, so a client created through the + // console or ssoadm typically carries the selector and no URI until one is configured. + final String url = CollectionUtils.getFirstItem(set); + if (StringUtils.isEmpty(url)) { + return false; } - final String url = set.iterator().next(); - // GHSA-f2cx-463q-7m2c: the resolver cache MUST be keyed by something tied to the // client registration, not by the attacker-controlled JWT 'iss' claim. Otherwise a // resolver seeded by one client (with its own keys) can be reused to validate a @@ -823,12 +836,10 @@ private boolean byX509Key(OAuth2Jwt jwt) throws IdRepoException, SSOException, C Set set = amIdentity.getAttribute(OAuth2Constants.OAuth2Client.CLIENT_JWT_PUBLIC_KEY); - if (set == null || set.isEmpty()) { - throw OAuthProblemException.OAuthError.SERVER_ERROR.handle(Request.getCurrent(), - "No Client Bearer Jwt Public key certificate set"); + String encodedCert = CollectionUtils.getFirstItem(set); + if (StringUtils.isEmpty(encodedCert)) { + return false; } - - String encodedCert = set.iterator().next(); X509Certificate certificate = pemDecoder.decodeX509Certificate(encodedCert); return jwt.isValid(signingManager.newRsaSigningHandler(certificate.getPublicKey())); diff --git a/openam-oauth2/src/test/java/org/forgerock/openam/oauth2/OpenAMClientRegistrationTest.java b/openam-oauth2/src/test/java/org/forgerock/openam/oauth2/OpenAMClientRegistrationTest.java index b58f593340..83c0eeb7ee 100644 --- a/openam-oauth2/src/test/java/org/forgerock/openam/oauth2/OpenAMClientRegistrationTest.java +++ b/openam-oauth2/src/test/java/org/forgerock/openam/oauth2/OpenAMClientRegistrationTest.java @@ -35,6 +35,8 @@ import java.security.KeyPairGenerator; import java.security.MessageDigest; import java.security.PublicKey; +import java.security.spec.ECGenParameterSpec; +import java.security.interfaces.ECPrivateKey; import java.security.interfaces.RSAPrivateKey; import java.security.interfaces.RSAPublicKey; import java.util.Arrays; @@ -49,6 +51,7 @@ import javax.crypto.spec.SecretKeySpec; +import org.forgerock.jaspi.modules.openid.resolvers.SharedSecretOpenIdResolverImpl; import org.forgerock.jaspi.modules.openid.resolvers.service.OpenIdResolverService; import org.forgerock.json.jose.builders.JwsHeaderBuilder; import org.forgerock.json.jose.builders.JwtBuilderFactory; @@ -526,7 +529,126 @@ public void verifyIdTokenIdentityRejectsWireAlgNone() throws Exception { assertThat(clientRegistration.verifyIdTokenIdentity(rawAssertion(clientId, "none"))).isFalse(); } - /** Compact serialisation with the given literal {@code alg} and an empty signature part. */ + /** + * AgentConfiguration.createAgent persists the schema defaults, so a client created through the + * console, ssoadm or /json/agents carries publicKeyLocation=jwks_uri and no jwks_uri. Nothing + * registered behind the selector is invalid_client, not server_error with a logged stack trace. + */ + @Test + public void verifyJwtIdentityReturnsFalseWhenRegisteredKeyLocationHasNoMaterial() throws Exception { + String clientId = "client1"; + given(amIdentity.getName()).willReturn(clientId); + given(amIdentity.getAttribute("userpassword")).willReturn(singleton("a-client-secret")); + given(amIdentity.getAttribute(IDTOKEN_SIGNED_RESPONSE_ALG)).willReturn(singleton("HS256")); + KeyPair clientKeys = generateRsaKeyPair(); + SigningHandler signer = new SigningManager().newRsaSigningHandler((RSAPrivateKey) clientKeys.getPrivate()); + OAuth2Jwt assertion = assertion(clientId, signer, JwsAlgorithm.RS256, "kid"); + + given(amIdentity.getAttribute(PUBLIC_KEY_SELECTOR)).willReturn(singleton("jwks_uri")); + given(amIdentity.getAttribute(JWKS_URI)).willReturn(null); + assertThat(clientRegistration.verifyJwtIdentity(assertion)).isFalse(); + given(amIdentity.getAttribute(JWKS_URI)).willReturn(Collections.emptySet()); + assertThat(clientRegistration.verifyJwtIdentity(assertion)).isFalse(); + given(amIdentity.getAttribute(JWKS_URI)).willReturn(singleton("")); + assertThat(clientRegistration.verifyJwtIdentity(assertion)).isFalse(); + + given(amIdentity.getAttribute(PUBLIC_KEY_SELECTOR)).willReturn(singleton("x509")); + given(amIdentity.getAttribute(CLIENT_JWT_PUBLIC_KEY)).willReturn(null); + assertThat(clientRegistration.verifyJwtIdentity(assertion)).isFalse(); + given(amIdentity.getAttribute(CLIENT_JWT_PUBLIC_KEY)).willReturn(singleton("")); + assertThat(clientRegistration.verifyJwtIdentity(assertion)).isFalse(); + + given(amIdentity.getAttribute(PUBLIC_KEY_SELECTOR)).willReturn(singleton("jwks")); + given(amIdentity.getAttribute(JWKS)).willReturn(null); + assertThat(clientRegistration.verifyJwtIdentity(assertion)).isFalse(); + given(amIdentity.getAttribute(JWKS)).willReturn(singleton("")); + assertThat(clientRegistration.verifyJwtIdentity(assertion)).isFalse(); + } + + /** A registered key that cannot verify the header's algorithm (RSA key, ES256 header) is invalid_client. */ + @Test + public void verifyJwtIdentityReturnsFalseWhenRegisteredKeyCannotVerifyAlgorithm() throws Exception { + String clientId = "client1"; + String kid = UUID.randomUUID().toString(); + KeyPair rsaKeys = generateRsaKeyPair(); + given(amIdentity.getName()).willReturn(clientId); + given(amIdentity.getAttribute("userpassword")).willReturn(singleton("a-client-secret")); + given(amIdentity.getAttribute(IDTOKEN_SIGNED_RESPONSE_ALG)).willReturn(singleton("HS256")); + given(amIdentity.getAttribute(PUBLIC_KEY_SELECTOR)).willReturn(singleton("jwks")); + given(amIdentity.getAttribute(JWKS)).willReturn(singleton(jwks((RSAPublicKey) rsaKeys.getPublic(), kid))); + + KeyPairGenerator gen = KeyPairGenerator.getInstance("EC"); + gen.initialize(new ECGenParameterSpec("secp256r1")); + SigningHandler ecSigner = new SigningManager().newEcdsaSigningHandler((ECPrivateKey) gen.generateKeyPair().getPrivate()); + + assertThat(clientRegistration.verifyJwtIdentity(assertion(clientId, ecSigner, JwsAlgorithm.ES256, kid))).isFalse(); + } + + /** + * A symmetric key served at jwks_uri reaches an HMAC handler inside the commons resolver, which + * cannot verify an RS256 assertion; that is invalid_client, not server_error. The resolver is + * seeded through the cache so the test needs no HTTP server. + */ + @Test + public void verifyJwtIdentityReturnsFalseForSymmetricKeyBehindJwksUri() throws Exception { + String clientId = "client1"; + String url = "https://jwks.invalid/" + UUID.randomUUID(); + given(amIdentity.getName()).willReturn(clientId); + given(amIdentity.getAttribute("userpassword")).willReturn(singleton("a-client-secret")); + given(amIdentity.getAttribute(IDTOKEN_SIGNED_RESPONSE_ALG)).willReturn(singleton("HS256")); + given(amIdentity.getAttribute(PUBLIC_KEY_SELECTOR)).willReturn(singleton("jwks_uri")); + given(amIdentity.getAttribute(JWKS_URI)).willReturn(singleton(url)); + KeyPair clientKeys = generateRsaKeyPair(); + SigningHandler signer = new SigningManager().newRsaSigningHandler((RSAPrivateKey) clientKeys.getPrivate()); + ClientJwksResolverCache.putIfAbsent(clientId + "|" + url, + new SharedSecretOpenIdResolverImpl(clientId, "a-shared-key-of-sufficient-length")); + try { + assertThat(clientRegistration.verifyJwtIdentity(assertion(clientId, signer, JwsAlgorithm.RS256, "kid"))) + .isFalse(); + } finally { + ClientJwksResolverCache.resetForTest(); + } + } + + /** + * A header without alg is what JwsHeader.getAlgorithm() maps to NONE, and the enum name "NONE" + * is the other spelling that reaches that arm; neither is a signed assertion. + */ + @Test + public void verifyJwtIdentityRejectsAssertionWithoutAlgOrWithEnumSpelledNone() throws Exception { + String clientId = "client1"; + given(amIdentity.getName()).willReturn(clientId); + given(amIdentity.getAttribute("userpassword")).willReturn(singleton("a-client-secret")); + given(amIdentity.getAttribute(IDTOKEN_SIGNED_RESPONSE_ALG)).willReturn(singleton("HS256")); + + assertThat(clientRegistration.verifyJwtIdentity(rawAssertion(clientId, null))).isFalse(); + assertThat(clientRegistration.verifyJwtIdentity(rawAssertion(clientId, "NONE"))).isFalse(); + } + + /** + * idTokenSignedResponseAlg is a free-text attribute; StatefulTokenStore upper-cases it when + * issuing, so the verifier must accept the same spelling, and an unknown value is not ours. + */ + @Test + public void verifyIdTokenIdentityNormalisesConfiguredAlgorithmAndRejectsUnknown() throws Exception { + String clientId = "client1"; + String secret = "a-client-secret-of-sufficient-length"; + given(amIdentity.getName()).willReturn(clientId); + given(amIdentity.getAttribute("userpassword")).willReturn(singleton(secret)); + SigningHandler signer = new SigningManager().newHmacSigningHandler(secret.getBytes(StandardCharsets.UTF_8)); + OAuth2Jwt idToken = assertion(clientId, signer, JwsAlgorithm.HS256, null); + + given(amIdentity.getAttribute(IDTOKEN_SIGNED_RESPONSE_ALG)).willReturn(singleton("hs256")); + assertThat(clientRegistration.verifyIdTokenIdentity(idToken)).isTrue(); + + given(amIdentity.getAttribute(IDTOKEN_SIGNED_RESPONSE_ALG)).willReturn(singleton("not-an-alg")); + assertThat(clientRegistration.verifyIdTokenIdentity(idToken)).isFalse(); + } + + /** + * Compact serialisation with the given literal {@code alg} ({@code null}: no alg member) and an + * empty signature part. + */ private static OAuth2Jwt rawAssertion(String clientId, String alg) { JwtClaimsSet claims = new JwtBuilderFactory().claims() .iss(clientId) @@ -535,7 +657,8 @@ private static OAuth2Jwt rawAssertion(String clientId, String alg) { .exp(new Date(System.currentTimeMillis() + 60_000L)) .iat(new Date()) .build(); - String header = Base64url.encode(("{\"typ\":\"JWT\",\"alg\":\"" + alg + "\"}").getBytes(StandardCharsets.UTF_8)); + String headerJson = alg == null ? "{\"typ\":\"JWT\"}" : "{\"typ\":\"JWT\",\"alg\":\"" + alg + "\"}"; + String header = Base64url.encode(headerJson.getBytes(StandardCharsets.UTF_8)); String payload = Base64url.encode(claims.build().getBytes(StandardCharsets.UTF_8)); return OAuth2Jwt.create(header + "." + payload + "."); } diff --git a/openam-oauth2/src/test/java/org/forgerock/openidconnect/restlet/IdTokenInfoTest.java b/openam-oauth2/src/test/java/org/forgerock/openidconnect/restlet/IdTokenInfoTest.java new file mode 100644 index 0000000000..d7dc9aa250 --- /dev/null +++ b/openam-oauth2/src/test/java/org/forgerock/openidconnect/restlet/IdTokenInfoTest.java @@ -0,0 +1,148 @@ +/* + * The contents of this file are subject to the terms of the Common Development and + * Distribution License (the License). You may not use this file except in compliance with the + * License. + * + * You can obtain a copy of the License at legal/CDDLv1.0.txt. See the License for the + * specific language governing permission and limitations under the License. + * + * When distributing Covered Software, include this CDDL Header Notice in each file and include + * the License file at legal/CDDLv1.0.txt. If applicable, add the following below the CDDL + * Header, with the fields enclosed by brackets [] replaced by your own identifying + * information: "Portions copyright [year] [name of copyright owner]". + * + * Copyright 2026 3A Systems, LLC. + */ + +package org.forgerock.openidconnect.restlet; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.BDDMockito.given; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; + +import java.nio.charset.StandardCharsets; +import java.util.Collections; +import java.util.Date; +import java.util.concurrent.ConcurrentHashMap; + +import jakarta.servlet.http.HttpServletRequest; + +import org.forgerock.json.jose.builders.JwtBuilderFactory; +import org.forgerock.json.jose.jws.JwsAlgorithm; +import org.forgerock.json.jose.jws.SigningManager; +import org.forgerock.oauth2.core.ClientAuthenticator; +import org.forgerock.oauth2.core.OAuth2Jwt; +import org.forgerock.oauth2.core.OAuth2ProviderSettings; +import org.forgerock.oauth2.core.OAuth2ProviderSettingsFactory; +import org.forgerock.oauth2.core.OAuth2Request; +import org.forgerock.oauth2.core.OAuth2RequestFactory; +import org.forgerock.oauth2.core.exceptions.BadRequestException; +import org.forgerock.oauth2.restlet.ExceptionHandler; +import org.forgerock.openam.core.realms.RealmTestHelper; +import org.forgerock.openam.oauth2.OAuth2Constants; +import org.forgerock.openam.oauth2.OAuth2UrisFactory; +import org.forgerock.openam.rest.jakarta.servlet.internal.ServletCall; +import org.forgerock.openidconnect.OpenIdConnectClientRegistration; +import org.forgerock.openidconnect.OpenIdConnectClientRegistrationStore; +import org.mockito.Mock; +import org.mockito.MockitoAnnotations; +import org.restlet.engine.adapter.HttpRequest; +import org.testng.annotations.AfterMethod; +import org.testng.annotations.BeforeMethod; +import org.testng.annotations.Test; + +public class IdTokenInfoTest { + + private static final String CLIENT_ID = "client1"; + + @Mock + private OpenIdConnectClientRegistrationStore clientRegistrationStore; + @Mock + private OAuth2RequestFactory requestFactory; + @Mock + private ExceptionHandler exceptionHandler; + @Mock + private ClientAuthenticator clientAuthenticator; + @Mock + private OAuth2UrisFactory urisFactory; + @Mock + private OAuth2ProviderSettingsFactory providerSettingsFactory; + @Mock + private OAuth2ProviderSettings providerSettings; + @Mock + private OpenIdConnectClientRegistration clientRegistration; + @Mock + private OAuth2Request request; + + private RealmTestHelper realmTestHelper; + private IdTokenInfo idTokenInfo; + + @BeforeMethod + public void setup() throws Exception { + MockitoAnnotations.initMocks(this); + realmTestHelper = new RealmTestHelper(); + realmTestHelper.setupRealmClass(); + idTokenInfo = new IdTokenInfo(clientRegistrationStore, requestFactory, exceptionHandler, + clientAuthenticator, urisFactory, providerSettingsFactory); + + // setRealmOnRequest() needs a restlet request with attributes and the servlet request behind it. + HttpRequest restletRequest = mock(HttpRequest.class); + ServletCall servletCall = mock(ServletCall.class); + given(restletRequest.getAttributes()).willReturn(new ConcurrentHashMap()); + given(restletRequest.getHttpCall()).willReturn(servletCall); + given(servletCall.getRequest()).willReturn(mock(HttpServletRequest.class)); + given(request.getRequest()).willReturn(restletRequest); + + given(clientRegistrationStore.get(eq(CLIENT_ID), any(OAuth2Request.class))).willReturn(clientRegistration); + given(clientRegistration.getIDTokenSignedResponseAlgorithm()).willReturn("HS256"); + given(providerSettingsFactory.get(request)).willReturn(providerSettings); + given(providerSettings.isIdTokenInfoClientAuthenticationEnabled()).willReturn(false); + } + + @AfterMethod + public void tearDown() { + realmTestHelper.tearDownRealmClass(); + } + + @Test + public void shouldReturnIdTokenIssuedForTheClient() throws Exception { + given(request.getParameter(OAuth2Constants.JWTTokenParams.ID_TOKEN)).willReturn(idToken()); + given(clientRegistration.verifyIdTokenIdentity(any(OAuth2Jwt.class))).willReturn(true); + + OAuth2Jwt result = idTokenInfo.validateIdToken(request); + + assertThat(result.getSignedJwt().getClaimsSet().getAudience()).containsExactly(CLIENT_ID); + verify(clientRegistration).verifyIdTokenIdentity(any(OAuth2Jwt.class)); + } + + /** + * The id_token is checked as an ID token this server issued (header alg pinned to the client's + * id_token_signed_response_alg), not as a client assertion: a token that only verifies as the + * latter is rejected. + */ + @Test(expectedExceptions = BadRequestException.class, expectedExceptionsMessageRegExp = "invalid id_token") + public void shouldRejectIdTokenThatVerifiesOnlyAsClientAssertion() throws Exception { + given(request.getParameter(OAuth2Constants.JWTTokenParams.ID_TOKEN)).willReturn(idToken()); + given(clientRegistration.verifyJwtIdentity(any(OAuth2Jwt.class))).willReturn(true); + given(clientRegistration.verifyIdTokenIdentity(any(OAuth2Jwt.class))).willReturn(false); + + idTokenInfo.validateIdToken(request); + } + + private static String idToken() { + return new JwtBuilderFactory() + .jws(new SigningManager().newHmacSigningHandler("secret".getBytes(StandardCharsets.UTF_8))) + .headers().alg(JwsAlgorithm.HS256).done() + .claims(new JwtBuilderFactory().claims() + .iss("https://openam.example.com/openam/oauth2") + .sub("user1") + .aud(Collections.singletonList(CLIENT_ID)) + .exp(new Date(System.currentTimeMillis() + 60_000L)) + .iat(new Date()) + .build()) + .build(); + } +} diff --git a/openam-uma/src/main/java/org/forgerock/openam/uma/IdTokenClaimGatherer.java b/openam-uma/src/main/java/org/forgerock/openam/uma/IdTokenClaimGatherer.java index c770569353..f5fa9946d2 100644 --- a/openam-uma/src/main/java/org/forgerock/openam/uma/IdTokenClaimGatherer.java +++ b/openam-uma/src/main/java/org/forgerock/openam/uma/IdTokenClaimGatherer.java @@ -12,6 +12,7 @@ * information: "Portions copyright [year] [name of copyright owner]". * * Copyright 2015-2016 ForgeRock AS. + * Portions Copyrighted 2026 3A Systems, LLC. */ package org.forgerock.openam.uma; @@ -83,8 +84,10 @@ public String getRequestingPartyId(OAuth2Request oAuth2Request, AccessToken auth OAuth2ProviderSettings oAuth2ProviderSettings = oauth2ProviderSettingsFactory.get(oAuth2Request); OAuth2Uris oAuth2Uris = oAuth2UrisFactory.get(oAuth2Request); - byte[] clientSecret = clientRegistrationStore.get(authorizationApiToken.getClientId(), oAuth2Request) - .getClientSecret().getBytes(Utils.CHARSET); + // A public client has no secret; verify() only needs it for an HMAC-signed id_token. + String secret = clientRegistrationStore.get(authorizationApiToken.getClientId(), oAuth2Request) + .getClientSecret(); + byte[] clientSecret = secret == null ? null : secret.getBytes(Utils.CHARSET); KeyPair keyPair = oAuth2ProviderSettings.getSigningKeyPair(idToken.getHeader().getAlgorithm()); if (!idToken.getClaimsSet().getIssuer().equals(oAuth2Uris.getIssuer())) { @@ -122,6 +125,8 @@ private boolean verify(byte[] clientSecret, KeyPair keyPair, SignedJwt signedJwt SigningHandler signingHandler; if (JwsAlgorithmType.RSA.equals(jwsAlgorithm.getAlgorithmType())) { signingHandler = signingManager.newRsaSigningHandler(keyPair.getPublic()); + } else if (clientSecret == null) { + return false; } else { signingHandler = signingManager.newHmacSigningHandler(clientSecret); } diff --git a/openam-uma/src/test/java/org/forgerock/openam/uma/IdTokenClaimGathererTest.java b/openam-uma/src/test/java/org/forgerock/openam/uma/IdTokenClaimGathererTest.java index 83d7e9a879..1113b17d21 100644 --- a/openam-uma/src/test/java/org/forgerock/openam/uma/IdTokenClaimGathererTest.java +++ b/openam-uma/src/test/java/org/forgerock/openam/uma/IdTokenClaimGathererTest.java @@ -160,6 +160,42 @@ public void shouldNotGatherIdTokenClaimTokenWhichIsIncorrectlySigned() { assertThat(requestingPartyId).isNull(); } + /** A public client has no secret; an RS256 id_token is verified with the provider's key alone. */ + @Test + public void shouldGatherRsaSignedIdTokenClaimTokenForClientWithoutSecret() { + + //Given + AccessToken authorizationApiToken = mockAuthorizationApiToken(); + given(clientRegistration.getClientSecret()).willReturn(null); + JsonValue claimToken = mockRsaIdTokenClaimToken("ISSUER"); + + setIdTokenAndOAuth2ProviderIssuers("ISSUER"); + + //When + String requestingPartyId = claimGatherer.getRequestingPartyId(oAuth2Request, authorizationApiToken, claimToken); + + //Then + assertThat(requestingPartyId).isEqualTo("REQUESTING_PARTY_ID"); + } + + /** An HMAC id_token from a client without a secret cannot be verified; that is a rejection, not an NPE. */ + @Test + public void shouldNotGatherHmacSignedIdTokenClaimTokenForClientWithoutSecret() { + + //Given + AccessToken authorizationApiToken = mockAuthorizationApiToken(); + given(clientRegistration.getClientSecret()).willReturn(null); + JsonValue claimToken = mockIdTokenClaimToken("ISSUER"); + + setIdTokenAndOAuth2ProviderIssuers("ISSUER"); + + //When + String requestingPartyId = claimGatherer.getRequestingPartyId(oAuth2Request, authorizationApiToken, claimToken); + + //Then + assertThat(requestingPartyId).isNull(); + } + private AccessToken mockAuthorizationApiToken() { AccessToken authorizationApiToken = mock(AccessToken.class); given(authorizationApiToken.getClientId()).willReturn("CLIENT_ID"); @@ -180,6 +216,17 @@ private JsonValue mockInvalidIdTokenClaimToken(String issuer) { return json("ID_TOKEN"); } + private JsonValue mockRsaIdTokenClaimToken(String issuer) { + given(jwsHeader.getAlgorithm()).willReturn(JwsAlgorithm.RS256); + given(idToken.getHeader()).willReturn(jwsHeader); + SigningHandler signingHandler = mock(SigningHandler.class); + given(signingManager.newRsaSigningHandler(any(java.security.Key.class))).willReturn(signingHandler); + given(idToken.verify(signingHandler)).willReturn(true); + given(claimsSet.getSubject()).willReturn("REQUESTING_PARTY_ID"); + given(claimsSet.getIssuer()).willReturn(issuer); + return json("ID_TOKEN"); + } + private void mockIdToken(boolean isValid) { given(jwsHeader.getAlgorithm()).willReturn(JwsAlgorithm.HS256); given(idToken.getHeader()).willReturn(jwsHeader); From cbe6fd92c503a0c384de4a0da9fd81b1269603d5 Mon Sep 17 00:00:00 2001 From: Valera V Harseko Date: Thu, 17 Sep 2026 14:59:29 +0300 Subject: [PATCH 4/4] [#1130] Treat any JwsException as invalid_client; normalise the configured alg in IdTokenInfo Review round 4 on #1131. - verifyJwtIdentity(), asymmetric arm: RSASigningHandler wraps the JDK's "Bad signature length" in JwsVerifyingException, a sibling of JwsSigningException under JwsException, so an RS256 assertion signed with a key of another length than the registered one still reached the generic catch (error-level stack trace + server_error). Catch JwsException; log the rejection at message level with the client id and header alg so a client that stops authenticating after a key rotation leaves a trace. - IdTokenInfo: the client-authentication gate parsed the free-text idTokenSignedResponseAlg with a strict valueOf, so a client configured "hs256" got server_error from /idtokeninfo for every token it was issued; upper-case as the token store does, and reject an unknown value as a bad request. Tests: 2048-bit registered JWK vs. 3072-bit signer -> false; IdTokenInfoTest accepts "hs256" and rejects "not-an-alg" with a BadRequestException. --- .../oauth2/OpenAMClientRegistration.java | 15 +++++++--- .../openidconnect/restlet/IdTokenInfo.java | 8 +++++- .../oauth2/OpenAMClientRegistrationTest.java | 28 ++++++++++++++++++- .../restlet/IdTokenInfoTest.java | 25 +++++++++++++++++ 4 files changed, 70 insertions(+), 6 deletions(-) diff --git a/openam-oauth2/src/main/java/org/forgerock/openam/oauth2/OpenAMClientRegistration.java b/openam-oauth2/src/main/java/org/forgerock/openam/oauth2/OpenAMClientRegistration.java index 212ac8e0fa..e66390f0b6 100644 --- a/openam-oauth2/src/main/java/org/forgerock/openam/oauth2/OpenAMClientRegistration.java +++ b/openam-oauth2/src/main/java/org/forgerock/openam/oauth2/OpenAMClientRegistration.java @@ -60,7 +60,7 @@ import org.forgerock.jaspi.modules.openid.resolvers.SharedSecretOpenIdResolverImpl; import org.forgerock.jaspi.modules.openid.resolvers.service.OpenIdResolverService; import org.forgerock.json.jose.exceptions.JweException; -import org.forgerock.json.jose.exceptions.JwsSigningException; +import org.forgerock.json.jose.exceptions.JwsException; import org.forgerock.json.jose.jwe.EncryptionMethod; import org.forgerock.json.jose.jwe.JweAlgorithm; import org.forgerock.json.jose.jwk.JWKSet; @@ -686,9 +686,16 @@ public boolean verifyJwtIdentity(final OAuth2Jwt jwt) { default: return byX509Key(jwt); } - } catch (JwsSigningException | IllegalArgumentException e) { - // The registered key does not fit the header's alg (RSA key vs. ES256, a symmetric - // key served at jwks_uri): the client cannot be verified with what it registered. + } catch (JwsException | IllegalArgumentException e) { + // Wrong key type, an alg the key cannot verify (RSA key vs. ES256, a symmetric key + // served at jwks_uri) or a signature the key cannot even parse (JwsVerifyingException, + // RSA: wrong length): the client cannot be verified with what it registered. Message + // level, so a client that stops authenticating after a key rotation leaves a trace + // without an error and a stack trace per unauthenticated request. + if (logger.messageEnabled()) { + logger.message("Client {} assertion with alg {} cannot be verified with its registered key: {}", + getClientId(), signatureAlgorithm, e.toString()); + } return false; } catch (Exception e) { throw Utils.createException("Client Bearer Jwt Public key", e, logger); diff --git a/openam-oauth2/src/main/java/org/forgerock/openidconnect/restlet/IdTokenInfo.java b/openam-oauth2/src/main/java/org/forgerock/openidconnect/restlet/IdTokenInfo.java index 5efa03387c..8944add862 100644 --- a/openam-oauth2/src/main/java/org/forgerock/openidconnect/restlet/IdTokenInfo.java +++ b/openam-oauth2/src/main/java/org/forgerock/openidconnect/restlet/IdTokenInfo.java @@ -159,7 +159,13 @@ OAuth2Jwt validateIdToken(OAuth2Request request) throws OAuth2Exception, RealmLo final OpenIdConnectClientRegistration clientRegistration = clientRegistrationStore.get(clientId, new ValidateIdTokenRequest(request, realm)); - JwsAlgorithm algorithm = JwsAlgorithm.valueOf(clientRegistration.getIDTokenSignedResponseAlgorithm()); + // A free-text attribute, upper-cased as the token store does when issuing. + final JwsAlgorithm algorithm; + try { + algorithm = JwsAlgorithm.valueOf(clientRegistration.getIDTokenSignedResponseAlgorithm().toUpperCase()); + } catch (IllegalArgumentException e) { + throw new BadRequestException("unsupported id_token_signed_response_alg"); + } boolean requiresClientAuthentication = providerSettingsFactory.get(request).isIdTokenInfoClientAuthenticationEnabled(); diff --git a/openam-oauth2/src/test/java/org/forgerock/openam/oauth2/OpenAMClientRegistrationTest.java b/openam-oauth2/src/test/java/org/forgerock/openam/oauth2/OpenAMClientRegistrationTest.java index 83c0eeb7ee..3084153f77 100644 --- a/openam-oauth2/src/test/java/org/forgerock/openam/oauth2/OpenAMClientRegistrationTest.java +++ b/openam-oauth2/src/test/java/org/forgerock/openam/oauth2/OpenAMClientRegistrationTest.java @@ -584,6 +584,28 @@ public void verifyJwtIdentityReturnsFalseWhenRegisteredKeyCannotVerifyAlgorithm( assertThat(clientRegistration.verifyJwtIdentity(assertion(clientId, ecSigner, JwsAlgorithm.ES256, kid))).isFalse(); } + /** + * RSASigningHandler wraps the JDK's "Bad signature length" in JwsVerifyingException, a sibling of + * JwsSigningException: a client that rotated to a longer key, or any sender choosing the + * signature length, must be invalid_client rather than server_error with a logged stack trace. + */ + @Test + public void verifyJwtIdentityReturnsFalseForRsaSignatureOfWrongLength() throws Exception { + String clientId = "client1"; + String kid = UUID.randomUUID().toString(); + KeyPair registeredKeys = generateRsaKeyPair(2048); + given(amIdentity.getName()).willReturn(clientId); + given(amIdentity.getAttribute("userpassword")).willReturn(singleton("a-client-secret")); + given(amIdentity.getAttribute(IDTOKEN_SIGNED_RESPONSE_ALG)).willReturn(singleton("HS256")); + given(amIdentity.getAttribute(PUBLIC_KEY_SELECTOR)).willReturn(singleton("jwks")); + given(amIdentity.getAttribute(JWKS)).willReturn(singleton(jwks((RSAPublicKey) registeredKeys.getPublic(), kid))); + + KeyPair rotatedKeys = generateRsaKeyPair(3072); + SigningHandler signer = new SigningManager().newRsaSigningHandler((RSAPrivateKey) rotatedKeys.getPrivate()); + + assertThat(clientRegistration.verifyJwtIdentity(assertion(clientId, signer, JwsAlgorithm.RS256, kid))).isFalse(); + } + /** * A symmetric key served at jwks_uri reaches an HMAC handler inside the commons resolver, which * cannot verify an RS256 assertion; that is invalid_client, not server_error. The resolver is @@ -679,8 +701,12 @@ private static OAuth2Jwt assertion(String clientId, SigningHandler signer, JwsAl } private static KeyPair generateRsaKeyPair() throws Exception { + return generateRsaKeyPair(2048); + } + + private static KeyPair generateRsaKeyPair(int bits) throws Exception { KeyPairGenerator gen = KeyPairGenerator.getInstance("RSA"); - gen.initialize(2048); + gen.initialize(bits); return gen.generateKeyPair(); } diff --git a/openam-oauth2/src/test/java/org/forgerock/openidconnect/restlet/IdTokenInfoTest.java b/openam-oauth2/src/test/java/org/forgerock/openidconnect/restlet/IdTokenInfoTest.java index d7dc9aa250..2fe6dcd6d4 100644 --- a/openam-oauth2/src/test/java/org/forgerock/openidconnect/restlet/IdTokenInfoTest.java +++ b/openam-oauth2/src/test/java/org/forgerock/openidconnect/restlet/IdTokenInfoTest.java @@ -132,6 +132,31 @@ public void shouldRejectIdTokenThatVerifiesOnlyAsClientAssertion() throws Except idTokenInfo.validateIdToken(request); } + /** + * id_token_signed_response_alg is a free-text attribute; the token store upper-cases it when + * issuing, so the same spelling must pass the client-authentication gate here. + */ + @Test + public void shouldAcceptLowerCaseConfiguredAlgorithm() throws Exception { + given(request.getParameter(OAuth2Constants.JWTTokenParams.ID_TOKEN)).willReturn(idToken()); + given(clientRegistration.getIDTokenSignedResponseAlgorithm()).willReturn("hs256"); + given(clientRegistration.verifyIdTokenIdentity(any(OAuth2Jwt.class))).willReturn(true); + + OAuth2Jwt result = idTokenInfo.validateIdToken(request); + + assertThat(result.getSignedJwt().getClaimsSet().getAudience()).containsExactly(CLIENT_ID); + } + + @Test(expectedExceptions = BadRequestException.class, + expectedExceptionsMessageRegExp = "unsupported id_token_signed_response_alg") + public void shouldRejectUnknownConfiguredAlgorithmAsBadRequest() throws Exception { + given(request.getParameter(OAuth2Constants.JWTTokenParams.ID_TOKEN)).willReturn(idToken()); + given(clientRegistration.getIDTokenSignedResponseAlgorithm()).willReturn("not-an-alg"); + given(clientRegistration.verifyIdTokenIdentity(any(OAuth2Jwt.class))).willReturn(true); + + idTokenInfo.validateIdToken(request); + } + private static String idToken() { return new JwtBuilderFactory() .jws(new SigningManager().newHmacSigningHandler("secret".getBytes(StandardCharsets.UTF_8)))