Skip to content
Permalink
Browse files Browse the repository at this point in the history
Enchanced policy wr to origin handling. The Origin: can now be checke…
…d on the server side, too.
  • Loading branch information
rhuss committed Apr 24, 2014
1 parent ec206b3 commit 2d9b168
Show file tree
Hide file tree
Showing 18 changed files with 166 additions and 54 deletions.
Expand Up @@ -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);
}

/**
Expand Down
12 changes: 11 additions & 1 deletion agent/core/src/main/java/org/jolokia/http/AgentServlet.java
Expand Up @@ -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);
Expand All @@ -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
Expand Down
Expand Up @@ -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) {
Expand Down Expand Up @@ -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.
*
Expand All @@ -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;
Expand Down
Expand Up @@ -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;
}
}
Expand Up @@ -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} */
Expand Down
Expand Up @@ -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);
}
Expand Up @@ -30,6 +30,8 @@
*/
public class CorsChecker extends AbstractChecker<String> {

private boolean strictChecking = false;

private List<Pattern> patterns;

/**
Expand All @@ -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>
*
Expand All @@ -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;
}
}
}
}
Expand All @@ -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;
}
}
Expand Down
Expand Up @@ -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();
}

Expand Down
23 changes: 13 additions & 10 deletions agent/core/src/test/java/org/jolokia/http/AgentServletTest.java
Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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/");
Expand Down Expand Up @@ -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/");
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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 {
Expand All @@ -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));
}

Expand Down Expand Up @@ -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/");
Expand Down
Expand Up @@ -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();
Expand Down Expand Up @@ -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);
Expand All @@ -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);
Expand Down
Expand Up @@ -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));
}


Expand Down
1 change: 1 addition & 0 deletions agent/core/src/test/resources/allow-origin2.xml
Expand Up @@ -20,6 +20,7 @@

<cors>
<allow-origin>*</allow-origin>

</cors>

</restrict>

0 comments on commit 2d9b168

Please sign in to comment.