Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

server whitelist 2 #686

Merged
merged 12 commits into from
Aug 7, 2020
Merged

server whitelist 2 #686

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
b008bc9
move whitelist to new package
sclassen Aug 6, 2020
9b44de7
cleanup java warnings
sclassen Aug 6, 2020
f21cfad
rename method
sclassen Aug 6, 2020
137f86c
cleanup java warnings
sclassen Aug 6, 2020
4521038
inline param which was always true
sclassen Aug 6, 2020
b53de73
convert WhitelistEntry to top level class
sclassen Aug 6, 2020
dc639e0
move matching logic into WhitelistEntry
sclassen Aug 6, 2020
3e06072
rename WhitelistEntry properties
sclassen Aug 6, 2020
bc585f5
parse effective whitelist URL in static factory method
sclassen Aug 6, 2020
c036294
reorder methods
sclassen Aug 6, 2020
ba83c43
assert not null only in entry method (public method)
sclassen Aug 6, 2020
7c3af41
cleanup test code
sclassen Aug 7, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 7 additions & 6 deletions ...ain/java/net/adoptopenjdk/icedteaweb/client/controlpanel/panels/ServerWhitelistPanel.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,8 @@
import net.adoptopenjdk.icedteaweb.i18n.Translator;
import net.adoptopenjdk.icedteaweb.jdk89access.SunMiscLauncher;
import net.sourceforge.jnlp.config.DeploymentConfiguration;
import net.sourceforge.jnlp.util.UrlWhiteListUtils;
import net.sourceforge.jnlp.util.whitelist.UrlWhiteListUtils;
import net.sourceforge.jnlp.util.whitelist.WhitelistEntry;

