KEYCLOAK-16444 Initialize JAXP components consistently

saml-core/src/main/java/org/keycloak/saml/processing/core/util/JAXPValidationUtil.java

@@ -22,6 +22,7 @@ import org.keycloak.saml.common.constants.GeneralConstants;
 import org.keycloak.saml.common.exceptions.ProcessingException;
 import org.keycloak.saml.common.util.DocumentUtil;
 import org.keycloak.saml.common.util.SecurityActions;
+import org.keycloak.saml.common.util.StaxParserUtil;
 import org.keycloak.saml.common.util.SystemPropertiesUtil;
 import org.w3c.dom.Node;
 import org.xml.sax.ErrorHandler;
@@ -37,6 +38,14 @@ import java.io.IOException;
 import java.io.InputStream;
 import java.net.URL;
 import java.util.List;
+import javax.xml.XMLConstants;
+import javax.xml.stream.XMLStreamException;
+import javax.xml.transform.stax.StAXSource;
+import org.xml.sax.SAXNotRecognizedException;
+import org.xml.sax.SAXNotSupportedException;
+import static org.keycloak.saml.common.util.DocumentUtil.feature_disallow_doctype_decl;
+import static org.keycloak.saml.common.util.DocumentUtil.feature_external_general_entities;
+import static org.keycloak.saml.common.util.DocumentUtil.feature_external_parameter_entities;
 
 /**
  * Utility class associated with JAXP Validation
@@ -52,12 +61,12 @@ public class JAXPValidationUtil {
 
     protected static SchemaFactory schemaFactory;
 
-    public static void validate(String str) throws SAXException, IOException {
-        validator().validate(new StreamSource(str));
-    }
-
     public static void validate(InputStream stream) throws SAXException, IOException {
-        validator().validate(new StreamSource(stream));
+        try {
+            validator().validate(new StAXSource(StaxParserUtil.getXMLEventReader(stream)));
+        } catch (XMLStreamException ex) {
+            throw new IOException(ex);
+        }
     }
 
     /**
@@ -86,11 +95,42 @@ public class JAXPValidationUtil {
                 throw logger.nullValueError("schema");
 
             validator = schema.newValidator();
+            // Do not optimize the following into setProperty(...) && setProperty(...).
+            // This way if it fails in the first setProperty, it will try the subsequent setProperty anyway
+            // which it would not due to short-circuiting in case of an && expression.
+            boolean successful1 = setProperty(validator, XMLConstants.ACCESS_EXTERNAL_DTD, "");
+            successful1 &= setProperty(validator, XMLConstants.ACCESS_EXTERNAL_SCHEMA, "");
+            boolean successful2 = setFeature(validator, feature_disallow_doctype_decl, true);
+            successful2 &= setFeature(validator, feature_external_general_entities, false);
+            successful2 &= setFeature(validator, feature_external_parameter_entities, false);
+            if (! successful1 && ! successful2) {
+                logger.warn("Cannot disable external access in XML validator");
+            }
             validator.setErrorHandler(new CustomErrorHandler());
         }
         return validator;
     }
 
+    private static boolean setProperty(Validator v, String property, String value) {
+        try {
+            v.setProperty(property, value);
+        } catch (SAXNotRecognizedException | SAXNotSupportedException ex) {
+            logger.debug("Cannot set " + property);
+            return false;
+        }
+        return true;
+    }
+
+    private static boolean setFeature(Validator v, String feature, boolean value) {
+        try {
+            v.setFeature(feature, value);
+        } catch (SAXNotRecognizedException | SAXNotSupportedException ex) {
+            logger.debug("Cannot set " + feature);
+            return false;
+        }
+        return true;
+    }
+
     private static Schema getSchema() throws IOException {
         boolean tccl_jaxp = SystemPropertiesUtil.getSystemProperty(GeneralConstants.TCCL_JAXP, "false").equalsIgnoreCase("true");
 
@@ -99,7 +139,7 @@ public class JAXPValidationUtil {
             if (tccl_jaxp) {
                 SecurityActions.setTCCL(JAXPValidationUtil.class.getClassLoader());
             }
-            schemaFactory = SchemaFactory.newInstance("http://www.w3.org/2001/XMLSchema");
+            schemaFactory = SchemaFactory.newInstance(XMLConstants.W3C_XML_SCHEMA_NS_URI);
 
             schemaFactory.setResourceResolver(new IDFedLSInputResolver());
             schemaFactory.setErrorHandler(new CustomErrorHandler());

saml-core/src/test/java/org/keycloak/saml/processing/core/util/JAXPValidationUtilTest.java

@@ -0,0 +1,76 @@
+/*
+ * Copyright 2020 Red Hat, Inc. and/or its affiliates
+ * and other contributors as indicated by the @author tags.
+ * 
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ * 
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * 
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.keycloak.saml.processing.core.util;
+
+import java.io.ByteArrayInputStream;
+import java.io.IOException;
+import org.hamcrest.Matcher;
+import org.hamcrest.Matchers;
+import org.junit.Test;
+import org.xml.sax.SAXException;
+import static org.junit.Assert.assertThat;
+
+/**
+ *
+ * @author hmlnarik
+ */
+public class JAXPValidationUtilTest {
+
+    private static final String REQUEST_VALID = "<samlp:AuthnRequest xmlns:samlp=\"urn:oasis:names:tc:SAML:2.0:protocol\" xmlns:saml=\"urn:oasis:names:tc:SAML:2.0:assertion\" ID=\"a123\" Version=\"2.0\" IssueInstant=\"2014-07-16T23:52:45Z\" >" +
+            "<saml:Issuer>urn:test</saml:Issuer>" +
+            "</samlp:AuthnRequest>";
+
+    private static final String REQUEST_FLAWED = "<samlp:AuthnRequest xmlns:samlp=\"urn:oasis:names:tc:SAML:2.0:protocol\" xmlns:saml=\"urn:oasis:names:tc:SAML:2.0:assertion\" ID=\"&heh;\" Version=\"2.0\" IssueInstant=\"2014-07-16T23:52:45Z\" >" +
+            "<saml:Issuer>urn:test</saml:Issuer>" +
+            "</samlp:AuthnRequest>";
+
+    private static final String REQUEST_FLAWED_LOCAL = "<samlp:AuthnRequest xmlns:samlp=\"urn:oasis:names:tc:SAML:2.0:protocol\" xmlns:saml=\"urn:oasis:names:tc:SAML:2.0:assertion\" ID=\"&heh;\" Version=\"2.0\" IssueInstant=\"2014-07-16T23:52:45Z\" >" +
+            "<saml:Issuer>urn:test</saml:Issuer>" +
+            "</samlp:AuthnRequest>";
+
+    private static final String REQUEST_INVALID = "<samlp:InvalidAuthnRequest xmlns:samlp=\"urn:oasis:names:tc:SAML:2.0:protocol\" xmlns:saml=\"urn:oasis:names:tc:SAML:2.0:assertion\" ID=\"a123\" Version=\"2.0\" IssueInstant=\"2014-07-16T23:52:45Z\" >" +
+            "<saml:Issuer>urn:test</saml:Issuer>" +
+            "</samlp:AuthnRequest>";
+
+
+    @Test
+    public void testServerSideValidator() throws Exception {
+        String preamble = "<!DOCTYPE AuthnRequest [" +
+                "<!ELEMENT AuthnRequest (#PCDATA)>" +
+                "<!ENTITY heh SYSTEM \"file:///etc/passwd\">" +
+                "]>";
+
+        assertInputValidation(REQUEST_VALID, Matchers.nullValue());
+
+        assertInputValidation(REQUEST_INVALID, Matchers.notNullValue());
+        assertInputValidation(preamble + REQUEST_FLAWED, Matchers.notNullValue());
+        assertInputValidation(preamble + REQUEST_FLAWED_LOCAL, Matchers.notNullValue());
+        assertInputValidation(preamble + "<AuthnRequest></AuthnRequest>", Matchers.notNullValue());
+    }
+
+    private void assertInputValidation(String s, Matcher<Object> matcher) {
+        String validationResult = null;
+        try {
+            JAXPValidationUtil.validate(new ByteArrayInputStream(s.getBytes()));
+        } catch (SAXException | IOException ex) {
+            validationResult = ex.getMessage();
+        }
+//        log.debugf("Validation result: '%s' for: %s", validationResult, s);
+        assertThat(s, validationResult, matcher);
+    }
+
+}

