Eclipse
/
jetty.project
mirror of https://github.com/jetty/jetty.project.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Issue #4193 - Fix InetAccess port control (#4206) 

* issue exclude/include con name InetAccesHandler - add better unit test

this logic:

        String name =
baseRequest.getHttpChannel().getConnector().getName();
        return _names.test(name) && _addrs.test(addr);

Is not correct. it's treating the connector name exactly like the
filter. But that's not what it's intended to do. It's supposed to tell
what connectors are applicable to this filter. And what connectors are
not affected.

For example in the unit test there exists 2 connectors:

http
tls

We want to restrict the http connector, but we want to leave tls
connector alone.

So we would specify:

include = 192.168.1.1-192.168.1.254
includeConnector = http

The way the logic is above, it is treating the connector name as if it's
the filter itself. Which is not what I intended.

What i need in psuedo-code is this:


   if (there are no "include connectors" OR if this connector is
included) AND (if this connector is not in the excluded list)
     ---> Then apply the IP filter.

Signed-off-by: Nicholas DiPiazza <nicholas.dipiazza@lucidworks.com>

* exclude should take precedence over include

Signed-off-by: Nicholas DiPiazza <nicholas.dipiazza@lucidworks.com>

* Issue #4193 InetAccessHandler

reverted changes to IncludeExcludeSet

Signed-off-by: Greg Wilkins <gregw@webtide.com>

* Issue #4193 InetAccessHandler

updates from review

Signed-off-by: Greg Wilkins <gregw@webtide.com>
This commit is contained in:
Greg Wilkins 2019-10-18 09:03:28 +11:00 committed by GitHub
parent 894fc9b115
commit 3d19f61122
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 119 additions and 32 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

11
jetty-server/src/main/java/org/eclipse/jetty/server/handler/InetAccessHandler.java
View File

@ -188,13 +188,16 @@ public class InetAccessHandler extends HandlerWrapper
protected boolean isAllowed(InetAddress addr, Request baseRequest, HttpServletRequest request)
{
String name = baseRequest.getHttpChannel().getConnector().getName();
boolean filterAppliesToConnector = _names.test(name);
boolean allowedByAddr = _addrs.test(addr);
if (LOG.isDebugEnabled())
{
Boolean allowedByName = _names.isIncludedAndNotExcluded(name);
Boolean allowedByAddr = _addrs.isIncludedAndNotExcluded(addr);
LOG.debug("{} allowedByName={} allowedByAddr={} for {}/{}", this, allowedByName, allowedByAddr, addr, request);
LOG.debug("name = {}/{} addr={}/{} appliesToConnector={} allowedByAddr={}",
name, _names, addr, _addrs, filterAppliesToConnector, allowedByAddr);
}
return _names.test(name) && _addrs.test(addr);
if (!filterAppliesToConnector)
return true;
return allowedByAddr;
}
@Override

104
jetty-server/src/test/java/org/eclipse/jetty/server/handler/InetAccessHandlerTest.java
View File

