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

Improve X-Forwarded-* request headers handling #38696

Merged
merged 35 commits into from
Aug 11, 2020
Merged
Show file tree
Hide file tree
Changes from 29 commits
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
ca97ca4
Improve X-Forwarded-* request headers handling
frenck Aug 9, 2020
6d1af44
Replace middleware in http server
frenck Aug 9, 2020
8d06bf2
Remove use of KEY_REAL_IP, replace by request.remote
frenck Aug 9, 2020
167748e
Only set up middleware when enabled and when trusted proxies are conf…
frenck Aug 10, 2020
1f6acbe
Guard socket connected peer is trusted
frenck Aug 10, 2020
46b7c7b
Comments for clarification on the processing
frenck Aug 10, 2020
2818d5d
Extensive information on the processing
frenck Aug 10, 2020
5a0da60
Tweaks information on the processing
frenck Aug 10, 2020
99ee73e
Remove real_ip/forwarded middleware from emulated hue, doesn't do any…
frenck Aug 10, 2020
f15fca9
Blow up on empty IP entry in X-Forwarded-For
frenck Aug 10, 2020
50e4837
Adjust forwarded tests setup signature
frenck Aug 10, 2020
87d9fe9
Fix common IP mock in http tests
frenck Aug 10, 2020
b440d8a
Fix auth tests
frenck Aug 10, 2020
7894b14
Adjust IP Ban handling
frenck Aug 10, 2020
08dd958
Fix emulated_hue tests
frenck Aug 10, 2020
ca5e7a6
Improve forwarded middleware tests + extend coverage
frenck Aug 10, 2020
6137099
Add more test cases with malformed data in forwarding headers
frenck Aug 10, 2020
370d060
Protect against empty host value
frenck Aug 10, 2020
e518fc3
Require number of proto elements to be 1 or match forwarded for length
frenck Aug 10, 2020
0e3b587
Add some serious test coverage to forwarded middleware
frenck Aug 10, 2020
3071fce
Log warning when forwarding headers received from untrusted connected…
frenck Aug 10, 2020
463cb46
Extend inline documentation in code
frenck Aug 10, 2020
3bcd039
Ensure ip_address is used with login flow
frenck Aug 10, 2020
82a1569
Apply suggestions from code review
frenck Aug 10, 2020
68c0025
Process review comments
frenck Aug 10, 2020
a6c1b1c
Check empty proto headers
frenck Aug 10, 2020
db48084
Fix X-Forwarded-Proto handling, adds tests
frenck Aug 10, 2020
bd6c75f
Additional tests for handling spaceless separators in headers
frenck Aug 10, 2020
4b164c6
Use for/else instead of list lookup
frenck Aug 10, 2020
c67eb13
Apply suggestions from code review
frenck Aug 11, 2020
cf3eaf3
async_setup_forwarded
frenck Aug 11, 2020
4892a99
Process review comments
frenck Aug 11, 2020
d8f953a
Apply suggestions from code review
frenck Aug 11, 2020
226f3e1
Update homeassistant/components/hassio/auth.py
pvizeli Aug 11, 2020
59de0c3
Black
frenck Aug 11, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 2 additions & 7 deletions homeassistant/components/auth/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,6 @@
User,
)
from homeassistant.components import websocket_api
from homeassistant.components.http import KEY_REAL_IP
from homeassistant.components.http.auth import async_sign_path
from homeassistant.components.http.ban import log_invalid_auth
from homeassistant.components.http.data_validator import RequestDataValidator
Expand Down Expand Up @@ -252,14 +251,10 @@ async def post(self, request):
return await self._async_handle_revoke_token(hass, data)

if grant_type == "authorization_code":
return await self._async_handle_auth_code(
hass, data, str(request[KEY_REAL_IP])
)
return await self._async_handle_auth_code(hass, data, request.remote)

if grant_type == "refresh_token":
return await self._async_handle_refresh_token(
hass, data, str(request[KEY_REAL_IP])
)
return await self._async_handle_refresh_token(hass, data, request.remote)

