KNOX-1308 - Implement safeguards against XML entity injection/expansion in the Admin API
authorPhil Zampino <pzampino@apache.org>
Mon, 14 May 2018 18:12:59 +0000 (14:12 -0400)
committerPhil Zampino <pzampino@apache.org>
Mon, 14 May 2018 18:12:59 +0000 (14:12 -0400)
gateway-service-admin/src/main/java/org/apache/knox/gateway/service/admin/TopologyMarshaller.java
gateway-test/src/test/java/org/apache/knox/gateway/GatewayAdminTopologyFuncTest.java

index 6d25982..dd95af9 100644 (file)
@@ -40,6 +40,10 @@ import javax.xml.bind.JAXBContext;
 import javax.xml.bind.JAXBException;
 import javax.xml.bind.Unmarshaller;
 import javax.xml.bind.Marshaller;
+import javax.xml.stream.XMLInputFactory;
+import javax.xml.stream.XMLStreamException;
+import javax.xml.stream.XMLStreamReader;
+import javax.xml.transform.stream.StreamSource;
 
 @Provider
 @Consumes({MediaType.APPLICATION_XML, MediaType.APPLICATION_JSON})
@@ -57,7 +61,13 @@ public class TopologyMarshaller implements MessageBodyWriter<Topology>, MessageB
   }
 
   @Override