@ -21,7 +21,9 @@ package org.eclipse.jetty.server.handler;
import java.io.IOException;
import java.net.Socket;
import java.nio.ByteBuffer;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
import java.util.stream.Stream;
import javax.servlet.ServletException;
import javax.servlet.http.HttpServletRequest;
@ -45,17 +47,20 @@ import static org.junit.jupiter.api.Assertions.assertEquals;
public class InetAccessHandlerTest
{
private static Server _server;
private static ServerConnector _connector;
private static ServerConnector _connector1;
private static ServerConnector _connector2;
private static InetAccessHandler _handler;
@BeforeAll
public static void setUp() throws Exception
{
_server = new Server();
_connector = new ServerConnector(_server);
_connector.setName("http");
_connector1 = new ServerConnector(_server);
_connector1.setName("http_connector1");
_connector2 = new ServerConnector(_server);
_connector2.setName("http_connector2");
_server.setConnectors(new Connector[]
{_connector});
{_connector1, _connector2});
_handler = new InetAccessHandler();
_handler.setHandler(new AbstractHandler()
@ -113,7 +118,21 @@ public class InetAccessHandlerTest
}
}
try (Socket socket = new Socket("127.0.0.1", _connector.getLocalPort());)
List<String> codePerConnector = new ArrayList<>();
for (String nextCode : code.split(";", -1))
{
if (nextCode.length() > 0)
{
codePerConnector.add(nextCode);
}
}
testConnector(_connector1.getLocalPort(), include, exclude, includeConnectors, excludeConnectors, codePerConnector.get(0));
testConnector(_connector2.getLocalPort(), include, exclude, includeConnectors, excludeConnectors, codePerConnector.get(1));
}
private void testConnector(int port, String include, String exclude, String includeConnectors, String excludeConnectors, String code) throws IOException {
try (Socket socket = new Socket("127.0.0.1", port);)
{
socket.setSoTimeout(5000);
@ -136,39 +155,68 @@ public class InetAccessHandlerTest
}
}
/**
* Data for this test.
* @return Format of data: include;exclude;includeConnectors;excludeConnectors;assertionStatusCodePerConnector
*/
public static Stream<Arguments> data()
{
Object[][] data = new Object[][]
{
// Empty lists
{"", "", "", "", "200"},
// Empty lists 1
{"", "", "", "", "200;200"},
// test simple filters
{"127.0.0.1", "", "", "", "200"},
{"127.0.0.1-127.0.0.254", "", "", "", "200"},
{"192.0.0.1", "", "", "", "403"},
{"192.0.0.1-192.0.0.254", "", "", "", "403"},
{"127.0.0.1", "", "", "", "200;200"},
{"127.0.0.1-127.0.0.254", "", "", "", "200;200"},
{"192.0.0.1", "", "", "", "403;403"},
{"192.0.0.1-192.0.0.254", "", "", "", "403;403"},
// test connector name filters
{"127.0.0.1", "", "http", "", "200"},
{"127.0.0.1-127.0.0.254", "", "http", "", "200"},
{"192.0.0.1", "", "http", "", "403"},
{"192.0.0.1-192.0.0.254", "", "http", "", "403"},
// test includeConnector
{"127.0.0.1", "", "http_connector1", "", "200;200"},
{"127.0.0.1-127.0.0.254", "", "http_connector1", "", "200;200"},
{"192.0.0.1", "", "http_connector1", "", "403;200"},
{"192.0.0.1-192.0.0.254", "", "http_connector1", "", "403;200"},
{"192.0.0.1", "", "http_connector2", "", "200;403"},
{"192.0.0.1-192.0.0.254", "", "http_connector2", "", "200;403"},
{"127.0.0.1", "", "nothttp", "", "403"},
{"127.0.0.1-127.0.0.254", "", "nothttp", "", "403"},
{"192.0.0.1", "", "nothttp", "", "403"},
{"192.0.0.1-192.0.0.254", "", "nothttp", "", "403"},
// test includeConnector names where none of them match
{"127.0.0.1", "", "nothttp", "", "200;200"},
{"127.0.0.1-127.0.0.254", "", "nothttp", "", "200;200"},
{"192.0.0.1", "", "nothttp", "", "200;200"},
{"192.0.0.1-192.0.0.254", "", "nothttp", "", "200;200"},
{"127.0.0.1", "", "", "http", "403"},
{"127.0.0.1-127.0.0.254", "", "", "http", "403"},
{"192.0.0.1", "", "", "http", "403"},
{"192.0.0.1-192.0.0.254", "", "", "http", "403"},
// text excludeConnector
{"127.0.0.1", "", "", "http_connector1", "200;200"},
{"127.0.0.1-127.0.0.254", "", "", "http_connector1", "200;200"},
{"192.0.0.1", "", "", "http_connector1", "200;403"},
{"192.0.0.1-192.0.0.254", "", "", "http_connector1", "200;403"},
{"192.0.0.1", "", "", "http_connector2", "403;200"},
{"192.0.0.1-192.0.0.254", "", "", "http_connector2", "403;200"},
{"127.0.0.1", "", "", "nothttp", "200"},
{"127.0.0.1-127.0.0.254", "", "", "nothttp", "200"},
{"192.0.0.1", "", "", "nothttp", "403"},
{"192.0.0.1-192.0.0.254", "", "", "nothttp", "403"},
// test excludeConnector where none of them match.
{"127.0.0.1", "", "", "nothttp", "200;200"},
{"127.0.0.1-127.0.0.254", "", "", "nothttp", "200;200"},
{"192.0.0.1", "", "", "nothttp", "403;403"},
{"192.0.0.1-192.0.0.254", "", "", "nothttp", "403;403"},
// both connectors are excluded
{"127.0.0.1", "", "", "http_connector1;http_connector2", "200;200"},
{"127.0.0.1-127.0.0.254", "", "", "http_connector1;http_connector2", "200;200"},
{"192.0.0.1", "", "", "http_connector1;http_connector2", "200;200"},
{"192.0.0.1-192.0.0.254", "", "", "http_connector1;http_connector2", "200;200"},
// both connectors are included
{"127.0.0.1", "", "http_connector1;http_connector2", "", "200;200"},
{"127.0.0.1-127.0.0.254", "", "http_connector1;http_connector2", "", "200;200"},
{"192.0.0.1", "", "http_connector1;http_connector2", "", "403;403"},
{"192.0.0.1-192.0.0.254", "", "http_connector1;http_connector2", "", "403;403"},
// exclude takes precedence over include
{"127.0.0.1", "", "http_connector1;http_connector2", "http_connector1;http_connector2", "200;200"},
{"127.0.0.1-127.0.0.254", "", "http_connector1;http_connector2", "http_connector1;http_connector2", "200;200"},
{"192.0.0.1", "", "http_connector1;http_connector2", "http_connector1;http_connector2", "200;200"},
{"192.0.0.1-192.0.0.254", "", "http_connector1;http_connector2", "http_connector1;http_connector2", "200;200"},
};
return Arrays.asList(data).stream().map(Arguments::of);
}

