pvillard31 commented on code in PR #11335:
URL: https://github.com/apache/nifi/pull/11335#discussion_r3408273131
##########
nifi-framework-bundle/nifi-framework/nifi-web/nifi-jetty/src/main/java/org/apache/nifi/web/server/connector/ProxyHeaderValidatorCustomizer.java:
##########
@@ -95,20 +101,40 @@ private void processProxyHostHeaders(final Request
request) {
final HttpFields requestHeaders = request.getHeaders();
for (final String proxyHostHeader : SUPPORTED_PROXY_HOST_HEADERS) {
- final String hostHeader = requestHeaders.get(proxyHostHeader);
+ String hostHeader = requestHeaders.get(proxyHostHeader);
// Include empty and blank values for enforced validation of
request headers
if (hostHeader == null) {
continue;
}
+
+ String[] hostHeaderParts = HOST_PORT_SEPARATOR.split(hostHeader);
+ if (hostHeaderParts.length == 2) {
+ hostHeader = hostHeaderParts[0];
+ }
+
// Allow proxy host header matching request host header based on
TLS SNI and DNS SAN requirements
if (requestHost.equals(hostHeader)) {
continue;
}
+
if (validProxyHosts.contains(hostHeader)) {
+ if (hostHeaderParts.length == 2) {
+ try {
+ int port = Integer.parseInt(hostHeaderParts[1]);
+ if (validPorts.contains(port)) {
+ continue;
+ } else {
+ throw new
HttpException.RuntimeException(HttpStatus.MISDIRECTED_REQUEST_421,
MISDIRECTED_REQUEST_REASON_INVALID_PORT);
+ }
+ } catch (NumberFormatException e) {
Review Comment:
```suggestion
} catch (final NumberFormatException e) {
```
##########
nifi-framework-bundle/nifi-framework/nifi-web/nifi-jetty/src/main/java/org/apache/nifi/web/server/connector/ProxyHeaderValidatorCustomizer.java:
##########
@@ -95,20 +101,40 @@ private void processProxyHostHeaders(final Request
request) {
final HttpFields requestHeaders = request.getHeaders();
for (final String proxyHostHeader : SUPPORTED_PROXY_HOST_HEADERS) {
- final String hostHeader = requestHeaders.get(proxyHostHeader);
+ String hostHeader = requestHeaders.get(proxyHostHeader);
// Include empty and blank values for enforced validation of
request headers
if (hostHeader == null) {
continue;
}
+
+ String[] hostHeaderParts = HOST_PORT_SEPARATOR.split(hostHeader);
+ if (hostHeaderParts.length == 2) {
+ hostHeader = hostHeaderParts[0];
+ }
+
// Allow proxy host header matching request host header based on
TLS SNI and DNS SAN requirements
if (requestHost.equals(hostHeader)) {
continue;
}
Review Comment:
Here the code continues before the port is checked, so a header like
X`-Forwarded-Host: <requestHost>:6666` still passes an arbitrary port through.
Should the port be validated on this branch too, the same way you did below?
Maybe with a small method to avoid code duplication.
##########
nifi-framework-bundle/nifi-framework/nifi-web/nifi-jetty/src/main/java/org/apache/nifi/web/server/connector/ProxyHeaderValidatorCustomizer.java:
##########
@@ -95,20 +101,40 @@ private void processProxyHostHeaders(final Request
request) {
final HttpFields requestHeaders = request.getHeaders();
for (final String proxyHostHeader : SUPPORTED_PROXY_HOST_HEADERS) {
- final String hostHeader = requestHeaders.get(proxyHostHeader);
+ String hostHeader = requestHeaders.get(proxyHostHeader);
// Include empty and blank values for enforced validation of
request headers
if (hostHeader == null) {
continue;
}
+
+ String[] hostHeaderParts = HOST_PORT_SEPARATOR.split(hostHeader);
+ if (hostHeaderParts.length == 2) {
+ hostHeader = hostHeaderParts[0];
+ }
+
// Allow proxy host header matching request host header based on
TLS SNI and DNS SAN requirements
if (requestHost.equals(hostHeader)) {
continue;
}
+
if (validProxyHosts.contains(hostHeader)) {
+ if (hostHeaderParts.length == 2) {
+ try {
+ int port = Integer.parseInt(hostHeaderParts[1]);
Review Comment:
```suggestion
final int port =
Integer.parseInt(hostHeaderParts[1]);
```
##########
nifi-framework-bundle/nifi-framework/nifi-web/nifi-jetty/src/main/java/org/apache/nifi/web/server/connector/ProxyHeaderValidatorCustomizer.java:
##########
@@ -95,20 +101,40 @@ private void processProxyHostHeaders(final Request
request) {
final HttpFields requestHeaders = request.getHeaders();
for (final String proxyHostHeader : SUPPORTED_PROXY_HOST_HEADERS) {
- final String hostHeader = requestHeaders.get(proxyHostHeader);
+ String hostHeader = requestHeaders.get(proxyHostHeader);
// Include empty and blank values for enforced validation of
request headers
if (hostHeader == null) {
continue;
}
+
+ String[] hostHeaderParts = HOST_PORT_SEPARATOR.split(hostHeader);
+ if (hostHeaderParts.length == 2) {
+ hostHeader = hostHeaderParts[0];
+ }
+
// Allow proxy host header matching request host header based on
TLS SNI and DNS SAN requirements
if (requestHost.equals(hostHeader)) {
continue;
}
+
if (validProxyHosts.contains(hostHeader)) {
+ if (hostHeaderParts.length == 2) {
+ try {
+ int port = Integer.parseInt(hostHeaderParts[1]);
+ if (validPorts.contains(port)) {
+ continue;
+ } else {
+ throw new
HttpException.RuntimeException(HttpStatus.MISDIRECTED_REQUEST_421,
MISDIRECTED_REQUEST_REASON_INVALID_PORT);
+ }
+ } catch (NumberFormatException e) {
+ throw new
HttpException.RuntimeException(HttpStatus.MISDIRECTED_REQUEST_421,
MISDIRECTED_REQUEST_REASON_INVALID_PORT);
+ }
+ }
continue;
}
throw new
HttpException.RuntimeException(HttpStatus.MISDIRECTED_REQUEST_421,
MISDIRECTED_REQUEST_REASON);
+
Review Comment:
```suggestion
throw new HttpException.RuntimeException(HttpStatus.MISDIRECTED_REQUEST_421,
MISDIRECTED_REQUEST_REASON);
```
##########
nifi-framework-bundle/nifi-framework/nifi-web/nifi-jetty/src/main/java/org/apache/nifi/web/server/connector/ProxyHeaderValidatorCustomizer.java:
##########
@@ -95,20 +101,40 @@ private void processProxyHostHeaders(final Request
request) {
final HttpFields requestHeaders = request.getHeaders();
for (final String proxyHostHeader : SUPPORTED_PROXY_HOST_HEADERS) {
- final String hostHeader = requestHeaders.get(proxyHostHeader);
+ String hostHeader = requestHeaders.get(proxyHostHeader);
// Include empty and blank values for enforced validation of
request headers
if (hostHeader == null) {
continue;
}
+
+ String[] hostHeaderParts = HOST_PORT_SEPARATOR.split(hostHeader);
Review Comment:
```suggestion
final String[] hostHeaderParts =
HOST_PORT_SEPARATOR.split(hostHeader);
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