-  public void writeTo(Topology instance, Class<?> type, Type genericType, Annotation[] annotations, MediaType mediaType, MultivaluedMap<String, Object> httpHeaders, OutputStream entityStream) throws IOException, WebApplicationException {
+  public void writeTo(Topology                       instance,
+                      Class<?>                       type,
+                      Type                           genericType,
+                      Annotation[]                   annotations,
+                      MediaType                      mediaType,
+                      MultivaluedMap<String, Object> httpHeaders,
+                      OutputStream                   entityStream) throws IOException, WebApplicationException {
     try {
       Map<String, Object> properties = new HashMap<>(1);
       properties.put( JAXBContextProperties.MEDIA_TYPE, mediaType.toString());
@@ -75,26 +85,43 @@ public class TopologyMarshaller implements MessageBodyWriter<Topology>, MessageB
   ///
   @Override
   public boolean isReadable(Class<?> type, Type genericType, Annotation[] annotations, MediaType mediaType) {
-    boolean readable = (type == Topology.class);
-    return readable;
+    return (type == Topology.class);
   }
 
   @Override
-  public Topology readFrom(Class<Topology> type, Type genericType, Annotation[] annotations, MediaType mediaType, MultivaluedMap<String, String> httpHeaders, InputStream entityStream) throws IOException, WebApplicationException {
+  public Topology readFrom(Class<Topology>                type,
+                           Type                           genericType,
+                           Annotation[]                   annotations,
+                           MediaType                      mediaType,
+                           MultivaluedMap<String, String> httpHeaders,
+                           InputStream                    entityStream) throws IOException, WebApplicationException {
+    Topology topology = null;
+
     try {
-      if(isReadable(type, genericType, annotations, mediaType)) {
+      if (isReadable(type, genericType, annotations, mediaType)) {
         Map<String, Object> properties = Collections.emptyMap();
         JAXBContext context = JAXBContext.newInstance(new Class[]{Topology.class}, properties);
-        InputStream is = entityStream;
+
         Unmarshaller u = context.createUnmarshaller();
         u.setProperty(UnmarshallerProperties.MEDIA_TYPE, mediaType.getType() + "/" + mediaType.getSubtype());
-        Topology topology = (Topology)u.unmarshal(is);
-        return topology;
+
+        if (mediaType.isCompatible(MediaType.APPLICATION_XML_TYPE)) {
+          // Safeguard against entity injection (KNOX-1308)
+          XMLInputFactory xif = XMLInputFactory.newFactory();
+          xif.setProperty(XMLInputFactory.IS_SUPPORTING_EXTERNAL_ENTITIES, false);
+          xif.setProperty(XMLInputFactory.SUPPORT_DTD, false);
+          xif.setProperty(XMLInputFactory.IS_REPLACING_ENTITY_REFERENCES, false);
+          XMLStreamReader xsr = xif.createXMLStreamReader(new StreamSource(entityStream));
+          topology = (Topology) u.unmarshal(xsr);
+        } else {
+          topology = (Topology) u.unmarshal(entityStream);
+        }
       }
-    } catch (JAXBException e) {
+    } catch (XMLStreamException | JAXBException e) {
       throw new IOException(e);
     }
-    return null;
+
+    return topology;
   }
 
 }
index ae9371d..b7eefb1 100644 (file)
@@ -72,6 +72,7 @@ import static org.hamcrest.CoreMatchers.nullValue;
 import static org.hamcrest.xml.HasXPath.hasXPath;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertThat;
 import static org.junit.Assert.fail;
 
@@ -732,6 +733,170 @@ public class GatewayAdminTopologyFuncTest {
   }
 
   @Test( timeout = TestUtils.LONG_TIMEOUT )
+  public void testPutTopologyWithEntityInjection() throws Exception {
+    LOG_ENTER() ;
+
+    final String MALICIOUS_PARAM_NAME = "exposed";
+
+    final String XML_WITH_INJECTION =
+        "<?xml version=\"1.0\" encoding=\"utf-8\"?>\n" +
+        "<!DOCTYPE foo [<!ENTITY xxeiltvf SYSTEM \"file:///etc/hosts\"> ]>\n" +
+        "<topology>\n" +
+        "    <gateway>\n" +
+        "        <provider>\n" +
+        "            <role>authentication</role>\n" +
+        "            <name>ShiroProvider</name>\n" +
+        "            <enabled>true</enabled>\n" +
+        "            <param>\n" +
+        "                <name>" + MALICIOUS_PARAM_NAME + "</name>\n" +
+        "                <value>&xxeiltvf;</value>\n" +
+        "            </param>\n" +
+        "            <param>\n" +
+        "                <name>sessionTimeout</name>\n" +
+        "                <value>30</value>\n" +
+        "            </param>\n" +
+        "            <param>\n" +
+        "                <name>main.ldapRealm</name>\n" +
+        "                <value>org.apache.knox.gateway.shirorealm.KnoxLdapRealm</value>\n" +
+        "            </param>\n" +
+        "            <param>\n" +
+        "                <name>main.ldapContextFactory</name>\n" +
+        "                <value>org.apache.knox.gateway.shirorealm.KnoxLdapContextFactory</value>\n" +
+        "            </param>\n" +
+        "            <param>\n" +
+        "                <name>main.ldapRealm.contextFactory</name>\n" +
+        "                <value>$ldapContextFactory</value>\n" +
+        "            </param>\n" +
+        "            <param>\n" +
+        "                <name>main.ldapRealm.userDnTemplate</name>\n" +
+        "                <value>uid={0},ou=people,dc=hadoop,dc=apache,dc=org</value>\n" +
+        "            </param>\n" +
+        "            <param>\n" +
+        "                <name>main.ldapRealm.contextFactory.url</name>\n" +
+        "                <value>ldap://localhost:33389</value>\n" +
+        "            </param>\n" +
+        "            <param>\n" +
+        "                <name>main.ldapRealm.contextFactory.authenticationMechanism</name>\n" +
+        "                <value>simple</value>\n" +
+        "            </param>\n" +
+        "            <param>\n" +
+        "                <name>urls./**</name>\n" +
+        "                <value>authcBasic</value>\n" +
+        "            </param>\n" +
+        "        </provider>\n" +
+        "    </gateway>\n" +
+        "    <service>\n" +
+        "        <role>NAMENODE</role>\n" +
+        "        <url>hdfs://localhost:8020</url>\n" +
+        "    </service>\n" +
+        "</topology>";
+
+    String username = "admin";
+    String password = "admin-password";
+    String url = clusterUrl + "/api/v1/topologies/test-put-with-entity-injection";
+
+    // Should get a response with missing entity reference value because of the entity injection prevention safeguard
+    String XML_RESPONSE = given().auth().preemptive().basic(username, password)
+                                 .contentType(MediaType.APPLICATION_XML)
+                                 .header("Accept", MediaType.APPLICATION_XML)
+                                 .body(XML_WITH_INJECTION)
+                                 .then()
+                                 .statusCode(HttpStatus.SC_OK)
+                                 .when().put(url).getBody().asString();
+
+    Document doc = XmlUtils.readXml(new InputSource(new StringReader(XML_RESPONSE)));
+    assertNotNull(doc);
+
+    assertThat(doc, hasXPath("/topology/gateway/provider[1]/param/name", containsString("exposed")));
+    assertThat(doc, hasXPath("/topology/gateway/provider[1]/param[\"" + MALICIOUS_PARAM_NAME + "\"]/value", is("")));
+
+    LOG_EXIT();
+  }
+
+
+  @Test( timeout = TestUtils.LONG_TIMEOUT )
+  public void testPutTopologyWithEntityExpansion() throws Exception {
+    LOG_ENTER() ;
+
+    final String MALICIOUS_PARAM_NAME = "expanded";
+
+    final String XML_WITH_INJECTION =
+        "<?xml version=\"1.0\" encoding=\"utf-8\"?>\n" +
+            "<!DOCTYPE foo [<!ENTITY xeevowya0 \"b68et\"><!ENTITY xeevowya1 \"&xeevowya0;&xeevowya0;\"><!ENTITY xeevowya2 \"&xeevowya1;&xeevowya1;\"><!ENTITY xeevowya3 \"&xeevowya2;&xeevowya2;\">]>\n" +
+            "<topology>\n" +
+            "    <gateway>\n" +
+            "        <provider>\n" +
+            "            <role>authentication</role>\n" +
+            "            <name>ShiroProvider</name>\n" +
+            "            <enabled>true</enabled>\n" +
+            "            <param>\n" +
+            "                <name>" + MALICIOUS_PARAM_NAME + "</name>\n" +
+            "                <value>&xeevowya3;</value>\n" +
+            "            </param>\n" +
+            "            <param>\n" +
+            "                <name>sessionTimeout</name>\n" +
+            "                <value>30</value>\n" +
+            "            </param>\n" +
+            "            <param>\n" +
+            "                <name>main.ldapRealm</name>\n" +
+            "                <value>org.apache.knox.gateway.shirorealm.KnoxLdapRealm</value>\n" +
+            "            </param>\n" +
+            "            <param>\n" +
+            "                <name>main.ldapContextFactory</name>\n" +
+            "                <value>org.apache.knox.gateway.shirorealm.KnoxLdapContextFactory</value>\n" +
+            "            </param>\n" +
+            "            <param>\n" +
+            "                <name>main.ldapRealm.contextFactory</name>\n" +
+            "                <value>$ldapContextFactory</value>\n" +
+            "            </param>\n" +
+            "            <param>\n" +
+            "                <name>main.ldapRealm.userDnTemplate</name>\n" +
+            "                <value>uid={0},ou=people,dc=hadoop,dc=apache,dc=org</value>\n" +
+            "            </param>\n" +
+            "            <param>\n" +
+            "                <name>main.ldapRealm.contextFactory.url</name>\n" +
+            "                <value>ldap://localhost:33389</value>\n" +
+            "            </param>\n" +
+            "            <param>\n" +
+            "                <name>main.ldapRealm.contextFactory.authenticationMechanism</name>\n" +
+            "                <value>simple</value>\n" +
+            "            </param>\n" +
+            "            <param>\n" +
+            "                <name>urls./**</name>\n" +
+            "                <value>authcBasic</value>\n" +
+            "            </param>\n" +
+            "        </provider>\n" +
+            "    </gateway>\n" +
+            "    <service>\n" +
+            "        <role>NAMENODE</role>\n" +
+            "        <url>hdfs://localhost:8020</url>\n" +
+            "    </service>\n" +
+            "</topology>";
+
+    String username = "admin";
+    String password = "admin-password";
+    String url = clusterUrl + "/api/v1/topologies/test-put-with-entity-injection";
+
+    // Should get a HTTP 500 response because of the entity injection prevention safeguard
+    String XML_RESPONSE = given().auth().preemptive().basic(username, password)
+                                 .contentType(MediaType.APPLICATION_XML)
+                                 .header("Accept", MediaType.APPLICATION_XML)
+                                 .body(XML_WITH_INJECTION)
+                                 .then()
+                                 .statusCode(HttpStatus.SC_OK)
+                                 .when().put(url).getBody().asString();
+
+    Document doc = XmlUtils.readXml(new InputSource(new StringReader(XML_RESPONSE)));
+    assertNotNull(doc);
+
+    assertThat(doc, hasXPath("/topology/gateway/provider[1]/param/name", containsString("expanded")));
+    assertThat(doc, hasXPath("/topology/gateway/provider[1]/param[\"" + MALICIOUS_PARAM_NAME + "\"]/value", is("")));
+
+    LOG_EXIT();
+  }
+
+
+  @Test( timeout = TestUtils.LONG_TIMEOUT )
   public void testXForwardedHeaders() {
     LOG_ENTER();