From 29712848ad423f52e42254a8344a3dc4e02788eb Mon Sep 17 00:00:00 2001 From: Robbie Trencheny Date: Sat, 9 Mar 2019 01:08:55 -0800 Subject: [PATCH 1/3] If registration supports encryption then require all payloads received are encrypted --- .../components/mobile_app/webhook.py | 18 ++++++++----- tests/components/mobile_app/__init__.py | 6 +++++ tests/components/mobile_app/const.py | 12 +++++++++ tests/components/mobile_app/test_webhook.py | 27 ++++++++++++++----- 4 files changed, 50 insertions(+), 13 deletions(-) diff --git a/homeassistant/components/mobile_app/webhook.py b/homeassistant/components/mobile_app/webhook.py index a5496c4395df4e..5e7c6125de3914 100644 --- a/homeassistant/components/mobile_app/webhook.py +++ b/homeassistant/components/mobile_app/webhook.py @@ -20,12 +20,12 @@ from homeassistant.helpers.storage import Store from homeassistant.helpers.typing import HomeAssistantType -from .const import (ATTR_APP_COMPONENT, DATA_DELETED_IDS, - ATTR_DEVICE_NAME, ATTR_EVENT_DATA, ATTR_EVENT_TYPE, - DATA_REGISTRATIONS, ATTR_TEMPLATE, ATTR_TEMPLATE_VARIABLES, - ATTR_WEBHOOK_DATA, ATTR_WEBHOOK_ENCRYPTED, - ATTR_WEBHOOK_ENCRYPTED_DATA, ATTR_WEBHOOK_TYPE, - CONF_SECRET, DOMAIN, WEBHOOK_PAYLOAD_SCHEMA, +from .const import (ATTR_APP_COMPONENT, ATTR_DEVICE_NAME, ATTR_EVENT_DATA, + ATTR_EVENT_TYPE, ATTR_SUPPORTS_ENCRYPTION, ATTR_TEMPLATE, + ATTR_TEMPLATE_VARIABLES, ATTR_WEBHOOK_DATA, + ATTR_WEBHOOK_ENCRYPTED, ATTR_WEBHOOK_ENCRYPTED_DATA, + ATTR_WEBHOOK_TYPE, CONF_SECRET, DATA_DELETED_IDS, + DATA_REGISTRATIONS, DOMAIN, WEBHOOK_PAYLOAD_SCHEMA, WEBHOOK_SCHEMAS, WEBHOOK_TYPE_CALL_SERVICE, WEBHOOK_TYPE_FIRE_EVENT, WEBHOOK_TYPE_RENDER_TEMPLATE, WEBHOOK_TYPE_UPDATE_LOCATION, @@ -77,6 +77,12 @@ async def handle_webhook(store: Store, hass: HomeAssistantType, _LOGGER.warning('Received invalid JSON from mobile_app') return empty_okay_response(status=HTTP_BAD_REQUEST) + if (req_data.get(ATTR_WEBHOOK_ENCRYPTED, False) is False and + registration[ATTR_SUPPORTS_ENCRYPTION]): + _LOGGER.warning("Refusing to accept unencrypted webhook from %s", + registration[ATTR_DEVICE_NAME]) + return empty_okay_response() + try: req_data = WEBHOOK_PAYLOAD_SCHEMA(req_data) except vol.Invalid as ex: diff --git a/tests/components/mobile_app/__init__.py b/tests/components/mobile_app/__init__.py index 02107eafb8110d..1f91eb7e442221 100644 --- a/tests/components/mobile_app/__init__.py +++ b/tests/components/mobile_app/__init__.py @@ -26,6 +26,12 @@ def webhook_client(hass, aiohttp_client, hass_storage, hass_admin_user): CONF_WEBHOOK_ID: 'mobile_app_test', 'device_name': 'Test Device', CONF_USER_ID: hass_admin_user.id, + }, + 'mobile_app_test_cleartext': { + 'supports_encryption': False, + CONF_WEBHOOK_ID: 'mobile_app_test_cleartext', + 'device_name': 'Test Device (Cleartext)', + CONF_USER_ID: hass_admin_user.id, } }, DATA_DELETED_IDS: [], diff --git a/tests/components/mobile_app/const.py b/tests/components/mobile_app/const.py index 423af7929a4c81..63b37932104bd3 100644 --- a/tests/components/mobile_app/const.py +++ b/tests/components/mobile_app/const.py @@ -32,6 +32,18 @@ 'supports_encryption': True } +REGISTER_CLEARTEXT = { + 'app_data': {'foo': 'bar'}, + 'app_id': 'io.homeassistant.mobile_app_test', + 'app_name': 'Mobile App Tests', + 'app_version': '1.0.0', + 'device_name': 'Test 1', + 'manufacturer': 'mobile_app', + 'model': 'Test', + 'os_version': '1.0', + 'supports_encryption': False +} + RENDER_TEMPLATE = { 'type': 'render_template', 'data': { diff --git a/tests/components/mobile_app/test_webhook.py b/tests/components/mobile_app/test_webhook.py index f2e838fb3cb373..bd628d48ddb43d 100644 --- a/tests/components/mobile_app/test_webhook.py +++ b/tests/components/mobile_app/test_webhook.py @@ -10,14 +10,14 @@ from . import authed_api_client, webhook_client # noqa: F401 -from .const import (CALL_SERVICE, FIRE_EVENT, REGISTER, RENDER_TEMPLATE, - UPDATE) +from .const import (CALL_SERVICE, FIRE_EVENT, REGISTER_CLEARTEXT, + RENDER_TEMPLATE, UPDATE) async def test_webhook_handle_render_template(webhook_client): # noqa: F811 """Test that we render templates properly.""" resp = await webhook_client.post( - '/api/webhook/mobile_app_test', + '/api/webhook/mobile_app_test_cleartext', json=RENDER_TEMPLATE ) @@ -32,7 +32,7 @@ async def test_webhook_handle_call_services(hass, webhook_client): # noqa: E501 calls = async_mock_service(hass, 'test', 'mobile_app') resp = await webhook_client.post( - '/api/webhook/mobile_app_test', + '/api/webhook/mobile_app_test_cleartext', json=CALL_SERVICE ) @@ -53,7 +53,7 @@ def store_event(event): hass.bus.async_listen('test_event', store_event) resp = await webhook_client.post( - '/api/webhook/mobile_app_test', + '/api/webhook/mobile_app_test_cleartext', json=FIRE_EVENT ) @@ -69,7 +69,7 @@ async def test_webhook_update_registration(webhook_client, hass_client): # noqa """Test that a we can update an existing registration via webhook.""" authed_api_client = await hass_client() # noqa: F811 register_resp = await authed_api_client.post( - '/api/mobile_app/registrations', json=REGISTER + '/api/mobile_app/registrations', json=REGISTER_CLEARTEXT ) assert register_resp.status == 201 @@ -96,7 +96,7 @@ async def test_webhook_update_registration(webhook_client, hass_client): # noqa async def test_webhook_returns_error_incorrect_json(webhook_client, caplog): # noqa: E501 F811 """Test that an error is returned when JSON is invalid.""" resp = await webhook_client.post( - '/api/webhook/mobile_app_test', + '/api/webhook/mobile_app_test_cleartext', data='not json' ) @@ -143,3 +143,16 @@ async def test_webhook_handle_decryption(webhook_client): # noqa: F811 json = await resp.json() assert json == {'rendered': 'Hello world'} + + +async def test_webhook_requires_encryption(webhook_client): # noqa: F811 + """Test that encrypted registrations only accept encrypted data.""" + resp = await webhook_client.post( + '/api/webhook/mobile_app_test', + json=RENDER_TEMPLATE + ) + + assert resp.status == 200 + + webhook_json = await resp.json() + assert webhook_json == {} From c45e160de91e81973a68c89d73808c4f7e04c88c Mon Sep 17 00:00:00 2001 From: Robbie Trencheny Date: Wed, 13 Mar 2019 15:04:42 -0700 Subject: [PATCH 2/3] Use error_response if payload isnt encrypted --- homeassistant/components/mobile_app/const.py | 1 + homeassistant/components/mobile_app/webhook.py | 11 ++++++----- tests/components/mobile_app/test_webhook.py | 6 ++++-- 3 files changed, 11 insertions(+), 7 deletions(-) diff --git a/homeassistant/components/mobile_app/const.py b/homeassistant/components/mobile_app/const.py index 60b4cde4708659..11b6f3e9865fb6 100644 --- a/homeassistant/components/mobile_app/const.py +++ b/homeassistant/components/mobile_app/const.py @@ -41,6 +41,7 @@ ATTR_WEBHOOK_ENCRYPTED_DATA = 'encrypted_data' ATTR_WEBHOOK_TYPE = 'type' +ERR_ENCRYPTION_REQUIRED = 'encryption_required' ERR_INVALID_COMPONENT = 'invalid_component' ERR_RENDER_FAILURE = 'render_failure' ERR_SAVE_FAILURE = 'save_failure' diff --git a/homeassistant/components/mobile_app/webhook.py b/homeassistant/components/mobile_app/webhook.py index d4a07825e4012f..5af3a8e49dc96b 100644 --- a/homeassistant/components/mobile_app/webhook.py +++ b/homeassistant/components/mobile_app/webhook.py @@ -25,10 +25,11 @@ ATTR_TEMPLATE_VARIABLES, ATTR_WEBHOOK_DATA, ATTR_WEBHOOK_ENCRYPTED, ATTR_WEBHOOK_ENCRYPTED_DATA, ATTR_WEBHOOK_TYPE, CONF_SECRET, DATA_DELETED_IDS, - DATA_REGISTRATIONS, DOMAIN, ERR_RENDER_FAILURE, - WEBHOOK_PAYLOAD_SCHEMA, WEBHOOK_SCHEMAS, - WEBHOOK_TYPE_CALL_SERVICE, WEBHOOK_TYPE_FIRE_EVENT, - WEBHOOK_TYPE_RENDER_TEMPLATE, WEBHOOK_TYPE_UPDATE_LOCATION, + DATA_REGISTRATIONS, DOMAIN, ERR_ENCRYPTION_REQUIRED, + ERR_RENDER_FAILURE, WEBHOOK_PAYLOAD_SCHEMA, + WEBHOOK_SCHEMAS, WEBHOOK_TYPE_CALL_SERVICE, + WEBHOOK_TYPE_FIRE_EVENT, WEBHOOK_TYPE_RENDER_TEMPLATE, + WEBHOOK_TYPE_UPDATE_LOCATION, WEBHOOK_TYPE_UPDATE_REGISTRATION) from .helpers import (_decrypt_payload, empty_okay_response, error_response, @@ -82,7 +83,7 @@ async def handle_webhook(store: Store, hass: HomeAssistantType, registration[ATTR_SUPPORTS_ENCRYPTION]): _LOGGER.warning("Refusing to accept unencrypted webhook from %s", registration[ATTR_DEVICE_NAME]) - return empty_okay_response() + return error_response(ERR_ENCRYPTION_REQUIRED, "Encryption required") try: req_data = WEBHOOK_PAYLOAD_SCHEMA(req_data) diff --git a/tests/components/mobile_app/test_webhook.py b/tests/components/mobile_app/test_webhook.py index db4c1a1f9e2dfa..bbdfcde93e731e 100644 --- a/tests/components/mobile_app/test_webhook.py +++ b/tests/components/mobile_app/test_webhook.py @@ -158,7 +158,9 @@ async def test_webhook_requires_encryption(webhook_client): # noqa: F811 json=RENDER_TEMPLATE ) - assert resp.status == 200 + assert resp.status == 400 webhook_json = await resp.json() - assert webhook_json == {} + assert 'error' in webhook_json + assert webhook_json['success'] is False + assert webhook_json['error']['code'] == 'encryption_required' From caaedb531c940e81c3232e1f1c9bbf0913187ed2 Mon Sep 17 00:00:00 2001 From: Robbie Trencheny Date: Wed, 13 Mar 2019 15:15:22 -0700 Subject: [PATCH 3/3] Fix encryption check conditional --- homeassistant/components/mobile_app/webhook.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/homeassistant/components/mobile_app/webhook.py b/homeassistant/components/mobile_app/webhook.py index 5af3a8e49dc96b..9efd1fcd9f8feb 100644 --- a/homeassistant/components/mobile_app/webhook.py +++ b/homeassistant/components/mobile_app/webhook.py @@ -79,7 +79,7 @@ async def handle_webhook(store: Store, hass: HomeAssistantType, _LOGGER.warning('Received invalid JSON from mobile_app') return empty_okay_response(status=HTTP_BAD_REQUEST) - if (req_data.get(ATTR_WEBHOOK_ENCRYPTED, False) is False and + if (ATTR_WEBHOOK_ENCRYPTED not in req_data and registration[ATTR_SUPPORTS_ENCRYPTION]): _LOGGER.warning("Refusing to accept unencrypted webhook from %s", registration[ATTR_DEVICE_NAME])