Skip to content
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

HTTPS and mod_remoteip #149

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

HTTPS and mod_remoteip #149

wants to merge 1 commit into from

Conversation

kncsvk
Copy link

@kncsvk kncsvk commented Jan 6, 2019

  • bug fix
  • BC break? no

I have web server setup with HTTP proxy and Apache web server with module mod_remoteip (This mod replace REMOTE_ADDR to real client IP instead of proxy IP).
HTTP works as expected but with HTTPS I have these problems:

With router:
$router[] = new Route('<presenter>/<action>[/<id>]', 'Dashboard:default');
Result URL was https://example.com:80/

With router:
$router[] = new Route('https://%host%/<presenter>/<action>[/<id>]', 'Dashboard:default');
Result in browser was redirect loop (ERR_TOO_MANY_REDIRECTS)

I searched in code and found that HTTP_X_FORWARDED* headers are ignored if they din't come from trusted proxy. After bypassing this check for HTTP_X_FORWARDED_PROTO and HTTP_X_FORWARDED_PORT router works again as expected.
I don't see any security problem if these two variables didn't come from trusted proxy.

Trust HTTP_X_FORWARDED_PROTO and HTTP_X_FORWARDED_PORT even if not coming from trusted proxy. It's not security problem like directly parsing HTTP_X_FORWARDED_FOR. This fixes HTTPS in proxy server setups with enabled mod_remoteip.
@dg dg force-pushed the master branch 10 times, most recently from 55b5376 to 45e2638 Compare February 13, 2019 04:39
@dg dg force-pushed the master branch 2 times, most recently from c2bbf57 to db54e1c Compare February 28, 2019 16:13
@dg dg force-pushed the master branch 9 times, most recently from 26f846b to 4be7cde Compare March 13, 2019 19:05
@dg dg force-pushed the master branch 2 times, most recently from 8fd7f28 to 51a304f Compare April 1, 2019 22:12
@mabar
Copy link
Contributor

mabar commented Jul 1, 2019

You can spam application with shitload of errors if app breaks with an incompatible url. In worst case would be e.g. possible send user an email with url of attacker website.

@JanTvrdik
Copy link
Contributor

You can always mark all proxies as trusted with setProxy('0.0.0.0/0') if your network setup ensures that your app is always behind trusted proxy.

@dg
Copy link
Member

dg commented Jul 3, 2019

@kncsvk does this work for you: (https://doc.nette.org/cs/3.0/configuring#toc-http-proxy)

http:
    proxy: 0.0.0.0/0

@dg dg force-pushed the master branch 2 times, most recently from 0011929 to ddf9e8e Compare October 17, 2019 23:18
@dg dg force-pushed the master branch 6 times, most recently from da24b94 to 540335c Compare March 20, 2023 13:43
@dg dg force-pushed the master branch 2 times, most recently from e7c7e2d to bf945f3 Compare August 5, 2023 19:08
@dg dg force-pushed the master branch 3 times, most recently from 9a14e6e to a20fb8f Compare November 14, 2023 18:31
@dg dg force-pushed the master branch 5 times, most recently from 55488bd to 2042d2e Compare December 11, 2023 13:01
@dg dg force-pushed the master branch 2 times, most recently from 4960652 to 5e67add Compare May 2, 2024 10:56
@dg dg force-pushed the master branch 5 times, most recently from 689f4ae to 33aae19 Compare November 5, 2024 06:45
@dg dg force-pushed the master branch 3 times, most recently from 9eafa57 to 09923de Compare January 12, 2025 16:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants