honeymoose
/
OpenSearch
mirror of https://github.com/honeymoose/OpenSearch.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Compliant SAML Response destination check (#31175) 

Make SAML Response Destination check compliant

Only validate the Destination element of an incoming SAML Response
if Destination is present and the SAML Response is signed.
The standard [1] - 3.5.5.2 and [2] - 3.2.2 does mention that the
Destination element is optional and should only be verified when
the SAML Response is signed. Some Identity Provider implementations
are known to not set a Destination XML Attribute in their SAML
responses when those are not signed, so this change also aims to
enhance interoperability.

[1] https://docs.oasis-open.org/security/saml/v2.0/saml-bindings-2.0-os.pdf
[2] https://docs.oasis-open.org/security/saml/v2.0/saml-core-2.0-os.pdf
This commit is contained in:
Ioannis Kakavas 2018-06-08 20:36:31 +03:00 committed by GitHub
parent 8f607071b6
commit b26aae3915
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 50 additions and 2 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/saml/SamlAuthenticator.java
View File

@ -159,10 +159,12 @@ class SamlAuthenticator extends SamlRequestHandler {
private void checkResponseDestination(Response response) {
final String asc = getSpConfiguration().getAscUrl();
if (asc.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 " + asc);
}
}
}
private Tuple<Assertion, List<Attribute>> extractDetails(Response response, Collection<String> allowedSamlRequestIds,
boolean requireSignedAssertions) {

48
x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/saml/SamlAuthenticatorTests.java
View File

@ -523,13 +523,59 @@ public class SamlAuthenticatorTests extends SamlTestCase {
"</assert:Attribute></assert:AttributeStatement>" +
"</assert:Assertion>" +
"</proto:Response>";
SamlToken token = token(signDoc(xml));
SamlToken token = randomBoolean() ? token(signDoc(xml)) : token(signAssertions(xml, idpSigningCertificatePair));
final ElasticsearchSecurityException exception = expectSamlException(() -> authenticator.authenticate(token));
assertThat(exception.getMessage(), containsString("destination"));
assertThat(exception.getCause(), nullValue());
assertThat(SamlUtils.isSamlException(exception), is(true));
}
public void testMissingDestinationIsNotRejectedForNotSignedResponse() throws Exception {
Instant now = clock.instant();
Instant validUntil = now.plusSeconds(30);
String sessionindex = randomId();
final String xml = "<?xml version='1.0' encoding='UTF-8'?>\n" +
"<proto:Response ID='" + randomId() + "' InResponseTo='" + requestId +
"' IssueInstant='" + now + "' Version='2.0'" +
" xmlns:proto='urn:oasis:names:tc:SAML:2.0:protocol'" +
" xmlns:assert='urn:oasis:names:tc:SAML:2.0:assertion'" +
" xmlns:xsi='http://www.w3.org/2001/XMLSchema-instance'" +
" xmlns:xs='http://www.w3.org/2001/XMLSchema'" +
" xmlns:ds='http://www.w3.org/2000/09/xmldsig#' >" +
"<assert:Issuer>" + IDP_ENTITY_ID + "</assert:Issuer>" +
"<proto:Status><proto:StatusCode Value='urn:oasis:names:tc:SAML:2.0:status:Success'/></proto:Status>" +
"<assert:Assertion ID='" + sessionindex + "' IssueInstant='" + now + "' Version='2.0'>" +
"<assert:Issuer>" + IDP_ENTITY_ID + "</assert:Issuer>" +
"<assert:Subject>" +
"<assert:NameID SPNameQualifier='" + SP_ENTITY_ID + "' Format='" + TRANSIENT + "'>randomopaquestring</assert:NameID>" +
"<assert:SubjectConfirmation Method='" + METHOD_BEARER + "'>" +
"<assert:SubjectConfirmationData NotOnOrAfter='" + validUntil + "' Recipient='" + SP_ACS_URL + "' " +
" InResponseTo='" + requestId + "'/>" +
"</assert:SubjectConfirmation>" +
"</assert:Subject>" +
"<assert:AuthnStatement AuthnInstant='" + now + "' SessionNotOnOrAfter='" + validUntil +
"' SessionIndex='" + sessionindex + "'>" +
"<assert:AuthnContext>" +
"<assert:AuthnContextClassRef>" + PASSWORD_AUTHN_CTX + "</assert:AuthnContextClassRef>" +
"</assert:AuthnContext>" +
"</assert:AuthnStatement>" +
"<assert:AttributeStatement><assert:Attribute " +
" NameFormat='urn:oasis:names:tc:SAML:2.0:attrname-format:uri' Name='urn:oid:0.9.2342.19200300.100.1.1'>" +
"<assert:AttributeValue xsi:type='xs:string'>daredevil</assert:AttributeValue>" +
"</assert:Attribute></assert:AttributeStatement>" +
"</assert:Assertion>" +
"</proto:Response>";
SamlToken token = token(signAssertions(xml, idpSigningCertificatePair));
final SamlAttributes attributes = authenticator.authenticate(token);
assertThat(attributes, notNullValue());
assertThat(attributes.attributes(), iterableWithSize(1));
final List<String> uid = attributes.getAttributeValues("urn:oid:0.9.2342.19200300.100.1.1");
assertThat(uid, contains("daredevil"));
assertThat(uid, iterableWithSize(1));
assertThat(attributes.name(), notNullValue());
assertThat(attributes.name().format, equalTo(TRANSIENT));
}
public void testIncorrectRequestIdIsRejected() throws Exception {
Instant now = clock.instant();
Instant validUntil = now.plusSeconds(30);