HTML Injection in Keycloak Admin REST API (#16765)

Resolves #GHSA-m4fv-gm5m-4725

Co-authored-by: Martin Bartoš <mabartos@redhat.com>
This commit is contained in:
Stian Thorgersen 2023-02-01 14:34:15 +01:00 committed by GitHub
parent e3ccba3903
commit d9025231f9
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
9 changed files with 118 additions and 6 deletions

View file

@ -16,10 +16,12 @@
*/
package org.keycloak.authentication.actiontoken.execactions;
import org.keycloak.TokenVerifier;
import org.keycloak.TokenVerifier.Predicate;
import org.keycloak.authentication.RequiredActionFactory;
import org.keycloak.authentication.RequiredActionProvider;
import org.keycloak.authentication.actiontoken.*;
import org.keycloak.authentication.requiredactions.util.RequiredActionsValidator;
import org.keycloak.events.Errors;
import org.keycloak.events.EventType;
import org.keycloak.forms.login.LoginFormsProvider;
@ -66,7 +68,8 @@ public class ExecuteActionsActionTokenHandler extends AbstractActionTokenHandler
Messages.INVALID_REDIRECT_URI
),
verifyEmail(tokenContext)
verifyEmail(tokenContext),
verifyRequiredActions(tokenContext)
);
}
@ -129,5 +132,10 @@ public class ExecuteActionsActionTokenHandler extends AbstractActionTokenHandler
.noneMatch(RequiredActionFactory::isOneTimeAction);
}
// Verify required actions included in the token are valid
protected TokenVerifier.Predicate<ExecuteActionsActionToken> verifyRequiredActions(ActionTokenContext<ExecuteActionsActionToken> tokenContext) {
return TokenUtils.checkThat(t -> RequiredActionsValidator.validRequiredActions(tokenContext.getSession(), t.getRequiredActions()),
Errors.RESOLVE_REQUIRED_ACTIONS, Messages.INVALID_TOKEN_REQUIRED_ACTIONS
);
}
}

View file

@ -0,0 +1,43 @@
/*
* Copyright 2022 Red Hat, Inc. and/or its affiliates
* and other contributors as indicated by the @author tags.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.keycloak.authentication.requiredactions.util;
import org.keycloak.authentication.RequiredActionProvider;
import org.keycloak.models.KeycloakSession;
import org.keycloak.models.KeycloakSessionFactory;
import java.util.List;
public class RequiredActionsValidator {
/**
* Validate provided required actions
*
* @param session the {@code KeycloakSession}
* @param requiredActions IDs of tested required actions
*/
public static boolean validRequiredActions(KeycloakSession session, List<String> requiredActions) {
final KeycloakSessionFactory sessionFactory = session.getKeycloakSessionFactory();
for (String action : requiredActions) {
if (sessionFactory.getProviderFactory(RequiredActionProvider.class, action) == null) {
return false;
}
}
return true;
}
}

View file

