-
Notifications
You must be signed in to change notification settings - Fork 169
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.
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
feat: check custom header for dev tools connection verification #18827
Changes from 1 commit
ea116b1
1e792da
7a7185a
d9050d5
429036c
f44a0b0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -23,6 +23,7 @@ | |
import java.util.List; | ||
import java.util.Locale; | ||
import java.util.Optional; | ||
import java.util.stream.Stream; | ||
|
||
import org.apache.commons.io.FilenameUtils; | ||
import org.jsoup.Jsoup; | ||
|
@@ -49,7 +50,6 @@ | |
import com.vaadin.flow.server.BootstrapHandler; | ||
import com.vaadin.flow.server.Constants; | ||
import com.vaadin.flow.server.DevToolsToken; | ||
import com.vaadin.flow.server.InitParameters; | ||
import com.vaadin.flow.server.Mode; | ||
import com.vaadin.flow.server.VaadinContext; | ||
import com.vaadin.flow.server.VaadinRequest; | ||
|
@@ -67,6 +67,8 @@ | |
import elemental.json.JsonObject; | ||
import elemental.json.impl.JsonUtil; | ||
|
||
import static com.vaadin.flow.server.InitParameters.SERVLET_PARAMETER_DEVMODE_HOSTS_ALLOWED; | ||
import static com.vaadin.flow.server.InitParameters.SERVLET_PARAMETER_DEVMODE_REMOTE_ADDRESS_HEADER; | ||
import static com.vaadin.flow.shared.ApplicationConstants.CONTENT_TYPE_TEXT_HTML_UTF_8; | ||
import static java.nio.charset.StandardCharsets.UTF_8; | ||
|
||
|
@@ -397,21 +399,32 @@ static boolean isAllowedDevToolsHost(AbstractConfiguration configuration, | |
VaadinRequest request) { | ||
String remoteAddress = request.getRemoteAddr(); | ||
String hostsAllowed = configuration.getStringProperty( | ||
InitParameters.SERVLET_PARAMETER_DEVMODE_HOSTS_ALLOWED, null); | ||
SERVLET_PARAMETER_DEVMODE_HOSTS_ALLOWED, null); | ||
|
||
if (!isAllowedDevToolsHost(remoteAddress, hostsAllowed)) { | ||
if (!isAllowedDevToolsHost(remoteAddress, hostsAllowed, true)) { | ||
return false; | ||
} | ||
String remoteHeaderIp = configuration.getStringProperty( | ||
SERVLET_PARAMETER_DEVMODE_REMOTE_ADDRESS_HEADER, null); | ||
if (remoteHeaderIp != null) { | ||
return isAllowedDevToolsHost(request.getHeader(remoteHeaderIp), | ||
hostsAllowed, false); | ||
} | ||
|
||
String forwardedFor = request.getHeader("X-Forwarded-For"); | ||
if (forwardedFor != null) { | ||
if (forwardedFor.contains(",")) { | ||
// X-Forwarded-For: <client>, <proxy1>, <proxy2> | ||
// Elements are comma-separated, with optional whitespace | ||
// surrounding the commas. | ||
forwardedFor = forwardedFor.split(",")[0]; | ||
} | ||
if (!isAllowedDevToolsHost(forwardedFor.trim(), hostsAllowed)) { | ||
return false; | ||
// Validate all hops | ||
return Stream.of(forwardedFor.split(",")).map(String::trim) | ||
.allMatch(ip -> isAllowedDevToolsHost(ip, hostsAllowed, | ||
false)); | ||
|
||
} else { | ||
return isAllowedDevToolsHost(forwardedFor.trim(), hostsAllowed, | ||
false); | ||
} | ||
} | ||
|
||
|
@@ -420,15 +433,17 @@ static boolean isAllowedDevToolsHost(AbstractConfiguration configuration, | |
} | ||
|
||
private static boolean isAllowedDevToolsHost(String remoteAddress, | ||
String hostsAllowed) { | ||
if (remoteAddress == null) { | ||
// No remote address available so we cannot check... | ||
String hostsAllowed, boolean allowLocal) { | ||
if (remoteAddress == null || (hostsAllowed == null && !allowLocal)) { | ||
// No check needed if the remote address is not available | ||
// or if local addresses must be rejected and there are no host | ||
// rules defined | ||
return false; | ||
} | ||
// Always allow localhost | ||
try { | ||
InetAddress inetAddress = InetAddress.getByName(remoteAddress); | ||
if (inetAddress.isLoopbackAddress()) { | ||
if (inetAddress.isLoopbackAddress() && allowLocal) { | ||
return true; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If it IS a local address and local is NOT allowed, should this not immediately return false, i.e. if it is a loopback address, always return There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good point There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. done |
||
} | ||
} catch (Exception e) { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are we assuming this is a single header with a single address? Should it be in the docs?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, the assumption is a single header with a single address, as this is supposed to be safely handled by the proxy server.
I'll update the docs
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done