diff --git a/modules/gplazma2-oidc/src/main/java/org/dcache/gplazma/oidc/OidcAuthPlugin.java b/modules/gplazma2-oidc/src/main/java/org/dcache/gplazma/oidc/OidcAuthPlugin.java index 8f427b91499..a241bdd59cb 100644 --- a/modules/gplazma2-oidc/src/main/java/org/dcache/gplazma/oidc/OidcAuthPlugin.java +++ b/modules/gplazma2-oidc/src/main/java/org/dcache/gplazma/oidc/OidcAuthPlugin.java @@ -27,6 +27,7 @@ import org.apache.http.client.HttpClient; import org.dcache.auth.BearerTokenCredential; import org.dcache.auth.OAuthProviderPrincipal; +import org.dcache.auth.Origin; import org.dcache.auth.attributes.Restriction; import org.dcache.gplazma.AuthenticationException; import org.dcache.gplazma.oidc.helpers.JsonHttpClient; @@ -58,6 +59,8 @@ public class OidcAuthPlugin implements GPlazmaAuthenticationPlugin { private final static String OIDC_ALLOWED_AUDIENCES = "gplazma.oidc.audience-targets"; private final static String OIDC_PROVIDER_PREFIX = "gplazma.oidc.provider!"; + private final static String SUPPRESS_AUDIENCE_TOKEN = "audience"; + private static final String DEFAULT_PROFILE_NAME = "oidc"; private static final Map PROFILES = Map.of("oidc", new OidcProfileFactory(), "scitokens", new ScitokensProfileFactory(), @@ -95,8 +98,15 @@ private static IdentityProvider createIdentityProvider(String name, String descr .flatMap(v -> Splitter.on(',').trimResults().splitToStream(v)) .collect(Collectors.toList()); - return new IdentityProvider(name, issuer, profile, client, discoveryCacheDuration, + var idp = new IdentityProvider(name, issuer, profile, client, discoveryCacheDuration, suppress); + if (idp.isSuppressed(SUPPRESS_AUDIENCE_TOKEN)) { + LOG.warn("Audience (\"aud\") checking is suppressed for OP {}. This makes dCache " + + "compatible with behaviour before version 8.2.0, but it also violates RFC " + + "\"MUST\" requirements and may have security implications.", + idp.getName()); + } + return idp; } catch (URISyntaxException e) { throw new IllegalArgumentException( "Invalid endpoint " + endpoint + ": " + e.getMessage()); @@ -186,7 +196,7 @@ public void authenticate(Set publicCredentials, Set privateCrede try { ExtractResult result = tokenProcessor.extract(token); checkAuthentication(!result.claims().isEmpty(), "processing token yielded no claims"); - checkAudience(result.claims()); + checkAudience(result, identifiedPrincipals); var idp = result.idp(); identifiedPrincipals.add(new OAuthProviderPrincipal(idp.getName())); @@ -220,22 +230,73 @@ private static void checkValid(String token) throws AuthenticationException { } } - private void checkAudience(Map claims) throws AuthenticationException { + private void checkAudience(ExtractResult result, Set principals) + throws AuthenticationException { + var claims = result.claims(); var audClaim = claims.get("aud"); if (audClaim == null) { return; } + var idp = result.idp(); + boolean suppressAudience = idp.isSuppressed(SUPPRESS_AUDIENCE_TOKEN); + if (audClaim.isArray()) { List audClaimAsList = new ArrayList<>(audClaim.size()); audClaim.elements().forEachRemaining(e -> audClaimAsList.add(e.textValue())); - checkAuthentication(audClaimAsList.stream().anyMatch(audienceTargets::contains), - "intended for %s", audClaimAsList); + + if (!audClaimAsList.stream().anyMatch(audienceTargets::contains)) { + if (suppressAudience) { + logAudienceSuppression(audClaimAsList.toString(), idp.getName(), claims, + principals, "one of these audiences"); + return; + } + throw new AuthenticationException("intended for " + audClaimAsList); + } } else { String aud = audClaim.textValue(); - checkAuthentication(audienceTargets.contains(aud), "intended for %s", aud); + if (!audienceTargets.contains(aud)) { + if (suppressAudience) { + logAudienceSuppression(aud, idp.getName(), claims, principals, "this audience"); + return; + } + throw new AuthenticationException("intended for " + aud); + } + } + } + + private static void logAudienceSuppression(String audience, String op, + Map claims, Set principals, String whatToAdd) { + Optional clientIPAddress = principals.stream() + .filter(Origin.class::isInstance) + .map(Origin.class::cast) + .findFirst() + .map(Principal::getName); + + LOG.warn("Accepting token with an incompatible audience \"{}\" issued by OP {} to {}{}. " + + "To prevent similar warnings, either add {} to the 'gplazma.oidc.audience-targets' " + + "configuration property or update the client to use an audience value already defined " + + "in this configuration property.", audience, op, + clientId(claims).map(id -> "client \"" + id + "\"").orElse("an unknown client"), + clientIPAddress.map(addr -> " connecting from " + addr).orElse(""), + whatToAdd); + } + + private static Optional clientId(Map claims) { + var clientIdClaim = claims.get("client_id"); + if (clientIdClaim != null && clientIdClaim.isTextual()) { + return Optional.of(clientIdClaim.textValue()); } + + // Some OPs (*cough* EGI CheckIn *cough*) violate RFC 9068 by issuing JWTs without a + // 'client_id' claim; however, they do provide an 'azp' claim. + var azpClaim = claims.get("azp"); + if (azpClaim != null && azpClaim.isTextual()) { + return Optional.of(azpClaim.textValue()); + } + + return Optional.empty(); } private static Collection nullToEmpty(final Collection collection) { diff --git a/modules/gplazma2-oidc/src/test/java/org/dcache/gplazma/oidc/OidcAuthPluginTest.java b/modules/gplazma2-oidc/src/test/java/org/dcache/gplazma/oidc/OidcAuthPluginTest.java index 99b834b530b..ed36afb82d5 100644 --- a/modules/gplazma2-oidc/src/test/java/org/dcache/gplazma/oidc/OidcAuthPluginTest.java +++ b/modules/gplazma2-oidc/src/test/java/org/dcache/gplazma/oidc/OidcAuthPluginTest.java @@ -567,6 +567,27 @@ public void shouldRejectSingleAuthClaimWithNoConfiguredAudiences() throws Except when(invoked().withBearerToken("FOO")); } + @Test + public void shouldAcceptSingleAuthClaimWithNoMatchingAudiencesIfAudienceSuppressed() throws Exception { + var profile = aProfile().thatReturns(aProfileResult() + .withPrincipals(Collections.singleton(new OidcSubjectPrincipal("sub-claim-value", "MY-OP")))) + .build(); + var op = anIp("MY-OP").withProfile(profile).withSuppress("audience").build(); + var claims = claims() + .withStringClaim("sub", "sub-claim-value") + .withStringClaim("aud", "another-service.example.org") + .build(); + given(aPlugin() + .withProperty("gplazma.oidc.audience-targets", "dcache.example.org") + .withTokenProcessor(aTokenProcessor().thatReturns(aResult().from(op).containing(claims)))); + + when(invoked().withBearerToken("FOO")); + + verify(processor).extract(eq("FOO")); + verify(profile).processClaims(eq(op), eq(claims)); + assertThat(principals, hasItem(new OidcSubjectPrincipal("sub-claim-value", "MY-OP"))); + } + @Test(expected=AuthenticationException.class) public void shouldRejectSingleAuthClaimWithDifferentConfiguredAudience() throws Exception { given(aPlugin() diff --git a/skel/share/defaults/gplazma.properties b/skel/share/defaults/gplazma.properties index 987de73441c..14c3f3fa6e3 100644 --- a/skel/share/defaults/gplazma.properties +++ b/skel/share/defaults/gplazma.properties @@ -382,6 +382,30 @@ gplazma.htpasswd.file.cache-period.unit = SECONDS # # The following suppress keywords are supported: # +# audience Suppress audience ("aud") claim verification. By +# default, dCache will check that the "aud" claim, if +# present, matches one of the identities configured via +# the 'gplazma.oidc.audience-targets' configuration +# property. The token is rejected if there is no +# match. This option suppresses this check, resulting +# in dCache accepting tokens with an arbitrary "aud" +# claim value. +# +# In general, audience verification provides a form of +# "damage limitation" if a token is misappropriated. +# The stolen token may only be used against the token's +# intended target service and not against any other +# service that the user is authorised. +# +# Suppressing audience verification may be needed as +# earlier versions of dCache lacked audience +# verification. Existing clients may be obtaining +# tokens with inappropriate audience fields. It is +# recommended that this option is used sparingly and +# only for a short time; i.e., a strategy is devised +# through which clients are updated and this +# suppression may be disabled. +# # offline Suppress offline validation. By default, dCache will # attempt to validate a JWT using the OP's public keys. # If the token is valid then the claims stored within