You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ranger.apache.org by Pradeep Agrawal <pr...@gmail.com> on 2019/06/19 16:22:08 UTC

Review Request 70893: RANGER-2377: Ranger KnoxSSO authentication when x-forwarded-host header is not forwarded

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70893/
-----------------------------------------------------------

Review request for ranger, Ankita Sinha, bhavik patel, Gautam Borad, Abhay Kulkarni, Madhan Neethiraj, Mehul Parikh, Nikhil P, Nitin Galave, Ramesh Mani, Sailaja Polavarapu, and Velmurugan Periasamy.


Bugs: RANGER-2377
    https://issues.apache.org/jira/browse/RANGER-2377


Repository: ranger


Description
-------

Ranger is unable to forward the request to Ranger if LB is SSL and KnoxSSO is enabled and x-forwarded-host header is not forwarded from LB. Usually Ranger expects that x-forwarded-host shall be provided by LB so current implementation forward the request to the same host but does not change the protocol to https if LB is also SSL(x-forwarded-proto)

Proposed solution: proposed patch contains changes which shall replace the x-forwarded-proto value in the request URL if request URL  contains protocol http while x-forwarded-proto value is https.


Diffs
-----

  security-admin/src/main/java/org/apache/ranger/security/web/filter/RangerSSOAuthenticationFilter.java c3fbe9c23 


Diff: https://reviews.apache.org/r/70893/diff/1/


Testing
-------

Tested knoxsso, knox proxy and ranger HA based authentications.


Thanks,

Pradeep Agrawal


Re: Review Request 70893: RANGER-2477: Ranger KnoxSSO authentication when x-forwarded-host header is not forwarded

Posted by Pradeep Agrawal <pr...@gmail.com>.

> On June 29, 2019, 2:58 a.m., Don Bosco Durai wrote:
> > security-admin/src/main/java/org/apache/ranger/security/web/filter/RangerSSOAuthenticationFilter.java
> > Lines 280 (patched)
> > <https://reviews.apache.org/r/70893/diff/1/?file=2151380#file2151380line280>
> >
> >     Just curios, what happens if the request URL is https and xForwardedProt is http? Is it a valid combination?
> >     
> >     Also, any reason, we are not checking just for "http:"? Instead, 2 conditions?

=>I don't think its a valid combination but it may happen.
Usually, xForwardedProto is either provided as header by user or it can be overridden by proxy/load-balancer. if its decided by proxy/load-balancer then it will be according to the request but user may make a mistake by putting http rather actually https is needed. 
line 283 will make the forwardURL similar to the requestURL and will ignore the xForwardedProto value.