36
jetty-util/src/main/java/org/eclipse/jetty/util/IncludeExcludeSet.java
View File

@ -56,6 +56,12 @@ public class IncludeExcludeSet<T, P> implements Predicate<P>
{
return set.contains(item);
}
@Override
public String toString()
{
return "CONTAINS";
}
}
/**
@ -233,4 +239,34 @@ public class IncludeExcludeSet<T, P> implements Predicate<P>
{
return _includes.isEmpty() && _excludes.isEmpty();
}
/**
* Match items in combined IncludeExcludeSets.
* @param item1 The item to match against set1
* @param set1 A IncludeExcludeSet to match item1 against
* @param item2 The item to match against set2
* @param set2 A IncludeExcludeSet to match item2 against
* @param <T1> The type of item1
* @param <T2> The type of item2
* @return True IFF <ul>
* <li>Neither item is excluded from their respective sets</li>
* <li>Both sets have no includes OR at least one of the items is included in its respective set</li>
* </ul>
*/
public static <T1,T2> boolean matchCombined(T1 item1, IncludeExcludeSet<?,T1> set1, T2 item2, IncludeExcludeSet<?,T2> set2)
{
Boolean match1 = set1.isIncludedAndNotExcluded(item1);
Boolean match2 = set2.isIncludedAndNotExcluded(item2);
// if we are excluded from either set, then we do not match
if (match1 == Boolean.FALSE || match2 == Boolean.FALSE)
return false;
// If either set has any includes, then we must be included by one of them
if (set1.hasIncludes() || set2.hasIncludes())
return match1 == Boolean.TRUE || match2 == Boolean.TRUE;
// If not excluded and no includes, then we match
return true;
}
}