[jira] [Commented] (NIFI-5748) Improve handling of X-Forwarded-* headers in URI Rewriting

["ASF GitHub Bot (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/NIFI-5748?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16675362#comment-16675362 ] 

ASF GitHub Bot commented on NIFI-5748:
--------------------------------------

GitHub user jtstorck opened a pull request:

    https://github.com/apache/nifi/pull/3129

    [WIP] NIFI-5748 Fixed proxy header support to use X-Forwarded-Host instead …

    …of X-ForwardedServer
    
    Added support for the context path header used by Traefik when proxying a service (X-Forwarded-Prefix)
    Added tests to ApplicationResourceTest for X-Forwarded-Context and X-Forwarded-Prefix
    Updated administration doc to include X-Forwarded-Prefix
    
    Thank you for submitting a contribution to Apache NiFi.
    
    In order to streamline the review of the contribution we ask you
    to ensure the following steps have been taken:
    
    ### For all changes:
    - [ ] Is there a JIRA ticket associated with this PR? Is it referenced 
         in the commit message?
    
    - [ ] Does your PR title start with NIFI-XXXX where XXXX is the JIRA number you are trying to resolve? Pay particular attention to the hyphen "-" character.
    
    - [ ] Has your PR been rebased against the latest commit within the target branch (typically master)?
    
    - [ ] Is your initial contribution a single, squashed commit?
    
    ### For code changes:
    - [ ] Have you ensured that the full suite of tests is executed via mvn -Pcontrib-check clean install at the root nifi folder?
    - [ ] Have you written or updated unit tests to verify your changes?
    - [ ] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under [ASF 2.0](http://www.apache.org/legal/resolved.html#category-a)? 
    - [ ] If applicable, have you updated the LICENSE file, including the main LICENSE file under nifi-assembly?
    - [ ] If applicable, have you updated the NOTICE file, including the main NOTICE file found under nifi-assembly?
    - [ ] If adding new Properties, have you added .displayName in addition to .name (programmatic access) for each of the new properties?
    
    ### For documentation related changes:
    - [ ] Have you ensured that format looks appropriate for the output in which it is rendered?
    
    ### Note:
    Please ensure that once the PR is submitted, you check travis-ci for build issues and submit an update to your PR as soon as possible.


You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/jtstorck/nifi NIFI-5748

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/nifi/pull/3129.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #3129
    
----
commit 49aecd1127132e2c4cb12639cb9d66b14dee60d0
Author: Jeff Storck <jt...@...>
Date:   2018-10-29T17:29:28Z

    NIFI-5748 Fixed proxy header support to use X-Forwarded-Host instead of X-ForwardedServer
    Added support for the context path header used by Traefik when proxying a service (X-Forwarded-Prefix)
    Added tests to ApplicationResourceTest for X-Forwarded-Context and X-Forwarded-Prefix
    Updated administration doc to include X-Forwarded-Prefix

----


> Improve handling of X-Forwarded-* headers in URI Rewriting
> ----------------------------------------------------------
>
>                 Key: NIFI-5748
>                 URL: https://issues.apache.org/jira/browse/NIFI-5748
>             Project: Apache NiFi
>          Issue Type: Improvement
>            Reporter: Kevin Doran
>            Assignee: Jeff Storck
>            Priority: Major
>
> This ticket is to improve the handling of headers used by popular proxies when rewriting URIs in NiFI. Currently, NiFi checks the following headers when determining how to re-write URLs it returns clients:
> From [ApplicationResource|https://github.com/apache/nifi/blob/2201f7746fd16874aefbd12d546565f5d105ab04/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/api/ApplicationResource.java#L110]:
> {code:java}
> public static final String PROXY_SCHEME_HTTP_HEADER = "X-ProxyScheme";
> public static final String PROXY_HOST_HTTP_HEADER = "X-ProxyHost";
> public static final String PROXY_PORT_HTTP_HEADER = "X-ProxyPort";
> public static final String PROXY_CONTEXT_PATH_HTTP_HEADER = "X-ProxyContextPath";
> public static final String FORWARDED_PROTO_HTTP_HEADER = "X-Forwarded-Proto";
> public static final String FORWARDED_HOST_HTTP_HEADER = "X-Forwarded-Server";
> public static final String FORWARDED_PORT_HTTP_HEADER = "X-Forwarded-Port";
> public static final String FORWARDED_CONTEXT_HTTP_HEADER = "X-Forwarded-Context";
> // ...
> final String scheme = getFirstHeaderValue(PROXY_SCHEME_HTTP_HEADER, FORWARDED_PROTO_HTTP_HEADER);
> final String host = getFirstHeaderValue(PROXY_HOST_HTTP_HEADER, FORWARDED_HOST_HTTP_HEADER);
> final String port = getFirstHeaderValue(PROXY_PORT_HTTP_HEADER, FORWARDED_PORT_HTTP_HEADER);
> {code}
> Based on this, it looks like if both {{X-Forwarded-Server}} and {{X-Forwarded-Host}} are set, that {{-Host}} will contain the hostname the user/client requested, and {{-Server}} will contain the hostname of the proxy server (ie, what the proxy server is able to determine from inspecting the hostname of the instance it is on). See this for more details:
> https://stackoverflow.com/questions/43689625/x-forwarded-host-vs-x-forwarded-server
> Here is a demo based on docker containers and a reverse-proxy called Traefik that shows where the current NiFi logic can break:
> https://gist.github.com/kevdoran/2892004ccbfbb856115c8a756d9d4538
> To use this gist, you can run the following:
> {noformat}
> wget -qO- https://gist.githubusercontent.com/kevdoran/2892004ccbfbb856115c8a756d9d4538/raw/fb72151900d4d8fdcf4919fe5c8a94805fbb8401/docker-compose.yml | docker-compose -f - up
> {noformat}
> Once the environment is up. Go to http://nifi.docker.localhost/nifi in Chrome and try adding/configuring/moving a processor. This will reproduce the issue.
> For this task, the following improvement is recommended:
> Change the Header (string literal) for FORWARDED_HOST_HTTP_HEADER from "X-Forwarded-Server" to "X-Forwarded-Host"
> Additionally, some proxies use a different header for the context path prefix. Traefik uses {{X-Forwarded-Prefix}}. There does not appear to be a universal standard. In the future, we could make this header configurable, but for now, perhaps we should add {{X-Forwarded-Prefix}} to the headers checked by NiFi so that Traefik is supported out-of-the-box.
> *Important:* After making this change, verify that proxying NiFi via Knox still works, i.e., Knox should be sending the X-Forwarded-Host header that matches what the user requested in the browser.
> This change applies to NiFi Registry as well.
> /cc [~jtstorck]



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)