testsuite/integration-arquillian/servers/app-server/undertow/src/main/java/org/keycloak/testsuite/arquillian/undertow/UndertowAppServer.java

@@ -61,6 +61,7 @@ import org.keycloak.common.util.reflections.Reflections;
 import org.keycloak.testsuite.arquillian.undertow.saml.util.RestSamlApplicationConfig;
 import org.keycloak.testsuite.utils.undertow.UndertowDeployerHelper;
 import org.keycloak.testsuite.utils.undertow.UndertowWarClassLoader;
+import java.io.InputStream;
 
 /**
  * @author <a href="mailto:vramik@redhat.com">Vlasta Ramik</a>
@@ -229,8 +230,12 @@ public class UndertowAppServer implements DeployableContainer<UndertowAppServerC
     }
 
     private boolean isJaxrsApp(WebArchive archive) throws DeploymentException {
-        try {
+        if (! archive.contains("/WEB-INF/web.xml")) {
-            return IOUtils.toString(archive.get("/WEB-INF/web.xml").getAsset().openStream(), Charset.forName("UTF-8"))
+            return false;
+        }
+        try (InputStream stream = archive.get("/WEB-INF/web.xml").getAsset().openStream()) {
+            return 
+              IOUtils.toString(stream, Charset.forName("UTF-8"))
                     .contains(Application.class.getName());
         } catch (IOException e) {
             throw new DeploymentException("Unable to read archive.", e);

testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/saml/SamlXMLAttacksTest.java

@@ -11,14 +11,36 @@ import org.junit.Test;
 import org.keycloak.saml.common.constants.GeneralConstants;
 import org.keycloak.saml.processing.web.util.PostBindingUtil;
 
+import org.keycloak.testsuite.arquillian.annotation.AppServerContainer;
+import org.keycloak.testsuite.utils.arquillian.ContainerConstants;
+import java.io.IOException;
 import java.io.UnsupportedEncodingException;
+import java.net.URL;
 import java.util.LinkedList;
 import java.util.List;
 
+import javax.ws.rs.core.Response;
+import org.hamcrest.Matcher;
+import org.jboss.arquillian.container.test.api.Deployment;
+import org.jboss.arquillian.container.test.api.OperateOnDeployment;
+import org.jboss.arquillian.test.api.ArquillianResource;
+import org.jboss.shrinkwrap.api.ShrinkWrap;
+import org.jboss.shrinkwrap.api.asset.StringAsset;
+import org.jboss.shrinkwrap.api.spec.WebArchive;
 import static org.hamcrest.CoreMatchers.containsString;
 import static org.junit.Assert.assertThat;
 import static org.keycloak.testsuite.util.Matchers.bodyHC;
+import static org.keycloak.testsuite.util.Matchers.statusCodeIsHC;
 
+@AppServerContainer(ContainerConstants.APP_SERVER_UNDERTOW)
+@AppServerContainer(ContainerConstants.APP_SERVER_WILDFLY)
+@AppServerContainer(ContainerConstants.APP_SERVER_WILDFLY_DEPRECATED)
+@AppServerContainer(ContainerConstants.APP_SERVER_EAP)
+@AppServerContainer(ContainerConstants.APP_SERVER_EAP6)
+@AppServerContainer(ContainerConstants.APP_SERVER_EAP71)
+@AppServerContainer(ContainerConstants.APP_SERVER_TOMCAT7)
+@AppServerContainer(ContainerConstants.APP_SERVER_TOMCAT8)
+@AppServerContainer(ContainerConstants.APP_SERVER_TOMCAT9)
 public class SamlXMLAttacksTest extends AbstractSamlTest {
 
     @Test(timeout = 4000)
@@ -64,4 +86,71 @@ public class SamlXMLAttacksTest extends AbstractSamlTest {
         }
     }
 
+    @Deployment(name = "DTD")
+    protected static WebArchive employee() {
+        String attackerDtd = "<!ENTITY % file SYSTEM \"file:///etc/passwd\">\n" +
+                     "<!ENTITY % eval \"<!ENTITY &#x25; error SYSTEM 'file:///nonexistent/%file;'>\">\n" +
+                     "%eval;\n" +
+                     "%error;";
+
+        return ShrinkWrap.create(WebArchive.class, "dtd.war")
+          .add(new StringAsset(attackerDtd), "/attacker.dtd");
+    }
+
+    private void assertBlackboxInputValidation(String s, Matcher<? super CloseableHttpResponse> matcher) throws IOException, RuntimeException {
+        try (CloseableHttpClient client = HttpClientBuilder.create().build()) {
+            HttpPost post = new HttpPost(getAuthServerSamlEndpoint(REALM_NAME));
+
+            List<NameValuePair> parameters = new LinkedList<>();
+            String encoded = PostBindingUtil.base64Encode(s);
+            parameters.add(new BasicNameValuePair(GeneralConstants.SAML_REQUEST_KEY, encoded));
+
+            UrlEncodedFormEntity formEntity;
+            try {
+                formEntity = new UrlEncodedFormEntity(parameters, "UTF-8");
+            } catch (UnsupportedEncodingException e) {
+                throw new RuntimeException(e);
+            }
+
+            post.setEntity(formEntity);
+
+            try (CloseableHttpResponse response = client.execute(post)) {
+                assertThat(response, matcher);
+            }
+        }
+    }
+
+    @Test
+    public void testValidator(@ArquillianResource @OperateOnDeployment("DTD") URL attackerDtdUrl) throws Exception {
+        String preamble = "<!DOCTYPE AuthnRequest [" +
+                "<!ELEMENT AuthnRequest (#PCDATA)>" +
+                "<!ENTITY % sp SYSTEM \"" + attackerDtdUrl + "/attacker.dtd\" >%sp;" +
+                "<!ENTITY heh SYSTEM \"file:///etc/passwd\">" +
+                "]>".replaceAll("//attacker", "/attacker");
+
+        assertBlackboxInputValidation(REQUEST_VALID, statusCodeIsHC(Response.Status.FOUND));
+
+        assertBlackboxInputValidation(REQUEST_INVALID, bodyHC(containsString("Invalid Request")));
+        assertBlackboxInputValidation(preamble + REQUEST_VALID, bodyHC(containsString("Invalid Request")));
+        assertBlackboxInputValidation(preamble + REQUEST_FLAWED, bodyHC(containsString("Invalid Request")));
+        assertBlackboxInputValidation(preamble + REQUEST_FLAWED_LOCAL, bodyHC(containsString("Invalid Request")));
+        assertBlackboxInputValidation(preamble + "<AuthnRequest></AuthnRequest>", bodyHC(containsString("Invalid Request")));
+    }
+
+    private static final String REQUEST_VALID = "<samlp:AuthnRequest xmlns:samlp=\"urn:oasis:names:tc:SAML:2.0:protocol\" xmlns:saml=\"urn:oasis:names:tc:SAML:2.0:assertion\" ID=\"a123\" Version=\"2.0\" IssueInstant=\"2014-07-16T23:52:45Z\" >" +
+            "<saml:Issuer>" + SAML_CLIENT_ID_SALES_POST + "</saml:Issuer>" +
+            "</samlp:AuthnRequest>";
+
+    private static final String REQUEST_FLAWED = "<samlp:AuthnRequest xmlns:samlp=\"urn:oasis:names:tc:SAML:2.0:protocol\" xmlns:saml=\"urn:oasis:names:tc:SAML:2.0:assertion\" ID=\"&sp;\" Version=\"2.0\" IssueInstant=\"2014-07-16T23:52:45Z\" >" +
+            "<saml:Issuer>" + SAML_CLIENT_ID_SALES_POST + "</saml:Issuer>" +
+            "</samlp:AuthnRequest>";
+
+    private static final String REQUEST_FLAWED_LOCAL = "<samlp:AuthnRequest xmlns:samlp=\"urn:oasis:names:tc:SAML:2.0:protocol\" xmlns:saml=\"urn:oasis:names:tc:SAML:2.0:assertion\" ID=\"&heh;\" Version=\"2.0\" IssueInstant=\"2014-07-16T23:52:45Z\" >" +
+            "<saml:Issuer>" + SAML_CLIENT_ID_SALES_POST + "</saml:Issuer>" +
+            "</samlp:AuthnRequest>";
+
+    private static final String REQUEST_INVALID = "<samlp:InvalidAuthnRequest xmlns:samlp=\"urn:oasis:names:tc:SAML:2.0:protocol\" xmlns:saml=\"urn:oasis:names:tc:SAML:2.0:assertion\" ID=\"a123\" Version=\"2.0\" IssueInstant=\"2014-07-16T23:52:45Z\" >" +
+            "<saml:Issuer>" + SAML_CLIENT_ID_SALES_POST + "</saml:Issuer>" +
+            "</samlp:AuthnRequest>";
+
 }