KEYCLOAK-5318 Verify signature on raw query parameters (#4445)
This commit is contained in:
parent
7191048f91
commit
e36b94d905
5 changed files with 71 additions and 10 deletions
|
@ -69,7 +69,7 @@ public class JaxrsSAML2BindingBuilder extends BaseSAML2BindingBuilder<JaxrsSAML2
|
|||
|
||||
private Response response(String redirectUri, boolean asRequest) throws ProcessingException, ConfigurationException, IOException {
|
||||
URI uri = generateURI(redirectUri, asRequest);
|
||||
if (logger.isDebugEnabled()) logger.trace("redirect-binding uri: " + uri.toString());
|
||||
logger.tracef("redirect-binding uri: %s", uri);
|
||||
CacheControl cacheControl = new CacheControl();
|
||||
cacheControl.setNoCache(true);
|
||||
return Response.status(302).location(uri)
|
||||
|
|
|
@ -128,6 +128,7 @@ public class SamlProtocolUtils {
|
|||
String request = encodedParams.getFirst(paramKey);
|
||||
String algorithm = encodedParams.getFirst(GeneralConstants.SAML_SIG_ALG_REQUEST_KEY);
|
||||
String signature = encodedParams.getFirst(GeneralConstants.SAML_SIGNATURE_REQUEST_KEY);
|
||||
String relayState = encodedParams.getFirst(GeneralConstants.RELAY_STATE);
|
||||
String decodedAlgorithm = uriInformation.getQueryParameters(true).getFirst(GeneralConstants.SAML_SIG_ALG_REQUEST_KEY);
|
||||
|
||||
if (request == null) throw new VerificationException("SAM was null");
|
||||
|
@ -139,13 +140,12 @@ public class SamlProtocolUtils {
|
|||
// Shibboleth doesn't sign the document for redirect binding.
|
||||
// todo maybe a flag?
|
||||
|
||||
UriBuilder builder = UriBuilder.fromPath("/")
|
||||
.queryParam(paramKey, request);
|
||||
StringBuilder rawQueryBuilder = new StringBuilder().append(paramKey).append("=").append(request);
|
||||
if (encodedParams.containsKey(GeneralConstants.RELAY_STATE)) {
|
||||
builder.queryParam(GeneralConstants.RELAY_STATE, encodedParams.getFirst(GeneralConstants.RELAY_STATE));
|
||||
rawQueryBuilder.append("&" + GeneralConstants.RELAY_STATE + "=").append(relayState);
|
||||
}
|
||||
builder.queryParam(GeneralConstants.SAML_SIG_ALG_REQUEST_KEY, algorithm);
|
||||
String rawQuery = builder.build().getRawQuery();
|
||||
rawQueryBuilder.append("&" + GeneralConstants.SAML_SIG_ALG_REQUEST_KEY + "=").append(algorithm);
|
||||
String rawQuery = rawQueryBuilder.toString();
|
||||
|
||||
try {
|
||||
byte[] decodedSignature = RedirectBindingUtil.urlBase64Decode(signature);
|
||||
|
|
|
@ -257,6 +257,11 @@ public class SamlService extends AuthorizationEndpointBase {
|
|||
protected Response loginRequest(String relayState, AuthnRequestType requestAbstractType, ClientModel client) {
|
||||
SamlClient samlClient = new SamlClient(client);
|
||||
// validate destination
|
||||
if (requestAbstractType.getDestination() == null && samlClient.requiresClientSignature()) {
|
||||
event.detail(Details.REASON, "invalid_destination");
|
||||
event.error(Errors.INVALID_SAML_AUTHN_REQUEST);
|
||||
return ErrorPage.error(session, Messages.INVALID_REQUEST);
|
||||
}
|
||||
if (! isValidDestination(requestAbstractType.getDestination())) {
|
||||
event.detail(Details.REASON, "invalid_destination");
|
||||
event.error(Errors.INVALID_SAML_AUTHN_REQUEST);
|
||||
|
|
|
@ -29,10 +29,11 @@ public abstract class AbstractSamlTest extends AbstractAuthTest {
|
|||
protected static final String SAML_ASSERTION_CONSUMER_URL_SALES_POST2 = "http://localhost:8080/sales-post2/";
|
||||
protected static final String SAML_CLIENT_ID_SALES_POST2 = "http://localhost:8081/sales-post2/";
|
||||
|
||||
protected static final String SAML_ASSERTION_CONSUMER_URL_SALES_POST_SIG = "http://localhost:8081/sales-post-sig/";
|
||||
protected static final String SAML_ASSERTION_CONSUMER_URL_SALES_POST_SIG = "http://localhost:8080/sales-post-sig/";
|
||||
protected static final String SAML_CLIENT_ID_SALES_POST_SIG = "http://localhost:8081/sales-post-sig/";
|
||||
protected static final String SAML_CLIENT_ID_SALES_POST_SIG_PRIVATE_KEY = "MIICdwIBADANBgkqhkiG9w0BAQEFAASCAmEwggJdAgEAAoGBAL72RWA8WmyGNLZpPaDnaWpO/aWpvWOC9dlWTuIUXgsA7pJ0JcpdFkjhK7MhGRU2kTVrjZXWRBGVqjOgWSWC9fmyN7y2Slr3KWr5vEZcz1FHJypG7YH6hZRXsCqrsGzRe53RZ4dBhFwSgKOqwUobLrmqQtEN1PvQVMmClIe2z6APAgMBAAECgYA1TV5+BzqiMi/Cfsux/wYAo33PYPq5LRPcj2fDWTYK0j7FaGAoBSW0QA3HmUR8FFgh1hyWJ1GmquTwNiDMBKsNhMdfOpUEl8CTVWsAamrJGr5M0wtQeReJOJIM5DpaqFf3dQYyYCBCbVGwtrr5/LUE78HbLzn5h/Y5TKWwpLpq8QJBAOyVEOI0V20SN2/I4vbS1Dv5b54pXFXEoDJDeAWMggOLMQtm34mrVbPpdX02ErUkW2VW6B4PW2Vb/4rXp8SYNhcCQQDOoqg38l4ZvbSt9CV0BRZ1BtvSOtABiWChJT/19gf1hUlPy+eR+E8FPAjD0f165/X/+VE/+MeW0d3lyHjbfRjJAkEA3es6SiW0+IAE9lum4saDBLsHA4JitaVaa6u0EuhpMK/JUpuuBfJs0vWkGs61H6u5+8ZYt5HKNrrkazW9joEFAwJBAJOw2r8yMmP/nbZ/vI1SXZzDjDaU5rtSb4h+UVsBwOqRm7a3LQq+CezZ3gHog15nkQKmNpacwDtiQVHNmeR3Y1ECQHyURjnpJyc2KChmSSO0RR3Z6N2Fk9i5L1Ip+/3MfA34j4MTa9UVnPwY+r+ItwzHXc+Rlphok1DgW1Bj+FEtKfk=";
|
||||
protected static final String SAML_CLIENT_ID_SALES_POST_SIG_PUBLIC_KEY = "MIGfMA0GCSqGSIb3DQEBAQUAA4GNADCBiQKBgQC+9kVgPFpshjS2aT2g52lqTv2lqb1jgvXZVk7iFF4LAO6SdCXKXRZI4SuzIRkVNpE1a42V1kQRlaozoFklgvX5sje8tkpa9ylq+bxGXM9RRycqRu2B+oWUV7Aqq7Bs0Xud0WeHQYRcEoCjqsFKGy65qkLRDdT70FTJgpSHts+gDwIDAQAB";
|
||||
protected static final String SAML_URL_SALES_POST_SIG = "http://localhost:8080/sales-post-sig/";
|
||||
protected static final String SAML_CLIENT_SALES_POST_SIG_PRIVATE_KEY = "MIICdQIBADANBgkqhkiG9w0BAQEFAASCAl8wggJbAgEAAoGBANUbxrvEY3pkiQNt55zJLKBwN+zKmNQw08ThAmOKzwHfXoK+xlDSFxNMtTKJGkeUdnKzaTfESEcEfKYULUA41y/NnOlvjS0CEsc7Wq0Ce63TSSGMB2NHea4tV0aQz/MwLsbmz2IjAFWHA5CHL5WwacIf3UTOSNnhJUSvnkomjJAlAgMBAAECgYANpO2gb/5+g5lSIuNFYov86bJq8r2+ODIW1OE2Rljioc6HSHeiDRF1JuAjECwikRrUVTBTZbnK8jqY14neJsWAKBzGo+ToaQALsNZ9B91DxxL50K5oVOzw5shAS9TnRjN40+KIXFED4ydq4JRdoqb8+cN+N3i0+Cu7tdm+UaHTAQJBAOwFs3ZwqQEqmv9vmgmIFwFpJm1aIw25gEOf3Hy45GP4bL/j0FQgwcXYRbLE5bPqhw/liLKc1GQ97bVm6zs8SvUCQQDnJZA6TFRMiDjezinE1J4e0v4RupyDniVjbE5ArTK5/FRVkjw4Ny0AqZUEyIIqlTeZlCq45pCJy4a2hymDGVJxAj9gzfXNnmezEsZ//kYvoqHM8lPQhifaeTsigW7tuOf0GPCBw+6uksDnZM0xhZCxOoArBPoMSEbU1pGo1Y2lvhUCQF6E5sBgHAybm53Ich4Rz4LNRqWbSIstrR5F2I3sBRU2kInZXZSjQ1zE+7HUCB4/nFfJ1dp8NdiTCEg1Zw072pECQQDnxyQALmWhQbBTl0tq6CwYf9rZDwBzxuY+CXB8Ky1gOmXwan96KZvV4rK8MQQs6HIiYC/j+5lX3A3zlXTFldaz";
|
||||
protected static final String SAML_CLIENT_SALES_POST_SIG_PUBLIC_KEY = "MIGfMA0GCSqGSIb3DQEBAQUAA4GNADCBiQKBgQDVG8a7xGN6ZIkDbeecySygcDfsypjUMNPE4QJjis8B316CvsZQ0hcTTLUyiRpHlHZys2k3xEhHBHymFC1AONcvzZzpb40tAhLHO1qtAnut00khjAdjR3muLVdGkM/zMC7G5s9iIwBVhwOQhy+VsGnCH91EzkjZ4SVEr55KJoyQJQIDAQAB";
|
||||
|
||||
protected static final String SAML_ASSERTION_CONSUMER_URL_SALES_POST_ENC = "http://localhost:8080/sales-post-enc/";
|
||||
protected static final String SAML_CLIENT_ID_SALES_POST_ENC = "http://localhost:8081/sales-post-enc/";
|
||||
|
|
|
@ -3,17 +3,24 @@ package org.keycloak.testsuite.saml;
|
|||
import org.junit.Test;
|
||||
import org.keycloak.dom.saml.v2.protocol.AuthnRequestType;
|
||||
import org.keycloak.protocol.saml.SamlProtocol;
|
||||
import org.keycloak.saml.SignatureAlgorithm;
|
||||
import org.keycloak.saml.common.constants.GeneralConstants;
|
||||
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.core.saml.v2.common.SAMLDocumentHolder;
|
||||
import org.keycloak.saml.processing.web.util.RedirectBindingUtil;
|
||||
import org.keycloak.services.resources.RealmsResource;
|
||||
import org.keycloak.testsuite.util.KeyUtils;
|
||||
import org.keycloak.testsuite.util.SamlClient;
|
||||
import org.keycloak.testsuite.util.SamlClient.Binding;
|
||||
import org.keycloak.testsuite.util.SamlClient.RedirectStrategyWithSwitchableFollowRedirect;
|
||||
import org.keycloak.testsuite.util.SamlClientBuilder;
|
||||
import java.net.URI;
|
||||
import java.security.Signature;
|
||||
import javax.ws.rs.core.Response;
|
||||
import javax.ws.rs.core.Response.Status;
|
||||
import javax.ws.rs.core.UriBuilder;
|
||||
import org.apache.http.client.methods.CloseableHttpResponse;
|
||||
import org.apache.http.client.methods.HttpUriRequest;
|
||||
|
@ -21,6 +28,7 @@ import org.apache.http.impl.client.CloseableHttpClient;
|
|||
import org.apache.http.impl.client.HttpClientBuilder;
|
||||
import org.apache.http.util.EntityUtils;
|
||||
import org.hamcrest.Matcher;
|
||||
import org.jboss.resteasy.util.Encode;
|
||||
import org.w3c.dom.Document;
|
||||
|
||||
import static org.hamcrest.CoreMatchers.not;
|
||||
|
@ -52,6 +60,53 @@ public class BasicSamlTest extends AbstractSamlTest {
|
|||
assertThat(documentToString(document.getSamlDocument()), not(containsString("InResponseTo=\"" + System.getProperty("java.version") + "\"")));
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testRedirectUrlSigned() throws Exception {
|
||||
testSpecialCharsInRelayState(null);
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testRedirectUrlUnencodedSpecialChars() throws Exception {
|
||||
testSpecialCharsInRelayState("New%20Document%20(1).doc");
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testRedirectUrlEncodedSpecialChars() throws Exception {
|
||||
testSpecialCharsInRelayState("New%20Document%20%281%29.doc");
|
||||
}
|
||||
|
||||
private void testSpecialCharsInRelayState(String encodedRelayState) throws Exception {
|
||||
AuthnRequestType loginRep = SamlClient.createLoginRequestDocument(SAML_CLIENT_ID_SALES_POST_SIG, SAML_ASSERTION_CONSUMER_URL_SALES_POST_SIG, getAuthServerSamlEndpoint(REALM_NAME));
|
||||
|
||||
Document doc = SAML2Request.convert(loginRep);
|
||||
URI redirect = Binding.REDIRECT.createSamlUnsignedRequest(getAuthServerSamlEndpoint(REALM_NAME), null, doc).getURI();
|
||||
String query = redirect.getRawQuery();
|
||||
SignatureAlgorithm signatureAlgorithm = SignatureAlgorithm.RSA_SHA256;
|
||||
|
||||
// now add the relayState
|
||||
String relayStatePart = encodedRelayState == null
|
||||
? ""
|
||||
: ("&" + GeneralConstants.RELAY_STATE + "=" + encodedRelayState);
|
||||
String sigAlgPart = "&" + GeneralConstants.SAML_SIG_ALG_REQUEST_KEY + "=" + Encode.encodeQueryParamAsIs(signatureAlgorithm.getXmlSignatureMethod());
|
||||
|
||||
Signature signature = signatureAlgorithm.createSignature();
|
||||
byte[] sig;
|
||||
|
||||
signature.initSign(KeyUtils.privateKeyFromString(SAML_CLIENT_SALES_POST_SIG_PRIVATE_KEY));
|
||||
signature.update(query.getBytes(GeneralConstants.SAML_CHARSET));
|
||||
signature.update(relayStatePart.getBytes(GeneralConstants.SAML_CHARSET));
|
||||
signature.update(sigAlgPart.getBytes(GeneralConstants.SAML_CHARSET));
|
||||
sig = signature.sign();
|
||||
|
||||
String encodedSig = RedirectBindingUtil.base64Encode(sig);
|
||||
String sigPart = "&" + GeneralConstants.SAML_SIGNATURE_REQUEST_KEY + "=" + Encode.encodeQueryParamAsIs(encodedSig);
|
||||
|
||||
new SamlClientBuilder()
|
||||
.navigateTo(redirect.toString() + relayStatePart + sigAlgPart + sigPart)
|
||||
.assertResponse(statusCodeIsHC(Status.OK))
|
||||
.execute();
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testNoDestinationPost() throws Exception {
|
||||
AuthnRequestType loginRep = SamlClient.createLoginRequestDocument(SAML_CLIENT_ID_SALES_POST, SAML_ASSERTION_CONSUMER_URL_SALES_POST, null);
|
||||
|
@ -85,7 +140,7 @@ public class BasicSamlTest extends AbstractSamlTest {
|
|||
AuthnRequestType loginRep = SamlClient.createLoginRequestDocument(SAML_CLIENT_ID_SALES_POST_SIG, SAML_ASSERTION_CONSUMER_URL_SALES_POST_SIG, null);
|
||||
|
||||
Document doc = SAML2Request.convert(loginRep);
|
||||
HttpUriRequest post = Binding.POST.createSamlSignedRequest(getAuthServerSamlEndpoint(REALM_NAME), null, doc, SAML_CLIENT_ID_SALES_POST_SIG_PRIVATE_KEY, SAML_CLIENT_ID_SALES_POST_SIG_PUBLIC_KEY);
|
||||
HttpUriRequest post = Binding.POST.createSamlSignedRequest(getAuthServerSamlEndpoint(REALM_NAME), null, doc, SAML_CLIENT_SALES_POST_SIG_PRIVATE_KEY, SAML_CLIENT_SALES_POST_SIG_PUBLIC_KEY);
|
||||
|
||||
try (CloseableHttpClient client = HttpClientBuilder.create().setRedirectStrategy(new RedirectStrategyWithSwitchableFollowRedirect()).build();
|
||||
CloseableHttpResponse response = client.execute(post)) {
|
||||
|
|
Loading…
Reference in a new issue