From 69e062a8ff566e233f93028ce64160b38142f719 Mon Sep 17 00:00:00 2001 From: Yang Wang Date: Mon, 29 Jun 2020 10:28:11 +1000 Subject: [PATCH] Support handling LogoutResponse from SAML idP (#56316) SAML idP sends back a LogoutResponse at the end of the logout workflow. It can be sent via either HTTP-Redirect binding or HTTP-POST binding. Currently, the HTTP-Redirect request is simply ignored by Kibana and never reaches ES. It does not cause any obvious issue and the workflow is completed normally from user's perspective. The HTTP-POST request results in a 404 error because POST request is not accepted by Kibana's logout end-point. This causes a non-trivial issue because it renders an error page in user's browser. In addition, some resources do not seem to be fully cleaned up due to the error, e.g. the username will be pre-filled when trying to login again after the 404 error. This PR solves both of the above issues from ES side with a new /_security/saml/complete_logout end-point. Changes are still needed on Kibana side to relay the messages. --- .../action/saml/SamlCompleteLogoutAction.java | 21 ++ .../saml/SamlCompleteLogoutRequest.java | 92 +++++++ .../saml/SamlCompleteLogoutResponse.java | 29 +++ .../action/saml/SamlLogoutResponse.java | 14 +- .../SamlPrepareAuthenticationResponse.java | 2 +- .../saml/SamlCompleteLogoutRequestTests.java | 38 +++ .../xpack/security/Security.java | 5 + .../TransportSamlCompleteLogoutAction.java | 62 +++++ .../saml/TransportSamlLogoutAction.java | 4 +- .../authc/saml/SamlAuthenticator.java | 83 ++----- .../authc/saml/SamlLogoutRequestHandler.java | 89 +------ .../authc/saml/SamlLogoutResponseHandler.java | 59 +++++ ...estHandler.java => SamlObjectHandler.java} | 83 ++++++- .../xpack/security/authc/saml/SamlRealm.java | 21 +- .../authc/saml/SamlResponseHandler.java | 93 +++++++ .../saml/RestSamlCompleteLogoutAction.java | 87 +++++++ .../action/saml/RestSamlLogoutAction.java | 1 + .../saml/TransportSamlLogoutActionTests.java | 1 + .../authc/saml/SamlAuthenticatorTests.java | 189 +-------------- ...amlLogoutResponseHandlerHttpPostTests.java | 94 +++++++ ...ogoutResponseHandlerHttpRedirectTests.java | 132 ++++++++++ ...Tests.java => SamlObjectHandlerTests.java} | 8 +- .../authc/saml/SamlRealmTestHelper.java | 3 +- .../security/authc/saml/SamlRealmTests.java | 2 +- .../authc/saml/SamlResponseHandlerTests.java | 229 ++++++++++++++++++ 25 files changed, 1087 insertions(+), 354 deletions(-) create mode 100644 x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/action/saml/SamlCompleteLogoutAction.java create mode 100644 x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/action/saml/SamlCompleteLogoutRequest.java create mode 100644 x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/action/saml/SamlCompleteLogoutResponse.java create mode 100644 x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/action/saml/SamlCompleteLogoutRequestTests.java create mode 100644 x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/action/saml/TransportSamlCompleteLogoutAction.java create mode 100644 x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/saml/SamlLogoutResponseHandler.java rename x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/saml/{SamlRequestHandler.java => SamlObjectHandler.java} (79%) create mode 100644 x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/saml/SamlResponseHandler.java create mode 100644 x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/rest/action/saml/RestSamlCompleteLogoutAction.java create mode 100644 x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/saml/SamlLogoutResponseHandlerHttpPostTests.java create mode 100644 x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/saml/SamlLogoutResponseHandlerHttpRedirectTests.java rename x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/saml/{SamlRequestHandlerTests.java => SamlObjectHandlerTests.java} (85%) create mode 100644 x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/saml/SamlResponseHandlerTests.java diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/action/saml/SamlCompleteLogoutAction.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/action/saml/SamlCompleteLogoutAction.java new file mode 100644 index 0000000000000..c0e95bd100a12 --- /dev/null +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/action/saml/SamlCompleteLogoutAction.java @@ -0,0 +1,21 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ +package org.elasticsearch.xpack.core.security.action.saml; + +import org.elasticsearch.action.ActionType; + +/** + * ActionType for completing SAML LogoutResponse + */ +public final class SamlCompleteLogoutAction extends ActionType { + + public static final String NAME = "cluster:admin/xpack/security/saml/complete_logout"; + public static final SamlCompleteLogoutAction INSTANCE = new SamlCompleteLogoutAction(); + + private SamlCompleteLogoutAction() { + super(NAME, SamlCompleteLogoutResponse::new); + } +} diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/action/saml/SamlCompleteLogoutRequest.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/action/saml/SamlCompleteLogoutRequest.java new file mode 100644 index 0000000000000..cd3d8e2dec590 --- /dev/null +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/action/saml/SamlCompleteLogoutRequest.java @@ -0,0 +1,92 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ +package org.elasticsearch.xpack.core.security.action.saml; + +import org.elasticsearch.action.ActionRequest; +import org.elasticsearch.action.ActionRequestValidationException; +import org.elasticsearch.common.Nullable; +import org.elasticsearch.common.Strings; +import org.elasticsearch.common.io.stream.StreamInput; + +import java.io.IOException; +import java.util.List; + +import static org.elasticsearch.action.ValidateActions.addValidationError; + +/** + * Represents a request to complete SAML LogoutResponse + */ +public final class SamlCompleteLogoutRequest extends ActionRequest { + + @Nullable + private String queryString; + @Nullable + private String content; + private List validRequestIds; + private String realm; + + public SamlCompleteLogoutRequest(StreamInput in) throws IOException { + super(in); + } + + public SamlCompleteLogoutRequest() { + } + + @Override + public ActionRequestValidationException validate() { + ActionRequestValidationException validationException = null; + if (Strings.hasText(realm) == false) { + validationException = addValidationError("realm may not be empty", validationException); + } + if (Strings.hasText(queryString) == false && Strings.hasText(content) == false) { + validationException = addValidationError("queryString and content may not both be empty", validationException); + } + if (Strings.hasText(queryString) && Strings.hasText(content)) { + validationException = addValidationError("queryString and content may not both present", validationException); + } + return validationException; + } + + public String getQueryString() { + return queryString; + } + + public void setQueryString(String queryString) { + this.queryString = queryString; + } + + public String getContent() { + return content; + } + + public void setContent(String content) { + this.content = content; + } + + public List getValidRequestIds() { + return validRequestIds; + } + + public void setValidRequestIds(List validRequestIds) { + this.validRequestIds = validRequestIds; + } + + public String getRealm() { + return realm; + } + + public void setRealm(String realm) { + this.realm = realm; + } + + public boolean isHttpRedirect() { + return queryString != null; + } + + public String getPayload() { + return isHttpRedirect() ? queryString : content; + } +} diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/action/saml/SamlCompleteLogoutResponse.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/action/saml/SamlCompleteLogoutResponse.java new file mode 100644 index 0000000000000..cc38b17a1dd20 --- /dev/null +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/action/saml/SamlCompleteLogoutResponse.java @@ -0,0 +1,29 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ +package org.elasticsearch.xpack.core.security.action.saml; + +import org.elasticsearch.action.ActionResponse; +import org.elasticsearch.common.io.stream.StreamInput; +import org.elasticsearch.common.io.stream.StreamOutput; + +import java.io.IOException; + +/** + * A response to complete the LogoutResponse from idP + */ +public final class SamlCompleteLogoutResponse extends ActionResponse { + + public SamlCompleteLogoutResponse(StreamInput in) throws IOException { + super(in); + } + + public SamlCompleteLogoutResponse() { + } + + @Override + public void writeTo(StreamOutput out) throws IOException { + } +} diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/action/saml/SamlLogoutResponse.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/action/saml/SamlLogoutResponse.java index dc176b6113451..c6599b7db1d9e 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/action/saml/SamlLogoutResponse.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/action/saml/SamlLogoutResponse.java @@ -16,24 +16,32 @@ */ public final class SamlLogoutResponse extends ActionResponse { - private String redirectUrl; + private final String requestId; + private final String redirectUrl; public SamlLogoutResponse(StreamInput in) throws IOException { super(in); + requestId = in.readString(); redirectUrl = in.readString(); } - public SamlLogoutResponse(String redirectUrl) { + public SamlLogoutResponse(String requestId, String redirectUrl) { + this.requestId = requestId; this.redirectUrl = redirectUrl; } + public String getRequestId() { + return requestId; + } + public String getRedirectUrl() { return redirectUrl; } @Override public void writeTo(StreamOutput out) throws IOException { + out.writeString(requestId); out.writeString(redirectUrl); } - } +} diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/action/saml/SamlPrepareAuthenticationResponse.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/action/saml/SamlPrepareAuthenticationResponse.java index 942d7d0b0afef..69adb7ac3df6c 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/action/saml/SamlPrepareAuthenticationResponse.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/action/saml/SamlPrepareAuthenticationResponse.java @@ -48,4 +48,4 @@ public void writeTo(StreamOutput out) throws IOException { out.writeString(redirectUrl); } - } +} diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/action/saml/SamlCompleteLogoutRequestTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/action/saml/SamlCompleteLogoutRequestTests.java new file mode 100644 index 0000000000000..3b9cbef75c13c --- /dev/null +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/action/saml/SamlCompleteLogoutRequestTests.java @@ -0,0 +1,38 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +package org.elasticsearch.xpack.core.security.action.saml; + +import org.elasticsearch.action.ActionRequestValidationException; +import org.elasticsearch.test.ESTestCase; + +import static org.hamcrest.Matchers.containsString; + +public class SamlCompleteLogoutRequestTests extends ESTestCase { + + public void testValidateFailsWhenQueryAndBodyBothNotExist() { + final SamlCompleteLogoutRequest samlCompleteLogoutRequest = new SamlCompleteLogoutRequest(); + samlCompleteLogoutRequest.setRealm("realm"); + final ActionRequestValidationException validationException = samlCompleteLogoutRequest.validate(); + assertThat(validationException.getMessage(), containsString("queryString and content may not both be empty")); + } + + public void testValidateFailsWhenQueryAndBodyBothSet() { + final SamlCompleteLogoutRequest samlCompleteLogoutRequest = new SamlCompleteLogoutRequest(); + samlCompleteLogoutRequest.setRealm("realm"); + samlCompleteLogoutRequest.setQueryString("queryString"); + samlCompleteLogoutRequest.setContent("content"); + final ActionRequestValidationException validationException = samlCompleteLogoutRequest.validate(); + assertThat(validationException.getMessage(), containsString("queryString and content may not both present")); + } + + public void testValidateFailsWhenRealmIsNotSet() { + final SamlCompleteLogoutRequest samlCompleteLogoutRequest = new SamlCompleteLogoutRequest(); + samlCompleteLogoutRequest.setQueryString("queryString"); + final ActionRequestValidationException validationException = samlCompleteLogoutRequest.validate(); + assertThat(validationException.getMessage(), containsString("realm may not be empty")); + } +} diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/Security.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/Security.java index 046bf80611a04..804f3bc59c3fc 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/Security.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/Security.java @@ -104,6 +104,7 @@ import org.elasticsearch.xpack.core.security.action.saml.SamlAuthenticateAction; import org.elasticsearch.xpack.core.security.action.saml.SamlInvalidateSessionAction; import org.elasticsearch.xpack.core.security.action.saml.SamlLogoutAction; +import org.elasticsearch.xpack.core.security.action.saml.SamlCompleteLogoutAction; import org.elasticsearch.xpack.core.security.action.saml.SamlPrepareAuthenticationAction; import org.elasticsearch.xpack.core.security.action.token.CreateTokenAction; import org.elasticsearch.xpack.core.security.action.token.InvalidateTokenAction; @@ -167,6 +168,7 @@ import org.elasticsearch.xpack.security.action.saml.TransportSamlAuthenticateAction; import org.elasticsearch.xpack.security.action.saml.TransportSamlInvalidateSessionAction; import org.elasticsearch.xpack.security.action.saml.TransportSamlLogoutAction; +import org.elasticsearch.xpack.security.action.saml.TransportSamlCompleteLogoutAction; import org.elasticsearch.xpack.security.action.saml.TransportSamlPrepareAuthenticationAction; import org.elasticsearch.xpack.security.action.token.TransportCreateTokenAction; import org.elasticsearch.xpack.security.action.token.TransportInvalidateTokenAction; @@ -233,6 +235,7 @@ import org.elasticsearch.xpack.security.rest.action.saml.RestSamlAuthenticateAction; import org.elasticsearch.xpack.security.rest.action.saml.RestSamlInvalidateSessionAction; import org.elasticsearch.xpack.security.rest.action.saml.RestSamlLogoutAction; +import org.elasticsearch.xpack.security.rest.action.saml.RestSamlCompleteLogoutAction; import org.elasticsearch.xpack.security.rest.action.saml.RestSamlPrepareAuthenticationAction; import org.elasticsearch.xpack.security.rest.action.user.RestChangePasswordAction; import org.elasticsearch.xpack.security.rest.action.user.RestDeleteUserAction; @@ -789,6 +792,7 @@ public void onIndexModule(IndexModule module) { new ActionHandler<>(SamlAuthenticateAction.INSTANCE, TransportSamlAuthenticateAction.class), new ActionHandler<>(SamlLogoutAction.INSTANCE, TransportSamlLogoutAction.class), new ActionHandler<>(SamlInvalidateSessionAction.INSTANCE, TransportSamlInvalidateSessionAction.class), + new ActionHandler<>(SamlCompleteLogoutAction.INSTANCE, TransportSamlCompleteLogoutAction.class), new ActionHandler<>(OpenIdConnectPrepareAuthenticationAction.INSTANCE, TransportOpenIdConnectPrepareAuthenticationAction.class), new ActionHandler<>(OpenIdConnectAuthenticateAction.INSTANCE, TransportOpenIdConnectAuthenticateAction.class), @@ -848,6 +852,7 @@ public List getRestHandlers(Settings settings, RestController restC new RestSamlAuthenticateAction(settings, getLicenseState()), new RestSamlLogoutAction(settings, getLicenseState()), new RestSamlInvalidateSessionAction(settings, getLicenseState()), + new RestSamlCompleteLogoutAction(settings, getLicenseState()), new RestOpenIdConnectPrepareAuthenticationAction(settings, getLicenseState()), new RestOpenIdConnectAuthenticateAction(settings, getLicenseState()), new RestOpenIdConnectLogoutAction(settings, getLicenseState()), diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/action/saml/TransportSamlCompleteLogoutAction.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/action/saml/TransportSamlCompleteLogoutAction.java new file mode 100644 index 0000000000000..f4893c15f1cae --- /dev/null +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/action/saml/TransportSamlCompleteLogoutAction.java @@ -0,0 +1,62 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ +package org.elasticsearch.xpack.security.action.saml; + +import org.elasticsearch.action.ActionListener; +import org.elasticsearch.action.support.ActionFilters; +import org.elasticsearch.action.support.HandledTransportAction; +import org.elasticsearch.common.inject.Inject; +import org.elasticsearch.tasks.Task; +import org.elasticsearch.transport.TransportService; +import org.elasticsearch.xpack.core.security.action.saml.SamlCompleteLogoutAction; +import org.elasticsearch.xpack.core.security.action.saml.SamlCompleteLogoutRequest; +import org.elasticsearch.xpack.core.security.action.saml.SamlCompleteLogoutResponse; +import org.elasticsearch.xpack.security.authc.Realms; +import org.elasticsearch.xpack.security.authc.saml.SamlLogoutResponseHandler; +import org.elasticsearch.xpack.security.authc.saml.SamlRealm; +import org.elasticsearch.xpack.security.authc.saml.SamlUtils; + +import java.util.List; + +import static org.elasticsearch.xpack.security.authc.saml.SamlRealm.findSamlRealms; + +/** + * Transport action responsible for completing SAML LogoutResponse + */ +public final class TransportSamlCompleteLogoutAction extends HandledTransportAction { + + private final Realms realms; + + @Inject + public TransportSamlCompleteLogoutAction(TransportService transportService, ActionFilters actionFilters, Realms realms) { + super(SamlCompleteLogoutAction.NAME, transportService, actionFilters, SamlCompleteLogoutRequest::new); + this.realms = realms; + } + + @Override + protected void doExecute(Task task, SamlCompleteLogoutRequest request, ActionListener listener) { + List realms = findSamlRealms(this.realms, request.getRealm(), null); + if (realms.isEmpty()) { + listener.onFailure(SamlUtils.samlException("Cannot find any matching realm with name [{}]", request.getRealm())); + } else if (realms.size() > 1) { + listener.onFailure(SamlUtils.samlException("Found multiple matching realms [{}] with name [{}]", realms, request.getRealm())); + } else { + processLogoutResponse(realms.get(0), request, listener); + } + } + + private void processLogoutResponse(SamlRealm samlRealm, SamlCompleteLogoutRequest request, + ActionListener listener) { + + final SamlLogoutResponseHandler logoutResponseHandler = samlRealm.getLogoutResponseHandler(); + try { + logoutResponseHandler.handle(request.isHttpRedirect(), request.getPayload(), request.getValidRequestIds()); + listener.onResponse(new SamlCompleteLogoutResponse()); + } catch (Exception e) { + listener.onFailure(e); + } + } +} diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/action/saml/TransportSamlLogoutAction.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/action/saml/TransportSamlLogoutAction.java index c7df4c34337bd..7611340e47788 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/action/saml/TransportSamlLogoutAction.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/action/saml/TransportSamlLogoutAction.java @@ -112,10 +112,10 @@ private SamlLogoutResponse buildResponse(Authentication authentication, Map metadata, String key) { diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/saml/SamlAuthenticator.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/saml/SamlAuthenticator.java index de5dbde2c7e44..fab8b265a43b6 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/saml/SamlAuthenticator.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/saml/SamlAuthenticator.java @@ -21,10 +21,6 @@ import org.opensaml.saml.saml2.core.EncryptedAssertion; import org.opensaml.saml.saml2.core.EncryptedAttribute; import org.opensaml.saml.saml2.core.Response; -import org.opensaml.saml.saml2.core.Status; -import org.opensaml.saml.saml2.core.StatusCode; -import org.opensaml.saml.saml2.core.StatusDetail; -import org.opensaml.saml.saml2.core.StatusMessage; import org.opensaml.saml.saml2.core.Subject; import org.opensaml.saml.saml2.core.SubjectConfirmation; import org.opensaml.saml.saml2.core.SubjectConfirmationData; @@ -46,7 +42,7 @@ /** * Processes the IdP's SAML Response for our AuthnRequest, validates it, and extracts the relevant properties. */ -class SamlAuthenticator extends SamlRequestHandler { +class SamlAuthenticator extends SamlResponseHandler { private static final String RESPONSE_TAG_NAME = "Response"; @@ -94,20 +90,8 @@ private SamlAttributes authenticateResponse(Element element, Collection requireSignedAssertions = true; } - if (Strings.hasText(response.getInResponseTo()) && allowedSamlRequestIds.contains(response.getInResponseTo()) == false) { - logger.debug("The SAML Response with ID [{}] is unsolicited. A user might have used a stale URL or the Identity Provider " + - "incorrectly populates the InResponseTo attribute", response.getID()); - throw samlException("SAML content is in-response-to [{}] but expected one of {} ", - response.getInResponseTo(), allowedSamlRequestIds); - } - - final Status status = response.getStatus(); - if (status == null || status.getStatusCode() == null) { - throw samlException("SAML Response has no status code"); - } - if (isSuccess(status) == false) { - throw samlException("SAML Response is not a 'success' response: {}", getStatusCodeMessage(status)); - } + checkInResponseTo(response, allowedSamlRequestIds); + checkStatus(response.getStatus()); checkIssuer(response.getIssuer(), response); checkResponseDestination(response); @@ -136,46 +120,6 @@ private SamlAttributes authenticateResponse(Element element, Collection return new SamlAttributes(nameId, session, attributes); } - private String getStatusCodeMessage(Status status) { - StatusCode firstLevel = status.getStatusCode(); - StatusCode subLevel = firstLevel.getStatusCode(); - StringBuilder sb = new StringBuilder(); - if (StatusCode.REQUESTER.equals(firstLevel.getValue())) { - sb.append("The SAML IdP did not grant the request. It indicated that the Elastic Stack side sent something invalid ("); - } else if (StatusCode.RESPONDER.equals(firstLevel.getValue())) { - sb.append("The request could not be granted due to an error in the SAML IDP side ("); - } else if (StatusCode.VERSION_MISMATCH.equals(firstLevel.getValue())) { - sb.append("The request could not be granted because the SAML IDP doesn't support SAML 2.0 ("); - } else { - sb.append("The request could not be granted, the SAML IDP responded with a non-standard Status code ("); - } - sb.append(firstLevel.getValue()).append(")."); - if (getMessage(status) != null) { - sb.append(" Message: [").append(getMessage(status)).append("]"); - } - if (getDetail(status) != null) { - sb.append(" Detail: [").append(getDetail(status)).append("]"); - } - if (null != subLevel) { - sb.append(" Specific status code which might indicate what the issue is: [").append(subLevel.getValue()).append("]"); - } - return sb.toString(); - } - - private String getMessage(Status status) { - final StatusMessage sm = status.getStatusMessage(); - return sm == null ? null : sm.getMessage(); - } - - private String getDetail(Status status) { - final StatusDetail sd = status.getStatusDetail(); - return sd == null ? null : SamlUtils.toString(sd.getDOM()); - } - - private boolean isSuccess(Status status) { - return status.getStatusCode().getValue().equals(StatusCode.SUCCESS); - } - private String getSessionIndex(Assertion assertion) { return assertion.getAuthnStatements().stream().map(as -> as.getSessionIndex()).filter(Objects::nonNull).findFirst().orElse(null); } @@ -333,18 +277,11 @@ private void checkSubject(Subject assertionSubject, XMLObject parent, Collection } checkRecipient(confirmationData.get(0)); checkLifetimeRestrictions(confirmationData.get(0)); - checkInResponseTo(confirmationData.get(0), allowedSamlRequestIds); + checkSubjectInResponseTo(confirmationData.get(0), allowedSamlRequestIds); } - private void checkRecipient(SubjectConfirmationData subjectConfirmationData) { - final SpConfiguration sp = getSpConfiguration(); - if (sp.getAscUrl().equals(subjectConfirmationData.getRecipient()) == false) { - throw samlException("SAML Assertion SubjectConfirmationData Recipient [{}] does not match expected value [{}]", - subjectConfirmationData.getRecipient(), sp.getAscUrl()); - } - } - - private void checkInResponseTo(SubjectConfirmationData subjectConfirmationData, Collection allowedSamlRequestIds) { + private void checkSubjectInResponseTo( + SubjectConfirmationData subjectConfirmationData, Collection allowedSamlRequestIds) { // Allow for IdP initiated SSO where InResponseTo MUST be missing if (Strings.hasText(subjectConfirmationData.getInResponseTo()) && allowedSamlRequestIds.contains(subjectConfirmationData.getInResponseTo()) == false) { @@ -353,6 +290,14 @@ private void checkInResponseTo(SubjectConfirmationData subjectConfirmationData, } } + private void checkRecipient(SubjectConfirmationData subjectConfirmationData) { + final SpConfiguration sp = getSpConfiguration(); + if (sp.getAscUrl().equals(subjectConfirmationData.getRecipient()) == false) { + throw samlException("SAML Assertion SubjectConfirmationData Recipient [{}] does not match expected value [{}]", + subjectConfirmationData.getRecipient(), sp.getAscUrl()); + } + } + private void checkAudienceRestrictions(List restrictions) { if (restrictions.stream().allMatch(this::checkAudienceRestriction) == false) { throw samlException("Conditions [{}] do not match required audience [{}]", diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/saml/SamlLogoutRequestHandler.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/saml/SamlLogoutRequestHandler.java index 9bd8527e373b2..97f406b56a2b9 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/saml/SamlLogoutRequestHandler.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/saml/SamlLogoutRequestHandler.java @@ -5,29 +5,16 @@ */ package org.elasticsearch.xpack.security.authc.saml; -import java.io.ByteArrayInputStream; -import java.io.ByteArrayOutputStream; -import java.io.IOException; -import java.nio.charset.StandardCharsets; import java.time.Clock; -import java.util.Base64; -import java.util.HashMap; -import java.util.Map; import java.util.Objects; -import java.util.zip.Inflater; -import java.util.zip.InflaterInputStream; import org.apache.logging.log4j.message.ParameterizedMessage; import org.elasticsearch.ElasticsearchSecurityException; -import org.elasticsearch.common.Strings; import org.elasticsearch.common.unit.TimeValue; -import org.elasticsearch.core.internal.io.Streams; -import org.elasticsearch.rest.RestUtils; import org.opensaml.saml.common.SAMLObject; import org.opensaml.saml.saml2.core.EncryptedID; import org.opensaml.saml.saml2.core.LogoutRequest; import org.opensaml.saml.saml2.core.NameID; -import org.opensaml.xmlsec.crypto.XMLSigningUtil; import org.opensaml.xmlsec.encryption.support.DecryptionException; import org.opensaml.xmlsec.signature.Signature; import org.w3c.dom.Element; @@ -37,7 +24,7 @@ /** * Processes a LogoutRequest for an IdP-initiated logout. */ -public class SamlLogoutRequestHandler extends SamlRequestHandler { +public class SamlLogoutRequestHandler extends SamlObjectHandler { private static final String REQUEST_TAG_NAME = "LogoutRequest"; @@ -57,9 +44,9 @@ public class SamlLogoutRequestHandler extends SamlRequestHandler { * @throws ElasticsearchSecurityException If the SAML is invalid for this realm/configuration */ public Result parseFromQueryString(String queryString) { - final ParsedQueryString parsed = parseQueryStringAndValidateSignature(queryString); + final ParsedQueryString parsed = parseQueryStringAndValidateSignature(queryString, "SAMLRequest"); - final Element root = parseSamlMessage(inflate(decodeBase64(parsed.samlRequest))); + final Element root = parseSamlMessage(inflate(decodeBase64(parsed.samlMessage))); if (REQUEST_TAG_NAME.equals(root.getLocalName()) && SAML_NAMESPACE.equals(root.getNamespaceURI())) { try { final LogoutRequest logoutRequest = buildXmlObject(root, LogoutRequest.class); @@ -74,26 +61,6 @@ public Result parseFromQueryString(String queryString) { } } - private ParsedQueryString parseQueryStringAndValidateSignature(String queryString) { - final String signatureInput = queryString.replaceAll("&Signature=.*$", ""); - final Map parameters = new HashMap<>(); - RestUtils.decodeQueryString(queryString, 0, parameters); - final String samlRequest = parameters.get("SAMLRequest"); - if (samlRequest == null) { - throw samlException("Could not parse SAMLRequest from query string: [{}]", queryString); - } - - final String relayState = parameters.get("RelayState"); - final String signatureAlgorithm = parameters.get("SigAlg"); - final String signature = parameters.get("Signature"); - if (signature == null || signatureAlgorithm == null) { - return new ParsedQueryString(samlRequest, false, relayState); - } - - validateSignature(signatureInput, signatureAlgorithm, signature); - return new ParsedQueryString(samlRequest, true, relayState); - } - private Result parseLogout(LogoutRequest logoutRequest, boolean requireSignature, String relayState) { final Signature signature = logoutRequest.getSignature(); if (signature == null) { @@ -111,44 +78,6 @@ private Result parseLogout(LogoutRequest logoutRequest, boolean requireSignature return new Result(logoutRequest.getID(), SamlNameId.fromXml(getNameID(logoutRequest)), getSessionIndex(logoutRequest), relayState); } - private void validateSignature(String inputString, String signatureAlgorithm, String signature) { - final byte[] sigBytes = decodeBase64(signature); - final byte[] inputBytes = inputString.getBytes(StandardCharsets.US_ASCII); - final String signatureText = Strings.cleanTruncate(signature, 32); - checkIdpSignature(credential -> { - if (XMLSigningUtil.verifyWithURI(credential, signatureAlgorithm, sigBytes, inputBytes)) { - logger.debug(() -> new ParameterizedMessage("SAML Signature [{}] matches credentials [{}] [{}]", - signatureText, credential.getEntityId(), credential.getPublicKey())); - return true; - } else { - logger.debug(() -> new ParameterizedMessage("SAML Signature [{}] failed against credentials [{}] [{}]", - signatureText, credential.getEntityId(), credential.getPublicKey())); - return false; - } - }, signatureText); - } - - private byte[] decodeBase64(String content) { - try { - return Base64.getDecoder().decode(content.replaceAll("\\s+", "")); - } catch (IllegalArgumentException e) { - logger.info("Failed to decode base64 string [{}] - {}", content, e.toString()); - throw samlException("SAML message cannot be Base64 decoded", e); - } - } - - private byte[] inflate(byte[] bytes) { - Inflater inflater = new Inflater(true); - try (ByteArrayInputStream in = new ByteArrayInputStream(bytes); - InflaterInputStream inflate = new InflaterInputStream(in, inflater); - ByteArrayOutputStream out = new ByteArrayOutputStream(bytes.length * 3 / 2)) { - Streams.copy(inflate, out); - return out.toByteArray(); - } catch (IOException e) { - throw samlException("SAML message cannot be inflated", e); - } - } - private NameID getNameID(LogoutRequest logoutRequest) { final NameID nameID = logoutRequest.getNameID(); if (nameID == null) { @@ -197,18 +126,6 @@ private void checkDestination(LogoutRequest request) { } } - static class ParsedQueryString { - final String samlRequest; - final boolean hasSignature; - final String relayState; - - ParsedQueryString(String samlRequest, boolean hasSignature, String relayState) { - this.samlRequest = samlRequest; - this.hasSignature = hasSignature; - this.relayState = relayState; - } - } - public static class Result { private final String requestId; private final SamlNameId nameId; diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/saml/SamlLogoutResponseHandler.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/saml/SamlLogoutResponseHandler.java new file mode 100644 index 0000000000000..7e17cb6c84dfa --- /dev/null +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/saml/SamlLogoutResponseHandler.java @@ -0,0 +1,59 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +package org.elasticsearch.xpack.security.authc.saml; + +import org.elasticsearch.common.unit.TimeValue; +import org.opensaml.saml.saml2.core.LogoutResponse; +import org.w3c.dom.Element; + +import java.time.Clock; +import java.util.Collection; + +import static org.elasticsearch.xpack.security.authc.saml.SamlUtils.samlException; + +public class SamlLogoutResponseHandler extends SamlResponseHandler { + + private static final String LOGOUT_RESPONSE_TAG_NAME = "LogoutResponse"; + + public SamlLogoutResponseHandler( + Clock clock, IdpConfiguration idp, SpConfiguration sp, TimeValue maxSkew) { + super(clock, idp, sp, maxSkew); + } + + public void handle(boolean httpRedirect, String payload, Collection allowedSamlRequestIds) { + final Element root; + if (httpRedirect) { + logger.debug("Process SAML LogoutResponse with HTTP-Redirect binding"); + final ParsedQueryString parsed = parseQueryStringAndValidateSignature(payload, "SAMLResponse"); + if (parsed.hasSignature == false){ + throw samlException("Query string is not signed, but is required for HTTP-Redirect binding"); + } + root = parseSamlMessage(inflate(decodeBase64(parsed.samlMessage))); + } else { + logger.debug("Process SAML LogoutResponse with HTTP-POST binding"); + root = parseSamlMessage(decodeBase64(payload)); + } + + if (LOGOUT_RESPONSE_TAG_NAME.equals(root.getLocalName()) && SAML_NAMESPACE.equals(root.getNamespaceURI())) { + final LogoutResponse logoutResponse = buildXmlObject(root, LogoutResponse.class); + // For HTTP-Redirect, the signature is already validated when parsing the query string + if (httpRedirect == false) { + if (logoutResponse.getSignature() == null) { + throw samlException("LogoutResponse is not signed, but is required for HTTP-Post binding"); + } + validateSignature(logoutResponse.getSignature()); + } + checkInResponseTo(logoutResponse, allowedSamlRequestIds); + checkStatus(logoutResponse.getStatus()); + checkIssuer(logoutResponse.getIssuer(), logoutResponse); + checkResponseDestination(logoutResponse, getSpConfiguration().getLogoutUrl()); + } else { + throw samlException("SAML content [{}] should have a root element of Namespace=[{}] Tag=[{}]", + root, SAML_NAMESPACE, LOGOUT_RESPONSE_TAG_NAME); + } + } +} diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/saml/SamlRequestHandler.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/saml/SamlObjectHandler.java similarity index 79% rename from x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/saml/SamlRequestHandler.java rename to x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/saml/SamlObjectHandler.java index ccd0ec22f20ab..4f68d5e380a53 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/saml/SamlRequestHandler.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/saml/SamlObjectHandler.java @@ -13,6 +13,8 @@ import org.elasticsearch.common.Nullable; import org.elasticsearch.common.Strings; import org.elasticsearch.common.unit.TimeValue; +import org.elasticsearch.core.internal.io.Streams; +import org.elasticsearch.rest.RestUtils; import org.elasticsearch.xpack.core.security.support.RestorableContextClassLoader; import org.joda.time.DateTime; import org.opensaml.core.xml.XMLObject; @@ -24,6 +26,7 @@ import org.opensaml.saml.security.impl.SAMLSignatureProfileValidator; import org.opensaml.security.credential.Credential; import org.opensaml.security.x509.X509Credential; +import org.opensaml.xmlsec.crypto.XMLSigningUtil; import org.opensaml.xmlsec.encryption.support.ChainingEncryptedKeyResolver; import org.opensaml.xmlsec.encryption.support.EncryptedKeyResolver; import org.opensaml.xmlsec.encryption.support.InlineEncryptedKeyResolver; @@ -46,7 +49,9 @@ import javax.xml.parsers.DocumentBuilder; import java.io.ByteArrayInputStream; +import java.io.ByteArrayOutputStream; import java.io.IOException; +import java.nio.charset.StandardCharsets; import java.security.AccessController; import java.security.PrivilegedActionException; import java.security.PrivilegedExceptionAction; @@ -58,14 +63,18 @@ import java.util.Base64; import java.util.Collection; import java.util.Collections; +import java.util.HashMap; import java.util.List; +import java.util.Map; import java.util.function.Predicate; import java.util.stream.Collectors; +import java.util.zip.Inflater; +import java.util.zip.InflaterInputStream; import static org.elasticsearch.xpack.security.authc.saml.SamlUtils.samlException; import static org.opensaml.core.xml.config.XMLObjectProviderRegistrySupport.getUnmarshallerFactory; -public class SamlRequestHandler { +public class SamlObjectHandler { protected static final String SAML_NAMESPACE = "urn:oasis:names:tc:SAML:2.0:protocol"; @@ -93,7 +102,7 @@ public class SamlRequestHandler { private final TimeValue maxSkew; private final UnmarshallerFactory unmarshallerFactory; - public SamlRequestHandler(Clock clock, IdpConfiguration idp, SpConfiguration sp, TimeValue maxSkew) { + public SamlObjectHandler(Clock clock, IdpConfiguration idp, SpConfiguration sp, TimeValue maxSkew) { this.clock = clock; this.idp = idp; this.sp = sp; @@ -322,4 +331,74 @@ protected void validateNotOnOrAfter(DateTime notOnOrAfter) { throw samlException("Rejecting SAML assertion because [{}] is on/after [{}]", pastNow, notOnOrAfter); } } + + protected ParsedQueryString parseQueryStringAndValidateSignature(String queryString, String samlMessageParameterName) { + final String signatureInput = queryString.replaceAll("&Signature=.*$", ""); + final Map parameters = new HashMap<>(); + RestUtils.decodeQueryString(queryString, 0, parameters); + final String samlMessage = parameters.get(samlMessageParameterName); + if (samlMessage == null) { + throw samlException("Could not parse {} from query string: [{}]", samlMessageParameterName, queryString); + } + + final String relayState = parameters.get("RelayState"); + final String signatureAlgorithm = parameters.get("SigAlg"); + final String signature = parameters.get("Signature"); + if (signature == null || signatureAlgorithm == null) { + return new ParsedQueryString(samlMessage, false, relayState); + } + + validateSignature(signatureInput, signatureAlgorithm, signature); + return new ParsedQueryString(samlMessage, true, relayState); + } + + private void validateSignature(String inputString, String signatureAlgorithm, String signature) { + final byte[] sigBytes = decodeBase64(signature); + final byte[] inputBytes = inputString.getBytes(StandardCharsets.US_ASCII); + final String signatureText = Strings.cleanTruncate(signature, 32); + checkIdpSignature(credential -> { + if (XMLSigningUtil.verifyWithURI(credential, signatureAlgorithm, sigBytes, inputBytes)) { + logger.debug(() -> new ParameterizedMessage("SAML Signature [{}] matches credentials [{}] [{}]", + signatureText, credential.getEntityId(), credential.getPublicKey())); + return true; + } else { + logger.debug(() -> new ParameterizedMessage("SAML Signature [{}] failed against credentials [{}] [{}]", + signatureText, credential.getEntityId(), credential.getPublicKey())); + return false; + } + }, signatureText); + } + + protected byte[] decodeBase64(String content) { + try { + return Base64.getDecoder().decode(content.replaceAll("\\s+", "")); + } catch (IllegalArgumentException e) { + logger.info("Failed to decode base64 string [{}] - {}", content, e.toString()); + throw samlException("SAML message cannot be Base64 decoded", e); + } + } + + protected byte[] inflate(byte[] bytes) { + Inflater inflater = new Inflater(true); + try (ByteArrayInputStream in = new ByteArrayInputStream(bytes); + InflaterInputStream inflate = new InflaterInputStream(in, inflater); + ByteArrayOutputStream out = new ByteArrayOutputStream(bytes.length * 3 / 2)) { + Streams.copy(inflate, out); + return out.toByteArray(); + } catch (IOException e) { + throw samlException("SAML message cannot be inflated", e); + } + } + + static class ParsedQueryString { + final String samlMessage; + final boolean hasSignature; + final String relayState; + + ParsedQueryString(String samlMessage, boolean hasSignature, String relayState) { + this.samlMessage = samlMessage; + this.hasSignature = hasSignature; + this.relayState = relayState; + } + } } diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/saml/SamlRealm.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/saml/SamlRealm.java index ea6b69113537c..6605cf7833d96 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/saml/SamlRealm.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/saml/SamlRealm.java @@ -152,6 +152,7 @@ public final class SamlRealm extends Realm implements Releasable { private final SamlLogoutRequestHandler logoutHandler; private final UserRoleMapper roleMapper; + private final SamlLogoutResponseHandler logoutResponseHandler; private final Supplier idpDescriptor; private final SpConfiguration serviceProvider; @@ -195,8 +196,11 @@ public static SamlRealm create(RealmConfig config, SSLService sslService, Resour final SamlAuthenticator authenticator = new SamlAuthenticator(clock, idpConfiguration, serviceProvider, maxSkew); final SamlLogoutRequestHandler logoutHandler = new SamlLogoutRequestHandler(clock, idpConfiguration, serviceProvider, maxSkew); + final SamlLogoutResponseHandler logoutResponseHandler = + new SamlLogoutResponseHandler(clock, idpConfiguration, serviceProvider, maxSkew); - final SamlRealm realm = new SamlRealm(config, roleMapper, authenticator, logoutHandler, idpDescriptor, serviceProvider); + final SamlRealm realm = new SamlRealm(config, roleMapper, authenticator, logoutHandler, + logoutResponseHandler, idpDescriptor, serviceProvider); // the metadata resolver needs to be destroyed since it runs a timer task in the background and destroying stops it! realm.releasables.add(() -> metadataResolver.destroy()); @@ -205,13 +209,20 @@ public static SamlRealm create(RealmConfig config, SSLService sslService, Resour } // For testing - SamlRealm(RealmConfig config, UserRoleMapper roleMapper, SamlAuthenticator authenticator, SamlLogoutRequestHandler logoutHandler, - Supplier idpDescriptor, SpConfiguration spConfiguration) throws Exception { + SamlRealm( + RealmConfig config, + UserRoleMapper roleMapper, + SamlAuthenticator authenticator, + SamlLogoutRequestHandler logoutHandler, + SamlLogoutResponseHandler logoutResponseHandler, + Supplier idpDescriptor, + SpConfiguration spConfiguration) throws Exception { super(config); this.roleMapper = roleMapper; this.authenticator = authenticator; this.logoutHandler = logoutHandler; + this.logoutResponseHandler = logoutResponseHandler; this.idpDescriptor = idpDescriptor; this.serviceProvider = spConfiguration; @@ -701,6 +712,10 @@ public SamlLogoutRequestHandler getLogoutHandler() { return this.logoutHandler; } + public SamlLogoutResponseHandler getLogoutResponseHandler() { + return logoutResponseHandler; + } + private static class FileListener implements FileChangesListener { private final Logger logger; diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/saml/SamlResponseHandler.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/saml/SamlResponseHandler.java new file mode 100644 index 0000000000000..8508218368c69 --- /dev/null +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/saml/SamlResponseHandler.java @@ -0,0 +1,93 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +package org.elasticsearch.xpack.security.authc.saml; + +import org.elasticsearch.common.Strings; +import org.elasticsearch.common.unit.TimeValue; +import org.opensaml.saml.saml2.core.Status; +import org.opensaml.saml.saml2.core.StatusCode; +import org.opensaml.saml.saml2.core.StatusDetail; +import org.opensaml.saml.saml2.core.StatusMessage; +import org.opensaml.saml.saml2.core.StatusResponseType; + +import java.time.Clock; +import java.util.Collection; + +import static org.elasticsearch.xpack.security.authc.saml.SamlUtils.samlException; + +public class SamlResponseHandler extends SamlObjectHandler { + public SamlResponseHandler(Clock clock, IdpConfiguration idp, SpConfiguration sp, TimeValue maxSkew) { + super(clock, idp, sp, maxSkew); + } + + protected void checkInResponseTo(StatusResponseType response, Collection allowedSamlRequestIds) { + if (Strings.hasText(response.getInResponseTo()) && allowedSamlRequestIds.contains(response.getInResponseTo()) == false) { + logger.debug("The SAML Response with ID [{}] is unsolicited. A user might have used a stale URL or the Identity Provider " + + "incorrectly populates the InResponseTo attribute", response.getID()); + throw samlException("SAML content is in-response-to [{}] but expected one of {} ", + response.getInResponseTo(), allowedSamlRequestIds); + } + } + + protected String getStatusCodeMessage(Status status) { + StatusCode firstLevel = status.getStatusCode(); + StatusCode subLevel = firstLevel.getStatusCode(); + StringBuilder sb = new StringBuilder(); + if (StatusCode.REQUESTER.equals(firstLevel.getValue())) { + sb.append("The SAML IdP did not grant the request. It indicated that the Elastic Stack side sent something invalid ("); + } else if (StatusCode.RESPONDER.equals(firstLevel.getValue())) { + sb.append("The request could not be granted due to an error in the SAML IDP side ("); + } else if (StatusCode.VERSION_MISMATCH.equals(firstLevel.getValue())) { + sb.append("The request could not be granted because the SAML IDP doesn't support SAML 2.0 ("); + } else { + sb.append("The request could not be granted, the SAML IDP responded with a non-standard Status code ("); + } + sb.append(firstLevel.getValue()).append(")."); + if (getMessage(status) != null) { + sb.append(" Message: [").append(getMessage(status)).append("]"); + } + if (getDetail(status) != null) { + sb.append(" Detail: [").append(getDetail(status)).append("]"); + } + if (null != subLevel) { + sb.append(" Specific status code which might indicate what the issue is: [").append(subLevel.getValue()).append("]"); + } + return sb.toString(); + } + + protected void checkResponseDestination(StatusResponseType response, String spConfiguredUrl) { + if (spConfiguredUrl.equals(response.getDestination()) == false) { + if (response.isSigned() || Strings.hasText(response.getDestination())) { + throw samlException("SAML response " + response.getID() + " is for destination " + response.getDestination() + + " but this realm uses " + spConfiguredUrl); + } + } + } + + protected void checkStatus(Status status) { + if (status == null || status.getStatusCode() == null) { + throw samlException("SAML Response has no status code"); + } + if (isSuccess(status) == false) { + throw samlException("SAML Response is not a 'success' response: {}", getStatusCodeMessage(status)); + } + } + + protected boolean isSuccess(Status status) { + return StatusCode.SUCCESS.equals(status.getStatusCode().getValue()); + } + + private String getMessage(Status status) { + final StatusMessage sm = status.getStatusMessage(); + return sm == null ? null : sm.getMessage(); + } + + private String getDetail(Status status) { + final StatusDetail sd = status.getStatusDetail(); + return sd == null ? null : SamlUtils.toString(sd.getDOM()); + } +} diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/rest/action/saml/RestSamlCompleteLogoutAction.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/rest/action/saml/RestSamlCompleteLogoutAction.java new file mode 100644 index 0000000000000..dfee78471e236 --- /dev/null +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/rest/action/saml/RestSamlCompleteLogoutAction.java @@ -0,0 +1,87 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +package org.elasticsearch.xpack.security.rest.action.saml; + +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; +import org.elasticsearch.client.node.NodeClient; +import org.elasticsearch.common.ParseField; +import org.elasticsearch.common.Strings; +import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.common.xcontent.ObjectParser; +import org.elasticsearch.common.xcontent.XContentBuilder; +import org.elasticsearch.common.xcontent.XContentParser; +import org.elasticsearch.license.XPackLicenseState; +import org.elasticsearch.rest.BytesRestResponse; +import org.elasticsearch.rest.RestRequest; +import org.elasticsearch.rest.RestResponse; +import org.elasticsearch.rest.RestStatus; +import org.elasticsearch.rest.action.RestBuilderListener; +import org.elasticsearch.xpack.core.security.action.saml.SamlCompleteLogoutAction; +import org.elasticsearch.xpack.core.security.action.saml.SamlCompleteLogoutRequest; +import org.elasticsearch.xpack.core.security.action.saml.SamlCompleteLogoutResponse; + +import java.io.IOException; +import java.util.List; + +import static org.elasticsearch.rest.RestRequest.Method.POST; + +/** + * This Rest endpoint handles SAML LogoutResponse sent from idP with either HTTP-Redirect or HTTP-Post binding. + * For HTTP-Redirect binding, it expects {@link SamlCompleteLogoutRequest#getPayload()} be set to the query + * string of the redirect URI. + * For HTTP-Post binding, it expects {@link SamlCompleteLogoutRequest#getPayload} be set to the value of + * SAMLResponse form parameter, i.e. caller of this API must do the work to extract the SAMLResponse value + * from body of the HTTP-Post request. The value must also be URL decoded if necessary. + */ +public class RestSamlCompleteLogoutAction extends SamlBaseRestHandler{ + + private static final Logger logger = LogManager.getLogger(RestSamlCompleteLogoutAction.class); + + static final ObjectParser + PARSER = new ObjectParser<>("saml_complete_logout", SamlCompleteLogoutRequest::new); + + static { + PARSER.declareStringOrNull(SamlCompleteLogoutRequest::setQueryString, new ParseField("queryString")); + PARSER.declareStringOrNull(SamlCompleteLogoutRequest::setContent, new ParseField("content")); + PARSER.declareStringArray(SamlCompleteLogoutRequest::setValidRequestIds, new ParseField("ids")); + PARSER.declareString(SamlCompleteLogoutRequest::setRealm, new ParseField("realm")); + } + + public RestSamlCompleteLogoutAction(Settings settings, XPackLicenseState licenseState) { + super(settings, licenseState); + } + + @Override + public String getName() { + return "security_saml_complete_logout_action"; + } + + @Override + public List routes() { + return List.of(new Route(POST, "/_security/saml/complete_logout")); + } + + @Override + protected RestChannelConsumer innerPrepareRequest(RestRequest request, NodeClient client) throws IOException { + try (XContentParser parser = request.contentParser()) { + final SamlCompleteLogoutRequest samlCompleteLogoutRequest = PARSER.parse(parser, null); + logger.trace("SAML LogoutResponse: [{}...] [{}...] [{}]", + Strings.cleanTruncate(samlCompleteLogoutRequest.getQueryString(), 128), + Strings.cleanTruncate(samlCompleteLogoutRequest.getContent(), 128), + samlCompleteLogoutRequest.getValidRequestIds()); + return channel -> client.execute(SamlCompleteLogoutAction.INSTANCE, samlCompleteLogoutRequest, + new RestBuilderListener<>(channel) { + @Override + public RestResponse buildResponse(SamlCompleteLogoutResponse response, XContentBuilder builder) throws Exception { + builder.startObject().endObject(); + return new BytesRestResponse(RestStatus.OK, builder); + } + }); + } + } +} diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/rest/action/saml/RestSamlLogoutAction.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/rest/action/saml/RestSamlLogoutAction.java index a7836f9ce059c..311b2c87b4866 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/rest/action/saml/RestSamlLogoutAction.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/rest/action/saml/RestSamlLogoutAction.java @@ -74,6 +74,7 @@ public RestChannelConsumer innerPrepareRequest(RestRequest request, NodeClient c @Override public RestResponse buildResponse(SamlLogoutResponse response, XContentBuilder builder) throws Exception { builder.startObject(); + builder.field("id", response.getRequestId()); builder.field("redirect", response.getRedirectUrl()); builder.endObject(); return new BytesRestResponse(RestStatus.OK, builder); diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/action/saml/TransportSamlLogoutActionTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/action/saml/TransportSamlLogoutActionTests.java index fd22990807cf1..2dbd8e9548b5e 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/action/saml/TransportSamlLogoutActionTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/action/saml/TransportSamlLogoutActionTests.java @@ -262,6 +262,7 @@ public void testLogoutInvalidatesToken() throws Exception { action.doExecute(mock(Task.class), request, listener); final SamlLogoutResponse response = listener.get(); assertThat(response, notNullValue()); + assertThat(response.getRequestId(), notNullValue()); assertThat(response.getRedirectUrl(), notNullValue()); final IndexRequest indexRequest1 = indexRequests.get(0); diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/saml/SamlAuthenticatorTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/saml/SamlAuthenticatorTests.java index f18a8a74ae484..d6bb9a1af7778 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/saml/SamlAuthenticatorTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/saml/SamlAuthenticatorTests.java @@ -8,8 +8,6 @@ import org.apache.logging.log4j.Level; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; -import org.apache.xml.security.Init; -import org.apache.xml.security.encryption.XMLCipher; import org.elasticsearch.ElasticsearchSecurityException; import org.elasticsearch.common.collect.Tuple; import org.elasticsearch.common.logging.Loggers; @@ -19,10 +17,7 @@ import org.elasticsearch.xpack.core.watcher.watch.ClockMock; import org.hamcrest.Matchers; import org.joda.time.DateTime; -import org.junit.AfterClass; import org.junit.Before; -import org.junit.BeforeClass; -import org.opensaml.core.xml.config.XMLObjectProviderRegistrySupport; import org.opensaml.core.xml.schema.XSString; import org.opensaml.core.xml.schema.impl.XSStringBuilder; import org.opensaml.saml.saml2.core.Assertion; @@ -53,40 +48,20 @@ import org.opensaml.xmlsec.encryption.support.DecryptionException; import org.opensaml.xmlsec.encryption.support.EncryptionConstants; import org.opensaml.xmlsec.encryption.support.KeyEncryptionParameters; -import org.opensaml.xmlsec.keyinfo.KeyInfoSupport; -import org.opensaml.xmlsec.signature.Signature; import org.opensaml.xmlsec.signature.support.SignatureException; -import org.opensaml.xmlsec.signature.support.Signer; import org.w3c.dom.Document; import org.w3c.dom.Element; -import org.w3c.dom.NodeList; import org.xml.sax.InputSource; import org.xml.sax.SAXException; -import javax.crypto.Cipher; import javax.crypto.KeyGenerator; import javax.crypto.SecretKey; -import javax.xml.crypto.dsig.CanonicalizationMethod; -import javax.xml.crypto.dsig.DigestMethod; -import javax.xml.crypto.dsig.Reference; -import javax.xml.crypto.dsig.SignatureMethod; -import javax.xml.crypto.dsig.SignedInfo; -import javax.xml.crypto.dsig.Transform; -import javax.xml.crypto.dsig.XMLSignature; -import javax.xml.crypto.dsig.XMLSignatureFactory; -import javax.xml.crypto.dsig.dom.DOMSignContext; -import javax.xml.crypto.dsig.keyinfo.KeyInfo; -import javax.xml.crypto.dsig.keyinfo.KeyInfoFactory; -import javax.xml.crypto.dsig.spec.C14NMethodParameterSpec; -import javax.xml.crypto.dsig.spec.TransformParameterSpec; -import javax.xml.parsers.DocumentBuilder; import javax.xml.parsers.DocumentBuilderFactory; import javax.xml.parsers.ParserConfigurationException; import java.io.IOException; import java.io.StringReader; import java.nio.charset.StandardCharsets; -import java.security.NoSuchAlgorithmException; import java.security.PrivateKey; import java.security.cert.X509Certificate; import java.time.Instant; @@ -103,7 +78,6 @@ import static java.util.Collections.singletonList; import static javax.xml.crypto.dsig.CanonicalizationMethod.EXCLUSIVE; import static javax.xml.crypto.dsig.CanonicalizationMethod.EXCLUSIVE_WITH_COMMENTS; -import static javax.xml.crypto.dsig.Transform.ENVELOPED; import static org.hamcrest.CoreMatchers.not; import static org.hamcrest.Matchers.contains; import static org.hamcrest.Matchers.containsInAnyOrder; @@ -123,71 +97,11 @@ import static org.opensaml.saml.saml2.core.SubjectConfirmation.METHOD_BEARER; import static org.opensaml.saml.saml2.core.SubjectConfirmation.METHOD_HOLDER_OF_KEY; -public class SamlAuthenticatorTests extends SamlTestCase { +public class SamlAuthenticatorTests extends SamlResponseHandlerTests { - private static final String SP_ENTITY_ID = "https://sp.saml.elastic.test/"; - private static final String IDP_ENTITY_ID = "https://idp.saml.elastic.test/"; - private static final String SP_ACS_URL = SP_ENTITY_ID + "sso/post"; private static final String UID_OID = "urn:oid:0.9.2342.19200300.100.1.1"; - private static Tuple idpSigningCertificatePair; - private static Tuple spSigningCertificatePair; - private static List> spEncryptionCertificatePairs; - - private static List supportedAesKeyLengths; - private static List supportedAesTransformations; - - private ClockMock clock; private SamlAuthenticator authenticator; - private String requestId; - private TimeValue maxSkew; - - @BeforeClass - public static void init() throws Exception { - SamlUtils.initialize(LogManager.getLogger(SamlAuthenticatorTests.class)); - // Initialise Apache XML security so that the signDoc methods work correctly. - Init.init(); - } - - @BeforeClass - public static void calculateAesLength() throws NoSuchAlgorithmException { - supportedAesKeyLengths = new ArrayList<>(); - supportedAesTransformations = new ArrayList<>(); - supportedAesKeyLengths.add(128); - supportedAesTransformations.add(XMLCipher.AES_128); - supportedAesTransformations.add(XMLCipher.AES_128_GCM); - if (Cipher.getMaxAllowedKeyLength("AES") > 128) { - supportedAesKeyLengths.add(192); - supportedAesKeyLengths.add(256); - supportedAesTransformations.add(XMLCipher.AES_192); - supportedAesTransformations.add(XMLCipher.AES_192_GCM); - supportedAesTransformations.add(XMLCipher.AES_256); - supportedAesTransformations.add(XMLCipher.AES_256_GCM); - } - } - - /** - * Generating X.509 credentials can be CPU intensive and slow, so we only want to do it once per class. - */ - @BeforeClass - public static void initCredentials() throws Exception { - idpSigningCertificatePair = readRandomKeyPair(randomSigningAlgorithm()); - spSigningCertificatePair = readRandomKeyPair(randomSigningAlgorithm()); - spEncryptionCertificatePairs = Arrays.asList(readKeyPair("ENCRYPTION_RSA_2048"), readKeyPair("ENCRYPTION_RSA_4096")); - } - - private static String randomSigningAlgorithm() { - return randomFrom("RSA", "DSA", "EC"); - } - - @AfterClass - public static void cleanup() { - idpSigningCertificatePair = null; - spSigningCertificatePair = null; - spEncryptionCertificatePairs = null; - supportedAesKeyLengths = null; - supportedAesTransformations = null; - } @Before public void setupAuthenticator() throws Exception { @@ -348,7 +262,7 @@ public void testSuccessfullyParseContentFromEncryptedAndSignedAssertion() throws } public void testSuccessfullyParseContentFromEncryptedAttribute() throws Exception { - final CryptoTransform signer = randomBoolean() ? this::signResponse : this::signResponse; + final CryptoTransform signer = randomBoolean() ? this::signResponse : this::signAssertions; final Instant now = clock.instant(); String xml = getSimpleResponseAsString(now); /** @@ -1251,7 +1165,8 @@ private interface CryptoTransform { } private String signResponse(Response response) throws Exception { - return signResponseElement(response, EXCLUSIVE, SamlAuthenticatorTests.idpSigningCertificatePair, true); + signSignableObject(response, EXCLUSIVE, SamlAuthenticatorTests.idpSigningCertificatePair); + return SamlUtils.getXmlContent(response, false); } private String signResponse(String xml) throws Exception { @@ -1266,42 +1181,6 @@ private String signResponse(String xml, String c14nMethod, Tuple key private String signResponseString(String xml, String c14nMethod, Tuple keyPair, boolean onlyResponse) throws Exception { - return signResponseElement(toResponse(xml), c14nMethod, keyPair, onlyResponse); - } - - private String signResponseElement(Response response, String c14nMethod, Tuple keyPair, - boolean onlyResponse) - throws Exception { - final Signature signature = SamlUtils.buildObject(Signature.class, Signature.DEFAULT_ELEMENT_NAME); - final Credential credential = new BasicCredential(keyPair.v1().getPublicKey(), keyPair.v2()); - final org.opensaml.xmlsec.signature.KeyInfo kf = SamlUtils.buildObject(org.opensaml.xmlsec.signature.KeyInfo.class, - org.opensaml.xmlsec.signature.KeyInfo.DEFAULT_ELEMENT_NAME); - KeyInfoSupport.addCertificate(kf, keyPair.v1()); - signature.setSigningCredential(credential); - signature.setSignatureAlgorithm(getSignatureAlgorithmURI(keyPair.v2())); - signature.setCanonicalizationAlgorithm(c14nMethod); - signature.setKeyInfo(kf); + final Response response = toResponse(xml); if (onlyResponse) { - response.setSignature(signature); + signSignableObject(response, c14nMethod, keyPair); } else { - response.getAssertions().get(0).setSignature(signature); + signSignableObject(response.getAssertions().get(0), c14nMethod, keyPair); } - XMLObjectProviderRegistrySupport.getMarshallerFactory().getMarshaller(response).marshall(response); - Signer.signObject(signature); return SamlUtils.getXmlContent(response, false); } - private void signElement(Element parent, String c14nMethod) throws Exception { - //We need to explicitly set the Id attribute, "ID" is just our convention - parent.setIdAttribute("ID", true); - final String refID = "#" + parent.getAttribute("ID"); - final X509Certificate certificate = idpSigningCertificatePair.v1(); - final PrivateKey privateKey = idpSigningCertificatePair.v2(); - final XMLSignatureFactory fac = XMLSignatureFactory.getInstance("DOM"); - final DigestMethod digestMethod = fac.newDigestMethod(randomFrom(DigestMethod.SHA256, DigestMethod.SHA512), null); - final Transform transform = fac.newTransform(ENVELOPED, (TransformParameterSpec) null); - // We don't "have to" set the reference explicitly since we're using enveloped signatures, but it helps with - // creating the XSW test cases - final Reference reference = fac.newReference(refID, digestMethod, singletonList(transform), null, null); - final SignatureMethod signatureMethod = fac.newSignatureMethod(getSignatureAlgorithmURI(privateKey), null); - final CanonicalizationMethod canonicalizationMethod = fac.newCanonicalizationMethod(c14nMethod, (C14NMethodParameterSpec) null); - - final SignedInfo signedInfo = fac.newSignedInfo(canonicalizationMethod, signatureMethod, singletonList(reference)); - KeyInfoFactory kif = fac.getKeyInfoFactory(); - javax.xml.crypto.dsig.keyinfo.X509Data data = kif.newX509Data(Collections.singletonList(certificate)); - final KeyInfo keyInfo = kif.newKeyInfo(singletonList(data)); - - final DOMSignContext dsc = new DOMSignContext(privateKey, parent); - dsc.setDefaultNamespacePrefix("ds"); - // According to the schema, the signature needs to be placed after the if there is one in the document - // If there are more than one we are dealing with a so we sign the Response and add the - // Signature after the Response - NodeList issuersList = parent.getElementsByTagNameNS(SAML20_NS, "Issuer"); - if (issuersList.getLength() > 0) { - dsc.setNextSibling(issuersList.item(0).getNextSibling()); - } - - final XMLSignature signature = fac.newXMLSignature(signedInfo, keyInfo); - signature.sign(dsc); - } - private Response encryptAssertions(String xml, Tuple keyPair) throws Exception { final Response response = toResponse(xml); final Encrypter samlEncrypter = getEncrypter(keyPair); @@ -1604,10 +1433,6 @@ private String getSimpleResponseFromXmlTemplate(Instant now, String nameId, Stri return NamedFormatter.format(xml, replacements); } - private String randomId() { - return SamlUtils.generateSecureNCName(randomIntBetween(12, 36)); - } - private SamlToken token(String content) { return token(content.getBytes(StandardCharsets.UTF_8)); } diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/saml/SamlLogoutResponseHandlerHttpPostTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/saml/SamlLogoutResponseHandlerHttpPostTests.java new file mode 100644 index 0000000000000..aff3e0aa610b9 --- /dev/null +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/saml/SamlLogoutResponseHandlerHttpPostTests.java @@ -0,0 +1,94 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +package org.elasticsearch.xpack.security.authc.saml; + +import org.elasticsearch.ElasticsearchSecurityException; +import org.elasticsearch.common.unit.TimeValue; +import org.elasticsearch.common.util.NamedFormatter; +import org.elasticsearch.xpack.core.watcher.watch.ClockMock; +import org.junit.Before; +import org.opensaml.saml.saml2.core.LogoutResponse; + +import java.nio.charset.StandardCharsets; +import java.util.Base64; +import java.util.HashMap; +import java.util.List; +import java.util.Map; + +import static java.util.Collections.emptyList; +import static java.util.Collections.emptyMap; +import static javax.xml.crypto.dsig.CanonicalizationMethod.EXCLUSIVE; +import static org.hamcrest.Matchers.containsString; + +public class SamlLogoutResponseHandlerHttpPostTests extends SamlResponseHandlerTests { + + private SamlLogoutResponseHandler samlLogoutResponseHandler; + + @Before + public void setupHandler() { + clock = new ClockMock(); + maxSkew = TimeValue.timeValueMinutes(1); + requestId = randomId(); + samlLogoutResponseHandler = new SamlLogoutResponseHandler(clock, + getIdpConfiguration(() -> buildOpenSamlCredential(idpSigningCertificatePair)), + getSpConfiguration(emptyList()), + maxSkew); + } + + public void testHandlerWorksWithHttpPostBinding() throws Exception { + final String payload = buildLogoutResponsePayload(emptyMap(), true); + samlLogoutResponseHandler.handle(false, payload, List.of(requestId)); + } + + public void testHandlerFailsWithHttpPostBindingAndNoSignature() throws Exception { + final String payload = buildLogoutResponsePayload(emptyMap(), false); + final ElasticsearchSecurityException e = + expectSamlException(() -> samlLogoutResponseHandler.handle(false, payload, List.of(requestId))); + assertThat(e.getMessage(), containsString("is not signed")); + } + + public void testHandlerWillThrowWhenStatusIsNotSuccess() throws Exception { + final Map replacements = new HashMap<>(); + replacements.put("status", "urn:oasis:names:tc:SAML:2.0:status:Requester"); + final String payload = buildLogoutResponsePayload(replacements, true); + final ElasticsearchSecurityException e = + expectSamlException(() -> samlLogoutResponseHandler.handle(false, payload, List.of(requestId))); + assertThat(e.getMessage(), containsString("not a 'success' response")); + } + + private String buildLogoutResponsePayload(Map data, boolean shouldSign) throws Exception { + final String template = "\n" + + "\n" + + " %(IDP_ENTITY_ID)\n" + + " \n" + + " \n" + + " \n" + + ""; + + Map replacements = new HashMap<>(data); + replacements.putIfAbsent("IDP_ENTITY_ID", IDP_ENTITY_ID); + replacements.putIfAbsent("now", clock.instant()); + replacements.putIfAbsent("randomId", requestId); + replacements.putIfAbsent("requestId", requestId); + replacements.putIfAbsent("SP_LOGOUT_URL", SP_LOGOUT_URL); + replacements.putIfAbsent("status", "urn:oasis:names:tc:SAML:2.0:status:Success"); + final String xml = NamedFormatter.format(template, replacements); + final String signed = shouldSign ? signLogoutResponseString(xml) : xml; + return Base64.getEncoder().encodeToString(signed.getBytes(StandardCharsets.UTF_8)); + } + + private String signLogoutResponseString(String xml) throws Exception { + final LogoutResponse logoutResponse = + samlLogoutResponseHandler.buildXmlObject(parseDocument(xml).getDocumentElement(), LogoutResponse.class); + signSignableObject(logoutResponse, EXCLUSIVE, idpSigningCertificatePair); + return SamlUtils.getXmlContent(logoutResponse, false); + } +} diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/saml/SamlLogoutResponseHandlerHttpRedirectTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/saml/SamlLogoutResponseHandlerHttpRedirectTests.java new file mode 100644 index 0000000000000..a8fdc4c50e784 --- /dev/null +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/saml/SamlLogoutResponseHandlerHttpRedirectTests.java @@ -0,0 +1,132 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +package org.elasticsearch.xpack.security.authc.saml; + +import org.elasticsearch.ElasticsearchSecurityException; +import org.elasticsearch.common.unit.TimeValue; +import org.elasticsearch.common.util.set.Sets; +import org.joda.time.DateTime; +import org.junit.AfterClass; +import org.junit.Before; +import org.junit.BeforeClass; +import org.opensaml.saml.saml2.core.Issuer; +import org.opensaml.saml.saml2.core.LogoutResponse; +import org.opensaml.saml.saml2.core.Status; +import org.opensaml.saml.saml2.core.StatusCode; +import org.opensaml.saml.saml2.core.impl.StatusBuilder; +import org.opensaml.saml.saml2.core.impl.StatusCodeBuilder; +import org.opensaml.security.x509.X509Credential; + +import java.net.URI; +import java.net.URISyntaxException; +import java.time.Clock; +import java.util.Collections; +import java.util.List; + +import static org.hamcrest.Matchers.containsString; + +public class SamlLogoutResponseHandlerHttpRedirectTests extends SamlTestCase { + + private static final String IDP_ENTITY_ID = "https://idp.test/"; + private static final String LOGOUT_URL = "https://sp.test/saml/logout"; + + private Clock clock; + private SamlLogoutResponseHandler samlLogoutResponseHandler; + + private static X509Credential credential; + + @BeforeClass + public static void setupCredential() throws Exception { + credential = (X509Credential) buildOpenSamlCredential(readRandomKeyPair()).get(0); + } + + @AfterClass + public static void clearCredential() { + credential = null; + } + + @Before + public void setupHandler() throws Exception { + clock = Clock.systemUTC(); + final IdpConfiguration idp = new IdpConfiguration(IDP_ENTITY_ID, () -> Collections.singletonList(credential)); + final X509Credential spCredential = (X509Credential) buildOpenSamlCredential(readRandomKeyPair()).get(0); + final SigningConfiguration signingConfiguration = new SigningConfiguration(Collections.singleton("*"), spCredential); + final SpConfiguration sp = new SpConfiguration( + "https://sp.test/", + "https://sp.test/saml/asc", + LOGOUT_URL, + signingConfiguration, + List.of(spCredential), + Collections.emptyList()); + samlLogoutResponseHandler = new SamlLogoutResponseHandler(clock, idp, sp, TimeValue.timeValueSeconds(1)); + } + + public void testHandlerWorks() throws URISyntaxException { + final String requestId = SamlUtils.generateSecureNCName(randomIntBetween(8, 30)); + final SigningConfiguration signingConfiguration = new SigningConfiguration(Sets.newHashSet("*"), credential); + final LogoutResponse logoutResponse = SamlUtils.buildObject(LogoutResponse.class, LogoutResponse.DEFAULT_ELEMENT_NAME); + logoutResponse.setDestination(LOGOUT_URL); + logoutResponse.setIssueInstant(new DateTime(clock.millis())); + logoutResponse.setID(SamlUtils.generateSecureNCName(randomIntBetween(8, 30))); + logoutResponse.setInResponseTo(requestId); + logoutResponse.setStatus(buildStatus(StatusCode.SUCCESS)); + + final Issuer issuer = SamlUtils.buildObject(Issuer.class, Issuer.DEFAULT_ELEMENT_NAME); + issuer.setValue(IDP_ENTITY_ID); + logoutResponse.setIssuer(issuer); + final String url = new SamlRedirect(logoutResponse, signingConfiguration).getRedirectUrl(); + samlLogoutResponseHandler.handle(true, new URI(url).getRawQuery(), List.of(requestId)); + } + + public void testHandlerFailsIfStatusIsNotSuccess() { + final String requestId = SamlUtils.generateSecureNCName(randomIntBetween(8, 30)); + final SigningConfiguration signingConfiguration = new SigningConfiguration(Sets.newHashSet("*"), credential); + final LogoutResponse logoutResponse = SamlUtils.buildObject(LogoutResponse.class, LogoutResponse.DEFAULT_ELEMENT_NAME); + logoutResponse.setDestination(LOGOUT_URL); + logoutResponse.setIssueInstant(new DateTime(clock.millis())); + logoutResponse.setID(SamlUtils.generateSecureNCName(randomIntBetween(8, 30))); + logoutResponse.setInResponseTo(requestId); + logoutResponse.setStatus(buildStatus(randomFrom(StatusCode.REQUESTER, StatusCode.RESPONDER))); + + final Issuer issuer = SamlUtils.buildObject(Issuer.class, Issuer.DEFAULT_ELEMENT_NAME); + issuer.setValue(IDP_ENTITY_ID); + logoutResponse.setIssuer(issuer); + final String url = new SamlRedirect(logoutResponse, signingConfiguration).getRedirectUrl(); + + final ElasticsearchSecurityException e = + expectSamlException(() -> samlLogoutResponseHandler.handle(true, new URI(url).getRawQuery(), List.of(requestId))); + assertThat(e.getMessage(), containsString("is not a 'success' response")); + } + + public void testHandlerWillFailWhenQueryStringNotSigned() { + final String requestId = SamlUtils.generateSecureNCName(randomIntBetween(8, 30)); + final SigningConfiguration signingConfiguration = new SigningConfiguration(Sets.newHashSet("*"), null); + final LogoutResponse logoutResponse = SamlUtils.buildObject(LogoutResponse.class, LogoutResponse.DEFAULT_ELEMENT_NAME); + logoutResponse.setDestination(LOGOUT_URL); + logoutResponse.setIssueInstant(new DateTime(clock.millis())); + logoutResponse.setID(SamlUtils.generateSecureNCName(randomIntBetween(8, 30))); + logoutResponse.setInResponseTo(requestId); + logoutResponse.setStatus(buildStatus(randomFrom(StatusCode.REQUESTER, StatusCode.RESPONDER))); + + final Issuer issuer = SamlUtils.buildObject(Issuer.class, Issuer.DEFAULT_ELEMENT_NAME); + issuer.setValue(IDP_ENTITY_ID); + logoutResponse.setIssuer(issuer); + final String url = new SamlRedirect(logoutResponse, signingConfiguration).getRedirectUrl(); + final ElasticsearchSecurityException e = + expectSamlException(() -> samlLogoutResponseHandler.handle(true, new URI(url).getRawQuery(), List.of(requestId))); + assertThat(e.getMessage(), containsString("Query string is not signed, but is required for HTTP-Redirect binding")); + } + + private Status buildStatus(String statusCodeValue) { + final Status status = new StatusBuilder().buildObject(); + final StatusCode statusCode = new StatusCodeBuilder().buildObject(); + statusCode.setValue(statusCodeValue); + status.setStatusCode(statusCode); + return status; + } + +} diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/saml/SamlRequestHandlerTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/saml/SamlObjectHandlerTests.java similarity index 85% rename from x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/saml/SamlRequestHandlerTests.java rename to x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/saml/SamlObjectHandlerTests.java index b5391ba861d0f..2eee62dabe335 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/saml/SamlRequestHandlerTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/saml/SamlObjectHandlerTests.java @@ -14,7 +14,7 @@ import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; -public class SamlRequestHandlerTests extends ESTestCase { +public class SamlObjectHandlerTests extends ESTestCase { public void testXmlObjectToTextWhenExceedsLength() { final int prefixLength = randomIntBetween(10, 30); @@ -28,7 +28,7 @@ public void testXmlObjectToTextWhenExceedsLength() { when(xml.getDOM()).thenReturn(element); when(element.getTextContent()).thenReturn(text); - assertThat(SamlRequestHandler.text(xml, prefixLength, suffixLength), equalTo(prefix + "..." + suffix)); + assertThat(SamlObjectHandler.text(xml, prefixLength, suffixLength), equalTo(prefix + "..." + suffix)); } public void testXmlObjectToTextPrefixOnly() { @@ -41,7 +41,7 @@ public void testXmlObjectToTextPrefixOnly() { when(xml.getDOM()).thenReturn(element); when(element.getTextContent()).thenReturn(text); - assertThat(SamlRequestHandler.text(xml, length, 0), equalTo(prefix + "...")); + assertThat(SamlObjectHandler.text(xml, length, 0), equalTo(prefix + "...")); } public void testXmlObjectToTextWhenShortedThanRequiredLength() { @@ -54,7 +54,7 @@ public void testXmlObjectToTextWhenShortedThanRequiredLength() { when(xml.getDOM()).thenReturn(element); when(element.getTextContent()).thenReturn(text); - assertThat(SamlRequestHandler.text(xml, prefixLength, suffixLength), equalTo(text)); + assertThat(SamlObjectHandler.text(xml, prefixLength, suffixLength), equalTo(text)); } } diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/saml/SamlRealmTestHelper.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/saml/SamlRealmTestHelper.java index c030e96f0900b..e6c7dc373f18e 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/saml/SamlRealmTestHelper.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/saml/SamlRealmTestHelper.java @@ -43,7 +43,8 @@ public static SamlRealm buildRealm(RealmConfig realmConfig, @Nullable X509Creden final SpConfiguration spConfiguration = new SpConfiguration(SP_ENTITY_ID, SP_ACS_URL, SP_LOGOUT_URL, new SigningConfiguration(Collections.singleton("*"), credential), Arrays.asList(credential), Collections.emptyList()); return new SamlRealm(realmConfig, mock(UserRoleMapper.class), mock(SamlAuthenticator.class), - mock(SamlLogoutRequestHandler.class), () -> idpDescriptor, spConfiguration); + mock(SamlLogoutRequestHandler.class), mock(SamlLogoutResponseHandler.class), + () -> idpDescriptor, spConfiguration); } public static void writeIdpMetadata(Path path, String idpEntityId) throws IOException { diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/saml/SamlRealmTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/saml/SamlRealmTests.java index fbe6790412fd2..e5fa620264e3b 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/saml/SamlRealmTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/saml/SamlRealmTests.java @@ -309,7 +309,7 @@ private void initializeRealms(Realm... realms) { public SamlRealm buildRealm(RealmConfig config, UserRoleMapper roleMapper, SamlAuthenticator authenticator, SamlLogoutRequestHandler logoutHandler, EntityDescriptor idp, SpConfiguration sp) throws Exception { try { - return new SamlRealm(config, roleMapper, authenticator, logoutHandler, () -> idp, sp); + return new SamlRealm(config, roleMapper, authenticator, logoutHandler, mock(SamlLogoutResponseHandler.class), () -> idp, sp); } catch (SettingsException e) { logger.info(new ParameterizedMessage("Settings are invalid:\n{}", config.settings().toDelimitedString('\n')), e); throw e; diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/saml/SamlResponseHandlerTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/saml/SamlResponseHandlerTests.java new file mode 100644 index 0000000000000..ccc6b2f978677 --- /dev/null +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/saml/SamlResponseHandlerTests.java @@ -0,0 +1,229 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +package org.elasticsearch.xpack.security.authc.saml; + +import org.apache.logging.log4j.LogManager; +import org.apache.xml.security.Init; +import org.apache.xml.security.encryption.XMLCipher; +import org.elasticsearch.common.collect.Tuple; +import org.elasticsearch.common.unit.TimeValue; +import org.elasticsearch.xpack.core.watcher.watch.ClockMock; +import org.junit.AfterClass; +import org.junit.BeforeClass; +import org.opensaml.core.xml.config.XMLObjectProviderRegistrySupport; +import org.opensaml.saml.common.SignableSAMLObject; +import org.opensaml.security.credential.BasicCredential; +import org.opensaml.security.credential.Credential; +import org.opensaml.security.x509.X509Credential; +import org.opensaml.xmlsec.keyinfo.KeyInfoSupport; +import org.opensaml.xmlsec.signature.Signature; +import org.opensaml.xmlsec.signature.support.Signer; +import org.w3c.dom.Document; +import org.w3c.dom.Element; +import org.w3c.dom.NodeList; +import org.xml.sax.InputSource; +import org.xml.sax.SAXException; + +import java.io.IOException; +import java.io.StringReader; +import java.security.NoSuchAlgorithmException; +import java.security.PrivateKey; +import java.security.cert.X509Certificate; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collections; +import java.util.List; +import java.util.function.Supplier; +import java.util.stream.Collectors; +import javax.crypto.Cipher; +import javax.xml.crypto.dsig.CanonicalizationMethod; +import javax.xml.crypto.dsig.DigestMethod; +import javax.xml.crypto.dsig.Reference; +import javax.xml.crypto.dsig.SignatureMethod; +import javax.xml.crypto.dsig.SignedInfo; +import javax.xml.crypto.dsig.Transform; +import javax.xml.crypto.dsig.XMLSignature; +import javax.xml.crypto.dsig.XMLSignatureFactory; +import javax.xml.crypto.dsig.dom.DOMSignContext; +import javax.xml.crypto.dsig.keyinfo.KeyInfo; +import javax.xml.crypto.dsig.keyinfo.KeyInfoFactory; +import javax.xml.crypto.dsig.spec.C14NMethodParameterSpec; +import javax.xml.crypto.dsig.spec.TransformParameterSpec; +import javax.xml.parsers.DocumentBuilder; +import javax.xml.parsers.DocumentBuilderFactory; +import javax.xml.parsers.ParserConfigurationException; + +import static java.util.Collections.singletonList; +import static javax.xml.crypto.dsig.Transform.ENVELOPED; +import static org.opensaml.saml.common.xml.SAMLConstants.SAML20_NS; + +public class SamlResponseHandlerTests extends SamlTestCase { + protected static final String SP_ENTITY_ID = "https://sp.saml.elastic.test/"; + protected static final String IDP_ENTITY_ID = "https://idp.saml.elastic.test/"; + protected static final String SP_ACS_URL = SP_ENTITY_ID + "sso/post"; + protected static final String SP_LOGOUT_URL = SP_ENTITY_ID + "sso/logout"; + protected static Tuple idpSigningCertificatePair; + protected static Tuple spSigningCertificatePair; + protected static List> spEncryptionCertificatePairs; + protected static List supportedAesKeyLengths; + protected static List supportedAesTransformations; + protected ClockMock clock; + protected String requestId; + protected TimeValue maxSkew; + + @BeforeClass + public static void init() throws Exception { + SamlUtils.initialize(LogManager.getLogger(SamlResponseHandlerTests.class)); + // Initialise Apache XML security so that the signDoc methods work correctly. + Init.init(); + } + + @BeforeClass + public static void calculateAesLength() throws NoSuchAlgorithmException { + supportedAesKeyLengths = new ArrayList<>(); + supportedAesTransformations = new ArrayList<>(); + supportedAesKeyLengths.add(128); + supportedAesTransformations.add(XMLCipher.AES_128); + supportedAesTransformations.add(XMLCipher.AES_128_GCM); + if (Cipher.getMaxAllowedKeyLength("AES") > 128) { + supportedAesKeyLengths.add(192); + supportedAesKeyLengths.add(256); + supportedAesTransformations.add(XMLCipher.AES_192); + supportedAesTransformations.add(XMLCipher.AES_192_GCM); + supportedAesTransformations.add(XMLCipher.AES_256); + supportedAesTransformations.add(XMLCipher.AES_256_GCM); + } + } + + /** + * Generating X.509 credentials can be CPU intensive and slow, so we only want to do it once per class. + */ + @BeforeClass + public static void initCredentials() throws Exception { + idpSigningCertificatePair = readRandomKeyPair(SamlResponseHandlerTests.randomSigningAlgorithm()); + spSigningCertificatePair = readRandomKeyPair(SamlResponseHandlerTests.randomSigningAlgorithm()); + spEncryptionCertificatePairs = Arrays.asList(readKeyPair("RSA_2048"), readKeyPair("RSA_4096")); + } + + protected static String randomSigningAlgorithm() { + return randomFrom("RSA", "DSA", "EC"); + } + + @AfterClass + public static void cleanup() { + idpSigningCertificatePair = null; + spSigningCertificatePair = null; + spEncryptionCertificatePairs = null; + supportedAesKeyLengths = null; + supportedAesTransformations = null; + } + + protected SpConfiguration getSpConfiguration(List reqAuthnCtxClassRef) { + final SigningConfiguration signingConfiguration = new SigningConfiguration( + Collections.singleton("*"), + (X509Credential) buildOpenSamlCredential(spSigningCertificatePair).get(0)); + final List spEncryptionCredentials = buildOpenSamlCredential(spEncryptionCertificatePairs).stream() + .map((cred) -> (X509Credential) cred).collect(Collectors.toList()); + return new SpConfiguration(SP_ENTITY_ID, SP_ACS_URL, SP_LOGOUT_URL, signingConfiguration, spEncryptionCredentials, + reqAuthnCtxClassRef); + } + + protected IdpConfiguration getIdpConfiguration(Supplier> credentials) { + return new IdpConfiguration(IDP_ENTITY_ID, credentials); + } + + protected String randomId() { + return SamlUtils.generateSecureNCName(randomIntBetween(12, 36)); + } + + protected Document parseDocument(String xml) throws ParserConfigurationException, SAXException, IOException { + final DocumentBuilderFactory dbf = DocumentBuilderFactory.newInstance(); + dbf.setNamespaceAware(true); + final DocumentBuilder documentBuilder = dbf.newDocumentBuilder(); + return documentBuilder.parse(new InputSource(new StringReader(xml))); + } + + /** + * Randomly selects digital signature algorithm URI for given private key + * algorithm ({@link PrivateKey#getAlgorithm()}). + * + * @param key + * {@link PrivateKey} + * @return algorithm URI + */ + protected String getSignatureAlgorithmURI(PrivateKey key) { + String algoUri = null; + switch (key.getAlgorithm()) { + case "RSA": + algoUri = randomFrom("http://www.w3.org/2001/04/xmldsig-more#rsa-sha256", + "http://www.w3.org/2001/04/xmldsig-more#rsa-sha512"); + break; + case "DSA": + algoUri = "http://www.w3.org/2009/xmldsig11#dsa-sha256"; + break; + case "EC": + algoUri = randomFrom("http://www.w3.org/2001/04/xmldsig-more#ecdsa-sha256", + "http://www.w3.org/2001/04/xmldsig-more#ecdsa-sha512"); + break; + default: + throw new IllegalArgumentException("Unsupported algorithm : " + key.getAlgorithm() + + " for signature, allowed values for private key algorithm are [RSA, DSA, EC]"); + } + return algoUri; + } + + protected void signElement(Element parent, String c14nMethod) throws Exception { + //We need to explicitly set the Id attribute, "ID" is just our convention + parent.setIdAttribute("ID", true); + final String refID = "#" + parent.getAttribute("ID"); + final X509Certificate certificate = idpSigningCertificatePair.v1(); + final PrivateKey privateKey = idpSigningCertificatePair.v2(); + final XMLSignatureFactory fac = XMLSignatureFactory.getInstance("DOM"); + final DigestMethod digestMethod = fac.newDigestMethod(randomFrom(DigestMethod.SHA256, DigestMethod.SHA512), null); + final Transform transform = fac.newTransform(ENVELOPED, (TransformParameterSpec) null); + // We don't "have to" set the reference explicitly since we're using enveloped signatures, but it helps with + // creating the XSW test cases + final Reference reference = fac.newReference(refID, digestMethod, singletonList(transform), null, null); + final SignatureMethod signatureMethod = fac.newSignatureMethod(getSignatureAlgorithmURI(privateKey), null); + final CanonicalizationMethod canonicalizationMethod = fac.newCanonicalizationMethod(c14nMethod, (C14NMethodParameterSpec) null); + + final SignedInfo signedInfo = fac.newSignedInfo(canonicalizationMethod, signatureMethod, singletonList(reference)); + KeyInfoFactory kif = fac.getKeyInfoFactory(); + javax.xml.crypto.dsig.keyinfo.X509Data data = kif.newX509Data(Collections.singletonList(certificate)); + final KeyInfo keyInfo = kif.newKeyInfo(singletonList(data)); + + final DOMSignContext dsc = new DOMSignContext(privateKey, parent); + dsc.setDefaultNamespacePrefix("ds"); + // According to the schema, the signature needs to be placed after the if there is one in the document + // If there are more than one we are dealing with a so we sign the Response and add the + // Signature after the Response + NodeList issuersList = parent.getElementsByTagNameNS(SAML20_NS, "Issuer"); + if (issuersList.getLength() > 0) { + dsc.setNextSibling(issuersList.item(0).getNextSibling()); + } + + final XMLSignature signature = fac.newXMLSignature(signedInfo, keyInfo); + signature.sign(dsc); + } + + protected void signSignableObject( + SignableSAMLObject signableObject, String c14nMethod, Tuple keyPair) + throws Exception { + final Signature signature = SamlUtils.buildObject(Signature.class, Signature.DEFAULT_ELEMENT_NAME); + final Credential credential = new BasicCredential(keyPair.v1().getPublicKey(), keyPair.v2()); + final org.opensaml.xmlsec.signature.KeyInfo kf = SamlUtils.buildObject(org.opensaml.xmlsec.signature.KeyInfo.class, + org.opensaml.xmlsec.signature.KeyInfo.DEFAULT_ELEMENT_NAME); + KeyInfoSupport.addCertificate(kf, keyPair.v1()); + signature.setSigningCredential(credential); + signature.setSignatureAlgorithm(getSignatureAlgorithmURI(keyPair.v2())); + signature.setCanonicalizationAlgorithm(c14nMethod); + signature.setKeyInfo(kf); + signableObject.setSignature(signature); + XMLObjectProviderRegistrySupport.getMarshallerFactory().getMarshaller(signableObject).marshall(signableObject); + Signer.signObject(signature); + } +}