From 1c2aa6638325ee0842b330b93aed2cfab4ca5151 Mon Sep 17 00:00:00 2001 From: Joachim Lusiardi Date: Wed, 6 Feb 2019 07:39:13 +0100 Subject: [PATCH 1/5] exchange tmp with decrypted as proposed by @koreth See https://github.com/jlusiardi/homekit_python/issues/89#issuecomment-460908543 --- homekit/http_impl/secure_http.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/homekit/http_impl/secure_http.py b/homekit/http_impl/secure_http.py index adcdb49c..10d03466 100644 --- a/homekit/http_impl/secure_http.py +++ b/homekit/http_impl/secure_http.py @@ -155,7 +155,7 @@ def _read_response(self, timeout=10): decrypted = self.decrypt_block(length, block, tag) # TODO how to react to False? - if tmp is not False: + if decrypted is not False: response.parse(decrypted) # check how long next block will be From 39fc3969d3a04b6aaa0389f0347c6ddefedd6c38 Mon Sep 17 00:00:00 2001 From: Joachim Lusiardi Date: Sat, 9 Feb 2019 21:09:06 +0100 Subject: [PATCH 2/5] adds closing the socket and an exception on transmission errors --- homekit/exceptions.py | 8 ++++++++ homekit/http_impl/secure_http.py | 3 +++ 2 files changed, 11 insertions(+) diff --git a/homekit/exceptions.py b/homekit/exceptions.py index 6b06e5e5..bd7027e9 100644 --- a/homekit/exceptions.py +++ b/homekit/exceptions.py @@ -163,6 +163,14 @@ def __init__(self, message): Exception.__init__(self, message) +class EncryptionError(HomeKitException): + """ + Used if during a transmission some errors occurred. + """ + def __init__(self, message): + Exception.__init__(self, message) + + class AccessoryDisconnectedError(HomeKitException): """ Used if a HomeKit disconnects part way through an operation or series of operations. diff --git a/homekit/http_impl/secure_http.py b/homekit/http_impl/secure_http.py index 10d03466..fc294f5b 100644 --- a/homekit/http_impl/secure_http.py +++ b/homekit/http_impl/secure_http.py @@ -157,6 +157,9 @@ def _read_response(self, timeout=10): # TODO how to react to False? if decrypted is not False: response.parse(decrypted) + else: + self.sock.close() + raise exceptions.EncryptionError('Error during transmission.') # check how long next block will be if int.from_bytes(tmp[0:2], 'little') < 1024: From 0bc82ffbbeb6c5408c1a4bfa63e1541a28882510 Mon Sep 17 00:00:00 2001 From: Joachim Lusiardi Date: Sun, 10 Feb 2019 17:17:10 +0100 Subject: [PATCH 3/5] add test for the case of transmission error --- homekit/exceptions.py | 5 +- homekit/http_impl/response.py | 7 +- homekit/http_impl/secure_http.py | 8 +- tests/__init__.py | 2 +- tests/secure_http_test.py | 149 +++++++++++++++++++++++++++++++ 5 files changed, 160 insertions(+), 11 deletions(-) create mode 100644 tests/secure_http_test.py diff --git a/homekit/exceptions.py b/homekit/exceptions.py index bd7027e9..c629600f 100644 --- a/homekit/exceptions.py +++ b/homekit/exceptions.py @@ -101,11 +101,12 @@ class InvalidError(ProtocolError): pass -class HttpException(HomeKitException): +class HttpException(Exception): """ Used within the HTTP Parser. """ - pass + def __init__(self, message): + Exception.__init__(self, message) class InvalidAuthTagError(ProtocolError): diff --git a/homekit/http_impl/response.py b/homekit/http_impl/response.py index d9980787..ba07ffaa 100644 --- a/homekit/http_impl/response.py +++ b/homekit/http_impl/response.py @@ -14,6 +14,7 @@ # limitations under the License. # +from homekit.exceptions import HttpException class HttpResponse(object): STATE_PRE_STATUS = 0 @@ -120,9 +121,3 @@ def get_http_name(self): if self.version is not None: return self.version.split('/')[0] return None - - -class HttpException(Exception): - def __init__(self): - pass - diff --git a/homekit/http_impl/secure_http.py b/homekit/http_impl/secure_http.py index fc294f5b..a049307b 100644 --- a/homekit/http_impl/secure_http.py +++ b/homekit/http_impl/secure_http.py @@ -31,6 +31,7 @@ class SecureHttp: """ class Wrapper: + # TODO not used anymore? def __init__(self, data): self.data = data @@ -38,6 +39,7 @@ def makefile(self, arg): return io.BytesIO(self.data) class HTTPResponseWrapper: + # TODO not used anymore? def __init__(self, data): self.data = data self.status = 200 @@ -45,7 +47,7 @@ def __init__(self, data): def read(self): return self.data - def __init__(self, session): + def __init__(self, session, timeout=10): """ Initializes the secure HTTP class. The required keys can be obtained with get_session_keys @@ -58,6 +60,7 @@ def __init__(self, session): self.c2a_key = session.c2a_key self.c2a_counter = 0 self.a2c_counter = 0 + self.timeout = timeout self.lock = threading.Lock() def get(self, target): @@ -89,12 +92,13 @@ def _handle_request(self, data): try: self.sock.send(len_bytes + ciper_and_mac[0] + ciper_and_mac[1]) - return self._read_response() + return self._read_response(self.timeout) except OSError as e: raise exceptions.AccessoryDisconnectedError(str(e)) @staticmethod def _parse(chunked_data): + # TODO not used anymore? splitter = b'\r\n' tmp = chunked_data.split(splitter, 1) length = int(tmp[0].decode(), 16) diff --git a/tests/__init__.py b/tests/__init__.py index 81e43fe8..4c2c5742 100644 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -23,4 +23,4 @@ from tests.characteristicsTypes_test import TestCharacteristicsTypes from tests.zeroconf_test import TestZeroconf from tests.controller_test import TestControllerPaired, TestControllerUnpaired - +from tests.secure_http_test import TestSecureHttp diff --git a/tests/secure_http_test.py b/tests/secure_http_test.py new file mode 100644 index 00000000..6cb4c014 --- /dev/null +++ b/tests/secure_http_test.py @@ -0,0 +1,149 @@ +# +# Copyright 2018 Joachim Lusiardi +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# + +import unittest +from unittest import mock +import socket + +from homekit.http_impl.secure_http import * +from homekit.exceptions import AccessoryDisconnectedError, EncryptionError +from homekit.crypto.chacha20poly1305 import chacha20_aead_encrypt, chacha20_aead_decrypt + + +class ResponseProvider(threading.Thread): + """ + + """ + + def __init__(self, sock, c2a_key, a2c_key, encryption_fail=False): + threading.Thread.__init__(self) + self.sock = sock + self.c2a_key = c2a_key + self.a2c_key = a2c_key + self.a2c_counter = 0 + self.c2a_counter = 0 + self.encryption_fail = encryption_fail + + def run(self): + # tmp = self.sock.recv(1024) + # length = int.from_bytes(tmp[0:2], 'little') + # tmp = tmp[2:] + # block = tmp[0:length] + # tmp = tmp[length:] + # tag = tmp[0:16] + # request = chacha20_aead_decrypt(length.to_bytes(2, byteorder='little'), + # self.c2a_key, + # self.c2a_counter.to_bytes(8, byteorder='little'), + # bytes([0, 0, 0, 0]), block + tag) + + self.c2a_counter += 1 + + data = 'HTTP/1.1 200 OK\r\nContent-Length: 0\r\n\r\n' + len_bytes = len(data).to_bytes(2, byteorder='little') + cnt_bytes = self.a2c_counter.to_bytes(8, byteorder='little') + self.a2c_counter += 1 + ciper_and_mac = chacha20_aead_encrypt(len_bytes, self.a2c_key, cnt_bytes, bytes([0, 0, 0, 0]), + data.encode()) + + if self.encryption_fail: + ciper_and_mac[0][0] = 0 + self.sock.send(len_bytes + ciper_and_mac[0] + ciper_and_mac[1]) + + +class TestSecureHttp(unittest.TestCase): + def test_get_on_disconnected_device(self): + with mock.patch('homekit.controller.Session') as session: + session.sock = socket.socket() + session.a2c_key = b'\x00' * 32 + session.c2a_key = b'\x00' * 32 + + sh = SecureHttp(session) + self.assertRaises(AccessoryDisconnectedError, sh.get, '/') + + def test_put_on_disconnected_device(self): + with mock.patch('homekit.controller.Session') as session: + session.sock = socket.socket() + session.a2c_key = b'\x00' * 32 + session.c2a_key = b'\x00' * 32 + + sh = SecureHttp(session) + self.assertRaises(AccessoryDisconnectedError, sh.put, '/', 'data') + + def test_post_on_disconnected_device(self): + with mock.patch('homekit.controller.Session') as session: + session.sock = socket.socket() + session.a2c_key = b'\x00' * 32 + session.c2a_key = b'\x00' * 32 + + sh = SecureHttp(session) + self.assertRaises(AccessoryDisconnectedError, sh.post, '/', 'data') + + def test_get_on_connected_device_timeout(self): + with mock.patch('homekit.controller.Session') as session: + controller_socket, accessory_socket = socket.socketpair() + + session.sock = controller_socket + + session.a2c_key = b'\x00' * 32 + session.c2a_key = b'\x00' * 32 + + sh = SecureHttp(session, timeout=1) + result = sh.get('/') + controller_socket.close() + accessory_socket.close() + self.assertIsNone(result.code) + + def test_get_on_connected_device(self): + controller_socket, accessory_socket = socket.socketpair() + + key_c2a = b'S2}\xb1}-l\n\x83\xe5}\'U\xc0\x1b\x0f\x08%X\xfdu\x1f\x9el/\x9bZ"\xec5\xa5P' + key_a2c = b'\x16\xab\xd3\xfe\x95{\xe56\x1fH\x81\xfd\x914\xa0@\xaa\x0e\xa6\xebw\xf2\xe3w:\x11/\x01\xbb;,\x1d' + + tthread = ResponseProvider(accessory_socket, key_c2a, key_a2c) + tthread.start() + + with mock.patch('homekit.controller.Session') as session: + session.sock = controller_socket + session.a2c_key = key_a2c + session.c2a_key = key_c2a + + sh = SecureHttp(session, timeout=10) + result = sh.get('/') + + controller_socket.close() + accessory_socket.close() + self.assertEqual(200, result.code) + self.assertEqual(b'', result.body) + + def test_get_on_connected_device_enc_fail(self): + controller_socket, accessory_socket = socket.socketpair() + + key_c2a = b'S2}\xb1}-l\n\x83\xe5}\'U\xc0\x1b\x0f\x08%X\xfdu\x1f\x9el/\x9bZ"\xec5\xa5P' + key_a2c = b'\x16\xab\xd3\xfe\x95{\xe56\x1fH\x81\xfd\x914\xa0@\xaa\x0e\xa6\xebw\xf2\xe3w:\x11/\x01\xbb;,\x1d' + + tthread = ResponseProvider(accessory_socket, key_c2a, key_a2c, encryption_fail=True) + tthread.start() + + with mock.patch('homekit.controller.Session') as session: + session.sock = controller_socket + session.a2c_key = key_a2c + session.c2a_key = key_c2a + + sh = SecureHttp(session, timeout=10) + self.assertRaises(EncryptionError, sh.get, '/') + + controller_socket.close() + accessory_socket.close() From 82c4735517cc6750e9f3cadd1bf7ea40dcc2d967 Mon Sep 17 00:00:00 2001 From: Joachim Lusiardi Date: Sun, 10 Feb 2019 19:43:35 +0100 Subject: [PATCH 4/5] add exception handling around close socket --- homekit/http_impl/secure_http.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/homekit/http_impl/secure_http.py b/homekit/http_impl/secure_http.py index a049307b..53b81145 100644 --- a/homekit/http_impl/secure_http.py +++ b/homekit/http_impl/secure_http.py @@ -158,11 +158,13 @@ def _read_response(self, timeout=10): tmp = tmp[16:] decrypted = self.decrypt_block(length, block, tag) - # TODO how to react to False? if decrypted is not False: response.parse(decrypted) else: - self.sock.close() + try: + self.sock.close() + except OSError: + pass raise exceptions.EncryptionError('Error during transmission.') # check how long next block will be From bddab3690909a689ed19c24283fa4a1ceee75be8 Mon Sep 17 00:00:00 2001 From: Joachim Lusiardi Date: Sun, 10 Feb 2019 20:02:28 +0100 Subject: [PATCH 5/5] add handlings for EncryptionError --- homekit/controller.py | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/homekit/controller.py b/homekit/controller.py index e8f9cb19..13e64d37 100644 --- a/homekit/controller.py +++ b/homekit/controller.py @@ -26,7 +26,8 @@ from homekit.zeroconf_impl import discover_homekit_devices, find_device_ip_and_port from homekit.protocol.statuscodes import HapStatusCodes from homekit.exceptions import AccessoryNotFoundError, ConfigLoadingError, UnknownError, UnpairedError, \ - AuthenticationError, ConfigSavingError, AlreadyPairedError, FormatError, AccessoryDisconnectedError + AuthenticationError, ConfigSavingError, AlreadyPairedError, FormatError, AccessoryDisconnectedError, \ + EncryptionError from homekit.http_impl.secure_http import SecureHttp from homekit.protocol import get_session_keys, perform_pair_setup from homekit.protocol.tlv import TLV, TlvParseException @@ -272,7 +273,7 @@ def list_accessories_and_characteristics(self): self.session = Session(self.pairing_data) try: response = self.session.get('/accessories') - except AccessoryDisconnectedError: + except (AccessoryDisconnectedError, EncryptionError): self.session.close() self.session = None raise @@ -305,7 +306,7 @@ def list_pairings(self): try: response = self.session.sec_http.post('/pairings', request_tlv.decode()) data = response.read() - except AccessoryDisconnectedError: + except (AccessoryDisconnectedError, EncryptionError): self.session.close() self.session = None raise @@ -373,7 +374,7 @@ def get_characteristics(self, characteristics, include_meta=False, include_perms try: response = self.session.get(url) - except AccessoryDisconnectedError: + except (AccessoryDisconnectedError, EncryptionError): self.session.close() self.session = None raise @@ -437,7 +438,7 @@ def put_characteristics(self, characteristics, do_conversion=False): try: response = self.session.put('/characteristics', data) - except AccessoryDisconnectedError: + except (AccessoryDisconnectedError, EncryptionError): self.session.close() self.session = None raise @@ -490,7 +491,7 @@ def get_events(self, characteristics, callback_fun, max_events=-1, max_seconds=- try: response = self.session.put('/characteristics', data) - except AccessoryDisconnectedError: + except (AccessoryDisconnectedError, EncryptionError): self.session.close() self.session = None raise @@ -521,7 +522,7 @@ def get_events(self, characteristics, callback_fun, max_events=-1, max_seconds=- try: r = self.session.sec_http.handle_event_response() body = r.read().decode() - except AccessoryDisconnectedError: + except (AccessoryDisconnectedError, EncryptionError): self.session.close() self.session = None raise