@ -219,6 +219,8 @@ public class Messages {
public static final String MISSING_IDENTITY_PROVIDER = "missingIdentityProviderMessage";
public static final String INVALID_TOKEN_REQUIRED_ACTIONS = "invalidTokenRequiredActions";
public static final String INVALID_FEDERATED_IDENTITY_ACTION = "invalidFederatedIdentityActionMessage";
public static final String FEDERATED_IDENTITY_NOT_ACTIVE = "federatedIdentityLinkNotActiveMessage";

View file

@ -20,8 +20,10 @@ import org.jboss.logging.Logger;
import org.jboss.resteasy.annotations.cache.NoCache;
import org.keycloak.authentication.RequiredActionProvider;
import org.keycloak.authentication.actiontoken.execactions.ExecuteActionsActionToken;
import org.keycloak.authentication.requiredactions.util.RequiredActionsValidator;
import org.keycloak.common.ClientConnection;
import org.keycloak.common.Profile;
import org.keycloak.common.util.CollectionUtil;
import org.keycloak.common.util.Time;
import org.keycloak.credential.CredentialModel;
import org.keycloak.email.EmailException;
@ -758,7 +760,7 @@ public class UserResource {
/**
* Send a update account email to the user
* Send an email to the user with a link they can click to execute particular actions.
*
* An email contains a link the user can click to perform a set of required actions.
* The redirectUri and clientId parameters are optional. If no redirect is given, then there will
@ -768,7 +770,7 @@ public class UserResource {
* @param redirectUri Redirect uri
* @param clientId Client id
* @param lifespan Number of seconds after which the generated token expires
* @param actions required actions the user needs to complete
* @param actions Required actions the user needs to complete
* @return
*/
@Path("execute-actions-email")
@ -798,6 +800,11 @@ public class UserResource {
clientId = Constants.ACCOUNT_MANAGEMENT_CLIENT_ID;
}
if (CollectionUtil.isNotEmpty(actions) && !RequiredActionsValidator.validRequiredActions(session, actions)) {
throw new WebApplicationException(
ErrorResponse.error("Provided invalid required actions", Status.BAD_REQUEST));
}
ClientModel client = realm.getClientByClientId(clientId);
if (client == null) {
logger.debugf("Client %s doesn't exist", clientId);

View file

@ -41,6 +41,7 @@ import org.keycloak.testsuite.AbstractTestRealmKeycloakTest;
import org.keycloak.testsuite.admin.ApiUtil;
import org.keycloak.testsuite.arquillian.AuthServerTestEnricher;
import org.keycloak.testsuite.arquillian.annotation.DisableFeature;
import org.keycloak.testsuite.arquillian.annotation.EnableFeature;
import org.keycloak.testsuite.auth.page.AuthRealm;
import org.keycloak.testsuite.cluster.AuthenticationSessionFailoverClusterTest;
import org.keycloak.testsuite.pages.AppPage;
@ -66,6 +67,7 @@ import java.io.IOException;
import java.util.Arrays;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import org.hamcrest.Matchers;
@ -74,9 +76,11 @@ import org.openqa.selenium.By;
import org.openqa.selenium.WebDriver;
import org.openqa.selenium.WebElement;
import static org.hamcrest.CoreMatchers.notNullValue;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.core.Is.is;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.fail;
/**
* @author <a href="mailto:sthorger@redhat.com">Stian Thorgersen</a>
@ -1027,4 +1031,30 @@ public class RequiredActionEmailVerificationTest extends AbstractTestRealmKeyclo
errorPage.assertCurrent();
assertEquals("The link you clicked is an old stale link and is no longer valid. Maybe you have already verified your email.", errorPage.getError());
}
@Test
@EnableFeature(value = Profile.Feature.UPDATE_EMAIL, skipRestart = true)
public void actionTokenWithInvalidRequiredActions() throws IOException {
// Send email with required action
testRealm().users().get(testUserId).executeActionsEmail(List.of(RequiredAction.UPDATE_EMAIL.name()));
Assert.assertEquals(1, greenMail.getReceivedMessages().length);
MimeMessage message = greenMail.getLastReceivedMessage();
MailUtils.EmailBody body = MailUtils.getBody(message);
assertThat(body, notNullValue());
final String link = MailUtils.getLink(body.getText());
assertThat(link, notNullValue());
// Disable feature and the required action UPDATE_EMAIL provider is not present
testingClient.disableFeature(Profile.Feature.UPDATE_EMAIL);
driver.navigate().to(link);
errorPage.assertCurrent();
// Required action included in the action token is not valid anymore, because we don't know the provider for it
assertThat(errorPage.getError(), is("Required actions included in the link are not valid"));
}
}

View file

@ -1559,7 +1559,25 @@ public class UserTest extends AbstractAdminTest {
userRep.setEnabled(true);
updateUser(user, userRep);
user.executeActionsEmail("invalidClientId", "invalidUri", actions);
user.executeActionsEmail(Arrays.asList(
UserModel.RequiredAction.UPDATE_PASSWORD.name(),
"invalid\"<img src=\"alert(0)\">")
);
fail("Expected failure");
} catch (ClientErrorException e) {
assertEquals(400, e.getResponse().getStatus());
ErrorRepresentation error = e.getResponse().readEntity(ErrorRepresentation.class);
Assert.assertEquals("Provided invalid required actions", error.getErrorMessage());
assertAdminEvents.assertEmpty();
}
try {
user.executeActionsEmail(
"invalidClientId",
"invalidUri",
Collections.singletonList(UserModel.RequiredAction.UPDATE_PASSWORD.name())
);
fail("Expected failure");
} catch (ClientErrorException e) {
assertEquals(400, e.getResponse().getStatus());

View file

@ -341,6 +341,8 @@ requiredAction.UPDATE_PASSWORD=Aktualizace hesla
requiredAction.UPDATE_PROFILE=Aktualizovat profil
requiredAction.VERIFY_EMAIL=Ověřit e-mail
invalidTokenRequiredActions=Požadované akce obsažené v daném odkazu nejsou validní
doX509Login=Budete přihlášeni jako\:
clientCertificate=Klientský X509 certifikát\:
noCertificate=[Žádný certifikát]

View file

@ -8,7 +8,7 @@
</#if>
<#elseif section = "form">
<div id="kc-info-message">
<p class="instruction">${message.summary}<#if requiredActions??><#list requiredActions>: <b><#items as reqActionItem>${msg("requiredAction.${reqActionItem}")}<#sep>, </#items></b></#list><#else></#if></p>
<p class="instruction">${message.summary}<#if requiredActions??><#list requiredActions>: <b><#items as reqActionItem>${kcSanitize(msg("requiredAction.${reqActionItem}"))?no_esc}<#sep>, </#items></b></#list><#else></#if></p>
<#if skipLink??>
<#else>
<#if pageRedirectUri?has_content>

View file

@ -394,6 +394,8 @@ requiredAction.VERIFY_EMAIL=Verify Email
requiredAction.CONFIGURE_RECOVERY_AUTHN_CODES=Generate Recovery Codes
requiredAction.webauthn-register-passwordless=Webauthn Register Passwordless
invalidTokenRequiredActions=Required actions included in the link are not valid
doX509Login=You will be logged in as\:
clientCertificate=X509 client certificate\:
noCertificate=[No Certificate]