=> When load balancer is in https and ranger is in http and knoxSSO is enabled and if x-forwarded-host is not provided then 
we can assume that the request can be forwarded to the same host from where the request is coming 
here though LB is in ssl, received requestURL was in http(bit strange may be LB issue probably similar to https://stackoverflow.com/questions/29469929/why-does-request-getrequesturl-return-non-https-url)
so to handle this situation I am considering xForwardedProto value which was https so replacing http with https.
since i am using startsWith() method and https starts with http so just for http case i need to add extra condition here as i want to replace http only and avoid wrong replaces like https -> httpss
if requestURL contains https then line 283 shall make the requestURL to be a forwardURL.


If there is a better way to handle this please advice.


- Pradeep


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70893/#review216240
-----------------------------------------------------------


On June 19, 2019, 4:22 p.m., Pradeep Agrawal wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70893/
> -----------------------------------------------------------
> 
> (Updated June 19, 2019, 4:22 p.m.)
> 
> 
> Review request for ranger, Ankita Sinha, bhavik patel, Gautam Borad, Abhay Kulkarni, Madhan Neethiraj, Mehul Parikh, Nikhil P, Nitin Galave, Ramesh Mani, Sailaja Polavarapu, and Velmurugan Periasamy.
> 
> 
> Bugs: RANGER-2477
>     https://issues.apache.org/jira/browse/RANGER-2477
> 
> 
> Repository: ranger
> 
> 
> Description
> -------
> 
> Ranger is unable to forward the request to Ranger if LB is SSL and KnoxSSO is enabled and x-forwarded-host header is not forwarded from LB. Usually Ranger expects that x-forwarded-host shall be provided by LB so current implementation forward the request to the same host but does not change the protocol to https if LB is also SSL(x-forwarded-proto)
> 
> Proposed solution: proposed patch contains changes which shall replace the x-forwarded-proto value in the request URL if request URL  contains protocol http while x-forwarded-proto value is https.
> 
> 
> Diffs
> -----
> 
>   security-admin/src/main/java/org/apache/ranger/security/web/filter/RangerSSOAuthenticationFilter.java c3fbe9c23 
> 
> 
> Diff: https://reviews.apache.org/r/70893/diff/1/
> 
> 
> Testing
> -------
> 
> Tested knoxsso, knox proxy and ranger HA based authentications.
> 
> 
> Thanks,
> 
> Pradeep Agrawal
> 
>


Re: Review Request 70893: RANGER-2477: Ranger KnoxSSO authentication when x-forwarded-host header is not forwarded

Posted by Don Bosco Durai <bo...@apache.org>.

> On June 29, 2019, 2:58 a.m., Don Bosco Durai wrote:
> > security-admin/src/main/java/org/apache/ranger/security/web/filter/RangerSSOAuthenticationFilter.java
> > Lines 280 (patched)
> > <https://reviews.apache.org/r/70893/diff/1/?file=2151380#file2151380line280>
> >
> >     Just curios, what happens if the request URL is https and xForwardedProt is http? Is it a valid combination?
> >     
> >     Also, any reason, we are not checking just for "http:"? Instead, 2 conditions?
> 
> Pradeep Agrawal wrote:
>     =>I don't think its a valid combination but it may happen.
>     Usually, xForwardedProto is either provided as header by user or it can be overridden by proxy/load-balancer. if its decided by proxy/load-balancer then it will be according to the request but user may make a mistake by putting http rather actually https is needed. 
>     line 283 will make the forwardURL similar to the requestURL and will ignore the xForwardedProto value.
>     
>     
>     => When load balancer is in https and ranger is in http and knoxSSO is enabled and if x-forwarded-host is not provided then 
>     we can assume that the request can be forwarded to the same host from where the request is coming 
>     here though LB is in ssl, received requestURL was in http(bit strange may be LB issue probably similar to https://stackoverflow.com/questions/29469929/why-does-request-getrequesturl-return-non-https-url)
>     so to handle this situation I am considering xForwardedProto value which was https so replacing http with https.
>     since i am using startsWith() method and https starts with http so just for http case i need to add extra condition here as i want to replace http only and avoid wrong replaces like https -> httpss
>     if requestURL contains https then line 283 shall make the requestURL to be a forwardURL.
>     
>     
>     If there is a better way to handle this please advice.

Pradeep, thanks for your explanation. Regarding the http check, I was seeing whether we can check only for starts with "http:", rather than starts with "http" and not "https". The net effect would be the same.


- Don Bosco


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70893/#review216240
-----------------------------------------------------------


On June 19, 2019, 4:22 p.m., Pradeep Agrawal wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70893/
> -----------------------------------------------------------
> 
> (Updated June 19, 2019, 4:22 p.m.)
> 
> 
> Review request for ranger, Ankita Sinha, bhavik patel, Gautam Borad, Abhay Kulkarni, Madhan Neethiraj, Mehul Parikh, Nikhil P, Nitin Galave, Ramesh Mani, Sailaja Polavarapu, and Velmurugan Periasamy.
> 
> 
> Bugs: RANGER-2477
>     https://issues.apache.org/jira/browse/RANGER-2477
> 
> 
> Repository: ranger
> 
> 
> Description
> -------
> 
> Ranger is unable to forward the request to Ranger if LB is SSL and KnoxSSO is enabled and x-forwarded-host header is not forwarded from LB. Usually Ranger expects that x-forwarded-host shall be provided by LB so current implementation forward the request to the same host but does not change the protocol to https if LB is also SSL(x-forwarded-proto)
> 
> Proposed solution: proposed patch contains changes which shall replace the x-forwarded-proto value in the request URL if request URL  contains protocol http while x-forwarded-proto value is https.
> 
> 
> Diffs
> -----
> 
>   security-admin/src/main/java/org/apache/ranger/security/web/filter/RangerSSOAuthenticationFilter.java c3fbe9c23 
> 
> 
> Diff: https://reviews.apache.org/r/70893/diff/1/
> 
> 
> Testing
> -------
> 
> Tested knoxsso, knox proxy and ranger HA based authentications.
> 
> 
> Thanks,
> 
> Pradeep Agrawal
> 
>


Re: Review Request 70893: RANGER-2477: Ranger KnoxSSO authentication when x-forwarded-host header is not forwarded

Posted by Don Bosco Durai <bo...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70893/#review216240
-----------------------------------------------------------




security-admin/src/main/java/org/apache/ranger/security/web/filter/RangerSSOAuthenticationFilter.java
Lines 280 (patched)
<https://reviews.apache.org/r/70893/#comment303371>

    Just curios, what happens if the request URL is https and xForwardedProt is http? Is it a valid combination?
    
    Also, any reason, we are not checking just for "http:"? Instead, 2 conditions?


- Don Bosco Durai


On June 19, 2019, 4:22 p.m., Pradeep Agrawal wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70893/
> -----------------------------------------------------------
> 
> (Updated June 19, 2019, 4:22 p.m.)
> 
> 
> Review request for ranger, Ankita Sinha, bhavik patel, Gautam Borad, Abhay Kulkarni, Madhan Neethiraj, Mehul Parikh, Nikhil P, Nitin Galave, Ramesh Mani, Sailaja Polavarapu, and Velmurugan Periasamy.
> 
> 
> Bugs: RANGER-2477
>     https://issues.apache.org/jira/browse/RANGER-2477
> 
> 
> Repository: ranger
> 
> 
> Description
> -------
> 
> Ranger is unable to forward the request to Ranger if LB is SSL and KnoxSSO is enabled and x-forwarded-host header is not forwarded from LB. Usually Ranger expects that x-forwarded-host shall be provided by LB so current implementation forward the request to the same host but does not change the protocol to https if LB is also SSL(x-forwarded-proto)
> 
> Proposed solution: proposed patch contains changes which shall replace the x-forwarded-proto value in the request URL if request URL  contains protocol http while x-forwarded-proto value is https.
> 
> 
> Diffs
> -----
> 
>   security-admin/src/main/java/org/apache/ranger/security/web/filter/RangerSSOAuthenticationFilter.java c3fbe9c23 
> 
> 
> Diff: https://reviews.apache.org/r/70893/diff/1/
> 
> 
> Testing
> -------
> 
> Tested knoxsso, knox proxy and ranger HA based authentications.
> 
> 
> Thanks,
> 
> Pradeep Agrawal
> 
>


Re: Review Request 70893: RANGER-2477: Ranger KnoxSSO authentication when x-forwarded-host header is not forwarded

Posted by Don Bosco Durai <bo...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70893/#review216247
-----------------------------------------------------------


Ship it!




Ship It!

- Don Bosco Durai


On June 29, 2019, 5:50 a.m., Pradeep Agrawal wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70893/
> -----------------------------------------------------------
> 
> (Updated June 29, 2019, 5:50 a.m.)
> 
> 
> Review request for ranger, Ankita Sinha, bhavik patel, Gautam Borad, Abhay Kulkarni, Madhan Neethiraj, Mehul Parikh, Nikhil P, Nitin Galave, Ramesh Mani, Sailaja Polavarapu, and Velmurugan Periasamy.
> 
> 
> Bugs: RANGER-2477
>     https://issues.apache.org/jira/browse/RANGER-2477
> 
> 
> Repository: ranger
> 
> 
> Description
> -------
> 
> Ranger is unable to forward the request to Ranger if LB is SSL and KnoxSSO is enabled and x-forwarded-host header is not forwarded from LB. Usually Ranger expects that x-forwarded-host shall be provided by LB so current implementation forward the request to the same host but does not change the protocol to https if LB is also SSL(x-forwarded-proto)
> 
> Proposed solution: proposed patch contains changes which shall replace the x-forwarded-proto value in the request URL if request URL  contains protocol http while x-forwarded-proto value is https.
> 
> 
> Diffs
> -----
> 
>   security-admin/src/main/java/org/apache/ranger/security/web/filter/RangerSSOAuthenticationFilter.java c3fbe9c23 
> 
> 
> Diff: https://reviews.apache.org/r/70893/diff/3/
> 
> 
> Testing
> -------
> 
> Tested knoxsso, knox proxy and ranger HA based authentications.
> 
> 
> Thanks,
> 
> Pradeep Agrawal
> 
>


Re: Review Request 70893: RANGER-2477: Ranger KnoxSSO authentication when x-forwarded-host header is not forwarded

Posted by Pradeep Agrawal <pr...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70893/
-----------------------------------------------------------

(Updated June 29, 2019, 5:50 a.m.)


Review request for ranger, Ankita Sinha, bhavik patel, Gautam Borad, Abhay Kulkarni, Madhan Neethiraj, Mehul Parikh, Nikhil P, Nitin Galave, Ramesh Mani, Sailaja Polavarapu, and Velmurugan Periasamy.


Changes
-------

Addressed review comment: Changed replace() with replaceFirst() to avoid unrequired replaces.


Bugs: RANGER-2477
    https://issues.apache.org/jira/browse/RANGER-2477


Repository: ranger


Description
-------

Ranger is unable to forward the request to Ranger if LB is SSL and KnoxSSO is enabled and x-forwarded-host header is not forwarded from LB. Usually Ranger expects that x-forwarded-host shall be provided by LB so current implementation forward the request to the same host but does not change the protocol to https if LB is also SSL(x-forwarded-proto)

Proposed solution: proposed patch contains changes which shall replace the x-forwarded-proto value in the request URL if request URL  contains protocol http while x-forwarded-proto value is https.


Diffs (updated)
-----

  security-admin/src/main/java/org/apache/ranger/security/web/filter/RangerSSOAuthenticationFilter.java c3fbe9c23 


Diff: https://reviews.apache.org/r/70893/diff/3/

Changes: https://reviews.apache.org/r/70893/diff/2-3/


Testing
-------

Tested knoxsso, knox proxy and ranger HA based authentications.


Thanks,

Pradeep Agrawal


Re: Review Request 70893: RANGER-2477: Ranger KnoxSSO authentication when x-forwarded-host header is not forwarded

Posted by Pradeep Agrawal <pr...@gmail.com>.

> On June 29, 2019, 5:35 a.m., Don Bosco Durai wrote:
> > security-admin/src/main/java/org/apache/ranger/security/web/filter/RangerSSOAuthenticationFilter.java
> > Lines 281 (patched)
> > <https://reviews.apache.org/r/70893/diff/2/?file=2152885#file2152885line281>
> >
> >     To be on the safe side, you might want to consider using relaceFirst().

Thanks.


- Pradeep


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70893/#review216245
-----------------------------------------------------------


On June 29, 2019, 5:50 a.m., Pradeep Agrawal wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70893/
> -----------------------------------------------------------
> 
> (Updated June 29, 2019, 5:50 a.m.)
> 
> 
> Review request for ranger, Ankita Sinha, bhavik patel, Gautam Borad, Abhay Kulkarni, Madhan Neethiraj, Mehul Parikh, Nikhil P, Nitin Galave, Ramesh Mani, Sailaja Polavarapu, and Velmurugan Periasamy.
> 
> 
> Bugs: RANGER-2477
>     https://issues.apache.org/jira/browse/RANGER-2477
> 
> 
> Repository: ranger
> 
> 
> Description
> -------
> 
> Ranger is unable to forward the request to Ranger if LB is SSL and KnoxSSO is enabled and x-forwarded-host header is not forwarded from LB. Usually Ranger expects that x-forwarded-host shall be provided by LB so current implementation forward the request to the same host but does not change the protocol to https if LB is also SSL(x-forwarded-proto)
> 
> Proposed solution: proposed patch contains changes which shall replace the x-forwarded-proto value in the request URL if request URL  contains protocol http while x-forwarded-proto value is https.
> 
> 
> Diffs
> -----
> 
>   security-admin/src/main/java/org/apache/ranger/security/web/filter/RangerSSOAuthenticationFilter.java c3fbe9c23 
> 
> 
> Diff: https://reviews.apache.org/r/70893/diff/3/
> 
> 
> Testing
> -------
> 
> Tested knoxsso, knox proxy and ranger HA based authentications.
> 
> 
> Thanks,
> 
> Pradeep Agrawal
> 
>


Re: Review Request 70893: RANGER-2477: Ranger KnoxSSO authentication when x-forwarded-host header is not forwarded

Posted by Don Bosco Durai <bo...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70893/#review216245
-----------------------------------------------------------




security-admin/src/main/java/org/apache/ranger/security/web/filter/RangerSSOAuthenticationFilter.java
Lines 281 (patched)
<https://reviews.apache.org/r/70893/#comment303375>

    To be on the safe side, you might want to consider using relaceFirst().


- Don Bosco Durai


On June 29, 2019, 5:04 a.m., Pradeep Agrawal wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70893/
> -----------------------------------------------------------
> 
> (Updated June 29, 2019, 5:04 a.m.)
> 
> 
> Review request for ranger, Ankita Sinha, bhavik patel, Gautam Borad, Abhay Kulkarni, Madhan Neethiraj, Mehul Parikh, Nikhil P, Nitin Galave, Ramesh Mani, Sailaja Polavarapu, and Velmurugan Periasamy.
> 
> 
> Bugs: RANGER-2477
>     https://issues.apache.org/jira/browse/RANGER-2477
> 
> 
> Repository: ranger
> 
> 
> Description
> -------
> 
> Ranger is unable to forward the request to Ranger if LB is SSL and KnoxSSO is enabled and x-forwarded-host header is not forwarded from LB. Usually Ranger expects that x-forwarded-host shall be provided by LB so current implementation forward the request to the same host but does not change the protocol to https if LB is also SSL(x-forwarded-proto)
> 
> Proposed solution: proposed patch contains changes which shall replace the x-forwarded-proto value in the request URL if request URL  contains protocol http while x-forwarded-proto value is https.
> 
> 
> Diffs
> -----
> 
>   security-admin/src/main/java/org/apache/ranger/security/web/filter/RangerSSOAuthenticationFilter.java c3fbe9c23 
> 
> 
> Diff: https://reviews.apache.org/r/70893/diff/2/
> 
> 
> Testing
> -------
> 
> Tested knoxsso, knox proxy and ranger HA based authentications.
> 
> 
> Thanks,
> 
> Pradeep Agrawal
> 
>


Re: Review Request 70893: RANGER-2477: Ranger KnoxSSO authentication when x-forwarded-host header is not forwarded

Posted by Pradeep Agrawal <pr...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70893/
-----------------------------------------------------------

(Updated June 29, 2019, 5:04 a.m.)


Review request for ranger, Ankita Sinha, bhavik patel, Gautam Borad, Abhay Kulkarni, Madhan Neethiraj, Mehul Parikh, Nikhil P, Nitin Galave, Ramesh Mani, Sailaja Polavarapu, and Velmurugan Periasamy.


Changes
-------

Addressed review comment


Bugs: RANGER-2477
    https://issues.apache.org/jira/browse/RANGER-2477


Repository: ranger


Description
-------

Ranger is unable to forward the request to Ranger if LB is SSL and KnoxSSO is enabled and x-forwarded-host header is not forwarded from LB. Usually Ranger expects that x-forwarded-host shall be provided by LB so current implementation forward the request to the same host but does not change the protocol to https if LB is also SSL(x-forwarded-proto)

Proposed solution: proposed patch contains changes which shall replace the x-forwarded-proto value in the request URL if request URL  contains protocol http while x-forwarded-proto value is https.


Diffs (updated)
-----

  security-admin/src/main/java/org/apache/ranger/security/web/filter/RangerSSOAuthenticationFilter.java c3fbe9c23 


Diff: https://reviews.apache.org/r/70893/diff/2/

Changes: https://reviews.apache.org/r/70893/diff/1-2/


Testing
-------

Tested knoxsso, knox proxy and ranger HA based authentications.


Thanks,

Pradeep Agrawal


Re: Review Request 70893: RANGER-2477: Ranger KnoxSSO authentication when x-forwarded-host header is not forwarded

Posted by Don Bosco Durai <bo...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70893/#review216244
-----------------------------------------------------------


Ship it!




Ship It!

- Don Bosco Durai


On June 19, 2019, 4:22 p.m., Pradeep Agrawal wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70893/
> -----------------------------------------------------------
> 
> (Updated June 19, 2019, 4:22 p.m.)
> 
> 
> Review request for ranger, Ankita Sinha, bhavik patel, Gautam Borad, Abhay Kulkarni, Madhan Neethiraj, Mehul Parikh, Nikhil P, Nitin Galave, Ramesh Mani, Sailaja Polavarapu, and Velmurugan Periasamy.
> 
> 
> Bugs: RANGER-2477
>     https://issues.apache.org/jira/browse/RANGER-2477
> 
> 
> Repository: ranger
> 
> 
> Description
> -------
> 
> Ranger is unable to forward the request to Ranger if LB is SSL and KnoxSSO is enabled and x-forwarded-host header is not forwarded from LB. Usually Ranger expects that x-forwarded-host shall be provided by LB so current implementation forward the request to the same host but does not change the protocol to https if LB is also SSL(x-forwarded-proto)
> 
> Proposed solution: proposed patch contains changes which shall replace the x-forwarded-proto value in the request URL if request URL  contains protocol http while x-forwarded-proto value is https.
> 
> 
> Diffs
> -----
> 
>   security-admin/src/main/java/org/apache/ranger/security/web/filter/RangerSSOAuthenticationFilter.java c3fbe9c23 
> 
> 
> Diff: https://reviews.apache.org/r/70893/diff/1/
> 
> 
> Testing
> -------
> 
> Tested knoxsso, knox proxy and ranger HA based authentications.
> 
> 
> Thanks,
> 
> Pradeep Agrawal
> 
>