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

Make sure use_x_forward_for and trusted_proxies must config together #15804

Conversation

awarecan
Copy link
Contributor

@awarecan awarecan commented Aug 3, 2018

Description:

Per http component document and PR #15204

use_x_forwarded_for (Optional): Enable parsing of the X-Forwarded-For header, passing on the client’s correct IP address in proxied setups. You must also whitelist trusted proxies using the trusted_proxies setting below for this to work.

However, we don't have config validation against this requirement.

This PR added config validation to make sure these two options are configured together

Related issue (if applicable): fixes #15544

Pull request in home-assistant.github.io with documentation (if applicable): home-assistant/home-assistant.github.io#<home-assistant.github.io PR number goes here>

Example entry for configuration.yaml (if applicable):

Checklist:

  • The code change is tested and works locally.
  • Local tests pass with tox. Your PR cannot be merged unless tests pass

If the code does not interact with devices:

  • Tests have been added to verify that the new code works.

http.CONF_TRUSTED_PROXIES: ['127.0.0.1']
}
}) is not True

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

blank line at end of file

assert await async_setup_component(hass, 'http', {
'http': {
http.CONF_TRUSTED_PROXIES: ['127.0.0.1']
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

continuation line missing indentation or outdented

"""Test use_x_forwarded_for must config together with trusted_proxies."""
assert await async_setup_component(hass, 'http', {
'http': {
http.CONF_TRUSTED_PROXIES: ['127.0.0.1']

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

continuation line missing indentation or outdented

@awarecan awarecan force-pushed the trusted-proxies-has-to-set-together-with-use-xff branch from 5385b67 to 8532412 Compare August 3, 2018 06:18
@awarecan
Copy link
Contributor Author

awarecan commented Aug 3, 2018

I don't have proxy server setup, need volunteer to test it.

cc @andersonshatch @mvn23

@balloob balloob merged commit 6f2000f into home-assistant:dev Aug 3, 2018
@ghost ghost removed the in progress label Aug 3, 2018
@awarecan awarecan deleted the trusted-proxies-has-to-set-together-with-use-xff branch August 3, 2018 12:18
@balloob balloob mentioned this pull request Aug 17, 2018
@brunohorta82
Copy link

brunohorta82 commented Aug 28, 2018

Hello in my case not show the real IP only de proxy IP
screenshot 2018-08-28 at 14 43 21

http: api_password: !secret api_password server_port: !secret api_port base_url: https://home.bhsystems.eu use_x_forwarded_for: true trusted_proxies: 127.0.0.1 ip_ban_enabled: true login_attempts_threshold: 5

Nginx

proxy_set_header Host $host; proxy_redirect http:// https://; proxy_http_version 1.1; proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for; proxy_set_header Upgrade $http_upgrade; proxy_set_header Connection $connection_upgrade; }

@awarecan
Copy link
Contributor Author

@brunohorta82 please do not use pull request to raise issue, create an issue and fill in the issue template.

@home-assistant home-assistant locked as resolved and limited conversation to collaborators Aug 28, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Risk with use_x_forwarded_for and no trusted_proxies when hass is behind a reverse proxy in a trusted_network
5 participants