HADOOP-11385. Prevent cross site scripting attack on JMXJSONServlet. Contributed by Haohui Mai.

hadoop-common-project/hadoop-common/CHANGES.txt

@@ -266,6 +266,9 @@ Release 2.7.0 - UNRELEASED
     HADOOP-11125. Remove redundant tests in TestOsSecureRandom.
     (Masanori Oyama via wheat9)
 
+    HADOOP-11385. Prevent cross site scripting attack on JMXJSONServlet.
+    (wheat9)
+
 Release 2.6.0 - 2014-11-18
 
   INCOMPATIBLE CHANGES

hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/jmx/JMXJsonServlet.java

@@ -17,12 +17,11 @@
 
 package org.apache.hadoop.jmx;
 
-import java.io.IOException;
+import org.apache.commons.logging.Log;
-import java.io.PrintWriter;
+import org.apache.commons.logging.LogFactory;
-import java.lang.management.ManagementFactory;
+import org.apache.hadoop.http.HttpServer2;
-import java.lang.reflect.Array;
+import org.codehaus.jackson.JsonFactory;
-import java.util.Iterator;
+import org.codehaus.jackson.JsonGenerator;
-import java.util.Set;
 
 import javax.management.AttributeNotFoundException;
 import javax.management.InstanceNotFoundException;