return self.json(
{"error": "unsupported_grant_type"}, status_code=HTTP_BAD_REQUEST
Expand Down
7 changes: 4 additions & 3 deletions homeassistant/components/auth/login_flow.py
Original file line number Diff line number Diff line change
Expand Up @@ -66,12 +66,13 @@
"version": 1
}
"""
from ipaddress import ip_address

from aiohttp import web
import voluptuous as vol
import voluptuous_serialize

from homeassistant import data_entry_flow
from homeassistant.components.http import KEY_REAL_IP
from homeassistant.components.http.ban import (
log_invalid_auth,
process_success_login,
Expand Down Expand Up @@ -183,7 +184,7 @@ async def post(self, request, data):
result = await self._flow_mgr.async_init(
handler,
context={
"ip_address": request[KEY_REAL_IP],
"ip_address": ip_address(request.remote),
"credential_only": data.get("type") == "link_user",
},
)
Expand Down Expand Up @@ -231,7 +232,7 @@ async def post(self, request, flow_id, data):
for flow in self._flow_mgr.async_progress():
if flow["flow_id"] == flow_id and flow["context"][
"ip_address"
] != request.get(KEY_REAL_IP):
] != ip_address(request.remote):
return self.json_message("IP address changed", HTTP_BAD_REQUEST)

result = await self._flow_mgr.async_configure(flow_id, data)
Expand Down
2 changes: 0 additions & 2 deletions homeassistant/components/emulated_hue/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
import voluptuous as vol

from homeassistant import util
from homeassistant.components.http import real_ip
from homeassistant.const import EVENT_HOMEASSISTANT_START, EVENT_HOMEASSISTANT_STOP
from homeassistant.exceptions import HomeAssistantError
import homeassistant.helpers.config_validation as cv
Expand Down Expand Up @@ -101,7 +100,6 @@ async def async_setup(hass, yaml_config):
app = web.Application()
app["hass"] = hass

real_ip.setup_real_ip(app, False, [])
# We misunderstood the startup signal. You're not allowed to change
# anything during startup. Temp workaround.
# pylint: disable=protected-access
Expand Down
18 changes: 9 additions & 9 deletions homeassistant/components/emulated_hue/hue_api.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
"""Support for a Hue API to control Home Assistant."""
import asyncio
import hashlib
from ipaddress import ip_address
import logging
import time

Expand Down Expand Up @@ -34,7 +35,6 @@
SUPPORT_SET_SPEED,
)
from homeassistant.components.http import HomeAssistantView
from homeassistant.components.http.const import KEY_REAL_IP
from homeassistant.components.humidifier.const import (
ATTR_HUMIDITY,
SERVICE_SET_HUMIDITY,
Expand Down Expand Up @@ -131,7 +131,7 @@ class HueUsernameView(HomeAssistantView):

async def post(self, request):
"""Handle a POST request."""
if not is_local(request[KEY_REAL_IP]):
if not is_local(ip_address(request.remote)):
return self.json_message("Only local IPs allowed", HTTP_UNAUTHORIZED)

try:
Expand Down Expand Up @@ -159,7 +159,7 @@ def __init__(self, config):
@core.callback
def get(self, request, username):
"""Process a request to make the Brilliant Lightpad work."""
if not is_local(request[KEY_REAL_IP]):
if not is_local(ip_address(request.remote)):
return self.json_message("Only local IPs allowed", HTTP_UNAUTHORIZED)

return self.json({})
Expand All @@ -179,7 +179,7 @@ def __init__(self, config):
@core.callback
def put(self, request, username):
"""Process a request to make the Logitech Pop working."""
if not is_local(request[KEY_REAL_IP]):
if not is_local(ip_address(request.remote)):
return self.json_message("Only local IPs allowed", HTTP_UNAUTHORIZED)

return self.json(
Expand Down Expand Up @@ -209,7 +209,7 @@ def __init__(self, config):
@core.callback
def get(self, request, username):
"""Process a request to get the list of available lights."""
if not is_local(request[KEY_REAL_IP]):
if not is_local(ip_address(request.remote)):
return self.json_message("Only local IPs allowed", HTTP_UNAUTHORIZED)

return self.json(create_list_of_entities(self.config, request))
Expand All @@ -229,7 +229,7 @@ def __init__(self, config):
@core.callback
def get(self, request, username):
"""Process a request to get the list of available lights."""
if not is_local(request[KEY_REAL_IP]):
if not is_local(ip_address(request.remote)):
return self.json_message("only local IPs allowed", HTTP_UNAUTHORIZED)
if username != HUE_API_USERNAME:
return self.json(UNAUTHORIZED_USER)
Expand All @@ -256,7 +256,7 @@ def __init__(self, config):
@core.callback
def get(self, request, username):
"""Process a request to get the configuration."""
if not is_local(request[KEY_REAL_IP]):
if not is_local(ip_address(request.remote)):
return self.json_message("only local IPs allowed", HTTP_UNAUTHORIZED)
if username != HUE_API_USERNAME:
return self.json(UNAUTHORIZED_USER)
Expand All @@ -280,7 +280,7 @@ def __init__(self, config):
@core.callback
def get(self, request, username, entity_id):
"""Process a request to get the state of an individual light."""
if not is_local(request[KEY_REAL_IP]):
if not is_local(ip_address(request.remote)):
return self.json_message("Only local IPs allowed", HTTP_UNAUTHORIZED)

hass = request.app["hass"]
Expand Down Expand Up @@ -321,7 +321,7 @@ def __init__(self, config):

async def put(self, request, username, entity_number):
"""Process a request to set the state of an individual light."""
if not is_local(request[KEY_REAL_IP]):
if not is_local(ip_address(request.remote)):
return self.json_message("Only local IPs allowed", HTTP_UNAUTHORIZED)

config = self.config
Expand Down
6 changes: 3 additions & 3 deletions homeassistant/components/hassio/auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@

from homeassistant.auth.models import User
from homeassistant.components.http import HomeAssistantView
from homeassistant.components.http.const import KEY_HASS_USER, KEY_REAL_IP
from homeassistant.components.http.const import KEY_HASS_USER
from homeassistant.components.http.data_validator import RequestDataValidator
from homeassistant.const import HTTP_OK
from homeassistant.core import callback
Expand Down Expand Up @@ -63,8 +63,8 @@ def _check_access(self, request: web.Request):
"""Check if this call is from Supervisor."""
# Check caller IP
hassio_ip = os.environ["HASSIO"].split(":")[0]
if request[KEY_REAL_IP] != ip_address(hassio_ip):
_LOGGER.error("Invalid auth request from %s", request[KEY_REAL_IP])
if ip_address(request.remote) != ip_address(hassio_ip):
pvizeli marked this conversation as resolved.
Show resolved Hide resolved
_LOGGER.error("Invalid auth request from %s", request.remote)
raise HTTPUnauthorized()

# Check caller token
Expand Down
12 changes: 8 additions & 4 deletions homeassistant/components/http/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,9 @@

from .auth import setup_auth
from .ban import setup_bans
from .const import KEY_AUTHENTICATED, KEY_HASS, KEY_HASS_USER, KEY_REAL_IP # noqa: F401
from .const import KEY_AUTHENTICATED, KEY_HASS, KEY_HASS_USER # noqa: F401
from .cors import setup_cors
from .real_ip import setup_real_ip
from .forwarded import setup_forwarded
from .request_context import setup_request_context
from .static import CACHE_HEADERS, CachingStaticResource
from .view import HomeAssistantView # noqa: F401
Expand Down Expand Up @@ -296,9 +296,13 @@ def __init__(
)
app[KEY_HASS] = hass

# This order matters
# Order matters, forwarded middleware needs to go first.
# Only register middleware if `use_x_forwarded_for` is enabled
# and trusted proxies are provided
if use_x_forwarded_for and trusted_proxies:
setup_forwarded(app, trusted_proxies)

setup_request_context(app, current_request)
setup_real_ip(app, use_x_forwarded_for, trusted_proxies)

if is_ban_enabled:
setup_bans(hass, app, login_threshold)
Expand Down
4 changes: 2 additions & 2 deletions homeassistant/components/http/auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
from homeassistant.core import callback
from homeassistant.util import dt as dt_util

from .const import KEY_AUTHENTICATED, KEY_HASS_USER, KEY_REAL_IP
from .const import KEY_AUTHENTICATED, KEY_HASS_USER

# mypy: allow-untyped-defs, no-check-untyped-defs

Expand Down Expand Up @@ -118,7 +118,7 @@ async def auth_middleware(request, handler):
if authenticated:
_LOGGER.debug(
"Authenticated %s for %s using %s",
request[KEY_REAL_IP],
request.remote,
request.path,
auth_type,
)
Expand Down
8 changes: 3 additions & 5 deletions homeassistant/components/http/ban.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,6 @@
import homeassistant.helpers.config_validation as cv
from homeassistant.util.yaml import dump

from .const import KEY_REAL_IP

# mypy: allow-untyped-defs, no-check-untyped-defs

_LOGGER = logging.getLogger(__name__)
Expand Down Expand Up @@ -61,7 +59,7 @@ async def ban_middleware(request, handler):
return await handler(request)

# Verify if IP is not banned
ip_address_ = request[KEY_REAL_IP]
ip_address_ = ip_address(request.remote)
is_banned = any(
ip_ban.ip_address == ip_address_ for ip_ban in request.app[KEY_BANNED_IPS]
)
Expand Down Expand Up @@ -95,7 +93,7 @@ async def process_wrong_login(request):
Increase failed login attempts counter for remote IP address.
Add ip ban entry if failed login attempts exceeds threshold.
"""
remote_addr = request[KEY_REAL_IP]
remote_addr = ip_address(request.remote)

msg = f"Login attempt or request with invalid authentication from {remote_addr}"
_LOGGER.warning(msg)
Expand Down Expand Up @@ -144,7 +142,7 @@ async def process_success_login(request):
No release IP address from banned list function, it can only be done by
manual modify ip bans config file.
"""
remote_addr = request[KEY_REAL_IP]
remote_addr = ip_address(request.remote)

# Check if ban middleware is loaded
if KEY_BANNED_IPS not in request.app or request.app[KEY_LOGIN_THRESHOLD] < 1:
Expand Down
1 change: 0 additions & 1 deletion homeassistant/components/http/const.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,3 @@
KEY_AUTHENTICATED = "ha_authenticated"
KEY_HASS = "hass"
KEY_HASS_USER = "hass_user"
KEY_REAL_IP = "ha_real_ip"
Loading