import javax.swing.BorderFactory;
import javax.swing.ImageIcon;
Expand Down Expand Up @@ -56,7 +57,7 @@ public ServerWhitelistPanel(final DeploymentConfiguration config) {

Assert.requireNonNull(config, "config");

final List<UrlWhiteListUtils.WhitelistEntry> whitelist = UrlWhiteListUtils.getApplicationUrlWhiteList();
final List<WhitelistEntry> whitelist = UrlWhiteListUtils.getApplicationUrlWhiteList();

final JTable table = new JTable(createTableModel(whitelist));
table.getTableHeader().setReorderingAllowed(false);
Expand Down Expand Up @@ -102,7 +103,7 @@ public Component getTableCellRendererComponent(JTable table, Object value, boole
add(scrollPane, BorderLayout.CENTER);
}

private TableModel createTableModel(final List<UrlWhiteListUtils.WhitelistEntry> whitelist) {
private TableModel createTableModel(final List<WhitelistEntry> whitelist) {
final String[] colNames = {R("SWPCol0Header"), R("SWPCol1Header")};
return new AbstractTableModel() {
@Override
Expand All @@ -121,12 +122,12 @@ public int getColumnCount() {

@Override
public Object getValueAt(int rowIndex, int columnIndex) {
UrlWhiteListUtils.WhitelistEntry whitelistEntry = whitelist.get(rowIndex);
WhitelistEntry whitelistEntry = whitelist.get(rowIndex);
switch (columnIndex) {
case 0:
return whitelistEntry.getWhitelistEntry();
return whitelistEntry.getRawWhitelistEntry();
case 1:
return new WhitelistEntryState(whitelistEntry.isValid(), whitelistEntry.isValid() ? whitelistEntry.getValidatedWhitelistEntry() : R("SWPINVALIDWLURL") + ": " + whitelistEntry.getErrorMessage());
return new WhitelistEntryState(whitelistEntry.isValid(), whitelistEntry.isValid() ? whitelistEntry.getEffectiveWhitelistEntry() : R("SWPINVALIDWLURL") + ": " + whitelistEntry.getErrorMessage());
default:
throw new IllegalArgumentException();
}
Expand Down
2 changes: 1 addition & 1 deletion core/src/main/java/net/adoptopenjdk/icedteaweb/resources/ResourceHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
import net.adoptopenjdk.icedteaweb.resources.initializer.ResourceInitializer;
import net.sourceforge.jnlp.cache.CacheUtil;
import net.sourceforge.jnlp.runtime.JNLPRuntime;
import net.sourceforge.jnlp.util.UrlWhiteListUtils;
import net.sourceforge.jnlp.util.whitelist.UrlWhiteListUtils;

import java.io.File;
import java.net.URL;
Expand Down
164 changes: 37 additions & 127 deletions ...rceforge/jnlp/util/UrlWhiteListUtils.java → ...nlp/util/whitelist/UrlWhiteListUtils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
package net.sourceforge.jnlp.util;
package net.sourceforge.jnlp.util.whitelist;

import net.adoptopenjdk.icedteaweb.Assert;
import net.adoptopenjdk.icedteaweb.StringUtils;
import net.adoptopenjdk.icedteaweb.logging.Logger;
import net.adoptopenjdk.icedteaweb.logging.LoggerFactory;
import net.sourceforge.jnlp.runtime.JNLPRuntime;
import net.sourceforge.jnlp.util.IpUtil;

import java.net.MalformedURLException;
import java.net.URL;
Expand All @@ -25,7 +26,7 @@ public class UrlWhiteListUtils {
public static final String HTTP = "http";
private static final String PROTOCOL_SEPARATOR = "://";

private final static Logger LOG = LoggerFactory.getLogger(UrlWhiteListUtils.class);
private static final Logger LOG = LoggerFactory.getLogger(UrlWhiteListUtils.class);

private static List<WhitelistEntry> applicationUrlWhiteList;
private static final Lock whiteListLock = new ReentrantLock();
Expand All @@ -37,7 +38,7 @@ public static List<WhitelistEntry> getApplicationUrlWhiteList() {
applicationUrlWhiteList = JNLPRuntime.getConfiguration().getPropertyAsList(KEY_SECURITY_SERVER_WHITELIST)
.stream()
.filter(s -> !StringUtils.isBlank(s))
.map(s -> UrlWhiteListUtils.validateWhitelistUrl(s))
.map(UrlWhiteListUtils::validateWhitelistUrl)
.collect(Collectors.toList());
}
return applicationUrlWhiteList;
Expand All @@ -47,10 +48,10 @@ public static List<WhitelistEntry> getApplicationUrlWhiteList() {
}

public static boolean isUrlInApplicationUrlWhitelist(final URL url) {
return isUrlInWhitelist(url, getApplicationUrlWhiteList(), true);
return isUrlInWhitelist(url, getApplicationUrlWhiteList());
}

static boolean isUrlInWhitelist(final URL url, final List<WhitelistEntry> whiteList, final boolean allowLocalhost) {
static boolean isUrlInWhitelist(final URL url, final List<WhitelistEntry> whiteList) {
Assert.requireNonNull(url, "url");
Assert.requireNonNull(whiteList, "whiteList");

Expand All @@ -59,68 +60,23 @@ static boolean isUrlInWhitelist(final URL url, final List<WhitelistEntry> whiteL
}

// is it localhost or loopback
if (allowLocalhost && IpUtil.isLocalhostOrLoopback(url)) {
if (IpUtil.isLocalhostOrLoopback(url)) {
return true; // local server need not be in whitelist
}

final boolean result = whiteList.stream().anyMatch(wlEntry -> {
try {
if (wlEntry.isValid()) { // ignore invalid whitelist entries
final URL wlUrl = new URL(wlEntry.getValidatedWhitelistEntry());
return isUrlProtocolMatching(wlUrl, url) && isUrlHostMatching(wlUrl, url) && isUrlPortMatching(wlUrl, url);
} else {
return false;
}
} catch (Exception e) {
LOG.warn("Bad white list url: " + wlEntry.getValidatedWhitelistEntry());
return false;
}
});
return result;
}

private static boolean isUrlProtocolMatching(URL url1, URL url2) {
Assert.requireNonNull(url1, "url1");
Assert.requireNonNull(url2, "url2");
return Objects.equals(url1.getProtocol(), url2.getProtocol());
}

private static boolean isUrlHostMatching(URL wlUrl, URL url) {
Assert.requireNonNull(wlUrl, "wlUrl");
Assert.requireNonNull(url, "url");

// proto://*:port
if (Objects.equals(wlUrl.getHost(), WILDCARD)) {
return true;
}

final String[] wlUrlHostParts = wlUrl.getHost().split(HOST_PART_REGEX);
final String[] urlHostParts = url.getHost().split(HOST_PART_REGEX);

if (wlUrlHostParts.length != urlHostParts.length) {
return false;
}
boolean result = true;
for (int i = 0; i < wlUrlHostParts.length; i++) {
// hostparts are equal if whitelist url has * or they are same
result = result && (Objects.equals(wlUrlHostParts[i], WILDCARD) || Objects.equals(wlUrlHostParts[i], urlHostParts[i]));
}
return result;
return whiteList.stream().anyMatch(wlEntry -> wlEntry.matches(url));
}

private static boolean isUrlPortMatching(URL wlUrl, URL url) {
Assert.requireNonNull(wlUrl, "wlUrl");
Assert.requireNonNull(url, "url");

if (wlUrl.getPort() != -1) {
// url does not have port then force default port as we do the same for whitelist url
if (url.getPort() == -1) {
return wlUrl.getPort() == url.getDefaultPort();
} else {
return wlUrl.getPort() == url.getPort();
}
static WhitelistEntry validateWhitelistUrl(final String wlUrlStr) {
Assert.requireNonNull(wlUrlStr, "wlUrlStr");
try {
final String validatedWLUrlProtocol = validateWhitelistUrlProtocol(wlUrlStr);
final String validatedWLUrlStr = validateWhitelistUrlPort(validatedWLUrlProtocol);
final URL validatedUrl = validateWhitelistUrlHost(validatedWLUrlStr);
return WhitelistEntry.validWhitelistEntry(wlUrlStr, validatedUrl);
} catch (Exception e) {
return WhitelistEntry.invalidWhitelistEntry(wlUrlStr, e.getMessage());
}
return true;
}

private static String validateWhitelistUrlProtocol(final String wlUrlStr) throws MalformedURLException {
Expand Down Expand Up @@ -151,42 +107,21 @@ private static String validateWhitelistUrlPort(final String wlUrlStr) throws Mal
} catch (Exception e) {
// if port is illegal due to * then replace * with ""
final int ind = wlUrlStr.lastIndexOf(":");
if (e.getCause() instanceof NumberFormatException && Objects.equals(wlUrlStr.substring(ind + 1, wlUrlStr.length()), WILDCARD)) {
if (e.getCause() instanceof NumberFormatException && Objects.equals(wlUrlStr.substring(ind + 1), WILDCARD)) {
return wlUrlStr.substring(0, ind);
}
throw e;
}
return wlUrlStr;
}

// IP Address => all digits, 4 parts, *, -
private static boolean isIP(final String wlUrlStr) {
final boolean hasValidChars = wlUrlStr.replace(HOST_PART_SEP.charAt(0), '0').chars().allMatch(c -> Character.isDigit(c) || c == WILDCARD.charAt(0) || c == '-');
final String[] ipParts = wlUrlStr.split(HOST_PART_REGEX);
return hasValidChars && ipParts.length == 4;
}

private static void validateIPPart(final String ipPart) throws Exception {
if (ipPart.contains(WILDCARD) || ipPart.contains("-")) {
throw new Exception(R("SWPINVALIDIPHOST"));
}
try {
final int ipPartInt = Integer.parseInt(ipPart);
if (ipPartInt < 0 || ipPartInt > 255) {
throw new Exception(R("SWPINVALIDIPHOST"));
}
} catch (NumberFormatException nfe) {
throw new Exception(R("SWPINVALIDIPHOST"));
}
}

private static void validateWhitelistUrlHost(final String wlUrlStr) throws Exception {
private static URL validateWhitelistUrlHost(final String wlUrlStr) throws Exception {
final URL wlURL = new URL(wlUrlStr);
final String hostStr = wlURL.getHost();

// Whitelist Host is *
if (Objects.equals(hostStr, WILDCARD)) {
return;
return wlURL;
}

final boolean isIPHost = isIP(hostStr);
Expand All @@ -201,53 +136,28 @@ private static void validateWhitelistUrlHost(final String wlUrlStr) throws Excep
}
}
}
}

static WhitelistEntry validateWhitelistUrl(final String wlUrlStr) {
Assert.requireNonNull(wlUrlStr, "wlUrlStr");
try {
final String validatedWLUrlProtocol = validateWhitelistUrlProtocol(wlUrlStr);
final String validatedWLUrlStr = validateWhitelistUrlPort(validatedWLUrlProtocol);
validateWhitelistUrlHost(validatedWLUrlStr);
return WhitelistEntry.validWhitelistEntry(wlUrlStr, validatedWLUrlStr);
} catch (Exception e) {
return WhitelistEntry.invalidWhitelistentry(wlUrlStr, e.getMessage());
}
return wlURL;
}

public static class WhitelistEntry {
private final String whitelistEntry;
private final String validatedWhitelistEntry;
private final String errorMessage;

public String getWhitelistEntry() {
return whitelistEntry;
}

public String getValidatedWhitelistEntry() {
return validatedWhitelistEntry;
}

public String getErrorMessage() {
return errorMessage;
}

private WhitelistEntry(final String whitelistEntry, final String validatedWhitelistEntry, final String errorMessage) {
this.whitelistEntry = whitelistEntry;
this.validatedWhitelistEntry = validatedWhitelistEntry;
this.errorMessage = errorMessage;
}

public boolean isValid() {
return errorMessage == null;
}
// IP Address => all digits, 4 parts, *, -
private static boolean isIP(final String wlUrlStr) {
final boolean hasValidChars = wlUrlStr.replace(HOST_PART_SEP.charAt(0), '0').chars().allMatch(c -> Character.isDigit(c) || c == WILDCARD.charAt(0) || c == '-');
final String[] ipParts = wlUrlStr.split(HOST_PART_REGEX);
return hasValidChars && ipParts.length == 4;
}

public static WhitelistEntry validWhitelistEntry(final String wlEntry, final String validatedEntry) {
return new WhitelistEntry(wlEntry, validatedEntry, null);
private static void validateIPPart(final String ipPart) throws Exception {
if (ipPart.contains(WILDCARD) || ipPart.contains("-")) {
throw new Exception(R("SWPINVALIDIPHOST"));
}

public static WhitelistEntry invalidWhitelistentry(final String wlEntry, final String errorMessage) {
return new WhitelistEntry(wlEntry, null, errorMessage);
try {
final int ipPartInt = Integer.parseInt(ipPart);
if (ipPartInt < 0 || ipPartInt > 255) {
throw new Exception(R("SWPINVALIDIPHOST"));
}
} catch (NumberFormatException nfe) {
throw new Exception(R("SWPINVALIDIPHOST"));
}
}
}
99 changes: 99 additions & 0 deletions core/src/main/java/net/sourceforge/jnlp/util/whitelist/WhitelistEntry.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,99 @@
package net.sourceforge.jnlp.util.whitelist;

import net.adoptopenjdk.icedteaweb.Assert;

import java.net.URL;
import java.util.Objects;
import java.util.Optional;

/**
* ...
*/
public class WhitelistEntry {
private static final String WILDCARD = "*";
private static final String HOST_PART_REGEX = "\\.";

private final String rawWhitelistEntry;
private final URL effectiveWhitelistEntry;
hendrikebbers marked this conversation as resolved.
Show resolved Hide resolved
private final String errorMessage;

static WhitelistEntry validWhitelistEntry(final String wlEntry, URL effectiveWhitelistEntry) {
Assert.requireNonNull(wlEntry, "wlEntry");
Assert.requireNonNull(effectiveWhitelistEntry, "effectiveWhitelistEntry");
return new WhitelistEntry(wlEntry, effectiveWhitelistEntry, null);
}

static WhitelistEntry invalidWhitelistEntry(final String wlEntry, final String errorMessage) {
Assert.requireNonNull(wlEntry, "wlEntry");
Assert.requireNonNull(errorMessage, "errorMessage");
return new WhitelistEntry(wlEntry, null, errorMessage);
}

private WhitelistEntry(final String rawWhitelistEntry, final URL effectiveWhitelistEntry, final String errorMessage) {
this.rawWhitelistEntry = rawWhitelistEntry;
this.effectiveWhitelistEntry = effectiveWhitelistEntry;
this.errorMessage = errorMessage;
}

public String getRawWhitelistEntry() {
return rawWhitelistEntry;
}

public String getEffectiveWhitelistEntry() {
return Optional.ofNullable(effectiveWhitelistEntry).map(Objects::toString).orElse(null);
}

public String getErrorMessage() {
return errorMessage;
}

public boolean isValid() {
return errorMessage == null;
}

public boolean matches(URL url) {
Assert.requireNonNull(url, "url");

if (isValid()) { // ignore invalid whitelist entries
return isUrlProtocolMatching(effectiveWhitelistEntry, url) && isUrlHostMatching(effectiveWhitelistEntry, url) && isUrlPortMatching(effectiveWhitelistEntry, url);
} else {
return false;
}
}
private static boolean isUrlProtocolMatching(URL url1, URL url2) {
return Objects.equals(url1.getProtocol(), url2.getProtocol());
}

private static boolean isUrlHostMatching(URL wlUrl, URL url) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please us better names :)

// proto://*:port
if (Objects.equals(wlUrl.getHost(), WILDCARD)) {
return true;
}

final String[] wlUrlHostParts = wlUrl.getHost().split(HOST_PART_REGEX);
final String[] urlHostParts = url.getHost().split(HOST_PART_REGEX);

if (wlUrlHostParts.length != urlHostParts.length) {
return false;
}

boolean result = true;
for (int i = 0; i < wlUrlHostParts.length; i++) {
// hostparts are equal if whitelist url has * or they are same
result = result && (Objects.equals(wlUrlHostParts[i], WILDCARD) || Objects.equals(wlUrlHostParts[i], urlHostParts[i]));
}
return result;
}

private static boolean isUrlPortMatching(URL wlUrl, URL url) {
if (wlUrl.getPort() != -1) {
// url does not have port then force default port as we do the same for whitelist url
if (url.getPort() == -1) {
return wlUrl.getPort() == url.getDefaultPort();
} else {
return wlUrl.getPort() == url.getPort();
}
}
return true;
}
}
Loading