knox-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From pzamp...@apache.org
Subject [4/4] knox git commit: KNOX-1308 - Implement safeguards against XML entity injection/expansion in the Admin API
Date Mon, 14 May 2018 18:46:33 GMT
KNOX-1308 - Implement safeguards against XML entity injection/expansion in the Admin API


Project: http://git-wip-us.apache.org/repos/asf/knox/repo
Commit: http://git-wip-us.apache.org/repos/asf/knox/commit/7439e696
Tree: http://git-wip-us.apache.org/repos/asf/knox/tree/7439e696
Diff: http://git-wip-us.apache.org/repos/asf/knox/diff/7439e696

Branch: refs/heads/master
Commit: 7439e6968d5ad9241ca242b04248080ea7e8c996
Parents: b83a8bc
Author: Phil Zampino <pzampino@apache.org>
Authored: Mon May 14 14:12:59 2018 -0400
Committer: Phil Zampino <pzampino@apache.org>
Committed: Mon May 14 14:12:59 2018 -0400

----------------------------------------------------------------------
 .../service/admin/TopologyMarshaller.java       |  47 ++++--
 .../gateway/GatewayAdminTopologyFuncTest.java   | 165 +++++++++++++++++++
 2 files changed, 202 insertions(+), 10 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/knox/blob/7439e696/gateway-service-admin/src/main/java/org/apache/knox/gateway/service/admin/TopologyMarshaller.java
----------------------------------------------------------------------
diff --git a/gateway-service-admin/src/main/java/org/apache/knox/gateway/service/admin/TopologyMarshaller.java
b/gateway-service-admin/src/main/java/org/apache/knox/gateway/service/admin/TopologyMarshaller.java
index 6d25982..dd95af9 100644
--- a/gateway-service-admin/src/main/java/org/apache/knox/gateway/service/admin/TopologyMarshaller.java
+++ b/gateway-service-admin/src/main/java/org/apache/knox/gateway/service/admin/TopologyMarshaller.java
@@ -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;
   }
 
 }

http://git-wip-us.apache.org/repos/asf/knox/blob/7439e696/gateway-test/src/test/java/org/apache/knox/gateway/GatewayAdminTopologyFuncTest.java
----------------------------------------------------------------------
diff --git a/gateway-test/src/test/java/org/apache/knox/gateway/GatewayAdminTopologyFuncTest.java
b/gateway-test/src/test/java/org/apache/knox/gateway/GatewayAdminTopologyFuncTest.java
index ae9371d..b7eefb1 100644
--- a/gateway-test/src/test/java/org/apache/knox/gateway/GatewayAdminTopologyFuncTest.java
+++ b/gateway-test/src/test/java/org/apache/knox/gateway/GatewayAdminTopologyFuncTest.java
@@ -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();
 


Mime
View raw message