@@ -43,12 +42,12 @@ import javax.servlet.ServletException;
 import javax.servlet.http.HttpServlet;
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
-
+import java.io.IOException;
-import org.apache.commons.logging.Log;
+import java.io.PrintWriter;
-import org.apache.commons.logging.LogFactory;
+import java.lang.management.ManagementFactory;
-import org.apache.hadoop.http.HttpServer2;
+import java.lang.reflect.Array;
-import org.codehaus.jackson.JsonFactory;
+import java.util.Iterator;
-import org.codehaus.jackson.JsonGenerator;
+import java.util.Set;
 
 /*
  * This servlet is based off of the JMXProxyServlet from Tomcat 7.0.14. It has
@@ -114,17 +113,16 @@ import org.codehaus.jackson.JsonGenerator;
  *  
  *  The bean's name and modelerType will be returned for all beans.
  *
- *  Optional paramater "callback" should be used to deliver JSONP response.
- *  
  */
 public class JMXJsonServlet extends HttpServlet {
   private static final Log LOG = LogFactory.getLog(JMXJsonServlet.class);
+  static final String ACCESS_CONTROL_ALLOW_METHODS =
+      "Access-Control-Allow-Methods";
+  static final String ACCESS_CONTROL_ALLOW_ORIGIN =
+      "Access-Control-Allow-Origin";
 
   private static final long serialVersionUID = 1L;
 
-  // ----------------------------------------------------- Instance Variables
-  private static final String CALLBACK_PARAM = "callback";
-
   /**
    * MBean server.
    */
@@ -164,17 +162,12 @@ public class JMXJsonServlet extends HttpServlet {
       }
       
       JsonGenerator jg = null;
-
+      try {
-      writer = response.getWriter();
+        writer = response.getWriter();
  
-      // "callback" parameter implies JSONP outpout
-      jsonpcb = request.getParameter(CALLBACK_PARAM);
-      if (jsonpcb != null) {
-        response.setContentType("application/javascript; charset=utf8");
-        writer.write(jsonpcb + "(");
-      } else {
         response.setContentType("application/json; charset=utf8");
-      }
+        response.setHeader(ACCESS_CONTROL_ALLOW_METHODS, "GET");
+        response.setHeader(ACCESS_CONTROL_ALLOW_ORIGIN, "*");
 
       JsonFactory jsonFactory = new JsonFactory();
       jg = jsonFactory.createJsonGenerator(writer);
@@ -209,14 +202,20 @@ public class JMXJsonServlet extends HttpServlet {
         
       }
 
-      // query per mbean
+        // query per mbean
-      String qry = request.getParameter("qry");
+        String qry = request.getParameter("qry");
-      if (qry == null) {
+        if (qry == null) {
-        qry = "*:*";
+          qry = "*:*";
+        }
+        listBeans(jg, new ObjectName(qry), null, response);
+      } finally {
+        if (jg != null) {
+          jg.close();
+        }
+        if (writer != null) {
+          writer.close();
+        }
       }
-      listBeans(jg, new ObjectName(qry), null, response);
-      jg.close();
-
     } catch ( IOException e ) {
       LOG.error("Caught an exception while processing JMX request", e);
       response.setStatus(HttpServletResponse.SC_INTERNAL_SERVER_ERROR);
@@ -224,9 +223,6 @@ public class JMXJsonServlet extends HttpServlet {
       LOG.error("Caught an exception while processing JMX request", e);
       response.setStatus(HttpServletResponse.SC_BAD_REQUEST);
     } finally {
-      if (jsonpcb != null) {
-         writer.write(");");
-      }
       if (writer != null) {
         writer.close();
       }

hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/jmx/TestJMXJsonServlet.java

@@ -18,20 +18,21 @@
 package org.apache.hadoop.jmx;
 
 
-import java.net.URL;
-import java.util.regex.Matcher;
-import java.util.regex.Pattern;
-
-import org.apache.commons.logging.Log;
-import org.apache.commons.logging.LogFactory;
 import org.apache.hadoop.http.HttpServer2;
 import org.apache.hadoop.http.HttpServerFunctionalTest;
 import org.junit.AfterClass;
 import org.junit.BeforeClass;
 import org.junit.Test;
 
+import java.net.HttpURLConnection;
+import java.net.URL;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+
+import static org.apache.hadoop.jmx.JMXJsonServlet.ACCESS_CONTROL_ALLOW_METHODS;
+import static org.apache.hadoop.jmx.JMXJsonServlet.ACCESS_CONTROL_ALLOW_ORIGIN;
+
 public class TestJMXJsonServlet extends HttpServerFunctionalTest {
-  private   static final Log LOG = LogFactory.getLog(TestJMXJsonServlet.class);
   private static HttpServer2 server;
   private static URL baseUrl;
 
@@ -53,54 +54,31 @@ public class TestJMXJsonServlet extends HttpServerFunctionalTest {
   
   @Test public void testQury() throws Exception {
     String result = readOutput(new URL(baseUrl, "/jmx?qry=java.lang:type=Runtime"));
-    LOG.info("/jmx?qry=java.lang:type=Runtime RESULT: "+result);
     assertReFind("\"name\"\\s*:\\s*\"java.lang:type=Runtime\"", result);
     assertReFind("\"modelerType\"", result);
     
     result = readOutput(new URL(baseUrl, "/jmx?qry=java.lang:type=Memory"));
-    LOG.info("/jmx?qry=java.lang:type=Memory RESULT: "+result);
     assertReFind("\"name\"\\s*:\\s*\"java.lang:type=Memory\"", result);
     assertReFind("\"modelerType\"", result);
     
     result = readOutput(new URL(baseUrl, "/jmx"));
-    LOG.info("/jmx RESULT: "+result);
     assertReFind("\"name\"\\s*:\\s*\"java.lang:type=Memory\"", result);
     
     // test to get an attribute of a mbean
     result = readOutput(new URL(baseUrl, 
         "/jmx?get=java.lang:type=Memory::HeapMemoryUsage"));
-    LOG.info("/jmx RESULT: "+result);
     assertReFind("\"name\"\\s*:\\s*\"java.lang:type=Memory\"", result);
     assertReFind("\"committed\"\\s*:", result);
     
     // negative test to get an attribute of a mbean
     result = readOutput(new URL(baseUrl, 
         "/jmx?get=java.lang:type=Memory::"));
-    LOG.info("/jmx RESULT: "+result);
     assertReFind("\"ERROR\"", result);
 
-    // test to get JSONP result
+    // test to CORS headers
-    result = readOutput(new URL(baseUrl, "/jmx?qry=java.lang:type=Memory&callback=mycallback1"));
+    HttpURLConnection conn = (HttpURLConnection)
-    LOG.info("/jmx?qry=java.lang:type=Memory&callback=mycallback RESULT: "+result);
+        new URL(baseUrl, "/jmx?qry=java.lang:type=Memory").openConnection();
-    assertReFind("^mycallback1\\(\\{", result);
+    assertEquals("GET", conn.getHeaderField(ACCESS_CONTROL_ALLOW_METHODS));
-    assertReFind("\\}\\);$", result);
+    assertNotNull(conn.getHeaderField(ACCESS_CONTROL_ALLOW_ORIGIN));
-
-    // negative test to get an attribute of a mbean as JSONP
-    result = readOutput(new URL(baseUrl,
-        "/jmx?get=java.lang:type=Memory::&callback=mycallback2"));
-    LOG.info("/jmx RESULT: "+result);
-    assertReFind("^mycallback2\\(\\{", result);
-    assertReFind("\"ERROR\"", result);
-    assertReFind("\\}\\);$", result);
-
-    // test to get an attribute of a mbean as JSONP
-    result = readOutput(new URL(baseUrl,
-        "/jmx?get=java.lang:type=Memory::HeapMemoryUsage&callback=mycallback3"));
-    LOG.info("/jmx RESULT: "+result);
-    assertReFind("^mycallback3\\(\\{", result);
-    assertReFind("\"name\"\\s*:\\s*\"java.lang:type=Memory\"", result);
-    assertReFind("\"committed\"\\s*:", result);
-    assertReFind("\\}\\);$", result);
-
   }
 }