NIFI-15967 Adjust Proxy Header Validation for Replicated Requests#11273
Conversation
| // Requests not authenticated with Client Certificates require header validation | ||
| if (peerCertificate == null) { | ||
| processProxyHostHeaders(request); | ||
| } else { |
There was a problem hiding this comment.
Should this branch also require the peer certificate to match a known cluster node identity (or come from the cluster-only trust store), so that a non-node user holding a valid client certificate cannot bypass proxy host validation simply by adding the request-replicated header?
There was a problem hiding this comment.
I considered that additional approach, but it places additional constraints on information that is not necessarily available. The current approach places inherent security constraints with mTLS, and also fits within current configuration boundaries.
| public class ProxyHeaderValidatorCustomizer implements HttpConfiguration.Customizer { | ||
| private static final String MISDIRECTED_REQUEST_REASON = "Invalid Proxy Host Requested"; | ||
|
|
||
| private static final String REQUEST_REPLICATED_HEADER = "request-replicated"; |
There was a problem hiding this comment.
Could we reference RequestReplicationHeader.REQUEST_REPLICATED.getHeader() (possibly after moving that enum, or just this single constant, into nifi-web-servlet-shared next to ProxyHeader) so the header value is defined in exactly one place?
There was a problem hiding this comment.
Thanks, I looked at options and ended up creating a new ReplicationHeader with just this new enum and put it in nifi-framework-core, as that appeared to be a better location given the cluster-focused nature of this header.
|
|
||
| private static final String HOST_HEADER = "Host"; | ||
|
|
||
| private static final String REQUEST_REPLICATED_HEADER = "request-replicated"; |
There was a problem hiding this comment.
Same question on the test side: can REQUEST_REPLICATED_HEADER here share the canonical constant rather than redefining the literal?
| final ConnectionMetaData connectionMetaData = request.getConnectionMetaData(); | ||
| final Connection connection = connectionMetaData.getConnection(); | ||
| final EndPoint endPoint = connection.getEndPoint(); | ||
| final EndPoint.SslSessionData sslSessionData = endPoint.getSslSessionData(); |
There was a problem hiding this comment.
Is there any code path under a successful isSecure() where connection.getEndPoint() or endPoint.getSslSessionData() could be null (for example during a protocol upgrade), and if so should we guard the chain rather than risk an NullPointerException inside the customizer?
There was a problem hiding this comment.
Good question, but no, at this point in the Connection handling, the SslSessionData can never be null.
- Removed RequestReplicationHeader.REQUEST_REPLICATED in favor of new enum
pvillard31
left a comment
pvillard31
left a comment
There was a problem hiding this comment.
Thanks for the updates @exceptionfactory - there are some test failures to address
Thanks @pvillard31, I pushed an update to adjust the filtering method in |
pvillard31
left a comment
pvillard31
left a comment
There was a problem hiding this comment.
Thanks, latest LGTM and can be merged after successful checks.
2 participants
Summary
NIFI-15967 Adjusts the implementation of proxy header validation to support authenticated and replicated requests between NiFi cluster nodes.
Proxy host header validation applies only when the application is enabled for HTTPS, so the implementation checks for the presence of X509 Peer Certificates in the TLS Session. This occurs after a successful TLS handshake, and the presence of the client certificates in the TLS Session indicates that mutual TLS authentication completed as expected. To differentiate requests, the implementation looks for the presence of the custom
request-replicatedheader, which the application sets when constructing forwarded requests. Without both of these qualifiers, standard proxy host header validation occurs, checking the values configured in thenifi.web.proxy.hostproperty, as implemented in NIFI-15953.Tracking
Please complete the following tracking steps prior to pull request creation.
Issue Tracking
Pull Request Tracking
NIFI-00000NIFI-00000VerifiedstatusPull Request Formatting
mainbranchVerification
Please indicate the verification steps performed prior to pull request creation.
Build
./mvnw clean install -P contrib-checkLicensing
LICENSEandNOTICEfilesDocumentation