fix(security): authenticate health endpoint and stop webhook-id leak
The /station/health route is registered directly on the aiohttp router, so HA's auth middleware only flags requests without blocking them - the endpoint was reachable unauthenticated and returned the full health snapshot. - health_status now requires HA authentication (bearer token or signed request) via KEY_AUTHENTICATED and returns 401 otherwise. - Mask the Ecowitt webhook id (the endpoint's only credential) in last_ingress paths via _sanitize_path, so it never enters the snapshot exposed by the health endpoint or the diagnostics download. - Redact ECOWITT_WEBHOOK_ID in diagnostics. - Compare the Ecowitt webhook id in constant time (hmac.compare_digest), matching the WU/WSLink credential checks. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
parent
b3aec61fc2
commit
bf6a02c5b3
|
|
@ -115,7 +115,10 @@ class WeatherDataUpdateCoordinator(DataUpdateCoordinator):
|
||||||
expected_webhook = self.config.options.get(ECOWITT_WEBHOOK_ID, "")
|
expected_webhook = self.config.options.get(ECOWITT_WEBHOOK_ID, "")
|
||||||
actual_webhook = webdata.match_info.get("webhook_id", "")
|
actual_webhook = webdata.match_info.get("webhook_id", "")
|
||||||
|
|
||||||
if not expected_webhook or actual_webhook != expected_webhook:
|
# Constant-time comparison to avoid leaking the webhook id via timing.
|
||||||
|
if not expected_webhook or not hmac.compare_digest(
|
||||||
|
actual_webhook.encode("utf-8"), expected_webhook.encode("utf-8")
|
||||||
|
):
|
||||||
_LOGGER.error("Ecowitt: invalid webhook ID")
|
_LOGGER.error("Ecowitt: invalid webhook ID")
|
||||||
if health:
|
if health:
|
||||||
health.update_ingress_result(
|
health.update_ingress_result(
|
||||||
|
|
|
||||||
|
|
@ -8,12 +8,21 @@ from typing import Any
|
||||||
from homeassistant.components.diagnostics import async_redact_data # pyright: ignore[reportUnknownVariableType]
|
from homeassistant.components.diagnostics import async_redact_data # pyright: ignore[reportUnknownVariableType]
|
||||||
from homeassistant.core import HomeAssistant
|
from homeassistant.core import HomeAssistant
|
||||||
|
|
||||||
from .const import API_ID, API_KEY, POCASI_CZ_API_ID, POCASI_CZ_API_KEY, WINDY_STATION_ID, WINDY_STATION_PW
|
from .const import (
|
||||||
|
API_ID,
|
||||||
|
API_KEY,
|
||||||
|
ECOWITT_WEBHOOK_ID,
|
||||||
|
POCASI_CZ_API_ID,
|
||||||
|
POCASI_CZ_API_KEY,
|
||||||
|
WINDY_STATION_ID,
|
||||||
|
WINDY_STATION_PW,
|
||||||
|
)
|
||||||
from .data import SWSConfigEntry
|
from .data import SWSConfigEntry
|
||||||
|
|
||||||
TO_REDACT = {
|
TO_REDACT = {
|
||||||
API_ID,
|
API_ID,
|
||||||
API_KEY,
|
API_KEY,
|
||||||
|
ECOWITT_WEBHOOK_ID,
|
||||||
POCASI_CZ_API_ID,
|
POCASI_CZ_API_ID,
|
||||||
POCASI_CZ_API_KEY,
|
POCASI_CZ_API_KEY,
|
||||||
WINDY_STATION_ID,
|
WINDY_STATION_ID,
|
||||||
|
|
|
||||||
|
|
@ -23,8 +23,10 @@ from typing import Any
|
||||||
import aiohttp
|
import aiohttp
|
||||||
from aiohttp import ClientConnectionError
|
from aiohttp import ClientConnectionError
|
||||||
import aiohttp.web
|
import aiohttp.web
|
||||||
|
from aiohttp.web_exceptions import HTTPUnauthorized
|
||||||
from py_typecheck import checked, checked_or
|
from py_typecheck import checked, checked_or
|
||||||
|
|
||||||
|
from homeassistant.components.http import KEY_AUTHENTICATED
|
||||||
from homeassistant.components.network import async_get_source_ip
|
from homeassistant.components.network import async_get_source_ip
|
||||||
from homeassistant.core import HomeAssistant
|
from homeassistant.core import HomeAssistant
|
||||||
from homeassistant.helpers.aiohttp_client import async_get_clientsession
|
from homeassistant.helpers.aiohttp_client import async_get_clientsession
|
||||||
|
|
@ -69,6 +71,18 @@ def _protocol_from_path(path: str) -> str:
|
||||||
return "unknown"
|
return "unknown"
|
||||||
|
|
||||||
|
|
||||||
|
def _sanitize_path(path: str) -> str:
|
||||||
|
"""Strip the secret Ecowitt webhook id from a path before storing/exposing it.
|
||||||
|
|
||||||
|
The Ecowitt endpoint is `/weatherhub/<webhook_id>` where the id is the only
|
||||||
|
credential. Keeping the raw path in the health snapshot would leak it via the
|
||||||
|
health endpoint and diagnostics, so mask the id segment.
|
||||||
|
"""
|
||||||
|
if path.startswith(ECOWITT_URL_PREFIX + "/"):
|
||||||
|
return ECOWITT_URL_PREFIX + "/***"
|
||||||
|
return path
|
||||||
|
|
||||||
|
|
||||||
def _empty_forwarding_state(enabled: bool) -> dict[str, Any]:
|
def _empty_forwarding_state(enabled: bool) -> dict[str, Any]:
|
||||||
"""Build the default forwarding status payload."""
|
"""Build the default forwarding status payload."""
|
||||||
return {
|
return {
|
||||||
|
|
@ -269,7 +283,7 @@ class HealthCoordinator(DataUpdateCoordinator):
|
||||||
data["last_ingress"] = {
|
data["last_ingress"] = {
|
||||||
"time": dt_util.utcnow().isoformat(),
|
"time": dt_util.utcnow().isoformat(),
|
||||||
"protocol": _protocol_from_path(request.path),
|
"protocol": _protocol_from_path(request.path),
|
||||||
"path": request.path,
|
"path": _sanitize_path(request.path),
|
||||||
"method": request.method,
|
"method": request.method,
|
||||||
"route_enabled": route_enabled,
|
"route_enabled": route_enabled,
|
||||||
"accepted": False,
|
"accepted": False,
|
||||||
|
|
@ -294,7 +308,7 @@ class HealthCoordinator(DataUpdateCoordinator):
|
||||||
{
|
{
|
||||||
"time": dt_util.utcnow().isoformat(),
|
"time": dt_util.utcnow().isoformat(),
|
||||||
"protocol": _protocol_from_path(request.path),
|
"protocol": _protocol_from_path(request.path),
|
||||||
"path": request.path,
|
"path": _sanitize_path(request.path),
|
||||||
"method": request.method,
|
"method": request.method,
|
||||||
"accepted": accepted,
|
"accepted": accepted,
|
||||||
"authorized": authorized,
|
"authorized": authorized,
|
||||||
|
|
@ -326,11 +340,23 @@ class HealthCoordinator(DataUpdateCoordinator):
|
||||||
self._refresh_summary(data)
|
self._refresh_summary(data)
|
||||||
self._commit(data)
|
self._commit(data)
|
||||||
|
|
||||||
async def health_status(self, _: aiohttp.web.Request) -> aiohttp.web.Response:
|
async def health_status(self, request: aiohttp.web.Request) -> aiohttp.web.Response:
|
||||||
"""Serve the current health snapshot over HTTP.
|
"""Serve the current health snapshot over HTTP.
|
||||||
|
|
||||||
|
Requires Home Assistant authentication. The route is registered directly on
|
||||||
|
the aiohttp router (so it can share the dispatcher), which means HA's auth
|
||||||
|
middleware only *flags* the request - it does not block it. We therefore
|
||||||
|
enforce auth here so the snapshot (internal URLs/IPs, add-on status, last
|
||||||
|
ingress) is never exposed to unauthenticated callers.
|
||||||
|
|
||||||
|
Auth is satisfied by a valid bearer token or a signed request, the same as
|
||||||
|
any HomeAssistantView.
|
||||||
|
|
||||||
The endpoint forces one refresh before returning so that the caller sees
|
The endpoint forces one refresh before returning so that the caller sees
|
||||||
a reasonably fresh add-on status.
|
a reasonably fresh add-on status.
|
||||||
"""
|
"""
|
||||||
|
if not request.get(KEY_AUTHENTICATED, False):
|
||||||
|
raise HTTPUnauthorized
|
||||||
|
|
||||||
await self.async_request_refresh()
|
await self.async_request_refresh()
|
||||||
return aiohttp.web.json_response(self.data, status=200)
|
return aiohttp.web.json_response(self.data, status=200)
|
||||||
|
|
|
||||||
|
|
@ -24,6 +24,7 @@ from unittest.mock import AsyncMock, MagicMock
|
||||||
|
|
||||||
import aiohttp
|
import aiohttp
|
||||||
from aiohttp import ClientConnectionError
|
from aiohttp import ClientConnectionError
|
||||||
|
from aiohttp.web_exceptions import HTTPUnauthorized
|
||||||
import pytest
|
import pytest
|
||||||
from pytest_homeassistant_custom_component.common import MockConfigEntry
|
from pytest_homeassistant_custom_component.common import MockConfigEntry
|
||||||
|
|
||||||
|
|
@ -42,6 +43,7 @@ from custom_components.sws12500.const import (
|
||||||
from custom_components.sws12500.data import SWSRuntimeData
|
from custom_components.sws12500.data import SWSRuntimeData
|
||||||
from custom_components.sws12500.health_coordinator import HealthCoordinator
|
from custom_components.sws12500.health_coordinator import HealthCoordinator
|
||||||
from custom_components.sws12500.routes import Routes
|
from custom_components.sws12500.routes import Routes
|
||||||
|
from homeassistant.components.http import KEY_AUTHENTICATED
|
||||||
|
|
||||||
# ---------------------------------------------------------------------------
|
# ---------------------------------------------------------------------------
|
||||||
# Helpers / fixtures
|
# Helpers / fixtures
|
||||||
|
|
@ -441,6 +443,20 @@ def test_record_dispatch_records_with_reason(hass, entry) -> None:
|
||||||
assert coordinator.data["integration_status"] == "degraded"
|
assert coordinator.data["integration_status"] == "degraded"
|
||||||
|
|
||||||
|
|
||||||
|
def test_record_dispatch_masks_ecowitt_webhook_id(hass, entry) -> None:
|
||||||
|
coordinator = HealthCoordinator(hass, entry)
|
||||||
|
_attach_runtime_data(entry, coordinator)
|
||||||
|
|
||||||
|
request = SimpleNamespace(path=ECOWITT_URL_PREFIX + "/supersecretid", method="POST")
|
||||||
|
coordinator.record_dispatch(request, route_enabled=True, reason=None)
|
||||||
|
|
||||||
|
ingress = coordinator.data["last_ingress"]
|
||||||
|
assert ingress["protocol"] == "ecowitt"
|
||||||
|
# The secret webhook id must never reach the (potentially exposed) snapshot.
|
||||||
|
assert ingress["path"] == ECOWITT_URL_PREFIX + "/***"
|
||||||
|
assert "supersecretid" not in ingress["path"]
|
||||||
|
|
||||||
|
|
||||||
# ---------------------------------------------------------------------------
|
# ---------------------------------------------------------------------------
|
||||||
# update_ingress_result
|
# update_ingress_result
|
||||||
# ---------------------------------------------------------------------------
|
# ---------------------------------------------------------------------------
|
||||||
|
|
@ -520,21 +536,36 @@ def test_update_forwarding(hass, entry) -> None:
|
||||||
# ---------------------------------------------------------------------------
|
# ---------------------------------------------------------------------------
|
||||||
|
|
||||||
|
|
||||||
async def test_health_status_endpoint(hass, entry, monkeypatch) -> None:
|
async def test_health_status_endpoint_authenticated(hass, entry, monkeypatch) -> None:
|
||||||
coordinator = HealthCoordinator(hass, entry)
|
coordinator = HealthCoordinator(hass, entry)
|
||||||
_attach_runtime_data(entry, coordinator)
|
_attach_runtime_data(entry, coordinator)
|
||||||
|
|
||||||
# Avoid network: stub the refresh that health_status awaits.
|
# Avoid network: stub the refresh that health_status awaits.
|
||||||
monkeypatch.setattr(coordinator, "async_request_refresh", AsyncMock(return_value=None))
|
monkeypatch.setattr(coordinator, "async_request_refresh", AsyncMock(return_value=None))
|
||||||
|
|
||||||
request = SimpleNamespace(path=HEALTH_URL, method="GET")
|
# aiohttp Request is dict-like; health_status only reads KEY_AUTHENTICATED.
|
||||||
response = await coordinator.health_status(request)
|
request = {KEY_AUTHENTICATED: True}
|
||||||
|
response = await coordinator.health_status(request) # type: ignore[arg-type]
|
||||||
|
|
||||||
assert isinstance(response, aiohttp.web.Response)
|
assert isinstance(response, aiohttp.web.Response)
|
||||||
assert response.status == 200
|
assert response.status == 200
|
||||||
coordinator.async_request_refresh.assert_awaited_once()
|
coordinator.async_request_refresh.assert_awaited_once()
|
||||||
|
|
||||||
|
|
||||||
|
async def test_health_status_endpoint_rejects_unauthenticated(hass, entry, monkeypatch) -> None:
|
||||||
|
coordinator = HealthCoordinator(hass, entry)
|
||||||
|
_attach_runtime_data(entry, coordinator)
|
||||||
|
|
||||||
|
refresh = AsyncMock(return_value=None)
|
||||||
|
monkeypatch.setattr(coordinator, "async_request_refresh", refresh)
|
||||||
|
|
||||||
|
# No KEY_AUTHENTICATED flag -> unauthenticated -> 401, no refresh triggered.
|
||||||
|
with pytest.raises(HTTPUnauthorized):
|
||||||
|
await coordinator.health_status({}) # type: ignore[arg-type]
|
||||||
|
|
||||||
|
refresh.assert_not_awaited()
|
||||||
|
|
||||||
|
|
||||||
# ---------------------------------------------------------------------------
|
# ---------------------------------------------------------------------------
|
||||||
# health_sensor.py module helpers
|
# health_sensor.py module helpers
|
||||||
# ---------------------------------------------------------------------------
|
# ---------------------------------------------------------------------------
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue