libre.sh/keycloak-scim
3
0
Fork 0
Code Issues 15 Pull requests Releases Packages Activity Actions

KEYCLOAK-7667 Fix namespace handling when decrypting assertion

Browse source
This commit is contained in:
Hynek Mlnarik 2018-06-20 13:55:51 +02:00 committed by Hynek Mlnařík
parent 1277f8bb3b
commit 6b968796ce
6 changed files with 69 additions and 30 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
adapters/saml/core/src/main/java/org/keycloak/adapters/saml/profile/AbstractSamlAuthenticationHandler.java
View file

@ -339,7 +339,7 @@ public abstract class AbstractSamlAuthenticationHandler implements SamlAuthentic
return AuthOutcome.FAILED;
}
try {
assertion = AssertionUtil.getAssertion(responseType, deployment.getDecryptionKey());
assertion = AssertionUtil.getAssertion(responseHolder, responseType, deployment.getDecryptionKey());
if (AssertionUtil.hasExpired(assertion)) {
return initiateLogin();
}

11
saml-core/src/main/java/org/keycloak/saml/processing/core/saml/v2/util/AssertionUtil.java
View file

@ -67,6 +67,7 @@ import java.util.Set;
import org.keycloak.rotation.HardcodedKeyLocator;
import org.keycloak.saml.common.constants.GeneralConstants;
import org.keycloak.saml.processing.core.saml.v2.common.SAMLDocumentHolder;
/**
* Utility to deal with assertions
@ -552,7 +553,7 @@ public class AssertionUtil {
return roles;
}
public static AssertionType getAssertion(ResponseType responseType, PrivateKey privateKey) throws ParsingException, ProcessingException, ConfigurationException {
public static AssertionType getAssertion(SAMLDocumentHolder holder, ResponseType responseType, PrivateKey privateKey) throws ParsingException, ProcessingException, ConfigurationException {
List<ResponseType.RTChoiceType> assertions = responseType.getAssertions();
if (assertions.isEmpty()) {
@ -566,7 +567,7 @@ public class AssertionUtil {
if (privateKey == null) {
throw new ProcessingException("Encryptd assertion and decrypt private key is null");
}
decryptAssertion(responseType, privateKey);
decryptAssertion(holder, responseType, privateKey);
}
return responseType.getAssertions().get(0).getAssertion();
@ -588,10 +589,8 @@ public class AssertionUtil {
* @param responseType a response containg an encrypted assertion
* @return the assertion element as it was decrypted. This can be used in signature verification.
*/
public static Element decryptAssertion(ResponseType responseType, PrivateKey privateKey) throws ParsingException, ProcessingException, ConfigurationException {
SAML2Response saml2Response = new SAML2Response();
Document doc = saml2Response.convert(responseType);
public static Element decryptAssertion(SAMLDocumentHolder holder, ResponseType responseType, PrivateKey privateKey) throws ParsingException, ProcessingException, ConfigurationException {
Document doc = holder.getSamlDocument();
Element enc = DocumentUtil.getElement(doc, new QName(JBossSAMLConstants.ENCRYPTED_ASSERTION.get()));
if (enc == null) {

10
saml-core/src/test/java/org/keycloak/saml/processing/core/parsers/saml/SAMLParserTest.java
View file

@ -67,7 +67,9 @@ import org.keycloak.saml.common.constants.JBossSAMLURIConstants;
import org.keycloak.saml.common.exceptions.ConfigurationException;
import org.keycloak.saml.common.exceptions.ParsingException;
import org.keycloak.saml.common.exceptions.ProcessingException;
import org.keycloak.saml.processing.api.saml.v2.request.SAML2Request;
import org.keycloak.saml.processing.api.saml.v2.response.SAML2Response;
import org.keycloak.saml.processing.core.saml.v2.common.SAMLDocumentHolder;
import org.keycloak.saml.processing.core.saml.v2.util.AssertionUtil;
import org.keycloak.saml.processing.core.saml.v2.util.XMLTimeUtil;
import org.w3c.dom.Element;
@ -137,6 +139,8 @@ public class SAMLParserTest {
Object parsedObject;
if (SAML2Object.class.isAssignableFrom(expectedType)) {
parsedObject = new SAML2Response().getSAML2ObjectFromStream(st);
} else if (SAMLDocumentHolder.class.isAssignableFrom(expectedType)) {
parsedObject = SAML2Request.getSAML2ObjectFromStream(st);
} else {
parsedObject = parser.parse(st);
}
@ -185,7 +189,9 @@ public class SAMLParserTest {
@Test
public void testSaml20EncryptedAssertionWithNewlines() throws Exception {
ResponseType resp = assertParsed("KEYCLOAK-4489-encrypted-assertion-with-newlines.xml", ResponseType.class);
SAMLDocumentHolder holder = assertParsed("KEYCLOAK-4489-encrypted-assertion-with-newlines.xml", SAMLDocumentHolder.class);
assertThat(holder.getSamlObject(), instanceOf(ResponseType.class));
ResponseType resp = (ResponseType) holder.getSamlObject();
assertThat(resp.getAssertions().size(), is(1));
ResponseType.RTChoiceType rtChoiceType = resp.getAssertions().get(0);
@ -193,7 +199,7 @@ public class SAMLParserTest {
assertNotNull(rtChoiceType.getEncryptedAssertion());
PrivateKey privateKey = DerUtils.decodePrivateKey(Base64.decode(PRIVATE_KEY));
AssertionUtil.decryptAssertion(resp, privateKey);
AssertionUtil.decryptAssertion(holder, resp, privateKey);
rtChoiceType = resp.getAssertions().get(0);
assertNotNull(rtChoiceType.getAssertion());

2
services/src/main/java/org/keycloak/broker/saml/SAMLEndpoint.java
View file

@ -366,7 +366,7 @@ public class SAMLEndpoint {
if (assertionIsEncrypted) {
// This methods writes the parsed and decrypted assertion back on the responseType parameter:
assertionElement = AssertionUtil.decryptAssertion(responseType, keys.getPrivateKey());
assertionElement = AssertionUtil.decryptAssertion(holder, responseType, keys.getPrivateKey());
} else {
/* We verify the assertion using original document to handle cases where the IdP
includes whitespace and/or newlines inside tags. */

4
testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/AbstractBrokerTest.java
View file

@ -65,6 +65,9 @@ public abstract class AbstractBrokerTest extends AbstractBaseBrokerTest {
resetUserPassword(realmResource.users().get(userId), bc.getUserPassword(), false);
if (testContext.isInitialized()) {
if (identityProviderResource == null) {
identityProviderResource = (IdentityProviderResource) testContext.getCustomValue("identityProviderResource");
}
return;
}
@ -72,6 +75,7 @@ public abstract class AbstractBrokerTest extends AbstractBaseBrokerTest {
RealmResource realm = adminClient.realm(bc.consumerRealmName());
realm.identityProviders().create(bc.setUpIdentityProvider(suiteContext)).close();
identityProviderResource = realm.identityProviders().get(bc.getIDPAlias());
testContext.setCustomValue("identityProviderResource", identityProviderResource);
// addClients
List<ClientRepresentation> clients = bc.createProviderClients(suiteContext);

70
testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/KcSamlSignedBrokerTest.java
View file

@ -36,7 +36,6 @@ import org.w3c.dom.NamedNodeMap;
import org.w3c.dom.Node;
import org.w3c.dom.NodeList;
import static org.keycloak.testsuite.broker.BrokerTestConstants.*;
import static org.keycloak.testsuite.broker.BrokerTestTools.encodeUrl;
import static org.keycloak.testsuite.util.Matchers.isSamlResponse;
public class KcSamlSignedBrokerTest extends KcSamlBrokerTest {
@ -113,8 +112,7 @@ public class KcSamlSignedBrokerTest extends KcSamlBrokerTest {
return new KcSamlSignedBrokerConfiguration();
}
@Test
public void testSignedEncryptedAssertions() throws Exception {
public void withSignedEncryptedAssertions(Runnable testBody, boolean signedAssertion, boolean encryptedAssertion) throws Exception {
ClientRepresentation client = adminClient.realm(bc.providerRealmName())
.clients()
.findByClientId(bc.getIDPClientIdInProviderRealm(suiteContext))
@ -130,33 +128,46 @@ public class KcSamlSignedBrokerTest extends KcSamlBrokerTest {
Assert.assertThat(consumerCert, Matchers.notNullValue());
try (Closeable idpUpdater = new IdentityProviderAttributeUpdater(identityProviderResource)
.setAttribute(SAMLIdentityProviderConfig.VALIDATE_SIGNATURE, "true")
.setAttribute(SAMLIdentityProviderConfig.WANT_ASSERTIONS_SIGNED, "true")
.setAttribute(SAMLIdentityProviderConfig.WANT_ASSERTIONS_ENCRYPTED, "true")
.setAttribute(SAMLIdentityProviderConfig.VALIDATE_SIGNATURE, Boolean.toString(signedAssertion))
.setAttribute(SAMLIdentityProviderConfig.WANT_ASSERTIONS_SIGNED, Boolean.toString(signedAssertion))
.setAttribute(SAMLIdentityProviderConfig.WANT_ASSERTIONS_ENCRYPTED, Boolean.toString(encryptedAssertion))
.setAttribute(SAMLIdentityProviderConfig.WANT_AUTHN_REQUESTS_SIGNED, "false")
.setAttribute(SAMLIdentityProviderConfig.SIGNING_CERTIFICATE_KEY, providerCert)
.update();
Closeable clientUpdater = new ClientAttributeUpdater(clientResource)
.setAttribute(SamlConfigAttributes.SAML_ENCRYPT, "true")
.setAttribute(SamlConfigAttributes.SAML_ENCRYPT, Boolean.toString(encryptedAssertion))
.setAttribute(SamlConfigAttributes.SAML_ENCRYPTION_CERTIFICATE_ATTRIBUTE, consumerCert)
.setAttribute(SamlConfigAttributes.SAML_SERVER_SIGNATURE, "false") // only sign assertions
.setAttribute(SamlConfigAttributes.SAML_ASSERTION_SIGNATURE, "true")
.setAttribute(SamlConfigAttributes.SAML_ASSERTION_SIGNATURE, Boolean.toString(signedAssertion))
.setAttribute(SamlConfigAttributes.SAML_CLIENT_SIGNATURE_ATTRIBUTE, "false")
.update())
{
// Login should pass because assertion is signed.
loginUser();
// Logout should fail because logout response is not signed.
driver.navigate().to(BrokerTestTools.getAuthRoot(suiteContext)
+ "/auth/realms/" + bc.providerRealmName()
+ "/protocol/" + "openid-connect"
+ "/logout?redirect_uri=" + encodeUrl(getAccountUrl(bc.providerRealmName())));
errorPage.assertCurrent();
testBody.run();
}
}
@Test
public void testSignedEncryptedAssertions() throws Exception {
withSignedEncryptedAssertions(this::testAssertionSignatureRespected, true, true);
}
@Test
public void testSignedAssertion() throws Exception {
withSignedEncryptedAssertions(this::testAssertionSignatureRespected, true, false);
}
private void testAssertionSignatureRespected() {
// Login should pass because assertion is signed.
loginUser();
// Logout should fail because logout response is not signed.
final String redirectUri = getAccountUrl(bc.providerRealmName());
final String logoutUri = oauth.realm(bc.providerRealmName()).getLogoutUrl().redirectUri(redirectUri).build();
driver.navigate().to(logoutUri);
errorPage.assertCurrent();
}
private Document extractNamespacesToTopLevelElement(Document original) {
HashMap<String, String> namespaces = new HashMap<>();
enumerateAndRemoveNamespaces(original.getDocumentElement(), namespaces);
@ -202,10 +213,15 @@ public class KcSamlSignedBrokerTest extends KcSamlBrokerTest {
// KEYCLOAK-5581
@Test
public void loginUserAllNamespacesInTopElement() throws Exception {
public void loginUserAllNamespacesInTopElement() {
AuthnRequestType loginRep = SamlClient.createLoginRequestDocument(AbstractSamlTest.SAML_CLIENT_ID_SALES_POST, AbstractSamlTest.SAML_ASSERTION_CONSUMER_URL_SALES_POST, null);
Document doc = extractNamespacesToTopLevelElement(SAML2Request.convert(loginRep));
Document doc;
try {
doc = extractNamespacesToTopLevelElement(SAML2Request.convert(loginRep));
} catch (Exception ex) {
throw new RuntimeException(ex);
}
SAMLDocumentHolder samlResponse = new SamlClientBuilder()
.authnRequest(getAuthServerSamlEndpoint(bc.consumerRealmName()), doc, Binding.POST).build() // Request to consumer IdP
@ -232,4 +248,18 @@ public class KcSamlSignedBrokerTest extends KcSamlBrokerTest {
Assert.assertThat(samlResponse.getSamlObject(), isSamlResponse(JBossSAMLURIConstants.STATUS_SUCCESS));
}
@Test
public void loginUserAllNamespacesInTopElementSignedEncryptedAssertion() throws Exception {
withSignedEncryptedAssertions(this::loginUserAllNamespacesInTopElement, true, true);
}
@Test
public void loginUserAllNamespacesInTopElementSignedAssertion() throws Exception {
withSignedEncryptedAssertions(this::loginUserAllNamespacesInTopElement, true, false);
}
@Test
public void loginUserAllNamespacesInTopElementEncryptedAssertion() throws Exception {
withSignedEncryptedAssertions(this::loginUserAllNamespacesInTopElement, false, true);
}
}