CVE-2014-0168

@@ -223,10 +223,11 @@ public boolean isRemoteAccessAllowed(String pRemoteHost, String pRemoteAddr) {
      * Check whether CORS access is allowed for the given origin.
      *
      * @param pOrigin origin URL which needs to be checked
-     * @return true if icors access is allowed
+     * @param pStrictChecking whether to a strict check (i.e. server side check)
+     * @return true if if cors access is allowed
      */
-    public boolean isCorsAccessAllowed(String pOrigin) {
-        return restrictor.isCorsAccessAllowed(pOrigin);
+    public boolean isOriginAllowed(String pOrigin,boolean pStrictChecking) {
+        return restrictor.isOriginAllowed(pOrigin, pStrictChecking);
     }
 
     /**

@@ -278,7 +278,8 @@ private void handle(ServletRequestHandler pReqHandler,HttpServletRequest pReq, H
         JSONAware json = null;
         try {
             // Check access policy
-            requestHandler.checkClientIPAccess(pReq.getRemoteHost(),pReq.getRemoteAddr());
+            requestHandler.checkAccess(pReq.getRemoteHost(), pReq.getRemoteAddr(),
+                                       getOriginOrReferer(pReq));
 
             // Remember the agent URL upon the first request. Needed for discovery
             updateAgentUrlIfNeeded(pReq);
@@ -304,6 +305,15 @@ private void handle(ServletRequestHandler pReqHandler,HttpServletRequest pReq, H
         }
     }
 
+    private String getOriginOrReferer(HttpServletRequest pReq) {
+        String origin = pReq.getHeader("Origin");
+        if (origin == null) {
+            origin = pReq.getHeader("Referer");
+        }
+        return origin != null ? origin.replaceAll("[\\n\\r]*","") : null;
+    }
+
+
     // Update the agent URL in the agent details if not already done
     private void updateAgentUrlIfNeeded(HttpServletRequest pReq) {
         // Lookup the Agent URL if needed

@@ -153,7 +153,7 @@ public JSONAware handlePostRequest(String pUri, InputStream pInputStream, String
      */
     public Map<String, String> handleCorsPreflightRequest(String pOrigin, String pRequestHeaders) {
         Map<String,String> ret = new HashMap<String, String>();
-        if (pOrigin != null && backendManager.isCorsAccessAllowed(pOrigin)) {
+        if (pOrigin != null && backendManager.isOriginAllowed(pOrigin,false)) {
             // CORS is allowed, we set exactly the origin in the header, so there are no problems with authentication
             ret.put("Access-Control-Allow-Origin","null".equals(pOrigin) ? "*" : pOrigin);
             if (pRequestHeaders != null) {
@@ -277,15 +277,19 @@ public JSONObject getErrorJSON(int pErrorCode, Throwable pExp, JmxRequest pJmxRe
      *
      * @param pHost host to check
      * @param pAddress address to check
+     * @param pOrigin (optional) origin header to check also.
      */
-    public void checkClientIPAccess(String pHost, String pAddress) {
+    public void checkAccess(String pHost, String pAddress, String pOrigin) {
         if (!backendManager.isRemoteAccessAllowed(pHost,pAddress)) {
             throw new SecurityException("No access from client " + pAddress + " allowed");
         }
+        if (pOrigin != null && !backendManager.isOriginAllowed(pOrigin,true)) {
+            throw new SecurityException("Origin " + pOrigin + " is not allowed to call this agent");
+        }
     }
 
     /**
-     * Check whether for the given host is a cross-browser request allowed. This check is deligated to the
+     * Check whether for the given host is a cross-browser request allowed. This check is delegated to the
      * backendmanager which is responsible for the security configuration.
      * Also, some sanity checks are applied.
      *
@@ -296,7 +300,7 @@ public String extractCorsOrigin(String pOrigin) {
         if (pOrigin != null) {
             // Prevent HTTP response splitting attacks
             String origin  = pOrigin.replaceAll("[\\n\\r]*","");
-            if (backendManager.isCorsAccessAllowed(origin)) {
+            if (backendManager.isOriginAllowed(origin,false)) {
                 return "null".equals(origin) ? "*" : origin;
             } else {
                 return null;

@@ -73,7 +73,7 @@ public final boolean isRemoteAccessAllowed(String... pHostOrAddress) {
     }
 
     /** {@inheritDoc} */
-    public boolean isCorsAccessAllowed(String pOrigin) {
+    public boolean isOriginAllowed(String pOrigin, boolean pIsStrictCheck) {
         return isAllowed;
     }
 }

@@ -30,6 +30,8 @@
  */
 public class CorsChecker extends AbstractChecker<String> {
 
+    private boolean strictChecking = false;
+
     private List<Pattern> patterns;
 
     /**
@@ -39,6 +41,8 @@ public class CorsChecker extends AbstractChecker<String> {
      *     &lt;cors&gt;
      *       &lt;allow-origin&gt;http://jolokia.org&lt;allow-origin&gt;
      *       &lt;allow-origin&gt;*://*.jmx4perl.org&gt;
+     *
+     *       &lt;strict-checking/&gt;
      *     &lt;/cors&gt;
      * </pre>
      *
@@ -56,10 +60,14 @@ public CorsChecker(Document pDoc) {
                     if (node.getNodeType() != Node.ELEMENT_NODE) {
                         continue;
                     }
-                    assertNodeName(node,"allow-origin");
-                    String p = node.getTextContent().trim().toLowerCase();
-                    p = Pattern.quote(p).replace("*","\\E.*\\Q");
-                    patterns.add(Pattern.compile("^" + p + "$"));
+                    assertNodeName(node,"allow-origin","strict-checking");
+                    if (node.getNodeName().equals("allow-origin")) {
+                        String p = node.getTextContent().trim().toLowerCase();
+                        p = Pattern.quote(p).replace("*", "\\E.*\\Q");
+                        patterns.add(Pattern.compile("^" + p + "$"));
+                    } else if (node.getNodeName().equals("strict-checking")) {
+                        strictChecking = true;
+                    }
                 }
             }
         }
@@ -68,11 +76,21 @@ public CorsChecker(Document pDoc) {
     /** {@inheritDoc} */
     @Override
     public boolean check(String pArg) {
+        return check(pArg,false);
+    }
+
+    public boolean check(String pOrigin, boolean pIsStrictCheck) {
+        // Method called during strict checking but we have not configured that
+        // So the check passes always.
+        if (pIsStrictCheck && !strictChecking) {
+            return true;
+        }
+
         if (patterns == null || patterns.size() == 0) {
             return true;
         }
         for (Pattern pattern : patterns) {
-            if (pattern.matcher(pArg).matches()) {
+            if (pattern.matcher(pOrigin).matches()) {
                 return true;
             }
         }

@@ -99,8 +99,8 @@ public boolean isRemoteAccessAllowed(String ... pHostOrAddress) {
     }
 
     /** {@inheritDoc} */
-    public boolean isCorsAccessAllowed(String pOrigin) {
-        return corsChecker.check(pOrigin);
+    public boolean isOriginAllowed(String pOrigin, boolean pIsStrictCheck) {
+        return corsChecker.check(pOrigin,pIsStrictCheck);
     }
 
     /** {@inheritDoc} */

@@ -90,8 +90,9 @@ public interface Restrictor {
      * <a href="https://developer.mozilla.org/en/http_access_control">CORS</a> specification
      * for details
      *
-     * @param pOrigin the "Origin:" URL provided within the request
+     * @param pOrigin the "Origin:" header provided within the request
+     * @param pIsStrictCheck whether doing a strict check
      * @return true if this cross browser request allowed, false otherwise
      */
-    boolean isCorsAccessAllowed(String pOrigin);
+    boolean isOriginAllowed(String pOrigin, boolean pIsStrictCheck);
 }

@@ -150,7 +150,7 @@ public void remoteAccessCheck() {
     @Test
     public void corsAccessCheck() {
         BackendManager backendManager = new BackendManager(config,log);
-        assertTrue(backendManager.isCorsAccessAllowed("http://bla.com"));
+        assertTrue(backendManager.isOriginAllowed("http://bla.com",false));
         backendManager.destroy();
     }
 

@@ -125,8 +125,8 @@ public void initWithCustomLogHandler() throws Exception {
         HttpTestUtil.prepareServletConfigMock(config, new String[]{ConfigKey.LOGHANDLER_CLASS.getKeyValue(), CustomLogHandler.class.getName()});
         HttpTestUtil.prepareServletContextMock(context,null);
 
-        expect(config.getServletContext()).andReturn(context).anyTimes();
-        expect(config.getServletName()).andReturn("jolokia").anyTimes();
+        expect(config.getServletContext()).andStubReturn(context);
+        expect(config.getServletName()).andStubReturn("jolokia");
         replay(config, context);
 
         servlet.init(config);
@@ -191,7 +191,7 @@ public void initWithAgentDiscoveryAndUrlCreationAfterGet() throws ServletExcepti
             StringWriter sw = initRequestResponseMocks();
             expect(request.getPathInfo()).andReturn(HttpTestUtil.HEAP_MEMORY_GET_REQUEST);
             expect(request.getParameter(ConfigKey.MIME_TYPE.getKeyValue())).andReturn("text/plain");
-            String url = "http://pirx:9876/jolokia";
+            String url = "http://10.9.11.1:9876/jolokia";
             StringBuffer buf = new StringBuffer();
             buf.append(url).append(HttpTestUtil.HEAP_MEMORY_GET_REQUEST);
             expect(request.getRequestURL()).andReturn(buf);
@@ -259,7 +259,8 @@ public void simpleGetWithUnsupportedGetParameterMapCall() throws ServletExceptio
         StringWriter sw = initRequestResponseMocks(
                 new Runnable() {
                     public void run() {
-                        expect(request.getHeader("Origin")).andReturn(null);
+                        expect(request.getHeader("Origin")).andStubReturn(null);
+                        expect(request.getHeader("Referer")).andStubReturn(null);
                         expect(request.getRemoteHost()).andReturn("localhost");
                         expect(request.getRemoteAddr()).andReturn("127.0.0.1");
                         expect(request.getRequestURI()).andReturn("/jolokia/");
@@ -368,7 +369,7 @@ private void checkCorsGetOrigin(final String in, final String out) throws Servle
         StringWriter sw = initRequestResponseMocks(
                 new Runnable() {
                     public void run() {
-                        expect(request.getHeader("Origin")).andReturn(in);
+                        expect(request.getHeader("Origin")).andStubReturn(in);
                         expect(request.getRemoteHost()).andReturn("localhost");
                         expect(request.getRemoteAddr()).andReturn("127.0.0.1");
                         expect(request.getRequestURI()).andReturn("/jolokia/");
@@ -464,8 +465,9 @@ public void debug() throws IOException, ServletException {
         context.log(find("time:"));
         context.log(find("Response:"));
         context.log(find("TestDetector"),isA(RuntimeException.class));
-        expectLastCall().anyTimes();
+        expectLastCall().asStub();
         replay(config, context);
+
         servlet.init(config);
 
         StringWriter sw = initRequestResponseMocks();
@@ -503,8 +505,8 @@ private void initConfigMocks(String[] pInitParams, String[] pContextParams,Strin
         HttpTestUtil.prepareServletContextMock(context, pContextParams);
 
 
-        expect(config.getServletContext()).andReturn(context).anyTimes();
-        expect(config.getServletName()).andReturn("jolokia").anyTimes();
+        expect(config.getServletContext()).andStubReturn(context);
+        expect(config.getServletName()).andStubReturn("jolokia");
         if (pExceptionClass != null) {
             context.log(find(pLogRegexp),isA(pExceptionClass));
         } else {
@@ -515,7 +517,7 @@ private void initConfigMocks(String[] pInitParams, String[] pContextParams,Strin
             }
         }
         context.log((String) anyObject());
-        expectLastCall().anyTimes();
+        expectLastCall().asStub();
         context.log(find("TestDetector"),isA(RuntimeException.class));
     }
 
@@ -569,7 +571,8 @@ public void run() {
     private Runnable getStandardRequestSetup() {
         return new Runnable() {
             public void run() {
-                expect(request.getHeader("Origin")).andReturn(null);
+                expect(request.getHeader("Origin")).andStubReturn(null);
+                expect(request.getHeader("Referer")).andStubReturn(null);
                 expect(request.getRemoteHost()).andReturn("localhost");
                 expect(request.getRemoteAddr()).andReturn("127.0.0.1");
                 expect(request.getRequestURI()).andReturn("/jolokia/");

@@ -65,17 +65,27 @@ public void accessAllowed() {
         expect(backend.isRemoteAccessAllowed("localhost","127.0.0.1")).andReturn(true);
         replay(backend);
 
-        handler.checkClientIPAccess("localhost","127.0.0.1");
+        handler.checkAccess("localhost", "127.0.0.1",null);
     }
 
     @Test(expectedExceptions = { SecurityException.class })
     public void accessDenied() {
         expect(backend.isRemoteAccessAllowed("localhost","127.0.0.1")).andReturn(false);
         replay(backend);
 
-        handler.checkClientIPAccess("localhost","127.0.0.1");
+        handler.checkAccess("localhost", "127.0.0.1",null);
     }
 
+    @Test(expectedExceptions = { SecurityException.class })
+    public void accessDeniedViaOrigin() {
+        expect(backend.isRemoteAccessAllowed("localhost","127.0.0.1")).andReturn(true);
+        expect(backend.isOriginAllowed("www.jolokia.org",true)).andReturn(false);
+        replay(backend);
+
+        handler.checkAccess("localhost", "127.0.0.1","www.jolokia.org");
+    }
+
+
     @Test
     public void get() throws InstanceNotFoundException, IOException, ReflectionException, AttributeNotFoundException, MBeanException {
         JSONObject resp = new JSONObject();
@@ -152,7 +162,7 @@ public void doublePost() throws IOException, InstanceNotFoundException, Reflecti
     public void preflightCheck() {
         String origin = "http://bla.com";
         String headers ="X-Data: Test";
-        expect(backend.isCorsAccessAllowed(origin)).andReturn(true);
+        expect(backend.isOriginAllowed(origin,false)).andReturn(true);
         replay(backend);
 
         Map<String,String> ret =  handler.handleCorsPreflightRequest(origin, headers);
@@ -163,7 +173,7 @@ public void preflightCheck() {
     public void preflightCheckNegative() {
         String origin = "http://bla.com";
         String headers ="X-Data: Test";
-        expect(backend.isCorsAccessAllowed(origin)).andReturn(false);
+        expect(backend.isOriginAllowed(origin,false)).andReturn(false);
         replay(backend);
 
         Map<String,String> ret =  handler.handleCorsPreflightRequest(origin, headers);

@@ -222,42 +222,55 @@ public void illegalHttpMethodTag() {
 
     @Test
     public void cors() {
+        InputStream is = getClass().getResourceAsStream("/allow-origin4.xml");
+        PolicyRestrictor restrictor = new PolicyRestrictor(is);
+
+        for (boolean strict : new boolean[] {true, false}) {
+            assertTrue(restrictor.isOriginAllowed("http://bla.com", strict));
+            assertFalse(restrictor.isOriginAllowed("http://www.jolokia.org", strict));
+            assertTrue(restrictor.isOriginAllowed("https://www.consol.de", strict));
+        }
+    }
+
+    @Test
+    public void corsStrictCheckingOff() {
         InputStream is = getClass().getResourceAsStream("/allow-origin1.xml");
         PolicyRestrictor restrictor = new PolicyRestrictor(is);
 
-        assertTrue(restrictor.isCorsAccessAllowed("http://bla.com"));
-        assertFalse(restrictor.isCorsAccessAllowed("http://www.jolokia.org"));
-        assertTrue(restrictor.isCorsAccessAllowed("https://www.consol.de"));
+        // Allways true since we want a strict check but strict checking is off.
+        assertTrue(restrictor.isOriginAllowed("http://bla.com", true));
+        assertTrue(restrictor.isOriginAllowed("http://www.jolokia.org", true));
+        assertTrue(restrictor.isOriginAllowed("https://www.consol.de", true));
     }
 
     @Test
     public void corsWildCard() {
         InputStream is = getClass().getResourceAsStream("/allow-origin2.xml");
         PolicyRestrictor restrictor = new PolicyRestrictor(is);
 
-        assertTrue(restrictor.isCorsAccessAllowed("http://bla.com"));
-        assertTrue(restrictor.isCorsAccessAllowed("http://www.jolokia.org"));
-        assertTrue(restrictor.isCorsAccessAllowed("http://www.consol.de"));
+        assertTrue(restrictor.isOriginAllowed("http://bla.com", false));
+        assertTrue(restrictor.isOriginAllowed("http://www.jolokia.org", false));
+        assertTrue(restrictor.isOriginAllowed("http://www.consol.de", false));
     }
 
     @Test
     public void corsEmpty() {
         InputStream is = getClass().getResourceAsStream("/allow-origin3.xml");
         PolicyRestrictor restrictor = new PolicyRestrictor(is);
 
-        assertTrue(restrictor.isCorsAccessAllowed("http://bla.com"));
-        assertTrue(restrictor.isCorsAccessAllowed("http://www.jolokia.org"));
-        assertTrue(restrictor.isCorsAccessAllowed("http://www.consol.de"));
+        assertTrue(restrictor.isOriginAllowed("http://bla.com", false));
+        assertTrue(restrictor.isOriginAllowed("http://www.jolokia.org", false));
+        assertTrue(restrictor.isOriginAllowed("http://www.consol.de", false));
     }
 
     @Test
     public void corsNoTags() {
         InputStream is = getClass().getResourceAsStream("/access-sample1.xml");
         PolicyRestrictor restrictor = new PolicyRestrictor(is);
 
-        assertTrue(restrictor.isCorsAccessAllowed("http://bla.com"));
-        assertTrue(restrictor.isCorsAccessAllowed("http://www.jolokia.org"));
-        assertTrue(restrictor.isCorsAccessAllowed("https://www.consol.de"));
+        assertTrue(restrictor.isOriginAllowed("http://bla.com", false));
+        assertTrue(restrictor.isOriginAllowed("http://www.jolokia.org", false));
+        assertTrue(restrictor.isOriginAllowed("https://www.consol.de", false));
     }
 
 

@@ -20,6 +20,7 @@
 
   <cors>
     <allow-origin>*</allow-origin>
+
   </cors>
 
 </restrict>

@@ -0,0 +1,27 @@
+<?xml version="1.0" encoding="UTF-8"?>
+
+<!--
+  ~ Copyright 2009-2013 Roland Huss
+  ~
+  ~ 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.
+  -->
+
+<restrict>
+
+  <cors>
+    <allow-origin>http://bla.com</allow-origin>
+    <allow-origin>http://blub.com</allow-origin>
+    <allow-origin>*://*.consol.de</allow-origin>
+    <strict-checking/>
+  </cors>
+</restrict>

@@ -165,7 +165,8 @@ public void handle(HttpExchange pExchange) throws IOException {
         try {
             // Check access policy
             InetSocketAddress address = pExchange.getRemoteAddress();
-            requestHandler.checkClientIPAccess(address.getHostName(),address.getAddress().getHostAddress());
+            requestHandler.checkAccess(address.getHostName(), address.getAddress().getHostAddress(),
+                                       extractOriginOrReferer(pExchange));
             String method = pExchange.getRequestMethod();
 
             // Dispatch for the proper HTTP request method
@@ -190,7 +191,6 @@ public void handle(HttpExchange pExchange) throws IOException {
 
     // ========================================================================
 
-
     private Restrictor createRestrictor(Configuration pConfig) {
         String location = pConfig.get(ConfigKey.POLICY_LOCATION);
         try {
@@ -209,6 +209,17 @@ private Restrictor createRestrictor(Configuration pConfig) {
         }
     }
 
+
+    // Used for checking origin or referer is an origin policy is enabled
+    private String extractOriginOrReferer(HttpExchange pExchange) {
+        Headers headers = pExchange.getRequestHeaders();
+        String origin = headers.getFirst("Origin");
+        if (origin == null) {
+            origin = headers.getFirst("Referer");
+        }
+        return origin != null ? origin.replaceAll("[\\n\\r]*","") : null;
+    }
+
     private JSONAware executeGetRequest(ParsedUri parsedUri) {
         return requestHandler.handleGetRequest(parsedUri.getUri().toString(),parsedUri.getPathInfo(), parsedUri.getParameterMap());
     }

@@ -166,13 +166,13 @@ public boolean isRemoteAccessAllowed(String... pHostOrAddress) {
     private static final RestrictorCheck CORS_CHECK = new RestrictorCheck() {
         /** {@inheritDoc} */
         public boolean check(Restrictor restrictor, Object... args) {
-            return restrictor.isCorsAccessAllowed((String) args[0]);
+            return restrictor.isOriginAllowed((String) args[0], (Boolean) args[1]);
         }
     };
 
     /** {@inheritDoc} */
-    public boolean isCorsAccessAllowed(String pOrigin) {
-        return checkRestrictorService(CORS_CHECK,pOrigin);
+    public boolean isOriginAllowed(String pOrigin, boolean pIsStrictCheck) {
+        return checkRestrictorService(CORS_CHECK,pOrigin,pIsStrictCheck);
     }
 
     // =======================================================================================================

@@ -83,7 +83,7 @@ public void withRestrictor() throws InvalidSyntaxException, MalformedObjectNameE
         assertFalse(restrictor.isAttributeWriteAllowed(new ObjectName("java.lang:type=Memory"), "HeapMemoryUsage"));
         assertTrue(restrictor.isOperationAllowed(new ObjectName("java.lang:type=Memory"), "gc"));
         assertFalse(restrictor.isRemoteAccessAllowed("localhost", "127.0.0.1"));
-        assertTrue(restrictor.isCorsAccessAllowed("http://bla.com"));
+        assertTrue(restrictor.isOriginAllowed("http://bla.com", false));
     }
 
     @Test(expectedExceptions = IllegalArgumentException.class,expectedExceptionsMessageRegExp = ".*Impossible.*")
@@ -132,7 +132,7 @@ public boolean isRemoteAccessAllowed(String... pHostOrAddress) {
             return remote;
         }
 
-        public boolean isCorsAccessAllowed(String pOrigin) {
+        public boolean isOriginAllowed(String pOrigin, boolean pIsStrictCheck) {
             return cors;
         }
     }

@@ -24,6 +24,10 @@
   </properties>
   <body>
     <release version="1.2.1" description="Release 1.2.1" date="2014-02-24">
+      <action dev="roland" type="add" date="2014-04-24">
+        Added "strict-checking" for jolokia-access.xml in the CORS
+        section in order to enable server side origin checking, too.
+      </action>
       <action dev="roland" type="fix" date="2014-04-10" issue="123">
         Fixed issue with JBoss 4.2.3 and JNDI lookups
       </action>

@@ -212,16 +212,25 @@
           <literal>Origin:</literal> header literally or a
           wildcard specification with <literal>*</literal>.          
         </para>
-      </section>
-      <programlisting language="xml"><![CDATA[
+        <programlisting language="xml"><![CDATA[
 <cors>
    <!-- Allow cross origin access from www.jolokia.org ... -->
    <allow-origin>http://www.jolokia.org</allow-origin>
 
    <!-- ... and all servers from jmx4perl.org with any protocol ->
    <allow-origin>*://*.jmx4perl.org</allow-origin>
+
+   <!-- Check for the proper origin on the server side, too -->
+   <strict-checking/>
 </cors>
 ]]></programlisting>
+        <para>
+          If the option <literal>&lt;strict-checking/&gt;</literal> is given in this section, too, then the given pattern
+          are not only used for CORS checking but also every request ist checked on th server side whether the
+          <literal>Origin:</literal> or <literal>Referer:</literal> header matches one of the given patterns.
+          This useful for protecting against Cross-Site Request Forgery.
+        </para>
+      </section>
       <section>
         <title>Example for a security policy</title>
         <para>