Skip to content

Enhance parsing of StatusCode in SAML Responses #38628

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
jkakavas merged 3 commits into from
Feb 11, 2019
Merged

Enhance parsing of StatusCode in SAML Responses #38628

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f17a74b
Enhance parsing of StatusCode in SAML Responses
jkakavas Feb 8, 2019
7a435a4
Merge remote-tracking branch 'origin/master' into handle-status-code
jkakavas Feb 8, 2019
ab70f12
feedback
jkakavas Feb 11, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 27 additions & 2 deletions ...security/src/main/java/org/elasticsearch/xpack/security/authc/saml/SamlAuthenticator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,8 +106,7 @@ private SamlAttributes authenticateResponse(Element element, Collection<String>
throw samlException("SAML Response has no status code");
}
if (isSuccess(status) == false) {
throw samlException("SAML Response is not a 'success' response: Code={} Message={} Detail={}",
status.getStatusCode().getValue(), getMessage(status), getDetail(status));
throw samlException("SAML Response is not a 'success' response: {}", getStatusCodeMessage(status));
}
checkIssuer(response.getIssuer(), response);
checkResponseDestination(response);
Expand Down Expand Up @@ -137,6 +136,32 @@ private SamlAttributes authenticateResponse(Element element, Collection<String>
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();
Expand Down
71 changes: 36 additions & 35 deletions ...ity/src/test/java/org/elasticsearch/xpack/security/authc/saml/SamlAuthenticatorTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1374,7 +1374,7 @@ public void testContentIsAcceptedIfRestrictedToOurAudience() throws Exception {
}

public void testContentIsRejectedIfNotMarkedAsSuccess() throws Exception {
final String xml = getSimpleResponse(clock.instant()).replace(StatusCode.SUCCESS, StatusCode.REQUESTER);
final String xml = getStatusFailedResponse();
final SamlToken token = token(signDoc(xml));
final ElasticsearchSecurityException exception = expectSamlException(() -> authenticator.authenticate(token));
assertThat(exception.getMessage(), containsString("not a 'success' response"));
Expand Down Expand Up @@ -1408,8 +1408,7 @@ public void testSignatureWrappingAttackOne() throws Exception {
<ForgedAssertion></ForgedAssertion>
</ForgedResponse>
*/
final Element response = (Element) legitimateDocument.
getElementsByTagNameNS(SAML20P_NS, "Response").item(0);
final Element response = (Element) legitimateDocument.getElementsByTagNameNS(SAML20P_NS, "Response").item(0);
final Element clonedResponse = (Element) response.cloneNode(true);
final Element clonedSignature = (Element) clonedResponse.
getElementsByTagNameNS("http://www.w3.org/2000/09/xmldsig#", "Signature").item(0);
Expand Down Expand Up @@ -1443,8 +1442,7 @@ public void testSignatureWrappingAttackTwo() throws Exception {
<ForgedAssertion></ForgedAssertion>
</ForgedResponse>
*/
final Element response = (Element) legitimateDocument.
getElementsByTagNameNS(SAML20P_NS, "Response").item(0);
final Element response = (Element) legitimateDocument.getElementsByTagNameNS(SAML20P_NS, "Response").item(0);
final Element clonedResponse = (Element) response.cloneNode(true);
final Element clonedSignature = (Element) clonedResponse.
getElementsByTagNameNS("http://www.w3.org/2000/09/xmldsig#", "Signature").item(0);
Expand Down Expand Up @@ -1482,8 +1480,7 @@ public void testSignatureWrappingAttackThree() throws Exception {
</LegitimateAssertion>
</Response>
*/
final Element response = (Element) legitimateDocument.
getElementsByTagNameNS(SAML20P_NS, "Response").item(0);
final Element response = (Element) legitimateDocument.getElementsByTagNameNS(SAML20P_NS, "Response").item(0);
final Element assertion = (Element) legitimateDocument.
getElementsByTagNameNS(SAML20_NS, "Assertion").item(0);
final Element forgedAssertion = (Element) assertion.cloneNode(true);
Expand Down Expand Up @@ -1522,10 +1519,8 @@ public void testSignatureWrappingAttackFour() throws Exception {
</ForgedAssertion>
</Response>
*/
final Element response = (Element) legitimateDocument.
getElementsByTagNameNS(SAML20P_NS, "Response").item(0);
final Element assertion = (Element) legitimateDocument.
getElementsByTagNameNS(SAML20_NS, "Assertion").item(0);
final Element response = (Element) legitimateDocument.getElementsByTagNameNS(SAML20P_NS, "Response").item(0);
final Element assertion = (Element) legitimateDocument.getElementsByTagNameNS(SAML20_NS, "Assertion").item(0);
final Element forgedAssertion = (Element) assertion.cloneNode(true);
forgedAssertion.setAttribute("ID", "_forged_assertion_id");
final Element clonedSignature = (Element) forgedAssertion.
Expand Down Expand Up @@ -1559,17 +1554,14 @@ public void testSignatureWrappingAttackFive() throws Exception {
<LegitimateAssertion></LegitimateAssertion>
</Response>
*/
final Element response = (Element) legitimateDocument.
getElementsByTagNameNS(SAML20P_NS, "Response").item(0);
final Element assertion = (Element) legitimateDocument.
getElementsByTagNameNS(SAML20_NS, "Assertion").item(0);
final Element response = (Element) legitimateDocument.getElementsByTagNameNS(SAML20P_NS, "Response").item(0);
final Element assertion = (Element) legitimateDocument.getElementsByTagNameNS(SAML20_NS, "Assertion").item(0);
final Element signature = (Element) assertion.
getElementsByTagNameNS("http://www.w3.org/2000/09/xmldsig#", "Signature").item(0);
getElementsByTagNameNS("http://www.w3.org/2000/09/xmldsig#", "Signature").item(0);
assertion.removeChild(signature);
final Element forgedAssertion = (Element) assertion.cloneNode(true);
forgedAssertion.setAttribute("ID", "_forged_assertion_id");
final Element issuer = (Element) forgedAssertion.
getElementsByTagNameNS(SAML20_NS, "Issuer").item(0);
final Element issuer = (Element) forgedAssertion.getElementsByTagNameNS(SAML20_NS, "Issuer").item(0);
forgedAssertion.insertBefore(signature, issuer.getNextSibling());
response.insertBefore(forgedAssertion, assertion);
final SamlToken forgedToken = token(SamlUtils.toString((legitimateDocument.getDocumentElement())));
Expand Down Expand Up @@ -1598,10 +1590,8 @@ public void testSignatureWrappingAttackSix() throws Exception {
</ForgedAssertion>
</Response>
*/
final Element response = (Element) legitimateDocument.
getElementsByTagNameNS(SAML20P_NS, "Response").item(0);
final Element assertion = (Element) legitimateDocument.
getElementsByTagNameNS(SAML20_NS, "Assertion").item(0);
final Element response = (Element) legitimateDocument.getElementsByTagNameNS(SAML20P_NS, "Response").item(0);
final Element assertion = (Element) legitimateDocument.getElementsByTagNameNS(SAML20_NS, "Assertion").item(0);
final Element forgedAssertion = (Element) assertion.cloneNode(true);
forgedAssertion.setAttribute("ID", "_forged_assertion_id");
final Element signature = (Element) assertion.
Expand All @@ -1610,8 +1600,7 @@ public void testSignatureWrappingAttackSix() throws Exception {
getElementsByTagNameNS("http://www.w3.org/2000/09/xmldsig#", "Signature").item(0);
forgedAssertion.removeChild(forgedSignature);
assertion.removeChild(signature);
final Element issuer = (Element) forgedAssertion.
getElementsByTagNameNS(SAML20_NS, "Issuer").item(0);
final Element issuer = (Element) forgedAssertion.getElementsByTagNameNS(SAML20_NS, "Issuer").item(0);
forgedAssertion.insertBefore(signature, issuer.getNextSibling());
signature.appendChild(assertion);
response.appendChild(forgedAssertion);
Expand Down Expand Up @@ -1642,11 +1631,9 @@ public void testSignatureWrappingAttackSeven() throws Exception {
</LegitimateAssertion>
</Response>
*/
final Element response = (Element) legitimateDocument.
getElementsByTagNameNS(SAML20P_NS, "Response").item(0);
final Element response = (Element) legitimateDocument.getElementsByTagNameNS(SAML20P_NS, "Response").item(0);
final Element extensions = legitimateDocument.createElement("Extensions");
final Element assertion = (Element) legitimateDocument.
getElementsByTagNameNS(SAML20_NS, "Assertion").item(0);
final Element assertion = (Element) legitimateDocument.getElementsByTagNameNS(SAML20_NS, "Assertion").item(0);
response.insertBefore(extensions, assertion);
final Element forgedAssertion = (Element) assertion.cloneNode(true);
forgedAssertion.setAttribute("ID", "_forged_assertion_id");
Expand Down Expand Up @@ -1683,10 +1670,8 @@ public void testSignatureWrappingAttackEight() throws Exception {
</ForgedAssertion>
</Response>
*/
final Element response = (Element) legitimateDocument.
getElementsByTagNameNS(SAML20P_NS, "Response").item(0);
final Element assertion = (Element) legitimateDocument.
getElementsByTagNameNS(SAML20_NS, "Assertion").item(0);
final Element response = (Element) legitimateDocument.getElementsByTagNameNS(SAML20P_NS, "Response").item(0);
final Element assertion = (Element) legitimateDocument.getElementsByTagNameNS(SAML20_NS, "Assertion").item(0);
final Element forgedAssertion = (Element) assertion.cloneNode(true);
forgedAssertion.setAttribute("ID", "_forged_assertion_id");
final Element signature = (Element) assertion.
Expand All @@ -1695,8 +1680,7 @@ public void testSignatureWrappingAttackEight() throws Exception {
getElementsByTagNameNS("http://www.w3.org/2000/09/xmldsig#", "Signature").item(0);
forgedAssertion.removeChild(forgedSignature);
assertion.removeChild(signature);
final Element issuer = (Element) forgedAssertion.
getElementsByTagNameNS(SAML20_NS, "Issuer").item(0);
final Element issuer = (Element) forgedAssertion.getElementsByTagNameNS(SAML20_NS, "Issuer").item(0);
forgedAssertion.insertBefore(signature, issuer.getNextSibling());
Element object = legitimateDocument.createElement("Object");
object.appendChild(assertion);
Expand Down Expand Up @@ -2034,7 +2018,7 @@ private void encryptElement(Element element, X509Certificate certificate, boolea
}

private Element buildEncryptedKeyElement(Document document, EncryptedKey encryptedKey, X509Certificate certificate)
throws XMLSecurityException {
throws XMLSecurityException {
final XMLCipher cipher = XMLCipher.getInstance();
final org.apache.xml.security.keys.KeyInfo keyInfo = new org.apache.xml.security.keys.KeyInfo(document);
final X509Data x509Data = new X509Data(document);
Expand All @@ -2054,6 +2038,23 @@ private Response toResponse(String xml) throws SAXException, IOException, Parser
return authenticator.buildXmlObject(doc.getDocumentElement(), Response.class);
}

private String getStatusFailedResponse() {
final Instant now = clock.instant();
return "<?xml version='1.0' encoding='UTF-8'?>\n" +
"<proto:Response Destination='" + SP_ACS_URL + "' 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:Requester'>" +
"<proto:StatusCode Value='urn:oasis:names:tc:SAML:2.0:status:InvalidNameIDPolicy'/></proto:StatusCode>" +
"</proto:Status>" +
"</proto:Response>";
}

private String getSimpleResponse(Instant now) {
return getSimpleResponse(now, randomAlphaOfLengthBetween(12, 18), randomId());
}
Expand Down